Re: [PATCH v5 04/15] scsi: ufs: verify hba controller hce reg value
From: Hannes Reinecke
Date: Tue Mar 01 2016 - 02:33:21 EST
On 02/28/2016 09:32 PM, Yaniv Gardi wrote:
> Sometimes due to hw issues it takes some time to the
> host controller register to update. In order to verify the register
> has updated, a polling is done until its value is set.
>
> In addition the functions ufshcd_hba_stop() and
> ufshcd_wait_for_register() was updated with an additional input
> parameter, indicating the timeout between reads will
> be done by sleeping or spinning the cpu.
>
> Signed-off-by: Raviv Shvili <rshvili@xxxxxxxxxxxxxx>
> Signed-off-by: Yaniv Gardi <ygardi@xxxxxxxxxxxxxx>
>
> ---
> drivers/scsi/ufs/ufshcd.c | 53 ++++++++++++++++++++++++++++-------------------
> drivers/scsi/ufs/ufshcd.h | 12 +++--------
> 2 files changed, 35 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 3400ceb..80031e6 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -240,11 +240,13 @@ static inline void ufshcd_disable_irq(struct ufs_hba *hba)
> * @val - wait condition
> * @interval_us - polling interval in microsecs
> * @timeout_ms - timeout in millisecs
> + * @can_sleep - perform sleep or just spin
> *
> * Returns -ETIMEDOUT on error, zero on success
> */
> -static int ufshcd_wait_for_register(struct ufs_hba *hba, u32 reg, u32 mask,
> - u32 val, unsigned long interval_us, unsigned long timeout_ms)
> +int ufshcd_wait_for_register(struct ufs_hba *hba, u32 reg, u32 mask,
> + u32 val, unsigned long interval_us,
> + unsigned long timeout_ms, bool can_sleep)
> {
> int err = 0;
> unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
> @@ -253,9 +255,10 @@ static int ufshcd_wait_for_register(struct ufs_hba *hba, u32 reg, u32 mask,
> val = val & mask;
>
> while ((ufshcd_readl(hba, reg) & mask) != val) {
> - /* wakeup within 50us of expiry */
> - usleep_range(interval_us, interval_us + 50);
> -
> + if (can_sleep)
> + usleep_range(interval_us, interval_us + 50);
> + else
> + udelay(interval_us);
> if (time_after(jiffies, timeout)) {
> if ((ufshcd_readl(hba, reg) & mask) != val)
> err = -ETIMEDOUT;
> @@ -1459,7 +1462,7 @@ ufshcd_clear_cmd(struct ufs_hba *hba, int tag)
> */
> err = ufshcd_wait_for_register(hba,
> REG_UTP_TRANSFER_REQ_DOOR_BELL,
> - mask, ~mask, 1000, 1000);
> + mask, ~mask, 1000, 1000, true);
>
> return err;
> }
> @@ -2815,6 +2818,23 @@ out:
> }
>
> /**
> + * ufshcd_hba_stop - Send controller to reset state
> + * @hba: per adapter instance
> + * @can_sleep: perform sleep or just spin
> + */
> +static inline void ufshcd_hba_stop(struct ufs_hba *hba, bool can_sleep)
> +{
> + int err;
> +
> + ufshcd_writel(hba, CONTROLLER_DISABLE, REG_CONTROLLER_ENABLE);
> + err = ufshcd_wait_for_register(hba, REG_CONTROLLER_ENABLE,
> + CONTROLLER_ENABLE, CONTROLLER_DISABLE,
> + 10, 1, can_sleep);
> + if (err)
> + dev_err(hba->dev, "%s: Controller disable failed\n", __func__);
> +}
> +
Shouldn't you return an error here?
If the controller disable failed you probably need a hard reset or
something, otherwise I would assume that every other command from that
point on will not work as expected.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@xxxxxxx +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)