Re: [PATCH v2 2/2] staging: panel: fix sparse warnings in lcd_write

[Willy Tarreau]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/