Re: [PATCH v5 07/15] media: meson-ir-tx: Ensure clock is disabled on unbind

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] The clock is automatically disabled on unbind via devres, but the hardware is not gracefully shut down first, violating the hardware state machine.

Pre-existing issues:
- [High] The driver overwrites genuine error codes (including -EPROBE_DEFER) with a hardcoded -ENODEV, breaking deferred probing.
--

--- Patch [7]: [PATCH v5 07/15] media: meson-ir-tx: Ensure clock is disabled on unbind ---
commit ed37040061f2257376fd955ba2cd00d04a46719c
Author: Sean Young <sean@mess.org>

    media: meson-ir-tx: Ensure clock is disabled on unbind
    
This commit updates the meson-ir-tx driver to use devm_clk_get_enabled() so
that the clock is automatically disabled during driver unbind.

> diff --git a/drivers/media/rc/meson-ir-tx.c b/drivers/media/rc/meson-ir-tx.c
> index fded2c256f2a6..e7bb107e6a843 100644
> --- a/drivers/media/rc/meson-ir-tx.c
> +++ b/drivers/media/rc/meson-ir-tx.c
> @@ -288,8 +288,8 @@ static int meson_irtx_mod_clock_probe(struct meson_irtx *ir,
>  	if (!np)
>  		return -ENODEV;
>  
> -	clock = devm_clk_get(ir->dev, "xtal");
> -	if (IS_ERR(clock) || clk_prepare_enable(clock))
> +	clock = devm_clk_get_enabled(ir->dev, "xtal");
> +	if (IS_ERR(clock))
>  		return -ENODEV;

[Severity: High]
This is a pre-existing issue, but does returning -ENODEV here in
meson_irtx_mod_clock_probe() overwrite the actual error code returned by
devm_clk_get_enabled(), such as -EPROBE_DEFER?

If the clock provider is not yet ready, returning -ENODEV instead of
propagating the error code could permanently fail to probe instead of
deferring. A similar override happens in meson_irtx_probe() with
platform_get_irq().

[Severity: Medium]
With devm_clk_get_enabled(), the clock is automatically disabled on unbind via
devres.

Is it safe to disable the clock without first shutting down the hardware block?

The driver configures the hardware in meson_irtx_probe(), but since there is
no .remove callback in meson_irtx_pd (or a corresponding devm action) to
gracefully shut down the hardware, it is left running. Disabling the clock
while the hardware is still active might freeze the hardware block or cause
bus lockups when the driver is unbound.

>  
>  	*clk_nr = IRB_MOD_XTAL3_CLK;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785338381.git.sean@mess.org?part=7

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic