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
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
