public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Daniel Gorsulowski <Daniel.Gorsulowski@esd.eu>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/1] at91: Add command to control up to 3 GPIO LEDs from the console
Date: Thu, 07 May 2009 08:01:15 +0200	[thread overview]
Message-ID: <4A02792B.8060102@esd.eu> (raw)
In-Reply-To: <20090506203728.9779B83420E8@gemini.denx.de>

Dear Wolfgang Denk,

Wolfgang Denk wrote:
> Dear Daniel Gorsulowski,
> 
> In message <1241619669338-git-send-email-Daniel.Gorsulowski@esd.eu> you wrote:
>> This patch allows any at91 board, implementing the GPIO LED API,
>> to control the LEDs from the console.
>>
>> led [ 1 | 2 | 3 | all ]  [ on | off ]
>>
>> Adding configuration items CONFIG_AT91_LED and CONFIG_CMD_LED
>> enables the command.
>> Moreover the GPIO Pins have to be defined by CONFIG_USER1_LED ...
>> CONFIG_USER3_LED.
> 
> Signed-off-by: line missing.
> 
Sorry, I'll fix that by the next patch.
>> ---
>>  common/Makefile                 |    1 +
>>  common/cmd_led.c                |   86 +++++++++++++++++++++++++++++++++++++++
>>  cpu/arm926ejs/at91/led.c        |   79 +++++++++++++++++++++++++++++++++++
>>  include/asm-arm/arch-at91/led.h |   52 +++++++++++++++++++++++
>>  4 files changed, 218 insertions(+), 0 deletions(-)
>>  create mode 100644 common/cmd_led.c
>>  create mode 100644 include/asm-arm/arch-at91/led.h
>>
>> diff --git a/common/Makefile b/common/Makefile
>> index b9f4ca7..e0f571c 100644
>> --- a/common/Makefile
>> +++ b/common/Makefile
>> @@ -103,6 +103,7 @@ COBJS-$(CONFIG_CMD_IMMAP) += cmd_immap.o
>>  COBJS-$(CONFIG_CMD_IRQ) += cmd_irq.o
>>  COBJS-$(CONFIG_CMD_ITEST) += cmd_itest.o
>>  COBJS-$(CONFIG_CMD_JFFS2) += cmd_jffs2.o
>> +COBJS-$(CONFIG_CMD_LED) += cmd_led.o
>>  COBJS-$(CONFIG_CMD_LICENSE) += cmd_license.o
>>  COBJS-y += cmd_load.o
>>  COBJS-$(CONFIG_LOGBUFFER) += cmd_log.o
>> diff --git a/common/cmd_led.c b/common/cmd_led.c
> 
> Ummm... common is for, well, for >>common<< stuff. If this code is
> specific to AT91 only, it should not go into common.
> 
IMHO this code is not specific to AT91 only. Well, other architectures does not
support the CONFIG_CMD_LED yet, but they could be implemented later.
> 
> ...
>> +U_BOOT_CMD(
>> +	led, 3, 1, do_led,
>> +	"[1|2|3|all] [on|off]",
>> +	"[1|2|3|all] [on|off] sets/clears led 1,2,3"
> 
> Do you really say "I set a LED? I clear a LED?"  I don't think so.
> 
Ok, I'll write "turns led 1,2,3 on/off".
>> diff --git a/include/asm-arm/arch-at91/led.h b/include/asm-arm/arch-at91/led.h
>> new file mode 100644
>> index 0000000..878b2cf
>> --- /dev/null
>> +++ b/include/asm-arm/arch-at91/led.h
>> @@ -0,0 +1,52 @@
>> +/*
>> + * (C) Copyright 2000-2004
>> + * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
> 
> You claim that I have written any of this code?  I decline.
> 
I took that structure from include/status_led.h, so i picked you up to the list.
But if you mean, I'll remove these lines.
> 
> Best regards,
> 
> Wolfgang Denk
>
Best Regards,
Daniel Gorsulowski

  parent reply	other threads:[~2009-05-07  6:01 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-06 14:21 [U-Boot] [PATCH 1/1] at91: Add command to control up to 3 GPIO LEDs from the console Daniel Gorsulowski
2009-05-06 20:37 ` Wolfgang Denk
2009-05-06 20:40   ` Mike Frysinger
2009-05-07  6:01   ` Daniel Gorsulowski [this message]
2009-05-07  7:32     ` Wolfgang Denk
2009-05-08  5:37       ` Daniel Gorsulowski
2009-05-07  6:31 ` Stefan Roese
2009-05-08  5:28   ` Daniel Gorsulowski
2009-05-08  6:21     ` Stefan Roese
2009-05-14  8:49       ` Daniel Gorsulowski

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=4A02792B.8060102@esd.eu \
    --to=daniel.gorsulowski@esd.eu \
    --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