| Message ID | 20221011021653.27277-1-gaoyankaigeren@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | ufs: core: Disable auto h8 before ssu | expand |
Hi Ten,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on jejb-scsi/for-next linus/master v6.0 next-20221010]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ten-Gao/ufs-core-Disable-auto-h8-before-ssu/20221011-101736
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: loongarch-allyesconfig
compiler: loongarch64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/fe9c4ce70b9c5de43eef7a4043facfda4c95c842
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Ten-Gao/ufs-core-Disable-auto-h8-before-ssu/20221011-101736
git checkout fe9c4ce70b9c5de43eef7a4043facfda4c95c842
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=loongarch SHELL=/bin/bash drivers/ufs/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/ufs/core/ufshcd.c:4265:6: warning: no previous prototype for 'ufshcd_auto_hibern8_disable' [-Wmissing-prototypes]
4265 | void ufshcd_auto_hibern8_disable(struct ufs_hba *hba)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/ufshcd_auto_hibern8_disable +4265 drivers/ufs/core/ufshcd.c
4264
> 4265 void ufshcd_auto_hibern8_disable(struct ufs_hba *hba)
4266 {
4267 if (!ufshcd_is_auto_hibern8_supported(hba))
4268 return;
4269
4270 ufshcd_writel(hba, 0, REG_AUTO_HIBERNATE_IDLE_TIMER);
4271 }
4272
> From: Ten Gao <ten.gao@unisoc.com> > > Ensure auto h8 will not hit dme h8,and there won't be two h8 in a row after > ssu. I don't think the hw should do that. Can you please share on which platform/host controller did you observe this? Thanks, Avri > > Signed-off-by: Ten Gao <ten.gao@unisoc.com> > --- > drivers/ufs/core/ufshcd.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index > a202d7d5240d..42f93648d796 100644 > --- a/drivers/ufs/core/ufshcd.c > +++ b/drivers/ufs/core/ufshcd.c > @@ -4256,6 +4256,14 @@ void ufshcd_auto_hibern8_update(struct ufs_hba > *hba, u32 ahit) } EXPORT_SYMBOL_GPL(ufshcd_auto_hibern8_update); > > +void ufshcd_auto_hibern8_disable(struct ufs_hba *hba) { > + if (!ufshcd_is_auto_hibern8_supported(hba)) > + return; > + > + ufshcd_writel(hba, 0, REG_AUTO_HIBERNATE_IDLE_TIMER); } > + > void ufshcd_auto_hibern8_enable(struct ufs_hba *hba) { > if (!ufshcd_is_auto_hibern8_supported(hba)) > @@ -9036,6 +9044,8 @@ static int __ufshcd_wl_suspend(struct ufs_hba > *hba, enum ufs_pm_op pm_op) > if (ret) > goto enable_scaling; > > + ufshcd_auto_hibern8_disable(hba); > + > if (req_dev_pwr_mode != hba->curr_dev_pwr_mode) { > if (pm_op != UFS_RUNTIME_PM) > /* ensure that bkops is disabled */ > -- > 2.17.1
Hi Ten,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on jejb-scsi/for-next linus/master v6.0 next-20221011]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ten-Gao/ufs-core-Disable-auto-h8-before-ssu/20221011-101736
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: i386-randconfig-a003-20221010
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/fe9c4ce70b9c5de43eef7a4043facfda4c95c842
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Ten-Gao/ufs-core-Disable-auto-h8-before-ssu/20221011-101736
git checkout fe9c4ce70b9c5de43eef7a4043facfda4c95c842
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/ufs/core/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/ufs/core/ufshcd.c:4265:6: warning: no previous prototype for function 'ufshcd_auto_hibern8_disable' [-Wmissing-prototypes]
void ufshcd_auto_hibern8_disable(struct ufs_hba *hba)
^
drivers/ufs/core/ufshcd.c:4265:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void ufshcd_auto_hibern8_disable(struct ufs_hba *hba)
^
static
1 warning generated.
vim +/ufshcd_auto_hibern8_disable +4265 drivers/ufs/core/ufshcd.c
4264
> 4265 void ufshcd_auto_hibern8_disable(struct ufs_hba *hba)
4266 {
4267 if (!ufshcd_is_auto_hibern8_supported(hba))
4268 return;
4269
4270 ufshcd_writel(hba, 0, REG_AUTO_HIBERNATE_IDLE_TIMER);
4271 }
4272
Dear Avri Unisoc reports resume fail on UFS(micron FS164) during suspend/resume test. We check host inserts auto H8 enter/exit event between SSU sleep command and H8 enter command in runtime suspend . Asfollows: SSU Sleep command --> auto H8 enter --> auto H8 exit --> H8 enter --> idle 2ms --> VCC off. However device AQL FW can’t enter LPM within 2ms after second H8 enter command. FW already enter LPM after receive auto H8 enter command , Next auto H8 exit command will trigger FW exit from LPM, it need take over 10ms, and FW can’t enter LPM again after second H8 enter command until device complete exit from LPM. So disable auto h8 before ssu is a reasonable solution to solve it. Hynix also has similar request. Avri Altman <Avri.Altman@wdc.com> 于2022年10月11日周二 15:07写道： > > > From: Ten Gao <ten.gao@unisoc.com> > > > > Ensure auto h8 will not hit dme h8,and there won't be two h8 in a row after > > ssu. > I don't think the hw should do that. > Can you please share on which platform/host controller did you observe this? > > Thanks, > Avri > > > > Signed-off-by: Ten Gao <ten.gao@unisoc.com> > > --- > > drivers/ufs/core/ufshcd.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index > > a202d7d5240d..42f93648d796 100644 > > --- a/drivers/ufs/core/ufshcd.c > > +++ b/drivers/ufs/core/ufshcd.c > > @@ -4256,6 +4256,14 @@ void ufshcd_auto_hibern8_update(struct ufs_hba > > *hba, u32 ahit) } EXPORT_SYMBOL_GPL(ufshcd_auto_hibern8_update); > > > > +void ufshcd_auto_hibern8_disable(struct ufs_hba *hba) { > > + if (!ufshcd_is_auto_hibern8_supported(hba)) > > + return; > > + > > + ufshcd_writel(hba, 0, REG_AUTO_HIBERNATE_IDLE_TIMER); } > > + > > void ufshcd_auto_hibern8_enable(struct ufs_hba *hba) { > > if (!ufshcd_is_auto_hibern8_supported(hba)) > > @@ -9036,6 +9044,8 @@ static int __ufshcd_wl_suspend(struct ufs_hba > > *hba, enum ufs_pm_op pm_op) > > if (ret) > > goto enable_scaling; > > > > + ufshcd_auto_hibern8_disable(hba); > > + > > if (req_dev_pwr_mode != hba->curr_dev_pwr_mode) { > > if (pm_op != UFS_RUNTIME_PM) > > /* ensure that bkops is disabled */ > > -- > > 2.17.1 >
> > Dear Avri > Unisoc reports resume fail on UFS(micron FS164) during suspend/resume > test. > We check host inserts auto H8 enter/exit event between SSU sleep > command and H8 enter command in runtime suspend . > Asfollows: SSU Sleep command --> auto H8 enter --> auto H8 exit --> > H8 enter --> idle 2ms --> VCC off. > However device AQL FW can’t enter LPM within 2ms after second H8 enter > command. > FW already enter LPM after receive auto H8 enter command , Next auto > H8 exit command will trigger FW exit from LPM, it need take over 10ms, and > FW can’t enter > LPM again after second H8 enter command until device complete exit from > LPM. So disable auto h8 before ssu is a reasonable solution to solve it. > Hynix also has similar request. Is this something common to all platforms? If not, and you need your platform to disable h8 before ssu, You can implement it in your own vop - see e.g. commit 9561f58442e4 (scsi: ufs: mediatek: Support vops pre suspend to disable auto-hibern8) Thanks, Avri > > Avri Altman <Avri.Altman@wdc.com> 于2022年10月11日周二 15:07写道： > > > > > From: Ten Gao <ten.gao@unisoc.com> > > > > > > Ensure auto h8 will not hit dme h8,and there won't be two h8 in a > > > row after ssu. > > I don't think the hw should do that. > > Can you please share on which platform/host controller did you observe > this? > > > > Thanks, > > Avri > > > > > > Signed-off-by: Ten Gao <ten.gao@unisoc.com> > > > --- > > > drivers/ufs/core/ufshcd.c | 10 ++++++++++ > > > 1 file changed, 10 insertions(+) > > > > > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > > > index > > > a202d7d5240d..42f93648d796 100644 > > > --- a/drivers/ufs/core/ufshcd.c > > > +++ b/drivers/ufs/core/ufshcd.c > > > @@ -4256,6 +4256,14 @@ void ufshcd_auto_hibern8_update(struct > > > ufs_hba *hba, u32 ahit) } > > > EXPORT_SYMBOL_GPL(ufshcd_auto_hibern8_update); > > > > > > +void ufshcd_auto_hibern8_disable(struct ufs_hba *hba) { > > > + if (!ufshcd_is_auto_hibern8_supported(hba)) > > > + return; > > > + > > > + ufshcd_writel(hba, 0, REG_AUTO_HIBERNATE_IDLE_TIMER); } > > > + > > > void ufshcd_auto_hibern8_enable(struct ufs_hba *hba) { > > > if (!ufshcd_is_auto_hibern8_supported(hba)) > > > @@ -9036,6 +9044,8 @@ static int __ufshcd_wl_suspend(struct ufs_hba > > > *hba, enum ufs_pm_op pm_op) > > > if (ret) > > > goto enable_scaling; > > > > > > + ufshcd_auto_hibern8_disable(hba); > > > + > > > if (req_dev_pwr_mode != hba->curr_dev_pwr_mode) { > > > if (pm_op != UFS_RUNTIME_PM) > > > /* ensure that bkops is disabled */ > > > -- > > > 2.17.1 > >
> > > > Dear Avri > > Unisoc reports resume fail on UFS(micron FS164) during suspend/resume > > test. > > We check host inserts auto H8 enter/exit event between SSU sleep > > command and H8 enter command in runtime suspend . > > Asfollows: SSU Sleep command --> auto H8 enter --> auto H8 exit --> > > H8 enter --> idle 2ms --> VCC off. > > However device AQL FW can’t enter LPM within 2ms after second H8 > enter > > command. > > FW already enter LPM after receive auto H8 enter command , Next auto > > H8 exit command will trigger FW exit from LPM, it need take over 10ms, > and > > FW can’t enter > > LPM again after second H8 enter command until device complete exit > from > > LPM. So disable auto h8 before ssu is a reasonable solution to solve it. > > Hynix also has similar request. > Is this something common to all platforms? > If not, and you need your platform to disable h8 before ssu, > You can implement it in your own vop - see e.g. > commit 9561f58442e4 (scsi: ufs: mediatek: Support vops pre suspend to > disable auto-hibern8) Maybe to further clarify, I am not saying that your suggestion doesn't make sense. It's just that you need, IMHO, to make it part of ufshcd_vops_suspend, which I think Its ufshcd_system_suspend for every platform. And you need to get an ack on that from the other platform owners (maybe except Stanley who is doing it already). Btw, you are not checking the pm_op so your are disabling auto-h8 for runtime-suspend As well. And maybe elaborate the commit log with your explanation above - making it more clear. Thanks, Avri > > Thanks, > Avri > > > > > Avri Altman <Avri.Altman@wdc.com> 于2022年10月11日周二 15:07写道 > ： > > > > > > > From: Ten Gao <ten.gao@unisoc.com> > > > > > > > > Ensure auto h8 will not hit dme h8,and there won't be two h8 in a > > > > row after ssu. > > > I don't think the hw should do that. > > > Can you please share on which platform/host controller did you observe > > this? > > > > > > Thanks, > > > Avri > > > > > > > > Signed-off-by: Ten Gao <ten.gao@unisoc.com> > > > > --- > > > > drivers/ufs/core/ufshcd.c | 10 ++++++++++ > > > > 1 file changed, 10 insertions(+) > > > > > > > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > > > > index > > > > a202d7d5240d..42f93648d796 100644 > > > > --- a/drivers/ufs/core/ufshcd.c > > > > +++ b/drivers/ufs/core/ufshcd.c > > > > @@ -4256,6 +4256,14 @@ void ufshcd_auto_hibern8_update(struct > > > > ufs_hba *hba, u32 ahit) } > > > > EXPORT_SYMBOL_GPL(ufshcd_auto_hibern8_update); > > > > > > > > +void ufshcd_auto_hibern8_disable(struct ufs_hba *hba) { > > > > + if (!ufshcd_is_auto_hibern8_supported(hba)) > > > > + return; > > > > + > > > > + ufshcd_writel(hba, 0, REG_AUTO_HIBERNATE_IDLE_TIMER); } > > > > + > > > > void ufshcd_auto_hibern8_enable(struct ufs_hba *hba) { > > > > if (!ufshcd_is_auto_hibern8_supported(hba)) > > > > @@ -9036,6 +9044,8 @@ static int __ufshcd_wl_suspend(struct > ufs_hba > > > > *hba, enum ufs_pm_op pm_op) > > > > if (ret) > > > > goto enable_scaling; > > > > > > > > + ufshcd_auto_hibern8_disable(hba); > > > > + > > > > if (req_dev_pwr_mode != hba->curr_dev_pwr_mode) { > > > > if (pm_op != UFS_RUNTIME_PM) > > > > /* ensure that bkops is disabled */ > > > > -- > > > > 2.17.1 > > >
Dear Avri TKS for your suggestion. and adjust timimg in own vop can solve this problem surely. but I think it's more of a common patch for all platforms, Patch can adapt different vendors,and align to ufshcd_auto_hibern8_enable in __ufshcd_wl_resume. Not distinguishing pm_op can refer to ufshcd_auto_hibern8_enable in __ufshcd_wl_resume. Avri Altman <Avri.Altman@wdc.com> 于2022年10月12日周三 16:13写道： > > > > > > > Dear Avri > > > Unisoc reports resume fail on UFS(micron FS164) during suspend/resume > > > test. > > > We check host inserts auto H8 enter/exit event between SSU sleep > > > command and H8 enter command in runtime suspend . > > > Asfollows: SSU Sleep command --> auto H8 enter --> auto H8 exit --> > > > H8 enter --> idle 2ms --> VCC off. > > > However device AQL FW can’t enter LPM within 2ms after second H8 > > enter > > > command. > > > FW already enter LPM after receive auto H8 enter command , Next auto > > > H8 exit command will trigger FW exit from LPM, it need take over 10ms, > > and > > > FW can’t enter > > > LPM again after second H8 enter command until device complete exit > > from > > > LPM. So disable auto h8 before ssu is a reasonable solution to solve it. > > > Hynix also has similar request. > > Is this something common to all platforms? > > If not, and you need your platform to disable h8 before ssu, > > You can implement it in your own vop - see e.g. > > commit 9561f58442e4 (scsi: ufs: mediatek: Support vops pre suspend to > > disable auto-hibern8) > Maybe to further clarify, I am not saying that your suggestion doesn't make sense. > It's just that you need, IMHO, to make it part of ufshcd_vops_suspend, which I think > Its ufshcd_system_suspend for every platform. > And you need to get an ack on that from the other platform owners (maybe except > Stanley who is doing it already). > > Btw, you are not checking the pm_op so your are disabling auto-h8 for runtime-suspend > As well. > > And maybe elaborate the commit log with your explanation above - making it more clear. > > Thanks, > Avri > > > > Thanks, > > Avri > > > > > > > > Avri Altman <Avri.Altman@wdc.com> 于2022年10月11日周二 15:07写道 > > ： > > > > > > > > > From: Ten Gao <ten.gao@unisoc.com> > > > > > > > > > > Ensure auto h8 will not hit dme h8,and there won't be two h8 in a > > > > > row after ssu. > > > > I don't think the hw should do that. > > > > Can you please share on which platform/host controller did you observe > > > this? > > > > > > > > Thanks, > > > > Avri > > > > > > > > > > Signed-off-by: Ten Gao <ten.gao@unisoc.com> > > > > > --- > > > > > drivers/ufs/core/ufshcd.c | 10 ++++++++++ > > > > > 1 file changed, 10 insertions(+) > > > > > > > > > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > > > > > index > > > > > a202d7d5240d..42f93648d796 100644 > > > > > --- a/drivers/ufs/core/ufshcd.c > > > > > +++ b/drivers/ufs/core/ufshcd.c > > > > > @@ -4256,6 +4256,14 @@ void ufshcd_auto_hibern8_update(struct > > > > > ufs_hba *hba, u32 ahit) } > > > > > EXPORT_SYMBOL_GPL(ufshcd_auto_hibern8_update); > > > > > > > > > > +void ufshcd_auto_hibern8_disable(struct ufs_hba *hba) { > > > > > + if (!ufshcd_is_auto_hibern8_supported(hba)) > > > > > + return; > > > > > + > > > > > + ufshcd_writel(hba, 0, REG_AUTO_HIBERNATE_IDLE_TIMER); } > > > > > + > > > > > void ufshcd_auto_hibern8_enable(struct ufs_hba *hba) { > > > > > if (!ufshcd_is_auto_hibern8_supported(hba)) > > > > > @@ -9036,6 +9044,8 @@ static int __ufshcd_wl_suspend(struct > > ufs_hba > > > > > *hba, enum ufs_pm_op pm_op) > > > > > if (ret) > > > > > goto enable_scaling; > > > > > > > > > > + ufshcd_auto_hibern8_disable(hba); > > > > > + > > > > > if (req_dev_pwr_mode != hba->curr_dev_pwr_mode) { > > > > > if (pm_op != UFS_RUNTIME_PM) > > > > > /* ensure that bkops is disabled */ > > > > > -- > > > > > 2.17.1 > > > >
On Tue, 2022-10-11 at 19:53 +0800, 高严凯 wrote: > Dear Avri > Unisoc reports resume fail on UFS(micron FS164) during > suspend/resume test. > We check host inserts auto H8 enter/exit event between SSU sleep > command and H8 enter command in runtime suspend . > Asfollows: SSU Sleep command --> auto H8 enter --> auto H8 exit --> > H8 enter --> idle 2ms --> VCC off. > However device AQL FW can’t enter LPM within 2ms after second H8 > enter command. > FW already enter LPM after receive auto H8 enter command , Next > auto > H8 exit command will trigger FW exit from LPM, it need take over > 10ms, > and FW can’t enter > LPM again after second H8 enter command until device complete exit > from LPM. So disable auto h8 before ssu is a reasonable solution to > solve it. > Hynix also has similar request. Hi gaoyan, The above sequence confuses me. UFSHCI has idle time before bringing Link into hibern8 mode, do you know what the settings are? Also, if auto-hibern8 is supported, in this case, the host-side SW should not send manual hibern8. If this is an issue to all UFS or UFSHCI, we should disable manual hibern8 if auto-hibern8 is enabled. or let customer to choose one of two. Kind regards, Bean
Dear Bean we can choose one of two h8 for sure, and we chosed auto h8 during sys active. when ufs suspend/resume, ufshcd_link_state_transtion will send DME h8 enter/exit always. that's why the above sequence show up. " UFSHCI has idle time before bringing Link into hibern8 mode, " I think ufs put the link to h8 during the time Bean Huo <huobean@gmail.com> 于2022年10月12日周三 22:25写道： > > On Tue, 2022-10-11 at 19:53 +0800, 高严凯 wrote: > > Dear Avri > > Unisoc reports resume fail on UFS(micron FS164) during > > suspend/resume test. > > We check host inserts auto H8 enter/exit event between SSU sleep > > command and H8 enter command in runtime suspend . > > Asfollows: SSU Sleep command --> auto H8 enter --> auto H8 exit --> > > H8 enter --> idle 2ms --> VCC off. > > However device AQL FW can’t enter LPM within 2ms after second H8 > > enter command. > > FW already enter LPM after receive auto H8 enter command , Next > > auto > > H8 exit command will trigger FW exit from LPM, it need take over > > 10ms, > > and FW can’t enter > > LPM again after second H8 enter command until device complete exit > > from LPM. So disable auto h8 before ssu is a reasonable solution to > > solve it. > > Hynix also has similar request. > > > Hi gaoyan, > > The above sequence confuses me. UFSHCI has idle time before bringing > Link into hibern8 mode, do you know what the settings are? > > Also, if auto-hibern8 is supported, in this case, the host-side SW > should not send manual hibern8. If this is an issue to all UFS or > UFSHCI, we should disable manual hibern8 if auto-hibern8 is enabled. or > let customer to choose one of two. > > > Kind regards, > Bean
On Fri, 2022-10-14 at 10:44 +0800, 高严凯 wrote: > " UFSHCI has idle time before bringing Link into hibern8 mode, " I > think ufs put the link to h8 during the time no, it is ufs controller. see the ufshci spec: "Auto-Hibern8 Idle Timer Value (AH8ITV): This is the timer that UFS subsystem must be idle before UFS host *controller* may put UniPro link into Hibernate state autonomously. The idle timer value is multiplied by the indicated timer scale to yield an absolute timer value. .... ... Any non-zero value will enable Auto-Hibernate idle timer. UFS host controller shall put Unipro link out of Hibernate state when the link communication is required. The mechanism to decide when the Unipro link needs to become active is host controller specific implementation, and is transparent to the software."
hi Bean Yes, This is the time from when the system enters idle to when auto h8 is issued. This patch is just to ensure that there is no continuous action of "auto h8 exit" before dme h8 enter, it has nothing to do with idle time. The reason I want to ensure this order is because when there is an action of "auto h8 exit", most devices take a certain amount of time to get the state ready. Immediately after the action of "auto h8 exit" is completed and "dme h8 enter" is followed, the device will be abnormal. And I think it's more of a common patch for all platforms, Patch can adapt different vendors,and align to ufshcd_auto_hibern8_enable in __ufshcd_wl_resume. Bean Huo <huobean@gmail.com> 于2022年10月14日周五 13:48写道： > > On Fri, 2022-10-14 at 10:44 +0800, 高严凯 wrote: > > " UFSHCI has idle time before bringing Link into hibern8 mode, " I > > think ufs put the link to h8 during the time > > > no, it is ufs controller. see the ufshci spec: > > "Auto-Hibern8 Idle Timer Value (AH8ITV): This is the timer that UFS > subsystem must be idle before UFS host *controller* may put UniPro link > into Hibernate state autonomously. The idle timer value is multiplied > by the indicated timer scale to yield an absolute timer value. .... > > ... > Any non-zero value will enable Auto-Hibernate idle timer. > UFS host controller shall put Unipro link out of Hibernate state when > the link communication is required. The mechanism to decide when the > Unipro link needs to become active is host controller specific > implementation, and is transparent to the software." > >
Dear ALL Do you agree this patch? I'd like to hear your opinion about this. Thanks. 高严凯 <gaoyankaigeren@gmail.com> 于2022年10月14日周五 18:15写道： > > hi Bean > Yes, This is the time from when the system enters idle to when auto h8 > is issued. > This patch is just to ensure that there is no continuous action of > "auto h8 exit" before dme h8 enter, it has nothing to do with idle > time. > The reason I want to ensure this order is because when there is an > action of "auto h8 exit", most devices take a certain amount of time > to get the state ready. > Immediately after the action of "auto h8 exit" is completed and "dme > h8 enter" is followed, the device will be abnormal. > And I think it's more of a common patch for all platforms, Patch can > adapt different vendors,and align to ufshcd_auto_hibern8_enable in > __ufshcd_wl_resume. > > Bean Huo <huobean@gmail.com> 于2022年10月14日周五 13:48写道： > > > > On Fri, 2022-10-14 at 10:44 +0800, 高严凯 wrote: > > > " UFSHCI has idle time before bringing Link into hibern8 mode, " I > > > think ufs put the link to h8 during the time > > > > > > no, it is ufs controller. see the ufshci spec: > > > > "Auto-Hibern8 Idle Timer Value (AH8ITV): This is the timer that UFS > > subsystem must be idle before UFS host *controller* may put UniPro link > > into Hibernate state autonomously. The idle timer value is multiplied > > by the indicated timer scale to yield an absolute timer value. .... > > > > ... > > Any non-zero value will enable Auto-Hibernate idle timer. > > UFS host controller shall put Unipro link out of Hibernate state when > > the link communication is required. The mechanism to decide when the > > Unipro link needs to become active is host controller specific > > implementation, and is transparent to the software." > > > >
On 10/19/22 22:15, 高严凯 wrote:
> Do you agree this patch? I'd like to hear your opinion about this.
Hi,
On Linux kernel mailing lists it is expected that replies are posted
below the original email instead of above. Please follow this
convention. More information is available at
https://en.wikipedia.org/wiki/Posting_style#Bottom-posting.
Please also do the following:
* Fix the issues reported by the kernel test robot.
* Follow the recommendations from Avri and Bean.
Thanks,
Bart.
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index a202d7d5240d..42f93648d796 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -4256,6 +4256,14 @@ void ufshcd_auto_hibern8_update(struct ufs_hba *hba, u32 ahit) } EXPORT_SYMBOL_GPL(ufshcd_auto_hibern8_update); +void ufshcd_auto_hibern8_disable(struct ufs_hba *hba) +{ + if (!ufshcd_is_auto_hibern8_supported(hba)) + return; + + ufshcd_writel(hba, 0, REG_AUTO_HIBERNATE_IDLE_TIMER); +} + void ufshcd_auto_hibern8_enable(struct ufs_hba *hba) { if (!ufshcd_is_auto_hibern8_supported(hba)) @@ -9036,6 +9044,8 @@ static int __ufshcd_wl_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) if (ret) goto enable_scaling; + ufshcd_auto_hibern8_disable(hba); + if (req_dev_pwr_mode != hba->curr_dev_pwr_mode) { if (pm_op != UFS_RUNTIME_PM) /* ensure that bkops is disabled */