| Message ID | 20241015045711.394434-1-avri.altman@wdc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | scsi: ufs: Use wait-for-reg in HCE init | expand |
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.
> 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. Done. > > > + while (retry) { > > Please change this loop from a while-loop into a for-loop. Done. > > > + 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? Done. > > > + 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". Done. Thanks, Avri > > Thanks, > > Bart.
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);
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(-)