[PATCH v3 6/6] board: add support for Schneider HMIBSC board
Caleb Connolly
caleb.connolly at linaro.org
Mon Apr 8 14:45:34 CEST 2024
Hi Sumit,
On 08/04/2024 11:13, Sumit Garg wrote:
> On Fri, 5 Apr 2024 at 20:16, Stephan Gerhold <stephan at gerhold.net> wrote:
>>
>> On Fri, Apr 05, 2024 at 02:37:42PM +0530, Sumit Garg wrote:
>>> Support for Schneider Electric HMIBSC. Features:
>>> - Qualcomm Snapdragon 410C SoC - APQ8016 (4xCortex A53, Adreno 306)
>>> - 2GiB RAM
>>> - 64GiB eMMC, SD slot
>>> - WiFi and Bluetooth
>>> - 2x Host, 1x Device USB port
>>> - HDMI
>>> - Discrete TPM2 chip over SPI
>>>
>>> Features enabled in U-Boot:
>>> - RAUC updates
>>> - Environment protection
>>> - USB based ethernet adaptors
>>>
>>> Signed-off-by: Sumit Garg <sumit.garg at linaro.org>
>>
>> I don't think this is a big deal but this patch would be a bit easier to
>> skim over if you move the (unmodified?) import of the Linux
>> apq8016-schneider-hmibsc.dts to a separate patch with a clear note in
>> the commit message
>>
>> - where it comes from (link to Linux patch), and
>> - that it can be removed again with a future update of the upstream DTs
>> in U-Boot (once it is applied upstream at least).
>>
>> You kind of have that information in the cover letter but I think it
>> would be good to have it in the commit message.
>
> Although the general practice in U-Boot is to have the board DTS file
> submitted along with board code, if it makes review easier via
> separating the unmodified import of the board DTS file then I can do
> that.
Given that in a few cycles we intend to drop the DTS you're adding here,
having a commit we can just revert will make that much simpler.
>
>>
>>> ---
>>> arch/arm/dts/apq8016-schneider-hmibsc.dts | 491 ++++++++++++++++++++++
>>> board/schneider/hmibsc/MAINTAINERS | 6 +
>>> configs/hmibsc_defconfig | 86 ++++
>>> doc/board/index.rst | 1 +
>>> doc/board/schneider/hmibsc.rst | 45 ++
>>> doc/board/schneider/index.rst | 9 +
>>> include/configs/hmibsc.h | 57 +++
>>> 7 files changed, 695 insertions(+)
>>> create mode 100644 arch/arm/dts/apq8016-schneider-hmibsc.dts
>>> create mode 100644 board/schneider/hmibsc/MAINTAINERS
>>> create mode 100644 configs/hmibsc_defconfig
>>> create mode 100644 doc/board/schneider/hmibsc.rst
>>> create mode 100644 doc/board/schneider/index.rst
>>> create mode 100644 include/configs/hmibsc.h
>>>
>>> [...]
>>> diff --git a/include/configs/hmibsc.h b/include/configs/hmibsc.h
>>> new file mode 100644
>>> index 00000000000..66dfa549ce1
>>> --- /dev/null
>>> +++ b/include/configs/hmibsc.h
>>> @@ -0,0 +1,57 @@
>>> +/* SPDX-License-Identifier: GPL-2.0+ */
>>> +/*
>>> + * Board configuration file for HMIBSC
>>> + *
>>> + * (C) Copyright 2024 Sumit Garg <sumit.garg at linaro.org>
>>> + */
>>> +
>>> +#ifndef __CONFIGS_HMIBSC_H
>>> +#define __CONFIGS_HMIBSC_H
>>> +
>>> +/* PHY needs a longer aneg time */
>>> +#define PHY_ANEG_TIMEOUT 8000
>>> +
>>> +#define HMIBSC_BOOTCOMMAND \
>>> + "setenv devtype mmc; setenv devnum 0; " \
>>> + "test -n \"${BOOT_ORDER}\" || setenv BOOT_ORDER \"A B\"; " \
>>> + "test -n \"${BOOT_A_LEFT}\" || setenv BOOT_A_LEFT 3; " \
>>> + "test -n \"${BOOT_B_LEFT}\" || setenv BOOT_B_LEFT 3; " \
>>> + "setenv raucslot; " \
>>> + "for BOOT_SLOT in \"${BOOT_ORDER}\"; do " \
>>> + " if test \"x${raucslot}\" != \"x\"; then " \
>>> + " echo \"skip remaining slots...\"; " \
>>> + " elif test \"x${BOOT_SLOT}\" = \"xA\"; then " \
>>> + " if test ${BOOT_A_LEFT} -gt 0; then " \
>>> + " setexpr BOOT_A_LEFT ${BOOT_A_LEFT} - 1; " \
>>> + " echo \"Found valid RAUC slot A\"; " \
>>> + " setenv raucslot \"rauc.slot=A\"; " \
>>> + " setenv raucpart A; setenv distro_bootpart 6;" \
>>> + " fi; " \
>>> + " elif test \"x${BOOT_SLOT}\" = \"xB\"; then " \
>>> + " if test ${BOOT_B_LEFT} -gt 0; then " \
>>> + " setexpr BOOT_B_LEFT ${BOOT_B_LEFT} - 1; " \
>>> + " echo \"Found valid RAUC slot B\"; " \
>>> + " setenv raucslot \"rauc.slot=B\"; " \
>>> + " setenv raucpart B; setenv distro_bootpart 7;" \
>>> + " fi; " \
>>> + " fi; " \
>>> + "done; " \
>>> + "if test -n \"${raucslot}\"; then " \
>>> + " setenv bootargs console=ttyMSM1 root=PARTLABEL=rootfs_${raucpart} rw rootwait ${raucslot}; " \
>>> + " saveenv; " \
>>> + "else " \
>>> + " echo \"No valid RAUC slot found. Resetting tries to 3\"; " \
>>> + " setenv BOOT_A_LEFT 3; " \
>>> + " setenv BOOT_B_LEFT 3; " \
>>> + " saveenv; " \
>>> + " reset; " \
>>> + "fi; " \
>>> + "load ${devtype} ${devnum}:${distro_bootpart} ${loadaddr} /boot/fitImage && bootm"
>>> +
>>> +#define CFG_EXTRA_ENV_SETTINGS \
>>> + "loadaddr=0x90000000\0" \
>>> + "bootcmd=" HMIBSC_BOOTCOMMAND "\0"
>>> +
>>
>> The "text-based environment" [1] is preferred nowadays, i.e. defining
>> these inside board/schneider/hmibsc/hmibsc.env instead (similar to how
>> DB410c has its environment defined in
>> board/qualcomm/dragonboard410c/dragonboard410c.env). This should also
>> avoid all the crazy escaping to encode it as C string. :D
>
> That makes sense, I will convert to that.
>
>>
>> However, I suspect that right now it would attempt to load this file
>> from board/qualcomm/hmibsc/hmibsc.env since you do not seem to have
>> CONFIG_SYS_VENDOR set correctly. It looks like Caleb removed the option
>> to customize CONFIG_SYS_VENDOR in commit 059d526af312 ("mach-snapdragon:
>> generalise board support"). It might be easiest to add a prompt in
>> arch/arm/mach-snapdragon/Kconfig to allow changing it (similar to
>> CONFIG_SYS_BOARD).
>
> Although the restriction for Qualcomm being the only vendor for boards
> based on snapdragon SoCs shouldn't be added in the first place but
> since that patch did a lot of things in one go made its review harder.
> However, we can always fix it which I will do for the next version.
CONFIG_DEFAULT_ENV_FILE can be set relative to the source tree root, so
you can put your .env file anywhere you like and specify the full path.
I'm fine with allowing CONFIG_SYS_VENDOR to be overriden in the same way
CONFIG_SYS_BOARD can be.
>
> -Sumit
>
>>
>> Thanks,
>> Stephan
>>
>> [1]: https://docs.u-boot.org/en/latest/usage/environment.html#text-based-environment
--
// Caleb (they/them)
More information about the U-Boot
mailing list