Re: [PATCH v3 6/6] board: add support for Schneider HMIBSC board

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@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.

---

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@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

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).

Thanks, Stephan

[1]: https://docs.u-boot.org/en/latest/usage/environment.html#text-based-environm...