Re: TSC2101/2102

[Imre Deak <imre.deak@solidboot.com>]

On Mon, Apr 02, 2007 at 09:43:58AM -0400, Eduardo Valentin wrote:
> Hi Imre Deak,
> 
> 
> 
> 
> On 3/31/07, Imre Deak <imre.deak@solidboot.com> wrote:
> >On Sat, Mar 31, 2007 at 08:52:17AM -0400, Eduardo Valentin wrote:
> >> Hello Tony,
> >>
> >>
> >>    This is an updated patch to fix lcd for H2 board. This one is
> >> applied against
> >> current linux omap (commit ).
> >>
> >>    The patch is a merge of what was discussed here and what is already
> >>    pushed.
> >>
> >> Signed-off-by: Eduardo Valentin <eduardo.valentin@indt.org.br>
> >>
> >> BR,
> >>
> >> Eduardo Valentin
> >>
> >> [...]
> >>
> >> Index: linux-omap-2.6/arch/arm/mach-omap1/board-h2.c
> >> ===================================================================
> >> --- linux-omap-2.6.orig/arch/arm/mach-omap1/board-h2.c        2007-03-30 
> >14:26:13.000000000 -0400
> >> +++ linux-omap-2.6/arch/arm/mach-omap1/board-h2.c     2007-03-30 
> >14:36:53.000000000 -0400
> >> @@ -305,6 +305,19 @@ struct {
> >>  #define TSC2101_MUX_MCLK_ON  R10_1610_MCLK_ON
> >>  #define TSC2101_MUX_MCLK_OFF R10_1610_MCLK_OFF
> >>
> >> +static void h2_lcd_dev_init(struct spi_device *tsc2101)
> >> +{
> >> +     /* The LCD is connected to the GPIO pins of the TSC2101, so
> >> +      * we have to tie them here. We can also register the LCD driver
> >> +      * first only here, where we know that the TSC driver is ready.
> >> +      */
> >> +
> >> +     device_initialize(&h2_lcd_device.dev);
> >> +
> >> +     dev_set_drvdata(&h2_lcd_device.dev, tsc2101);
> >
> >Is this really necessary? Why isn't it ok to pass it through the
> >platform_data?
> 
>   I tested also with
> h2_lcd_device.dev.platform_data = tsc2101;
> 
>   But the lcd is not initialized using this line. During the board
> boot, it prints:
> lcd_h2 lcd_h2: no platform data

Odd. Calling platform_device_add is one bug at least so that should be
fixed as follows:

+static void h2_lcd_dev_init(struct spi_device *tsc2101)
+{
+	/* The LCD is connected to the GPIO pins of the TSC2101, so
+	 * we have to tie them here. We can also register the LCD driver
+	 * first only here, where we know that the TSC driver is ready.
+	 */
+	h2_lcd_device.dev.platform_data = tsc2101;
+
+	platform_device_register(&h2_lcd_device);
+}

> 
>  I know that it is weird, but that's why I used dev_set_drvdata().

I think it's not a good thing to do, since the related field is for driver
private data.

--Imre