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

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

[Sean Young]

clk_prepare_enable() needs a call to clk_disable_unprepare() on
driver unbind. Make it devm managed.

Fixes: 49be1c78d575 ("media: rc: introduce Meson IR TX driver")
Signed-off-by: Sean Young <sean@mess.org>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: stable@vger.kernel.org
---
 drivers/media/rc/meson-ir-tx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/rc/meson-ir-tx.c b/drivers/media/rc/meson-ir-tx.c
index fded2c256f2a..e7bb107e6a84 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;
 
 	*clk_nr = IRB_MOD_XTAL3_CLK;
-- 
2.55.0


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

[PATCH v5 08/15] media: meson-ir-tx: Ensure rc_free_device() is called on unbind

[Sean Young]

Make rc_dev devm managed.

Fixes: dccc0c3ddf8f ("media: rc: fix race between unregister and urb/irq callbacks")
Signed-off-by: Sean Young <sean@mess.org>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Cc: stable@vger.kernel.org
---
 drivers/media/rc/meson-ir-tx.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/media/rc/meson-ir-tx.c b/drivers/media/rc/meson-ir-tx.c
index e7bb107e6a84..abb107d19e8c 100644
--- a/drivers/media/rc/meson-ir-tx.c
+++ b/drivers/media/rc/meson-ir-tx.c
@@ -345,7 +345,7 @@ static int meson_irtx_probe(struct platform_device *pdev)
 	if (ret)
 		return dev_err_probe(dev, ret, "irq request failed\n");
 
-	rc = rc_allocate_device(RC_DRIVER_IR_RAW_TX);
+	rc = devm_rc_allocate_device(dev, RC_DRIVER_IR_RAW_TX);
 	if (!rc)
 		return -ENOMEM;
 
@@ -358,10 +358,8 @@ static int meson_irtx_probe(struct platform_device *pdev)
 	rc->s_tx_duty_cycle = meson_irtx_set_duty_cycle;
 
 	ret = devm_rc_register_device(dev, rc);
-	if (ret < 0) {
-		rc_free_device(rc);
+	if (ret < 0)
 		return dev_err_probe(dev, ret, "rc_dev registration failed\n");
-	}
 
 	return 0;
 }
-- 
2.55.0


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

[PATCH v5 09/15] media: meson-ir-tx: Ensure probe error is propagated

[Sean Young]

devm_clk_get_enabled() may return -EPROBE_DEFER which needs to be
propagated else the probe will not be deferred, it will fail instead.

Fixes: 49be1c78d575 ("media: rc: introduce Meson IR TX driver")
Signed-off-by: Sean Young <sean@mess.org>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Cc: stable@vger.kernel.org
---
 drivers/media/rc/meson-ir-tx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/rc/meson-ir-tx.c b/drivers/media/rc/meson-ir-tx.c
index abb107d19e8c..174d5135e1bb 100644
--- a/drivers/media/rc/meson-ir-tx.c
+++ b/drivers/media/rc/meson-ir-tx.c
@@ -290,7 +290,7 @@ static int meson_irtx_mod_clock_probe(struct meson_irtx *ir,
 
 	clock = devm_clk_get_enabled(ir->dev, "xtal");
 	if (IS_ERR(clock))
-		return -ENODEV;
+		return PTR_ERR(clock);
 
 	*clk_nr = IRB_MOD_XTAL3_CLK;
 	ir->clk_rate = clk_get_rate(clock) / 3;
@@ -324,7 +324,7 @@ static int meson_irtx_probe(struct platform_device *pdev)
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
-		return -ENODEV;
+		return irq;
 
 	ir->dev = dev;
 	ir->carrier = MIRTX_DEFAULT_CARRIER;
-- 
2.55.0


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

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

[sashiko-bot]

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