On Mon, Aug 24, 2015 at 11:51:46AM +0200, Bernhard Nortmann wrote: > Hello Simon! > > Am 23.08.2015 23:21, schrieb Simon Glass: > >Hi Bernard, > > > >[...] > >If this is a new option it should be added to Kconfig. Otherwise: > > > >Reviewed-by: Simon Glass <s...@chromium.org> > Right. Kconfig wasn't on my agenda, but I agree that it should go in there. > Unfortunately this points out further problems. The existing CONFIG_GPIO_LED > is not part of Kconfig, but gets set via board-specific (.h) > includes. Kconfig > has/supports CONFIG_LED_GPIO, but that is related to the driver model > variant, and doesn't affect inclusion of drivers/misc/gpio_led.c ... > admittedly it's a bit of a mess. > > >I am not a huge fan of the existing API. I would suggest that if you > >have the energy you could replace it with something like: > > > >enum led_colour_t { > > led_red, > > red_green, > > ... > >}; > > > >int led_set_on(enum led_colour_t colour, bool on) > Yes, I've been code-dancing a bit back and forth to get this stuff to fit in > with the existing API. One reason I have selected these "stub" functions > is that they are meant to match (and replace) the existing weak definitions > in arch/arm/lib/board.c. Also, my impression is that they exist as separate > functions to allow common/cmd_led.c to use them as function pointers, > namely for the construction of the led_commands[] array. > > The API may not be pretty, but I have in fact tried to keep it as close to > the original as possible, with the goal of allowing to simply substitute > GPIO numbers for the "LED bits". > > >BTW there is a device-tree-enabled LED driver node (see drivers/led). > >It might be worth considering using this on some sunxi boards. > Thanks! I'll definitely have a look into it. > (That one is related to the CONFIG_LED_GPIO mentioned above.)
So do you want to do these further clean-ups on top of this series or in addition to? Thanks! -- Tom
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot