Re: [PATCH 1/7] fbdev: mmp: Remove redundant dev_err()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The driver unconditionally overrides the error codes returned by platform_get_irq() and devm_request_irq() to fixed values (-ENOENT and -ENXIO), breaking probe deferral.
- [High] The shared interrupt handler unconditionally returns IRQ_HANDLED without checking if it actually serviced an interrupt.
- [High] The driver lacks a .remove callback but does not suppress sysfs unbind, leading to Use-After-Free and resource leaks.
--

--- Patch [1]: [PATCH 1/7] fbdev: mmp: Remove redundant dev_err() ---
commit 6ce9511a0259fe7a61c4cdd52d067da8431b2740
Author: Pan Chuang <panchuang@vivo.com>

fbdev: mmp: Remove redundant dev_err()

This commit removes a redundant dev_err() call from mmphw_probe().
The devm_request_irq() function already automatically logs detailed error
messages on failure.

> diff --git a/drivers/video/fbdev/mmp/hw/mmp_ctrl.c b/drivers/video/fbdev/mmp/hw/mmp_ctrl.c
> index 75bbdc0b4aa65..2c457da67a9f1 100644
> --- a/drivers/video/fbdev/mmp/hw/mmp_ctrl.c
> +++ b/drivers/video/fbdev/mmp/hw/mmp_ctrl.c
> @@ -505,8 +505,6 @@ static int mmphw_probe(struct platform_device *pdev)
>  	ret = devm_request_irq(ctrl->dev, ctrl->irq, ctrl_handle_irq,
>  		IRQF_SHARED, "lcd_controller", ctrl);

[Severity: High]
This is a pre-existing issue, but it looks like ctrl_handle_irq()
unconditionally returns IRQ_HANDLED.

If it doesn't check whether it actually serviced an interrupt, could it mask
unhandled interrupts on a shared line?

drivers/video/fbdev/mmp/hw/mmp_ctrl.c:ctrl_handle_irq() {
    ...
    return IRQ_HANDLED;
}

Can this break the kernel's spurious interrupt detection and lead to an
interrupt storm if a sharing device misbehaves?

[Severity: High]
This is a pre-existing issue, but since mmphw_driver lacks a .remove
callback and does not suppress sysfs unbind, what happens if the device is
unbound via sysfs?

When unbound, the driver core frees devm-managed resources, but the display
paths and SPI controller appear to be left registered:

static struct platform_driver mmphw_driver = {
        .driver         = {
                .name   = "mmp-disp",
        },
        .probe          = mmphw_probe,
};

Could this leave dangling pointers and unmapped memory active, leading to
use-after-free conditions?

>  	if (ret < 0) {
> -		dev_err(ctrl->dev, "%s unable to request IRQ %d\n",
> -				__func__, ctrl->irq);
>  		ret = -ENXIO;

[Severity: High]
This is a pre-existing issue, but overriding the return value of
devm_request_irq() to a fixed -ENXIO seems to swallow important error codes
like -EPROBE_DEFER.

Earlier in mmphw_probe(), the return value of platform_get_irq() is also
overridden to a fixed -ENOENT:

    irq = platform_get_irq(pdev, 0);
    if (irq < 0) {
            ret = -ENOENT;
            goto failed;
    }

Can this prevent proper probe deferral if dependencies like the IRQ
controller are not yet ready?

>  		goto failed;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722075803.570532-1-panchuang@vivo.com?part=1