On Thu, 17 Apr 2025 03:39:35 +0000
Yixun Lan <d...@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.przyw...@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
> >   
> 

Reply via email to