scsi: ufs: qcom: avoid re-init quirk when gears match

Message ID 20240119185537.3091366-11-echanude@redhat.com
State New
Headers
Series scsi: ufs: qcom: avoid re-init quirk when gears match |

Commit Message

Eric Chanudet Jan. 19, 2024, 6:55 p.m. UTC
  On sa8775p-ride, probing the hba will go through the
UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH path although the power info
are same during the second init.

If the host is at least v4, ufs_qcom_get_hs_gear() picked the highest
supported gear when setting the host_params. After the negotiation, if
the host and device are on the same gear, it is the highest gear
supported between the two. Skip the re-init to save some time.

Signed-off-by: Eric Chanudet <echanude@redhat.com>
---

"trace_event=ufs:ufshcd_init" reports the time spent where the re-init
quirk is performed. On sa8775p-ride:
Baseline:
  0.355879: ufshcd_init: 1d84000.ufs: took 103377 usecs, dev_state: UFS_ACTIVE_PWR_MODE, link_state: UIC_LINK_ACTIVE_STATE, err 0
With this patch:
  0.297676: ufshcd_init: 1d84000.ufs: took 43553 usecs, dev_state: UFS_ACTIVE_PWR_MODE, link_state: UIC_LINK_ACTIVE_STATE, err 0

 drivers/ufs/host/ufs-qcom.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
  

Comments

Andrew Halaney Jan. 19, 2024, 8:07 p.m. UTC | #1
On Fri, Jan 19, 2024 at 01:55:47PM -0500, Eric Chanudet wrote:
> On sa8775p-ride, probing the hba will go through the
> UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH path although the power info
> are same during the second init.
> 
> If the host is at least v4, ufs_qcom_get_hs_gear() picked the highest
> supported gear when setting the host_params. After the negotiation, if
> the host and device are on the same gear, it is the highest gear
> supported between the two. Skip the re-init to save some time.
> 
> Signed-off-by: Eric Chanudet <echanude@redhat.com>
> ---
> 
> "trace_event=ufs:ufshcd_init" reports the time spent where the re-init
> quirk is performed. On sa8775p-ride:
> Baseline:
>   0.355879: ufshcd_init: 1d84000.ufs: took 103377 usecs, dev_state: UFS_ACTIVE_PWR_MODE, link_state: UIC_LINK_ACTIVE_STATE, err 0
> With this patch:
>   0.297676: ufshcd_init: 1d84000.ufs: took 43553 usecs, dev_state: UFS_ACTIVE_PWR_MODE, link_state: UIC_LINK_ACTIVE_STATE, err 0
> 
>  drivers/ufs/host/ufs-qcom.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index 39eef470f8fa..f9f161340e78 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -738,8 +738,12 @@ static int ufs_qcom_pwr_change_notify(struct ufs_hba *hba,
>  		 * the second init can program the optimal PHY settings. This allows one to start
>  		 * the first init with either the minimum or the maximum support gear.
>  		 */
> -		if (hba->ufshcd_state == UFSHCD_STATE_RESET)
> +		if (hba->ufshcd_state == UFSHCD_STATE_RESET) {
> +			if (host->hw_ver.major >= 0x4 &&

Is this check really necessary?

The initial phy_gear state is something like this (my phrasing of
ufs_qcom_set_phy_gear()):

    if hw_ver < 4:
        # Comments about powering up with minimum gear (with no
        # reasoning in the comment afaict), and mentions switching
        # to higher gear in reinit quirk. This is opposite of the later
        # versions which start at the max and scale down
        phy_gear = UFS_HS_G2

    else if hw_ver == 4:
        phy_gear = hs_tx_gear # (so afaict always UFS_HS_G4)

    else if hw_ver >= 5:
        phy_gear = hs_tx_gear # (What ever the max is for this version)

        if dev_major:
            # Clears the reinit quirk in ufs_qcom_set_phy_gear() if the
            # device version is provided by bootloader / controller
            # because we already found it out and can init directly
            # to the ideal gear
            quirks &= ~UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH

            if dev_major < 4:
                # Sets gear to UFS_HS_G4 to save power for UFS 3.1 and
                # older devices
                phy_gear = UFS_HS_G4

I guess what I'm saying is that I'm not totally seeing how this check
is dependent on the controller version. To me, if we're in the ideal
gear already and we know it, we should *not* reinit, no matter what the
controller version is. That's assuming there's not some other reasoning
for the quirk, but as far as I understand it the quirk exists because we
have to start with *some* phy gear value so we can talk to the device,
and once we discover what the device is capable of it makes sense to
scale down (or up for older controllers) to the ideal gear setting for
the attached device. Unfortunately to do the change we have to
reprogram the phy which I guess is only acceptable if we reprogram
everything (hence the reinit).

Does that make sense or do you think I'm missing something? I think say
even for an older controller this makes sense, if its attached to a
UFS_HS_G2 capable device there is no reason to reinit since we started
up in the ideal configuration.

Thanks,
Andrew

> +			    host_params->hs_tx_gear == dev_req_params->gear_tx)
> +				hba->quirks &= ~UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH;
>  			host->phy_gear = dev_req_params->gear_tx;
> +		}
>  
>  		/* enable the device ref clock before changing to HS mode */
>  		if (!ufshcd_is_hs_mode(&hba->pwr_info) &&
> -- 
> 2.43.0
> 
>
  
Andrew Halaney Jan. 19, 2024, 8:33 p.m. UTC | #2
On Fri, Jan 19, 2024 at 02:07:15PM -0600, Andrew Halaney wrote:
> On Fri, Jan 19, 2024 at 01:55:47PM -0500, Eric Chanudet wrote:
> > On sa8775p-ride, probing the hba will go through the
> > UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH path although the power info
> > are same during the second init.
> > 
> > If the host is at least v4, ufs_qcom_get_hs_gear() picked the highest
> > supported gear when setting the host_params. After the negotiation, if
> > the host and device are on the same gear, it is the highest gear
> > supported between the two. Skip the re-init to save some time.
> > 
> > Signed-off-by: Eric Chanudet <echanude@redhat.com>
> > ---
> > 
> > "trace_event=ufs:ufshcd_init" reports the time spent where the re-init
> > quirk is performed. On sa8775p-ride:
> > Baseline:
> >   0.355879: ufshcd_init: 1d84000.ufs: took 103377 usecs, dev_state: UFS_ACTIVE_PWR_MODE, link_state: UIC_LINK_ACTIVE_STATE, err 0
> > With this patch:
> >   0.297676: ufshcd_init: 1d84000.ufs: took 43553 usecs, dev_state: UFS_ACTIVE_PWR_MODE, link_state: UIC_LINK_ACTIVE_STATE, err 0
> > 
> >  drivers/ufs/host/ufs-qcom.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> > index 39eef470f8fa..f9f161340e78 100644
> > --- a/drivers/ufs/host/ufs-qcom.c
> > +++ b/drivers/ufs/host/ufs-qcom.c
> > @@ -738,8 +738,12 @@ static int ufs_qcom_pwr_change_notify(struct ufs_hba *hba,
> >  		 * the second init can program the optimal PHY settings. This allows one to start
> >  		 * the first init with either the minimum or the maximum support gear.
> >  		 */
> > -		if (hba->ufshcd_state == UFSHCD_STATE_RESET)
> > +		if (hba->ufshcd_state == UFSHCD_STATE_RESET) {
> > +			if (host->hw_ver.major >= 0x4 &&
> 
> Is this check really necessary?
> 
> The initial phy_gear state is something like this (my phrasing of
> ufs_qcom_set_phy_gear()):
> 
>     if hw_ver < 4:
>         # Comments about powering up with minimum gear (with no
>         # reasoning in the comment afaict), and mentions switching
>         # to higher gear in reinit quirk. This is opposite of the later
>         # versions which start at the max and scale down
>         phy_gear = UFS_HS_G2
> 
>     else if hw_ver == 4:
>         phy_gear = hs_tx_gear # (so afaict always UFS_HS_G4)
> 
>     else if hw_ver >= 5:
>         phy_gear = hs_tx_gear # (What ever the max is for this version)
> 
>         if dev_major:
>             # Clears the reinit quirk in ufs_qcom_set_phy_gear() if the
>             # device version is provided by bootloader / controller
>             # because we already found it out and can init directly
>             # to the ideal gear
>             quirks &= ~UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH
> 
>             if dev_major < 4:
>                 # Sets gear to UFS_HS_G4 to save power for UFS 3.1 and
>                 # older devices
>                 phy_gear = UFS_HS_G4
> 
> I guess what I'm saying is that I'm not totally seeing how this check
> is dependent on the controller version. To me, if we're in the ideal
> gear already and we know it, we should *not* reinit, no matter what the
> controller version is. That's assuming there's not some other reasoning
> for the quirk, but as far as I understand it the quirk exists because we
> have to start with *some* phy gear value so we can talk to the device,
> and once we discover what the device is capable of it makes sense to
> scale down (or up for older controllers) to the ideal gear setting for
> the attached device. Unfortunately to do the change we have to
> reprogram the phy which I guess is only acceptable if we reprogram
> everything (hence the reinit).
> 
> Does that make sense or do you think I'm missing something? I think say
> even for an older controller this makes sense, if its attached to a
> UFS_HS_G2 capable device there is no reason to reinit since we started
> up in the ideal configuration.

I guess what I'm saying is shouldn't the check be something like:

    if (dev_req_params->gear_tx == host->phy_gear) {
        // Skip reinit since we started up in the agreed upon gear
        hba->quirks &= ~UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH;
    }

Which is independent of the controller version?

> 
> Thanks,
> Andrew
> 
> > +			    host_params->hs_tx_gear == dev_req_params->gear_tx)
> > +				hba->quirks &= ~UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH;
> >  			host->phy_gear = dev_req_params->gear_tx;
> > +		}
> >  
> >  		/* enable the device ref clock before changing to HS mode */
> >  		if (!ufshcd_is_hs_mode(&hba->pwr_info) &&
> > -- 
> > 2.43.0
> > 
> >
  
Eric Chanudet Jan. 19, 2024, 9:33 p.m. UTC | #3
On Fri, Jan 19, 2024 at 02:33:32PM -0600, Andrew Halaney wrote:
> On Fri, Jan 19, 2024 at 02:07:15PM -0600, Andrew Halaney wrote:
> > On Fri, Jan 19, 2024 at 01:55:47PM -0500, Eric Chanudet wrote:
> > > On sa8775p-ride, probing the hba will go through the
> > > UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH path although the power info
> > > are same during the second init.
> > > 
> > > If the host is at least v4, ufs_qcom_get_hs_gear() picked the highest
> > > supported gear when setting the host_params. After the negotiation, if
> > > the host and device are on the same gear, it is the highest gear
> > > supported between the two. Skip the re-init to save some time.
> > > 
> > > Signed-off-by: Eric Chanudet <echanude@redhat.com>
> > > ---
> > > 
> > > "trace_event=ufs:ufshcd_init" reports the time spent where the re-init
> > > quirk is performed. On sa8775p-ride:
> > > Baseline:
> > >   0.355879: ufshcd_init: 1d84000.ufs: took 103377 usecs, dev_state: UFS_ACTIVE_PWR_MODE, link_state: UIC_LINK_ACTIVE_STATE, err 0
> > > With this patch:
> > >   0.297676: ufshcd_init: 1d84000.ufs: took 43553 usecs, dev_state: UFS_ACTIVE_PWR_MODE, link_state: UIC_LINK_ACTIVE_STATE, err 0
> > > 
> > >  drivers/ufs/host/ufs-qcom.c | 6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> > > index 39eef470f8fa..f9f161340e78 100644
> > > --- a/drivers/ufs/host/ufs-qcom.c
> > > +++ b/drivers/ufs/host/ufs-qcom.c
> > > @@ -738,8 +738,12 @@ static int ufs_qcom_pwr_change_notify(struct ufs_hba *hba,
> > >  		 * the second init can program the optimal PHY settings. This allows one to start
> > >  		 * the first init with either the minimum or the maximum support gear.
> > >  		 */
> > > -		if (hba->ufshcd_state == UFSHCD_STATE_RESET)
> > > +		if (hba->ufshcd_state == UFSHCD_STATE_RESET) {
> > > +			if (host->hw_ver.major >= 0x4 &&
> > 
> > Is this check really necessary?

I *think* so.

For example, if hw_ver < 4, ufs_qcom_set_phy_gear() has a comment saying
"power up the PHY using minimum supported gear (UFS_HS_G2). Switching to
max gear will be performed during reinit if supported."

> > 
> > The initial phy_gear state is something like this (my phrasing of
> > ufs_qcom_set_phy_gear()):
> > 
> >     if hw_ver < 4:
> >         # Comments about powering up with minimum gear (with no
> >         # reasoning in the comment afaict), and mentions switching
> >         # to higher gear in reinit quirk. This is opposite of the later
> >         # versions which start at the max and scale down
> >         phy_gear = UFS_HS_G2

IIUC, the device would not be able to negotiate a gear higher than the
minimum set for the phy_gear on initialization.

ufshcd_init_host_params() and ufs_qcom_get_hs_gear() both set the
controller <v4 host_params to G3. So if the device is HS capable, the
re-init would set G3, instead of the G2 selected by
ufs_qcom_set_phy_gear().

Assuming I'm not loosing track somewhere, the sequence of calls would go
something like this:

ufshcd_init
 ufshcd_hba_init
  ufshcd_variant_hba_init
   ufshcd_vops_init
    ufs_qcom_init
     ufs_qcom_set_host_params /* if hw_ver < 4: phy_gear = G2 */
 ufshcd_hba_enable
  ufshcd_hba_execute_hce
   ufshcd_vops_hce_enable_notify(PRE_CHANGE)
    ufs_qcom_hce_enable_notify /* vops.hce_enable_notify */
     ufs_qcom_power_up_sequence
      phy_set_mode_ext(phy, mode, host->phy_gear);
 async_schedule(ufshcd_async_scan, hba)
 ...
 ufshcd_async_scan
  ufshcd_device_init
  ufshcd_probe_hba /* where the re-init quirk happens */
   ufshcd_device_init
    ufshcd_vops_pwr_change_notify(PRE_CHANGE)
  
So that would limit the device to G2? In this circumstances, the re-init
would instead re-initialize to G3.

> I guess what I'm saying is shouldn't the check be something like:
> 
>     if (dev_req_params->gear_tx == host->phy_gear) {
>         // Skip reinit since we started up in the agreed upon gear
>         hba->quirks &= ~UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH;
>     }
  
Manivannan Sadhasivam Jan. 23, 2024, 2:36 p.m. UTC | #4
On Fri, Jan 19, 2024 at 04:33:10PM -0500, Eric Chanudet wrote:
> On Fri, Jan 19, 2024 at 02:33:32PM -0600, Andrew Halaney wrote:
> > On Fri, Jan 19, 2024 at 02:07:15PM -0600, Andrew Halaney wrote:
> > > On Fri, Jan 19, 2024 at 01:55:47PM -0500, Eric Chanudet wrote:
> > > > On sa8775p-ride, probing the hba will go through the
> > > > UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH path although the power info
> > > > are same during the second init.
> > > > 
> > > > If the host is at least v4, ufs_qcom_get_hs_gear() picked the highest
> > > > supported gear when setting the host_params. After the negotiation, if
> > > > the host and device are on the same gear, it is the highest gear
> > > > supported between the two. Skip the re-init to save some time.
> > > > 
> > > > Signed-off-by: Eric Chanudet <echanude@redhat.com>
> > > > ---
> > > > 
> > > > "trace_event=ufs:ufshcd_init" reports the time spent where the re-init
> > > > quirk is performed. On sa8775p-ride:
> > > > Baseline:
> > > >   0.355879: ufshcd_init: 1d84000.ufs: took 103377 usecs, dev_state: UFS_ACTIVE_PWR_MODE, link_state: UIC_LINK_ACTIVE_STATE, err 0
> > > > With this patch:
> > > >   0.297676: ufshcd_init: 1d84000.ufs: took 43553 usecs, dev_state: UFS_ACTIVE_PWR_MODE, link_state: UIC_LINK_ACTIVE_STATE, err 0
> > > > 
> > > >  drivers/ufs/host/ufs-qcom.c | 6 +++++-
> > > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> > > > index 39eef470f8fa..f9f161340e78 100644
> > > > --- a/drivers/ufs/host/ufs-qcom.c
> > > > +++ b/drivers/ufs/host/ufs-qcom.c
> > > > @@ -738,8 +738,12 @@ static int ufs_qcom_pwr_change_notify(struct ufs_hba *hba,
> > > >  		 * the second init can program the optimal PHY settings. This allows one to start
> > > >  		 * the first init with either the minimum or the maximum support gear.
> > > >  		 */
> > > > -		if (hba->ufshcd_state == UFSHCD_STATE_RESET)
> > > > +		if (hba->ufshcd_state == UFSHCD_STATE_RESET) {
> > > > +			if (host->hw_ver.major >= 0x4 &&
> > > 
> > > Is this check really necessary?
> 
> I *think* so.
> 
> For example, if hw_ver < 4, ufs_qcom_set_phy_gear() has a comment saying
> "power up the PHY using minimum supported gear (UFS_HS_G2). Switching to
> max gear will be performed during reinit if supported."
> 
> > > 
> > > The initial phy_gear state is something like this (my phrasing of
> > > ufs_qcom_set_phy_gear()):
> > > 
> > >     if hw_ver < 4:
> > >         # Comments about powering up with minimum gear (with no
> > >         # reasoning in the comment afaict), and mentions switching
> > >         # to higher gear in reinit quirk. This is opposite of the later
> > >         # versions which start at the max and scale down
> > >         phy_gear = UFS_HS_G2
> 
> IIUC, the device would not be able to negotiate a gear higher than the
> minimum set for the phy_gear on initialization.
> 
> ufshcd_init_host_params() and ufs_qcom_get_hs_gear() both set the
> controller <v4 host_params to G3. So if the device is HS capable, the
> re-init would set G3, instead of the G2 selected by
> ufs_qcom_set_phy_gear().
> 

REINIT quirk is applicable for controllers starting from v4 only, because legacy
controllers don't need separate PHY init sequences. So you can get rid of that
check.

- Mani
  
Manivannan Sadhasivam Jan. 23, 2024, 3:11 p.m. UTC | #5
On Fri, Jan 19, 2024 at 01:55:47PM -0500, Eric Chanudet wrote:
> On sa8775p-ride, probing the hba will go through the
> UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH path although the power info
> are same during the second init.
> 
> If the host is at least v4, ufs_qcom_get_hs_gear() picked the highest
> supported gear when setting the host_params. After the negotiation, if
> the host and device are on the same gear, it is the highest gear
> supported between the two. Skip the re-init to save some time.
> 
> Signed-off-by: Eric Chanudet <echanude@redhat.com>
> ---
> 
> "trace_event=ufs:ufshcd_init" reports the time spent where the re-init
> quirk is performed. On sa8775p-ride:
> Baseline:
>   0.355879: ufshcd_init: 1d84000.ufs: took 103377 usecs, dev_state: UFS_ACTIVE_PWR_MODE, link_state: UIC_LINK_ACTIVE_STATE, err 0
> With this patch:
>   0.297676: ufshcd_init: 1d84000.ufs: took 43553 usecs, dev_state: UFS_ACTIVE_PWR_MODE, link_state: UIC_LINK_ACTIVE_STATE, err 0
> 
>  drivers/ufs/host/ufs-qcom.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index 39eef470f8fa..f9f161340e78 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -738,8 +738,12 @@ static int ufs_qcom_pwr_change_notify(struct ufs_hba *hba,
>  		 * the second init can program the optimal PHY settings. This allows one to start
>  		 * the first init with either the minimum or the maximum support gear.
>  		 */
> -		if (hba->ufshcd_state == UFSHCD_STATE_RESET)
> +		if (hba->ufshcd_state == UFSHCD_STATE_RESET) {
> +			if (host->hw_ver.major >= 0x4 &&

You can get rid of this check as I said in the reply.

> +			    host_params->hs_tx_gear == dev_req_params->gear_tx)

How about?

			/*
			 * Skip REINIT if the negotiated gear matches with the
			 * initial phy_gear. Otherwise, update the phy_gear to
			 * program the optimal gear setting during REINIT.
			 */
			if (host->phy_gear == dev_req_params->gear_tx)
				hba->quirks &= ~UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH;
			else
				host->phy_gear = dev_req_params->gear_tx;

- Mani

> +				hba->quirks &= ~UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH;
>  			host->phy_gear = dev_req_params->gear_tx;
> +		}
>  
>  		/* enable the device ref clock before changing to HS mode */
>  		if (!ufshcd_is_hs_mode(&hba->pwr_info) &&
> -- 
> 2.43.0
>
  
Eric Chanudet Jan. 23, 2024, 3:18 p.m. UTC | #6
On Tue, Jan 23, 2024 at 08:06:15PM +0530, Manivannan Sadhasivam wrote:
> On Fri, Jan 19, 2024 at 04:33:10PM -0500, Eric Chanudet wrote:
> > On Fri, Jan 19, 2024 at 02:33:32PM -0600, Andrew Halaney wrote:
> > > On Fri, Jan 19, 2024 at 02:07:15PM -0600, Andrew Halaney wrote:
> > > > On Fri, Jan 19, 2024 at 01:55:47PM -0500, Eric Chanudet wrote:
> > > > > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> > > > > index 39eef470f8fa..f9f161340e78 100644
> > > > > --- a/drivers/ufs/host/ufs-qcom.c
> > > > > +++ b/drivers/ufs/host/ufs-qcom.c
> > > > > @@ -738,8 +738,12 @@ static int ufs_qcom_pwr_change_notify(struct ufs_hba *hba,
> > > > >  		 * the second init can program the optimal PHY settings. This allows one to start
> > > > >  		 * the first init with either the minimum or the maximum support gear.
> > > > >  		 */
> > > > > -		if (hba->ufshcd_state == UFSHCD_STATE_RESET)
> > > > > +		if (hba->ufshcd_state == UFSHCD_STATE_RESET) {
> > > > > +			if (host->hw_ver.major >= 0x4 &&
> > > > 
> > > > Is this check really necessary?
> > 
> > I *think* so.
> > 
> > For example, if hw_ver < 4, ufs_qcom_set_phy_gear() has a comment saying
> > "power up the PHY using minimum supported gear (UFS_HS_G2). Switching to
> > max gear will be performed during reinit if supported."
> > 
> > > > 
> > > > The initial phy_gear state is something like this (my phrasing of
> > > > ufs_qcom_set_phy_gear()):
> > > > 
> > > >     if hw_ver < 4:
> > > >         # Comments about powering up with minimum gear (with no
> > > >         # reasoning in the comment afaict), and mentions switching
> > > >         # to higher gear in reinit quirk. This is opposite of the later
> > > >         # versions which start at the max and scale down
> > > >         phy_gear = UFS_HS_G2
> > 
> > IIUC, the device would not be able to negotiate a gear higher than the
> > minimum set for the phy_gear on initialization.
> > 
> > ufshcd_init_host_params() and ufs_qcom_get_hs_gear() both set the
> > controller <v4 host_params to G3. So if the device is HS capable, the
> > re-init would set G3, instead of the G2 selected by
> > ufs_qcom_set_phy_gear().
> > 
> 
> REINIT quirk is applicable for controllers starting from v4 only, because legacy
> controllers don't need separate PHY init sequences. So you can get rid of that
> check.

My bad, I overlooked the check in ufs_qcom_advertise_quirks().
Thank you, I'll send a v2 soon.

> - Mani
> 
> -- 
> மணிவண்ணன் சதாசிவம்
>
  

Patch

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 39eef470f8fa..f9f161340e78 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -738,8 +738,12 @@  static int ufs_qcom_pwr_change_notify(struct ufs_hba *hba,
 		 * the second init can program the optimal PHY settings. This allows one to start
 		 * the first init with either the minimum or the maximum support gear.
 		 */
-		if (hba->ufshcd_state == UFSHCD_STATE_RESET)
+		if (hba->ufshcd_state == UFSHCD_STATE_RESET) {
+			if (host->hw_ver.major >= 0x4 &&
+			    host_params->hs_tx_gear == dev_req_params->gear_tx)
+				hba->quirks &= ~UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH;
 			host->phy_gear = dev_req_params->gear_tx;
+		}
 
 		/* enable the device ref clock before changing to HS mode */
 		if (!ufshcd_is_hs_mode(&hba->pwr_info) &&