[PATCH v2] scsi: ufs: modify Tactive time setting conditions

SEO HOYOUNG posted 1 patch 4 years, 5 months ago
drivers/scsi/ufs/ufshcd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH v2] scsi: ufs: modify Tactive time setting conditions
Posted by SEO HOYOUNG 4 years, 5 months ago
The Tactive time determine the waiting time before burst
at hibern8 exit and is determined by H/W at linkup state
However, in the case of samsung devices, guided host's Tactive time
+100us for stability.
If the HCI's Tactive time is equal or greater than the device,
+100us should be set.

Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 1049e41abd5b..460d2b440d2e 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -7815,7 +7815,7 @@ static int ufshcd_quirk_tune_host_pa_tactivate(struct ufs_hba *hba)
 	peer_pa_tactivate_us = peer_pa_tactivate *
 			     gran_to_us_table[peer_granularity - 1];
 
-	if (pa_tactivate_us > peer_pa_tactivate_us) {
+	if (pa_tactivate_us >= peer_pa_tactivate_us) {
 		u32 new_peer_pa_tactivate;
 
 		new_peer_pa_tactivate = pa_tactivate_us /
-- 
2.26.0

RE: [PATCH v2] scsi: ufs: modify Tactive time setting conditions
Posted by Avri Altman 4 years, 5 months ago
> The Tactive time determine the waiting time before burst
> at hibern8 exit and is determined by H/W at linkup state
> However, in the case of samsung devices, guided host's Tactive time
> +100us for stability.
> If the HCI's Tactive time is equal or greater than the device,
> +100us should be set.
This way you are changing this for everyone - not just for Samsung.
e.g. Qualcomm are using this quirk as well for WDC devices.

Thanks,
Avri

> 
> Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
> ---
>  drivers/scsi/ufs/ufshcd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 1049e41abd5b..460d2b440d2e 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -7815,7 +7815,7 @@ static int ufshcd_quirk_tune_host_pa_tactivate(struct
> ufs_hba *hba)
>         peer_pa_tactivate_us = peer_pa_tactivate *
>                              gran_to_us_table[peer_granularity - 1];
> 
> -       if (pa_tactivate_us > peer_pa_tactivate_us) {
> +       if (pa_tactivate_us >= peer_pa_tactivate_us) {
>                 u32 new_peer_pa_tactivate;
> 
>                 new_peer_pa_tactivate = pa_tactivate_us /
> --
> 2.26.0

RE: [PATCH v2] scsi: ufs: modify Tactive time setting conditions
Posted by 서호영 4 years, 5 months ago
Hi,
If register "UFS_DEVICE_QUIRK_HOST_PA_TACTIVATE" quirk,
using this Tact time function.
And I mean all Samsung device use UFS_DEVICE_QUIRK_HOST_PA_TACTIVATE quirk below this. so I told that.
static struct ufs_dev_fix ufs_fixups[] = {                    
         /* UFS cards deviations table */                      
         UFS_FIX(UFS_VENDOR_MICRON, UFS_ANY_MODEL,             
                 UFS_DEVICE_QUIRK_DELAY_BEFORE_LPM |           
                 UFS_DEVICE_QUIRK_SWAP_L2P_ENTRY_FOR_HPB_READ),
         UFS_FIX(UFS_VENDOR_SAMSUNG, UFS_ANY_MODEL,            
                 UFS_DEVICE_QUIRK_DELAY_BEFORE_LPM |           
                 UFS_DEVICE_QUIRK_HOST_PA_TACTIVATE |

Other vendors can use it if necessary.
And for stability, the device tact time must be longer than host tact time.
(I already check Major device vendors)
That's reason why use this function.
But the Host tact time and Device tact time same, stability may not be satisfied.
So I changed this way

Thanks,
HOYOUNG.

> -----Original Message-----
> From: Avri Altman [mailto:Avri.Altman@wdc.com]
> Sent: Friday, January 7, 2022 7:57 PM
> To: SEO HOYOUNG; linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org;
> alim.akhtar@samsung.com; jejb@linux.ibm.com; martin.petersen@oracle.com;
> beanhuo@micron.com; asutoshd@codeaurora.org; cang@codeaurora.org;
> bvanassche@acm.org
> Subject: RE: [PATCH v2] scsi: ufs: modify Tactive time setting conditions
> 
> > The Tactive time determine the waiting time before burst at hibern8
> > exit and is determined by H/W at linkup state However, in the case of
> > samsung devices, guided host's Tactive time
> > +100us for stability.
> > If the HCI's Tactive time is equal or greater than the device,
> > +100us should be set.
> This way you are changing this for everyone - not just for Samsung.
> e.g. Qualcomm are using this quirk as well for WDC devices.
> 
> Thanks,
> Avri
> 
> >
> > Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
> > ---
> >  drivers/scsi/ufs/ufshcd.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 1049e41abd5b..460d2b440d2e 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -7815,7 +7815,7 @@ static int
> > ufshcd_quirk_tune_host_pa_tactivate(struct
> > ufs_hba *hba)
> >         peer_pa_tactivate_us = peer_pa_tactivate *
> >                              gran_to_us_table[peer_granularity - 1];
> >
> > -       if (pa_tactivate_us > peer_pa_tactivate_us) {
> > +       if (pa_tactivate_us >= peer_pa_tactivate_us) {
> >                 u32 new_peer_pa_tactivate;
> >
> >                 new_peer_pa_tactivate = pa_tactivate_us /
> > --
> > 2.26.0


RE: [PATCH v2] scsi: ufs: modify Tactive time setting conditions
Posted by Avri Altman 4 years, 5 months ago
> 
> Hi,
> If register "UFS_DEVICE_QUIRK_HOST_PA_TACTIVATE" quirk,
> using this Tact time function.
> And I mean all Samsung device use UFS_DEVICE_QUIRK_HOST_PA_TACTIVATE
> quirk below this. so I told that.
> static struct ufs_dev_fix ufs_fixups[] = {
>          /* UFS cards deviations table */
>          UFS_FIX(UFS_VENDOR_MICRON, UFS_ANY_MODEL,
>                  UFS_DEVICE_QUIRK_DELAY_BEFORE_LPM |
>                  UFS_DEVICE_QUIRK_SWAP_L2P_ENTRY_FOR_HPB_READ),
>          UFS_FIX(UFS_VENDOR_SAMSUNG, UFS_ANY_MODEL,
>                  UFS_DEVICE_QUIRK_DELAY_BEFORE_LPM |
>                  UFS_DEVICE_QUIRK_HOST_PA_TACTIVATE |
> 
> Other vendors can use it if necessary.
> And for stability, the device tact time must be longer than host tact time.
> (I already check Major device vendors)
> That's reason why use this function.
> But the Host tact time and Device tact time same, stability may not be satisfied.
> So I changed this way
Yes - I agree.
Looks good to me.

Thanks,
Avri

> 
> Thanks,
> HOYOUNG.
> 
> > -----Original Message-----
> > From: Avri Altman [mailto:Avri.Altman@wdc.com]
> > Sent: Friday, January 7, 2022 7:57 PM
> > To: SEO HOYOUNG; linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org;
> > alim.akhtar@samsung.com; jejb@linux.ibm.com;
> martin.petersen@oracle.com;
> > beanhuo@micron.com; asutoshd@codeaurora.org; cang@codeaurora.org;
> > bvanassche@acm.org
> > Subject: RE: [PATCH v2] scsi: ufs: modify Tactive time setting conditions
> >
> > > The Tactive time determine the waiting time before burst at hibern8
> > > exit and is determined by H/W at linkup state However, in the case of
> > > samsung devices, guided host's Tactive time
> > > +100us for stability.
> > > If the HCI's Tactive time is equal or greater than the device,
> > > +100us should be set.
> > This way you are changing this for everyone - not just for Samsung.
> > e.g. Qualcomm are using this quirk as well for WDC devices.
> >
> > Thanks,
> > Avri
> >
> > >
> > > Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
> > > ---
> > >  drivers/scsi/ufs/ufshcd.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > > index 1049e41abd5b..460d2b440d2e 100644
> > > --- a/drivers/scsi/ufs/ufshcd.c
> > > +++ b/drivers/scsi/ufs/ufshcd.c
> > > @@ -7815,7 +7815,7 @@ static int
> > > ufshcd_quirk_tune_host_pa_tactivate(struct
> > > ufs_hba *hba)
> > >         peer_pa_tactivate_us = peer_pa_tactivate *
> > >                              gran_to_us_table[peer_granularity - 1];
> > >
> > > -       if (pa_tactivate_us > peer_pa_tactivate_us) {
> > > +       if (pa_tactivate_us >= peer_pa_tactivate_us) {
> > >                 u32 new_peer_pa_tactivate;
> > >
> > >                 new_peer_pa_tactivate = pa_tactivate_us /
> > > --
> > > 2.26.0
> 

Re: [PATCH v2] scsi: ufs: modify Tactive time setting conditions
Posted by Martin K. Petersen 4 years, 5 months ago
> If register "UFS_DEVICE_QUIRK_HOST_PA_TACTIVATE" quirk, using this
> Tact time function.

Applied to 5.17/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering
RE: [PATCH v2] scsi: ufs: modify Tactive time setting conditions
Posted by Alim Akhtar 4 years, 5 months ago
Hello Seo Hoyoung,

>-----Original Message-----
>From: SEO HOYOUNG [mailto:hy50.seo@samsung.com]
>Sent: Friday, January 7, 2022 3:09 AM
>To: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org;
>alim.akhtar@samsung.com; avri.altman@wdc.com; jejb@linux.ibm.com;
>martin.petersen@oracle.com; beanhuo@micron.com;
>asutoshd@codeaurora.org; cang@codeaurora.org; bvanassche@acm.org
>Cc: SEO HOYOUNG <hy50.seo@samsung.com>
>Subject: [PATCH v2] scsi: ufs: modify Tactive time setting conditions
>
>The Tactive time determine the waiting time before burst at hibern8 exit and
>is determined by H/W at linkup state However, in the case of samsung
>devices, guided host's Tactive time
>+100us for stability.
>If the HCI's Tactive time is equal or greater than the device,
>+100us should be set.
>
>Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
>---
Thanks!
Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>

> drivers/scsi/ufs/ufshcd.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index
>1049e41abd5b..460d2b440d2e 100644
>--- a/drivers/scsi/ufs/ufshcd.c
>+++ b/drivers/scsi/ufs/ufshcd.c
>@@ -7815,7 +7815,7 @@ static int
>ufshcd_quirk_tune_host_pa_tactivate(struct ufs_hba *hba)
> 	peer_pa_tactivate_us = peer_pa_tactivate *
> 			     gran_to_us_table[peer_granularity - 1];
>
>-	if (pa_tactivate_us > peer_pa_tactivate_us) {
>+	if (pa_tactivate_us >= peer_pa_tactivate_us) {
> 		u32 new_peer_pa_tactivate;
>
> 		new_peer_pa_tactivate = pa_tactivate_us /
>--
>2.26.0


Re: [PATCH v2] scsi: ufs: modify Tactive time setting conditions
Posted by Martin K. Petersen 4 years, 5 months ago
On Fri, 7 Jan 2022 06:39:24 +0900, SEO HOYOUNG wrote:

> The Tactive time determine the waiting time before burst
> at hibern8 exit and is determined by H/W at linkup state
> However, in the case of samsung devices, guided host's Tactive time
> +100us for stability.
> If the HCI's Tactive time is equal or greater than the device,
> +100us should be set.
> 
> [...]

Applied to 5.17/scsi-queue, thanks!

[1/1] scsi: ufs: modify Tactive time setting conditions
      https://git.kernel.org/mkp/scsi/c/9008661e1960

-- 
Martin K. Petersen	Oracle Linux Engineering