Skip to contentSkip to conversation

Please fix lcd_sx1.c

PATCH New Russell King - ARM Linux 2 participants 4 messages Nov 23, 2008 – Nov 25, 2008 Framebuffer layer

lore
Share & tools
lore
#0 lore raw

kautobuild is reporting this error against sx1_defconfig:

drivers/video/omap/lcd_sx1.c: In function 'epson_sendbyte':
drivers/video/omap/lcd_sx1.c:90: error: implicit declaration of function 'OMAP_MCBSP_WRITE' drivers/video/omap/lcd_sx1.c:90: error: 'PCR0' undeclared (first use in this function) drivers/video/omap/lcd_sx1.c:90: error: (Each undeclared identifier is reported only once drivers/video/omap/lcd_sx1.c:90: error: for each function it appears in.) make[3]: *** [drivers/video/omap/lcd_sx1.o] Error 1

Some comments on this:

1. Unused definitions:

#define GPIO_DATA_INPUT         0xfffce000
#define GPIO_DATA_OUTPUT        0xfffce004
#define GPIO_DIR_CONTROL        0xfffce008
#define GPIO_INT_CONTROL        0xfffce00c
#define GPIO_INT_MASK           0xfffce010
#define GPIO_INT_STATUS         0xfffce014
#define GPIO_PIN_CONTROL        0xfffce018

...
#define DSP_REG 0xE1017024

2. Interferes with MCBSP3 by writing direct to the control register. What if someone else is using MCBSP3?

        OMAP_MCBSP_WRITE(OMAP1510_MCBSP3_BASE, PCR0, 0x2200);
        OMAP_MCBSP_WRITE(OMAP1510_MCBSP3_BASE, PCR0, 0x2202);

Besides, these aren't available for this driver (and quite rightfully so.)

3. doesn't check the omap_mcbsp_request() return value:

        omap_mcbsp_request(OMAP_MCBSP3);

4. doesn't check the return code from gpio_request():

        gpio_request(A_LCD_SSC_RD, "lcd_ssc_rd");
        gpio_request(A_LCD_SSC_SD, "lcd_ssc_sd");
        gpio_request(_A_LCD_RESET, "lcd_reset");
        gpio_request(_A_LCD_SSC_CS, "lcd_ssc_cs");
        gpio_request(_A_LCD_SSC_A0, "lcd_ssc_a0");

To me, it looks like this driver was tossed over into mainline without any thought - it has never been in a buildable state. Please fix or remove this driver from mainline.

#1 lore raw
Quoted 3 lines · Russell King - ARM Linux

To me, it looks like this driver was tossed over into mainline without any thought - it has never been in a buildable state. Please fix or remove this driver from mainline.

Actually, it was buildable, until this commit:

commit b4b58f5834a023dab67201db9a626bef07bb200c

Author: Chandra Shekhar <[email protected]>
Date:   Wed Oct 8 10:01:39 2008 +0300

ARM: OMAP: Allocate McBSP devices dynamically

Based on Chandra's earlier patches in linux-omap tree.

Note that omap1_mcbsp_check and omap2_mcbsp_check are no longer needed as there's now omap_mcbsp_check_valid_id() defined.

Also some functions can now be marked __init.

which means it's a regression and therefore needs fixing before 2.6.28.

#2 lore raw
Quoted 18 lines · Russell King - ARM Linux
Quoted 3 lines · Russell King - ARM Linux

To me, it looks like this driver was tossed over into mainline without any thought - it has never been in a buildable state. Please fix or remove this driver from mainline.

Actually, it was buildable, until this commit:

commit b4b58f5834a023dab67201db9a626bef07bb200c

Author: Chandra Shekhar <[email protected]>
Date:   Wed Oct 8 10:01:39 2008 +0300

ARM: OMAP: Allocate McBSP devices dynamically

Based on Chandra's earlier patches in linux-omap tree.

Note that omap1_mcbsp_check and omap2_mcbsp_check are no longer needed as there's now omap_mcbsp_check_valid_id() defined.

Quoted 6 lines

Also some functions can now be marked __init.

which means it's a regression and therefore needs fixing before 2.6.28.

Will send you a patch for this today.

Regards,

Tony

#3 lore raw

PATCH 1/1 ARM: OMAP: Remove broken LCD driver for SX1 (Re: Please fix lcd_sx1.c)

Quoted 19 lines · Tony Lindgren
Quoted 18 lines · Russell King - ARM Linux
Quoted 3 lines · Russell King - ARM Linux

To me, it looks like this driver was tossed over into mainline without any thought - it has never been in a buildable state. Please fix or remove this driver from mainline.

Actually, it was buildable, until this commit:

commit b4b58f5834a023dab67201db9a626bef07bb200c

Author: Chandra Shekhar <[email protected]>
Date:   Wed Oct 8 10:01:39 2008 +0300

ARM: OMAP: Allocate McBSP devices dynamically

Based on Chandra's earlier patches in linux-omap tree.

Note that omap1_mcbsp_check and omap2_mcbsp_check are no longer needed as there's now omap_mcbsp_check_valid_id() defined.

Quoted 8 lines
Quoted 6 lines

Also some functions can now be marked __init.

which means it's a regression and therefore needs fixing before 2.6.28.

Will send you a patch for this today.

Well looks like there's no way I can fix this. I don't have the hardware, so people using sx1 would need to fix this.

Looks like the driver is attempting to use SPI mode, but is doing it in a different way compared to mcbsp.c.

I'm suggesting the following patch unless somebody can do a real patch for this over next few days.

Regards,

Tony

From 7953031da4200323ab5d85bd514054ca4ba9d225 Mon Sep 17 00:00:00 2001
From: Tony Lindgren <[email protected]>
Date: Mon, 24 Nov 2008 18:11:16 -0800
Subject: [PATCH] ARM: OMAP: Remove broken LCD driver for SX1

Recently the omap McBSP code was cleaned up to get rid of direct McBSP register tinkering by the drivers. Looks like lcd_sx1.c never got converted, and now it breaks builds.

It seems the lcd_sx1.c driver is attempting SPI mode, but doing it in a different way compared to omap_mcbsp_set_spi_mode().

Remove the broken driver, patches welcome to add it back when done properly by patching both mcbsp.c and lcd_sx1.c.

drivers/video/omap/Makefile +0 -1
index 99da8b6..ed13889 100644
@@ -23,7 +23,6 @@ objs-y$(CONFIG_MACH_OMAP_PALMZ71) += lcd_palmz71.o
 objs-$(CONFIG_ARCH_OMAP16XX)$(CONFIG_MACH_OMAP_INNOVATOR) += lcd_inn1610.o
 objs-$(CONFIG_ARCH_OMAP15XX)$(CONFIG_MACH_OMAP_INNOVATOR) += lcd_inn1510.o
 objs-y$(CONFIG_MACH_OMAP_OSK) += lcd_osk.o
-objs-y$(CONFIG_MACH_SX1) += lcd_sx1.o
 
 omapfb-objs := $(objs-yy)
 
drivers/video/omap/lcd_sx1.c deleted +0 -327

327 lines not on this page · raw message