| Message ID | 20230731165456.799784-2-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | stm32/hash - Convert to platform remove callback returning void | expand |
Hello, Thanks for the modification. This should be applied for fixes/stable. Please add Cc: stable@vger.kernel.org in your commit message. Best regards, Thomas On 7/31/23 18:54, Uwe Kleine-König wrote: > If pm_runtime_get() (disguised as pm_runtime_resume_and_get()) fails, this > means the clk wasn't prepared and enabled. Returning early in this case > however is wrong as then the following resource frees are skipped and this > is never catched up. So do all the cleanups but clk_disable_unprepare(). > > Also don't emit a warning, as stm32_hash_runtime_resume() already emitted > one. > > Note that the return value of stm32_hash_remove() is mostly ignored by > the device core. The only effect of returning zero instead of an error > value is to suppress another warning in platform_remove(). So return 0 > even if pm_runtime_resume_and_get() failed. > > Fixes: 8b4d566de6a5 ("crypto: stm32/hash - Add power management support") > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > drivers/crypto/stm32/stm32-hash.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/crypto/stm32/stm32-hash.c b/drivers/crypto/stm32/stm32-hash.c > index 88a186c3dd78..75d281edae2a 100644 > --- a/drivers/crypto/stm32/stm32-hash.c > +++ b/drivers/crypto/stm32/stm32-hash.c > @@ -2121,9 +2121,7 @@ static int stm32_hash_remove(struct platform_device *pdev) > if (!hdev) > return -ENODEV; > > - ret = pm_runtime_resume_and_get(hdev->dev); > - if (ret < 0) > - return ret; > + ret = pm_runtime_get_sync(hdev->dev); > > stm32_hash_unregister_algs(hdev); > > @@ -2139,7 +2137,8 @@ static int stm32_hash_remove(struct platform_device *pdev) > pm_runtime_disable(hdev->dev); > pm_runtime_put_noidle(hdev->dev); > > - clk_disable_unprepare(hdev->clk); > + if (ret >= 0) > + clk_disable_unprepare(hdev->clk); > > return 0; > }
Hello, On Wed, Aug 09, 2023 at 09:44:30AM +0200, Thomas BOURGOIN wrote: > Thanks for the modification. > This should be applied for fixes/stable. > Please add Cc: stable@vger.kernel.org in your commit message. I usually let maintainers decide if they want this Cc line and in practise the Fixes: line seems to be enough for the stable team to pick up a commit for backporting. If your mail means I should resend the patch just to add the Cc: line, please tell me again. Should I resent patches 2 and 3 then, too? Best regards Uwe
Hello, On 8/9/23 12:58, Uwe Kleine-König wrote: > I usually let maintainers decide if they want this Cc line and in > practise the Fixes: line seems to be enough for the stable team to pick > up a commit for backporting. Ok, I thought this was required to apply the patch correctly on previous stable releases. (Someone asked me to do it on one of my previous patch) > If your mail means I should resend the patch just to add the Cc: line, > please tell me again. Should I resent patches 2 and 3 then, too? No, patch 3 will break the driver on previous version because remove_new does not exist. I don't think it's mandatory for patch 2, it's an optimisation it does not fix something broken. The driver works as intended with the condition so no need to remove it. Thomas
diff --git a/drivers/crypto/stm32/stm32-hash.c b/drivers/crypto/stm32/stm32-hash.c index 88a186c3dd78..75d281edae2a 100644 --- a/drivers/crypto/stm32/stm32-hash.c +++ b/drivers/crypto/stm32/stm32-hash.c @@ -2121,9 +2121,7 @@ static int stm32_hash_remove(struct platform_device *pdev) if (!hdev) return -ENODEV; - ret = pm_runtime_resume_and_get(hdev->dev); - if (ret < 0) - return ret; + ret = pm_runtime_get_sync(hdev->dev); stm32_hash_unregister_algs(hdev); @@ -2139,7 +2137,8 @@ static int stm32_hash_remove(struct platform_device *pdev) pm_runtime_disable(hdev->dev); pm_runtime_put_noidle(hdev->dev); - clk_disable_unprepare(hdev->clk); + if (ret >= 0) + clk_disable_unprepare(hdev->clk); return 0; }
If pm_runtime_get() (disguised as pm_runtime_resume_and_get()) fails, this means the clk wasn't prepared and enabled. Returning early in this case however is wrong as then the following resource frees are skipped and this is never catched up. So do all the cleanups but clk_disable_unprepare(). Also don't emit a warning, as stm32_hash_runtime_resume() already emitted one. Note that the return value of stm32_hash_remove() is mostly ignored by the device core. The only effect of returning zero instead of an error value is to suppress another warning in platform_remove(). So return 0 even if pm_runtime_resume_and_get() failed. Fixes: 8b4d566de6a5 ("crypto: stm32/hash - Add power management support") Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/crypto/stm32/stm32-hash.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)