Re: [PATCH 1/3] media: cec: ao-cec: Remove redundant dev_err()

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

Pre-existing issues:
- [High] Missing check for `platform_get_irq()` returning a negative error code causes failures like `-EPROBE_DEFER` to be lost and converted into `-EINVAL`.
--

--- Patch [1]: [PATCH 1/3] media: cec: ao-cec: Remove redundant dev_err() ---
commit 1f47d2ef0ba63ca2af1f7aacf1d74819b91ea940
Author: Pan Chuang <[email protected]>

media: cec: ao-cec: Remove redundant dev_err()

This commit removes redundant dev_err() calls after
devm_request_threaded_irq() in the Meson AO CEC drivers, as the core irq
code now logs these errors.

> diff --git a/drivers/media/cec/platform/meson/ao-cec-g12a.c b/drivers/media/cec/platform/meson/ao-cec-g12a.c
> --- a/drivers/media/cec/platform/meson/ao-cec-g12a.c
> +++ b/drivers/media/cec/platform/meson/ao-cec-g12a.c
> @@ -689,10 +689,8 @@ static int meson_ao_cec_g12a_probe(struct platform_device *pdev)
>  					meson_ao_cec_g12a_irq,
>  					meson_ao_cec_g12a_irq_thread,
>  					0, NULL, ao_cec);

[Severity: High]
This isn't a bug introduced by this patch, but since you are modifying the
error handling here, is there a missing check for irq < 0 before calling
devm_request_threaded_irq() in meson_ao_cec_g12a_probe()?

platform_get_irq() can return negative error codes like -EPROBE_DEFER.
Because devm_request_threaded_irq() takes an unsigned int for the IRQ number,
a negative value gets cast to a large positive integer and fails with -EINVAL.

This masks the original -EPROBE_DEFER and causes the probe to fail permanently
instead of deferring.

> -	if (ret) {
> -		dev_err(&pdev->dev, "irq request failed\n");
> +	if (ret)
>  		goto out_probe_adapter;
> -	}

[ ... ]

> diff --git a/drivers/media/cec/platform/meson/ao-cec.c b/drivers/media/cec/platform/meson/ao-cec.c
> --- a/drivers/media/cec/platform/meson/ao-cec.c
> +++ b/drivers/media/cec/platform/meson/ao-cec.c
> @@ -636,10 +636,8 @@ static int meson_ao_cec_probe(struct platform_device *pdev)
>  					meson_ao_cec_irq,
>  					meson_ao_cec_irq_thread,
>  					0, NULL, ao_cec);

[Severity: High]
This is the same pre-existing issue as above, but in meson_ao_cec_probe().
Does this irq value need to be checked for < 0 before it gets implicitly cast
to an unsigned int by devm_request_threaded_irq(), masking -EPROBE_DEFER?

> -	if (ret) {
> -		dev_err(&pdev->dev, "irq request failed\n");
> +	if (ret)
>  		goto out_probe_adapter;
> -	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

_______________________________________________
linux-amlogic mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-amlogic