From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bernhard Nortmann Date: Mon, 24 Aug 2015 11:51:46 +0200 Subject: [U-Boot] [PATCH 1/2] add generic stubs for GPIO LEDs In-Reply-To: References: <1440162801-27754-1-git-send-email-bernhard.nortmann@web.de> <1440162801-27754-2-git-send-email-bernhard.nortmann@web.de> Message-ID: <55DAE932.6070905@web.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 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.) Regards, B. Nortmann