* [PATCH v3 0/1] scsi: ufs: Add hibernation callbacks @ 2023-01-20 11:33 Anjana Hari 2023-01-20 11:33 ` [PATCH v3 1/1] " Anjana Hari 2023-01-20 21:12 ` [PATCH v3 0/1] " Bart Van Assche 0 siblings, 2 replies; 6+ messages in thread From: Anjana Hari @ 2023-01-20 11:33 UTC (permalink / raw) To: agross, andersson, jejb, martin.petersen Cc: alim.akhtar, avri.altman, bvanassche, konrad.dybcio, linux-scsi, linux-kernel, quic_narepall, quic_nitirawa, quic_rampraka, Anjana Hari This patch adds hibernation callbacks in UFS driver. Please take a look and let us know your thoughts. v3: -Address compilation issues v2: - Addressed Bart's comments - Moved core and host related changes to single patch - Note to Bart: Regrading the comment to pass "restore" as an argument instead of adding a new member to ufs_hba structure, adding new function argument in core file (ufshcd.c) is forcing us to make changes to other vendor files to fix the compilation errors. Hence we have retained our original change. Please let us know your inputs on this. Initial version: - Adds hibernation callbacks - freeze, restore and thaw, required for suspend to disk feature. Anjana Hari (1): scsi: ufs: Add hibernation callbacks drivers/ufs/core/ufshcd.c | 62 +++++++++++++++++++++++++++++++++++++ drivers/ufs/host/ufs-qcom.c | 6 +++- include/ufs/ufshcd.h | 8 +++++ 3 files changed, 75 insertions(+), 1 deletion(-) -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/1] scsi: ufs: Add hibernation callbacks 2023-01-20 11:33 [PATCH v3 0/1] scsi: ufs: Add hibernation callbacks Anjana Hari @ 2023-01-20 11:33 ` Anjana Hari 2023-01-20 21:27 ` Bart Van Assche ` (2 more replies) 2023-01-20 21:12 ` [PATCH v3 0/1] " Bart Van Assche 1 sibling, 3 replies; 6+ messages in thread From: Anjana Hari @ 2023-01-20 11:33 UTC (permalink / raw) To: agross, andersson, jejb, martin.petersen Cc: alim.akhtar, avri.altman, bvanassche, konrad.dybcio, linux-scsi, linux-kernel, quic_narepall, quic_nitirawa, quic_rampraka, Anjana Hari Adds freeze, thaw and restore callbacks for hibernate and restore functionality. Signed-off-by: Anjana Hari <quic_ahari@quicinc.com> --- drivers/ufs/core/ufshcd.c | 62 +++++++++++++++++++++++++++++++++++++ drivers/ufs/host/ufs-qcom.c | 6 +++- include/ufs/ufshcd.h | 8 +++++ 3 files changed, 75 insertions(+), 1 deletion(-) diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index fcd46251f7a8..d68222bb73ad 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -9826,11 +9826,36 @@ static int ufshcd_resume(struct ufs_hba *hba) /* enable the host irq as host controller would be active soon */ ufshcd_enable_irq(hba); + + if (hba->restore) { + /* Configure UTRL and UTMRL base address registers */ + ufshcd_writel(hba, lower_32_bits(hba->utrdl_dma_addr), + REG_UTP_TRANSFER_REQ_LIST_BASE_L); + ufshcd_writel(hba, upper_32_bits(hba->utrdl_dma_addr), + REG_UTP_TRANSFER_REQ_LIST_BASE_H); + ufshcd_writel(hba, lower_32_bits(hba->utmrdl_dma_addr), + REG_UTP_TASK_REQ_LIST_BASE_L); + ufshcd_writel(hba, upper_32_bits(hba->utmrdl_dma_addr), + REG_UTP_TASK_REQ_LIST_BASE_H); + /* Make sure that UTRL and UTMRL base address registers + * are updated with the latest queue addresses. Only after + * updating these addresses, we can queue the new commands. + */ + mb(); + } + + /* Resuming from hibernate, assume that link was OFF */ + if (hba->restore) + ufshcd_set_link_off(hba); + goto out; disable_vreg: ufshcd_vreg_set_lpm(hba); out: + if (hba->restore) + hba->restore = false; + if (ret) ufshcd_update_evt_hist(hba, UFS_EVT_RESUME_ERR, (u32)ret); return ret; @@ -9989,6 +10014,43 @@ void ufshcd_remove(struct ufs_hba *hba) } EXPORT_SYMBOL_GPL(ufshcd_remove); +int ufshcd_system_freeze(struct device *dev) +{ + + struct ufs_hba *hba = dev_get_drvdata(dev); + int ret = 0; + + /* + * Run time resume the controller to make sure + * the PM work queue threads do not try to resume + * the child (scsi host), which leads to errors as + * the controller is not yet resumed. + */ + pm_runtime_get_sync(hba->dev); + ret = ufshcd_system_suspend(dev); + pm_runtime_put_sync(hba->dev); + + return ret; +} +EXPORT_SYMBOL_GPL(ufshcd_system_freeze); + +int ufshcd_system_restore(struct device *dev) +{ + + struct ufs_hba *hba = dev_get_drvdata(dev); + + hba->restore = true; + return ufshcd_system_resume(dev); + +} +EXPORT_SYMBOL_GPL(ufshcd_system_restore); + +int ufshcd_system_thaw(struct device *dev) +{ + return ufshcd_system_resume(dev); +} +EXPORT_SYMBOL_GPL(ufshcd_system_thaw); + /** * ufshcd_dealloc_host - deallocate Host Bus Adapter (HBA) * @hba: pointer to Host Bus Adapter (HBA) diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c index 681da3ea7154..c92e041c5361 100644 --- a/drivers/ufs/host/ufs-qcom.c +++ b/drivers/ufs/host/ufs-qcom.c @@ -1714,10 +1714,14 @@ MODULE_DEVICE_TABLE(acpi, ufs_qcom_acpi_match); #endif static const struct dev_pm_ops ufs_qcom_pm_ops = { - SET_SYSTEM_SLEEP_PM_OPS(ufshcd_system_suspend, ufshcd_system_resume) SET_RUNTIME_PM_OPS(ufshcd_runtime_suspend, ufshcd_runtime_resume, NULL) .prepare = ufshcd_suspend_prepare, .complete = ufshcd_resume_complete, + .suspend = ufshcd_system_suspend, + .resume = ufshcd_system_resume, + .freeze = ufshcd_system_freeze, + .restore = ufshcd_system_restore, + .thaw = ufshcd_system_thaw, }; static struct platform_driver ufs_qcom_pltform = { diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h index 1779238d8a56..6f50390ca262 100644 --- a/include/ufs/ufshcd.h +++ b/include/ufs/ufshcd.h @@ -1071,6 +1071,9 @@ struct ufs_hba { struct ufs_hw_queue *uhq; struct ufs_hw_queue *dev_cmd_queue; struct ufshcd_mcq_opr_info_t mcq_opr[OPR_MAX]; + + /* Distinguish between resume and restore */ + bool restore; }; /** @@ -1278,6 +1281,11 @@ extern int ufshcd_system_suspend(struct device *dev); extern int ufshcd_system_resume(struct device *dev); #endif extern int ufshcd_shutdown(struct ufs_hba *hba); + +extern int ufshcd_system_freeze(struct device *dev); +extern int ufshcd_system_thaw(struct device *dev); +extern int ufshcd_system_restore(struct device *dev); + extern int ufshcd_dme_configure_adapt(struct ufs_hba *hba, int agreed_gear, int adapt_val); -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/1] scsi: ufs: Add hibernation callbacks 2023-01-20 11:33 ` [PATCH v3 1/1] " Anjana Hari @ 2023-01-20 21:27 ` Bart Van Assche 2023-01-21 4:42 ` kernel test robot 2023-01-21 4:42 ` kernel test robot 2 siblings, 0 replies; 6+ messages in thread From: Bart Van Assche @ 2023-01-20 21:27 UTC (permalink / raw) To: Anjana Hari, agross, andersson, jejb, martin.petersen Cc: alim.akhtar, avri.altman, konrad.dybcio, linux-scsi, linux-kernel, quic_narepall, quic_nitirawa, quic_rampraka On 1/20/23 03:33, Anjana Hari wrote: > + /* Make sure that UTRL and UTMRL base address registers > + * are updated with the latest queue addresses. Only after > + * updating these addresses, we can queue the new commands. > + */ Please follow the kernel coding style and start comment blocks with "/*" on a line by its own. > + /* Resuming from hibernate, assume that link was OFF */ > + if (hba->restore) > + ufshcd_set_link_off(hba); Why two successive 'if (hba->restore)' statements? Can these two if-statements be combined into a single if-statement? > +int ufshcd_system_freeze(struct device *dev) > +{ > + > + struct ufs_hba *hba = dev_get_drvdata(dev); > + int ret = 0; > + > + /* > + * Run time resume the controller to make sure > + * the PM work queue threads do not try to resume > + * the child (scsi host), which leads to errors as > + * the controller is not yet resumed. > + */ > + pm_runtime_get_sync(hba->dev); > + ret = ufshcd_system_suspend(dev); > + pm_runtime_put_sync(hba->dev); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(ufshcd_system_freeze); Why does the above function use 'hba->dev' instead of 'dev'? I do not understand the comment in the above function. My understanding is that the power management core serializes hibernation and runtime power management. How could runtime resume be in progress when ufshcd_system_freeze() is called? Additionally, shouldn't ufshcd_system_freeze() skip the UFS controller if it is already runtime suspended instead of runtime resuming it? Some time ago I added the following code in sd_suspend_system() (commit 9131bff6a9f1 ("scsi: core: pm: Only runtime resume if necessary"): if (pm_runtime_suspended(dev)) return 0; Thanks, Bart. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/1] scsi: ufs: Add hibernation callbacks 2023-01-20 11:33 ` [PATCH v3 1/1] " Anjana Hari 2023-01-20 21:27 ` Bart Van Assche @ 2023-01-21 4:42 ` kernel test robot 2023-01-21 4:42 ` kernel test robot 2 siblings, 0 replies; 6+ messages in thread From: kernel test robot @ 2023-01-21 4:42 UTC (permalink / raw) To: Anjana Hari, agross, andersson, jejb, martin.petersen Cc: oe-kbuild-all, alim.akhtar, avri.altman, bvanassche, konrad.dybcio, linux-scsi, linux-kernel, quic_narepall, quic_nitirawa, quic_rampraka, Anjana Hari Hi Anjana, Thank you for the patch! Yet something to improve: [auto build test ERROR on mkp-scsi/for-next] [also build test ERROR on next-20230120] [cannot apply to jejb-scsi/for-next linus/master v6.2-rc4] [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/Anjana-Hari/scsi-ufs-Add-hibernation-callbacks/20230120-193447 base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next patch link: https://lore.kernel.org/r/20230120113321.30433-2-quic_ahari%40quicinc.com patch subject: [PATCH v3 1/1] scsi: ufs: Add hibernation callbacks config: arc-randconfig-r043-20230119 (https://download.01.org/0day-ci/archive/20230121/202301211209.zKDppcLI-lkp@intel.com/config) compiler: arceb-elf-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/239ad2244616006dd39bc9a5380108435d168a86 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Anjana-Hari/scsi-ufs-Add-hibernation-callbacks/20230120-193447 git checkout 239ad2244616006dd39bc9a5380108435d168a86 # 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=arc olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc 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 errors (new ones prefixed by >>): drivers/ufs/core/ufshcd.c: In function 'ufshcd_system_freeze': >> drivers/ufs/core/ufshcd.c:10004:15: error: implicit declaration of function 'ufshcd_system_suspend'; did you mean 'trace_ufshcd_system_suspend'? [-Werror=implicit-function-declaration] 10004 | ret = ufshcd_system_suspend(dev); | ^~~~~~~~~~~~~~~~~~~~~ | trace_ufshcd_system_suspend drivers/ufs/core/ufshcd.c: In function 'ufshcd_system_restore': >> drivers/ufs/core/ufshcd.c:10017:16: error: implicit declaration of function 'ufshcd_system_resume'; did you mean 'ufshcd_system_restore'? [-Werror=implicit-function-declaration] 10017 | return ufshcd_system_resume(dev); | ^~~~~~~~~~~~~~~~~~~~ | ufshcd_system_restore cc1: some warnings being treated as errors vim +10004 drivers/ufs/core/ufshcd.c 9993 9994 struct ufs_hba *hba = dev_get_drvdata(dev); 9995 int ret = 0; 9996 9997 /* 9998 * Run time resume the controller to make sure 9999 * the PM work queue threads do not try to resume 10000 * the child (scsi host), which leads to errors as 10001 * the controller is not yet resumed. 10002 */ 10003 pm_runtime_get_sync(hba->dev); 10004 ret = ufshcd_system_suspend(dev); 10005 pm_runtime_put_sync(hba->dev); 10006 10007 return ret; 10008 } 10009 EXPORT_SYMBOL_GPL(ufshcd_system_freeze); 10010 10011 int ufshcd_system_restore(struct device *dev) 10012 { 10013 10014 struct ufs_hba *hba = dev_get_drvdata(dev); 10015 10016 hba->restore = true; 10017 return ufshcd_system_resume(dev); 10018 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/1] scsi: ufs: Add hibernation callbacks 2023-01-20 11:33 ` [PATCH v3 1/1] " Anjana Hari 2023-01-20 21:27 ` Bart Van Assche 2023-01-21 4:42 ` kernel test robot @ 2023-01-21 4:42 ` kernel test robot 2 siblings, 0 replies; 6+ messages in thread From: kernel test robot @ 2023-01-21 4:42 UTC (permalink / raw) To: Anjana Hari, agross, andersson, jejb, martin.petersen Cc: llvm, oe-kbuild-all, alim.akhtar, avri.altman, bvanassche, konrad.dybcio, linux-scsi, linux-kernel, quic_narepall, quic_nitirawa, quic_rampraka, Anjana Hari Hi Anjana, Thank you for the patch! Yet something to improve: [auto build test ERROR on mkp-scsi/for-next] [also build test ERROR on next-20230120] [cannot apply to jejb-scsi/for-next linus/master v6.2-rc4] [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/Anjana-Hari/scsi-ufs-Add-hibernation-callbacks/20230120-193447 base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next patch link: https://lore.kernel.org/r/20230120113321.30433-2-quic_ahari%40quicinc.com patch subject: [PATCH v3 1/1] scsi: ufs: Add hibernation callbacks config: riscv-randconfig-r042-20230119 (https://download.01.org/0day-ci/archive/20230121/202301211209.4byNcJjL-lkp@intel.com/config) compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 4196ca3278f78c6e19246e54ab0ecb364e37d66a) 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 # install riscv cross compiling tool for clang build # apt-get install binutils-riscv64-linux-gnu # https://github.com/intel-lab-lkp/linux/commit/239ad2244616006dd39bc9a5380108435d168a86 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Anjana-Hari/scsi-ufs-Add-hibernation-callbacks/20230120-193447 git checkout 239ad2244616006dd39bc9a5380108435d168a86 # 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=riscv olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv 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 errors (new ones prefixed by >>): >> drivers/ufs/core/ufshcd.c:10004:8: error: call to undeclared function 'ufshcd_system_suspend'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] ret = ufshcd_system_suspend(dev); ^ >> drivers/ufs/core/ufshcd.c:10017:9: error: call to undeclared function 'ufshcd_system_resume'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] return ufshcd_system_resume(dev); ^ drivers/ufs/core/ufshcd.c:10017:9: note: did you mean 'ufshcd_system_restore'? drivers/ufs/core/ufshcd.c:10011:5: note: 'ufshcd_system_restore' declared here int ufshcd_system_restore(struct device *dev) ^ drivers/ufs/core/ufshcd.c:10024:9: error: call to undeclared function 'ufshcd_system_resume'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] return ufshcd_system_resume(dev); ^ drivers/ufs/core/ufshcd.c:10048:44: warning: shift count >= width of type [-Wshift-count-overflow] if (!dma_set_mask_and_coherent(hba->dev, DMA_BIT_MASK(64))) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~ include/linux/dma-mapping.h:76:54: note: expanded from macro 'DMA_BIT_MASK' #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1)) ^ include/linux/compiler.h:56:47: note: expanded from macro 'if' #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) ) ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~ include/linux/compiler.h:58:52: note: expanded from macro '__trace_if_var' #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond)) ^~~~ drivers/ufs/core/ufshcd.c:10048:44: warning: shift count >= width of type [-Wshift-count-overflow] if (!dma_set_mask_and_coherent(hba->dev, DMA_BIT_MASK(64))) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~ include/linux/dma-mapping.h:76:54: note: expanded from macro 'DMA_BIT_MASK' #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1)) ^ include/linux/compiler.h:56:47: note: expanded from macro 'if' #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) ) ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~ include/linux/compiler.h:58:61: note: expanded from macro '__trace_if_var' #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond)) ^~~~ drivers/ufs/core/ufshcd.c:10048:44: warning: shift count >= width of type [-Wshift-count-overflow] if (!dma_set_mask_and_coherent(hba->dev, DMA_BIT_MASK(64))) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~ include/linux/dma-mapping.h:76:54: note: expanded from macro 'DMA_BIT_MASK' #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1)) ^ include/linux/compiler.h:56:47: note: expanded from macro 'if' #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) ) ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~ include/linux/compiler.h:58:86: note: expanded from macro '__trace_if_var' #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond)) ~~~~~~~~~~~~~~~~~^~~~~ include/linux/compiler.h:69:3: note: expanded from macro '__trace_if_value' (cond) ? \ ^~~~ 3 warnings and 3 errors generated. vim +/ufshcd_system_suspend +10004 drivers/ufs/core/ufshcd.c 9993 9994 struct ufs_hba *hba = dev_get_drvdata(dev); 9995 int ret = 0; 9996 9997 /* 9998 * Run time resume the controller to make sure 9999 * the PM work queue threads do not try to resume 10000 * the child (scsi host), which leads to errors as 10001 * the controller is not yet resumed. 10002 */ 10003 pm_runtime_get_sync(hba->dev); 10004 ret = ufshcd_system_suspend(dev); 10005 pm_runtime_put_sync(hba->dev); 10006 10007 return ret; 10008 } 10009 EXPORT_SYMBOL_GPL(ufshcd_system_freeze); 10010 10011 int ufshcd_system_restore(struct device *dev) 10012 { 10013 10014 struct ufs_hba *hba = dev_get_drvdata(dev); 10015 10016 hba->restore = true; 10017 return ufshcd_system_resume(dev); 10018 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 0/1] scsi: ufs: Add hibernation callbacks 2023-01-20 11:33 [PATCH v3 0/1] scsi: ufs: Add hibernation callbacks Anjana Hari 2023-01-20 11:33 ` [PATCH v3 1/1] " Anjana Hari @ 2023-01-20 21:12 ` Bart Van Assche 1 sibling, 0 replies; 6+ messages in thread From: Bart Van Assche @ 2023-01-20 21:12 UTC (permalink / raw) To: Anjana Hari, agross, andersson, jejb, martin.petersen Cc: alim.akhtar, avri.altman, konrad.dybcio, linux-scsi, linux-kernel, quic_narepall, quic_nitirawa, quic_rampraka On 1/20/23 03:33, Anjana Hari wrote: > - Note to Bart: Regrading the comment to pass "restore" as an > argument instead of adding a new member to ufs_hba structure, adding > new function argument in core file (ufshcd.c) is forcing us to make > changes to other vendor files to fix the compilation errors. Hence > we have retained our original change. Please let us know your inputs > on this. Storing state information in a structure member that can be passed as a function argument makes code harder to read and to maintain than necessary. Please address my request before this patch goes upstream. I'm concerned if someone would try to address my request after this patch went upstream that there would be no motivation from your side to help with testing the refactoring patch. I think the patch below shows that it is easy to eliminate the new 'restore' member variable. Please note that the patch below has not been tested in any way. --- drivers/ufs/core/ufshcd.c | 48 +++++++++++++++++++-------------------- include/ufs/ufshcd.h | 3 --- 2 files changed, 23 insertions(+), 28 deletions(-) diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 19608f3a38f9..b5cfbc1fccc6 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -9801,34 +9801,11 @@ static int ufshcd_resume(struct ufs_hba *hba) /* enable the host irq as host controller would be active soon */ ufshcd_enable_irq(hba); - if (hba->restore) { - /* Configure UTRL and UTMRL base address registers */ - ufshcd_writel(hba, lower_32_bits(hba->utrdl_dma_addr), - REG_UTP_TRANSFER_REQ_LIST_BASE_L); - ufshcd_writel(hba, upper_32_bits(hba->utrdl_dma_addr), - REG_UTP_TRANSFER_REQ_LIST_BASE_H); - ufshcd_writel(hba, lower_32_bits(hba->utmrdl_dma_addr), - REG_UTP_TASK_REQ_LIST_BASE_L); - ufshcd_writel(hba, upper_32_bits(hba->utmrdl_dma_addr), - REG_UTP_TASK_REQ_LIST_BASE_H); - /* Make sure that UTRL and UTMRL base address registers - * are updated with the latest queue addresses. Only after - * updating these addresses, we can queue the new commands. - */ - mb(); - } - - /* Resuming from hibernate, assume that link was OFF */ - if (hba->restore) - ufshcd_set_link_off(hba); - goto out; disable_vreg: ufshcd_vreg_set_lpm(hba); out: - if (hba->restore) - hba->restore = false; if (ret) ufshcd_update_evt_hist(hba, UFS_EVT_RESUME_ERR, (u32)ret); @@ -10012,10 +9989,31 @@ int ufshcd_system_restore(struct device *dev) { struct ufs_hba *hba = dev_get_drvdata(dev); + int ret; - hba->restore = true; - return ufshcd_system_resume(dev); + ret = ufshcd_system_resume(dev); + if (ret) + return ret; + + /* Configure UTRL and UTMRL base address registers */ + ufshcd_writel(hba, lower_32_bits(hba->utrdl_dma_addr), + REG_UTP_TRANSFER_REQ_LIST_BASE_L); + ufshcd_writel(hba, upper_32_bits(hba->utrdl_dma_addr), + REG_UTP_TRANSFER_REQ_LIST_BASE_H); + ufshcd_writel(hba, lower_32_bits(hba->utmrdl_dma_addr), + REG_UTP_TASK_REQ_LIST_BASE_L); + ufshcd_writel(hba, upper_32_bits(hba->utmrdl_dma_addr), + REG_UTP_TASK_REQ_LIST_BASE_H); + /* Make sure that UTRL and UTMRL base address registers + * are updated with the latest queue addresses. Only after + * updating these addresses, we can queue the new commands. + */ + mb(); + /* Resuming from hibernate, assume that link was OFF */ + ufshcd_set_link_off(hba); + + return 0; } EXPORT_SYMBOL_GPL(ufshcd_system_restore); diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h index 6f50390ca262..1d6dd13e1651 100644 --- a/include/ufs/ufshcd.h +++ b/include/ufs/ufshcd.h @@ -1071,9 +1071,6 @@ struct ufs_hba { struct ufs_hw_queue *uhq; struct ufs_hw_queue *dev_cmd_queue; struct ufshcd_mcq_opr_info_t mcq_opr[OPR_MAX]; - - /* Distinguish between resume and restore */ - bool restore; }; /** ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-01-21 4:43 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-01-20 11:33 [PATCH v3 0/1] scsi: ufs: Add hibernation callbacks Anjana Hari 2023-01-20 11:33 ` [PATCH v3 1/1] " Anjana Hari 2023-01-20 21:27 ` Bart Van Assche 2023-01-21 4:42 ` kernel test robot 2023-01-21 4:42 ` kernel test robot 2023-01-20 21:12 ` [PATCH v3 0/1] " Bart Van Assche
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).