[PATCH] spi: rockchip: Resolve unbalanced runtime PM / system PM handling

[PATCH] spi: rockchip: Resolve unbalanced runtime PM / system PM handling

[Brian Norris]

Commit e882575efc77 ("spi: rockchip: Suspend and resume the bus during
NOIRQ_SYSTEM_SLEEP_PM ops") stopped respecting runtime PM status and
simply disabled clocks unconditionally when suspending the system. This
causes problems when the device is already runtime suspended when we go
to sleep -- in which case we double-disable clocks and produce a
WARNing.

Switch back to pm_runtime_force_{suspend,resume}(), because that still
seems like the right thing to do, and the aforementioned commit makes no
explanation why it stopped using it.

Also, refactor some of the resume() error handling, because it's not
actually a good idea to re-disable clocks on failure.

Fixes: e882575efc77 ("spi: rockchip: Suspend and resume the bus during NOIRQ_SYSTEM_SLEEP_PM ops")
Cc: <stable@vger.kernel.org>
Reported-by: "Ondřej Jirman" <megi@xff.cz>
Closes: https://lore.kernel.org/lkml/20220621154218.sau54jeij4bunf56@core/
Signed-off-by: Brian Norris <briannorris@chromium.org>
---

 drivers/spi/spi-rockchip.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
index e1ecd96c7858..f30af4316b8b 100644
--- a/drivers/spi/spi-rockchip.c
+++ b/drivers/spi/spi-rockchip.c
@@ -951,8 +951,11 @@ static int rockchip_spi_suspend(struct device *dev)
 	if (ret < 0)
 		return ret;
 
-	clk_disable_unprepare(rs->spiclk);
-	clk_disable_unprepare(rs->apb_pclk);
+	ret = pm_runtime_force_suspend(dev);
+	if (ret < 0) {
+		spi_controller_resume(ctlr);
+		return ret;
+	}
 
 	pinctrl_pm_select_sleep_state(dev);
 
@@ -967,21 +970,11 @@ static int rockchip_spi_resume(struct device *dev)
 
 	pinctrl_pm_select_default_state(dev);
 
-	ret = clk_prepare_enable(rs->apb_pclk);
+	ret = pm_runtime_force_resume(dev);
 	if (ret < 0)
 		return ret;
 
-	ret = clk_prepare_enable(rs->spiclk);
-	if (ret < 0)
-		clk_disable_unprepare(rs->apb_pclk);
-
-	ret = spi_controller_resume(ctlr);
-	if (ret < 0) {
-		clk_disable_unprepare(rs->spiclk);
-		clk_disable_unprepare(rs->apb_pclk);
-	}
-
-	return 0;
+	return spi_controller_resume(ctlr);
 }
 #endif /* CONFIG_PM_SLEEP */
 
-- 
2.46.0.295.g3b9ea8a38a-goog

Re: [PATCH] spi: rockchip: Resolve unbalanced runtime PM / system PM handling

[kernel test robot]

Hi Brian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rockchip/for-next]
[also build test WARNING on broonie-sound/for-next broonie-spi/for-next linus/master v6.11-rc5 next-20240826]
[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/Brian-Norris/spi-rockchip-Resolve-unbalanced-runtime-PM-system-PM-handling/20240826-135245
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git for-next
patch link:    https://lore.kernel.org/r/20240823214235.1718769-1-briannorris%40chromium.org
patch subject: [PATCH] spi: rockchip: Resolve unbalanced runtime PM / system PM handling
config: loongarch-allmodconfig (https://download.01.org/0day-ci/archive/20240827/202408271225.Zh4Kc31M-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240827/202408271225.Zh4Kc31M-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408271225.Zh4Kc31M-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/spi/spi-rockchip.c: In function 'rockchip_spi_suspend':
>> drivers/spi/spi-rockchip.c:948:30: warning: unused variable 'rs' [-Wunused-variable]
     948 |         struct rockchip_spi *rs = spi_controller_get_devdata(ctlr);
         |                              ^~
   drivers/spi/spi-rockchip.c: In function 'rockchip_spi_resume':
   drivers/spi/spi-rockchip.c:969:30: warning: unused variable 'rs' [-Wunused-variable]
     969 |         struct rockchip_spi *rs = spi_controller_get_devdata(ctlr);
         |                              ^~


vim +/rs +948 drivers/spi/spi-rockchip.c

64e36824b32b06 addy ke      2014-07-01  942  
64e36824b32b06 addy ke      2014-07-01  943  #ifdef CONFIG_PM_SLEEP
64e36824b32b06 addy ke      2014-07-01  944  static int rockchip_spi_suspend(struct device *dev)
64e36824b32b06 addy ke      2014-07-01  945  {
43de979ddc099c Jeffy Chen   2017-08-07  946  	int ret;
d66571a20f68f1 Chris Ruehl  2020-05-11  947  	struct spi_controller *ctlr = dev_get_drvdata(dev);
e882575efc771f shengfei Xu  2022-02-16 @948  	struct rockchip_spi *rs = spi_controller_get_devdata(ctlr);
64e36824b32b06 addy ke      2014-07-01  949  
d66571a20f68f1 Chris Ruehl  2020-05-11  950  	ret = spi_controller_suspend(ctlr);
43de979ddc099c Jeffy Chen   2017-08-07  951  	if (ret < 0)
64e36824b32b06 addy ke      2014-07-01  952  		return ret;
64e36824b32b06 addy ke      2014-07-01  953  
8d3fb5bc7d0206 Brian Norris 2024-08-23  954  	ret = pm_runtime_force_suspend(dev);
8d3fb5bc7d0206 Brian Norris 2024-08-23  955  	if (ret < 0) {
8d3fb5bc7d0206 Brian Norris 2024-08-23  956  		spi_controller_resume(ctlr);
8d3fb5bc7d0206 Brian Norris 2024-08-23  957  		return ret;
8d3fb5bc7d0206 Brian Norris 2024-08-23  958  	}
64e36824b32b06 addy ke      2014-07-01  959  
23e291c2e4c84a Brian Norris 2016-12-16  960  	pinctrl_pm_select_sleep_state(dev);
23e291c2e4c84a Brian Norris 2016-12-16  961  
43de979ddc099c Jeffy Chen   2017-08-07  962  	return 0;
64e36824b32b06 addy ke      2014-07-01  963  }
64e36824b32b06 addy ke      2014-07-01  964  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki