| Message ID | 20240823214235.1718769-1-briannorris@chromium.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | spi: rockchip: Resolve unbalanced runtime PM / system PM handling | expand |
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
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 */
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(-)