| Message ID | 315adcc5dfc6aa5c001448401dda4065e33deef2.1578453062.git.baolin.wang7@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Some improvements for OMAP hwspinlock | expand |
On Tue 07 Jan 19:14 PST 2020, Baolin Wang wrote: > Use devm_hwspin_lock_register() to register the hwlock controller instead of > unregistering the hwlock controller explicitly when removing the device. > > Signed-off-by: Baolin Wang <baolin.wang7@gmail.com> > --- > drivers/hwspinlock/omap_hwspinlock.c | 13 ++----------- > 1 file changed, 2 insertions(+), 11 deletions(-) > > diff --git a/drivers/hwspinlock/omap_hwspinlock.c b/drivers/hwspinlock/omap_hwspinlock.c > index 3b05560..9e8a8c2 100644 > --- a/drivers/hwspinlock/omap_hwspinlock.c > +++ b/drivers/hwspinlock/omap_hwspinlock.c > @@ -131,8 +131,8 @@ static int omap_hwspinlock_probe(struct platform_device *pdev) > for (i = 0, hwlock = &bank->lock[0]; i < num_locks; i++, hwlock++) > hwlock->priv = io_base + LOCK_BASE_OFFSET + sizeof(u32) * i; > > - ret = hwspin_lock_register(bank, &pdev->dev, &omap_hwspinlock_ops, > - base_id, num_locks); > + ret = devm_hwspin_lock_register(&pdev->dev, bank, &omap_hwspinlock_ops, > + base_id, num_locks); > if (ret) > goto runtime_err; > > @@ -148,15 +148,6 @@ static int omap_hwspinlock_probe(struct platform_device *pdev) > > static int omap_hwspinlock_remove(struct platform_device *pdev) > { > - struct hwspinlock_device *bank = platform_get_drvdata(pdev); > - int ret; > - > - ret = hwspin_lock_unregister(bank); > - if (ret) { > - dev_err(&pdev->dev, "%s failed: %d\n", __func__, ret); > - return ret; > - } > - Relying on devm_hwspin_lock_register() to hwspin_lock_unregister() will mean that pm_runtime_disable() will now be called before the spinlocks are unregistered. Regards, Bjorn > pm_runtime_disable(&pdev->dev); > > return 0; > -- > 1.7.9.5 >
Hi Bjorn, On Wed, Jan 22, 2020 at 8:15 AM Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > > On Tue 07 Jan 19:14 PST 2020, Baolin Wang wrote: > > > Use devm_hwspin_lock_register() to register the hwlock controller instead of > > unregistering the hwlock controller explicitly when removing the device. > > > > Signed-off-by: Baolin Wang <baolin.wang7@gmail.com> > > --- > > drivers/hwspinlock/omap_hwspinlock.c | 13 ++----------- > > 1 file changed, 2 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/hwspinlock/omap_hwspinlock.c b/drivers/hwspinlock/omap_hwspinlock.c > > index 3b05560..9e8a8c2 100644 > > --- a/drivers/hwspinlock/omap_hwspinlock.c > > +++ b/drivers/hwspinlock/omap_hwspinlock.c > > @@ -131,8 +131,8 @@ static int omap_hwspinlock_probe(struct platform_device *pdev) > > for (i = 0, hwlock = &bank->lock[0]; i < num_locks; i++, hwlock++) > > hwlock->priv = io_base + LOCK_BASE_OFFSET + sizeof(u32) * i; > > > > - ret = hwspin_lock_register(bank, &pdev->dev, &omap_hwspinlock_ops, > > - base_id, num_locks); > > + ret = devm_hwspin_lock_register(&pdev->dev, bank, &omap_hwspinlock_ops, > > + base_id, num_locks); > > if (ret) > > goto runtime_err; > > > > @@ -148,15 +148,6 @@ static int omap_hwspinlock_probe(struct platform_device *pdev) > > > > static int omap_hwspinlock_remove(struct platform_device *pdev) > > { > > - struct hwspinlock_device *bank = platform_get_drvdata(pdev); > > - int ret; > > - > > - ret = hwspin_lock_unregister(bank); > > - if (ret) { > > - dev_err(&pdev->dev, "%s failed: %d\n", __func__, ret); > > - return ret; > > - } > > - > > Relying on devm_hwspin_lock_register() to hwspin_lock_unregister() will > mean that pm_runtime_disable() will now be called before the spinlocks > are unregistered. Yes, you are right. Thanks for catching this issue.
diff --git a/drivers/hwspinlock/omap_hwspinlock.c b/drivers/hwspinlock/omap_hwspinlock.c index 3b05560..9e8a8c2 100644 --- a/drivers/hwspinlock/omap_hwspinlock.c +++ b/drivers/hwspinlock/omap_hwspinlock.c @@ -131,8 +131,8 @@ static int omap_hwspinlock_probe(struct platform_device *pdev) for (i = 0, hwlock = &bank->lock[0]; i < num_locks; i++, hwlock++) hwlock->priv = io_base + LOCK_BASE_OFFSET + sizeof(u32) * i; - ret = hwspin_lock_register(bank, &pdev->dev, &omap_hwspinlock_ops, - base_id, num_locks); + ret = devm_hwspin_lock_register(&pdev->dev, bank, &omap_hwspinlock_ops, + base_id, num_locks); if (ret) goto runtime_err; @@ -148,15 +148,6 @@ static int omap_hwspinlock_probe(struct platform_device *pdev) static int omap_hwspinlock_remove(struct platform_device *pdev) { - struct hwspinlock_device *bank = platform_get_drvdata(pdev); - int ret; - - ret = hwspin_lock_unregister(bank); - if (ret) { - dev_err(&pdev->dev, "%s failed: %d\n", __func__, ret); - return ret; - } - pm_runtime_disable(&pdev->dev); return 0;
Use devm_hwspin_lock_register() to register the hwlock controller instead of unregistering the hwlock controller explicitly when removing the device. Signed-off-by: Baolin Wang <baolin.wang7@gmail.com> --- drivers/hwspinlock/omap_hwspinlock.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-)