Messages in this thread | | | Date | Fri, 25 Apr 2014 07:56:09 +0200 | From | Willy Tarreau <> | 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
| |