Message ID | 20211011135836.1045437-1-fabien.dessenne@foss.st.com (mailing list archive) |
---|---|
State | Accepted |
Headers | show |
Series | [v2] hwspinlock: stm32: enable clock at probe | expand |
Hi Any comments on this patch? BR Fabien On 11/10/2021 15:58, Fabien Dessenne wrote: > Set the clock during probe and keep its control during suspend / resume > operations. > This fixes an issue when CONFIG_PM is not set and where the clock is > never enabled. > > Make use of devm_ functions to simplify the code. > > Signed-off-by: Fabien Dessenne <fabien.dessenne@foss.st.com> > --- > Changes since v1: > - Call platform_set_drvdata() before pm_runtime_...() calls > --- > drivers/hwspinlock/stm32_hwspinlock.c | 58 +++++++++++++++++---------- > 1 file changed, 37 insertions(+), 21 deletions(-) > > diff --git a/drivers/hwspinlock/stm32_hwspinlock.c b/drivers/hwspinlock/stm32_hwspinlock.c > index 3ad0ce0da4d9..5bd11a7fab65 100644 > --- a/drivers/hwspinlock/stm32_hwspinlock.c > +++ b/drivers/hwspinlock/stm32_hwspinlock.c > @@ -54,8 +54,23 @@ static const struct hwspinlock_ops stm32_hwspinlock_ops = { > .relax = stm32_hwspinlock_relax, > }; > > +static void stm32_hwspinlock_disable_clk(void *data) > +{ > + struct platform_device *pdev = data; > + struct stm32_hwspinlock *hw = platform_get_drvdata(pdev); > + struct device *dev = &pdev->dev; > + > + pm_runtime_get_sync(dev); > + pm_runtime_disable(dev); > + pm_runtime_set_suspended(dev); > + pm_runtime_put_noidle(dev); > + > + clk_disable_unprepare(hw->clk); > +} > + > static int stm32_hwspinlock_probe(struct platform_device *pdev) > { > + struct device *dev = &pdev->dev; > struct stm32_hwspinlock *hw; > void __iomem *io_base; > size_t array_size; > @@ -66,41 +81,43 @@ static int stm32_hwspinlock_probe(struct platform_device *pdev) > return PTR_ERR(io_base); > > array_size = STM32_MUTEX_NUM_LOCKS * sizeof(struct hwspinlock); > - hw = devm_kzalloc(&pdev->dev, sizeof(*hw) + array_size, GFP_KERNEL); > + hw = devm_kzalloc(dev, sizeof(*hw) + array_size, GFP_KERNEL); > if (!hw) > return -ENOMEM; > > - hw->clk = devm_clk_get(&pdev->dev, "hsem"); > + hw->clk = devm_clk_get(dev, "hsem"); > if (IS_ERR(hw->clk)) > return PTR_ERR(hw->clk); > > - for (i = 0; i < STM32_MUTEX_NUM_LOCKS; i++) > - hw->bank.lock[i].priv = io_base + i * sizeof(u32); > + ret = clk_prepare_enable(hw->clk); > + if (ret) { > + dev_err(dev, "Failed to prepare_enable clock\n"); > + return ret; > + } > > platform_set_drvdata(pdev, hw); > - pm_runtime_enable(&pdev->dev); > > - ret = hwspin_lock_register(&hw->bank, &pdev->dev, &stm32_hwspinlock_ops, > - 0, STM32_MUTEX_NUM_LOCKS); > + pm_runtime_get_noresume(dev); > + pm_runtime_set_active(dev); > + pm_runtime_enable(dev); > + pm_runtime_put(dev); > > - if (ret) > - pm_runtime_disable(&pdev->dev); > + ret = devm_add_action_or_reset(dev, stm32_hwspinlock_disable_clk, pdev); > + if (ret) { > + dev_err(dev, "Failed to register action\n"); > + return ret; > + } > > - return ret; > -} > + for (i = 0; i < STM32_MUTEX_NUM_LOCKS; i++) > + hw->bank.lock[i].priv = io_base + i * sizeof(u32); > > -static int stm32_hwspinlock_remove(struct platform_device *pdev) > -{ > - struct stm32_hwspinlock *hw = platform_get_drvdata(pdev); > - int ret; > + ret = devm_hwspin_lock_register(dev, &hw->bank, &stm32_hwspinlock_ops, > + 0, STM32_MUTEX_NUM_LOCKS); > > - ret = hwspin_lock_unregister(&hw->bank); > if (ret) > - dev_err(&pdev->dev, "%s failed: %d\n", __func__, ret); > - > - pm_runtime_disable(&pdev->dev); > + dev_err(dev, "Failed to register hwspinlock\n"); > > - return 0; > + return ret; > } > > static int __maybe_unused stm32_hwspinlock_runtime_suspend(struct device *dev) > @@ -135,7 +152,6 @@ MODULE_DEVICE_TABLE(of, stm32_hwpinlock_ids); > > static struct platform_driver stm32_hwspinlock_driver = { > .probe = stm32_hwspinlock_probe, > - .remove = stm32_hwspinlock_remove, > .driver = { > .name = "stm32_hwspinlock", > .of_match_table = stm32_hwpinlock_ids, >
diff --git a/drivers/hwspinlock/stm32_hwspinlock.c b/drivers/hwspinlock/stm32_hwspinlock.c index 3ad0ce0da4d9..5bd11a7fab65 100644 --- a/drivers/hwspinlock/stm32_hwspinlock.c +++ b/drivers/hwspinlock/stm32_hwspinlock.c @@ -54,8 +54,23 @@ static const struct hwspinlock_ops stm32_hwspinlock_ops = { .relax = stm32_hwspinlock_relax, }; +static void stm32_hwspinlock_disable_clk(void *data) +{ + struct platform_device *pdev = data; + struct stm32_hwspinlock *hw = platform_get_drvdata(pdev); + struct device *dev = &pdev->dev; + + pm_runtime_get_sync(dev); + pm_runtime_disable(dev); + pm_runtime_set_suspended(dev); + pm_runtime_put_noidle(dev); + + clk_disable_unprepare(hw->clk); +} + static int stm32_hwspinlock_probe(struct platform_device *pdev) { + struct device *dev = &pdev->dev; struct stm32_hwspinlock *hw; void __iomem *io_base; size_t array_size; @@ -66,41 +81,43 @@ static int stm32_hwspinlock_probe(struct platform_device *pdev) return PTR_ERR(io_base); array_size = STM32_MUTEX_NUM_LOCKS * sizeof(struct hwspinlock); - hw = devm_kzalloc(&pdev->dev, sizeof(*hw) + array_size, GFP_KERNEL); + hw = devm_kzalloc(dev, sizeof(*hw) + array_size, GFP_KERNEL); if (!hw) return -ENOMEM; - hw->clk = devm_clk_get(&pdev->dev, "hsem"); + hw->clk = devm_clk_get(dev, "hsem"); if (IS_ERR(hw->clk)) return PTR_ERR(hw->clk); - for (i = 0; i < STM32_MUTEX_NUM_LOCKS; i++) - hw->bank.lock[i].priv = io_base + i * sizeof(u32); + ret = clk_prepare_enable(hw->clk); + if (ret) { + dev_err(dev, "Failed to prepare_enable clock\n"); + return ret; + } platform_set_drvdata(pdev, hw); - pm_runtime_enable(&pdev->dev); - ret = hwspin_lock_register(&hw->bank, &pdev->dev, &stm32_hwspinlock_ops, - 0, STM32_MUTEX_NUM_LOCKS); + pm_runtime_get_noresume(dev); + pm_runtime_set_active(dev); + pm_runtime_enable(dev); + pm_runtime_put(dev); - if (ret) - pm_runtime_disable(&pdev->dev); + ret = devm_add_action_or_reset(dev, stm32_hwspinlock_disable_clk, pdev); + if (ret) { + dev_err(dev, "Failed to register action\n"); + return ret; + } - return ret; -} + for (i = 0; i < STM32_MUTEX_NUM_LOCKS; i++) + hw->bank.lock[i].priv = io_base + i * sizeof(u32); -static int stm32_hwspinlock_remove(struct platform_device *pdev) -{ - struct stm32_hwspinlock *hw = platform_get_drvdata(pdev); - int ret; + ret = devm_hwspin_lock_register(dev, &hw->bank, &stm32_hwspinlock_ops, + 0, STM32_MUTEX_NUM_LOCKS); - ret = hwspin_lock_unregister(&hw->bank); if (ret) - dev_err(&pdev->dev, "%s failed: %d\n", __func__, ret); - - pm_runtime_disable(&pdev->dev); + dev_err(dev, "Failed to register hwspinlock\n"); - return 0; + return ret; } static int __maybe_unused stm32_hwspinlock_runtime_suspend(struct device *dev) @@ -135,7 +152,6 @@ MODULE_DEVICE_TABLE(of, stm32_hwpinlock_ids); static struct platform_driver stm32_hwspinlock_driver = { .probe = stm32_hwspinlock_probe, - .remove = stm32_hwspinlock_remove, .driver = { .name = "stm32_hwspinlock", .of_match_table = stm32_hwpinlock_ids,
Set the clock during probe and keep its control during suspend / resume operations. This fixes an issue when CONFIG_PM is not set and where the clock is never enabled. Make use of devm_ functions to simplify the code. Signed-off-by: Fabien Dessenne <fabien.dessenne@foss.st.com> --- Changes since v1: - Call platform_set_drvdata() before pm_runtime_...() calls --- drivers/hwspinlock/stm32_hwspinlock.c | 58 +++++++++++++++++---------- 1 file changed, 37 insertions(+), 21 deletions(-)