Hi Alexey,
are you still around? Your patch looks good, just two things...
On Fri, Jun 09, 2023 at 12:55:13AM +0200, Andi Shyti wrote:
> From: Alexey Khoroshilov <khoroshilov@xxxxxxxxx>
>
> pm_runtime_set_suspended() does not lead to call of suspend callback,
> so clk may be left undisabled in hix5hd2_i2c_remove().
>
> By the way, the patch adds error handling for clk_prepare_enable().
could you please put this in a separate patch?
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Alexey Khoroshilov <khoroshilov@xxxxxxxxx>
> Signed-off-by: Andi Shyti <andi.shyti@xxxxxxxxxx>
if you're taking this, feel free to drop my SoB...
> ---
> Hi,
>
> Look what I fished from the far December 2017 :)
>
> It looked better to respin it rather than replying to such an old
> mail.
>
> I haven't made any modification to the patch exept for a little
> rebase conflict. Here's a full changelog, anyway.
>
> Changelog
> =========
> v1 -> v2:
> - Fished this out from the muddy pond.
> - Added my SoB
> - Fixed rebase conflict
... but please keep the changelog.
> Andi
>
> drivers/i2c/busses/i2c-hix5hd2.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-hix5hd2.c b/drivers/i2c/busses/i2c-hix5hd2.c
> index 0e34cbaca22dc..ec775ffefa9fc 100644
> --- a/drivers/i2c/busses/i2c-hix5hd2.c
> +++ b/drivers/i2c/busses/i2c-hix5hd2.c
> @@ -421,7 +421,11 @@ static int hix5hd2_i2c_probe(struct platform_device *pdev)
> dev_err(&pdev->dev, "cannot get clock\n");
> return PTR_ERR(priv->clk);
> }
> - clk_prepare_enable(priv->clk);
> + ret = clk_prepare_enable(priv->clk);
please use devm_clk_get_enabled();
Thanks,
Andi
> + if (ret) {
> + dev_err(&pdev->dev, "cannot enable clock\n");
> + return ret;
> + }
>
> strscpy(priv->adap.name, "hix5hd2-i2c", sizeof(priv->adap.name));
> priv->dev = &pdev->dev;
> @@ -469,8 +473,10 @@ static int hix5hd2_i2c_remove(struct platform_device *pdev)
> struct hix5hd2_i2c_priv *priv = platform_get_drvdata(pdev);
>
> i2c_del_adapter(&priv->adap);
> - pm_runtime_disable(priv->dev);
> - pm_runtime_set_suspended(priv->dev);
> +
> + /* Make sure priv->clk is disabled */
> + pm_runtime_force_suspend(priv->dev);
> +
> clk_disable_unprepare(priv->clk);
>
> return 0;
> --
> 2.40.1
>
