Re: [-mm patch 2/5] SharpSL: Add cxx00 support to the Corgi LCD driver

[Richard Purdie <rpurdie@rpsys.net>]

On Thu, 2005-09-08 at 13:34 +0100, Russell King wrote: 
> On Tue, Sep 06, 2005 at 12:53:48PM +0100, Richard Purdie wrote:
> > +/*
> > + * Corgi/Spitz Touchscreen to LCD interface
> > + */
> > +unsigned long inline corgi_get_hsync_len(void) 
> > +{
> > +	if (machine_is_corgi() || machine_is_shepherd() || machine_is_husky()) {
> > +#ifdef CONFIG_PXA_SHARP_C7xx
> > +		return w100fb_get_hsynclen(&corgifb_device.dev);
> > +#endif
> > +	} else if (machine_is_spitz() || machine_is_akita() || machine_is_borzoi()) {
> > +#ifdef CONFIG_PXA_SHARP_Cxx00
> > +		return pxafb_get_hsync_time(&pxafb_device.dev);
> > +#endif
> 
> This means you have to force these drivers to be built (since this file
> will always be built for sharp stuff.)  This doesn't seem like a good
> solution.

It was made inline so in theory only the touchscreen drivers had the
dependency. I then moved it to another file which kind of breaks things.

Would using symbol_put and symbol_get be a better solution? I was warned
off those functions in the past but this would appear to be an ideal use
as if the framebuffer drivers aren't loaded, we don't care about this.

> > +#define SyncHS(x)   while((GPLR(x) & GPIO_bit(x)) == 0); while((GPLR(x) & GPIO_bit(x)) != 0);
> 
> That's particularly gruesome - firstly, two statements inside a macro.
> Secondly, no barrier() or cpu_relax() in there (as the kernel janitors
> like to see.)  It won't make any difference to the generated code, but
> makes other folk happier.

This needs to be a fast piece of code as its in an interrupt handler and
we go to great lengths to time things to avoid interference from the
lcd. I need to check what timing overhead those functions have...

Richard