All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] Cubieboard2:SUN7I:Add LED BOOT support
       [not found] <ada@thorsis.com>
@ 2021-12-08 14:22 ` Javad Rahimi
  2021-12-08 14:25   ` Aw: " Frank Wunderlich
  0 siblings, 1 reply; 11+ messages in thread
From: Javad Rahimi @ 2021-12-08 14:22 UTC (permalink / raw)
  To: u-boot; +Cc: Javad Rahimi

This feature makes it possible to assign one of
LED1(PH20) and LED2(PH21) to BOOT process LED.
User should activates the "Enable status LED API" in
"Device Drivers -> LED Support"

Signed-off-by: Javad Rahimi <javad321javad@gmail.com>
---
This is my first contributation in open source world.
I'm sorry if I have mistakes in my commits and versioning.
I do my best to learn fast.

Changes in v2:
- Missed braces added
- Unnecessary debug removed
- Some typo fixed

 board/sunxi/board.c | 49 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 4f5747c34a..5e2f6ae902 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -1002,3 +1002,52 @@ int board_fit_config_name_match(const char *name)
 	return ret;
 }
 #endif
+
+#if defined(CONFIG_LED_STATUS) && defined(CONFIG_LED_STATUS_BOOT) && defined(CONFIG_LED_STATUS_BOARD_SPECIFIC)
+
+#define CUBIE2_LED_BOOT_GPIO  "PH20"
+static int gpio_boot_led;
+
+void __led_init(led_id_t mask, int state)
+{
+	int ret;
+
+	if (mask != CONFIG_LED_STATUS_BOOT)
+		return;
+
+	ret = gpio_lookup_name(CUBIE2_LED_BOOT_GPIO, NULL, NULL, &gpio_boot_led);
+
+	if (ret)
+		return;
+
+	ret = gpio_request(gpio_boot_led, "boot_led");
+	if (ret == -1) {
+		debug("[gpio_request] Error:%d\n", ret);
+		return;
+	}
+
+	ret = gpio_direction_output(gpio_boot_led, 1);
+	if (ret == -1) {
+		debug("[gpio_direction_output] Error:%d\n", ret);
+		return;
+	}
+	__led_set(mask, state);
+}
+
+void __led_set(led_id_t mask, int state)
+{
+	if (mask != CONFIG_LED_STATUS_BOOT)
+		return;
+
+	gpio_set_value(gpio_boot_led, state);
+}
+
+void __led_toggle(led_id_t mask)
+{
+	if (mask != CONFIG_LED_STATUS_BOOT)
+		return;
+
+	gpio_set_value(gpio_boot_led, !gpio_get_value(gpio_boot_led));
+}
+
+#endif
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Aw: [PATCH v2] Cubieboard2:SUN7I:Add LED BOOT support
  2021-12-08 14:22 ` [PATCH v2] Cubieboard2:SUN7I:Add LED BOOT support Javad Rahimi
@ 2021-12-08 14:25   ` Frank Wunderlich
  2021-12-08 14:34     ` Andre Przywara
  0 siblings, 1 reply; 11+ messages in thread
From: Frank Wunderlich @ 2021-12-08 14:25 UTC (permalink / raw)
  To: Javad Rahimi, Jagan Teki, Andre Przywara; +Cc: u-boot, Javad Rahimi

Hi,

you should add maintainer email for your patch

$ scripts/get_maintainer.pl board/sunxi/board.c
Jagan Teki <jagan@amarulasolutions.com> (maintainer:ARM SUNXI)
Andre Przywara <andre.przywara@arm.com> (maintainer:ARM SUNXI)
u-boot@lists.denx.de (open list)

regards Frank


> Gesendet: Mittwoch, 08. Dezember 2021 um 15:22 Uhr
> Von: "Javad Rahimi" <javad321javad@gmail.com>
> An: u-boot@lists.denx.de
> Cc: "Javad Rahimi" <javad321javad@gmail.com>
> Betreff: [PATCH v2] Cubieboard2:SUN7I:Add LED BOOT support
>
> This feature makes it possible to assign one of
> LED1(PH20) and LED2(PH21) to BOOT process LED.
> User should activates the "Enable status LED API" in
> "Device Drivers -> LED Support"
>
> Signed-off-by: Javad Rahimi <javad321javad@gmail.com>
> ---
> This is my first contributation in open source world.
> I'm sorry if I have mistakes in my commits and versioning.
> I do my best to learn fast.
>
> Changes in v2:
> - Missed braces added
> - Unnecessary debug removed
> - Some typo fixed
>
>  board/sunxi/board.c | 49 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
>
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index 4f5747c34a..5e2f6ae902 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -1002,3 +1002,52 @@ int board_fit_config_name_match(const char *name)
>  	return ret;
>  }
>  #endif
> +
> +#if defined(CONFIG_LED_STATUS) && defined(CONFIG_LED_STATUS_BOOT) && defined(CONFIG_LED_STATUS_BOARD_SPECIFIC)
> +
> +#define CUBIE2_LED_BOOT_GPIO  "PH20"
> +static int gpio_boot_led;
> +
> +void __led_init(led_id_t mask, int state)
> +{
> +	int ret;
> +
> +	if (mask != CONFIG_LED_STATUS_BOOT)
> +		return;
> +
> +	ret = gpio_lookup_name(CUBIE2_LED_BOOT_GPIO, NULL, NULL, &gpio_boot_led);
> +
> +	if (ret)
> +		return;
> +
> +	ret = gpio_request(gpio_boot_led, "boot_led");
> +	if (ret == -1) {
> +		debug("[gpio_request] Error:%d\n", ret);
> +		return;
> +	}
> +
> +	ret = gpio_direction_output(gpio_boot_led, 1);
> +	if (ret == -1) {
> +		debug("[gpio_direction_output] Error:%d\n", ret);
> +		return;
> +	}
> +	__led_set(mask, state);
> +}
> +
> +void __led_set(led_id_t mask, int state)
> +{
> +	if (mask != CONFIG_LED_STATUS_BOOT)
> +		return;
> +
> +	gpio_set_value(gpio_boot_led, state);
> +}
> +
> +void __led_toggle(led_id_t mask)
> +{
> +	if (mask != CONFIG_LED_STATUS_BOOT)
> +		return;
> +
> +	gpio_set_value(gpio_boot_led, !gpio_get_value(gpio_boot_led));
> +}
> +
> +#endif
> --
> 2.25.1
>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] Cubieboard2:SUN7I:Add LED BOOT support
  2021-12-08 14:25   ` Aw: " Frank Wunderlich
@ 2021-12-08 14:34     ` Andre Przywara
  2021-12-08 15:49       ` Javad Rahimi
       [not found]       ` <CAL245av-S3yyrxwZ8A3W2wdkYWC3bwOvPWh1BoaU25JEZ8uOvQ@mail.gmail.com>
  0 siblings, 2 replies; 11+ messages in thread
From: Andre Przywara @ 2021-12-08 14:34 UTC (permalink / raw)
  To: Frank Wunderlich; +Cc: Javad Rahimi, Jagan Teki, u-boot

On Wed, 8 Dec 2021 15:25:54 +0100
Frank Wunderlich <frank-w@public-files.de> wrote:

Hi,

> you should add maintainer email for your patch
> 
> $ scripts/get_maintainer.pl board/sunxi/board.c
> Jagan Teki <jagan@amarulasolutions.com> (maintainer:ARM SUNXI)
> Andre Przywara <andre.przywara@arm.com> (maintainer:ARM SUNXI)
> u-boot@lists.denx.de (open list)

Thanks Frank!

> > Gesendet: Mittwoch, 08. Dezember 2021 um 15:22 Uhr
> > Von: "Javad Rahimi" <javad321javad@gmail.com>
> > An: u-boot@lists.denx.de
> > Cc: "Javad Rahimi" <javad321javad@gmail.com>
> > Betreff: [PATCH v2] Cubieboard2:SUN7I:Add LED BOOT support
> >
> > This feature makes it possible to assign one of
> > LED1(PH20) and LED2(PH21) to BOOT process LED.
> > User should activates the "Enable status LED API" in
> > "Device Drivers -> LED Support"

Please have a look at the current pinephone_defconfig, because that uses
the boot LED already in a much easier fashion:
https://source.denx.de/u-boot/u-boot/-/commit/0534153fd1

Cheers,
Andre

> >
> > Signed-off-by: Javad Rahimi <javad321javad@gmail.com>
> > ---
> > This is my first contributation in open source world.
> > I'm sorry if I have mistakes in my commits and versioning.
> > I do my best to learn fast.
> >
> > Changes in v2:
> > - Missed braces added
> > - Unnecessary debug removed
> > - Some typo fixed
> >
> >  board/sunxi/board.c | 49 +++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 49 insertions(+)
> >
> > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> > index 4f5747c34a..5e2f6ae902 100644
> > --- a/board/sunxi/board.c
> > +++ b/board/sunxi/board.c
> > @@ -1002,3 +1002,52 @@ int board_fit_config_name_match(const char *name)
> >  	return ret;
> >  }
> >  #endif
> > +
> > +#if defined(CONFIG_LED_STATUS) && defined(CONFIG_LED_STATUS_BOOT) && defined(CONFIG_LED_STATUS_BOARD_SPECIFIC)
> > +
> > +#define CUBIE2_LED_BOOT_GPIO  "PH20"
> > +static int gpio_boot_led;
> > +
> > +void __led_init(led_id_t mask, int state)
> > +{
> > +	int ret;
> > +
> > +	if (mask != CONFIG_LED_STATUS_BOOT)
> > +		return;
> > +
> > +	ret = gpio_lookup_name(CUBIE2_LED_BOOT_GPIO, NULL, NULL, &gpio_boot_led);
> > +
> > +	if (ret)
> > +		return;
> > +
> > +	ret = gpio_request(gpio_boot_led, "boot_led");
> > +	if (ret == -1) {
> > +		debug("[gpio_request] Error:%d\n", ret);
> > +		return;
> > +	}
> > +
> > +	ret = gpio_direction_output(gpio_boot_led, 1);
> > +	if (ret == -1) {
> > +		debug("[gpio_direction_output] Error:%d\n", ret);
> > +		return;
> > +	}
> > +	__led_set(mask, state);
> > +}
> > +
> > +void __led_set(led_id_t mask, int state)
> > +{
> > +	if (mask != CONFIG_LED_STATUS_BOOT)
> > +		return;
> > +
> > +	gpio_set_value(gpio_boot_led, state);
> > +}
> > +
> > +void __led_toggle(led_id_t mask)
> > +{
> > +	if (mask != CONFIG_LED_STATUS_BOOT)
> > +		return;
> > +
> > +	gpio_set_value(gpio_boot_led, !gpio_get_value(gpio_boot_led));
> > +}
> > +
> > +#endif
> > --
> > 2.25.1
> >
> >  


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] Cubieboard2:SUN7I:Add LED BOOT support
  2021-12-08 14:34     ` Andre Przywara
@ 2021-12-08 15:49       ` Javad Rahimi
       [not found]       ` <CAL245av-S3yyrxwZ8A3W2wdkYWC3bwOvPWh1BoaU25JEZ8uOvQ@mail.gmail.com>
  1 sibling, 0 replies; 11+ messages in thread
From: Javad Rahimi @ 2021-12-08 15:49 UTC (permalink / raw)
  To: Andre Przywara; +Cc: Jagan Teki, u-boot, hdegoede

On Wed, Dec 8, 2021 at 6:05 PM Andre Przywara <andre.przywara@arm.com> wrote:
>
> On Wed, 8 Dec 2021 15:25:54 +0100
> Frank Wunderlich <frank-w@public-files.de> wrote:
>
> Hi,
>
> > you should add maintainer email for your patch
> >
> > $ scripts/get_maintainer.pl board/sunxi/board.c
> > Jagan Teki <jagan@amarulasolutions.com> (maintainer:ARM SUNXI)
> > Andre Przywara <andre.przywara@arm.com> (maintainer:ARM SUNXI)
> > u-boot@lists.denx.de (open list)
>
> Thanks Frank!
>
Hi Andre,
Thanks for your comments. I studied the pinephone_defconfig.
By default, when activating the same options on Cubieboard2_defconfig
it shows linker error for `__led_init` and `__led_set`.
In other words, they are not defined. So, in this patch, I added the
implementation for these functions for this board.

Regards,
Javad
> > > Gesendet: Mittwoch, 08. Dezember 2021 um 15:22 Uhr
> > > Von: "Javad Rahimi" <javad321javad@gmail.com>
> > > An: u-boot@lists.denx.de
> > > Cc: "Javad Rahimi" <javad321javad@gmail.com>
> > > Betreff: [PATCH v2] Cubieboard2:SUN7I:Add LED BOOT support
> > >
> > > This feature makes it possible to assign one of
> > > LED1(PH20) and LED2(PH21) to BOOT process LED.
> > > User should activates the "Enable status LED API" in
> > > "Device Drivers -> LED Support"
>
> Please have a look at the current pinephone_defconfig, because that uses
> the boot LED already in a much easier fashion:
> https://source.denx.de/u-boot/u-boot/-/commit/0534153fd1
>
> Cheers,
> Andre
>
> > >
> > > Signed-off-by: Javad Rahimi <javad321javad@gmail.com>
> > > ---
> > > This is my first contributation in open source world.
> > > I'm sorry if I have mistakes in my commits and versioning.
> > > I do my best to learn fast.
> > >
> > > Changes in v2:
> > > - Missed braces added
> > > - Unnecessary debug removed
> > > - Some typo fixed
> > >
> > >  board/sunxi/board.c | 49 +++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 49 insertions(+)
> > >
> > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> > > index 4f5747c34a..5e2f6ae902 100644
> > > --- a/board/sunxi/board.c
> > > +++ b/board/sunxi/board.c
> > > @@ -1002,3 +1002,52 @@ int board_fit_config_name_match(const char *name)
> > >     return ret;
> > >  }
> > >  #endif
> > > +
> > > +#if defined(CONFIG_LED_STATUS) && defined(CONFIG_LED_STATUS_BOOT) && defined(CONFIG_LED_STATUS_BOARD_SPECIFIC)
> > > +
> > > +#define CUBIE2_LED_BOOT_GPIO  "PH20"
> > > +static int gpio_boot_led;
> > > +
> > > +void __led_init(led_id_t mask, int state)
> > > +{
> > > +   int ret;
> > > +
> > > +   if (mask != CONFIG_LED_STATUS_BOOT)
> > > +           return;
> > > +
> > > +   ret = gpio_lookup_name(CUBIE2_LED_BOOT_GPIO, NULL, NULL, &gpio_boot_led);
> > > +
> > > +   if (ret)
> > > +           return;
> > > +
> > > +   ret = gpio_request(gpio_boot_led, "boot_led");
> > > +   if (ret == -1) {
> > > +           debug("[gpio_request] Error:%d\n", ret);
> > > +           return;
> > > +   }
> > > +
> > > +   ret = gpio_direction_output(gpio_boot_led, 1);
> > > +   if (ret == -1) {
> > > +           debug("[gpio_direction_output] Error:%d\n", ret);
> > > +           return;
> > > +   }
> > > +   __led_set(mask, state);
> > > +}
> > > +
> > > +void __led_set(led_id_t mask, int state)
> > > +{
> > > +   if (mask != CONFIG_LED_STATUS_BOOT)
> > > +           return;
> > > +
> > > +   gpio_set_value(gpio_boot_led, state);
> > > +}
> > > +
> > > +void __led_toggle(led_id_t mask)
> > > +{
> > > +   if (mask != CONFIG_LED_STATUS_BOOT)
> > > +           return;
> > > +
> > > +   gpio_set_value(gpio_boot_led, !gpio_get_value(gpio_boot_led));
> > > +}
> > > +
> > > +#endif
> > > --
> > > 2.25.1
> > >
> > >
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] Cubieboard2:SUN7I:Add LED BOOT support
       [not found]         ` <20211208170328.7e7c4e29@donnerap.cambridge.arm.com>
@ 2021-12-08 17:18           ` Javad Rahimi
  2021-12-08 17:43             ` Andre Przywara
  0 siblings, 1 reply; 11+ messages in thread
From: Javad Rahimi @ 2021-12-08 17:18 UTC (permalink / raw)
  To: Andre Przywara; +Cc: hdegoede, Jagan Teki, u-boot

On Wed, Dec 8, 2021 at 8:33 PM Andre Przywara <andre.przywara@arm.com> wrote:
>
> On Wed, 8 Dec 2021 19:14:22 +0330
> Javad Rahimi <javad321javad@gmail.com> wrote:
>
> > On Wed, Dec 8, 2021 at 6:05 PM Andre Przywara <andre.przywara@arm.com> wrote:
> > >
> > > On Wed, 8 Dec 2021 15:25:54 +0100
> > > Frank Wunderlich <frank-w@public-files.de> wrote:
> > >
> > > Hi,
> > >
> > > > you should add maintainer email for your patch
> > > >
> > > > $ scripts/get_maintainer.pl board/sunxi/board.c
> > > > Jagan Teki <jagan@amarulasolutions.com> (maintainer:ARM SUNXI)
> > > > Andre Przywara <andre.przywara@arm.com> (maintainer:ARM SUNXI)
> > > > u-boot@lists.denx.de (open list)
> > >
> > > Thanks Frank!
> > >
> > > > > Gesendet: Mittwoch, 08. Dezember 2021 um 15:22 Uhr
> > > > > Von: "Javad Rahimi" <javad321javad@gmail.com>
> > > > > An: u-boot@lists.denx.de
> > > > > Cc: "Javad Rahimi" <javad321javad@gmail.com>
> > > > > Betreff: [PATCH v2] Cubieboard2:SUN7I:Add LED BOOT support
> > > > >
> > > > > This feature makes it possible to assign one of
> > > > > LED1(PH20) and LED2(PH21) to BOOT process LED.
> > > > > User should activates the "Enable status LED API" in
> > > > > "Device Drivers -> LED Support"
> > >
> > > Please have a look at the current pinephone_defconfig, because that uses
> > > the boot LED already in a much easier fashion:
> > > https://source.denx.de/u-boot/u-boot/-/commit/0534153fd1
> > >
> > > Cheers,
> > > Andre
> > >
> > Hi Andre,
> > Thanks for your comments. I studied the pinephone_defconfig.
> > By default, when activating the same options on Cubieboard2_defconfig
> > it shows linker error for `__led_init` and `__led_set`.
> > In other words, they are not defined. So, in this patch, I added the
> >  implementation for these functions for this board.
>
> Did you add the:
> CONFIG_SPL_DRIVERS_MISC=y
> line as well? And re-ran make Cubieboard2_defconfig?
> Because that seemed to work for me.
>
> Cheers,
> Andre
>
>
When I add CONFIG_SPL_DRIVERS_MISC=y
By flashing SD card and turning on the board, It freezes in this step:
```
U-Boot SPL 2022.01-rc3-00025-gf570594bc9-dirty (Dec 08 2021 - 20:36:51 +0330)
```
-----------------------------------------
The customized defconfig file for Cubieboard:
```
CONFIG_ARM=y
CONFIG_ARCH_SUNXI=y
CONFIG_DEFAULT_DEVICE_TREE="sun7i-a20-cubieboard2"
CONFIG_SPL_DRIVERS_MISC=y
CONFIG_SPL=y
CONFIG_MACH_SUN7I=y
CONFIG_DRAM_CLK=480
CONFIG_MMC0_CD_PIN="PH1"
CONFIG_SATAPWR="PB8"
CONFIG_AHCI=y
# CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
CONFIG_SPL_I2C=y
CONFIG_SCSI_AHCI=y
CONFIG_SYS_I2C_MVTWSI=y
CONFIG_SYS_I2C_SLAVE=0x7f
CONFIG_SYS_I2C_SPEED=400000
CONFIG_PHY_REALTEK=y
CONFIG_ETH_DESIGNWARE=y
CONFIG_MII=y
CONFIG_SUN7I_GMAC=y
CONFIG_SCSI=y
CONFIG_USB_EHCI_HCD=y
CONFIG_USB_OHCI_HCD=y
CONFIG_LED_STATUS=y
CONFIG_LED_STATUS_GPIO=y
CONFIG_LED_STATUS0=y
CONFIG_LED_STATUS_BIT=114
CONFIG_LED_STATUS_STATE=2
```
Best Regards,
Javad
> > > > >
> > > > > Signed-off-by: Javad Rahimi <javad321javad@gmail.com>
> > > > > ---
> > > > > This is my first contributation in open source world.
> > > > > I'm sorry if I have mistakes in my commits and versioning.
> > > > > I do my best to learn fast.
> > > > >
> > > > > Changes in v2:
> > > > > - Missed braces added
> > > > > - Unnecessary debug removed
> > > > > - Some typo fixed
> > > > >
> > > > >  board/sunxi/board.c | 49 +++++++++++++++++++++++++++++++++++++++++++++
> > > > >  1 file changed, 49 insertions(+)
> > > > >
> > > > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> > > > > index 4f5747c34a..5e2f6ae902 100644
> > > > > --- a/board/sunxi/board.c
> > > > > +++ b/board/sunxi/board.c
> > > > > @@ -1002,3 +1002,52 @@ int board_fit_config_name_match(const char *name)
> > > > >     return ret;
> > > > >  }
> > > > >  #endif
> > > > > +
> > > > > +#if defined(CONFIG_LED_STATUS) && defined(CONFIG_LED_STATUS_BOOT) && defined(CONFIG_LED_STATUS_BOARD_SPECIFIC)
> > > > > +
> > > > > +#define CUBIE2_LED_BOOT_GPIO  "PH20"
> > > > > +static int gpio_boot_led;
> > > > > +
> > > > > +void __led_init(led_id_t mask, int state)
> > > > > +{
> > > > > +   int ret;
> > > > > +
> > > > > +   if (mask != CONFIG_LED_STATUS_BOOT)
> > > > > +           return;
> > > > > +
> > > > > +   ret = gpio_lookup_name(CUBIE2_LED_BOOT_GPIO, NULL, NULL, &gpio_boot_led);
> > > > > +
> > > > > +   if (ret)
> > > > > +           return;
> > > > > +
> > > > > +   ret = gpio_request(gpio_boot_led, "boot_led");
> > > > > +   if (ret == -1) {
> > > > > +           debug("[gpio_request] Error:%d\n", ret);
> > > > > +           return;
> > > > > +   }
> > > > > +
> > > > > +   ret = gpio_direction_output(gpio_boot_led, 1);
> > > > > +   if (ret == -1) {
> > > > > +           debug("[gpio_direction_output] Error:%d\n", ret);
> > > > > +           return;
> > > > > +   }
> > > > > +   __led_set(mask, state);
> > > > > +}
> > > > > +
> > > > > +void __led_set(led_id_t mask, int state)
> > > > > +{
> > > > > +   if (mask != CONFIG_LED_STATUS_BOOT)
> > > > > +           return;
> > > > > +
> > > > > +   gpio_set_value(gpio_boot_led, state);
> > > > > +}
> > > > > +
> > > > > +void __led_toggle(led_id_t mask)
> > > > > +{
> > > > > +   if (mask != CONFIG_LED_STATUS_BOOT)
> > > > > +           return;
> > > > > +
> > > > > +   gpio_set_value(gpio_boot_led, !gpio_get_value(gpio_boot_led));
> > > > > +}
> > > > > +
> > > > > +#endif
> > > > > --
> > > > > 2.25.1
> > > > >
> > > > >
> > >
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] Cubieboard2:SUN7I:Add LED BOOT support
  2021-12-08 17:18           ` Javad Rahimi
@ 2021-12-08 17:43             ` Andre Przywara
  2021-12-09  2:54               ` Javad Rahimi
  0 siblings, 1 reply; 11+ messages in thread
From: Andre Przywara @ 2021-12-08 17:43 UTC (permalink / raw)
  To: Javad Rahimi; +Cc: hdegoede, Jagan Teki, u-boot

On Wed, 8 Dec 2021 20:48:54 +0330
Javad Rahimi <javad321javad@gmail.com> wrote:

> On Wed, Dec 8, 2021 at 8:33 PM Andre Przywara <andre.przywara@arm.com> wrote:
> >
> > On Wed, 8 Dec 2021 19:14:22 +0330
> > Javad Rahimi <javad321javad@gmail.com> wrote:
> >  
> > > On Wed, Dec 8, 2021 at 6:05 PM Andre Przywara <andre.przywara@arm.com> wrote:  
> > > >
> > > > On Wed, 8 Dec 2021 15:25:54 +0100
> > > > Frank Wunderlich <frank-w@public-files.de> wrote:
> > > >
> > > > Hi,
> > > >  
> > > > > you should add maintainer email for your patch
> > > > >
> > > > > $ scripts/get_maintainer.pl board/sunxi/board.c
> > > > > Jagan Teki <jagan@amarulasolutions.com> (maintainer:ARM SUNXI)
> > > > > Andre Przywara <andre.przywara@arm.com> (maintainer:ARM SUNXI)
> > > > > u-boot@lists.denx.de (open list)  
> > > >
> > > > Thanks Frank!
> > > >  
> > > > > > Gesendet: Mittwoch, 08. Dezember 2021 um 15:22 Uhr
> > > > > > Von: "Javad Rahimi" <javad321javad@gmail.com>
> > > > > > An: u-boot@lists.denx.de
> > > > > > Cc: "Javad Rahimi" <javad321javad@gmail.com>
> > > > > > Betreff: [PATCH v2] Cubieboard2:SUN7I:Add LED BOOT support
> > > > > >
> > > > > > This feature makes it possible to assign one of
> > > > > > LED1(PH20) and LED2(PH21) to BOOT process LED.
> > > > > > User should activates the "Enable status LED API" in
> > > > > > "Device Drivers -> LED Support"  
> > > >
> > > > Please have a look at the current pinephone_defconfig, because that uses
> > > > the boot LED already in a much easier fashion:
> > > > https://source.denx.de/u-boot/u-boot/-/commit/0534153fd1
> > > >
> > > > Cheers,
> > > > Andre
> > > >  
> > > Hi Andre,
> > > Thanks for your comments. I studied the pinephone_defconfig.
> > > By default, when activating the same options on Cubieboard2_defconfig
> > > it shows linker error for `__led_init` and `__led_set`.
> > > In other words, they are not defined. So, in this patch, I added the
> > >  implementation for these functions for this board.  
> >
> > Did you add the:
> > CONFIG_SPL_DRIVERS_MISC=y
> > line as well? And re-ran make Cubieboard2_defconfig?
> > Because that seemed to work for me.
> >
> > Cheers,
> > Andre
> >
> >  
> When I add CONFIG_SPL_DRIVERS_MISC=y
> By flashing SD card and turning on the board, It freezes in this step:
> ```
> U-Boot SPL 2022.01-rc3-00025-gf570594bc9-dirty (Dec 08 2021 - 20:36:51 +0330)
> ```
> -----------------------------------------
> The customized defconfig file for Cubieboard:
> ```
> CONFIG_ARM=y
> CONFIG_ARCH_SUNXI=y
> CONFIG_DEFAULT_DEVICE_TREE="sun7i-a20-cubieboard2"
> CONFIG_SPL_DRIVERS_MISC=y
> CONFIG_SPL=y
> CONFIG_MACH_SUN7I=y
> CONFIG_DRAM_CLK=480
> CONFIG_MMC0_CD_PIN="PH1"
> CONFIG_SATAPWR="PB8"
> CONFIG_AHCI=y
> # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
> CONFIG_SPL_I2C=y
> CONFIG_SCSI_AHCI=y
> CONFIG_SYS_I2C_MVTWSI=y
> CONFIG_SYS_I2C_SLAVE=0x7f
> CONFIG_SYS_I2C_SPEED=400000
> CONFIG_PHY_REALTEK=y
> CONFIG_ETH_DESIGNWARE=y
> CONFIG_MII=y
> CONFIG_SUN7I_GMAC=y
> CONFIG_SCSI=y
> CONFIG_USB_EHCI_HCD=y
> CONFIG_USB_OHCI_HCD=y
> CONFIG_LED_STATUS=y
> CONFIG_LED_STATUS_GPIO=y
> CONFIG_LED_STATUS0=y
> CONFIG_LED_STATUS_BIT=114

This is the GPIO number, you need to adjust this to point to the pin your
LED is connected to. PH20 would be 244, for instance:
('H' - 'A') * 32 + 20

Cheers,
Andre

> CONFIG_LED_STATUS_STATE=2
> ```
> Best Regards,
> Javad
> > > > > >
> > > > > > Signed-off-by: Javad Rahimi <javad321javad@gmail.com>
> > > > > > ---
> > > > > > This is my first contributation in open source world.
> > > > > > I'm sorry if I have mistakes in my commits and versioning.
> > > > > > I do my best to learn fast.
> > > > > >
> > > > > > Changes in v2:
> > > > > > - Missed braces added
> > > > > > - Unnecessary debug removed
> > > > > > - Some typo fixed
> > > > > >
> > > > > >  board/sunxi/board.c | 49 +++++++++++++++++++++++++++++++++++++++++++++
> > > > > >  1 file changed, 49 insertions(+)
> > > > > >
> > > > > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> > > > > > index 4f5747c34a..5e2f6ae902 100644
> > > > > > --- a/board/sunxi/board.c
> > > > > > +++ b/board/sunxi/board.c
> > > > > > @@ -1002,3 +1002,52 @@ int board_fit_config_name_match(const char *name)
> > > > > >     return ret;
> > > > > >  }
> > > > > >  #endif
> > > > > > +
> > > > > > +#if defined(CONFIG_LED_STATUS) && defined(CONFIG_LED_STATUS_BOOT) && defined(CONFIG_LED_STATUS_BOARD_SPECIFIC)
> > > > > > +
> > > > > > +#define CUBIE2_LED_BOOT_GPIO  "PH20"
> > > > > > +static int gpio_boot_led;
> > > > > > +
> > > > > > +void __led_init(led_id_t mask, int state)
> > > > > > +{
> > > > > > +   int ret;
> > > > > > +
> > > > > > +   if (mask != CONFIG_LED_STATUS_BOOT)
> > > > > > +           return;
> > > > > > +
> > > > > > +   ret = gpio_lookup_name(CUBIE2_LED_BOOT_GPIO, NULL, NULL, &gpio_boot_led);
> > > > > > +
> > > > > > +   if (ret)
> > > > > > +           return;
> > > > > > +
> > > > > > +   ret = gpio_request(gpio_boot_led, "boot_led");
> > > > > > +   if (ret == -1) {
> > > > > > +           debug("[gpio_request] Error:%d\n", ret);
> > > > > > +           return;
> > > > > > +   }
> > > > > > +
> > > > > > +   ret = gpio_direction_output(gpio_boot_led, 1);
> > > > > > +   if (ret == -1) {
> > > > > > +           debug("[gpio_direction_output] Error:%d\n", ret);
> > > > > > +           return;
> > > > > > +   }
> > > > > > +   __led_set(mask, state);
> > > > > > +}
> > > > > > +
> > > > > > +void __led_set(led_id_t mask, int state)
> > > > > > +{
> > > > > > +   if (mask != CONFIG_LED_STATUS_BOOT)
> > > > > > +           return;
> > > > > > +
> > > > > > +   gpio_set_value(gpio_boot_led, state);
> > > > > > +}
> > > > > > +
> > > > > > +void __led_toggle(led_id_t mask)
> > > > > > +{
> > > > > > +   if (mask != CONFIG_LED_STATUS_BOOT)
> > > > > > +           return;
> > > > > > +
> > > > > > +   gpio_set_value(gpio_boot_led, !gpio_get_value(gpio_boot_led));
> > > > > > +}
> > > > > > +
> > > > > > +#endif
> > > > > > --
> > > > > > 2.25.1
> > > > > >
> > > > > >  
> > > >  
> >  


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] Cubieboard2:SUN7I:Add LED BOOT support
  2021-12-08 17:43             ` Andre Przywara
@ 2021-12-09  2:54               ` Javad Rahimi
  2021-12-09 11:08                 ` Javad Rahimi
  0 siblings, 1 reply; 11+ messages in thread
From: Javad Rahimi @ 2021-12-09  2:54 UTC (permalink / raw)
  To: Andre Przywara; +Cc: hdegoede, u-boot, Jagan Teki

On Wed, Dec 8, 2021 at 9:13 PM Andre Przywara <andre.przywara@arm.com> wrote:
>
> On Wed, 8 Dec 2021 20:48:54 +0330
> Javad Rahimi <javad321javad@gmail.com> wrote:
>
> > On Wed, Dec 8, 2021 at 8:33 PM Andre Przywara <andre.przywara@arm.com> wrote:
> > >
> > > On Wed, 8 Dec 2021 19:14:22 +0330
> > > Javad Rahimi <javad321javad@gmail.com> wrote:
> > >
> > > > On Wed, Dec 8, 2021 at 6:05 PM Andre Przywara <andre.przywara@arm.com> wrote:
> > > > >
> > > > > On Wed, 8 Dec 2021 15:25:54 +0100
> > > > > Frank Wunderlich <frank-w@public-files.de> wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > > you should add maintainer email for your patch
> > > > > >
> > > > > > $ scripts/get_maintainer.pl board/sunxi/board.c
> > > > > > Jagan Teki <jagan@amarulasolutions.com> (maintainer:ARM SUNXI)
> > > > > > Andre Przywara <andre.przywara@arm.com> (maintainer:ARM SUNXI)
> > > > > > u-boot@lists.denx.de (open list)
> > > > >
> > > > > Thanks Frank!
> > > > >
> > > > > > > Gesendet: Mittwoch, 08. Dezember 2021 um 15:22 Uhr
> > > > > > > Von: "Javad Rahimi" <javad321javad@gmail.com>
> > > > > > > An: u-boot@lists.denx.de
> > > > > > > Cc: "Javad Rahimi" <javad321javad@gmail.com>
> > > > > > > Betreff: [PATCH v2] Cubieboard2:SUN7I:Add LED BOOT support
> > > > > > >
> > > > > > > This feature makes it possible to assign one of
> > > > > > > LED1(PH20) and LED2(PH21) to BOOT process LED.
> > > > > > > User should activates the "Enable status LED API" in
> > > > > > > "Device Drivers -> LED Support"
> > > > >
> > > > > Please have a look at the current pinephone_defconfig, because that uses
> > > > > the boot LED already in a much easier fashion:
> > > > > https://source.denx.de/u-boot/u-boot/-/commit/0534153fd1
> > > > >
> > > > > Cheers,
> > > > > Andre
> > > > >
> > > > Hi Andre,
> > > > Thanks for your comments. I studied the pinephone_defconfig.
> > > > By default, when activating the same options on Cubieboard2_defconfig
> > > > it shows linker error for `__led_init` and `__led_set`.
> > > > In other words, they are not defined. So, in this patch, I added the
> > > >  implementation for these functions for this board.
> > >
> > > Did you add the:
> > > CONFIG_SPL_DRIVERS_MISC=y
> > > line as well? And re-ran make Cubieboard2_defconfig?
> > > Because that seemed to work for me.
> > >
> > > Cheers,
> > > Andre
> > >
> > >
> > When I add CONFIG_SPL_DRIVERS_MISC=y
> > By flashing SD card and turning on the board, It freezes in this step:
> > ```
> > U-Boot SPL 2022.01-rc3-00025-gf570594bc9-dirty (Dec 08 2021 - 20:36:51 +0330)
> > ```
> > -----------------------------------------
> > The customized defconfig file for Cubieboard:
> > ```
> > CONFIG_ARM=y
> > CONFIG_ARCH_SUNXI=y
> > CONFIG_DEFAULT_DEVICE_TREE="sun7i-a20-cubieboard2"
> > CONFIG_SPL_DRIVERS_MISC=y
> > CONFIG_SPL=y
> > CONFIG_MACH_SUN7I=y
> > CONFIG_DRAM_CLK=480
> > CONFIG_MMC0_CD_PIN="PH1"
> > CONFIG_SATAPWR="PB8"
> > CONFIG_AHCI=y
> > # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
> > CONFIG_SPL_I2C=y
> > CONFIG_SCSI_AHCI=y
> > CONFIG_SYS_I2C_MVTWSI=y
> > CONFIG_SYS_I2C_SLAVE=0x7f
> > CONFIG_SYS_I2C_SPEED=400000
> > CONFIG_PHY_REALTEK=y
> > CONFIG_ETH_DESIGNWARE=y
> > CONFIG_MII=y
> > CONFIG_SUN7I_GMAC=y
> > CONFIG_SCSI=y
> > CONFIG_USB_EHCI_HCD=y
> > CONFIG_USB_OHCI_HCD=y
> > CONFIG_LED_STATUS=y
> > CONFIG_LED_STATUS_GPIO=y
> > CONFIG_LED_STATUS0=y
> > CONFIG_LED_STATUS_BIT=114
>
> This is the GPIO number, you need to adjust this to point to the pin your
> LED is connected to. PH20 would be 244, for instance:
> ('H' - 'A') * 32 + 20
>
> Cheers,
> Andre
>
Thanks,
Now the LED is ON, but the loader still freezes in (SPL, I think)
```
U-Boot SPL 2022.01-rc3-00025-gf570594bc9-dirty (Dec 09 2021 - 06:19:40 +0330)
```
Best Regards,
Javad
> > CONFIG_LED_STATUS_STATE=2
> > ```
> > Best Regards,
> > Javad
> > > > > > >
> > > > > > > Signed-off-by: Javad Rahimi <javad321javad@gmail.com>
> > > > > > > ---
> > > > > > > This is my first contributation in open source world.
> > > > > > > I'm sorry if I have mistakes in my commits and versioning.
> > > > > > > I do my best to learn fast.
> > > > > > >
> > > > > > > Changes in v2:
> > > > > > > - Missed braces added
> > > > > > > - Unnecessary debug removed
> > > > > > > - Some typo fixed
> > > > > > >
> > > > > > >  board/sunxi/board.c | 49 +++++++++++++++++++++++++++++++++++++++++++++
> > > > > > >  1 file changed, 49 insertions(+)
> > > > > > >
> > > > > > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> > > > > > > index 4f5747c34a..5e2f6ae902 100644
> > > > > > > --- a/board/sunxi/board.c
> > > > > > > +++ b/board/sunxi/board.c
> > > > > > > @@ -1002,3 +1002,52 @@ int board_fit_config_name_match(const char *name)
> > > > > > >     return ret;
> > > > > > >  }
> > > > > > >  #endif
> > > > > > > +
> > > > > > > +#if defined(CONFIG_LED_STATUS) && defined(CONFIG_LED_STATUS_BOOT) && defined(CONFIG_LED_STATUS_BOARD_SPECIFIC)
> > > > > > > +
> > > > > > > +#define CUBIE2_LED_BOOT_GPIO  "PH20"
> > > > > > > +static int gpio_boot_led;
> > > > > > > +
> > > > > > > +void __led_init(led_id_t mask, int state)
> > > > > > > +{
> > > > > > > +   int ret;
> > > > > > > +
> > > > > > > +   if (mask != CONFIG_LED_STATUS_BOOT)
> > > > > > > +           return;
> > > > > > > +
> > > > > > > +   ret = gpio_lookup_name(CUBIE2_LED_BOOT_GPIO, NULL, NULL, &gpio_boot_led);
> > > > > > > +
> > > > > > > +   if (ret)
> > > > > > > +           return;
> > > > > > > +
> > > > > > > +   ret = gpio_request(gpio_boot_led, "boot_led");
> > > > > > > +   if (ret == -1) {
> > > > > > > +           debug("[gpio_request] Error:%d\n", ret);
> > > > > > > +           return;
> > > > > > > +   }
> > > > > > > +
> > > > > > > +   ret = gpio_direction_output(gpio_boot_led, 1);
> > > > > > > +   if (ret == -1) {
> > > > > > > +           debug("[gpio_direction_output] Error:%d\n", ret);
> > > > > > > +           return;
> > > > > > > +   }
> > > > > > > +   __led_set(mask, state);
> > > > > > > +}
> > > > > > > +
> > > > > > > +void __led_set(led_id_t mask, int state)
> > > > > > > +{
> > > > > > > +   if (mask != CONFIG_LED_STATUS_BOOT)
> > > > > > > +           return;
> > > > > > > +
> > > > > > > +   gpio_set_value(gpio_boot_led, state);
> > > > > > > +}
> > > > > > > +
> > > > > > > +void __led_toggle(led_id_t mask)
> > > > > > > +{
> > > > > > > +   if (mask != CONFIG_LED_STATUS_BOOT)
> > > > > > > +           return;
> > > > > > > +
> > > > > > > +   gpio_set_value(gpio_boot_led, !gpio_get_value(gpio_boot_led));
> > > > > > > +}
> > > > > > > +
> > > > > > > +#endif
> > > > > > > --
> > > > > > > 2.25.1
> > > > > > >
> > > > > > >
> > > > >
> > >
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] Cubieboard2:SUN7I:Add LED BOOT support
  2021-12-09  2:54               ` Javad Rahimi
@ 2021-12-09 11:08                 ` Javad Rahimi
  2021-12-09 11:24                   ` Andre Przywara
  0 siblings, 1 reply; 11+ messages in thread
From: Javad Rahimi @ 2021-12-09 11:08 UTC (permalink / raw)
  To: Andre Przywara; +Cc: u-boot, Jagan Teki, hdegoede

`

On Thu, Dec 9, 2021 at 6:24 AM Javad Rahimi <javad321javad@gmail.com> wrote:
>
> On Wed, Dec 8, 2021 at 9:13 PM Andre Przywara <andre.przywara@arm.com> wrote:
> >
> > On Wed, 8 Dec 2021 20:48:54 +0330
> > Javad Rahimi <javad321javad@gmail.com> wrote:
> >
> > > On Wed, Dec 8, 2021 at 8:33 PM Andre Przywara <andre.przywara@arm.com> wrote:
> > > >
> > > > On Wed, 8 Dec 2021 19:14:22 +0330
> > > > Javad Rahimi <javad321javad@gmail.com> wrote:
> > > >
> > > > > On Wed, Dec 8, 2021 at 6:05 PM Andre Przywara <andre.przywara@arm.com> wrote:
> > > > > >
> > > > > > On Wed, 8 Dec 2021 15:25:54 +0100
> > > > > > Frank Wunderlich <frank-w@public-files.de> wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > > you should add maintainer email for your patch
> > > > > > >
> > > > > > > $ scripts/get_maintainer.pl board/sunxi/board.c
> > > > > > > Jagan Teki <jagan@amarulasolutions.com> (maintainer:ARM SUNXI)
> > > > > > > Andre Przywara <andre.przywara@arm.com> (maintainer:ARM SUNXI)
> > > > > > > u-boot@lists.denx.de (open list)
> > > > > >
> > > > > > Thanks Frank!
> > > > > >
> > > > > > > > Gesendet: Mittwoch, 08. Dezember 2021 um 15:22 Uhr
> > > > > > > > Von: "Javad Rahimi" <javad321javad@gmail.com>
> > > > > > > > An: u-boot@lists.denx.de
> > > > > > > > Cc: "Javad Rahimi" <javad321javad@gmail.com>
> > > > > > > > Betreff: [PATCH v2] Cubieboard2:SUN7I:Add LED BOOT support
> > > > > > > >
> > > > > > > > This feature makes it possible to assign one of
> > > > > > > > LED1(PH20) and LED2(PH21) to BOOT process LED.
> > > > > > > > User should activates the "Enable status LED API" in
> > > > > > > > "Device Drivers -> LED Support"
> > > > > >
> > > > > > Please have a look at the current pinephone_defconfig, because that uses
> > > > > > the boot LED already in a much easier fashion:
> > > > > > https://source.denx.de/u-boot/u-boot/-/commit/0534153fd1
> > > > > >
> > > > > > Cheers,
> > > > > > Andre
> > > > > >
> > > > > Hi Andre,
> > > > > Thanks for your comments. I studied the pinephone_defconfig.
> > > > > By default, when activating the same options on Cubieboard2_defconfig
> > > > > it shows linker error for `__led_init` and `__led_set`.
> > > > > In other words, they are not defined. So, in this patch, I added the
> > > > >  implementation for these functions for this board.
> > > >
> > > > Did you add the:
> > > > CONFIG_SPL_DRIVERS_MISC=y
> > > > line as well? And re-ran make Cubieboard2_defconfig?
> > > > Because that seemed to work for me.
> > > >
> > > > Cheers,
> > > > Andre
> > > >
> > > >
> > > When I add CONFIG_SPL_DRIVERS_MISC=y
> > > By flashing SD card and turning on the board, It freezes in this step:
> > > ```
> > > U-Boot SPL 2022.01-rc3-00025-gf570594bc9-dirty (Dec 08 2021 - 20:36:51 +0330)
> > > ```
> > > -----------------------------------------
> > > The customized defconfig file for Cubieboard:
> > > ```
> > > CONFIG_ARM=y
> > > CONFIG_ARCH_SUNXI=y
> > > CONFIG_DEFAULT_DEVICE_TREE="sun7i-a20-cubieboard2"
> > > CONFIG_SPL_DRIVERS_MISC=y
> > > CONFIG_SPL=y
> > > CONFIG_MACH_SUN7I=y
> > > CONFIG_DRAM_CLK=480
> > > CONFIG_MMC0_CD_PIN="PH1"
> > > CONFIG_SATAPWR="PB8"
> > > CONFIG_AHCI=y
> > > # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
> > > CONFIG_SPL_I2C=y
> > > CONFIG_SCSI_AHCI=y
> > > CONFIG_SYS_I2C_MVTWSI=y
> > > CONFIG_SYS_I2C_SLAVE=0x7f
> > > CONFIG_SYS_I2C_SPEED=400000
> > > CONFIG_PHY_REALTEK=y
> > > CONFIG_ETH_DESIGNWARE=y
> > > CONFIG_MII=y
> > > CONFIG_SUN7I_GMAC=y
> > > CONFIG_SCSI=y
> > > CONFIG_USB_EHCI_HCD=y
> > > CONFIG_USB_OHCI_HCD=y
> > > CONFIG_LED_STATUS=y
> > > CONFIG_LED_STATUS_GPIO=y
> > > CONFIG_LED_STATUS0=y
> > > CONFIG_LED_STATUS_BIT=114
> >
> > This is the GPIO number, you need to adjust this to point to the pin your
> > LED is connected to. PH20 would be 244, for instance:
> > ('H' - 'A') * 32 + 20
> >
> > Cheers,
> > Andre
> >
> Thanks,
> Now the LED is ON, but the loader still freezes in (SPL, I think)
> ```
> U-Boot SPL 2022.01-rc3-00025-gf570594bc9-dirty (Dec 09 2021 - 06:19:40 +0330)
> ```
> Best Regards,
> Javad
Hi Andre,
The freeze problem solved when I placed `status_led_init` function
after
gd->ram_size = sunxi_dram_init();
When it is called before sunxi_dram_init(), the bootloader freezes.

Best Regards,
Javad
> > > CONFIG_LED_STATUS_STATE=2
> > > ```
> > > Best Regards,
> > > Javad
> > > > > > > >
> > > > > > > > Signed-off-by: Javad Rahimi <javad321javad@gmail.com>
> > > > > > > > ---
> > > > > > > > This is my first contributation in open source world.
> > > > > > > > I'm sorry if I have mistakes in my commits and versioning.
> > > > > > > > I do my best to learn fast.
> > > > > > > >
> > > > > > > > Changes in v2:
> > > > > > > > - Missed braces added
> > > > > > > > - Unnecessary debug removed
> > > > > > > > - Some typo fixed
> > > > > > > >
> > > > > > > >  board/sunxi/board.c | 49 +++++++++++++++++++++++++++++++++++++++++++++
> > > > > > > >  1 file changed, 49 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> > > > > > > > index 4f5747c34a..5e2f6ae902 100644
> > > > > > > > --- a/board/sunxi/board.c
> > > > > > > > +++ b/board/sunxi/board.c
> > > > > > > > @@ -1002,3 +1002,52 @@ int board_fit_config_name_match(const char *name)
> > > > > > > >     return ret;
> > > > > > > >  }
> > > > > > > >  #endif
> > > > > > > > +
> > > > > > > > +#if defined(CONFIG_LED_STATUS) && defined(CONFIG_LED_STATUS_BOOT) && defined(CONFIG_LED_STATUS_BOARD_SPECIFIC)
> > > > > > > > +
> > > > > > > > +#define CUBIE2_LED_BOOT_GPIO  "PH20"
> > > > > > > > +static int gpio_boot_led;
> > > > > > > > +
> > > > > > > > +void __led_init(led_id_t mask, int state)
> > > > > > > > +{
> > > > > > > > +   int ret;
> > > > > > > > +
> > > > > > > > +   if (mask != CONFIG_LED_STATUS_BOOT)
> > > > > > > > +           return;
> > > > > > > > +
> > > > > > > > +   ret = gpio_lookup_name(CUBIE2_LED_BOOT_GPIO, NULL, NULL, &gpio_boot_led);
> > > > > > > > +
> > > > > > > > +   if (ret)
> > > > > > > > +           return;
> > > > > > > > +
> > > > > > > > +   ret = gpio_request(gpio_boot_led, "boot_led");
> > > > > > > > +   if (ret == -1) {
> > > > > > > > +           debug("[gpio_request] Error:%d\n", ret);
> > > > > > > > +           return;
> > > > > > > > +   }
> > > > > > > > +
> > > > > > > > +   ret = gpio_direction_output(gpio_boot_led, 1);
> > > > > > > > +   if (ret == -1) {
> > > > > > > > +           debug("[gpio_direction_output] Error:%d\n", ret);
> > > > > > > > +           return;
> > > > > > > > +   }
> > > > > > > > +   __led_set(mask, state);
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +void __led_set(led_id_t mask, int state)
> > > > > > > > +{
> > > > > > > > +   if (mask != CONFIG_LED_STATUS_BOOT)
> > > > > > > > +           return;
> > > > > > > > +
> > > > > > > > +   gpio_set_value(gpio_boot_led, state);
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +void __led_toggle(led_id_t mask)
> > > > > > > > +{
> > > > > > > > +   if (mask != CONFIG_LED_STATUS_BOOT)
> > > > > > > > +           return;
> > > > > > > > +
> > > > > > > > +   gpio_set_value(gpio_boot_led, !gpio_get_value(gpio_boot_led));
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +#endif
> > > > > > > > --
> > > > > > > > 2.25.1
> > > > > > > >
> > > > > > > >
> > > > > >
> > > >
> >

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] Cubieboard2:SUN7I:Add LED BOOT support
  2021-12-09 11:08                 ` Javad Rahimi
@ 2021-12-09 11:24                   ` Andre Przywara
  2021-12-09 12:21                     ` Javad Rahimi
  0 siblings, 1 reply; 11+ messages in thread
From: Andre Przywara @ 2021-12-09 11:24 UTC (permalink / raw)
  To: Javad Rahimi; +Cc: u-boot, Jagan Teki, hdegoede

On Thu, 9 Dec 2021 14:38:08 +0330
Javad Rahimi <javad321javad@gmail.com> wrote:

Hi Javad,

> On Thu, Dec 9, 2021 at 6:24 AM Javad Rahimi <javad321javad@gmail.com> wrote:
> >
> > On Wed, Dec 8, 2021 at 9:13 PM Andre Przywara <andre.przywara@arm.com> wrote:  
> > >
> > > On Wed, 8 Dec 2021 20:48:54 +0330
> > > Javad Rahimi <javad321javad@gmail.com> wrote:
> > >  
> > > > On Wed, Dec 8, 2021 at 8:33 PM Andre Przywara <andre.przywara@arm.com> wrote:  
> > > > >
> > > > > On Wed, 8 Dec 2021 19:14:22 +0330
> > > > > Javad Rahimi <javad321javad@gmail.com> wrote:
> > > > >  
> > > > > > On Wed, Dec 8, 2021 at 6:05 PM Andre Przywara <andre.przywara@arm.com> wrote:  
> > > > > > >
> > > > > > > On Wed, 8 Dec 2021 15:25:54 +0100
> > > > > > > Frank Wunderlich <frank-w@public-files.de> wrote:
> > > > > > >
> > > > > > > Hi,
> > > > > > >  
> > > > > > > > you should add maintainer email for your patch
> > > > > > > >
> > > > > > > > $ scripts/get_maintainer.pl board/sunxi/board.c
> > > > > > > > Jagan Teki <jagan@amarulasolutions.com> (maintainer:ARM SUNXI)
> > > > > > > > Andre Przywara <andre.przywara@arm.com> (maintainer:ARM SUNXI)
> > > > > > > > u-boot@lists.denx.de (open list)  
> > > > > > >
> > > > > > > Thanks Frank!
> > > > > > >  
> > > > > > > > > Gesendet: Mittwoch, 08. Dezember 2021 um 15:22 Uhr
> > > > > > > > > Von: "Javad Rahimi" <javad321javad@gmail.com>
> > > > > > > > > An: u-boot@lists.denx.de
> > > > > > > > > Cc: "Javad Rahimi" <javad321javad@gmail.com>
> > > > > > > > > Betreff: [PATCH v2] Cubieboard2:SUN7I:Add LED BOOT support
> > > > > > > > >
> > > > > > > > > This feature makes it possible to assign one of
> > > > > > > > > LED1(PH20) and LED2(PH21) to BOOT process LED.
> > > > > > > > > User should activates the "Enable status LED API" in
> > > > > > > > > "Device Drivers -> LED Support"  
> > > > > > >
> > > > > > > Please have a look at the current pinephone_defconfig, because that uses
> > > > > > > the boot LED already in a much easier fashion:
> > > > > > > https://source.denx.de/u-boot/u-boot/-/commit/0534153fd1
> > > > > > >
> > > > > > > Cheers,
> > > > > > > Andre
> > > > > > >  
> > > > > > Hi Andre,
> > > > > > Thanks for your comments. I studied the pinephone_defconfig.
> > > > > > By default, when activating the same options on Cubieboard2_defconfig
> > > > > > it shows linker error for `__led_init` and `__led_set`.
> > > > > > In other words, they are not defined. So, in this patch, I added the
> > > > > >  implementation for these functions for this board.  
> > > > >
> > > > > Did you add the:
> > > > > CONFIG_SPL_DRIVERS_MISC=y
> > > > > line as well? And re-ran make Cubieboard2_defconfig?
> > > > > Because that seemed to work for me.
> > > > >
> > > > > Cheers,
> > > > > Andre
> > > > >
> > > > >  
> > > > When I add CONFIG_SPL_DRIVERS_MISC=y
> > > > By flashing SD card and turning on the board, It freezes in this step:
> > > > ```
> > > > U-Boot SPL 2022.01-rc3-00025-gf570594bc9-dirty (Dec 08 2021 - 20:36:51 +0330)
> > > > ```
> > > > -----------------------------------------
> > > > The customized defconfig file for Cubieboard:
> > > > ```
> > > > CONFIG_ARM=y
> > > > CONFIG_ARCH_SUNXI=y
> > > > CONFIG_DEFAULT_DEVICE_TREE="sun7i-a20-cubieboard2"
> > > > CONFIG_SPL_DRIVERS_MISC=y
> > > > CONFIG_SPL=y
> > > > CONFIG_MACH_SUN7I=y
> > > > CONFIG_DRAM_CLK=480
> > > > CONFIG_MMC0_CD_PIN="PH1"
> > > > CONFIG_SATAPWR="PB8"
> > > > CONFIG_AHCI=y
> > > > # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
> > > > CONFIG_SPL_I2C=y
> > > > CONFIG_SCSI_AHCI=y
> > > > CONFIG_SYS_I2C_MVTWSI=y
> > > > CONFIG_SYS_I2C_SLAVE=0x7f
> > > > CONFIG_SYS_I2C_SPEED=400000
> > > > CONFIG_PHY_REALTEK=y
> > > > CONFIG_ETH_DESIGNWARE=y
> > > > CONFIG_MII=y
> > > > CONFIG_SUN7I_GMAC=y
> > > > CONFIG_SCSI=y
> > > > CONFIG_USB_EHCI_HCD=y
> > > > CONFIG_USB_OHCI_HCD=y
> > > > CONFIG_LED_STATUS=y
> > > > CONFIG_LED_STATUS_GPIO=y
> > > > CONFIG_LED_STATUS0=y
> > > > CONFIG_LED_STATUS_BIT=114  
> > >
> > > This is the GPIO number, you need to adjust this to point to the pin your
> > > LED is connected to. PH20 would be 244, for instance:
> > > ('H' - 'A') * 32 + 20
> > >
> > > Cheers,
> > > Andre
> > >  
> > Thanks,
> > Now the LED is ON, but the loader still freezes in (SPL, I think)
> > ```
> > U-Boot SPL 2022.01-rc3-00025-gf570594bc9-dirty (Dec 09 2021 - 06:19:40 +0330)
> > ```
> > Best Regards,
> > Javad  
> Hi Andre,
> The freeze problem solved when I placed `status_led_init` function
> after
> gd->ram_size = sunxi_dram_init();
> When it is called before sunxi_dram_init(), the bootloader freezes.

That's probably our old friend .bss in SPL again: To save precious SRAM
space, we place the BSS section in DRAM, so it's only available after the
DRAM init happened. So far the code that runs before DRAM init is not using
variables placed in BSS, so everything works. Possible solutions are:
1) see if we can get rid of BSS variables in the LED code (my bet is on
"static int status_led_init_done = 0;" as the culprit)
2) move the LED init after the DRAM init. That's probably a bit late,
though, depends a bit on what people expect from the boot LED.
3) I guess the A20 can afford BSS in SRAM, but that would not help with
other (64-bit) boards. I actually wonder how it works there, though?

I'd prefer 1), but 2) is surely easier.

Cheers,
Andre

> 
> Best Regards,
> Javad
> > > > CONFIG_LED_STATUS_STATE=2
> > > > ```
> > > > Best Regards,
> > > > Javad  
> > > > > > > > >
> > > > > > > > > Signed-off-by: Javad Rahimi <javad321javad@gmail.com>
> > > > > > > > > ---
> > > > > > > > > This is my first contributation in open source world.
> > > > > > > > > I'm sorry if I have mistakes in my commits and versioning.
> > > > > > > > > I do my best to learn fast.
> > > > > > > > >
> > > > > > > > > Changes in v2:
> > > > > > > > > - Missed braces added
> > > > > > > > > - Unnecessary debug removed
> > > > > > > > > - Some typo fixed
> > > > > > > > >
> > > > > > > > >  board/sunxi/board.c | 49 +++++++++++++++++++++++++++++++++++++++++++++
> > > > > > > > >  1 file changed, 49 insertions(+)
> > > > > > > > >
> > > > > > > > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> > > > > > > > > index 4f5747c34a..5e2f6ae902 100644
> > > > > > > > > --- a/board/sunxi/board.c
> > > > > > > > > +++ b/board/sunxi/board.c
> > > > > > > > > @@ -1002,3 +1002,52 @@ int board_fit_config_name_match(const char *name)
> > > > > > > > >     return ret;
> > > > > > > > >  }
> > > > > > > > >  #endif
> > > > > > > > > +
> > > > > > > > > +#if defined(CONFIG_LED_STATUS) && defined(CONFIG_LED_STATUS_BOOT) && defined(CONFIG_LED_STATUS_BOARD_SPECIFIC)
> > > > > > > > > +
> > > > > > > > > +#define CUBIE2_LED_BOOT_GPIO  "PH20"
> > > > > > > > > +static int gpio_boot_led;
> > > > > > > > > +
> > > > > > > > > +void __led_init(led_id_t mask, int state)
> > > > > > > > > +{
> > > > > > > > > +   int ret;
> > > > > > > > > +
> > > > > > > > > +   if (mask != CONFIG_LED_STATUS_BOOT)
> > > > > > > > > +           return;
> > > > > > > > > +
> > > > > > > > > +   ret = gpio_lookup_name(CUBIE2_LED_BOOT_GPIO, NULL, NULL, &gpio_boot_led);
> > > > > > > > > +
> > > > > > > > > +   if (ret)
> > > > > > > > > +           return;
> > > > > > > > > +
> > > > > > > > > +   ret = gpio_request(gpio_boot_led, "boot_led");
> > > > > > > > > +   if (ret == -1) {
> > > > > > > > > +           debug("[gpio_request] Error:%d\n", ret);
> > > > > > > > > +           return;
> > > > > > > > > +   }
> > > > > > > > > +
> > > > > > > > > +   ret = gpio_direction_output(gpio_boot_led, 1);
> > > > > > > > > +   if (ret == -1) {
> > > > > > > > > +           debug("[gpio_direction_output] Error:%d\n", ret);
> > > > > > > > > +           return;
> > > > > > > > > +   }
> > > > > > > > > +   __led_set(mask, state);
> > > > > > > > > +}
> > > > > > > > > +
> > > > > > > > > +void __led_set(led_id_t mask, int state)
> > > > > > > > > +{
> > > > > > > > > +   if (mask != CONFIG_LED_STATUS_BOOT)
> > > > > > > > > +           return;
> > > > > > > > > +
> > > > > > > > > +   gpio_set_value(gpio_boot_led, state);
> > > > > > > > > +}
> > > > > > > > > +
> > > > > > > > > +void __led_toggle(led_id_t mask)
> > > > > > > > > +{
> > > > > > > > > +   if (mask != CONFIG_LED_STATUS_BOOT)
> > > > > > > > > +           return;
> > > > > > > > > +
> > > > > > > > > +   gpio_set_value(gpio_boot_led, !gpio_get_value(gpio_boot_led));
> > > > > > > > > +}
> > > > > > > > > +
> > > > > > > > > +#endif
> > > > > > > > > --
> > > > > > > > > 2.25.1
> > > > > > > > >
> > > > > > > > >  
> > > > > > >  
> > > > >  
> > >  


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] Cubieboard2:SUN7I:Add LED BOOT support
  2021-12-09 11:24                   ` Andre Przywara
@ 2021-12-09 12:21                     ` Javad Rahimi
  2021-12-09 14:33                       ` Andre Przywara
  0 siblings, 1 reply; 11+ messages in thread
From: Javad Rahimi @ 2021-12-09 12:21 UTC (permalink / raw)
  To: Andre Przywara; +Cc: u-boot, Jagan Teki, hdegoede

On Thu, Dec 9, 2021 at 2:54 PM Andre Przywara <andre.przywara@arm.com> wrote:
Hi Andre,
>
> On Thu, 9 Dec 2021 14:38:08 +0330
> Javad Rahimi <javad321javad@gmail.com> wrote:
>
> Hi Javad,
>
> > On Thu, Dec 9, 2021 at 6:24 AM Javad Rahimi <javad321javad@gmail.com> wrote:
> > >
> > > On Wed, Dec 8, 2021 at 9:13 PM Andre Przywara <andre.przywara@arm.com> wrote:
> > > >
> > > > On Wed, 8 Dec 2021 20:48:54 +0330
> > > > Javad Rahimi <javad321javad@gmail.com> wrote:
> > > >
> > > > > On Wed, Dec 8, 2021 at 8:33 PM Andre Przywara <andre.przywara@arm.com> wrote:
> > > > > >
> > > > > > On Wed, 8 Dec 2021 19:14:22 +0330
> > > > > > Javad Rahimi <javad321javad@gmail.com> wrote:
> > > > > >
> > > > > > > On Wed, Dec 8, 2021 at 6:05 PM Andre Przywara <andre.przywara@arm.com> wrote:
> > > > > > > >
> > > > > > > > On Wed, 8 Dec 2021 15:25:54 +0100
> > > > > > > > Frank Wunderlich <frank-w@public-files.de> wrote:
> > > > > > > >
> > > > > > > > Hi,
> > > > > > > >
> > > > > > > > > you should add maintainer email for your patch
> > > > > > > > >
> > > > > > > > > $ scripts/get_maintainer.pl board/sunxi/board.c
> > > > > > > > > Jagan Teki <jagan@amarulasolutions.com> (maintainer:ARM SUNXI)
> > > > > > > > > Andre Przywara <andre.przywara@arm.com> (maintainer:ARM SUNXI)
> > > > > > > > > u-boot@lists.denx.de (open list)
> > > > > > > >
> > > > > > > > Thanks Frank!
> > > > > > > >
> > > > > > > > > > Gesendet: Mittwoch, 08. Dezember 2021 um 15:22 Uhr
> > > > > > > > > > Von: "Javad Rahimi" <javad321javad@gmail.com>
> > > > > > > > > > An: u-boot@lists.denx.de
> > > > > > > > > > Cc: "Javad Rahimi" <javad321javad@gmail.com>
> > > > > > > > > > Betreff: [PATCH v2] Cubieboard2:SUN7I:Add LED BOOT support
> > > > > > > > > >
> > > > > > > > > > This feature makes it possible to assign one of
> > > > > > > > > > LED1(PH20) and LED2(PH21) to BOOT process LED.
> > > > > > > > > > User should activates the "Enable status LED API" in
> > > > > > > > > > "Device Drivers -> LED Support"
> > > > > > > >
> > > > > > > > Please have a look at the current pinephone_defconfig, because that uses
> > > > > > > > the boot LED already in a much easier fashion:
> > > > > > > > https://source.denx.de/u-boot/u-boot/-/commit/0534153fd1
> > > > > > > >
> > > > > > > > Cheers,
> > > > > > > > Andre
> > > > > > > >
> > > > > > > Hi Andre,
> > > > > > > Thanks for your comments. I studied the pinephone_defconfig.
> > > > > > > By default, when activating the same options on Cubieboard2_defconfig
> > > > > > > it shows linker error for `__led_init` and `__led_set`.
> > > > > > > In other words, they are not defined. So, in this patch, I added the
> > > > > > >  implementation for these functions for this board.
> > > > > >
> > > > > > Did you add the:
> > > > > > CONFIG_SPL_DRIVERS_MISC=y
> > > > > > line as well? And re-ran make Cubieboard2_defconfig?
> > > > > > Because that seemed to work for me.
> > > > > >
> > > > > > Cheers,
> > > > > > Andre
> > > > > >
> > > > > >
> > > > > When I add CONFIG_SPL_DRIVERS_MISC=y
> > > > > By flashing SD card and turning on the board, It freezes in this step:
> > > > > ```
> > > > > U-Boot SPL 2022.01-rc3-00025-gf570594bc9-dirty (Dec 08 2021 - 20:36:51 +0330)
> > > > > ```
> > > > > -----------------------------------------
> > > > > The customized defconfig file for Cubieboard:
> > > > > ```
> > > > > CONFIG_ARM=y
> > > > > CONFIG_ARCH_SUNXI=y
> > > > > CONFIG_DEFAULT_DEVICE_TREE="sun7i-a20-cubieboard2"
> > > > > CONFIG_SPL_DRIVERS_MISC=y
> > > > > CONFIG_SPL=y
> > > > > CONFIG_MACH_SUN7I=y
> > > > > CONFIG_DRAM_CLK=480
> > > > > CONFIG_MMC0_CD_PIN="PH1"
> > > > > CONFIG_SATAPWR="PB8"
> > > > > CONFIG_AHCI=y
> > > > > # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
> > > > > CONFIG_SPL_I2C=y
> > > > > CONFIG_SCSI_AHCI=y
> > > > > CONFIG_SYS_I2C_MVTWSI=y
> > > > > CONFIG_SYS_I2C_SLAVE=0x7f
> > > > > CONFIG_SYS_I2C_SPEED=400000
> > > > > CONFIG_PHY_REALTEK=y
> > > > > CONFIG_ETH_DESIGNWARE=y
> > > > > CONFIG_MII=y
> > > > > CONFIG_SUN7I_GMAC=y
> > > > > CONFIG_SCSI=y
> > > > > CONFIG_USB_EHCI_HCD=y
> > > > > CONFIG_USB_OHCI_HCD=y
> > > > > CONFIG_LED_STATUS=y
> > > > > CONFIG_LED_STATUS_GPIO=y
> > > > > CONFIG_LED_STATUS0=y
> > > > > CONFIG_LED_STATUS_BIT=114
> > > >
> > > > This is the GPIO number, you need to adjust this to point to the pin your
> > > > LED is connected to. PH20 would be 244, for instance:
> > > > ('H' - 'A') * 32 + 20
> > > >
> > > > Cheers,
> > > > Andre
> > > >
> > > Thanks,
> > > Now the LED is ON, but the loader still freezes in (SPL, I think)
> > > ```
> > > U-Boot SPL 2022.01-rc3-00025-gf570594bc9-dirty (Dec 09 2021 - 06:19:40 +0330)
> > > ```
> > > Best Regards,
> > > Javad
> > Hi Andre,
> > The freeze problem solved when I placed `status_led_init` function
> > after
> > gd->ram_size = sunxi_dram_init();
> > When it is called before sunxi_dram_init(), the bootloader freezes.
>
> That's probably our old friend .bss in SPL again: To save precious SRAM
> space, we place the BSS section in DRAM, so it's only available after the
> DRAM init happened. So far the code that runs before DRAM init is not using
> variables placed in BSS, so everything works. Possible solutions are:
> 1) see if we can get rid of BSS variables in the LED code (my bet is on
> "static int status_led_init_done = 0;" as the culprit)
> 2) move the LED init after the DRAM init. That's probably a bit late,
> though, depends a bit on what people expect from the boot LED.
> 3) I guess the A20 can afford BSS in SRAM, but that would not help with
> other (64-bit) boards. I actually wonder how it works there, though?
>
> I'd prefer 1), but 2) is surely easier.
>
> Cheers,
> Andre
>
Your guess was true. The `status_led_init_done` was the culprit.
I removed this variable and added a one-bit variable named "initialized"
to the `led_dev_t ` . In this case, when `__led_init` is called, we can
set it for every single LED. In this case, we can check `ld->initilized`
for each led rather than blindly checking the `status_led_init_done`
global variable.

Best Regards,
Javad
> >
> > Best Regards,
> > Javad
> > > > > CONFIG_LED_STATUS_STATE=2
> > > > > ```
> > > > > Best Regards,
> > > > > Javad
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Javad Rahimi <javad321javad@gmail.com>
> > > > > > > > > > ---
> > > > > > > > > > This is my first contributation in open source world.
> > > > > > > > > > I'm sorry if I have mistakes in my commits and versioning.
> > > > > > > > > > I do my best to learn fast.
> > > > > > > > > >
> > > > > > > > > > Changes in v2:
> > > > > > > > > > - Missed braces added
> > > > > > > > > > - Unnecessary debug removed
> > > > > > > > > > - Some typo fixed
> > > > > > > > > >
> > > > > > > > > >  board/sunxi/board.c | 49 +++++++++++++++++++++++++++++++++++++++++++++
> > > > > > > > > >  1 file changed, 49 insertions(+)
> > > > > > > > > >
> > > > > > > > > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> > > > > > > > > > index 4f5747c34a..5e2f6ae902 100644
> > > > > > > > > > --- a/board/sunxi/board.c
> > > > > > > > > > +++ b/board/sunxi/board.c
> > > > > > > > > > @@ -1002,3 +1002,52 @@ int board_fit_config_name_match(const char *name)
> > > > > > > > > >     return ret;
> > > > > > > > > >  }
> > > > > > > > > >  #endif
> > > > > > > > > > +
> > > > > > > > > > +#if defined(CONFIG_LED_STATUS) && defined(CONFIG_LED_STATUS_BOOT) && defined(CONFIG_LED_STATUS_BOARD_SPECIFIC)
> > > > > > > > > > +
> > > > > > > > > > +#define CUBIE2_LED_BOOT_GPIO  "PH20"
> > > > > > > > > > +static int gpio_boot_led;
> > > > > > > > > > +
> > > > > > > > > > +void __led_init(led_id_t mask, int state)
> > > > > > > > > > +{
> > > > > > > > > > +   int ret;
> > > > > > > > > > +
> > > > > > > > > > +   if (mask != CONFIG_LED_STATUS_BOOT)
> > > > > > > > > > +           return;
> > > > > > > > > > +
> > > > > > > > > > +   ret = gpio_lookup_name(CUBIE2_LED_BOOT_GPIO, NULL, NULL, &gpio_boot_led);
> > > > > > > > > > +
> > > > > > > > > > +   if (ret)
> > > > > > > > > > +           return;
> > > > > > > > > > +
> > > > > > > > > > +   ret = gpio_request(gpio_boot_led, "boot_led");
> > > > > > > > > > +   if (ret == -1) {
> > > > > > > > > > +           debug("[gpio_request] Error:%d\n", ret);
> > > > > > > > > > +           return;
> > > > > > > > > > +   }
> > > > > > > > > > +
> > > > > > > > > > +   ret = gpio_direction_output(gpio_boot_led, 1);
> > > > > > > > > > +   if (ret == -1) {
> > > > > > > > > > +           debug("[gpio_direction_output] Error:%d\n", ret);
> > > > > > > > > > +           return;
> > > > > > > > > > +   }
> > > > > > > > > > +   __led_set(mask, state);
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +void __led_set(led_id_t mask, int state)
> > > > > > > > > > +{
> > > > > > > > > > +   if (mask != CONFIG_LED_STATUS_BOOT)
> > > > > > > > > > +           return;
> > > > > > > > > > +
> > > > > > > > > > +   gpio_set_value(gpio_boot_led, state);
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +void __led_toggle(led_id_t mask)
> > > > > > > > > > +{
> > > > > > > > > > +   if (mask != CONFIG_LED_STATUS_BOOT)
> > > > > > > > > > +           return;
> > > > > > > > > > +
> > > > > > > > > > +   gpio_set_value(gpio_boot_led, !gpio_get_value(gpio_boot_led));
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +#endif
> > > > > > > > > > --
> > > > > > > > > > 2.25.1
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > >
> > > > > >
> > > >
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] Cubieboard2:SUN7I:Add LED BOOT support
  2021-12-09 12:21                     ` Javad Rahimi
@ 2021-12-09 14:33                       ` Andre Przywara
  0 siblings, 0 replies; 11+ messages in thread
From: Andre Przywara @ 2021-12-09 14:33 UTC (permalink / raw)
  To: Javad Rahimi; +Cc: u-boot, Jagan Teki, hdegoede

On Thu, 9 Dec 2021 15:51:50 +0330
Javad Rahimi <javad321javad@gmail.com> wrote:

Hi,

> On Thu, Dec 9, 2021 at 2:54 PM Andre Przywara <andre.przywara@arm.com> wrote:
> Hi Andre,
> >
> > On Thu, 9 Dec 2021 14:38:08 +0330
> > Javad Rahimi <javad321javad@gmail.com> wrote:
> >
> > Hi Javad,
> >  
> > > On Thu, Dec 9, 2021 at 6:24 AM Javad Rahimi <javad321javad@gmail.com> wrote:  
> > > >
> > > > On Wed, Dec 8, 2021 at 9:13 PM Andre Przywara <andre.przywara@arm.com> wrote:  
> > > > >
> > > > > On Wed, 8 Dec 2021 20:48:54 +0330
> > > > > Javad Rahimi <javad321javad@gmail.com> wrote:
> > > > >  
> > > > > > On Wed, Dec 8, 2021 at 8:33 PM Andre Przywara <andre.przywara@arm.com> wrote:  
> > > > > > >
> > > > > > > On Wed, 8 Dec 2021 19:14:22 +0330
> > > > > > > Javad Rahimi <javad321javad@gmail.com> wrote:
> > > > > > >  
> > > > > > > > On Wed, Dec 8, 2021 at 6:05 PM Andre Przywara <andre.przywara@arm.com> wrote:  
> > > > > > > > >
> > > > > > > > > On Wed, 8 Dec 2021 15:25:54 +0100
> > > > > > > > > Frank Wunderlich <frank-w@public-files.de> wrote:
> > > > > > > > >
> > > > > > > > > Hi,
> > > > > > > > >  
> > > > > > > > > > you should add maintainer email for your patch
> > > > > > > > > >
> > > > > > > > > > $ scripts/get_maintainer.pl board/sunxi/board.c
> > > > > > > > > > Jagan Teki <jagan@amarulasolutions.com> (maintainer:ARM SUNXI)
> > > > > > > > > > Andre Przywara <andre.przywara@arm.com> (maintainer:ARM SUNXI)
> > > > > > > > > > u-boot@lists.denx.de (open list)  
> > > > > > > > >
> > > > > > > > > Thanks Frank!
> > > > > > > > >  
> > > > > > > > > > > Gesendet: Mittwoch, 08. Dezember 2021 um 15:22 Uhr
> > > > > > > > > > > Von: "Javad Rahimi" <javad321javad@gmail.com>
> > > > > > > > > > > An: u-boot@lists.denx.de
> > > > > > > > > > > Cc: "Javad Rahimi" <javad321javad@gmail.com>
> > > > > > > > > > > Betreff: [PATCH v2] Cubieboard2:SUN7I:Add LED BOOT support
> > > > > > > > > > >
> > > > > > > > > > > This feature makes it possible to assign one of
> > > > > > > > > > > LED1(PH20) and LED2(PH21) to BOOT process LED.
> > > > > > > > > > > User should activates the "Enable status LED API" in
> > > > > > > > > > > "Device Drivers -> LED Support"  
> > > > > > > > >
> > > > > > > > > Please have a look at the current pinephone_defconfig, because that uses
> > > > > > > > > the boot LED already in a much easier fashion:
> > > > > > > > > https://source.denx.de/u-boot/u-boot/-/commit/0534153fd1
> > > > > > > > >
> > > > > > > > > Cheers,
> > > > > > > > > Andre
> > > > > > > > >  
> > > > > > > > Hi Andre,
> > > > > > > > Thanks for your comments. I studied the pinephone_defconfig.
> > > > > > > > By default, when activating the same options on Cubieboard2_defconfig
> > > > > > > > it shows linker error for `__led_init` and `__led_set`.
> > > > > > > > In other words, they are not defined. So, in this patch, I added the
> > > > > > > >  implementation for these functions for this board.  
> > > > > > >
> > > > > > > Did you add the:
> > > > > > > CONFIG_SPL_DRIVERS_MISC=y
> > > > > > > line as well? And re-ran make Cubieboard2_defconfig?
> > > > > > > Because that seemed to work for me.
> > > > > > >
> > > > > > > Cheers,
> > > > > > > Andre
> > > > > > >
> > > > > > >  
> > > > > > When I add CONFIG_SPL_DRIVERS_MISC=y
> > > > > > By flashing SD card and turning on the board, It freezes in this step:
> > > > > > ```
> > > > > > U-Boot SPL 2022.01-rc3-00025-gf570594bc9-dirty (Dec 08 2021 - 20:36:51 +0330)
> > > > > > ```
> > > > > > -----------------------------------------
> > > > > > The customized defconfig file for Cubieboard:
> > > > > > ```
> > > > > > CONFIG_ARM=y
> > > > > > CONFIG_ARCH_SUNXI=y
> > > > > > CONFIG_DEFAULT_DEVICE_TREE="sun7i-a20-cubieboard2"
> > > > > > CONFIG_SPL_DRIVERS_MISC=y
> > > > > > CONFIG_SPL=y
> > > > > > CONFIG_MACH_SUN7I=y
> > > > > > CONFIG_DRAM_CLK=480
> > > > > > CONFIG_MMC0_CD_PIN="PH1"
> > > > > > CONFIG_SATAPWR="PB8"
> > > > > > CONFIG_AHCI=y
> > > > > > # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
> > > > > > CONFIG_SPL_I2C=y
> > > > > > CONFIG_SCSI_AHCI=y
> > > > > > CONFIG_SYS_I2C_MVTWSI=y
> > > > > > CONFIG_SYS_I2C_SLAVE=0x7f
> > > > > > CONFIG_SYS_I2C_SPEED=400000
> > > > > > CONFIG_PHY_REALTEK=y
> > > > > > CONFIG_ETH_DESIGNWARE=y
> > > > > > CONFIG_MII=y
> > > > > > CONFIG_SUN7I_GMAC=y
> > > > > > CONFIG_SCSI=y
> > > > > > CONFIG_USB_EHCI_HCD=y
> > > > > > CONFIG_USB_OHCI_HCD=y
> > > > > > CONFIG_LED_STATUS=y
> > > > > > CONFIG_LED_STATUS_GPIO=y
> > > > > > CONFIG_LED_STATUS0=y
> > > > > > CONFIG_LED_STATUS_BIT=114  
> > > > >
> > > > > This is the GPIO number, you need to adjust this to point to the pin your
> > > > > LED is connected to. PH20 would be 244, for instance:
> > > > > ('H' - 'A') * 32 + 20
> > > > >
> > > > > Cheers,
> > > > > Andre
> > > > >  
> > > > Thanks,
> > > > Now the LED is ON, but the loader still freezes in (SPL, I think)
> > > > ```
> > > > U-Boot SPL 2022.01-rc3-00025-gf570594bc9-dirty (Dec 09 2021 - 06:19:40 +0330)
> > > > ```
> > > > Best Regards,
> > > > Javad  
> > > Hi Andre,
> > > The freeze problem solved when I placed `status_led_init` function
> > > after
> > > gd->ram_size = sunxi_dram_init();
> > > When it is called before sunxi_dram_init(), the bootloader freezes.  
> >
> > That's probably our old friend .bss in SPL again: To save precious SRAM
> > space, we place the BSS section in DRAM, so it's only available after the
> > DRAM init happened. So far the code that runs before DRAM init is not using
> > variables placed in BSS, so everything works. Possible solutions are:
> > 1) see if we can get rid of BSS variables in the LED code (my bet is on
> > "static int status_led_init_done = 0;" as the culprit)
> > 2) move the LED init after the DRAM init. That's probably a bit late,
> > though, depends a bit on what people expect from the boot LED.
> > 3) I guess the A20 can afford BSS in SRAM, but that would not help with
> > other (64-bit) boards. I actually wonder how it works there, though?
> >
> > I'd prefer 1), but 2) is surely easier.
> >
> > Cheers,
> > Andre
> >  
> Your guess was true. The `status_led_init_done` was the culprit.
> I removed this variable and added a one-bit variable named "initialized"
> to the `led_dev_t ` . In this case, when `__led_init` is called, we can
> set it for every single LED. In this case, we can check `ld->initilized`
> for each led rather than blindly checking the `status_led_init_done`
> global variable.

Interesting idea, though I don't know too much about this code, and why we
had the global variable in the first place.
Can you prepare a patch and send it to the mailing list, so that people
can have a look and discuss this?

Cheers,
Andre

> Best Regards,
> Javad
> > >
> > > Best Regards,
> > > Javad  
> > > > > > CONFIG_LED_STATUS_STATE=2
> > > > > > ```
> > > > > > Best Regards,
> > > > > > Javad  
> > > > > > > > > > >
> > > > > > > > > > > Signed-off-by: Javad Rahimi <javad321javad@gmail.com>
> > > > > > > > > > > ---
> > > > > > > > > > > This is my first contributation in open source world.
> > > > > > > > > > > I'm sorry if I have mistakes in my commits and versioning.
> > > > > > > > > > > I do my best to learn fast.
> > > > > > > > > > >
> > > > > > > > > > > Changes in v2:
> > > > > > > > > > > - Missed braces added
> > > > > > > > > > > - Unnecessary debug removed
> > > > > > > > > > > - Some typo fixed
> > > > > > > > > > >
> > > > > > > > > > >  board/sunxi/board.c | 49 +++++++++++++++++++++++++++++++++++++++++++++
> > > > > > > > > > >  1 file changed, 49 insertions(+)
> > > > > > > > > > >
> > > > > > > > > > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> > > > > > > > > > > index 4f5747c34a..5e2f6ae902 100644
> > > > > > > > > > > --- a/board/sunxi/board.c
> > > > > > > > > > > +++ b/board/sunxi/board.c
> > > > > > > > > > > @@ -1002,3 +1002,52 @@ int board_fit_config_name_match(const char *name)
> > > > > > > > > > >     return ret;
> > > > > > > > > > >  }
> > > > > > > > > > >  #endif
> > > > > > > > > > > +
> > > > > > > > > > > +#if defined(CONFIG_LED_STATUS) && defined(CONFIG_LED_STATUS_BOOT) && defined(CONFIG_LED_STATUS_BOARD_SPECIFIC)
> > > > > > > > > > > +
> > > > > > > > > > > +#define CUBIE2_LED_BOOT_GPIO  "PH20"
> > > > > > > > > > > +static int gpio_boot_led;
> > > > > > > > > > > +
> > > > > > > > > > > +void __led_init(led_id_t mask, int state)
> > > > > > > > > > > +{
> > > > > > > > > > > +   int ret;
> > > > > > > > > > > +
> > > > > > > > > > > +   if (mask != CONFIG_LED_STATUS_BOOT)
> > > > > > > > > > > +           return;
> > > > > > > > > > > +
> > > > > > > > > > > +   ret = gpio_lookup_name(CUBIE2_LED_BOOT_GPIO, NULL, NULL, &gpio_boot_led);
> > > > > > > > > > > +
> > > > > > > > > > > +   if (ret)
> > > > > > > > > > > +           return;
> > > > > > > > > > > +
> > > > > > > > > > > +   ret = gpio_request(gpio_boot_led, "boot_led");
> > > > > > > > > > > +   if (ret == -1) {
> > > > > > > > > > > +           debug("[gpio_request] Error:%d\n", ret);
> > > > > > > > > > > +           return;
> > > > > > > > > > > +   }
> > > > > > > > > > > +
> > > > > > > > > > > +   ret = gpio_direction_output(gpio_boot_led, 1);
> > > > > > > > > > > +   if (ret == -1) {
> > > > > > > > > > > +           debug("[gpio_direction_output] Error:%d\n", ret);
> > > > > > > > > > > +           return;
> > > > > > > > > > > +   }
> > > > > > > > > > > +   __led_set(mask, state);
> > > > > > > > > > > +}
> > > > > > > > > > > +
> > > > > > > > > > > +void __led_set(led_id_t mask, int state)
> > > > > > > > > > > +{
> > > > > > > > > > > +   if (mask != CONFIG_LED_STATUS_BOOT)
> > > > > > > > > > > +           return;
> > > > > > > > > > > +
> > > > > > > > > > > +   gpio_set_value(gpio_boot_led, state);
> > > > > > > > > > > +}
> > > > > > > > > > > +
> > > > > > > > > > > +void __led_toggle(led_id_t mask)
> > > > > > > > > > > +{
> > > > > > > > > > > +   if (mask != CONFIG_LED_STATUS_BOOT)
> > > > > > > > > > > +           return;
> > > > > > > > > > > +
> > > > > > > > > > > +   gpio_set_value(gpio_boot_led, !gpio_get_value(gpio_boot_led));
> > > > > > > > > > > +}
> > > > > > > > > > > +
> > > > > > > > > > > +#endif
> > > > > > > > > > > --
> > > > > > > > > > > 2.25.1
> > > > > > > > > > >
> > > > > > > > > > >  
> > > > > > > > >  
> > > > > > >  
> > > > >  
> >  


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-12-09 14:33 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <ada@thorsis.com>
2021-12-08 14:22 ` [PATCH v2] Cubieboard2:SUN7I:Add LED BOOT support Javad Rahimi
2021-12-08 14:25   ` Aw: " Frank Wunderlich
2021-12-08 14:34     ` Andre Przywara
2021-12-08 15:49       ` Javad Rahimi
     [not found]       ` <CAL245av-S3yyrxwZ8A3W2wdkYWC3bwOvPWh1BoaU25JEZ8uOvQ@mail.gmail.com>
     [not found]         ` <20211208170328.7e7c4e29@donnerap.cambridge.arm.com>
2021-12-08 17:18           ` Javad Rahimi
2021-12-08 17:43             ` Andre Przywara
2021-12-09  2:54               ` Javad Rahimi
2021-12-09 11:08                 ` Javad Rahimi
2021-12-09 11:24                   ` Andre Przywara
2021-12-09 12:21                     ` Javad Rahimi
2021-12-09 14:33                       ` Andre Przywara

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.