Commit 5cac1095 authored by Bart Van Assche's avatar Bart Van Assche Committed by Martin K. Petersen
Browse files

scsi: ufs: Make ufshcd_wait_for_register() sleep instead of busy-waiting 

The ufshcd_wait_for_register() function either sleeps or spins until the
specified register has reached the desired value. Busy-waiting is not only
considered a bad practice but also has a bad impact on energy
consumption. Always sleep instead of spinning by making sure that all
ufshcd_wait_for_register() calls happen from a context where it is allowed
to sleep. The only function call that has to be moved is the
ufshcd_hba_stop() call in ufshcd_host_reset_and_restore().

Link: https://lore.kernel.org/r/20200507222750.19113-1-bvanassche@acm.org


Cc: Can Guo <cang@codeaurora.org>
Cc: Avri Altman <avri.altman@wdc.com>
Cc: Bean Huo <beanhuo@micron.com>
Cc: Alim Akhtar <alim.akhtar@samsung.com>
Cc: Asutosh Das <asutoshd@codeaurora.org>
Tested-by: default avatarBean Huo <beanhuo@micron.com>
Reviewed-by: default avatarStanley Chu <stanley.chu@mediatek.com>
Reviewed-by: default avatarBean Huo <beanhuo@micron.com>
Reviewed-by: default avatarAsutosh Das <asutoshd@codeaurora.org>
Signed-off-by: default avatarBart Van Assche <bvanassche@acm.org>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
parent d0b1e4a6

drivers/scsi/ufs/ufshcd.c

+28 −23
Original line number Diff line number Diff line
@@ -562,21 +562,21 @@ void ufshcd_delay_us(unsigned long us, unsigned long tolerance) 
}

EXPORT_SYMBOL_GPL(ufshcd_delay_us);



/*

/**

 * ufshcd_wait_for_register - wait for register value to change

 * @hba - per-adapter interface

 * @reg - mmio register offset

 * @mask - mask to apply to read register value

 * @val - wait condition

 * @interval_us - polling interval in microsecs

 * @timeout_ms - timeout in millisecs

 * @can_sleep - perform sleep or just spin

 * @hba: per-adapter interface

 * @reg: mmio register offset

 * @mask: mask to apply to the read register value

 * @val: value to wait for

 * @interval_us: polling interval in microseconds

 * @timeout_ms: timeout in milliseconds

 *

 * Returns -ETIMEDOUT on error, zero on success

 * Return:

 * -ETIMEDOUT on error, zero on success.

 */

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)

				unsigned long timeout_ms)

{

	int err = 0;

	unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
@@ -585,10 +585,7 @@ int ufshcd_wait_for_register(struct ufs_hba *hba, u32 reg, u32 mask, 
	val = val & mask;



	while ((ufshcd_readl(hba, reg) & mask) != val) {

		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;
@@ -2577,7 +2574,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, true);

			mask, ~mask, 1000, 1000);



	return err;

}
@@ -4247,16 +4244,23 @@ EXPORT_SYMBOL_GPL(ufshcd_make_hba_operational); 
/**

 * 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)

static inline void ufshcd_hba_stop(struct ufs_hba *hba)

{

	unsigned long flags;

	int err;



	/*

	 * Obtain the host lock to prevent that the controller is disabled

	 * while the UFS interrupt handler is active on another CPU.

	 */

	spin_lock_irqsave(hba->host->host_lock, flags);

	ufshcd_writel(hba, CONTROLLER_DISABLE,  REG_CONTROLLER_ENABLE);

	spin_unlock_irqrestore(hba->host->host_lock, flags);



	err = ufshcd_wait_for_register(hba, REG_CONTROLLER_ENABLE,

					CONTROLLER_ENABLE, CONTROLLER_DISABLE,

					10, 1, can_sleep);

					10, 1);

	if (err)

		dev_err(hba->dev, "%s: Controller disable failed\n", __func__);

}
@@ -4277,7 +4281,7 @@ int ufshcd_hba_enable(struct ufs_hba *hba) 


	if (!ufshcd_is_hba_active(hba))

		/* change controller state to "reset state" */

		ufshcd_hba_stop(hba, true);

		ufshcd_hba_stop(hba);



	/* UniPro link is disabled at this point */

	ufshcd_set_link_off(hba);
@@ -5906,7 +5910,7 @@ static int ufshcd_clear_tm_cmd(struct ufs_hba *hba, int tag) 
	/* poll for max. 1 sec to clear door bell register by h/w */

	err = ufshcd_wait_for_register(hba,

			REG_UTP_TASK_REQ_DOOR_BELL,

			mask, 0, 1000, 1000, true);

			mask, 0, 1000, 1000);

out:

	return err;

}
@@ -6482,8 +6486,9 @@ static int ufshcd_host_reset_and_restore(struct ufs_hba *hba) 
	 * Stop the host controller and complete the requests

	 * cleared by h/w

	 */

	ufshcd_hba_stop(hba);



	spin_lock_irqsave(hba->host->host_lock, flags);

	ufshcd_hba_stop(hba, false);

	hba->silence_err_logs = true;

	ufshcd_complete_requests(hba);

	hba->silence_err_logs = false;
@@ -8049,7 +8054,7 @@ static int ufshcd_link_state_transition(struct ufs_hba *hba, 
		 * Change controller state to "reset state" which

		 * should also put the link in off/reset state

		 */

		ufshcd_hba_stop(hba, true);

		ufshcd_hba_stop(hba);

		/*

		 * TODO: Check if we need any delay to make sure that

		 * controller is reset
@@ -8608,7 +8613,7 @@ void ufshcd_remove(struct ufs_hba *hba) 
	scsi_remove_host(hba->host);

	/* disable interrupts */

	ufshcd_disable_intr(hba, hba->intr_mask);

	ufshcd_hba_stop(hba, true);

	ufshcd_hba_stop(hba);



	ufshcd_exit_clk_scaling(hba);

	ufshcd_exit_clk_gating(hba);

drivers/scsi/ufs/ufshcd.h

+1 −1
Original line number Diff line number Diff line
@@ -831,7 +831,7 @@ int ufshcd_uic_hibern8_exit(struct ufs_hba *hba); 
void ufshcd_delay_us(unsigned long us, unsigned long tolerance);

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);

				unsigned long timeout_ms);

void ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba, struct clk *refclk);

void ufshcd_update_reg_hist(struct ufs_err_reg_hist *reg_hist,

			    u32 reg);