public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Tom Rini <trini@konsulko.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/2] add generic stubs for GPIO LEDs
Date: Fri, 28 Aug 2015 11:00:42 -0400	[thread overview]
Message-ID: <20150828150042.GG25532@bill-the-cat> (raw)
In-Reply-To: <55DAE932.6070905@web.de>

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 <sjg@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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150828/ca807ae8/attachment.sig>

  reply	other threads:[~2015-08-28 15:00 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-21 13:13 [U-Boot] [PATCH 0/2] GPIO LED improvements (resend) Bernhard Nortmann
2015-08-21 13:13 ` [U-Boot] [PATCH 1/2] add generic stubs for GPIO LEDs Bernhard Nortmann
2015-08-23 21:21   ` Simon Glass
2015-08-24  9:51     ` Bernhard Nortmann
2015-08-28 15:00       ` Tom Rini [this message]
2015-09-01 17:37         ` Bernhard Nortmann
2015-10-24 21:14   ` [U-Boot] [U-Boot,1/2] " Tom Rini
2015-08-21 13:13 ` [U-Boot] [PATCH 2/2] allow LED initialization without STATUS_LED_BOOT Bernhard Nortmann
2015-10-24 21:14   ` [U-Boot] [U-Boot, " Tom Rini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150828150042.GG25532@bill-the-cat \
    --to=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox