Re: [PATCH] spi: omap2-mcspi: Correctly handle devm_clk_get_optional() errors

[Lars Pedersen <lapeddk@xxxxxxxxx>]

On Fri, 17 Jan 2025 at 17:16, Mark Brown <broonie@xxxxxxxxxx> 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@xxxxxxxxx>
>
> Fixes: 4c6ac5446d06 ("spi: omap2-mcspi: Fix the IS_ERR() bug for devm_clk_get_optional_enabled()")
> Reported-by: Lars Pedersen <lapeddk@xxxxxxxxx>
> Signed-off-by: Mark Brown <broonie@xxxxxxxxxx>
> ---
>  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@xxxxxxxxxx>
>