| Message ID | 20241125125338.905146-1-vamshigajjela@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | scsi: ufs: core: Fix link_startup_again on success | expand |
On 11/25/24 4:53 AM, Vamshi Gajjela wrote: > Set link_startup_again to false after a successful > ufshcd_dme_link_startup operation and confirmation of device presence. > Prevents unnecessary link startup attempts when the previous operation > has succeeded. > > Signed-off-by: Vamshi Gajjela <vamshigajjela@google.com> > Fixes: 7caf489b99a4 ("scsi: ufs: issue link starup 2 times if device isn't active") > Cc: stable@vger.kernel.org Shouldn't your Signed-off-by come after the Fixes: and Cc: stable tags? Anyway, since this patch looks good to me: Reviewed-by: Bart Van Assche <bvanassche@acm.org>
On Mon, Nov 25, 2024 at 06:23:37PM +0530, Vamshi Gajjela wrote: > Set link_startup_again to false after a successful > ufshcd_dme_link_startup operation and confirmation of device presence. > Prevents unnecessary link startup attempts when the previous operation > has succeeded. > > Signed-off-by: Vamshi Gajjela <vamshigajjela@google.com> > Fixes: 7caf489b99a4 ("scsi: ufs: issue link starup 2 times if device isn't active") > Cc: stable@vger.kernel.org > --- > drivers/ufs/core/ufshcd.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > index abbe7135a977..cc1d15002ab5 100644 > --- a/drivers/ufs/core/ufshcd.c > +++ b/drivers/ufs/core/ufshcd.c > @@ -4994,6 +4994,10 @@ static int ufshcd_link_startup(struct ufs_hba *hba) > goto out; > } > > + /* link_startup success and device is present */ > + if (!ret && ufshcd_is_device_present(hba)) > + link_startup_again = false; > + Using 'link_startup_again' flag looks messy. Can't we just check the device state after first link startup and if it is still not active, then try link startup one more time? (Assuming that the device state won't be active after first link startup). Also, we should move the link startup and its associated check to a helper function and call them instead of (ab)using the goto label. Like, ret = __ufshcd_link_startup() if (ret) goto fail /* try link startup one more time if the device is not active */ if (!ufshcd_is_ufs_dev_active()) { ret = __ufshcd_link_startup() if (ret) goto fail } - Mani > /* > * DME link lost indication is only received when link is up, > * but we can't be sure if the link is up until link startup > -- > 2.47.0.371.ga323438b13-goog >
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index abbe7135a977..cc1d15002ab5 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -4994,6 +4994,10 @@ static int ufshcd_link_startup(struct ufs_hba *hba) goto out; } + /* link_startup success and device is present */ + if (!ret && ufshcd_is_device_present(hba)) + link_startup_again = false; + /* * DME link lost indication is only received when link is up, * but we can't be sure if the link is up until link startup
Set link_startup_again to false after a successful ufshcd_dme_link_startup operation and confirmation of device presence. Prevents unnecessary link startup attempts when the previous operation has succeeded. Signed-off-by: Vamshi Gajjela <vamshigajjela@google.com> Fixes: 7caf489b99a4 ("scsi: ufs: issue link starup 2 times if device isn't active") Cc: stable@vger.kernel.org --- drivers/ufs/core/ufshcd.c | 4 ++++ 1 file changed, 4 insertions(+)