[PATCH v3 1/2] sunxi: fix support board-specific CONFIG_PREBOOT

[PATCH v3 1/2] sunxi: fix support board-specific CONFIG_PREBOOT

[Jonas Smedegaard]

commit 37304aaf60bf ("Convert CONFIG_USE_PREBOOT and CONFIG_PREBOOT to
Kconfig") intended to support CONFIG_PREBOOT, but
include/configs/sunxi-common.h hardcodes preboot as part of internally
defined CONSOLE_STDIN_SETTINGS, silently ignoring any board-specific
CONFIG_PREBOOT.

This commit moves sunxi-specific CONFIG_PREBOOT to Kconfig,
which supports board-specific override.

Tested-by: Jonas Smedegaard <dr@jones.dk>
Signed-off-by: Jonas Smedegaard <dr@jones.dk>
Series-Cc: Jagan Teki <jagan@amarulasolutions.com>
Series-Cc: Lukasz Majewski <lukma@denx.de>
Series-Cc: Andre Przywara <andre.przywara@arm.com>

---


Changes in v3:
- move default setting to KConfig, thanks to Andre Przywara and Lukasz Majewski

Changes in v2:
- Rephrase commit message to clarify relationship with KConfig entries

---
 arch/arm/mach-sunxi/Kconfig    | 3 +++
 include/configs/sunxi-common.h | 1 -
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
index 3a3b673430..9f16d903a0 100644
--- a/arch/arm/mach-sunxi/Kconfig
+++ b/arch/arm/mach-sunxi/Kconfig
@@ -48,6 +48,9 @@ config DRAM_SUN50I_H6
 	  Select this dram controller driver for some sun50i platforms,
 	  like H6.
 
+config PREBOOT
+	default "usb start" if USB_KEYBOARD
+
 config SUN6I_P2WI
 	bool "Allwinner sun6i internal P2WI controller"
 	help
diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
index 0ef289fd64..69ef65193e 100644
--- a/include/configs/sunxi-common.h
+++ b/include/configs/sunxi-common.h
@@ -429,7 +429,6 @@ extern int soft_i2c_gpio_scl;
 
 #ifdef CONFIG_USB_KEYBOARD
 #define CONSOLE_STDIN_SETTINGS \
-	"preboot=usb start\0" \
 	"stdin=serial,usbkbd\0"
 #else
 #define CONSOLE_STDIN_SETTINGS \
-- 
2.25.1

[PATCH v3 2/2] sun50i: a64: A64-Teres-I board detect builtin keyboard

[Jonas Smedegaard]

A64-Teres-I board is a laptop which comes with a builtin keyboard.
The keyboard+trackpad controller pauses for 2 seconds at a firmware
prompt before loading its HID interface.

U-Boot needs to wait equally long to reliably enable the keyboard.

Tested-by: Jonas Smedegaard <dr@jones.dk>
Signed-off-by: Jonas Smedegaard <dr@jones.dk>
Series-Cc: Jagan Teki <jagan@amarulasolutions.com>
Series-Cc: Lukasz Majewski <lukma@denx.de>
Series-Cc: Andre Przywara <andre.przywara@arm.com>

---


Changes in v3:
- (none)
Changes in v2:
- (none)

---
 configs/teres_i_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/teres_i_defconfig b/configs/teres_i_defconfig
index 1019dab54a..61bbf76b1e 100644
--- a/configs/teres_i_defconfig
+++ b/configs/teres_i_defconfig
@@ -8,6 +8,7 @@ CONFIG_MMC_SUNXI_SLOT_EXTRA=2
 CONFIG_USB1_VBUS_PIN="PL7"
 CONFIG_I2C0_ENABLE=y
 CONFIG_USE_PREBOOT=y
+CONFIG_PREBOOT="setenv usb_pgood_delay 2000; usb reset"
 # CONFIG_SPL_DOS_PARTITION is not set
 # CONFIG_SPL_EFI_PARTITION is not set
 CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-teres-i"
-- 
2.25.1

[PATCH v3 1/2] sunxi: fix support board-specific CONFIG_PREBOOT

[Andre Przywara]

On Tue,  3 Mar 2020 16:08:00 +0100
Jonas Smedegaard <dr@jones.dk> wrote:

Hi,

> commit 37304aaf60bf ("Convert CONFIG_USE_PREBOOT and CONFIG_PREBOOT to
> Kconfig") intended to support CONFIG_PREBOOT, but
> include/configs/sunxi-common.h hardcodes preboot as part of internally
> defined CONSOLE_STDIN_SETTINGS, silently ignoring any board-specific
> CONFIG_PREBOOT.
> 
> This commit moves sunxi-specific CONFIG_PREBOOT to Kconfig,
> which supports board-specific override.

Yes, thanks for that! Actually seems to fix some minor annoyance as well, were preboot was defined twice in the default environment.

> Tested-by: Jonas Smedegaard <dr@jones.dk>

Some nit: This is somewhat implicit when you are the author. At least that's the hope ;-)

> Signed-off-by: Jonas Smedegaard <dr@jones.dk>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

> Series-Cc: Jagan Teki <jagan@amarulasolutions.com>
> Series-Cc: Lukasz Majewski <lukma@denx.de>
> Series-Cc: Andre Przywara <andre.przywara@arm.com>

Is this because of patman? If this applies to the whole series, I typically just add CC:s to the git send-email command line. that keeps the commits cleaner. I am wondering if this tag should be added to the cover letter then, because patman requires those tags only in one commit of a series.

Thanks,
Andre.

> ---
> 
> 
> Changes in v3:
> - move default setting to KConfig, thanks to Andre Przywara and Lukasz Majewski
> 
> Changes in v2:
> - Rephrase commit message to clarify relationship with KConfig entries
> 
> ---
>  arch/arm/mach-sunxi/Kconfig    | 3 +++
>  include/configs/sunxi-common.h | 1 -
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> index 3a3b673430..9f16d903a0 100644
> --- a/arch/arm/mach-sunxi/Kconfig
> +++ b/arch/arm/mach-sunxi/Kconfig
> @@ -48,6 +48,9 @@ config DRAM_SUN50I_H6
>  	  Select this dram controller driver for some sun50i platforms,
>  	  like H6.
>  
> +config PREBOOT
> +	default "usb start" if USB_KEYBOARD
> +
>  config SUN6I_P2WI
>  	bool "Allwinner sun6i internal P2WI controller"
>  	help
> diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
> index 0ef289fd64..69ef65193e 100644
> --- a/include/configs/sunxi-common.h
> +++ b/include/configs/sunxi-common.h
> @@ -429,7 +429,6 @@ extern int soft_i2c_gpio_scl;
>  
>  #ifdef CONFIG_USB_KEYBOARD
>  #define CONSOLE_STDIN_SETTINGS \
> -	"preboot=usb start\0" \
>  	"stdin=serial,usbkbd\0"
>  #else
>  #define CONSOLE_STDIN_SETTINGS \

[PATCH v3 1/2] sunxi: fix support board-specific CONFIG_PREBOOT

[Jonas Smedegaard]

Quoting Andre Przywara (2020-03-04 11:59:40)
> On Tue,  3 Mar 2020 16:08:00 +0100
> Jonas Smedegaard <dr@jones.dk> wrote:
> > Tested-by: Jonas Smedegaard <dr@jones.dk>
> 
> Some nit: This is somewhat implicit when you are the author. At least that's the hope ;-)

Ah, ok.  Will keep that in mind for future patch proposals.


> > Signed-off-by: Jonas Smedegaard <dr@jones.dk>
> 
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Thanks!


> > Series-Cc: Jagan Teki <jagan@amarulasolutions.com>
> > Series-Cc: Lukasz Majewski <lukma@denx.de>
> > Series-Cc: Andre Przywara <andre.przywara@arm.com>
> 
> Is this because of patman? If this applies to the whole series, I 
> typically just add CC:s to the git send-email command line. that keeps 
> the commits cleaner. I am wondering if this tag should be added to the 
> cover letter then, because patman requires those tags only in one 
> commit of a series.

You mean you just memorize relevant Cc addresses and add them manually?

I am new to patman and to contributing patches like this, and just 
fumbled my way to something that seemed to work.  I shall try adopt your 
described approach for future patch proposals, thanks!


 - Jonas

-- 
 * Jonas Smedegaard - idealist & Internet-arkitekt
 * Tlf.: +45 40843136  Website: http://dr.jones.dk/

 [x] quote me freely  [ ] ask before reusing  [ ] keep private
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200304/72cc95a4/attachment.sig>

[PATCH v3 1/2] sunxi: fix support board-specific CONFIG_PREBOOT

[Jagan Teki]

On Tue, Mar 3, 2020 at 8:37 PM Jonas Smedegaard <dr@jones.dk> wrote:
>
> commit 37304aaf60bf ("Convert CONFIG_USE_PREBOOT and CONFIG_PREBOOT to
> Kconfig") intended to support CONFIG_PREBOOT, but
> include/configs/sunxi-common.h hardcodes preboot as part of internally
> defined CONSOLE_STDIN_SETTINGS, silently ignoring any board-specific
> CONFIG_PREBOOT.
>
> This commit moves sunxi-specific CONFIG_PREBOOT to Kconfig,
> which supports board-specific override.
>
> Tested-by: Jonas Smedegaard <dr@jones.dk>
> Signed-off-by: Jonas Smedegaard <dr@jones.dk>
> Series-Cc: Jagan Teki <jagan@amarulasolutions.com>
> Series-Cc: Lukasz Majewski <lukma@denx.de>
> Series-Cc: Andre Przywara <andre.przywara@arm.com>
>
> ---
>
>
> Changes in v3:
> - move default setting to KConfig, thanks to Andre Przywara and Lukasz Majewski
>
> Changes in v2:
> - Rephrase commit message to clarify relationship with KConfig entries
>
> ---
>  arch/arm/mach-sunxi/Kconfig    | 3 +++
>  include/configs/sunxi-common.h | 1 -
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> index 3a3b673430..9f16d903a0 100644
> --- a/arch/arm/mach-sunxi/Kconfig
> +++ b/arch/arm/mach-sunxi/Kconfig
> @@ -48,6 +48,9 @@ config DRAM_SUN50I_H6
>           Select this dram controller driver for some sun50i platforms,
>           like H6.
>
> +config PREBOOT
> +       default "usb start" if USB_KEYBOARD
> +

This is already available in common/Kconfig better select there with
proper depends.

[PATCH v3 1/2] sunxi: fix support board-specific CONFIG_PREBOOT

[Jonas Smedegaard]

Quoting Jagan Teki (2020-03-28 15:32:46)
> On Tue, Mar 3, 2020 at 8:37 PM Jonas Smedegaard <dr@jones.dk> wrote:
> >
> > commit 37304aaf60bf ("Convert CONFIG_USE_PREBOOT and CONFIG_PREBOOT to
> > Kconfig") intended to support CONFIG_PREBOOT, but
> > include/configs/sunxi-common.h hardcodes preboot as part of internally
> > defined CONSOLE_STDIN_SETTINGS, silently ignoring any board-specific
> > CONFIG_PREBOOT.
> >
> > This commit moves sunxi-specific CONFIG_PREBOOT to Kconfig,
> > which supports board-specific override.
> >
> > Tested-by: Jonas Smedegaard <dr@jones.dk>
> > Signed-off-by: Jonas Smedegaard <dr@jones.dk>
> > Series-Cc: Jagan Teki <jagan@amarulasolutions.com>
> > Series-Cc: Lukasz Majewski <lukma@denx.de>
> > Series-Cc: Andre Przywara <andre.przywara@arm.com>
> >
> > ---
> >
> >
> > Changes in v3:
> > - move default setting to KConfig, thanks to Andre Przywara and Lukasz Majewski
> >
> > Changes in v2:
> > - Rephrase commit message to clarify relationship with KConfig entries
> >
> > ---
> >  arch/arm/mach-sunxi/Kconfig    | 3 +++
> >  include/configs/sunxi-common.h | 1 -
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> > index 3a3b673430..9f16d903a0 100644
> > --- a/arch/arm/mach-sunxi/Kconfig
> > +++ b/arch/arm/mach-sunxi/Kconfig
> > @@ -48,6 +48,9 @@ config DRAM_SUN50I_H6
> >           Select this dram controller driver for some sun50i platforms,
> >           like H6.
> >
> > +config PREBOOT
> > +       default "usb start" if USB_KEYBOARD
> > +
> 
> This is already available in common/Kconfig better select there with
> proper depends.

Makes sense, I will make another revision...

Thanks!

 - Jonas

-- 
 * Jonas Smedegaard - idealist & Internet-arkitekt
 * Tlf.: +45 40843136  Website: http://dr.jones.dk/

 [x] quote me freely  [ ] ask before reusing  [ ] keep private
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200328/c4b32199/attachment.sig>

[PATCH v3 1/2] sunxi: fix support board-specific CONFIG_PREBOOT

[André Przywara]

On 28/03/2020 14:32, Jagan Teki wrote:
> On Tue, Mar 3, 2020 at 8:37 PM Jonas Smedegaard <dr@jones.dk> wrote:
>>
>> commit 37304aaf60bf ("Convert CONFIG_USE_PREBOOT and CONFIG_PREBOOT to
>> Kconfig") intended to support CONFIG_PREBOOT, but
>> include/configs/sunxi-common.h hardcodes preboot as part of internally
>> defined CONSOLE_STDIN_SETTINGS, silently ignoring any board-specific
>> CONFIG_PREBOOT.
>>
>> This commit moves sunxi-specific CONFIG_PREBOOT to Kconfig,
>> which supports board-specific override.
>>
>> Tested-by: Jonas Smedegaard <dr@jones.dk>
>> Signed-off-by: Jonas Smedegaard <dr@jones.dk>
>> Series-Cc: Jagan Teki <jagan@amarulasolutions.com>
>> Series-Cc: Lukasz Majewski <lukma@denx.de>
>> Series-Cc: Andre Przywara <andre.przywara@arm.com>
>>
>> ---
>>
>>
>> Changes in v3:
>> - move default setting to KConfig, thanks to Andre Przywara and Lukasz Majewski
>>
>> Changes in v2:
>> - Rephrase commit message to clarify relationship with KConfig entries
>>
>> ---
>>  arch/arm/mach-sunxi/Kconfig    | 3 +++
>>  include/configs/sunxi-common.h | 1 -
>>  2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
>> index 3a3b673430..9f16d903a0 100644
>> --- a/arch/arm/mach-sunxi/Kconfig
>> +++ b/arch/arm/mach-sunxi/Kconfig
>> @@ -48,6 +48,9 @@ config DRAM_SUN50I_H6
>>           Select this dram controller driver for some sun50i platforms,
>>           like H6.
>>
>> +config PREBOOT
>> +       default "usb start" if USB_KEYBOARD
>> +
> 
> This is already available in common/Kconfig better select there with
> proper depends.

Well, it's defined in common/Kconfig, but we just set the value here.
This scheme is used all over the place already, check SYS_CLK_FREQ,
SYS_CONFIG_NAME,  SYS_BOARD, SYS_SOC and so on.

So I don't think it's a good idea to define those platform specific
default values at the place of their original definition.
Yes, we are doing this alot at the moment (especially for sunxi, and
mostly only for sunxi), but I think this is starting to get out of hands
now. If this is setting an example, we would clutter those platform
specific settings all over the various subsystems.

Actually I started some patches to move those "default xxx if
ARCH_SUNXI" lines to arch/arm/mach-sunxi, so they are all in one place.

So I think Jonas' patch is the right thing to do - unless it's really
generic, so if we would drop the "if ARCH_SUNXI" clause (which would
make some sense for this particular setting).

Cheers,
Andre.

[PATCH v3 1/2] sunxi: fix support board-specific CONFIG_PREBOOT

[Jagan Teki]

On Sat, Mar 28, 2020 at 9:38 PM Andr? Przywara <andre.przywara@arm.com> wrote:
>
> On 28/03/2020 14:32, Jagan Teki wrote:
> > On Tue, Mar 3, 2020 at 8:37 PM Jonas Smedegaard <dr@jones.dk> wrote:
> >>
> >> commit 37304aaf60bf ("Convert CONFIG_USE_PREBOOT and CONFIG_PREBOOT to
> >> Kconfig") intended to support CONFIG_PREBOOT, but
> >> include/configs/sunxi-common.h hardcodes preboot as part of internally
> >> defined CONSOLE_STDIN_SETTINGS, silently ignoring any board-specific
> >> CONFIG_PREBOOT.
> >>
> >> This commit moves sunxi-specific CONFIG_PREBOOT to Kconfig,
> >> which supports board-specific override.
> >>
> >> Tested-by: Jonas Smedegaard <dr@jones.dk>
> >> Signed-off-by: Jonas Smedegaard <dr@jones.dk>
> >> Series-Cc: Jagan Teki <jagan@amarulasolutions.com>
> >> Series-Cc: Lukasz Majewski <lukma@denx.de>
> >> Series-Cc: Andre Przywara <andre.przywara@arm.com>
> >>
> >> ---
> >>
> >>
> >> Changes in v3:
> >> - move default setting to KConfig, thanks to Andre Przywara and Lukasz Majewski
> >>
> >> Changes in v2:
> >> - Rephrase commit message to clarify relationship with KConfig entries
> >>
> >> ---
> >>  arch/arm/mach-sunxi/Kconfig    | 3 +++
> >>  include/configs/sunxi-common.h | 1 -
> >>  2 files changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> >> index 3a3b673430..9f16d903a0 100644
> >> --- a/arch/arm/mach-sunxi/Kconfig
> >> +++ b/arch/arm/mach-sunxi/Kconfig
> >> @@ -48,6 +48,9 @@ config DRAM_SUN50I_H6
> >>           Select this dram controller driver for some sun50i platforms,
> >>           like H6.
> >>
> >> +config PREBOOT
> >> +       default "usb start" if USB_KEYBOARD
> >> +
> >
> > This is already available in common/Kconfig better select there with
> > proper depends.
>
> Well, it's defined in common/Kconfig, but we just set the value here.
> This scheme is used all over the place already, check SYS_CLK_FREQ,
> SYS_CONFIG_NAME,  SYS_BOARD, SYS_SOC and so on.
>
> So I don't think it's a good idea to define those platform specific
> default values at the place of their original definition.
> Yes, we are doing this alot at the moment (especially for sunxi, and
> mostly only for sunxi), but I think this is starting to get out of hands
> now. If this is setting an example, we would clutter those platform
> specific settings all over the various subsystems.
>
> Actually I started some patches to move those "default xxx if
> ARCH_SUNXI" lines to arch/arm/mach-sunxi, so they are all in one place.
>
> So I think Jonas' patch is the right thing to do - unless it's really
> generic, so if we would drop the "if ARCH_SUNXI" clause (which would
> make some sense for this particular setting).

Yes, my idea is to skip platform depends here keeping USB_KEYBOARD
depends as 'usb start' is used many defconfigs. On that note, the new
patch need to change all places, but if require any other depends,
those need to change carefully.

Jagan.