public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Wolfgang Denk <wd@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] Add 'led' command
Date: Thu, 21 Apr 2011 00:04:48 +0200	[thread overview]
Message-ID: <20110420220448.47CDF152160@gemini.denx.de> (raw)
In-Reply-To: <1299013329-29931-1-git-send-email-jkridner@beagleboard.org>

Dear Jason Kridner,

In message <1299013329-29931-1-git-send-email-jkridner@beagleboard.org> you wrote:
> This patch allows any board implementing the coloured LED API
> to control the LEDs from the console.
> 
> led [green | yellow | red | all ]  [ on | off ]
> 
> or
> 
> led [ 1 | 2 | 3 | all ]  [ on | off ]

I still wonder if such a patch will help to get rid of the ton of LEd
drivers we already have in U-Boot, or if it just adds another such
interface?

Which drivers ("led.c" files) will be obsoleted by this patch?


If it is intended to be a generic interface - how can this then be
combined with the status_led.c driver? 

The configuration "green", "yellow", "red" seems to be very specific
to me - I guess this applies just to very few boards?

...
> +struct led_tbl_s {
> +	char		*string;	/* String for use in the command */
> +	led_id_t	mask;		/* Mask used for calling __led_set() */
> +	void		(*on)(void);	/* Optional fucntion for turning LED on */
> +	void		(*off)(void);	/* Optional fucntion for turning LED on */
> +};
> +
> +typedef struct led_tbl_s led_tbl_t;
> +
> +static const led_tbl_t led_commands[] = {
> +#ifdef CONFIG_BOARD_SPECIFIC_LED
> +#ifdef STATUS_LED_BIT
> +	{ "0", STATUS_LED_BIT, NULL, NULL },
> +#endif
> +#ifdef STATUS_LED_BIT1
> +	{ "1", STATUS_LED_BIT1, NULL, NULL },
> +#endif
> +#ifdef STATUS_LED_BIT2
> +	{ "2", STATUS_LED_BIT2, NULL, NULL },
> +#endif
> +#ifdef STATUS_LED_BIT3
> +	{ "3", STATUS_LED_BIT3, NULL, NULL },
> +#endif

What are these "status bits" good for when they have no actual
handlers attached?  Where are they actually used?


> +#ifdef STATUS_LED_GREEN
> +	{ "green", STATUS_LED_GREEN, green_LED_off, green_LED_on },
> +#endif
> +#ifdef STATUS_LED_YELLOW
> +	{ "yellow", STATUS_LED_YELLOW, yellow_LED_off, yellow_LED_on },
> +#endif
> +#ifdef STATUS_LED_RED
> +	{ "red", STATUS_LED_RED, red_LED_off, red_LED_on },
> +#endif
> +#ifdef STATUS_LED_BLUE
> +	{ "blue", STATUS_LED_BLUE, blue_LED_off, blue_LED_on },
> +#endif

We do not allow CamelCase identifiers (like "green_LED_off").


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Sometimes a man will tell his bartender things he'll never tell his doctor.
	-- Dr. Phillip Boyce, "The Menagerie" ("The Cage"),
	   stardate unknown.

  reply	other threads:[~2011-04-20 22:04 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-01 21:02 [U-Boot] [PATCH] Add 'led' command Jason Kridner
2011-04-20 22:04 ` Wolfgang Denk [this message]
     [not found]   ` <BANLkTinRwxu7FxdB33sJaF=F=EuYk+pbFg@mail.gmail.com>
2011-04-21 13:17     ` [U-Boot] [beagleboard] " Jason Kridner
2011-04-21 14:49       ` Jason Kridner

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=20110420220448.47CDF152160@gemini.denx.de \
    --to=wd@denx.de \
    --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