| Message ID | 1359548001-14278-7-git-send-email-otavio@ossystems.com.br |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
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
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
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
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
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
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
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
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
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
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
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(+)