diff mbox series

[v2,2/3] sunxi: add "fake" FEL pin support

Message ID 20250417000539.3709-3-andre.przywara@arm.com
State New
Delegated to: Andre Przywara
Headers show
Series sunxi: add "fake" FEL button feature | expand

Commit Message

Andre Przywara April 17, 2025, 12:05 a.m. UTC
Some boards with Allwinner SoCs feature a "FEL" key, sometimes also
labelled "uboot", which triggers the BootROM FEL mode, when pressed upon
power-on or reset. This allows to access the SoC's memory via USB OTG,
and to upload and execute code. There is a tool to upload our U-Boot image
and immediately boot it, when the SoC is in FEL mode.

To mimic this convenient behaviour on boards without such a dedicated key,
we can query a GPIO pin very early in the SPL boot, then trigger the
BootROM FEL routine.  There has not been much of a SoC or board setup at
this point, so we enter the BROM in a rather pristine state still. On
64-bit SoCs the required AArch32 reset guarantees a clean CPU state anyway.

Any GPIO can be used for that, the signal is expected to be active low,
consequently we enable the pull-up resistors for that pin. A board (or a
user) is expected to specify the GPIO name using the
CONFIG_SUNXI_FAKE_FEL_PIN Kconfig variable. When this variable is not set,
the compiler will optimise away the call.

Call the code first thing in board_init_f(), which is the first sunxi
specific C routine.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/mach-sunxi/Kconfig | 10 ++++++++++
 arch/arm/mach-sunxi/board.c | 31 +++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

Comments

Yixun Lan April 17, 2025, 3:39 a.m. UTC | #1
Hi Andre,

On 01:05 Thu 17 Apr     , Andre Przywara wrote:
> Some boards with Allwinner SoCs feature a "FEL" key, sometimes also
> labelled "uboot", which triggers the BootROM FEL mode, when pressed upon
> power-on or reset. This allows to access the SoC's memory via USB OTG,
> and to upload and execute code. There is a tool to upload our U-Boot image
> and immediately boot it, when the SoC is in FEL mode.
> 
> To mimic this convenient behaviour on boards without such a dedicated key,
> we can query a GPIO pin very early in the SPL boot, then trigger the
> BootROM FEL routine.  There has not been much of a SoC or board setup at
> this point, so we enter the BROM in a rather pristine state still. On
> 64-bit SoCs the required AArch32 reset guarantees a clean CPU state anyway.
> 
> Any GPIO can be used for that, the signal is expected to be active low,
> consequently we enable the pull-up resistors for that pin. A board (or a
> user) is expected to specify the GPIO name using the
> CONFIG_SUNXI_FAKE_FEL_PIN Kconfig variable. When this variable is not set,
> the compiler will optimise away the call.
> 
> Call the code first thing in board_init_f(), which is the first sunxi
> specific C routine.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/arm/mach-sunxi/Kconfig | 10 ++++++++++
>  arch/arm/mach-sunxi/board.c | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+)
> 
> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> index ab432390d3c..f1cfdb548bc 100644
> --- a/arch/arm/mach-sunxi/Kconfig
> +++ b/arch/arm/mach-sunxi/Kconfig
> @@ -825,6 +825,16 @@ config USB3_VBUS_PIN
>  	---help---
>  	See USB1_VBUS_PIN help text.
>  
> +config SUNXI_FAKE_FEL_PIN
> +	string "fake FEL GPIO pin"
> +	default ""
> +	---help---
> +	Define a GPIO that shall force entering FEL mode when a button
> +	connected to this pin is pressed at boot time. This must be an
> +	active low signal, the internal pull-up resistors are activated.
> +	This takes a string in the format understood by sunxi_name_to_gpio,
> +	e.g. PH1 for pin 1 of port H.
> +
>  config I2C0_ENABLE
>  	bool "Enable I2C/TWI controller 0"
>  	default y if MACH_SUN4I || MACH_SUN5I || MACH_SUN7I || MACH_SUN8I_R40
> diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
> index 701899ee4b2..4ee0b333176 100644
> --- a/arch/arm/mach-sunxi/board.c
> +++ b/arch/arm/mach-sunxi/board.c
> @@ -457,8 +457,39 @@ u32 spl_mmc_boot_mode(struct mmc *mmc, const u32 boot_device)
>  	return result;
>  }
>  
> +static void check_fake_fel_button(void)
> +{
> +	u32 brom_entry = 0x20;
> +	int pin, value, mux;
> +
> +	/* check for empty string at compile time */
> +	if (sizeof(CONFIG_SUNXI_FAKE_FEL_PIN) == sizeof(""))
use 'sizeof("")' to express it's an empty string _explicitly_? 
otherwise, I'd prefer '0' simply (save few chars)..

> +		return;
> +
> +	pin = sunxi_name_to_gpio(CONFIG_SUNXI_FAKE_FEL_PIN);
> +	if (pin < 0)
> +		return;
> +
> +	mux = sunxi_gpio_get_cfgpin(pin);
> +	sunxi_gpio_set_cfgpin(pin, SUNXI_GPIO_INPUT);
...
> +	sunxi_gpio_set_pull(pin, SUNXI_GPIO_PULL_UP);
it seems this doesn't work for me.

while I'm testing on A527, using pin PD6 (GPIO4_D6 in schematic)
it goes directly to FEL mode, not sure if I setup things wrong..
CONFIG_SUNXI_FAKE_FEL_PIN="PD6"

> +	value = gpio_get_value(pin);
> +	sunxi_gpio_set_cfgpin(pin, mux);
> +
> +	if (value)
> +		return;
> +
> +	/* Older SoCs maps the BootROM high in the address space. */
> +	if (fel_stash.sctlr & BIT(13))
> +		brom_entry |= 0xffff0000;
> +
> +	return_to_fel(0, brom_entry);
> +}
> +
>  void board_init_f(ulong dummy)
>  {
> +	check_fake_fel_button();
> +
this isn't a problem, I can understand calling it here will make board
enter FEL mode as early as possible..

just wondering if better to move this function after preloader_console_init(),
and log out a message to let user know explicitly - entering FEL mode from SPL..
(I personally find it helpful)

but, if you prefer not to change, then fine by me..

>  	sunxi_sram_init();
>  
>  	/* Enable non-secure access to some peripherals */
> -- 
> 2.46.3
>
Yixun Lan April 17, 2025, 4:08 a.m. UTC | #2
Hi Andre,

On 03:39 Thu 17 Apr     , Yixun Lan wrote:
> Hi Andre,
> 
> On 01:05 Thu 17 Apr     , Andre Przywara wrote:
> > Some boards with Allwinner SoCs feature a "FEL" key, sometimes also
> > labelled "uboot", which triggers the BootROM FEL mode, when pressed upon
> > power-on or reset. This allows to access the SoC's memory via USB OTG,
> > and to upload and execute code. There is a tool to upload our U-Boot image
> > and immediately boot it, when the SoC is in FEL mode.
> > 
> > To mimic this convenient behaviour on boards without such a dedicated key,
> > we can query a GPIO pin very early in the SPL boot, then trigger the
> > BootROM FEL routine.  There has not been much of a SoC or board setup at
> > this point, so we enter the BROM in a rather pristine state still. On
> > 64-bit SoCs the required AArch32 reset guarantees a clean CPU state anyway.
> > 
> > Any GPIO can be used for that, the signal is expected to be active low,
> > consequently we enable the pull-up resistors for that pin. A board (or a
> > user) is expected to specify the GPIO name using the
> > CONFIG_SUNXI_FAKE_FEL_PIN Kconfig variable. When this variable is not set,
> > the compiler will optimise away the call.
> > 
> > Call the code first thing in board_init_f(), which is the first sunxi
> > specific C routine.
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  arch/arm/mach-sunxi/Kconfig | 10 ++++++++++
> >  arch/arm/mach-sunxi/board.c | 31 +++++++++++++++++++++++++++++++
> >  2 files changed, 41 insertions(+)
> > 
> > diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> > index ab432390d3c..f1cfdb548bc 100644
> > --- a/arch/arm/mach-sunxi/Kconfig
> > +++ b/arch/arm/mach-sunxi/Kconfig
> > @@ -825,6 +825,16 @@ config USB3_VBUS_PIN
> >  	---help---
> >  	See USB1_VBUS_PIN help text.
> >  
> > +config SUNXI_FAKE_FEL_PIN
> > +	string "fake FEL GPIO pin"
> > +	default ""
> > +	---help---
> > +	Define a GPIO that shall force entering FEL mode when a button
> > +	connected to this pin is pressed at boot time. This must be an
> > +	active low signal, the internal pull-up resistors are activated.
> > +	This takes a string in the format understood by sunxi_name_to_gpio,
> > +	e.g. PH1 for pin 1 of port H.
> > +
> >  config I2C0_ENABLE
> >  	bool "Enable I2C/TWI controller 0"
> >  	default y if MACH_SUN4I || MACH_SUN5I || MACH_SUN7I || MACH_SUN8I_R40
> > diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
> > index 701899ee4b2..4ee0b333176 100644
> > --- a/arch/arm/mach-sunxi/board.c
> > +++ b/arch/arm/mach-sunxi/board.c
> > @@ -457,8 +457,39 @@ u32 spl_mmc_boot_mode(struct mmc *mmc, const u32 boot_device)
> >  	return result;
> >  }
> >  
> > +static void check_fake_fel_button(void)
> > +{
> > +	u32 brom_entry = 0x20;
> > +	int pin, value, mux;
> > +
> > +	/* check for empty string at compile time */
> > +	if (sizeof(CONFIG_SUNXI_FAKE_FEL_PIN) == sizeof(""))
> use 'sizeof("")' to express it's an empty string _explicitly_? 
> otherwise, I'd prefer '0' simply (save few chars)..
> 
> > +		return;
> > +
> > +	pin = sunxi_name_to_gpio(CONFIG_SUNXI_FAKE_FEL_PIN);
> > +	if (pin < 0)
> > +		return;
> > +
> > +	mux = sunxi_gpio_get_cfgpin(pin);
> > +	sunxi_gpio_set_cfgpin(pin, SUNXI_GPIO_INPUT);
> ...
> > +	sunxi_gpio_set_pull(pin, SUNXI_GPIO_PULL_UP);
> it seems this doesn't work for me.
> 
> while I'm testing on A527, using pin PD6 (GPIO4_D6 in schematic)
> it goes directly to FEL mode, not sure if I setup things wrong..
> CONFIG_SUNXI_FAKE_FEL_PIN="PD6"
>
I did wrong pin mapping (not familiar with..), so switch to "PI6" (GPIO4_A5),
it works then, thanks

> > +	value = gpio_get_value(pin);
> > +	sunxi_gpio_set_cfgpin(pin, mux);
> > +
> > +	if (value)
> > +		return;
> > +
> > +	/* Older SoCs maps the BootROM high in the address space. */
> > +	if (fel_stash.sctlr & BIT(13))
> > +		brom_entry |= 0xffff0000;
> > +
> > +	return_to_fel(0, brom_entry);
> > +}
> > +
> >  void board_init_f(ulong dummy)
> >  {
> > +	check_fake_fel_button();
> > +
> this isn't a problem, I can understand calling it here will make board
> enter FEL mode as early as possible..
> 
> just wondering if better to move this function after preloader_console_init(),
> and log out a message to let user know explicitly - entering FEL mode from SPL..
> (I personally find it helpful)
> 
> but, if you prefer not to change, then fine by me..
> 
> >  	sunxi_sram_init();
> >  
> >  	/* Enable non-secure access to some peripherals */
> > -- 
> > 2.46.3
> > 
> 
> -- 
> Yixun Lan (dlan)
Quentin Schulz April 18, 2025, 11:28 a.m. UTC | #3
Hi Andre,

On 4/17/25 2:05 AM, Andre Przywara wrote:
> Some boards with Allwinner SoCs feature a "FEL" key, sometimes also
> labelled "uboot", which triggers the BootROM FEL mode, when pressed upon
> power-on or reset. This allows to access the SoC's memory via USB OTG,
> and to upload and execute code. There is a tool to upload our U-Boot image
> and immediately boot it, when the SoC is in FEL mode.
> 
> To mimic this convenient behaviour on boards without such a dedicated key,
> we can query a GPIO pin very early in the SPL boot, then trigger the
> BootROM FEL routine.  There has not been much of a SoC or board setup at
> this point, so we enter the BROM in a rather pristine state still. On
> 64-bit SoCs the required AArch32 reset guarantees a clean CPU state anyway.
> 
> Any GPIO can be used for that, the signal is expected to be active low,
> consequently we enable the pull-up resistors for that pin. A board (or a
> user) is expected to specify the GPIO name using the
> CONFIG_SUNXI_FAKE_FEL_PIN Kconfig variable. When this variable is not set,
> the compiler will optimise away the call.
> 
> Call the code first thing in board_init_f(), which is the first sunxi
> specific C routine.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>   arch/arm/mach-sunxi/Kconfig | 10 ++++++++++
>   arch/arm/mach-sunxi/board.c | 31 +++++++++++++++++++++++++++++++
>   2 files changed, 41 insertions(+)
> 
> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> index ab432390d3c..f1cfdb548bc 100644
> --- a/arch/arm/mach-sunxi/Kconfig
> +++ b/arch/arm/mach-sunxi/Kconfig
> @@ -825,6 +825,16 @@ config USB3_VBUS_PIN
>   	---help---
>   	See USB1_VBUS_PIN help text.
>   
> +config SUNXI_FAKE_FEL_PIN
> +	string "fake FEL GPIO pin"
> +	default ""
> +	---help---
> +	Define a GPIO that shall force entering FEL mode when a button
> +	connected to this pin is pressed at boot time. This must be an
> +	active low signal, the internal pull-up resistors are activated.
> +	This takes a string in the format understood by sunxi_name_to_gpio,
> +	e.g. PH1 for pin 1 of port H.
> +

Why not use the DT for that? Then you wouldn't even need to assume the 
polarity of the signal or whether pull-up/downs need to be activated, etc.

You can have the property in the -u-boot.dtsi then if you want?

While the FEL button on the X96 is "fake", it does what it says, just in 
software, maybe that is close enough to "hardware definition" which 
would make it suitable for the DT (well, we also store binman nodes in 
the DT, which aren't strictly speaking hardware definition either :) ).

Cheers,
Quentin
Yixun Lan April 18, 2025, 10:51 p.m. UTC | #4
Hi

On 13:28 Fri 18 Apr     , Quentin Schulz wrote:
> Hi Andre,
> 
> On 4/17/25 2:05 AM, Andre Przywara wrote:
> > Some boards with Allwinner SoCs feature a "FEL" key, sometimes also
> > labelled "uboot", which triggers the BootROM FEL mode, when pressed upon
> > power-on or reset. This allows to access the SoC's memory via USB OTG,
> > and to upload and execute code. There is a tool to upload our U-Boot image
> > and immediately boot it, when the SoC is in FEL mode.
> > 
> > To mimic this convenient behaviour on boards without such a dedicated key,
> > we can query a GPIO pin very early in the SPL boot, then trigger the
> > BootROM FEL routine.  There has not been much of a SoC or board setup at
> > this point, so we enter the BROM in a rather pristine state still. On
> > 64-bit SoCs the required AArch32 reset guarantees a clean CPU state anyway.
> > 
> > Any GPIO can be used for that, the signal is expected to be active low,
> > consequently we enable the pull-up resistors for that pin. A board (or a
> > user) is expected to specify the GPIO name using the
> > CONFIG_SUNXI_FAKE_FEL_PIN Kconfig variable. When this variable is not set,
> > the compiler will optimise away the call.
> > 
> > Call the code first thing in board_init_f(), which is the first sunxi
> > specific C routine.
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >   arch/arm/mach-sunxi/Kconfig | 10 ++++++++++
> >   arch/arm/mach-sunxi/board.c | 31 +++++++++++++++++++++++++++++++
> >   2 files changed, 41 insertions(+)
> > 
> > diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> > index ab432390d3c..f1cfdb548bc 100644
> > --- a/arch/arm/mach-sunxi/Kconfig
> > +++ b/arch/arm/mach-sunxi/Kconfig
> > @@ -825,6 +825,16 @@ config USB3_VBUS_PIN
> >   	---help---
> >   	See USB1_VBUS_PIN help text.
> >   
> > +config SUNXI_FAKE_FEL_PIN
> > +	string "fake FEL GPIO pin"
> > +	default ""
> > +	---help---
> > +	Define a GPIO that shall force entering FEL mode when a button
> > +	connected to this pin is pressed at boot time. This must be an
> > +	active low signal, the internal pull-up resistors are activated.
> > +	This takes a string in the format understood by sunxi_name_to_gpio,
> > +	e.g. PH1 for pin 1 of port H.
> > +
> 
> Why not use the DT for that? Then you wouldn't even need to assume the 
> polarity of the signal or whether pull-up/downs need to be activated, etc.
> 
> You can have the property in the -u-boot.dtsi then if you want?
> 
I've raised similar question in v1, but it's in SPL which has no DT available
see Andre's commment
https://lore.kernel.org/all/20250409110126.2cc59d30@donnerap.manchester.arm.com/

maybe better add a SPL prefix to config? or some info in help docs to
state clearly it should only work in SPL phase?

> While the FEL button on the X96 is "fake", it does what it says, just in 
> software, maybe that is close enough to "hardware definition" which 
> would make it suitable for the DT (well, we also store binman nodes in 
> the DT, which aren't strictly speaking hardware definition either :) ).
> 
> Cheers,
> Quentin
Andre Przywara April 21, 2025, 9:29 p.m. UTC | #5
On Fri, 18 Apr 2025 13:28:23 +0200
Quentin Schulz <quentin.schulz@cherry.de> wrote:

Hi Quentin,

thanks for having a look!

> Hi Andre,
> 
> On 4/17/25 2:05 AM, Andre Przywara wrote:
> > Some boards with Allwinner SoCs feature a "FEL" key, sometimes also
> > labelled "uboot", which triggers the BootROM FEL mode, when pressed upon
> > power-on or reset. This allows to access the SoC's memory via USB OTG,
> > and to upload and execute code. There is a tool to upload our U-Boot image
> > and immediately boot it, when the SoC is in FEL mode.
> > 
> > To mimic this convenient behaviour on boards without such a dedicated key,
> > we can query a GPIO pin very early in the SPL boot, then trigger the
> > BootROM FEL routine.  There has not been much of a SoC or board setup at
> > this point, so we enter the BROM in a rather pristine state still. On
> > 64-bit SoCs the required AArch32 reset guarantees a clean CPU state anyway.
> > 
> > Any GPIO can be used for that, the signal is expected to be active low,
> > consequently we enable the pull-up resistors for that pin. A board (or a
> > user) is expected to specify the GPIO name using the
> > CONFIG_SUNXI_FAKE_FEL_PIN Kconfig variable. When this variable is not set,
> > the compiler will optimise away the call.
> > 
> > Call the code first thing in board_init_f(), which is the first sunxi
> > specific C routine.
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >   arch/arm/mach-sunxi/Kconfig | 10 ++++++++++
> >   arch/arm/mach-sunxi/board.c | 31 +++++++++++++++++++++++++++++++
> >   2 files changed, 41 insertions(+)
> > 
> > diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> > index ab432390d3c..f1cfdb548bc 100644
> > --- a/arch/arm/mach-sunxi/Kconfig
> > +++ b/arch/arm/mach-sunxi/Kconfig
> > @@ -825,6 +825,16 @@ config USB3_VBUS_PIN
> >   	---help---
> >   	See USB1_VBUS_PIN help text.
> >   
> > +config SUNXI_FAKE_FEL_PIN
> > +	string "fake FEL GPIO pin"
> > +	default ""
> > +	---help---
> > +	Define a GPIO that shall force entering FEL mode when a button
> > +	connected to this pin is pressed at boot time. This must be an
> > +	active low signal, the internal pull-up resistors are activated.
> > +	This takes a string in the format understood by sunxi_name_to_gpio,
> > +	e.g. PH1 for pin 1 of port H.
> > +  
> 
> Why not use the DT for that? Then you wouldn't even need to assume the 
> polarity of the signal or whether pull-up/downs need to be activated, etc.

As Yixun Lan already pointed out, the DT is not available at this
point, and doing several pull-ups to get this information from the DT
into the SPL image are really over the top for this purpose.
This is more a sweet hacker device: I often have devices with eMMC and
SPI flash, but without a FEL button. So the idea was to just pick a
GPIO and use menuconfig to set it. Then I could just connect this pin
to GND during boot, to get into FEL and test-boot firmware. "Connect to
GND" could really be a jumper or even the tip of a screwdriver ;-)
So I don't think this qualifies to being defined in the DT, really.

> You can have the property in the -u-boot.dtsi then if you want?
> 
> While the FEL button on the X96 is "fake", it does what it says, just in 
> software, maybe that is close enough to "hardware definition" which 
> would make it suitable for the DT (well, we also store binman nodes in 

Yes, I have a patch to add this particular button as a GPIO button into
the DT, so people can use it for whatever they want in Linux (trigger
reboot, update, you name it). But this is rather orthogonal to this
problem, as mentioned above.

> the DT, which aren't strictly speaking hardware definition either :) ).

Please don't get me started on this, we don't need to make it worse ;-)

Cheers,
Andre
Andre Przywara April 21, 2025, 9:42 p.m. UTC | #6
On Thu, 17 Apr 2025 03:39:35 +0000
Yixun Lan <dlan@gentoo.org> wrote:

Hi,

> Hi Andre,
> 
> On 01:05 Thu 17 Apr     , Andre Przywara wrote:
> > Some boards with Allwinner SoCs feature a "FEL" key, sometimes also
> > labelled "uboot", which triggers the BootROM FEL mode, when pressed upon
> > power-on or reset. This allows to access the SoC's memory via USB OTG,
> > and to upload and execute code. There is a tool to upload our U-Boot image
> > and immediately boot it, when the SoC is in FEL mode.
> > 
> > To mimic this convenient behaviour on boards without such a dedicated key,
> > we can query a GPIO pin very early in the SPL boot, then trigger the
> > BootROM FEL routine.  There has not been much of a SoC or board setup at
> > this point, so we enter the BROM in a rather pristine state still. On
> > 64-bit SoCs the required AArch32 reset guarantees a clean CPU state anyway.
> > 
> > Any GPIO can be used for that, the signal is expected to be active low,
> > consequently we enable the pull-up resistors for that pin. A board (or a
> > user) is expected to specify the GPIO name using the
> > CONFIG_SUNXI_FAKE_FEL_PIN Kconfig variable. When this variable is not set,
> > the compiler will optimise away the call.
> > 
> > Call the code first thing in board_init_f(), which is the first sunxi
> > specific C routine.
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  arch/arm/mach-sunxi/Kconfig | 10 ++++++++++
> >  arch/arm/mach-sunxi/board.c | 31 +++++++++++++++++++++++++++++++
> >  2 files changed, 41 insertions(+)
> > 
> > diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> > index ab432390d3c..f1cfdb548bc 100644
> > --- a/arch/arm/mach-sunxi/Kconfig
> > +++ b/arch/arm/mach-sunxi/Kconfig
> > @@ -825,6 +825,16 @@ config USB3_VBUS_PIN
> >  	---help---
> >  	See USB1_VBUS_PIN help text.
> >  
> > +config SUNXI_FAKE_FEL_PIN
> > +	string "fake FEL GPIO pin"
> > +	default ""
> > +	---help---
> > +	Define a GPIO that shall force entering FEL mode when a button
> > +	connected to this pin is pressed at boot time. This must be an
> > +	active low signal, the internal pull-up resistors are activated.
> > +	This takes a string in the format understood by sunxi_name_to_gpio,
> > +	e.g. PH1 for pin 1 of port H.
> > +
> >  config I2C0_ENABLE
> >  	bool "Enable I2C/TWI controller 0"
> >  	default y if MACH_SUN4I || MACH_SUN5I || MACH_SUN7I || MACH_SUN8I_R40
> > diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
> > index 701899ee4b2..4ee0b333176 100644
> > --- a/arch/arm/mach-sunxi/board.c
> > +++ b/arch/arm/mach-sunxi/board.c
> > @@ -457,8 +457,39 @@ u32 spl_mmc_boot_mode(struct mmc *mmc, const u32 boot_device)
> >  	return result;
> >  }
> >  
> > +static void check_fake_fel_button(void)
> > +{
> > +	u32 brom_entry = 0x20;
> > +	int pin, value, mux;
> > +
> > +	/* check for empty string at compile time */
> > +	if (sizeof(CONFIG_SUNXI_FAKE_FEL_PIN) == sizeof(""))  
> use 'sizeof("")' to express it's an empty string _explicitly_? 

Yes, that was the idea.

> otherwise, I'd prefer '0' simply (save few chars)..

sizeof("") is always 0, but 0 on the other hand can mean many things. So
I used this to be specific and make it clear that I mean the empty
string here.

> > +		return;
> > +
> > +	pin = sunxi_name_to_gpio(CONFIG_SUNXI_FAKE_FEL_PIN);
> > +	if (pin < 0)
> > +		return;
> > +
> > +	mux = sunxi_gpio_get_cfgpin(pin);
> > +	sunxi_gpio_set_cfgpin(pin, SUNXI_GPIO_INPUT);  
> ...
> > +	sunxi_gpio_set_pull(pin, SUNXI_GPIO_PULL_UP);  
> it seems this doesn't work for me.
> 
> while I'm testing on A527, using pin PD6 (GPIO4_D6 in schematic)
> it goes directly to FEL mode, not sure if I setup things wrong..
> CONFIG_SUNXI_FAKE_FEL_PIN="PD6"

As you already figured yourself, GPIO4_D6 is one of the few pins you
cannot use, since it's not a GPIO, but going to GPADC2. Due to their
analogue nature, those pins are not multiplexed and are hardwired to
the GPADC device. Utilising them would be theoretically possible as
well, but much more complicated, and being easy and simple was a
requirement for this patch, as for instance many H6 and A64 SPL builds
are already quite tight when it comes to the code size.
 
> 
> > +	value = gpio_get_value(pin);
> > +	sunxi_gpio_set_cfgpin(pin, mux);
> > +
> > +	if (value)
> > +		return;
> > +
> > +	/* Older SoCs maps the BootROM high in the address space. */
> > +	if (fel_stash.sctlr & BIT(13))
> > +		brom_entry |= 0xffff0000;
> > +
> > +	return_to_fel(0, brom_entry);
> > +}
> > +
> >  void board_init_f(ulong dummy)
> >  {
> > +	check_fake_fel_button();
> > +  
> this isn't a problem, I can understand calling it here will make board
> enter FEL mode as early as possible..
> 
> just wondering if better to move this function after preloader_console_init(),
> and log out a message to let user know explicitly - entering FEL mode from SPL..
> (I personally find it helpful)

I understand where you are coming from, and would see this confirmation
as useful as well, but FEL mode is really supposed to be entered as
early as possible (ideally straight from the BootROM), and in a rather
clean SoC state. As the BootROM does not know about or touch the UART,
this would be different from a real FEL boot.
I often use FEL to dump some SoC state at reset, or do experiments in an
"unspoiled" environment, so doing as little as possible was a real goal
of this patch.

Thanks anyway for looking at the patch and voicing your concerns and
opinions!

Cheers,
Andre
 
> but, if you prefer not to change, then fine by me..
> 
> >  	sunxi_sram_init();
> >  
> >  	/* Enable non-secure access to some peripherals */
> > -- 
> > 2.46.3
> >   
>
Quentin Schulz April 22, 2025, 10:49 a.m. UTC | #7
Hi Andre and Yixun Lan,

On 4/21/25 11:29 PM, Andre Przywara wrote:
> On Fri, 18 Apr 2025 13:28:23 +0200
> Quentin Schulz <quentin.schulz@cherry.de> wrote:
> 
> Hi Quentin,
> 
> thanks for having a look!
> 
>> Hi Andre,
>>
>> On 4/17/25 2:05 AM, Andre Przywara wrote:
>>> Some boards with Allwinner SoCs feature a "FEL" key, sometimes also
>>> labelled "uboot", which triggers the BootROM FEL mode, when pressed upon
>>> power-on or reset. This allows to access the SoC's memory via USB OTG,
>>> and to upload and execute code. There is a tool to upload our U-Boot image
>>> and immediately boot it, when the SoC is in FEL mode.
>>>
>>> To mimic this convenient behaviour on boards without such a dedicated key,
>>> we can query a GPIO pin very early in the SPL boot, then trigger the
>>> BootROM FEL routine.  There has not been much of a SoC or board setup at
>>> this point, so we enter the BROM in a rather pristine state still. On
>>> 64-bit SoCs the required AArch32 reset guarantees a clean CPU state anyway.
>>>
>>> Any GPIO can be used for that, the signal is expected to be active low,
>>> consequently we enable the pull-up resistors for that pin. A board (or a
>>> user) is expected to specify the GPIO name using the
>>> CONFIG_SUNXI_FAKE_FEL_PIN Kconfig variable. When this variable is not set,
>>> the compiler will optimise away the call.
>>>
>>> Call the code first thing in board_init_f(), which is the first sunxi
>>> specific C routine.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>> ---
>>>    arch/arm/mach-sunxi/Kconfig | 10 ++++++++++
>>>    arch/arm/mach-sunxi/board.c | 31 +++++++++++++++++++++++++++++++
>>>    2 files changed, 41 insertions(+)
>>>
>>> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
>>> index ab432390d3c..f1cfdb548bc 100644
>>> --- a/arch/arm/mach-sunxi/Kconfig
>>> +++ b/arch/arm/mach-sunxi/Kconfig
>>> @@ -825,6 +825,16 @@ config USB3_VBUS_PIN
>>>    	---help---
>>>    	See USB1_VBUS_PIN help text.
>>>    
>>> +config SUNXI_FAKE_FEL_PIN
>>> +	string "fake FEL GPIO pin"
>>> +	default ""
>>> +	---help---
>>> +	Define a GPIO that shall force entering FEL mode when a button
>>> +	connected to this pin is pressed at boot time. This must be an
>>> +	active low signal, the internal pull-up resistors are activated.
>>> +	This takes a string in the format understood by sunxi_name_to_gpio,
>>> +	e.g. PH1 for pin 1 of port H.
>>> +
>>
>> Why not use the DT for that? Then you wouldn't even need to assume the
>> polarity of the signal or whether pull-up/downs need to be activated, etc.
> 
> As Yixun Lan already pointed out, the DT is not available at this
> point, and doing several pull-ups to get this information from the DT
> into the SPL image are really over the top for this purpose.

OK, we do have something "similar" for Rockchip boards, via the 
sysreset-gpio DT property in /config, see 
arch/arm/mach-rockchip/rk3399/rk3399.c and 
arch/arm/dts/rk3399-puma-haikou-u-boot.dtsi.

I guess something similar could be implemented IFF there's an actual DT 
in SPL (or even TPL). We for sure have DT in SPL for most if not all 
Rockchip devices, and probably in TPL as well. Hence why I sometimes 
forget other Arm boards may not have DT in those stages :)

> This is more a sweet hacker device: I often have devices with eMMC and
> SPI flash, but without a FEL button. So the idea was to just pick a
> GPIO and use menuconfig to set it. Then I could just connect this pin
> to GND during boot, to get into FEL and test-boot firmware. "Connect to
> GND" could really be a jumper or even the tip of a screwdriver ;-)
> So I don't think this qualifies to being defined in the DT, really.
> 

If I understood correctly, this GPIO is essentially not fixed, you just 
pick a random one that you like and have access to and it may differ 
depending on the user I guess?

Then yeah, I understand.

Can you please specify this in the commit log and also why we are not 
going the DT route (if I read correctly, in addition to it not being 
suitable for DT due to the ability to the function being available on 
any unused GPIO, there's also no DT available in the stage when this 
needs to be done?).

>> You can have the property in the -u-boot.dtsi then if you want?
>>
>> While the FEL button on the X96 is "fake", it does what it says, just in
>> software, maybe that is close enough to "hardware definition" which
>> would make it suitable for the DT (well, we also store binman nodes in
> 
> Yes, I have a patch to add this particular button as a GPIO button into
> the DT, so people can use it for whatever they want in Linux (trigger
> reboot, update, you name it). But this is rather orthogonal to this
> problem, as mentioned above.
> 

Mmmmm but this will be in the Linux kernel DT and I assume you want the 
same GPIO to be used in U-Boot and in Linux, so it would probably be 
best to make sure they stay in sync? How are you planning to do that?

Cheers,
Quentin
Andre Przywara April 22, 2025, 12:11 p.m. UTC | #8
On Tue, 22 Apr 2025 12:49:40 +0200
Quentin Schulz <quentin.schulz@cherry.de> wrote:

Hi,

> Hi Andre and Yixun Lan,
> 
> On 4/21/25 11:29 PM, Andre Przywara wrote:
> > On Fri, 18 Apr 2025 13:28:23 +0200
> > Quentin Schulz <quentin.schulz@cherry.de> wrote:
> > 
> > Hi Quentin,
> > 
> > thanks for having a look!
> >   
> >> Hi Andre,
> >>
> >> On 4/17/25 2:05 AM, Andre Przywara wrote:  
> >>> Some boards with Allwinner SoCs feature a "FEL" key, sometimes also
> >>> labelled "uboot", which triggers the BootROM FEL mode, when pressed upon
> >>> power-on or reset. This allows to access the SoC's memory via USB OTG,
> >>> and to upload and execute code. There is a tool to upload our U-Boot image
> >>> and immediately boot it, when the SoC is in FEL mode.
> >>>
> >>> To mimic this convenient behaviour on boards without such a dedicated key,
> >>> we can query a GPIO pin very early in the SPL boot, then trigger the
> >>> BootROM FEL routine.  There has not been much of a SoC or board setup at
> >>> this point, so we enter the BROM in a rather pristine state still. On
> >>> 64-bit SoCs the required AArch32 reset guarantees a clean CPU state anyway.
> >>>
> >>> Any GPIO can be used for that, the signal is expected to be active low,
> >>> consequently we enable the pull-up resistors for that pin. A board (or a
> >>> user) is expected to specify the GPIO name using the
> >>> CONFIG_SUNXI_FAKE_FEL_PIN Kconfig variable. When this variable is not set,
> >>> the compiler will optimise away the call.
> >>>
> >>> Call the code first thing in board_init_f(), which is the first sunxi
> >>> specific C routine.
> >>>
> >>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> >>> ---
> >>>    arch/arm/mach-sunxi/Kconfig | 10 ++++++++++
> >>>    arch/arm/mach-sunxi/board.c | 31 +++++++++++++++++++++++++++++++
> >>>    2 files changed, 41 insertions(+)
> >>>
> >>> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> >>> index ab432390d3c..f1cfdb548bc 100644
> >>> --- a/arch/arm/mach-sunxi/Kconfig
> >>> +++ b/arch/arm/mach-sunxi/Kconfig
> >>> @@ -825,6 +825,16 @@ config USB3_VBUS_PIN
> >>>    	---help---
> >>>    	See USB1_VBUS_PIN help text.
> >>>    
> >>> +config SUNXI_FAKE_FEL_PIN
> >>> +	string "fake FEL GPIO pin"
> >>> +	default ""
> >>> +	---help---
> >>> +	Define a GPIO that shall force entering FEL mode when a button
> >>> +	connected to this pin is pressed at boot time. This must be an
> >>> +	active low signal, the internal pull-up resistors are activated.
> >>> +	This takes a string in the format understood by sunxi_name_to_gpio,
> >>> +	e.g. PH1 for pin 1 of port H.
> >>> +  
> >>
> >> Why not use the DT for that? Then you wouldn't even need to assume the
> >> polarity of the signal or whether pull-up/downs need to be activated, etc.  
> > 
> > As Yixun Lan already pointed out, the DT is not available at this
> > point, and doing several pull-ups to get this information from the DT
> > into the SPL image are really over the top for this purpose.  
> 
> OK, we do have something "similar" for Rockchip boards, via the 
> sysreset-gpio DT property in /config, see 
> arch/arm/mach-rockchip/rk3399/rk3399.c and 
> arch/arm/dts/rk3399-puma-haikou-u-boot.dtsi.
> 
> I guess something similar could be implemented IFF there's an actual DT 
> in SPL (or even TPL). We for sure have DT in SPL for most if not all 
> Rockchip devices, and probably in TPL as well. Hence why I sometimes 
> forget other Arm boards may not have DT in those stages :)

Yeah, I know, there is some push for DT anywhere, but it would be quite
some pain for sunxi, for very little benefit. We are very tight in the SPL
code size for some SoCs, up to the point where for instance enabling this
code here made the build break on some SoCs (I found something to free
some code space elsewhere, might send that later). So pulling libfdt and
DM code in would require going TPL, I guess. But I don't see the reason,
really, as I tend to see the SPL more as the continuation of the BootROM,
which manages without board specific knowledge at all. And for the small
bits of info we need, we can happily use Kconfig. Most of it is actually
SoC specific, not board specific, so users don't get bothered normally,
and it just works (TM).
There is tons of work and cleanup to do on the sunxi side, and were
already have quite some backlog, so I want to avoid introducing more
construction sites.

> > This is more a sweet hacker device: I often have devices with eMMC and
> > SPI flash, but without a FEL button. So the idea was to just pick a
> > GPIO and use menuconfig to set it. Then I could just connect this pin
> > to GND during boot, to get into FEL and test-boot firmware. "Connect to
> > GND" could really be a jumper or even the tip of a screwdriver ;-)
> > So I don't think this qualifies to being defined in the DT, really.
> >   
> 
> If I understood correctly, this GPIO is essentially not fixed, you just 
> pick a random one that you like and have access to and it may differ 
> depending on the user I guess?

Depends, for some boards which have this button (X96 Mate) it's more
obvious, but yeah, I wanted to keep this hackable, so DT wouldn't be a
good place anyway.

> Then yeah, I understand.
> 
> Can you please specify this in the commit log and also why we are not 
> going the DT route (if I read correctly, in addition to it not being 
> suitable for DT due to the ability to the function being available on 
> any unused GPIO, there's also no DT available in the stage when this 
> needs to be done?).

Sure, will do.

> >> You can have the property in the -u-boot.dtsi then if you want?
> >>
> >> While the FEL button on the X96 is "fake", it does what it says, just in
> >> software, maybe that is close enough to "hardware definition" which
> >> would make it suitable for the DT (well, we also store binman nodes in  
> > 
> > Yes, I have a patch to add this particular button as a GPIO button into
> > the DT, so people can use it for whatever they want in Linux (trigger
> > reboot, update, you name it). But this is rather orthogonal to this
> > problem, as mentioned above.
> >   
> 
> Mmmmm but this will be in the Linux kernel DT and I assume you want the 
> same GPIO to be used in U-Boot and in Linux, so it would probably be 
> best to make sure they stay in sync? How are you planning to do that?

On the Allwinner side we were syncing the DTs regularly for years already,
and had no conflicting or different bindings at all. So I prefer to think
of "the DT", with Linux or U-Boot just carrying slightly different
versions for some time.

Some SoCs (and all upcoming ones) use OF_UPSTREAM now, but I cannot roll
this out to everyone, because there was one compatibility break in the DT
in the past, and I want to keep support for older kernels with the U-Boot
copy of the DT, so that the Debian Bullseye kernel on an installer USB
drive would still work, for instance.

Cheers,
Andre
Quentin Schulz April 22, 2025, 2:30 p.m. UTC | #9
Hi Andre,

On 4/22/25 2:11 PM, Andre Przywara wrote:
> On Tue, 22 Apr 2025 12:49:40 +0200
> Quentin Schulz <quentin.schulz@cherry.de> wrote:
> 
> Hi,
> 
>> Hi Andre and Yixun Lan,
>>
>> On 4/21/25 11:29 PM, Andre Przywara wrote:
>>> On Fri, 18 Apr 2025 13:28:23 +0200
>>> Quentin Schulz <quentin.schulz@cherry.de> wrote:
>>>
>>> Hi Quentin,
>>>
>>> thanks for having a look!
>>>    
>>>> Hi Andre,
>>>>
>>>> On 4/17/25 2:05 AM, Andre Przywara wrote:
>>>>> Some boards with Allwinner SoCs feature a "FEL" key, sometimes also
>>>>> labelled "uboot", which triggers the BootROM FEL mode, when pressed upon
>>>>> power-on or reset. This allows to access the SoC's memory via USB OTG,
>>>>> and to upload and execute code. There is a tool to upload our U-Boot image
>>>>> and immediately boot it, when the SoC is in FEL mode.
>>>>>
>>>>> To mimic this convenient behaviour on boards without such a dedicated key,
>>>>> we can query a GPIO pin very early in the SPL boot, then trigger the
>>>>> BootROM FEL routine.  There has not been much of a SoC or board setup at
>>>>> this point, so we enter the BROM in a rather pristine state still. On
>>>>> 64-bit SoCs the required AArch32 reset guarantees a clean CPU state anyway.
>>>>>
>>>>> Any GPIO can be used for that, the signal is expected to be active low,
>>>>> consequently we enable the pull-up resistors for that pin. A board (or a
>>>>> user) is expected to specify the GPIO name using the
>>>>> CONFIG_SUNXI_FAKE_FEL_PIN Kconfig variable. When this variable is not set,
>>>>> the compiler will optimise away the call.
>>>>>
>>>>> Call the code first thing in board_init_f(), which is the first sunxi
>>>>> specific C routine.
>>>>>
>>>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>>>> ---
>>>>>     arch/arm/mach-sunxi/Kconfig | 10 ++++++++++
>>>>>     arch/arm/mach-sunxi/board.c | 31 +++++++++++++++++++++++++++++++
>>>>>     2 files changed, 41 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
>>>>> index ab432390d3c..f1cfdb548bc 100644
>>>>> --- a/arch/arm/mach-sunxi/Kconfig
>>>>> +++ b/arch/arm/mach-sunxi/Kconfig
>>>>> @@ -825,6 +825,16 @@ config USB3_VBUS_PIN
>>>>>     	---help---
>>>>>     	See USB1_VBUS_PIN help text.
>>>>>     
>>>>> +config SUNXI_FAKE_FEL_PIN
>>>>> +	string "fake FEL GPIO pin"
>>>>> +	default ""
>>>>> +	---help---
>>>>> +	Define a GPIO that shall force entering FEL mode when a button
>>>>> +	connected to this pin is pressed at boot time. This must be an
>>>>> +	active low signal, the internal pull-up resistors are activated.
>>>>> +	This takes a string in the format understood by sunxi_name_to_gpio,
>>>>> +	e.g. PH1 for pin 1 of port H.
>>>>> +
>>>>
>>>> Why not use the DT for that? Then you wouldn't even need to assume the
>>>> polarity of the signal or whether pull-up/downs need to be activated, etc.
>>>
>>> As Yixun Lan already pointed out, the DT is not available at this
>>> point, and doing several pull-ups to get this information from the DT
>>> into the SPL image are really over the top for this purpose.
>>
>> OK, we do have something "similar" for Rockchip boards, via the
>> sysreset-gpio DT property in /config, see
>> arch/arm/mach-rockchip/rk3399/rk3399.c and
>> arch/arm/dts/rk3399-puma-haikou-u-boot.dtsi.
>>
>> I guess something similar could be implemented IFF there's an actual DT
>> in SPL (or even TPL). We for sure have DT in SPL for most if not all
>> Rockchip devices, and probably in TPL as well. Hence why I sometimes
>> forget other Arm boards may not have DT in those stages :)
> 
> Yeah, I know, there is some push for DT anywhere, but it would be quite
> some pain for sunxi, for very little benefit. We are very tight in the SPL
> code size for some SoCs, up to the point where for instance enabling this
> code here made the build break on some SoCs (I found something to free
> some code space elsewhere, might send that later). So pulling libfdt and
> DM code in would require going TPL, I guess. But I don't see the reason,

We are also very limited on some Rockchip SoCs, see PX30 which has a TPL 
without DM support and where adding a printf (there are already some) is 
sometimes too much to fit into the SRAM, so I understand. We do have 
some other SoCs that have DM enabled in TPL, e.g. RK3399 imply it.

> really, as I tend to see the SPL more as the continuation of the BootROM,
> which manages without board specific knowledge at all. And for the small
> bits of info we need, we can happily use Kconfig. Most of it is actually
> SoC specific, not board specific, so users don't get bothered normally,
> and it just works (TM).

Jonas has started to support "generic" images for Rockchip boards based 
on the recommended hardware design specified by Rockchip themselves. 
Most companies do mostly respect it, so that seems to be working quite 
nicely.

Depending on what exactly you want to support with U-Boot, a DM-less SPL 
may be difficult. e.g. if you want to support a fallback storage medium 
for loading u-boot.itb (or proper, don't know what's being used on 
Allwinner) that differs from the one used to load the SPL by the 
BootROM, then you possibly cannot rely on the BootROM initializing the 
PHYs, controllers, pinmuxes and pinconfs. KConfig may be usable for this 
but that will make things cumbersome to support.

> There is tons of work and cleanup to do on the sunxi side, and were
> already have quite some backlog, so I want to avoid introducing more
> construction sites.
> 

Fair, it also doesn't mean that what's currently added cannot be 
migrated later on :)

[...]

>>>> You can have the property in the -u-boot.dtsi then if you want?
>>>>
>>>> While the FEL button on the X96 is "fake", it does what it says, just in
>>>> software, maybe that is close enough to "hardware definition" which
>>>> would make it suitable for the DT (well, we also store binman nodes in
>>>
>>> Yes, I have a patch to add this particular button as a GPIO button into
>>> the DT, so people can use it for whatever they want in Linux (trigger
>>> reboot, update, you name it). But this is rather orthogonal to this
>>> problem, as mentioned above.
>>>    
>>
>> Mmmmm but this will be in the Linux kernel DT and I assume you want the
>> same GPIO to be used in U-Boot and in Linux, so it would probably be
>> best to make sure they stay in sync? How are you planning to do that?
> 
> On the Allwinner side we were syncing the DTs regularly for years already,
> and had no conflicting or different bindings at all. So I prefer to think
> of "the DT", with Linux or U-Boot just carrying slightly different
> versions for some time.
> 

If the kernel DTB is coming from U-Boot, it should be much less 
difficult to keep this synced. But if it isn't, you would need to patch 
it live before booting the kernel for example.

In any case, I think the conclusion is that DT cannot be used (yet?) for 
that so this thread is now essentially just me being curious :)

Cheers,
Quentin
Andre Przywara May 12, 2025, 12:39 p.m. UTC | #10
On Tue, 22 Apr 2025 16:30:04 +0200
Quentin Schulz <quentin.schulz@cherry.de> wrote:

Hi Quentin,

just found this in my draft folder. It's not really related to this
patch anymore, but you seemed to be interested, and I am happy to
explain some of the specialities for sunxi in U-Boot, since it differs
in many things from other platforms.
So see below...

> Hi Andre,
> 
> On 4/22/25 2:11 PM, Andre Przywara wrote:
> > On Tue, 22 Apr 2025 12:49:40 +0200
> > Quentin Schulz <quentin.schulz@cherry.de> wrote:
> > 
> > Hi,
> >   
> >> Hi Andre and Yixun Lan,
> >>
> >> On 4/21/25 11:29 PM, Andre Przywara wrote:  
> >>> On Fri, 18 Apr 2025 13:28:23 +0200
> >>> Quentin Schulz <quentin.schulz@cherry.de> wrote:
> >>>
> >>> Hi Quentin,
> >>>
> >>> thanks for having a look!
> >>>      
> >>>> Hi Andre,
> >>>>
> >>>> On 4/17/25 2:05 AM, Andre Przywara wrote:  
> >>>>> Some boards with Allwinner SoCs feature a "FEL" key, sometimes also
> >>>>> labelled "uboot", which triggers the BootROM FEL mode, when pressed upon
> >>>>> power-on or reset. This allows to access the SoC's memory via USB OTG,
> >>>>> and to upload and execute code. There is a tool to upload our U-Boot image
> >>>>> and immediately boot it, when the SoC is in FEL mode.
> >>>>>
> >>>>> To mimic this convenient behaviour on boards without such a dedicated key,
> >>>>> we can query a GPIO pin very early in the SPL boot, then trigger the
> >>>>> BootROM FEL routine.  There has not been much of a SoC or board setup at
> >>>>> this point, so we enter the BROM in a rather pristine state still. On
> >>>>> 64-bit SoCs the required AArch32 reset guarantees a clean CPU state anyway.
> >>>>>
> >>>>> Any GPIO can be used for that, the signal is expected to be active low,
> >>>>> consequently we enable the pull-up resistors for that pin. A board (or a
> >>>>> user) is expected to specify the GPIO name using the
> >>>>> CONFIG_SUNXI_FAKE_FEL_PIN Kconfig variable. When this variable is not set,
> >>>>> the compiler will optimise away the call.
> >>>>>
> >>>>> Call the code first thing in board_init_f(), which is the first sunxi
> >>>>> specific C routine.
> >>>>>
> >>>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> >>>>> ---
> >>>>>     arch/arm/mach-sunxi/Kconfig | 10 ++++++++++
> >>>>>     arch/arm/mach-sunxi/board.c | 31 +++++++++++++++++++++++++++++++
> >>>>>     2 files changed, 41 insertions(+)
> >>>>>
> >>>>> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> >>>>> index ab432390d3c..f1cfdb548bc 100644
> >>>>> --- a/arch/arm/mach-sunxi/Kconfig
> >>>>> +++ b/arch/arm/mach-sunxi/Kconfig
> >>>>> @@ -825,6 +825,16 @@ config USB3_VBUS_PIN
> >>>>>     	---help---
> >>>>>     	See USB1_VBUS_PIN help text.
> >>>>>     
> >>>>> +config SUNXI_FAKE_FEL_PIN
> >>>>> +	string "fake FEL GPIO pin"
> >>>>> +	default ""
> >>>>> +	---help---
> >>>>> +	Define a GPIO that shall force entering FEL mode when a button
> >>>>> +	connected to this pin is pressed at boot time. This must be an
> >>>>> +	active low signal, the internal pull-up resistors are activated.
> >>>>> +	This takes a string in the format understood by sunxi_name_to_gpio,
> >>>>> +	e.g. PH1 for pin 1 of port H.
> >>>>> +  
> >>>>
> >>>> Why not use the DT for that? Then you wouldn't even need to assume the
> >>>> polarity of the signal or whether pull-up/downs need to be activated, etc.  
> >>>
> >>> As Yixun Lan already pointed out, the DT is not available at this
> >>> point, and doing several pull-ups to get this information from the DT
> >>> into the SPL image are really over the top for this purpose.  
> >>
> >> OK, we do have something "similar" for Rockchip boards, via the
> >> sysreset-gpio DT property in /config, see
> >> arch/arm/mach-rockchip/rk3399/rk3399.c and
> >> arch/arm/dts/rk3399-puma-haikou-u-boot.dtsi.
> >>
> >> I guess something similar could be implemented IFF there's an actual DT
> >> in SPL (or even TPL). We for sure have DT in SPL for most if not all
> >> Rockchip devices, and probably in TPL as well. Hence why I sometimes
> >> forget other Arm boards may not have DT in those stages :)  
> > 
> > Yeah, I know, there is some push for DT anywhere, but it would be quite
> > some pain for sunxi, for very little benefit. We are very tight in the SPL
> > code size for some SoCs, up to the point where for instance enabling this
> > code here made the build break on some SoCs (I found something to free
> > some code space elsewhere, might send that later). So pulling libfdt and
> > DM code in would require going TPL, I guess. But I don't see the reason,  
> 
> We are also very limited on some Rockchip SoCs, see PX30 which has a TPL 
> without DM support and where adding a printf (there are already some) is 
> sometimes too much to fit into the SRAM, so I understand. We do have 
> some other SoCs that have DM enabled in TPL, e.g. RK3399 imply it.

Yes, the size problem is *one* aspect. The early Allwinner SoCs have
24KB usable SRAM, but it's fine there because their code size is smaller
because of ARMv7 and Thumb2. The A64 has a 32KB limit, and it's really
tight there, because it's 64-bit, so no Thumb and larger pointers.
Later SoCs lifted the 32K limit, though we still try to keep it as small
as possible.

> > really, as I tend to see the SPL more as the continuation of the BootROM,
> > which manages without board specific knowledge at all. And for the small
> > bits of info we need, we can happily use Kconfig. Most of it is actually
> > SoC specific, not board specific, so users don't get bothered normally,
> > and it just works (TM).  
> 
> Jonas has started to support "generic" images for Rockchip boards based 
> on the recommended hardware design specified by Rockchip themselves. 
> Most companies do mostly respect it, so that seems to be working quite 
> nicely.

I wouldn't be aware of a "recommended" board design for Allwinner, but
as a matter of fact many companies copy their reference design -
probably more out of laziness ;-). This brings the variation down to a
manageable level, so we can define default values in Kconfig, so
defconfigs stay small.

> Depending on what exactly you want to support with U-Boot, a DM-less SPL 
> may be difficult. e.g. if you want to support a fallback storage medium 
> for loading u-boot.itb (or proper, don't know what's being used on 
> Allwinner) that differs from the one used to load the SPL by the 
> BootROM, then you possibly cannot rely on the BootROM initializing the 
> PHYs, controllers, pinmuxes and pinconfs.

We do not rely on any of those bits being setup, actually, but we
naturally follow the BootROM in its decision process. The BROM stores
the boot media used in a byte in SRAM, so we know where we have been
loaded from, so can continue loading from there. But this is just a
decision between SD card, eMMC, NOR flash and FEL mode (maskROM in RK
lingo). We know that SD card boot must have been from MMC0 on the PortF
pins, and NOR flash is only SPI0 on PortC, on so on. The mux values and
MMIO base addresses are per SoC, so those two values are easily stored
in a header or in Kconfig, where we put them *once*, when we add
support for a new SoC - and they are also quite stable across
generations. So there is really not a strong case for DT here. In fact
so far the mux *value* required isn't even stored in the DT, but in a
table in the pinctrl driver.

> KConfig may be usable for this 
> but that will make things cumbersome to support.
> 
> > There is tons of work and cleanup to do on the sunxi side, and were
> > already have quite some backlog, so I want to avoid introducing more
> > construction sites.
> 
> Fair, it also doesn't mean that what's currently added cannot be 
> migrated later on :)

Sure, but at the moment we are severely review limited, so unless that
changes dramatically, I don't see that happening any time soon.

> 
> [...]
> 
> >>>> You can have the property in the -u-boot.dtsi then if you want?
> >>>>
> >>>> While the FEL button on the X96 is "fake", it does what it says, just in
> >>>> software, maybe that is close enough to "hardware definition" which
> >>>> would make it suitable for the DT (well, we also store binman nodes in  
> >>>
> >>> Yes, I have a patch to add this particular button as a GPIO button into
> >>> the DT, so people can use it for whatever they want in Linux (trigger
> >>> reboot, update, you name it). But this is rather orthogonal to this
> >>> problem, as mentioned above.
> >>>      
> >>
> >> Mmmmm but this will be in the Linux kernel DT and I assume you want the
> >> same GPIO to be used in U-Boot and in Linux, so it would probably be
> >> best to make sure they stay in sync? How are you planning to do that?  
> > 
> > On the Allwinner side we were syncing the DTs regularly for years already,
> > and had no conflicting or different bindings at all. So I prefer to think
> > of "the DT", with Linux or U-Boot just carrying slightly different
> > versions for some time.
>   
> If the kernel DTB is coming from U-Boot, it should be much less 
> difficult to keep this synced.

Yes, that's what I am pushing for: don't bother loading a DTB, just use
the one already provided by U-Boot, as this makes the whole kernel boot
much less board specific: just give it a kernel with the right drivers
in and it boots. And it works really nicely with U-Boot on SPI flash or
on the eMMC boot partition.

> But if it isn't, you would need to patch 
> it live before booting the kernel for example.

We do this for the MAC address and the memory info, but I'd really like
to keep this minimal, and be it as an incentive for people to use
$fdtcontroladdr ;-)

Cheers,
Andre

> In any case, I think the conclusion is that DT cannot be used (yet?) for 
> that so this thread is now essentially just me being curious :)
> 
> Cheers,
> Quentin
Quentin Schulz May 15, 2025, 3:13 p.m. UTC | #11
Hi Andre,

On 5/12/25 2:39 PM, Andre Przywara wrote:
> On Tue, 22 Apr 2025 16:30:04 +0200
> Quentin Schulz <quentin.schulz@cherry.de> wrote:
> 
> Hi Quentin,
> 
> just found this in my draft folder. It's not really related to this
> patch anymore, but you seemed to be interested, and I am happy to
> explain some of the specialities for sunxi in U-Boot, since it differs
> in many things from other platforms.
> So see below...
> 

Thanks for taking the time to share this with me :)

[...]

>> Jonas has started to support "generic" images for Rockchip boards based
>> on the recommended hardware design specified by Rockchip themselves.
>> Most companies do mostly respect it, so that seems to be working quite
>> nicely.
> 
> I wouldn't be aware of a "recommended" board design for Allwinner, but
> as a matter of fact many companies copy their reference design -
> probably more out of laziness ;-). This brings the variation down to a
> manageable level, so we can define default values in Kconfig, so
> defconfigs stay small.
> 

Yeah I meant reference design and not recommended design :)

I assume it's not necessarily laziness, but also makes it less prone to 
HW issues and also hit "SW currently doesn't support that despite the 
datasheet saying the HW does".

>> Depending on what exactly you want to support with U-Boot, a DM-less SPL
>> may be difficult. e.g. if you want to support a fallback storage medium
>> for loading u-boot.itb (or proper, don't know what's being used on
>> Allwinner) that differs from the one used to load the SPL by the
>> BootROM, then you possibly cannot rely on the BootROM initializing the
>> PHYs, controllers, pinmuxes and pinconfs.
> 
> We do not rely on any of those bits being setup, actually, but we
> naturally follow the BootROM in its decision process. The BROM stores
> the boot media used in a byte in SRAM, so we know where we have been
> loaded from, so can continue loading from there. But this is just a
> decision between SD card, eMMC, NOR flash and FEL mode (maskROM in RK
> lingo). We know that SD card boot must have been from MMC0 on the PortF
> pins, and NOR flash is only SPI0 on PortC, on so on. The mux values and
> MMIO base addresses are per SoC, so those two values are easily stored
> in a header or in Kconfig, where we put them *once*, when we add
> support for a new SoC - and they are also quite stable across
> generations. So there is really not a strong case for DT here. In fact
> so far the mux *value* required isn't even stored in the DT, but in a
> table in the pinctrl driver.
> 

I meant if for example U-Boot proper stored on the eMMC (when the BROM 
loads SPL from eMMC) may be corrupted, and U-Boot should be able to try 
loading from other storage media, e.g. SD or SPI-NOR. If you don't have 
DT, you have to setup the controllers, PHYs, resets, etc... by hand in C 
files instead. Not very convenient when you have something that is made 
for that: DT :)

I'm often bringing this topic because I went through the pain of 
supporting this fallback mechanism on our RK3399 Puma board which has 9 
booting scenarios (TPL+SPL on eMMC/SD/SPI and U-Boot on eMMC/SD/SPI and 
you can boot proper from any of the TPL+SPL if the same medium as 
TPL+SPL somehow is corrupted). It could actually be more if/when there's 
support for USB loading mainline U-Boot.

>> KConfig may be usable for this
>> but that will make things cumbersome to support.
>>
>>> There is tons of work and cleanup to do on the sunxi side, and were
>>> already have quite some backlog, so I want to avoid introducing more
>>> construction sites.
>>
>> Fair, it also doesn't mean that what's currently added cannot be
>> migrated later on :)
> 
> Sure, but at the moment we are severely review limited, so unless that
> changes dramatically, I don't see that happening any time soon.
> 

"Later on" has no deadline :) It was also not a way to pressure you into 
doing it (now or later).

Cheers,
Quentin
diff mbox series

Patch

diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
index ab432390d3c..f1cfdb548bc 100644
--- a/arch/arm/mach-sunxi/Kconfig
+++ b/arch/arm/mach-sunxi/Kconfig
@@ -825,6 +825,16 @@  config USB3_VBUS_PIN
 	---help---
 	See USB1_VBUS_PIN help text.
 
+config SUNXI_FAKE_FEL_PIN
+	string "fake FEL GPIO pin"
+	default ""
+	---help---
+	Define a GPIO that shall force entering FEL mode when a button
+	connected to this pin is pressed at boot time. This must be an
+	active low signal, the internal pull-up resistors are activated.
+	This takes a string in the format understood by sunxi_name_to_gpio,
+	e.g. PH1 for pin 1 of port H.
+
 config I2C0_ENABLE
 	bool "Enable I2C/TWI controller 0"
 	default y if MACH_SUN4I || MACH_SUN5I || MACH_SUN7I || MACH_SUN8I_R40
diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
index 701899ee4b2..4ee0b333176 100644
--- a/arch/arm/mach-sunxi/board.c
+++ b/arch/arm/mach-sunxi/board.c
@@ -457,8 +457,39 @@  u32 spl_mmc_boot_mode(struct mmc *mmc, const u32 boot_device)
 	return result;
 }
 
+static void check_fake_fel_button(void)
+{
+	u32 brom_entry = 0x20;
+	int pin, value, mux;
+
+	/* check for empty string at compile time */
+	if (sizeof(CONFIG_SUNXI_FAKE_FEL_PIN) == sizeof(""))
+		return;
+
+	pin = sunxi_name_to_gpio(CONFIG_SUNXI_FAKE_FEL_PIN);
+	if (pin < 0)
+		return;
+
+	mux = sunxi_gpio_get_cfgpin(pin);
+	sunxi_gpio_set_cfgpin(pin, SUNXI_GPIO_INPUT);
+	sunxi_gpio_set_pull(pin, SUNXI_GPIO_PULL_UP);
+	value = gpio_get_value(pin);
+	sunxi_gpio_set_cfgpin(pin, mux);
+
+	if (value)
+		return;
+
+	/* Older SoCs maps the BootROM high in the address space. */
+	if (fel_stash.sctlr & BIT(13))
+		brom_entry |= 0xffff0000;
+
+	return_to_fel(0, brom_entry);
+}
+
 void board_init_f(ulong dummy)
 {
+	check_fake_fel_button();
+
 	sunxi_sram_init();
 
 	/* Enable non-secure access to some peripherals */