| Message ID | 20240528-topic-sm8x50-ufs-core-link-startup-again-v1-1-146ca43e80b6@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ufs: core: remove link_startup_again logic | expand |
Hi Neil, On 28/05/24 14:06, Neil Armstrong wrote: > The link_startup_again logic was added in Linux to handle device > that were set in LinkDown state, which should not be the case since U-boot > doesn't set LinkDown state are init, and Linux sets the device active s/are/at ? > in ufshcd_init() for the first link startup. > > While it worked to far, it breaks link startup for Qualcomm Controllers v5, > let's just remove the logic. > I don't see any repercussions ATM, but will need to get a wider opinion with different controllers before acking. > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > drivers/ufs/ufs.c | 8 -------- > 1 file changed, 8 deletions(-) > > diff --git a/drivers/ufs/ufs.c b/drivers/ufs/ufs.c > index be64bf971f1..28c244b70e5 100644 > --- a/drivers/ufs/ufs.c > +++ b/drivers/ufs/ufs.c > @@ -456,9 +456,7 @@ static int ufshcd_link_startup(struct ufs_hba *hba) > { > int ret; > int retries = DME_LINKSTARTUP_RETRIES; > - bool link_startup_again = true; > > -link_startup: > do { > ufshcd_ops_link_startup_notify(hba, PRE_CHANGE); > > @@ -484,12 +482,6 @@ link_startup: > /* failed to get the link up... retire */ > goto out; > > - if (link_startup_again) { > - link_startup_again = false; > - retries = DME_LINKSTARTUP_RETRIES; > - goto link_startup; > - } > - > /* Mark that link is up in PWM-G1, 1-lane, SLOW-AUTO mode */ > ufshcd_init_pwr_info(hba); > > > --- > base-commit: 7e52d6ccfb76e2afc2d183b357abe2a2e2f948cf > change-id: 20240528-topic-sm8x50-ufs-core-link-startup-again-bc2cf907c164 > > Best regards,
Hi, On Tue, 28 May 2024 10:36:21 +0200, Neil Armstrong wrote: > The link_startup_again logic was added in Linux to handle device > that were set in LinkDown state, which should not be the case since U-boot > doesn't set LinkDown state are init, and Linux sets the device active > in ufshcd_init() for the first link startup. > > While it worked to far, it breaks link startup for Qualcomm Controllers v5, > let's just remove the logic. > > [...] Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-ufs (u-boot-ufs-next) [1/1] ufs: core: remove link_startup_again logic https://source.denx.de/u-boot/custodians/u-boot-ufs/-/commit/67e291d147cd40c9b638515dc1bfa3c52d390959
diff --git a/drivers/ufs/ufs.c b/drivers/ufs/ufs.c index be64bf971f1..28c244b70e5 100644 --- a/drivers/ufs/ufs.c +++ b/drivers/ufs/ufs.c @@ -456,9 +456,7 @@ static int ufshcd_link_startup(struct ufs_hba *hba) { int ret; int retries = DME_LINKSTARTUP_RETRIES; - bool link_startup_again = true; -link_startup: do { ufshcd_ops_link_startup_notify(hba, PRE_CHANGE); @@ -484,12 +482,6 @@ link_startup: /* failed to get the link up... retire */ goto out; - if (link_startup_again) { - link_startup_again = false; - retries = DME_LINKSTARTUP_RETRIES; - goto link_startup; - } - /* Mark that link is up in PWM-G1, 1-lane, SLOW-AUTO mode */ ufshcd_init_pwr_info(hba);
The link_startup_again logic was added in Linux to handle device that were set in LinkDown state, which should not be the case since U-boot doesn't set LinkDown state are init, and Linux sets the device active in ufshcd_init() for the first link startup. While it worked to far, it breaks link startup for Qualcomm Controllers v5, let's just remove the logic. Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> --- drivers/ufs/ufs.c | 8 -------- 1 file changed, 8 deletions(-) --- base-commit: 7e52d6ccfb76e2afc2d183b357abe2a2e2f948cf change-id: 20240528-topic-sm8x50-ufs-core-link-startup-again-bc2cf907c164 Best regards,