[PATCH] scsi: ufs: Use wait-for-reg in HCE init

Avri Altman posted 1 patch 1 month, 1 week ago
There is a newer version of this series
drivers/ufs/core/ufshcd.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
[PATCH] scsi: ufs: Use wait-for-reg in HCE init
Posted by Avri Altman 1 month, 1 week ago
Instead of open-coding it.  Code simplification - no functional change.

Signed-off-by: Avri Altman <avri.altman@wdc.com>
---
 drivers/ufs/core/ufshcd.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 9e6d008f4ea4..f23d37c227e1 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -4818,8 +4818,7 @@ EXPORT_SYMBOL_GPL(ufshcd_hba_stop);
  */
 static int ufshcd_hba_execute_hce(struct ufs_hba *hba)
 {
-	int retry_outer = 3;
-	int retry_inner;
+	int retry = 3;
 
 start:
 	if (ufshcd_is_hba_active(hba))
@@ -4847,22 +4846,20 @@ static int ufshcd_hba_execute_hce(struct ufs_hba *hba)
 	ufshcd_delay_us(hba->vps->hba_enable_delay_us, 100);
 
 	/* wait for the host controller to complete initialization */
-	retry_inner = 50;
-	while (!ufshcd_is_hba_active(hba)) {
-		if (retry_inner) {
-			retry_inner--;
+	while (retry) {
+		if (!ufshcd_wait_for_register(hba, REG_CONTROLLER_ENABLE, CONTROLLER_ENABLE,
+					      CONTROLLER_ENABLE, 1000, 50)) {
+			break;
 		} else {
-			dev_err(hba->dev,
-				"Controller enable failed\n");
-			if (retry_outer) {
-				retry_outer--;
-				goto start;
-			}
-			return -EIO;
+			dev_err(hba->dev, "Controller enable failed\n");
+			retry--;
+			goto start;
 		}
-		usleep_range(1000, 1100);
 	}
 
+	if (!retry)
+		return -EIO;
+
 	/* enable UIC related interrupts */
 	ufshcd_enable_intr(hba, UFSHCD_UIC_MASK);
 
-- 
2.25.1
Re: [PATCH] scsi: ufs: Use wait-for-reg in HCE init
Posted by Bart Van Assche 1 month, 1 week ago
On 10/14/24 9:57 PM, Avri Altman wrote:
> Instead of open-coding it.  Code simplification - no functional change.

Please use full sentences in the patch description.

> +	while (retry) {

Please change this loop from a while-loop into a for-loop.

> +		if (!ufshcd_wait_for_register(hba, REG_CONTROLLER_ENABLE, CONTROLLER_ENABLE,
> +					      CONTROLLER_ENABLE, 1000, 50)) {
> +			break;
>   		} else {

"else" is not needed after a "break" statement, isn't it?

> +			dev_err(hba->dev, "Controller enable failed\n");

Please fix the grammar of this message, e.g. by changing this message
into "Enabling controller failed" or "Enabling the controller failed".

Thanks,

Bart.