| Message ID | 20221027062013epcms2p7af8fbc60caaba50686233492fb3dfd93@epcms2p7 (mailing list archive) |
|---|---|
| State | Deferred |
| Headers | show |
| Series | scsi: ufs: core: Use 'else' in ufshcd_hba_enable() | expand |
On 10/26/22 23:20, Keoseong Park wrote: > Convert if (!ret) { ... } if (ret) { ... } into > if (!ret) { ... } else { ... }. > > Signed-off-by: Keoseong Park <keosung.park@samsung.com> > --- > drivers/ufs/core/ufshcd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > index 7256e6c43ca6..3b794be53287 100644 > --- a/drivers/ufs/core/ufshcd.c > +++ b/drivers/ufs/core/ufshcd.c > @@ -4670,7 +4670,7 @@ int ufshcd_hba_enable(struct ufs_hba *hba) > ret = ufshcd_dme_enable(hba); > if (!ret) > ufshcd_vops_hce_enable_notify(hba, POST_CHANGE); > - if (ret) > + else > dev_err(hba->dev, > "Host controller enable failed with non-hce\n"); > } If ufshcd_hba_enable() is modified it should be converted into the widely used "if error return" style. How about the untested patch below? diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 05feec10fad3..89f8b86c3f85 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -4668,14 +4668,16 @@ int ufshcd_hba_enable(struct ufs_hba *hba) /* enable UIC related interrupts */ ufshcd_enable_intr(hba, UFSHCD_UIC_MASK); ret = ufshcd_dme_reset(hba); - if (!ret) { - ret = ufshcd_dme_enable(hba); - if (!ret) - ufshcd_vops_hce_enable_notify(hba, POST_CHANGE); - if (ret) - dev_err(hba->dev, - "Host controller enable failed with non-hce\n"); + if (ret) { + dev_err(hba->dev, "DME_RESET failed\n"); + return ret; + } + ret = ufshcd_dme_enable(hba); + if (ret) { + dev_err(hba->dev, "Enabling DME failed\n"); + return ret; } + ufshcd_vops_hce_enable_notify(hba, POST_CHANGE); } else { ret = ufshcd_hba_execute_hce(hba); }
Hi Bart, >On 10/26/22 23:20, Keoseong Park wrote: >> Convert if (!ret) { ... } if (ret) { ... } into >> if (!ret) { ... } else { ... }. >> >> Signed-off-by: Keoseong Park <keosung.park@samsung.com> >> --- >> drivers/ufs/core/ufshcd.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c >> index 7256e6c43ca6..3b794be53287 100644 >> --- a/drivers/ufs/core/ufshcd.c >> +++ b/drivers/ufs/core/ufshcd.c >> @@ -4670,7 +4670,7 @@ int ufshcd_hba_enable(struct ufs_hba *hba) >> ret = ufshcd_dme_enable(hba); >> if (!ret) >> ufshcd_vops_hce_enable_notify(hba, POST_CHANGE); >> - if (ret) >> + else >> dev_err(hba->dev, >> "Host controller enable failed with non-hce\n"); >> } > >If ufshcd_hba_enable() is modified it should be converted into the widely >used "if error return" style. How about the untested patch below? Thanks for your feedback. That seems to be better. I will apply it to the next version. Best Regards, Keoseong > >diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c >index 05feec10fad3..89f8b86c3f85 100644 >--- a/drivers/ufs/core/ufshcd.c >+++ b/drivers/ufs/core/ufshcd.c >@@ -4668,14 +4668,16 @@ int ufshcd_hba_enable(struct ufs_hba *hba) > /* enable UIC related interrupts */ > ufshcd_enable_intr(hba, UFSHCD_UIC_MASK); > ret = ufshcd_dme_reset(hba); >- if (!ret) { >- ret = ufshcd_dme_enable(hba); >- if (!ret) >- ufshcd_vops_hce_enable_notify(hba, POST_CHANGE); >- if (ret) >- dev_err(hba->dev, >- "Host controller enable failed with non-hce\n"); >+ if (ret) { >+ dev_err(hba->dev, "DME_RESET failed\n"); >+ return ret; >+ } >+ ret = ufshcd_dme_enable(hba); >+ if (ret) { >+ dev_err(hba->dev, "Enabling DME failed\n"); >+ return ret; > } >+ ufshcd_vops_hce_enable_notify(hba, POST_CHANGE); > } else { > ret = ufshcd_hba_execute_hce(hba); > }
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 7256e6c43ca6..3b794be53287 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -4670,7 +4670,7 @@ int ufshcd_hba_enable(struct ufs_hba *hba) ret = ufshcd_dme_enable(hba); if (!ret) ufshcd_vops_hce_enable_notify(hba, POST_CHANGE); - if (ret) + else dev_err(hba->dev, "Host controller enable failed with non-hce\n"); }
Convert if (!ret) { ... } if (ret) { ... } into if (!ret) { ... } else { ... }. Signed-off-by: Keoseong Park <keosung.park@samsung.com> --- drivers/ufs/core/ufshcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)