Patchwork [U-Boot,v4,2/2] OMAP3: igep00x0: add boot status GPIO LED

login
register
mail settings
Submitter Javier Martinez Canillas
Date Dec. 26, 2012, 3:24 a.m.
Message ID <1356492241-14015-2-git-send-email-javier.martinez@collabora.co.uk>
Download mbox | patch
Permalink /patch/208140/
State Superseded
Delegated to: Tom Rini
Headers show

Comments

Javier Martinez Canillas - Dec. 26, 2012, 3:24 a.m.
This patch adds an GPIO LED boot status for IGEP boards.

The GPIO LED used is the red LED0 while the Linux kernel
uses the green LED0 as the boot status.

By using different GPIO LEDs, the user can know in which
step of the boot process the board currently is.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---

Changes since v3:
    - Avoid code duplication but having a single show_boot_progress()
      function as suggested by Wolfgang Denk.

Changes since v2:
    - Use show_boot_progress() instead implementing yet another boot status
      signalling as suggested by Wolfgang Denk.

Changes since v1:
    - Don't set gd->bd->bi_arch_number since is done in arch/arm/lib/board.c
    - Use CONFIG_MACH_TYPE == MACH_TYPE_IGEP0020 instead of check bi_arch_number
      as suggested by Igor Grinberg.

 board/isee/igep00x0/igep00x0.c |   15 +++++++++++++++
 board/isee/igep00x0/igep00x0.h |    8 ++++++++
 include/configs/igep00x0.h     |    3 +++
 3 files changed, 26 insertions(+), 0 deletions(-)
Igor Grinberg - Dec. 27, 2012, 1:16 p.m.
On 12/26/12 05:24, Javier Martinez Canillas wrote:
> This patch adds an GPIO LED boot status for IGEP boards.
> 
> The GPIO LED used is the red LED0 while the Linux kernel
> uses the green LED0 as the boot status.
> 
> By using different GPIO LEDs, the user can know in which
> step of the boot process the board currently is.
> 
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
> 
> Changes since v3:
>     - Avoid code duplication but having a single show_boot_progress()
>       function as suggested by Wolfgang Denk.
> 
> Changes since v2:
>     - Use show_boot_progress() instead implementing yet another boot status
>       signalling as suggested by Wolfgang Denk.
> 
> Changes since v1:
>     - Don't set gd->bd->bi_arch_number since is done in arch/arm/lib/board.c
>     - Use CONFIG_MACH_TYPE == MACH_TYPE_IGEP0020 instead of check bi_arch_number
>       as suggested by Igor Grinberg.
> 
>  board/isee/igep00x0/igep00x0.c |   15 +++++++++++++++
>  board/isee/igep00x0/igep00x0.h |    8 ++++++++
>  include/configs/igep00x0.h     |    3 +++
>  3 files changed, 26 insertions(+), 0 deletions(-)
> 
> diff --git a/board/isee/igep00x0/igep00x0.c b/board/isee/igep00x0/igep00x0.c
> index c8b2fbf..c432129 100644
> --- a/board/isee/igep00x0/igep00x0.c
> +++ b/board/isee/igep00x0/igep00x0.c
> @@ -62,6 +62,21 @@ int board_init(void)
>  	return 0;
>  }
>  
> +#if defined(CONFIG_SHOW_BOOT_PROGRESS) && !defined(CONFIG_SPL_BUILD)
> +void show_boot_progress(int val)
> +{
> +	if (val < 0) {
> +		/* something went wrong */
> +		return;
> +	}
> +
> +	if (!gpio_request(IGEP00X0_GPIO_LED, "")) {
> +		gpio_direction_output(IGEP00X0_GPIO_LED, 0);
> +		gpio_set_value(IGEP00X0_GPIO_LED, 1);

gpio_direction_output() also sets the value.
Why do you need to call gpio_set_value() afterwards?
Also, you don't have any delay in between,
so it does not look like a "blink"...

> +	}
> +}
> +#endif
> +
>  #ifdef CONFIG_SPL_BUILD
>  /*
>   * Routine: omap_rev_string
> diff --git a/board/isee/igep00x0/igep00x0.h b/board/isee/igep00x0/igep00x0.h
> index 83822d2..4b81960 100644
> --- a/board/isee/igep00x0/igep00x0.h
> +++ b/board/isee/igep00x0/igep00x0.h
> @@ -23,6 +23,14 @@
>  #ifndef _IGEP00X0_H_
>  #define _IGEP00X0_H_
>  
> +#if (CONFIG_MACH_TYPE == MACH_TYPE_IGEP0020)
> +#define IGEP00X0_GPIO_LED 27
> +#endif
> +
> +#if (CONFIG_MACH_TYPE == MACH_TYPE_IGEP0030)
> +#define IGEP00X0_GPIO_LED 16
> +#endif
> +
>  const omap3_sysinfo sysinfo = {
>  	DDR_STACKED,
>  #if (CONFIG_MACH_TYPE == MACH_TYPE_IGEP0020)
> diff --git a/include/configs/igep00x0.h b/include/configs/igep00x0.h
> index be7937d..07b1d3a 100644
> --- a/include/configs/igep00x0.h
> +++ b/include/configs/igep00x0.h
> @@ -82,6 +82,9 @@
>  #define CONFIG_OMAP_HSMMC		1
>  #define CONFIG_DOS_PARTITION		1
>  
> +/* define to enable boot progress via leds */
> +#define CONFIG_SHOW_BOOT_PROGRESS
> +
>  /* USB */
>  #define CONFIG_MUSB_UDC			1
>  #define CONFIG_USB_OMAP3		1
Javier Martinez Canillas - Dec. 27, 2012, 1:26 p.m.
On 12/27/2012 02:16 PM, Igor Grinberg wrote:> On 12/26/12 05:24,
Javier Martinez Canillas wrote:
>> This patch adds an GPIO LED boot status for IGEP boards.
>>
>> The GPIO LED used is the red LED0 while the Linux kernel
>> uses the green LED0 as the boot status.
>>
>> By using different GPIO LEDs, the user can know in which
>> step of the boot process the board currently is.
>>
>> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>> ---
>>
>> Changes since v3:
>>     - Avoid code duplication but having a single show_boot_progress()
>>       function as suggested by Wolfgang Denk.
>>
>> Changes since v2:
>>     - Use show_boot_progress() instead implementing yet another boot status
>>       signalling as suggested by Wolfgang Denk.
>>
>> Changes since v1:
>>     - Don't set gd->bd->bi_arch_number since is done in arch/arm/lib/board.c
>>     - Use CONFIG_MACH_TYPE == MACH_TYPE_IGEP0020 instead of check bi_arch_number
>>       as suggested by Igor Grinberg.
>>
>>  board/isee/igep00x0/igep00x0.c |   15 +++++++++++++++
>>  board/isee/igep00x0/igep00x0.h |    8 ++++++++
>>  include/configs/igep00x0.h     |    3 +++
>>  3 files changed, 26 insertions(+), 0 deletions(-)
>>
>> diff --git a/board/isee/igep00x0/igep00x0.c b/board/isee/igep00x0/igep00x0.c
>> index c8b2fbf..c432129 100644
>> --- a/board/isee/igep00x0/igep00x0.c
>> +++ b/board/isee/igep00x0/igep00x0.c
>> @@ -62,6 +62,21 @@ int board_init(void)
>>  	return 0;
>>  }
>>
>> +#if defined(CONFIG_SHOW_BOOT_PROGRESS) && !defined(CONFIG_SPL_BUILD)
>> +void show_boot_progress(int val)
>> +{
>> +	if (val < 0) {
>> +		/* something went wrong */
>> +		return;
>> +	}
>> +
>> +	if (!gpio_request(IGEP00X0_GPIO_LED, "")) {
>> +		gpio_direction_output(IGEP00X0_GPIO_LED, 0);
>> +		gpio_set_value(IGEP00X0_GPIO_LED, 1);
>
> gpio_direction_output() also sets the value.
> Why do you need to call gpio_set_value() afterwards?
> Also, you don't have any delay in between,
> so it does not look like a "blink"...
>

Hi Igor,

Yes, it's not meant to blink, but keep the led on I just didn't know
that it also sets the value.

Will send a new version with gpio_direction_output(IGEP00X0_GPIO_LED,
1) and remove gpio_set_value().

>> +	}
>> +}
>> +#endif
>> +
>>  #ifdef CONFIG_SPL_BUILD
>>  /*
>>   * Routine: omap_rev_string
>> diff --git a/board/isee/igep00x0/igep00x0.h b/board/isee/igep00x0/igep00x0.h
>> index 83822d2..4b81960 100644
>> --- a/board/isee/igep00x0/igep00x0.h
>> +++ b/board/isee/igep00x0/igep00x0.h
>> @@ -23,6 +23,14 @@
>>  #ifndef _IGEP00X0_H_
>>  #define _IGEP00X0_H_
>>
>> +#if (CONFIG_MACH_TYPE == MACH_TYPE_IGEP0020)
>> +#define IGEP00X0_GPIO_LED 27
>> +#endif
>> +
>> +#if (CONFIG_MACH_TYPE == MACH_TYPE_IGEP0030)
>> +#define IGEP00X0_GPIO_LED 16
>> +#endif
>> +
>>  const omap3_sysinfo sysinfo = {
>>  	DDR_STACKED,
>>  #if (CONFIG_MACH_TYPE == MACH_TYPE_IGEP0020)
>> diff --git a/include/configs/igep00x0.h b/include/configs/igep00x0.h
>> index be7937d..07b1d3a 100644
>> --- a/include/configs/igep00x0.h
>> +++ b/include/configs/igep00x0.h
>> @@ -82,6 +82,9 @@
>>  #define CONFIG_OMAP_HSMMC		1
>>  #define CONFIG_DOS_PARTITION		1
>>
>> +/* define to enable boot progress via leds */
>> +#define CONFIG_SHOW_BOOT_PROGRESS
>> +
>>  /* USB */
>>  #define CONFIG_MUSB_UDC			1
>>  #define CONFIG_USB_OMAP3		1
>

Thanks a lot and best regards,
Javier

Patch

diff --git a/board/isee/igep00x0/igep00x0.c b/board/isee/igep00x0/igep00x0.c
index c8b2fbf..c432129 100644
--- a/board/isee/igep00x0/igep00x0.c
+++ b/board/isee/igep00x0/igep00x0.c
@@ -62,6 +62,21 @@  int board_init(void)
 	return 0;
 }
 
+#if defined(CONFIG_SHOW_BOOT_PROGRESS) && !defined(CONFIG_SPL_BUILD)
+void show_boot_progress(int val)
+{
+	if (val < 0) {
+		/* something went wrong */
+		return;
+	}
+
+	if (!gpio_request(IGEP00X0_GPIO_LED, "")) {
+		gpio_direction_output(IGEP00X0_GPIO_LED, 0);
+		gpio_set_value(IGEP00X0_GPIO_LED, 1);
+	}
+}
+#endif
+
 #ifdef CONFIG_SPL_BUILD
 /*
  * Routine: omap_rev_string
diff --git a/board/isee/igep00x0/igep00x0.h b/board/isee/igep00x0/igep00x0.h
index 83822d2..4b81960 100644
--- a/board/isee/igep00x0/igep00x0.h
+++ b/board/isee/igep00x0/igep00x0.h
@@ -23,6 +23,14 @@ 
 #ifndef _IGEP00X0_H_
 #define _IGEP00X0_H_
 
+#if (CONFIG_MACH_TYPE == MACH_TYPE_IGEP0020)
+#define IGEP00X0_GPIO_LED 27
+#endif
+
+#if (CONFIG_MACH_TYPE == MACH_TYPE_IGEP0030)
+#define IGEP00X0_GPIO_LED 16
+#endif
+
 const omap3_sysinfo sysinfo = {
 	DDR_STACKED,
 #if (CONFIG_MACH_TYPE == MACH_TYPE_IGEP0020)
diff --git a/include/configs/igep00x0.h b/include/configs/igep00x0.h
index be7937d..07b1d3a 100644
--- a/include/configs/igep00x0.h
+++ b/include/configs/igep00x0.h
@@ -82,6 +82,9 @@ 
 #define CONFIG_OMAP_HSMMC		1
 #define CONFIG_DOS_PARTITION		1
 
+/* define to enable boot progress via leds */
+#define CONFIG_SHOW_BOOT_PROGRESS
+
 /* USB */
 #define CONFIG_MUSB_UDC			1
 #define CONFIG_USB_OMAP3		1