On Thu, Jun 12, 2008 at 10:20 PM, Eric Miao [off-list ref] wrote:

Jaya Kumar wrote:

quoted

This patch converts am200epd to use pxafb's fb and to stop performing
direct access to LCDC registers. It now uses the generic GPIO api.

Mmm..., this patch makes the code much cleaner now, see my comments below.

Thanks :)

quoted

+static struct pxafb_mach_info am200_fb_info = {
+     .modes          = &am200_fb_mode_6inch,
+     .num_modes      = 1,
+     .lccr0          = LCCR0_Pas | LCCR0_Sngl | LCCR0_Color,
+     .lccr3          = 0,
+     .lcd_conn       = LCD_TYPE_COLOR_TFT | LCD_PCLK_EDGE_FALL |
+                       LCD_AC_BIAS_FREQ(24),

You don't need to specify both lccrX and lcd_conn, the pxafb.c should
be smart enough to guess a correct LCCRx setting based on lcd_conn.

Ok, will fix.

Can set_pxa_fb_info() be used here instead of re-allocating a new
platform_device with almost same setting as pxa_device_fb?

Actually, I had tried to do that and ran into problems on driver
unload. This is because pxa_device_fb is a statically declared struct
and not allocated using platform_device_alloc. So it is missing a
bunch of stuff needed for device release. I thought about changing
set_pxa_fb_info to do alloc instead but it seems too invasive. This
duplication seems easier.

quoted

-     return set_irq_type(IRQ_GPIO(RDY_GPIO_PIN), IRQT_FALLING);
+     ret = set_irq_type(IRQ_GPIO(RDY_GPIO_PIN), IRQT_FALLING);

This can be specified in request_irq(), e.g.

       request_irq(..., IRQF_DISABLED | IRQF_TRIGGER_FALLING, ...)

the rest of the edge configuration work will be automatically done.

Ok, will fix.

Thanks,
jaya

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php