Re: [PATCH 6/6] fbdev: sh_mobile_lcdc: check for fb_deferred_io_init() error

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] fbdev: sh_mobile_lcdc: clock leak on fb_deferred_io_init() failure
--

commit 5d69f4a8e3d4f95bd3f24c9c5188ad04101636e7
Author: Lorenzo Stoakes (ARM) <ljs@kernel.org>

fbdev: sh_mobile_lcdc: check for fb_deferred_io_init() error

This commit updates sh_mobile_lcdc_start() to check the return value of
fb_deferred_io_init(). If it fails, info->fbdefio is cleared to prevent
sh_mobile_lcdc_stop() from cleaning up uninitialized deferred I/O state,
and the function returns the error.

> diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
> index e8324b01700f6..1409a5cc736b0 100644
> --- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
> +++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
> @@ -1042,7 +1042,11 @@ static int sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
>  			ch->defio.deferred_io = sh_mobile_lcdc_deferred_io;
>  			ch->defio.delay = msecs_to_jiffies(tmp);
>  			ch->info->fbdefio = &ch->defio;
> -			fb_deferred_io_init(ch->info);
> +			ret = fb_deferred_io_init(ch->info);
> +			if (ret) {
> +				ch->info->fbdefio = NULL;
> +				return ret;
> +			}

[Severity: High]
Does this early return bypass unwinding the clocks and stopping the hardware?

Earlier in sh_mobile_lcdc_start(), the clocks are enabled and the hardware is
started. When called via the FBIOPUT_VSCREENINFO ioctl through
sh_mobile_lcdc_set_par(), a failure here exits without disabling the clocks or
stopping the hardware:

drivers/video/fbdev/sh_mobile_lcdcfb.c:sh_mobile_lcdc_set_par() {
    ...
	ret = sh_mobile_lcdc_start(ch->lcdc);
	if (ret < 0)
		dev_err(info->dev, "%s: unable to restart LCDC\n", __func__);
    ...
}

Additionally, in a multi-channel setup during probe, the failure on one
channel bypasses sh_mobile_lcdc_channel_fb_register(). This causes the error
handler sh_mobile_lcdc_stop() to explicitly call clk_on() for the successful
channel, creating an unmatched increment:

drivers/video/fbdev/sh_mobile_lcdcfb.c:sh_mobile_lcdc_stop() {
    ...
		if (ch->info && ch->info->fbdefio) {
			...
			sh_mobile_lcdc_clk_on(priv);
		}
    ...
}

Could this lead to a permanent leak of the hardware clock reference count
(priv->hw_usecnt) and leave the LCDC hardware running in an error state?

>  		}
>  
>  		sh_mobile_lcdc_display_on(ch);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260926-fix-fbdefio-error-handling-v1-0-a94810b6e263@kernel.org?part=6