linux-kernel - Re: [PATCH v2 2/2] staging: panel: fix sparse warnings in lcd

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]

Date:	Fri, 25 Apr 2014 07:56:09 +0200
From:	Willy Tarreau <w@....eu>
To:	Bastien Armand <armand.bastien@...oste.net>
Cc:	Dan Carpenter <dan.carpenter@...cle.com>,
	Willy Tarreau <willy@...a-x.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Monam Agarwal <monamagarwal123@...il.com>,
	Jake Champlin <jake.champlin.27@...il.com>,
	Arnd Bergmann <arnd@...db.de>, devel@...verdev.osuosl.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] staging: panel: fix sparse warnings in lcd_write

On Wed, Apr 23, 2014 at 07:35:02PM +0200, Bastien Armand wrote:
> On Tue, Apr 22, 2014 at 01:01:45PM +0300, Dan Carpenter wrote:
> > Btw, this whole function is terrible.  It should be reading larger
> > chunks at once instead of get_user() for each character.

Just for the record, very small character counts may be sent to an LCD
panel, in general these are 2 lines of 16 characters, and at most 2x40,
and changes are very rare. The worst case will be if someone displays
the time of day, it will change every second.

> > > +	if (lcd_enabled && lcd_initialized) {
> > > +		for (; count-- > 0; tmp++) {
> > > +			if (!in_interrupt() && (((count + 1) & 0x1f) == 0))
> > > +				/* let's be a little nice with other processes
> > > +				   that need some CPU */
> > > +				schedule();
> > 
> > This schedule() isn't needed.  It just prints a line or two at start up
> > and some other debug output or something.  Small small.
> > 
> 
> I hesitated to remove it. I leave it here as it was allready in lcd_write.
> Perhaps, I could send another patch to remove it.

I believe it can go. I have some memories of it when I was developing the
driver because I didn't know if some LCDs would need long pauses between
each character. Hmmm well, thinking about it after re-reading the code,
we could wait up to 20+40+120 = 180 microseconds when sending one command
(eg: position change), or 20+40+45 = 105 microseconds when sending one
char. That's basically 180+40*105 = 4.185 milliseconds for one full line,
or 8 milliseconds for two lines of 40 chars. So maybe we should keep the
schedule() in the end...

Best regards,
Willy

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/