linux-kernel - Re: [PATCH] fbdev: au1100fb: Add missing check for clk

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <zytpnyodschvn4mmpllxp62yg3o77hjl7l5nyckoxyuvucjyaj@xsxbybnyzd44>
Date: Wed, 28 Jan 2026 15:55:44 +0100
From: Uwe Kleine-König <u.kleine-koenig@...libre.com>
To: Helge Deller <deller@....de>
Cc: Chen Ni <nichen@...as.ac.cn>, elfring@...rs.sourceforge.net, 
	linux-fbdev@...r.kernel.org, dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] fbdev: au1100fb: Add missing check for clk_enable

On Wed, Jan 28, 2026 at 02:29:37PM +0100, Helge Deller wrote:
> On 1/28/26 10:10, Chen Ni wrote:
> > Add check for the return value of clk_enable() andreturn the error
> > if it fails in order to catch the error.
> 
> Why?
> If it really fails, the screen and everything stay blank,
> so it's even worse....
> It seems nobody is actually checking the return value, most likely
> because if it fails, the system will sleep forever anyway.
> Or am I missing something?
> 
> For now I reject this patch.

I think it's the right approach to check the return value. If
au1100fb_drv_resume() fails, the system as a whole will still be
up and it will know that the fb failed to resume.

With the status quo it will assume it's properly resumed despite it
didn't.

Still there are some things so criticise:

	- s/Add missing check for clk_enable/Check return value of clk_enable() in .resume().
	- s/  / /

While looking at the driver I spotted another issue: There is a single

	static struct au1100fb_regs fbregs;

to store the register values. This only works if there are <= 1 such
devices (and wastes memory if there is no such device).

Usually there is at most one such device, but it's cheap to move the
struct into struct au1100fb_device, and if it's only to make the driver
a better template for new drivers.

Best regards
Uwe

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)