[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 at 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 at jones.dk>
>> Signed-off-by: Jonas Smedegaard <dr at jones.dk>
>> Series-Cc: Jagan Teki <jagan at amarulasolutions.com>
>> Series-Cc: Lukasz Majewski <lukma at denx.de>
>> Series-Cc: Andre Przywara <andre.przywara at 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.