[patch v4 06/10] efikamx: add leds support

[maramaopercheseimorto@gmail.com (Alberto Panizzo)]

Hi,
On mar, 2010-11-02 at 10:35 +0100, Arnaud Patard wrote:
> Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de> writes:
> 
> Hi,
> > Hello,
> >
> > On Sat, Oct 30, 2010 at 09:54:47AM +0200, Marek Vasut wrote:
> >> On Wednesday 27 October 2010 14:40:51 Arnaud Patard wrote:
> >> > The efika mx a 3 leds (1 blue, 1 red, 1 green) connected on GPIOS 3
> >> > 13/14/15. Also, some special care is done for default trigger of blue led
> >> > for mmc as the mmc host used is different between hw revisions
> >> > 
> >> > Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
> >> > Index: linux-2.6-submit/arch/arm/mach-mx5/board-mx51_efikamx.c
> >> > ===================================================================
> >> > --- linux-2.6-submit.orig/arch/arm/mach-mx5/board-mx51_efikamx.c	2010-10-27
> >> > 11:26:16.000000000 +0200 +++
> >> > linux-2.6-submit/arch/arm/mach-mx5/board-mx51_efikamx.c	2010-10-27
> >> > 11:27:38.000000000 +0200 @@ -18,6 +18,7 @@
> >> >  #include <linux/platform_device.h>
> >> >  #include <linux/i2c.h>
> >> >  #include <linux/gpio.h>
> >> > +#include <linux/leds.h>
> >> >  #include <linux/delay.h>
> >> >  #include <linux/io.h>
> >> >  #include <linux/fsl_devices.h>
> >> > @@ -43,6 +44,10 @@
> >> >  #define EFIKAMX_PCBID1		(2*32 + 17)
> >> >  #define EFIKAMX_PCBID2		(2*32 + 11)
> >> > 
> >> > +#define EFIKAMX_BLUE_LED	(2*32 + 13)
> >> > +#define EFIKAMX_GREEN_LED	(2*32 + 14)
> >> > +#define EFIKAMX_RED_LED		(2*32 + 15)
> >> > +
> >> >  /* the pci ids pin have pull up. they're driven low according to board id
> >> > */ #define MX51_PAD_PCBID0	IOMUX_PAD(0x518, 0x130, 3, 0x0,   0,
> >> > PAD_CTL_PUS_100K_UP) #define MX51_PAD_PCBID1	IOMUX_PAD(0x51C, 0x134, 3,
> >> > 0x0,   0, PAD_CTL_PUS_100K_UP) @@ -81,6 +86,11 @@
> >> >  	MX51_PAD_GPIO_1_1__ESDHC1_WP,
> >> >  	MX51_PAD_GPIO_1_7__ESDHC2_WP,
> >> >  	MX51_PAD_GPIO_1_8__ESDHC2_CD,
> >> > +
> >> > +	/* leds */
> >> > +	MX51_PAD_CSI1_D9__GPIO_3_13,
> >> > +	MX51_PAD_CSI1_VSYNC__GPIO_3_14,
> >> > +	MX51_PAD_CSI1_HSYNC__GPIO_3_15,
> >> >  };
> >> > 
> >> >  /* Serial ports */
> >> > @@ -179,6 +189,37 @@
> >> >  	}
> >> >  }
> >> 
> >> Maybe this could be modularized ?
> >> 
> >> #ifdef CONFIG_LEDS_GPIO
> >> ... the platform_data stuff below ...
> >> 
> >> efikamx_register_leds()
> >> {
> >> 	platform_device_register();
> >> }
> >> #else
> >> static inline void efikamx_register_leds() {}
> >> #endif
> >> 
> >> board_init()
> >> {
> >> ...
> >> efikamx_register_leds();
> >> ...
> >> }
> >> 
> >> What do you think ? Cheers
> > IMHO it's better to register all devices independently of the available
> > drivers.  So for me the used approach is OK.
> 
> I don't really see the point of doing that. It just looks a good way to
> make the code harder to read imho. Moreover, it doesn't bring a lot by
> doing that - if you don't want the leds-gpio stuff, just disable it at
> kernel configuration and you're done.
> 
> Arnaud

The great advantage of having all the supported devices registered
even if the corresponding driver is not currently built is that if you 
flash the kernel image to the board building this image with a subset 
of supported drivers and you need for some reason to change the 
software configuration (adding another driver) you don't have to rebuild
the kernel image and re-flash it on the board. Instead you can add the
new module on the filesystem and load it at runtime.

Lot of efforts were and are done to build the kernel more general as it 
can be to reduce the versioning issues with a really minimal difference
of footprint.

Best Regards,

-- 
Alberto!

        Be Persistent!
                - Greg Kroah-Hartman (FOSDEM 2010)