linux-kernel - Re: [PATCH] spi: omap2-mcspi: Correctly handle devm_clk_get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKd8=GucMNr2s7ci7TbA7_kwQ8AwwNGWV-Aobeqa_anouEk=rA@mail.gmail.com>
Date: Tue, 21 Jan 2025 13:00:24 +0100
From: Lars Pedersen <lapeddk@...il.com>
To: Mark Brown <broonie@...nel.org>
Cc: Purushothama Siddaiah <psiddaiah@...sta.com>, Corey Minyard <cminyard@...sta.com>, linux-spi@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] spi: omap2-mcspi: Correctly handle devm_clk_get_optional()
 errors

On Fri, 17 Jan 2025 at 17:16, Mark Brown <broonie@...nel.org> wrote:
>
> devm_clk_get_optional() returns NULL for missing clocks and a PTR_ERR()
> if there is a clock but we fail to get it, but currently we only handle
> the latter case and do so as though the clock was missing.  If we get an
> error back we should handle that as an error since the clock exists but
> we failed to get it, if we get NULL then the clock doesn't exist and we
> should handle that.

Hi Mark.

I have now tested the patch on our board and everything now works as
expected. Thanks!

Tested-by: Lars Pedersen <lapeddk@...il.com>
>
> Fixes: 4c6ac5446d06 ("spi: omap2-mcspi: Fix the IS_ERR() bug for devm_clk_get_optional_enabled()")
> Reported-by: Lars Pedersen <lapeddk@...il.com>
> Signed-off-by: Mark Brown <broonie@...nel.org>
> ---
>  drivers/spi/spi-omap2-mcspi.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c
> index add6247d348190452918900b145c3c5a00e409b3..29c616e2c408cf26b150a853f789128d003db1f0 100644
> --- a/drivers/spi/spi-omap2-mcspi.c
> +++ b/drivers/spi/spi-omap2-mcspi.c
> @@ -1561,10 +1561,15 @@ static int omap2_mcspi_probe(struct platform_device *pdev)
>         }
>
>         mcspi->ref_clk = devm_clk_get_optional_enabled(&pdev->dev, NULL);
> -       if (IS_ERR(mcspi->ref_clk))
> -               mcspi->ref_clk_hz = OMAP2_MCSPI_MAX_FREQ;
> -       else
> +       if (IS_ERR(mcspi->ref_clk)) {
> +               status = PTR_ERR(mcspi->ref_clk);
> +               dev_err_probe(&pdev->dev, status, "Failed to get ref_clk");
> +               goto free_ctlr;
> +       }
> +       if (mcspi->ref_clk)
>                 mcspi->ref_clk_hz = clk_get_rate(mcspi->ref_clk);
> +       else
> +               mcspi->ref_clk_hz = OMAP2_MCSPI_MAX_FREQ;
>         ctlr->max_speed_hz = mcspi->ref_clk_hz;
>         ctlr->min_speed_hz = mcspi->ref_clk_hz >> 15;
>
>
> ---
> base-commit: 9d89551994a430b50c4fffcb1e617a057fa76e20
> change-id: 20250116-spi-fix-omap2-optional-84aa541869e6
>
> Best regards,
> --
> Mark Brown <broonie@...nel.org>
>