diff mbox series

[v6,10/10] scsi: ufs: ufs-qcom: Add support for UFS device version detection

Message ID 1701246516-11626-11-git-send-email-quic_cang@quicinc.com (mailing list archive)
State Superseded
Headers show
Series Enable HS-G5 support on SM8550 | expand

Commit Message

Can Guo Nov. 29, 2023, 8:28 a.m. UTC 
From: "Bao D. Nguyen" <quic_nguyenb@quicinc.com>

A spare register in UFS host controller is used to indicate the UFS device
version. The spare register is populated by bootloader for now, but in
future it will be populated by HW automatically during link startup with
its best efforts in any boot stages prior to Linux.

During host driver init, read the spare register, if it is not populated
with a UFS device version, go ahead with the dual init mechanism. If a UFS
device version is in there, use the UFS device version together with host
controller's HW version to decide the proper PHY gear which should be used
to configure the UFS PHY without going through the second init.

Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
Signed-off-by: Can Guo <quic_cang@quicinc.com>
---
 drivers/ufs/host/ufs-qcom.c | 23 ++++++++++++++++++-----
 drivers/ufs/host/ufs-qcom.h |  2 ++
 2 files changed, 20 insertions(+), 5 deletions(-)

Comments

Eric Chanudet Nov. 29, 2023, 7:04 p.m. UTC | #1 
On Wed, Nov 29, 2023 at 12:28:35AM -0800, Can Guo wrote:
> From: "Bao D. Nguyen" <quic_nguyenb@quicinc.com>
> 
> A spare register in UFS host controller is used to indicate the UFS device
> version. The spare register is populated by bootloader for now, but in
> future it will be populated by HW automatically during link startup with
> its best efforts in any boot stages prior to Linux.
> 
> During host driver init, read the spare register, if it is not populated
> with a UFS device version, go ahead with the dual init mechanism. If a UFS
> device version is in there, use the UFS device version together with host
> controller's HW version to decide the proper PHY gear which should be used
> to configure the UFS PHY without going through the second init.
> 
> Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
> Signed-off-by: Can Guo <quic_cang@quicinc.com>
> ---
>  drivers/ufs/host/ufs-qcom.c | 23 ++++++++++++++++++-----
>  drivers/ufs/host/ufs-qcom.h |  2 ++
>  2 files changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index 9c0ebbc..e94dea2 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -1068,15 +1068,28 @@ static void ufs_qcom_advertise_quirks(struct ufs_hba *hba)
>  static void ufs_qcom_set_phy_gear(struct ufs_qcom_host *host)
>  {
>  	struct ufs_host_params *host_params = &host->host_params;
> +	u32 val, dev_major = 0;
>  
>  	host->phy_gear = host_params->hs_tx_gear;
>  
> -	/*
> -	 * Power up the PHY using the minimum supported gear (UFS_HS_G2).
> -	 * Switching to max gear will be performed during reinit if supported.
> -	 */
> -	if (host->hw_ver.major < 0x4)
> +	if (host->hw_ver.major < 0x4) {
> +		/*
> +		 * Power up the PHY using the minimum supported gear (UFS_HS_G2).
> +		 * Switching to max gear will be performed during reinit if supported.
> +		 */
>  		host->phy_gear = UFS_HS_G2;
> +	} else {
> +		val = ufshcd_readl(host->hba, REG_UFS_DEBUG_SPARE_CFG);
> +		dev_major = FIELD_GET(GENMASK(7, 4), val);
> +
> +		/* UFS device version populated, no need to do init twice */

This change documents the content of the register as "UFS device
version", both inline in the code and in the commit description. Earlier
in this series[1], Mani mentioned it was the gear info:
> > [...]populating a spare register in the bootloader with the max gear
> > info that the bootloader has already found[...]

Is it the gear number as in HS-G4? Or the version as in UFS controller
revision 3.1?

[1] https://lore.kernel.org/all/20231018124741.GA47321@thinkpad/

> +		if (dev_major != 0)
> +			host->hba->quirks &= ~UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH;
> +
> +		/* For UFS 3.1 and older, apply HS-G4 PHY gear to save power */
> +		if (dev_major < 0x4 && dev_major > 0)
> +			host->phy_gear = UFS_HS_G4;
> +	}
>  }
>  
>  static void ufs_qcom_set_host_params(struct ufs_hba *hba)
> diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h
> index 11419eb..d12fc5a 100644
> --- a/drivers/ufs/host/ufs-qcom.h
> +++ b/drivers/ufs/host/ufs-qcom.h
> @@ -54,6 +54,8 @@ enum {
>  	UFS_AH8_CFG				= 0xFC,
>  
>  	REG_UFS_CFG3				= 0x271C,
> +
> +	REG_UFS_DEBUG_SPARE_CFG			= 0x284C,
>  };
>  
>  /* QCOM UFS host controller vendor specific debug registers */
> -- 
> 2.7.4
> 
>
Manivannan Sadhasivam Nov. 30, 2023, 7:25 a.m. UTC | #2 
On Wed, Nov 29, 2023 at 12:28:35AM -0800, Can Guo wrote:
> From: "Bao D. Nguyen" <quic_nguyenb@quicinc.com>
> 
> A spare register in UFS host controller is used to indicate the UFS device
> version. The spare register is populated by bootloader for now, but in
> future it will be populated by HW automatically during link startup with
> its best efforts in any boot stages prior to Linux.
> 
> During host driver init, read the spare register, if it is not populated
> with a UFS device version, go ahead with the dual init mechanism. If a UFS
> device version is in there, use the UFS device version together with host
> controller's HW version to decide the proper PHY gear which should be used
> to configure the UFS PHY without going through the second init.
> 
> Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
> Signed-off-by: Can Guo <quic_cang@quicinc.com>
> ---
>  drivers/ufs/host/ufs-qcom.c | 23 ++++++++++++++++++-----
>  drivers/ufs/host/ufs-qcom.h |  2 ++
>  2 files changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index 9c0ebbc..e94dea2 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -1068,15 +1068,28 @@ static void ufs_qcom_advertise_quirks(struct ufs_hba *hba)
>  static void ufs_qcom_set_phy_gear(struct ufs_qcom_host *host)
>  {
>  	struct ufs_host_params *host_params = &host->host_params;
> +	u32 val, dev_major = 0;

No need to initialize dev_major.

>  
>  	host->phy_gear = host_params->hs_tx_gear;
>  
> -	/*
> -	 * Power up the PHY using the minimum supported gear (UFS_HS_G2).
> -	 * Switching to max gear will be performed during reinit if supported.
> -	 */
> -	if (host->hw_ver.major < 0x4)
> +	if (host->hw_ver.major < 0x4) {
> +		/*
> +		 * Power up the PHY using the minimum supported gear (UFS_HS_G2).
> +		 * Switching to max gear will be performed during reinit if supported.
> +		 */
>  		host->phy_gear = UFS_HS_G2;
> +	} else {

Can you please add a comment here to describe what is happening?

> +		val = ufshcd_readl(host->hba, REG_UFS_DEBUG_SPARE_CFG);
> +		dev_major = FIELD_GET(GENMASK(7, 4), val);

It'd be good to add a macro for GENMASK().

> +
> +		/* UFS device version populated, no need to do init twice */

"Since the UFS device version is populated, let's remove the REINIT quirk as the
negotiated gear won't change during boot. So there is no need to do reinit."

> +		if (dev_major != 0)

0x0

> +			host->hba->quirks &= ~UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH;
> +
> +		/* For UFS 3.1 and older, apply HS-G4 PHY gear to save power */
> +		if (dev_major < 0x4 && dev_major > 0)

if (dev_major > 0x0 && dev_major < 0x4)

- Mani

> +			host->phy_gear = UFS_HS_G4;
> +	}
>  }
>  
>  static void ufs_qcom_set_host_params(struct ufs_hba *hba)
> diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h
> index 11419eb..d12fc5a 100644
> --- a/drivers/ufs/host/ufs-qcom.h
> +++ b/drivers/ufs/host/ufs-qcom.h
> @@ -54,6 +54,8 @@ enum {
>  	UFS_AH8_CFG				= 0xFC,
>  
>  	REG_UFS_CFG3				= 0x271C,
> +
> +	REG_UFS_DEBUG_SPARE_CFG			= 0x284C,
>  };
>  
>  /* QCOM UFS host controller vendor specific debug registers */
> -- 
> 2.7.4
> 
>
Manivannan Sadhasivam Nov. 30, 2023, 7:28 a.m. UTC | #3 
On Wed, Nov 29, 2023 at 02:04:35PM -0500, Eric Chanudet wrote:
> On Wed, Nov 29, 2023 at 12:28:35AM -0800, Can Guo wrote:
> > From: "Bao D. Nguyen" <quic_nguyenb@quicinc.com>
> > 
> > A spare register in UFS host controller is used to indicate the UFS device
> > version. The spare register is populated by bootloader for now, but in
> > future it will be populated by HW automatically during link startup with
> > its best efforts in any boot stages prior to Linux.
> > 
> > During host driver init, read the spare register, if it is not populated
> > with a UFS device version, go ahead with the dual init mechanism. If a UFS
> > device version is in there, use the UFS device version together with host
> > controller's HW version to decide the proper PHY gear which should be used
> > to configure the UFS PHY without going through the second init.
> > 
> > Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
> > Signed-off-by: Can Guo <quic_cang@quicinc.com>
> > ---
> >  drivers/ufs/host/ufs-qcom.c | 23 ++++++++++++++++++-----
> >  drivers/ufs/host/ufs-qcom.h |  2 ++
> >  2 files changed, 20 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> > index 9c0ebbc..e94dea2 100644
> > --- a/drivers/ufs/host/ufs-qcom.c
> > +++ b/drivers/ufs/host/ufs-qcom.c
> > @@ -1068,15 +1068,28 @@ static void ufs_qcom_advertise_quirks(struct ufs_hba *hba)
> >  static void ufs_qcom_set_phy_gear(struct ufs_qcom_host *host)
> >  {
> >  	struct ufs_host_params *host_params = &host->host_params;
> > +	u32 val, dev_major = 0;
> >  
> >  	host->phy_gear = host_params->hs_tx_gear;
> >  
> > -	/*
> > -	 * Power up the PHY using the minimum supported gear (UFS_HS_G2).
> > -	 * Switching to max gear will be performed during reinit if supported.
> > -	 */
> > -	if (host->hw_ver.major < 0x4)
> > +	if (host->hw_ver.major < 0x4) {
> > +		/*
> > +		 * Power up the PHY using the minimum supported gear (UFS_HS_G2).
> > +		 * Switching to max gear will be performed during reinit if supported.
> > +		 */
> >  		host->phy_gear = UFS_HS_G2;
> > +	} else {
> > +		val = ufshcd_readl(host->hba, REG_UFS_DEBUG_SPARE_CFG);
> > +		dev_major = FIELD_GET(GENMASK(7, 4), val);
> > +
> > +		/* UFS device version populated, no need to do init twice */
> 
> This change documents the content of the register as "UFS device
> version", both inline in the code and in the commit description. Earlier
> in this series[1], Mani mentioned it was the gear info:
> > > [...]populating a spare register in the bootloader with the max gear
> > > info that the bootloader has already found[...]
> 
> Is it the gear number as in HS-G4? Or the version as in UFS controller
> revision 3.1?
> 

That was a mistake from my end, apologies. The register has the device version
only and the driver uses both device and host controller's version info to
decide the initial gear.

- Mani

> [1] https://lore.kernel.org/all/20231018124741.GA47321@thinkpad/
> 
> > +		if (dev_major != 0)
> > +			host->hba->quirks &= ~UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH;
> > +
> > +		/* For UFS 3.1 and older, apply HS-G4 PHY gear to save power */
> > +		if (dev_major < 0x4 && dev_major > 0)
> > +			host->phy_gear = UFS_HS_G4;
> > +	}
> >  }
> >  
> >  static void ufs_qcom_set_host_params(struct ufs_hba *hba)
> > diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h
> > index 11419eb..d12fc5a 100644
> > --- a/drivers/ufs/host/ufs-qcom.h
> > +++ b/drivers/ufs/host/ufs-qcom.h
> > @@ -54,6 +54,8 @@ enum {
> >  	UFS_AH8_CFG				= 0xFC,
> >  
> >  	REG_UFS_CFG3				= 0x271C,
> > +
> > +	REG_UFS_DEBUG_SPARE_CFG			= 0x284C,
> >  };
> >  
> >  /* QCOM UFS host controller vendor specific debug registers */
> > -- 
> > 2.7.4
> > 
> > 
> 
> -- 
> Eric Chanudet
> 
>
Can Guo Nov. 30, 2023, 8:21 a.m. UTC | #4 
On 11/30/2023 3:25 PM, Manivannan Sadhasivam wrote:
> On Wed, Nov 29, 2023 at 12:28:35AM -0800, Can Guo wrote:
>> From: "Bao D. Nguyen" <quic_nguyenb@quicinc.com>
>>
>> A spare register in UFS host controller is used to indicate the UFS device
>> version. The spare register is populated by bootloader for now, but in
>> future it will be populated by HW automatically during link startup with
>> its best efforts in any boot stages prior to Linux.
>>
>> During host driver init, read the spare register, if it is not populated
>> with a UFS device version, go ahead with the dual init mechanism. If a UFS
>> device version is in there, use the UFS device version together with host
>> controller's HW version to decide the proper PHY gear which should be used
>> to configure the UFS PHY without going through the second init.
>>
>> Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
>> Signed-off-by: Can Guo <quic_cang@quicinc.com>
>> ---
>>   drivers/ufs/host/ufs-qcom.c | 23 ++++++++++++++++++-----
>>   drivers/ufs/host/ufs-qcom.h |  2 ++
>>   2 files changed, 20 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
>> index 9c0ebbc..e94dea2 100644
>> --- a/drivers/ufs/host/ufs-qcom.c
>> +++ b/drivers/ufs/host/ufs-qcom.c
>> @@ -1068,15 +1068,28 @@ static void ufs_qcom_advertise_quirks(struct ufs_hba *hba)
>>   static void ufs_qcom_set_phy_gear(struct ufs_qcom_host *host)
>>   {
>>   	struct ufs_host_params *host_params = &host->host_params;
>> +	u32 val, dev_major = 0;
> 
> No need to initialize dev_major.

True.

> 
>>   
>>   	host->phy_gear = host_params->hs_tx_gear;
>>   
>> -	/*
>> -	 * Power up the PHY using the minimum supported gear (UFS_HS_G2).
>> -	 * Switching to max gear will be performed during reinit if supported.
>> -	 */
>> -	if (host->hw_ver.major < 0x4)
>> +	if (host->hw_ver.major < 0x4) {
>> +		/*
>> +		 * Power up the PHY using the minimum supported gear (UFS_HS_G2).
>> +		 * Switching to max gear will be performed during reinit if supported.
>> +		 */
>>   		host->phy_gear = UFS_HS_G2;
>> +	} else {
> 
> Can you please add a comment here to describe what is happening?

Will do.

> 
>> +		val = ufshcd_readl(host->hba, REG_UFS_DEBUG_SPARE_CFG);
>> +		dev_major = FIELD_GET(GENMASK(7, 4), val);
> 
> It'd be good to add a macro for GENMASK().

OK

> 
>> +
>> +		/* UFS device version populated, no need to do init twice */
> 
> "Since the UFS device version is populated, let's remove the REINIT quirk as the
> negotiated gear won't change during boot. So there is no need to do reinit."
> 

Nice comment.

>> +		if (dev_major != 0)
> 
> 0x0

Done

> 
>> +			host->hba->quirks &= ~UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH;
>> +
>> +		/* For UFS 3.1 and older, apply HS-G4 PHY gear to save power */
>> +		if (dev_major < 0x4 && dev_major > 0)
> 
> if (dev_major > 0x0 && dev_major < 0x4)
> 

Sure.

Thanks,
Can Guo.

> - Mani
> 
>> +			host->phy_gear = UFS_HS_G4;
>> +	}
>>   }
>>   
>>   static void ufs_qcom_set_host_params(struct ufs_hba *hba)
>> diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h
>> index 11419eb..d12fc5a 100644
>> --- a/drivers/ufs/host/ufs-qcom.h
>> +++ b/drivers/ufs/host/ufs-qcom.h
>> @@ -54,6 +54,8 @@ enum {
>>   	UFS_AH8_CFG				= 0xFC,
>>   
>>   	REG_UFS_CFG3				= 0x271C,
>> +
>> +	REG_UFS_DEBUG_SPARE_CFG			= 0x284C,
>>   };
>>   
>>   /* QCOM UFS host controller vendor specific debug registers */
>> -- 
>> 2.7.4
>>
>>
>
diff mbox series

Patch

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 9c0ebbc..e94dea2 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -1068,15 +1068,28 @@  static void ufs_qcom_advertise_quirks(struct ufs_hba *hba)
 static void ufs_qcom_set_phy_gear(struct ufs_qcom_host *host)
 {
 	struct ufs_host_params *host_params = &host->host_params;
+	u32 val, dev_major = 0;
 
 	host->phy_gear = host_params->hs_tx_gear;
 
-	/*
-	 * Power up the PHY using the minimum supported gear (UFS_HS_G2).
-	 * Switching to max gear will be performed during reinit if supported.
-	 */
-	if (host->hw_ver.major < 0x4)
+	if (host->hw_ver.major < 0x4) {
+		/*
+		 * Power up the PHY using the minimum supported gear (UFS_HS_G2).
+		 * Switching to max gear will be performed during reinit if supported.
+		 */
 		host->phy_gear = UFS_HS_G2;
+	} else {
+		val = ufshcd_readl(host->hba, REG_UFS_DEBUG_SPARE_CFG);
+		dev_major = FIELD_GET(GENMASK(7, 4), val);
+
+		/* UFS device version populated, no need to do init twice */
+		if (dev_major != 0)
+			host->hba->quirks &= ~UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH;
+
+		/* For UFS 3.1 and older, apply HS-G4 PHY gear to save power */
+		if (dev_major < 0x4 && dev_major > 0)
+			host->phy_gear = UFS_HS_G4;
+	}
 }
 
 static void ufs_qcom_set_host_params(struct ufs_hba *hba)
diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h
index 11419eb..d12fc5a 100644
--- a/drivers/ufs/host/ufs-qcom.h
+++ b/drivers/ufs/host/ufs-qcom.h
@@ -54,6 +54,8 @@  enum {
 	UFS_AH8_CFG				= 0xFC,
 
 	REG_UFS_CFG3				= 0x271C,
+
+	REG_UFS_DEBUG_SPARE_CFG			= 0x284C,
 };
 
 /* QCOM UFS host controller vendor specific debug registers */