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) <[email protected]> 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/[email protected]?part=6
