* Felipe Balbi <me@felipebalbi.com> [090316 14:57]:
> On Mon, Mar 16, 2009 at 01:38:55PM -0700, Tony Lindgren wrote:
> > * Tony Lindgren <tony@atomide.com> [090313 15:46]:
> > > From: Steve Sakoman <sakoman@gmail.com>
> > > 
> > > An upcoming Overo expansion board includes an ADS7846 touchscreen controller.
> > > 
> > > This patch adds support via the ads7846 driver when enabled in the
> > > kernel config.
> > 
> > Here's this one refreshed to apply without the smsc911x patch.
> > 
> > Tony
> 
> > From b4b560d6323bd56a7c450b81263d09464d8c6e36 Mon Sep 17 00:00:00 2001
> > From: Steve Sakoman <sakoman@gmail.com>
> > Date: Mon, 16 Mar 2009 13:28:00 -0700
> > Subject: [PATCH] ARM: OMAP3: Add ADS7846 touchscreen support to Overo platform, v2
> > 
> > An upcoming Overo expansion board includes an ADS7846 touchscreen controller.
> > 
> > This patch adds support via the ads7846 driver when enabled in the
> > kernel config.
> > 
> > Signed-off-by: Steve Sakoman <steve@sakoman.com>
> > Signed-off-by: Tony Lindgren <tony@atomide.com>
> > 
> > diff --git a/arch/arm/mach-omap2/board-overo.c b/arch/arm/mach-omap2/board-overo.c
> > index d785aa8..4d03b70 100644
> > --- a/arch/arm/mach-omap2/board-overo.c
> > +++ b/arch/arm/mach-omap2/board-overo.c
> > @@ -52,6 +52,66 @@
> >  #define GPMC_CS0_BASE  0x60
> >  #define GPMC_CS_SIZE   0x30
> >  
> > +#if defined(CONFIG_TOUCHSCREEN_ADS7846) || \
> > +	defined(CONFIG_TOUCHSCREEN_ADS7846_MODULE)
> 
> I'm wondering if this is really necessary since ...
> 
> > +#include <mach/mcspi.h>
> > +#include <linux/spi/spi.h>
> > +#include <linux/spi/ads7846.h>
> > +
> > +static struct omap2_mcspi_device_config ads7846_mcspi_config = {
> 
> __initdata ??
> 
> > +	.turbo_mode	= 0,
> > +	.single_channel	= 1,	/* 0: slave, 1: master */
> > +};
> > +
> > +

To me it looks like turbo_mode and single_channel can get
cleaned away. I'll add that to my list of things todo for
all the boards.
 
> one blank line enough ?

Removed in this updated patch.
 
> > +static int ads7846_get_pendown_state(void)
> > +{
> > +	return !gpio_get_value(OVERO_GPIO_PENDOWN);
> > +}
> > +
> > +static struct ads7846_platform_data ads7846_config = {
> > +	.x_max			= 0x0fff,
> > +	.y_max			= 0x0fff,
> > +	.x_plate_ohms		= 180,
> > +	.pressure_max		= 255,
> > +	.debounce_max		= 10,
> > +	.debounce_tol		= 3,
> > +	.debounce_rep		= 1,
> > +	.get_pendown_state	= ads7846_get_pendown_state,
> > +	.keep_vref_on		= 1,
> > +};
> > +
> > +static struct spi_board_info overo_spi_board_info[] __initdata = {
> 
> ... this is initdata, meaning that this region will be freed anyways after
> __init even if the driver doesn't probe. So I guess we could remove the
> ifdeferry and mark the other ads7846-related stuff as either __init or
> __initdata.

Hmm I guess binary size is another consideration, so let's keep it as is
for now, and if we decide to do something, let's do it for all the
boards.

Tony

 
> > +	{
> > +		.modalias		= "ads7846",
> > +		.bus_num		= 1,
> > +		.chip_select		= 0,
> > +		.max_speed_hz		= 1500000,
> > +		.controller_data	= &ads7846_mcspi_config,
> > +		.irq			= OMAP_GPIO_IRQ(OVERO_GPIO_PENDOWN),
> > +		.platform_data		= &ads7846_config,
> > +	}
> > +};
> 
> -- 
> balbi