Patchwork [U-Boot,06/10] mx23_olinuxino: Add support for status LED

login
register
mail settings
Submitter Otavio Salvador
Date Jan. 30, 2013, 12:13 p.m.
Message ID <1359548001-14278-7-git-send-email-otavio@ossystems.com.br>
Download mbox | patch
Permalink /patch/216878/
State Changes Requested
Delegated to: Stefano Babic
Headers show

Comments

Otavio Salvador - Jan. 30, 2013, 12:13 p.m.
This allow user to know if the bootloader is running, even without a
serial console.

Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
---
 board/olimex/mx23_olinuxino/mx23_olinuxino.c |  7 +++++++
 board/olimex/mx23_olinuxino/spl_boot.c       |  4 ++++
 include/configs/mx23_olinuxino.h             | 12 ++++++++++++
 3 files changed, 23 insertions(+)
Marek Vasut - Jan. 30, 2013, 2:13 p.m.
Dear Otavio Salvador,

> This allow user to know if the bootloader is running, even without a
> serial console.
> 
> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>

Uh oh, how does this know which GPIO to toggle to drive the led this time ?

> ---
>  board/olimex/mx23_olinuxino/mx23_olinuxino.c |  7 +++++++
>  board/olimex/mx23_olinuxino/spl_boot.c       |  4 ++++
>  include/configs/mx23_olinuxino.h             | 12 ++++++++++++
>  3 files changed, 23 insertions(+)
> 
> diff --git a/board/olimex/mx23_olinuxino/mx23_olinuxino.c
> b/board/olimex/mx23_olinuxino/mx23_olinuxino.c index 6a6053b..2501417
> 100644
> --- a/board/olimex/mx23_olinuxino/mx23_olinuxino.c
> +++ b/board/olimex/mx23_olinuxino/mx23_olinuxino.c
> @@ -28,6 +28,9 @@
>  #include <asm/arch/imx-regs.h>
>  #include <asm/arch/clock.h>
>  #include <asm/arch/sys_proto.h>
> +#ifdef CONFIG_STATUS_LED
> +#include <status_led.h>
> +#endif
> 
>  DECLARE_GLOBAL_DATA_PTR;
> 
> @@ -67,5 +70,9 @@ int board_init(void)
>  	/* Adress of boot parameters */
>  	gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100;
> 
> +#if defined(CONFIG_STATUS_LED) && defined(STATUS_LED_BOOT)
> +	status_led_set(STATUS_LED_BOOT, STATUS_LED_STATE);
> +#endif
> +
>  	return 0;
>  }
> diff --git a/board/olimex/mx23_olinuxino/spl_boot.c
> b/board/olimex/mx23_olinuxino/spl_boot.c index 7def8bc..3bbf5ad 100644
> --- a/board/olimex/mx23_olinuxino/spl_boot.c
> +++ b/board/olimex/mx23_olinuxino/spl_boot.c
> @@ -84,6 +84,10 @@ const iomux_cfg_t iomux_setup[] = {
>  	MX23_PAD_EMI_RASN__EMI_RASN | MUX_CONFIG_EMI,
>  	MX23_PAD_EMI_WEN__EMI_WEN | MUX_CONFIG_EMI,
> 
> +	/* Green LED */
> +	MX23_PAD_SSP1_DETECT__GPIO_2_1 |
> +		(MXS_PAD_3V3 | MXS_PAD_4MA | MXS_PAD_NOPULL),
> +
>  	/* MMC 0 */
>  	MX23_PAD_SSP1_CMD__SSP1_CMD | MUX_CONFIG_SSP,
>  	MX23_PAD_SSP1_DATA0__SSP1_DATA0 | MUX_CONFIG_SSP,
> diff --git a/include/configs/mx23_olinuxino.h
> b/include/configs/mx23_olinuxino.h index 7983c5d..968aec8 100644
> --- a/include/configs/mx23_olinuxino.h
> +++ b/include/configs/mx23_olinuxino.h
> @@ -56,6 +56,7 @@
>  #define	CONFIG_CMD_EXT2
>  #define	CONFIG_CMD_FAT
>  #define	CONFIG_CMD_GPIO
> +#define	CONFIG_CMD_LED
>  #define	CONFIG_CMD_MMC
> 
>  /*
> @@ -112,6 +113,17 @@
>  #define	CONFIG_BAUDRATE			115200	/* Default baud rate */
> 
>  /*
> + * Status LED
> + */
> +#define	CONFIG_STATUS_LED
> +#define	CONFIG_GPIO_LED
> +#define	CONFIG_BOARD_SPECIFIC_LED
> +#define	STATUS_LED_BOOT	0
> +#define	STATUS_LED_BIT	10
> +#define	STATUS_LED_STATE	STATUS_LED_ON
> +#define	STATUS_LED_PERIOD	(CONFIG_SYS_HZ / 2)
> +
> +/*
>   * MMC Driver
>   */
>  #ifdef	CONFIG_CMD_MMC

Best regards,
Marek Vasut
Otavio Salvador - Jan. 30, 2013, 3:34 p.m.
On Wed, Jan 30, 2013 at 12:13 PM, Marek Vasut <marex@denx.de> wrote:
> Dear Otavio Salvador,
>
>> This allow user to know if the bootloader is running, even without a
>> serial console.
>>
>> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
>
> Uh oh, how does this know which GPIO to toggle to drive the led this time ?

The problem wasn't the code but me. I wasn't able to find the right
GPIO number at that time.

--
Otavio Salvador                             O.S. Systems
E-mail: otavio@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br
Marek Vasut - Jan. 30, 2013, 3:39 p.m.
Dear Otavio Salvador,

> On Wed, Jan 30, 2013 at 12:13 PM, Marek Vasut <marex@denx.de> wrote:
> > Dear Otavio Salvador,
> > 
> >> This allow user to know if the bootloader is running, even without a
> >> serial console.
> >> 
> >> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
> > 
> > Uh oh, how does this know which GPIO to toggle to drive the led this time
> > ?
> 
> The problem wasn't the code but me. I wasn't able to find the right
> GPIO number at that time.

This is not my question. My question is how does this toggle the GPIO for the 
LED?

Moreover, you never set the LED GPIO as output.

Best regards,
Marek Vasut
Otavio Salvador - Jan. 30, 2013, 3:50 p.m.
On Wed, Jan 30, 2013 at 1:39 PM, Marek Vasut <marex@denx.de> wrote:
> Dear Otavio Salvador,
>
>> On Wed, Jan 30, 2013 at 12:13 PM, Marek Vasut <marex@denx.de> wrote:
>> > Dear Otavio Salvador,
>> >
>> >> This allow user to know if the bootloader is running, even without a
>> >> serial console.
>> >>
>> >> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
>> >
>> > Uh oh, how does this know which GPIO to toggle to drive the led this time
>> > ?
>>
>> The problem wasn't the code but me. I wasn't able to find the right
>> GPIO number at that time.
>
> This is not my question. My question is how does this toggle the GPIO for the
> LED?

gpio_led driver (drivers/misc/gpio_led.c) does it.

...
void __led_init(led_id_t mask, int state)
{
        gpio_request(mask, "gpio_led");
        gpio_direction_output(mask, state == STATUS_LED_ON);
}

void __led_set(led_id_t mask, int state)
{
        gpio_set_value(mask, state == STATUS_LED_ON);
}
...

> Moreover, you never set the LED GPIO as output.

The driver handles it by itself.

See above.

--
Otavio Salvador                             O.S. Systems
E-mail: otavio@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br
Marek Vasut - Jan. 30, 2013, 4:05 p.m.
Dear Otavio Salvador,

> On Wed, Jan 30, 2013 at 1:39 PM, Marek Vasut <marex@denx.de> wrote:
> > Dear Otavio Salvador,
> > 
> >> On Wed, Jan 30, 2013 at 12:13 PM, Marek Vasut <marex@denx.de> wrote:
> >> > Dear Otavio Salvador,
> >> > 
> >> >> This allow user to know if the bootloader is running, even without a
> >> >> serial console.
> >> >> 
> >> >> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
> >> > 
> >> > Uh oh, how does this know which GPIO to toggle to drive the led this
> >> > time ?
> >> 
> >> The problem wasn't the code but me. I wasn't able to find the right
> >> GPIO number at that time.
> > 
> > This is not my question. My question is how does this toggle the GPIO for
> > the LED?
> 
> gpio_led driver (drivers/misc/gpio_led.c) does it.
> 
> ...
> void __led_init(led_id_t mask, int state)
> {
>         gpio_request(mask, "gpio_led");
>         gpio_direction_output(mask, state == STATUS_LED_ON);
> }
> 
> void __led_set(led_id_t mask, int state)
> {
>         gpio_set_value(mask, state == STATUS_LED_ON);
> }
> ...

Ok, this didn't explain much to me.

> > Moreover, you never set the LED GPIO as output.
> 
> The driver handles it by itself.

Oh ok.

Now that I did read through the code, I have few more questions:

Why can't STATUS_LED_BIT be the MX23_PAD_SSP1_DETECT__GPIO_2_1 now? Did you test 
CMD_LED, does it work when toggling the LED?

Best regards,
Marek Vasut
Otavio Salvador - Jan. 30, 2013, 4:08 p.m.
On Wed, Jan 30, 2013 at 2:05 PM, Marek Vasut <marex@denx.de> wrote:
> Dear Otavio Salvador,
>
>> On Wed, Jan 30, 2013 at 1:39 PM, Marek Vasut <marex@denx.de> wrote:
>> > Dear Otavio Salvador,
>> >
>> >> On Wed, Jan 30, 2013 at 12:13 PM, Marek Vasut <marex@denx.de> wrote:
>> >> > Dear Otavio Salvador,
>> >> >
>> >> >> This allow user to know if the bootloader is running, even without a
>> >> >> serial console.
>> >> >>
>> >> >> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
>> >> >
>> >> > Uh oh, how does this know which GPIO to toggle to drive the led this
>> >> > time ?
>> >>
>> >> The problem wasn't the code but me. I wasn't able to find the right
>> >> GPIO number at that time.
>> >
>> > This is not my question. My question is how does this toggle the GPIO for
>> > the LED?
>>
>> gpio_led driver (drivers/misc/gpio_led.c) does it.
>>
>> ...
>> void __led_init(led_id_t mask, int state)
>> {
>>         gpio_request(mask, "gpio_led");
>>         gpio_direction_output(mask, state == STATUS_LED_ON);
>> }
>>
>> void __led_set(led_id_t mask, int state)
>> {
>>         gpio_set_value(mask, state == STATUS_LED_ON);
>> }
>> ...
>
> Ok, this didn't explain much to me.
>
>> > Moreover, you never set the LED GPIO as output.
>>
>> The driver handles it by itself.
>
> Oh ok.
>
> Now that I did read through the code, I have few more questions:
>
> Why can't STATUS_LED_BIT be the MX23_PAD_SSP1_DETECT__GPIO_2_1 now?

It can but than we need to include the iomux-mx23.h header. It in the
end is the same thing.

> Did you test
> CMD_LED, does it work when toggling the LED?

It does. Of course I tested it :)

--
Otavio Salvador                             O.S. Systems
E-mail: otavio@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br
Marek Vasut - Jan. 30, 2013, 4:15 p.m.
Dear Otavio Salvador,

> On Wed, Jan 30, 2013 at 2:05 PM, Marek Vasut <marex@denx.de> wrote:
> > Dear Otavio Salvador,
> > 
> >> On Wed, Jan 30, 2013 at 1:39 PM, Marek Vasut <marex@denx.de> wrote:
> >> > Dear Otavio Salvador,
> >> > 
> >> >> On Wed, Jan 30, 2013 at 12:13 PM, Marek Vasut <marex@denx.de> wrote:
> >> >> > Dear Otavio Salvador,
> >> >> > 
> >> >> >> This allow user to know if the bootloader is running, even without
> >> >> >> a serial console.
> >> >> >> 
> >> >> >> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
> >> >> > 
> >> >> > Uh oh, how does this know which GPIO to toggle to drive the led
> >> >> > this time ?
> >> >> 
> >> >> The problem wasn't the code but me. I wasn't able to find the right
> >> >> GPIO number at that time.
> >> > 
> >> > This is not my question. My question is how does this toggle the GPIO
> >> > for the LED?
> >> 
> >> gpio_led driver (drivers/misc/gpio_led.c) does it.
> >> 
> >> ...
> >> void __led_init(led_id_t mask, int state)
> >> {
> >> 
> >>         gpio_request(mask, "gpio_led");
> >>         gpio_direction_output(mask, state == STATUS_LED_ON);
> >> 
> >> }
> >> 
> >> void __led_set(led_id_t mask, int state)
> >> {
> >> 
> >>         gpio_set_value(mask, state == STATUS_LED_ON);
> >> 
> >> }
> >> ...
> > 
> > Ok, this didn't explain much to me.
> > 
> >> > Moreover, you never set the LED GPIO as output.
> >> 
> >> The driver handles it by itself.
> > 
> > Oh ok.
> > 
> > Now that I did read through the code, I have few more questions:
> > 
> > Why can't STATUS_LED_BIT be the MX23_PAD_SSP1_DETECT__GPIO_2_1 now?
> 
> It can but than we need to include the iomux-mx23.h header. It in the
> end is the same thing.

In the end, when I read the code in two hours, I'll be wondering what this magic 
junk is. Thus, we will go for this and apply the adjustment for iomux.

Best regards,
Marek Vasut
Otavio Salvador - Jan. 30, 2013, 5:02 p.m.
On Wed, Jan 30, 2013 at 2:15 PM, Marek Vasut <marex@denx.de> wrote:
> Dear Otavio Salvador,
>
>> On Wed, Jan 30, 2013 at 2:05 PM, Marek Vasut <marex@denx.de> wrote:
>> > Dear Otavio Salvador,
>> >
>> >> On Wed, Jan 30, 2013 at 1:39 PM, Marek Vasut <marex@denx.de> wrote:
>> >> > Dear Otavio Salvador,
>> >> >
>> >> >> On Wed, Jan 30, 2013 at 12:13 PM, Marek Vasut <marex@denx.de> wrote:
>> >> >> > Dear Otavio Salvador,
>> >> >> >
>> >> >> >> This allow user to know if the bootloader is running, even without
>> >> >> >> a serial console.
>> >> >> >>
>> >> >> >> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
>> >> >> >
>> >> >> > Uh oh, how does this know which GPIO to toggle to drive the led
>> >> >> > this time ?
>> >> >>
>> >> >> The problem wasn't the code but me. I wasn't able to find the right
>> >> >> GPIO number at that time.
>> >> >
>> >> > This is not my question. My question is how does this toggle the GPIO
>> >> > for the LED?
>> >>
>> >> gpio_led driver (drivers/misc/gpio_led.c) does it.
>> >>
>> >> ...
>> >> void __led_init(led_id_t mask, int state)
>> >> {
>> >>
>> >>         gpio_request(mask, "gpio_led");
>> >>         gpio_direction_output(mask, state == STATUS_LED_ON);
>> >>
>> >> }
>> >>
>> >> void __led_set(led_id_t mask, int state)
>> >> {
>> >>
>> >>         gpio_set_value(mask, state == STATUS_LED_ON);
>> >>
>> >> }
>> >> ...
>> >
>> > Ok, this didn't explain much to me.
>> >
>> >> > Moreover, you never set the LED GPIO as output.
>> >>
>> >> The driver handles it by itself.
>> >
>> > Oh ok.
>> >
>> > Now that I did read through the code, I have few more questions:
>> >
>> > Why can't STATUS_LED_BIT be the MX23_PAD_SSP1_DETECT__GPIO_2_1 now?
>>
>> It can but than we need to include the iomux-mx23.h header. It in the
>> end is the same thing.
>
> In the end, when I read the code in two hours, I'll be wondering what this magic
> junk is. Thus, we will go for this and apply the adjustment for iomux.

So in the end you prefer me to use the MUX name? I am fine in doing it for v2.

--
Otavio Salvador                             O.S. Systems
E-mail: otavio@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br
Marek Vasut - Jan. 30, 2013, 6:14 p.m.
Dear Otavio Salvador,

> On Wed, Jan 30, 2013 at 2:15 PM, Marek Vasut <marex@denx.de> wrote:
> > Dear Otavio Salvador,
> > 
> >> On Wed, Jan 30, 2013 at 2:05 PM, Marek Vasut <marex@denx.de> wrote:
> >> > Dear Otavio Salvador,
> >> > 
> >> >> On Wed, Jan 30, 2013 at 1:39 PM, Marek Vasut <marex@denx.de> wrote:
> >> >> > Dear Otavio Salvador,
> >> >> > 
> >> >> >> On Wed, Jan 30, 2013 at 12:13 PM, Marek Vasut <marex@denx.de> wrote:
> >> >> >> > Dear Otavio Salvador,
> >> >> >> > 
> >> >> >> >> This allow user to know if the bootloader is running, even
> >> >> >> >> without a serial console.
> >> >> >> >> 
> >> >> >> >> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
> >> >> >> > 
> >> >> >> > Uh oh, how does this know which GPIO to toggle to drive the led
> >> >> >> > this time ?
> >> >> >> 
> >> >> >> The problem wasn't the code but me. I wasn't able to find the
> >> >> >> right GPIO number at that time.
> >> >> > 
> >> >> > This is not my question. My question is how does this toggle the
> >> >> > GPIO for the LED?
> >> >> 
> >> >> gpio_led driver (drivers/misc/gpio_led.c) does it.
> >> >> 
> >> >> ...
> >> >> void __led_init(led_id_t mask, int state)
> >> >> {
> >> >> 
> >> >>         gpio_request(mask, "gpio_led");
> >> >>         gpio_direction_output(mask, state == STATUS_LED_ON);
> >> >> 
> >> >> }
> >> >> 
> >> >> void __led_set(led_id_t mask, int state)
> >> >> {
> >> >> 
> >> >>         gpio_set_value(mask, state == STATUS_LED_ON);
> >> >> 
> >> >> }
> >> >> ...
> >> > 
> >> > Ok, this didn't explain much to me.
> >> > 
> >> >> > Moreover, you never set the LED GPIO as output.
> >> >> 
> >> >> The driver handles it by itself.
> >> > 
> >> > Oh ok.
> >> > 
> >> > Now that I did read through the code, I have few more questions:
> >> > 
> >> > Why can't STATUS_LED_BIT be the MX23_PAD_SSP1_DETECT__GPIO_2_1 now?
> >> 
> >> It can but than we need to include the iomux-mx23.h header. It in the
> >> end is the same thing.
> > 
> > In the end, when I read the code in two hours, I'll be wondering what
> > this magic junk is. Thus, we will go for this and apply the adjustment
> > for iomux.
> 
> So in the end you prefer me to use the MUX name? I am fine in doing it for
> v2.

Yes, for my part, I'd prefer to use the mux name.

Best regards,
Marek Vasut

Patch

diff --git a/board/olimex/mx23_olinuxino/mx23_olinuxino.c b/board/olimex/mx23_olinuxino/mx23_olinuxino.c
index 6a6053b..2501417 100644
--- a/board/olimex/mx23_olinuxino/mx23_olinuxino.c
+++ b/board/olimex/mx23_olinuxino/mx23_olinuxino.c
@@ -28,6 +28,9 @@ 
 #include <asm/arch/imx-regs.h>
 #include <asm/arch/clock.h>
 #include <asm/arch/sys_proto.h>
+#ifdef CONFIG_STATUS_LED
+#include <status_led.h>
+#endif
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -67,5 +70,9 @@  int board_init(void)
 	/* Adress of boot parameters */
 	gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100;
 
+#if defined(CONFIG_STATUS_LED) && defined(STATUS_LED_BOOT)
+	status_led_set(STATUS_LED_BOOT, STATUS_LED_STATE);
+#endif
+
 	return 0;
 }
diff --git a/board/olimex/mx23_olinuxino/spl_boot.c b/board/olimex/mx23_olinuxino/spl_boot.c
index 7def8bc..3bbf5ad 100644
--- a/board/olimex/mx23_olinuxino/spl_boot.c
+++ b/board/olimex/mx23_olinuxino/spl_boot.c
@@ -84,6 +84,10 @@  const iomux_cfg_t iomux_setup[] = {
 	MX23_PAD_EMI_RASN__EMI_RASN | MUX_CONFIG_EMI,
 	MX23_PAD_EMI_WEN__EMI_WEN | MUX_CONFIG_EMI,
 
+	/* Green LED */
+	MX23_PAD_SSP1_DETECT__GPIO_2_1 |
+		(MXS_PAD_3V3 | MXS_PAD_4MA | MXS_PAD_NOPULL),
+
 	/* MMC 0 */
 	MX23_PAD_SSP1_CMD__SSP1_CMD | MUX_CONFIG_SSP,
 	MX23_PAD_SSP1_DATA0__SSP1_DATA0 | MUX_CONFIG_SSP,
diff --git a/include/configs/mx23_olinuxino.h b/include/configs/mx23_olinuxino.h
index 7983c5d..968aec8 100644
--- a/include/configs/mx23_olinuxino.h
+++ b/include/configs/mx23_olinuxino.h
@@ -56,6 +56,7 @@ 
 #define	CONFIG_CMD_EXT2
 #define	CONFIG_CMD_FAT
 #define	CONFIG_CMD_GPIO
+#define	CONFIG_CMD_LED
 #define	CONFIG_CMD_MMC
 
 /*
@@ -112,6 +113,17 @@ 
 #define	CONFIG_BAUDRATE			115200	/* Default baud rate */
 
 /*
+ * Status LED
+ */
+#define	CONFIG_STATUS_LED
+#define	CONFIG_GPIO_LED
+#define	CONFIG_BOARD_SPECIFIC_LED
+#define	STATUS_LED_BOOT	0
+#define	STATUS_LED_BIT	10
+#define	STATUS_LED_STATE	STATUS_LED_ON
+#define	STATUS_LED_PERIOD	(CONFIG_SYS_HZ / 2)
+
+/*
  * MMC Driver
  */
 #ifdef	CONFIG_CMD_MMC