Re: [DistroKit] [PATCH v3 5/8] v8a: rockchip: use common environment

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 23.06.23 15:04, Roland Hieber wrote:
> On Wed, May 31, 2023 at 01:47:41PM +0200, Ahmad Fatoum wrote:
>> This effectively reverts commit 95014f4d8a50 ("v8a: rockchip: copy default
>> barebox env"). The common environment and the rockchip one are now
>> identical.
>>
>> The spi-nor part is just for the Rockchip board though, so let's make
>> its execution conditional and drop barebox-rock3a-defaultenv.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  .../barebox-common-defaultenv/init/bootsource       | 10 +++++++---
>>  .../barebox-rock3a-defaultenv/init/bootsource       | 13 -------------
>>  .../nv/linux.bootargs.loglevel                      |  1 -
>>  configs/platform-v8a/rules/barebox-rockchip.make    |  2 +-
>>  4 files changed, 8 insertions(+), 18 deletions(-)
>>  delete mode 100644 configs/platform-v8a/barebox-rock3a-defaultenv/init/bootsource
>>  delete mode 100644 configs/platform-v8a/barebox-rock3a-defaultenv/nv/linux.bootargs.loglevel
>>
>> diff --git a/configs/platform-v8a/barebox-common-defaultenv/init/bootsource b/configs/platform-v8a/barebox-common-defaultenv/init/bootsource
>> index 8f00bced5df2..db1915321531 100644
>> --- a/configs/platform-v8a/barebox-common-defaultenv/init/bootsource
>> +++ b/configs/platform-v8a/barebox-common-defaultenv/init/bootsource
>> @@ -8,7 +8,11 @@ fi
>>  if [ "$bootsource" = mmc ]; then
>>  	detect mmc$bootsource_instance
>>  	global.boot.default="mmc$bootsource_instance net"
>> -elif [ "$bootsource" = "spi-nor" ]; then
>> -	usbgadget -A '/dev/mmc0(sd)'
>> -	global.autoboot=abort
>> +fi
>> +
>> +if [ "${global.model}" = "Radxa ROCK3 Model A" ]; then
> 
> At first I thought it would be better to match the board compatible (it
> looks more machine-parsable and less subject to change), but then I
> found out that there isn't a variable for that in barebox yet… Do you
> think it makes sense to add one too? It seems like the code would be a
> bit different than for the model though…

barebox v2023.07.0 should contain an of_compatible command exactly for
this use case[1]. This can be used instead of the model once we update.

I'd only advise this for scripts though. If you have a set of nv variables
that's board specific, you could install the defaultenv into the board dir
in the barebox source tree and add bbenv-y to the board Makefile.

[1]: https://github.com/barebox/barebox/commit/bf7405c00016beb7860729d18d4fe53772b81db7

> 
>  - Roland
> 
>> +	if [ "$bootsource" = "spi-nor" ]; then
>> +		usbgadget -A '/dev/mmc1(sd)'
>> +		global.autoboot=abort
>> +	fi
>>  fi
>> diff --git a/configs/platform-v8a/barebox-rock3a-defaultenv/init/bootsource b/configs/platform-v8a/barebox-rock3a-defaultenv/init/bootsource
>> deleted file mode 100644
>> index bc81609a49c0..000000000000
>> --- a/configs/platform-v8a/barebox-rock3a-defaultenv/init/bootsource
>> +++ /dev/null
>> @@ -1,13 +0,0 @@
>> -#!/bin/sh
>> -
>> -if [ -n "${nv.boot.default}" ]; then
>> -	exit
>> -fi
>> -
>> -if [ "$bootsource" = mmc ]; then
>> -	detect mmc$bootsource_instance
>> -	global.boot.default="mmc$bootsource_instance net"
>> -elif [ "$bootsource" = "spi-nor" ]; then
>> -	usbgadget -A '/dev/mmc1(sd)'
>> -	global.autoboot=abort
>> -fi
>> diff --git a/configs/platform-v8a/barebox-rock3a-defaultenv/nv/linux.bootargs.loglevel b/configs/platform-v8a/barebox-rock3a-defaultenv/nv/linux.bootargs.loglevel
>> deleted file mode 100644
>> index 705752ca416d..000000000000
>> --- a/configs/platform-v8a/barebox-rock3a-defaultenv/nv/linux.bootargs.loglevel
>> +++ /dev/null
>> @@ -1 +0,0 @@
>> -loglevel=5 systemd.log_level=warning systemd.show_status=auto
>> diff --git a/configs/platform-v8a/rules/barebox-rockchip.make b/configs/platform-v8a/rules/barebox-rockchip.make
>> index 805c8749d710..1ca7e8c109ee 100644
>> --- a/configs/platform-v8a/rules/barebox-rockchip.make
>> +++ b/configs/platform-v8a/rules/barebox-rockchip.make
>> @@ -69,7 +69,7 @@ $(BAREBOX_ROCKCHIP_CONFIG):
>>  endif
>>  
>>  BAREBOX_ROCKCHIP_EXTRA_ENV_PATH  := $(foreach path, \
>> -                $(call remove_quotes,"barebox-rock3a-defaultenv"), \
>> +                $(call remove_quotes,"barebox-common-defaultenv"), \
>>                  $(call ptx/in-platformconfigdir,$(path)))
>>  BAREBOX_ROCKCHIP_EXTRA_ENV_DEPS  := \
>>          $(BAREBOX_ROCKCHIP_EXTRA_ENV_PATH) \
>> -- 
>> 2.39.2
>>
>>
>>
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |