Messages in this thread | | | Date | Mon, 24 Apr 2023 15:56:06 +0530 | Subject | Re: [PATCH] spi: bcm63xx: remove PM_SLEEP based conditional compilation | From | Dhruva Gole <> |
| |
Hi Jonas,
On 24/04/23 13:50, Jonas Gorski wrote: > On Fri, 21 Apr 2023 at 19:17, Florian Fainelli <f.fainelli@gmail.com> wrote: >> >> On 4/20/23 05:16, Dhruva Gole wrote: >>> Get rid of conditional compilation based on CONFIG_PM_SLEEP because >>> it may introduce build issues with certain configs where it maybe disabled >>> This is because if above config is not enabled the suspend-resume >>> functions are never part of the code but the bcm63xx_spi_pm_ops struct >>> still inits them to non-existent suspend-resume functions. >>> >>> Fixes: b42dfed83d95 ("spi: add Broadcom BCM63xx SPI controller driver") >>> >>> Signed-off-by: Dhruva Gole <d-gole@ti.com> >>> --- >>> drivers/spi/spi-bcm63xx.c | 2 -- >>> 1 file changed, 2 deletions(-) >>> >>> diff --git a/drivers/spi/spi-bcm63xx.c b/drivers/spi/spi-bcm63xx.c >>> index 96633a0051b1..99395932074c 100644 >>> --- a/drivers/spi/spi-bcm63xx.c >>> +++ b/drivers/spi/spi-bcm63xx.c >>> @@ -617,7 +617,6 @@ static void bcm63xx_spi_remove(struct platform_device *pdev) >>> clk_disable_unprepare(bs->clk); >>> } >>> >>> -#ifdef CONFIG_PM_SLEEP >>> static int bcm63xx_spi_suspend(struct device *dev) >> >> Don't we need a __maybe_unused here? > > Actually the premise of this patch is wrong, and should rather be reverted. > > The bcm63xx_spi_pm_ops struct is initialized with > SET_SYSTEM_SLEEP_PM_OPS(), which is defined as > > #ifdef CONFIG_PM_SLEEP > #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \ > SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) > #else > #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) > #endif >
Thanks for pointing this out, I must have missed this. Anyway, I have sent another patch to migrate to using DEFINE_SIMPLE_DEV_PM_OPS as per Mark's suggestion [0]. There I think it would be necessary to remove the CONFIG_PM_SLEEP checks in the driver. So no need to revert this patch.
> so for !CONFIG_PM_SLEEP it won't initialize the struct at all (or > reference non-existing functions), and therefore there will be no > build issues. > > Regards, > Jonas
[0] https://lore.kernel.org/all/e65683c1-9f1b-4cfb-8e14-087ef7d69595@sirena.org.uk/
-- Thanks and Regards, Dhruva Gole
| |