[PATCH v2] scsi: ufs: disable auto hibern8 while entering suspend

[PATCH v2] scsi: ufs: disable auto hibern8 while entering suspend

[SEO HOYOUNG]

v1-> v2: fixed no previous prototype warning
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):
>> drivers/scsi/ufs/ufshcd.c:4207:6: warning: no previous prototype 
for 'ufshcd_auto_hibern8_disable' [-Wmissing-prototypes]
4207 | void ufshcd_auto_hibern8_disable(struct ufs_hba *hba)

If using auto hibern8 mode, need to disable auto hibern8 while
entering suspend.
When using auto hibern8 mode, it does not seem right to send a uic command
for entry into hibern8 in the next line(ufshcd_lik_state_transition(..))
It seem right to send after disable auto hibern8.

In addition, if the auto hibern8 mode supported, it is enabled in resume.
So it seems that it will be paired only when auto hibern8 is disabled
while entering suspend.

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

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 460d2b440d2e..a6edbbd8ca2c 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -254,6 +254,7 @@ static void ufshcd_wb_toggle_flush_during_h8(struct ufs_hba *hba, bool set);
 static inline void ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool enable);
 static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba);
 static void ufshcd_hba_vreg_set_hpm(struct ufs_hba *hba);
+static void ufshcd_auto_hibern8_disable(struct ufs_hba *hba);
 
 static inline void ufshcd_enable_irq(struct ufs_hba *hba)
 {
@@ -4204,6 +4205,18 @@ void ufshcd_auto_hibern8_update(struct ufs_hba *hba, u32 ahit)
 }
 EXPORT_SYMBOL_GPL(ufshcd_auto_hibern8_update);
 
+static void ufshcd_auto_hibern8_disable(struct ufs_hba *hba)
+{
+	unsigned long flags;
+
+	if (!ufshcd_is_auto_hibern8_supported(hba))
+		return;
+
+	spin_lock_irqsave(hba->host->host_lock, flags);
+	ufshcd_writel(hba, 0, REG_AUTO_HIBERNATE_IDLE_TIMER);
+	spin_unlock_irqrestore(hba->host->host_lock, flags);
+}
+
 void ufshcd_auto_hibern8_enable(struct ufs_hba *hba)
 {
 	unsigned long flags;
@@ -8925,6 +8938,7 @@ static int __ufshcd_wl_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 	 * with the link off, so do not check for bkops.
 	 */
 	check_for_bkops = !ufshcd_is_ufs_dev_deepsleep(hba);
+	ufshcd_auto_hibern8_disable(hba);
 	ret = ufshcd_link_state_transition(hba, req_link_state, check_for_bkops);
 	if (ret)
 		goto set_dev_active;
-- 
2.26.0

Re: [PATCH v2] scsi: ufs: disable auto hibern8 while entering suspend

[Stanley Chu]

Hi Hoyoung,

On Tue, 2022-01-25 at 03:06 +0900, SEO HOYOUNG wrote:
> v1-> v2: fixed no previous prototype warning
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All warnings (new ones prefixed by >>):
> > > drivers/scsi/ufs/ufshcd.c:4207:6: warning: no previous prototype 
> 
> for 'ufshcd_auto_hibern8_disable' [-Wmissing-prototypes]
> 4207 | void ufshcd_auto_hibern8_disable(struct ufs_hba *hba)
> 
> If using auto hibern8 mode, need to disable auto hibern8 while
> entering suspend.
> When using auto hibern8 mode, it does not seem right to send a uic
> command

The UFSHCI spec does not mention the above rule.
Why would you need to disable AH8 before using UIC command to enter H8?

> for entry into hibern8 in the next
> line(ufshcd_lik_state_transition(..))
> It seem right to send after disable auto hibern8.
> 
> In addition, if the auto hibern8 mode supported, it is enabled in
> resume.
> So it seems that it will be paired only when auto hibern8 is disabled
> while entering suspend.
> 
> Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
> ---
>  drivers/scsi/ufs/ufshcd.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 460d2b440d2e..a6edbbd8ca2c 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -254,6 +254,7 @@ static void
> ufshcd_wb_toggle_flush_during_h8(struct ufs_hba *hba, bool set);
>  static inline void ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool
> enable);
>  static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba);
>  static void ufshcd_hba_vreg_set_hpm(struct ufs_hba *hba);
> +static void ufshcd_auto_hibern8_disable(struct ufs_hba *hba);
>  
>  static inline void ufshcd_enable_irq(struct ufs_hba *hba)
>  {
> @@ -4204,6 +4205,18 @@ void ufshcd_auto_hibern8_update(struct ufs_hba
> *hba, u32 ahit)
>  }
>  EXPORT_SYMBOL_GPL(ufshcd_auto_hibern8_update);
>  
> +static void ufshcd_auto_hibern8_disable(struct ufs_hba *hba)
> +{
> +	unsigned long flags;
> +
> +	if (!ufshcd_is_auto_hibern8_supported(hba))
> +		return;
> +
> +	spin_lock_irqsave(hba->host->host_lock, flags);
> +	ufshcd_writel(hba, 0, REG_AUTO_HIBERNATE_IDLE_TIMER);
> +	spin_unlock_irqrestore(hba->host->host_lock, flags);
> +}
> +
>  void ufshcd_auto_hibern8_enable(struct ufs_hba *hba)
>  {
>  	unsigned long flags;
> @@ -8925,6 +8938,7 @@ static int __ufshcd_wl_suspend(struct ufs_hba
> *hba, enum ufs_pm_op pm_op)
>  	 * with the link off, so do not check for bkops.
>  	 */
>  	check_for_bkops = !ufshcd_is_ufs_dev_deepsleep(hba);
> +	ufshcd_auto_hibern8_disable(hba);
>  	ret = ufshcd_link_state_transition(hba, req_link_state,
> check_for_bkops);
>  	if (ret)
>  		goto set_dev_active;

RE: [PATCH v2] scsi: ufs: disable auto hibern8 while entering suspend

[서호영]

Hi,
I think content is lacking in the UFSHCI spec.
In the process, AH8 is in operation, and it seems that sending the command
hibern8 by manual has a defeat.
I don't know what the all hci vendor's hardware design will be, but there is a possibility that ah8 and manual hibern8 may overlap.
So if is operating in ah8, it is thought that it will be safer to disable ah8 before sending hibern8 command.

> -----Original Message-----
> From: Stanley Chu [mailto:stanley.chu@mediatek.com]
> Sent: Wednesday, January 26, 2022 10:22 AM
> To: SEO HOYOUNG; 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; bhoon95.kim@samsung.com;
> kwmad.kim@samsung.com
> Cc: kernel test robot; peter.wang@mediatek.com
> Subject: Re: [PATCH v2] scsi: ufs: disable auto hibern8 while entering
> suspend
> 
> Hi Hoyoung,
> 
> On Tue, 2022-01-25 at 03:06 +0900, SEO HOYOUNG wrote:
> > v1-> v2: fixed no previous prototype warning
> > Reported-by: kernel test robot <lkp@intel.com>
> >
> > All warnings (new ones prefixed by >>):
> > > > drivers/scsi/ufs/ufshcd.c:4207:6: warning: no previous prototype
> >
> > for 'ufshcd_auto_hibern8_disable' [-Wmissing-prototypes]
> > 4207 | void ufshcd_auto_hibern8_disable(struct ufs_hba *hba)
> >
> > If using auto hibern8 mode, need to disable auto hibern8 while
> > entering suspend.
> > When using auto hibern8 mode, it does not seem right to send a uic
> > command
> 
> The UFSHCI spec does not mention the above rule.
> Why would you need to disable AH8 before using UIC command to enter H8?
> 
> > for entry into hibern8 in the next
> > line(ufshcd_lik_state_transition(..))
> > It seem right to send after disable auto hibern8.
> >
> > In addition, if the auto hibern8 mode supported, it is enabled in
> > resume.
> > So it seems that it will be paired only when auto hibern8 is disabled
> > while entering suspend.
> >
> > Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
> > ---
> >  drivers/scsi/ufs/ufshcd.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 460d2b440d2e..a6edbbd8ca2c 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -254,6 +254,7 @@ static void
> > ufshcd_wb_toggle_flush_during_h8(struct ufs_hba *hba, bool set);
> > static inline void ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool
> > enable);  static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba);
> > static void ufshcd_hba_vreg_set_hpm(struct ufs_hba *hba);
> > +static void ufshcd_auto_hibern8_disable(struct ufs_hba *hba);
> >
> >  static inline void ufshcd_enable_irq(struct ufs_hba *hba)  { @@
> > -4204,6 +4205,18 @@ void ufshcd_auto_hibern8_update(struct ufs_hba
> > *hba, u32 ahit)  }  EXPORT_SYMBOL_GPL(ufshcd_auto_hibern8_update);
> >
> > +static void ufshcd_auto_hibern8_disable(struct ufs_hba *hba) {
> > +	unsigned long flags;
> > +
> > +	if (!ufshcd_is_auto_hibern8_supported(hba))
> > +		return;
> > +
> > +	spin_lock_irqsave(hba->host->host_lock, flags);
> > +	ufshcd_writel(hba, 0, REG_AUTO_HIBERNATE_IDLE_TIMER);
> > +	spin_unlock_irqrestore(hba->host->host_lock, flags); }
> > +
> >  void ufshcd_auto_hibern8_enable(struct ufs_hba *hba)  {
> >  	unsigned long flags;
> > @@ -8925,6 +8938,7 @@ static int __ufshcd_wl_suspend(struct ufs_hba
> > *hba, enum ufs_pm_op pm_op)
> >  	 * with the link off, so do not check for bkops.
> >  	 */
> >  	check_for_bkops = !ufshcd_is_ufs_dev_deepsleep(hba);
> > +	ufshcd_auto_hibern8_disable(hba);
> >  	ret = ufshcd_link_state_transition(hba, req_link_state,
> > check_for_bkops);
> >  	if (ret)
> >  		goto set_dev_active;

RE: [PATCH v2] scsi: ufs: disable auto hibern8 while entering suspend

[Hoyoung SEO]

Hi,
Please check this patch.
If there is any other opinion, please give me comments
Thanks

> -----Original Message-----
> From: 서호영 [mailto:hy50.seo@samsung.com]
> Sent: Wednesday, January 26, 2022 2:35 PM
> 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';
> 'bhoon95.kim@samsung.com'; 'kwmad.kim@samsung.com'
> Cc: 'kernel test robot'
> Subject: RE: [PATCH v2] scsi: ufs: disable auto hibern8 while entering
> suspend
> 
> Hi,
> I think content is lacking in the UFSHCI spec.
> In the process, AH8 is in operation, and it seems that sending the command
> hibern8 by manual has a defeat.
> I don't know what the all hci vendor's hardware design will be, but there
> is a possibility that ah8 and manual hibern8 may overlap.
> So if is operating in ah8, it is thought that it will be safer to disable
> ah8 before sending hibern8 command.
> 
> > -----Original Message-----
> > From: Stanley Chu [mailto:stanley.chu@mediatek.com]
> > Sent: Wednesday, January 26, 2022 10:22 AM
> > To: SEO HOYOUNG; 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; bhoon95.kim@samsung.com; kwmad.kim@samsung.com
> > Cc: kernel test robot; peter.wang@mediatek.com
> > Subject: Re: [PATCH v2] scsi: ufs: disable auto hibern8 while entering
> > suspend
> >
> > Hi Hoyoung,
> >
> > On Tue, 2022-01-25 at 03:06 +0900, SEO HOYOUNG wrote:
> > > v1-> v2: fixed no previous prototype warning
> > > Reported-by: kernel test robot <lkp@intel.com>
> > >
> > > All warnings (new ones prefixed by >>):
> > > > > drivers/scsi/ufs/ufshcd.c:4207:6: warning: no previous prototype
> > >
> > > for 'ufshcd_auto_hibern8_disable' [-Wmissing-prototypes]
> > > 4207 | void ufshcd_auto_hibern8_disable(struct ufs_hba *hba)
> > >
> > > If using auto hibern8 mode, need to disable auto hibern8 while
> > > entering suspend.
> > > When using auto hibern8 mode, it does not seem right to send a uic
> > > command
> >
> > The UFSHCI spec does not mention the above rule.
> > Why would you need to disable AH8 before using UIC command to enter H8?
> >
> > > for entry into hibern8 in the next
> > > line(ufshcd_lik_state_transition(..))
> > > It seem right to send after disable auto hibern8.
> > >
> > > In addition, if the auto hibern8 mode supported, it is enabled in
> > > resume.
> > > So it seems that it will be paired only when auto hibern8 is
> > > disabled while entering suspend.
> > >
> > > Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
> > > ---
> > >  drivers/scsi/ufs/ufshcd.c | 14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > >
> > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > > index 460d2b440d2e..a6edbbd8ca2c 100644
> > > --- a/drivers/scsi/ufs/ufshcd.c
> > > +++ b/drivers/scsi/ufs/ufshcd.c
> > > @@ -254,6 +254,7 @@ static void
> > > ufshcd_wb_toggle_flush_during_h8(struct ufs_hba *hba, bool set);
> > > static inline void ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool
> > > enable);  static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba);
> > > static void ufshcd_hba_vreg_set_hpm(struct ufs_hba *hba);
> > > +static void ufshcd_auto_hibern8_disable(struct ufs_hba *hba);
> > >
> > >  static inline void ufshcd_enable_irq(struct ufs_hba *hba)  { @@
> > > -4204,6 +4205,18 @@ void ufshcd_auto_hibern8_update(struct ufs_hba
> > > *hba, u32 ahit)  }  EXPORT_SYMBOL_GPL(ufshcd_auto_hibern8_update);
> > >
> > > +static void ufshcd_auto_hibern8_disable(struct ufs_hba *hba) {
> > > +	unsigned long flags;
> > > +
> > > +	if (!ufshcd_is_auto_hibern8_supported(hba))
> > > +		return;
> > > +
> > > +	spin_lock_irqsave(hba->host->host_lock, flags);
> > > +	ufshcd_writel(hba, 0, REG_AUTO_HIBERNATE_IDLE_TIMER);
> > > +	spin_unlock_irqrestore(hba->host->host_lock, flags); }
> > > +
> > >  void ufshcd_auto_hibern8_enable(struct ufs_hba *hba)  {
> > >  	unsigned long flags;
> > > @@ -8925,6 +8938,7 @@ static int __ufshcd_wl_suspend(struct ufs_hba
> > > *hba, enum ufs_pm_op pm_op)
> > >  	 * with the link off, so do not check for bkops.
> > >  	 */
> > >  	check_for_bkops = !ufshcd_is_ufs_dev_deepsleep(hba);
> > > +	ufshcd_auto_hibern8_disable(hba);
> > >  	ret = ufshcd_link_state_transition(hba, req_link_state,
> > > check_for_bkops);
> > >  	if (ret)
> > >  		goto set_dev_active;

RE: [PATCH v2] scsi: ufs: disable auto hibern8 while entering suspend

[Avri Altman]

> Hi,
> Please check this patch.
> If there is any other opinion, please give me comments Thanks
> 
> > -----Original Message-----
> > From: 서호영 [mailto:hy50.seo@samsung.com]
> > Sent: Wednesday, January 26, 2022 2:35 PM
> > 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';
> 'bhoon95.kim@samsung.com'; 'kwmad.kim@samsung.com'
> > Cc: 'kernel test robot'
> > Subject: RE: [PATCH v2] scsi: ufs: disable auto hibern8 while entering
> > suspend
> >
> > Hi,
> > I think content is lacking in the UFSHCI spec.
> > In the process, AH8 is in operation, and it seems that sending the
> > command
> > hibern8 by manual has a defeat.
> > I don't know what the all hci vendor's hardware design will be, but
> > there is a possibility that ah8 and manual hibern8 may overlap.
> > So if is operating in ah8, it is thought that it will be safer to
> > disable
> > ah8 before sending hibern8 command.
Hi,
Thank you for your patch.
Maybe better to promote your idea in JEDEC first.

Thanks,
Avri
> >
> > > -----Original Message-----
> > > From: Stanley Chu [mailto:stanley.chu@mediatek.com]
> > > Sent: Wednesday, January 26, 2022 10:22 AM
> > > To: SEO HOYOUNG; 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; bhoon95.kim@samsung.com;
> kwmad.kim@samsung.com
> > > Cc: kernel test robot; peter.wang@mediatek.com
> > > Subject: Re: [PATCH v2] scsi: ufs: disable auto hibern8 while
> > > entering suspend
> > >
> > > Hi Hoyoung,
> > >
> > > On Tue, 2022-01-25 at 03:06 +0900, SEO HOYOUNG wrote:
> > > > v1-> v2: fixed no previous prototype warning
> > > > Reported-by: kernel test robot <lkp@intel.com>
> > > >
> > > > All warnings (new ones prefixed by >>):
> > > > > > drivers/scsi/ufs/ufshcd.c:4207:6: warning: no previous
> > > > > > prototype
> > > >
> > > > for 'ufshcd_auto_hibern8_disable' [-Wmissing-prototypes]
> > > > 4207 | void ufshcd_auto_hibern8_disable(struct ufs_hba *hba)
> > > >
> > > > If using auto hibern8 mode, need to disable auto hibern8 while
> > > > entering suspend.
> > > > When using auto hibern8 mode, it does not seem right to send a uic
> > > > command
> > >
> > > The UFSHCI spec does not mention the above rule.
> > > Why would you need to disable AH8 before using UIC command to enter
> H8?
> > >
> > > > for entry into hibern8 in the next
> > > > line(ufshcd_lik_state_transition(..))
> > > > It seem right to send after disable auto hibern8.
> > > >
> > > > In addition, if the auto hibern8 mode supported, it is enabled in
> > > > resume.
> > > > So it seems that it will be paired only when auto hibern8 is
> > > > disabled while entering suspend.
> > > >
> > > > Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
> > > > ---
> > > >  drivers/scsi/ufs/ufshcd.c | 14 ++++++++++++++
> > > >  1 file changed, 14 insertions(+)
> > > >
> > > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > > > index 460d2b440d2e..a6edbbd8ca2c 100644
> > > > --- a/drivers/scsi/ufs/ufshcd.c
> > > > +++ b/drivers/scsi/ufs/ufshcd.c
> > > > @@ -254,6 +254,7 @@ static void
> > > > ufshcd_wb_toggle_flush_during_h8(struct ufs_hba *hba, bool set);
> > > > static inline void ufshcd_wb_toggle_flush(struct ufs_hba *hba,
> > > > bool enable);  static void ufshcd_hba_vreg_set_lpm(struct ufs_hba
> > > > *hba); static void ufshcd_hba_vreg_set_hpm(struct ufs_hba *hba);
> > > > +static void ufshcd_auto_hibern8_disable(struct ufs_hba *hba);
> > > >
> > > >  static inline void ufshcd_enable_irq(struct ufs_hba *hba)  { @@
> > > > -4204,6 +4205,18 @@ void ufshcd_auto_hibern8_update(struct
> ufs_hba
> > > > *hba, u32 ahit)  }
> EXPORT_SYMBOL_GPL(ufshcd_auto_hibern8_update);
> > > >
> > > > +static void ufshcd_auto_hibern8_disable(struct ufs_hba *hba) {
> > > > +unsigned long flags;
> > > > +
> > > > + if (!ufshcd_is_auto_hibern8_supported(hba))
> > > > +         return;
> > > > +
> > > > + spin_lock_irqsave(hba->host->host_lock, flags);
> > > > + ufshcd_writel(hba, 0, REG_AUTO_HIBERNATE_IDLE_TIMER);
> > > > + spin_unlock_irqrestore(hba->host->host_lock, flags); }
> > > > +
> > > >  void ufshcd_auto_hibern8_enable(struct ufs_hba *hba)  {
> > > >   unsigned long flags;
> > > > @@ -8925,6 +8938,7 @@ static int __ufshcd_wl_suspend(struct
> > > > ufs_hba *hba, enum ufs_pm_op pm_op)
> > > >    * with the link off, so do not check for bkops.
> > > >    */
> > > >   check_for_bkops = !ufshcd_is_ufs_dev_deepsleep(hba);
> > > > + ufshcd_auto_hibern8_disable(hba);
> > > >   ret = ufshcd_link_state_transition(hba, req_link_state,
> > > > check_for_bkops);
> > > >   if (ret)
> > > >           goto set_dev_active;
> 
> 

RE: [PATCH v2] scsi: ufs: disable auto hibern8 while entering suspend

[Alim Akhtar]

Hi Hoyoung

>-----Original Message-----
>From: Hoyoung SEO [mailto:hy50.seo@samsung.com]
>Sent: Thursday, February 3, 2022 5:43 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;
>bhoon95.kim@samsung.com; kwmad.kim@samsung.com
>Cc: 'kernel test robot' <lkp@intel.com>
>Subject: RE: [PATCH v2] scsi: ufs: disable auto hibern8 while entering suspend
>
>Hi,
>Please check this patch.
>If there is any other opinion, please give me comments Thanks
>
>> -----Original Message-----
>> From: 서호영 [mailto:hy50.seo@samsung.com]
>> Sent: Wednesday, January 26, 2022 2:35 PM
>> 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';
>'bhoon95.kim@samsung.com'; 'kwmad.kim@samsung.com'
>> Cc: 'kernel test robot'
>> Subject: RE: [PATCH v2] scsi: ufs: disable auto hibern8 while entering
>> suspend
>>
>> Hi,
>> I think content is lacking in the UFSHCI spec.
>> In the process, AH8 is in operation, and it seems that sending the
>> command
>> hibern8 by manual has a defeat.
>> I don't know what the all hci vendor's hardware design will be, but
>> there is a possibility that ah8 and manual hibern8 may overlap.
>> So if is operating in ah8, it is thought that it will be safer to
>> disable
>> ah8 before sending hibern8 command.
>>
I am not sure, if this problem is generic and faced by all other UFS vendors.
If not, how about having a vendor specific call back for your platform only?
Just a thought.

>> > -----Original Message-----
>> > From: Stanley Chu [mailto:stanley.chu@mediatek.com]
>> > Sent: Wednesday, January 26, 2022 10:22 AM
>> > To: SEO HOYOUNG; 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; bhoon95.kim@samsung.com;
>kwmad.kim@samsung.com
>> > Cc: kernel test robot; peter.wang@mediatek.com
>> > Subject: Re: [PATCH v2] scsi: ufs: disable auto hibern8 while
>> > entering suspend
>> >
>> > Hi Hoyoung,
>> >
>> > On Tue, 2022-01-25 at 03:06 +0900, SEO HOYOUNG wrote:
>> > > v1-> v2: fixed no previous prototype warning
>> > > Reported-by: kernel test robot <lkp@intel.com>
>> > >
>> > > All warnings (new ones prefixed by >>):
>> > > > > drivers/scsi/ufs/ufshcd.c:4207:6: warning: no previous
>> > > > > prototype
>> > >
>> > > for 'ufshcd_auto_hibern8_disable' [-Wmissing-prototypes]
>> > > 4207 | void ufshcd_auto_hibern8_disable(struct ufs_hba *hba)
>> > >
>> > > If using auto hibern8 mode, need to disable auto hibern8 while
>> > > entering suspend.
>> > > When using auto hibern8 mode, it does not seem right to send a uic
>> > > command
>> >
>> > The UFSHCI spec does not mention the above rule.
>> > Why would you need to disable AH8 before using UIC command to enter
>H8?
>> >
>> > > for entry into hibern8 in the next
>> > > line(ufshcd_lik_state_transition(..))
>> > > It seem right to send after disable auto hibern8.
>> > >
>> > > In addition, if the auto hibern8 mode supported, it is enabled in
>> > > resume.
>> > > So it seems that it will be paired only when auto hibern8 is
>> > > disabled while entering suspend.
>> > >
>> > > Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
>> > > ---
>> > >  drivers/scsi/ufs/ufshcd.c | 14 ++++++++++++++
>> > >  1 file changed, 14 insertions(+)
>> > >
>> > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> > > index 460d2b440d2e..a6edbbd8ca2c 100644
>> > > --- a/drivers/scsi/ufs/ufshcd.c
>> > > +++ b/drivers/scsi/ufs/ufshcd.c
>> > > @@ -254,6 +254,7 @@ static void
>> > > ufshcd_wb_toggle_flush_during_h8(struct ufs_hba *hba, bool set);
>> > > static inline void ufshcd_wb_toggle_flush(struct ufs_hba *hba,
>> > > bool enable);  static void ufshcd_hba_vreg_set_lpm(struct ufs_hba
>> > > *hba); static void ufshcd_hba_vreg_set_hpm(struct ufs_hba *hba);
>> > > +static void ufshcd_auto_hibern8_disable(struct ufs_hba *hba);
>> > >
>> > >  static inline void ufshcd_enable_irq(struct ufs_hba *hba)  { @@
>> > > -4204,6 +4205,18 @@ void ufshcd_auto_hibern8_update(struct ufs_hba
>> > > *hba, u32 ahit)  }
>EXPORT_SYMBOL_GPL(ufshcd_auto_hibern8_update);
>> > >
>> > > +static void ufshcd_auto_hibern8_disable(struct ufs_hba *hba) {
>> > > +	unsigned long flags;
>> > > +
>> > > +	if (!ufshcd_is_auto_hibern8_supported(hba))
>> > > +		return;
>> > > +
>> > > +	spin_lock_irqsave(hba->host->host_lock, flags);
>> > > +	ufshcd_writel(hba, 0, REG_AUTO_HIBERNATE_IDLE_TIMER);
>> > > +	spin_unlock_irqrestore(hba->host->host_lock, flags); }
>> > > +
>> > >  void ufshcd_auto_hibern8_enable(struct ufs_hba *hba)  {
>> > >  	unsigned long flags;
>> > > @@ -8925,6 +8938,7 @@ static int __ufshcd_wl_suspend(struct
>> > > ufs_hba *hba, enum ufs_pm_op pm_op)
>> > >  	 * with the link off, so do not check for bkops.
>> > >  	 */
>> > >  	check_for_bkops = !ufshcd_is_ufs_dev_deepsleep(hba);
>> > > +	ufshcd_auto_hibern8_disable(hba);
>> > >  	ret = ufshcd_link_state_transition(hba, req_link_state,
>> > > check_for_bkops);
>> > >  	if (ret)
>> > >  		goto set_dev_active;
>
>

Re: [PATCH v2] scsi: ufs: disable auto hibern8 while entering suspend

[Bart Van Assche]

On 2/2/22 22:50, Alim Akhtar wrote:
> I am not sure, if this problem is generic and faced by all other UFS vendors.
> If not, how about having a vendor specific call back for your platform only?
> Just a thought.

I agree with the above. Since the code change does not follow from the 
spec, it should be guarded with a check for a new quirk flag.

Thanks,

Bart.