Re: [PATCH v3 6/6] board: add support for Schneider HMIBSC board
Hi Sumit, On 08/04/2024 11:13, Sumit Garg wrote: On Fri, 5 Apr 2024 at 20:16, Stephan Gerhold 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 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 000..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 + */ + +#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=0x9000\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 059
Re: [PATCH v3 6/6] board: add support for Schneider HMIBSC board
On Fri, 5 Apr 2024 at 20:16, Stephan Gerhold 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 > > 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. > > > --- > > 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 000..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 > > + */ > > + > > +#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=0x9000\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 t
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 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 000..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 > + */ > + > +#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=0x9000\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-environment
[PATCH v3 6/6] board: add support for Schneider HMIBSC board
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 --- 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/arch/arm/dts/apq8016-schneider-hmibsc.dts b/arch/arm/dts/apq8016-schneider-hmibsc.dts new file mode 100644 index 000..75c6137e5a1 --- /dev/null +++ b/arch/arm/dts/apq8016-schneider-hmibsc.dts @@ -0,0 +1,491 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2015, The Linux Foundation. All rights reserved. + * Copyright (c) 2024, Linaro Ltd. + */ + +/dts-v1/; + +#include "msm8916-pm8916.dtsi" +#include +#include +#include +#include +#include +#include + +/ { + model = "Schneider Electric HMIBSC Board"; + compatible = "schneider,apq8016-hmibsc", "qcom,apq8016"; + + aliases { + i2c1 = &blsp_i2c6; + i2c3 = &blsp_i2c4; + i2c4 = &blsp_i2c3; + mmc0 = &sdhc_1; /* eMMC */ + mmc1 = &sdhc_2; /* SD card */ + serial0 = &blsp_uart1; + serial1 = &blsp_uart2; + spi0 = &blsp_spi5; + usid0 = &pm8916_0; + }; + + chosen { + stdout-path = "serial0"; + }; + + hdmi-out { + compatible = "hdmi-connector"; + type = "a"; + + port { + hdmi_con: endpoint { + remote-endpoint = <&adv7533_out>; + }; + }; + }; + + gpio-keys { + compatible = "gpio-keys"; + autorepeat; + pinctrl-0 = <&msm_key_volp_n_default>; + pinctrl-names = "default"; + + button { + label = "Volume Up"; + linux,code = ; + gpios = <&tlmm 107 GPIO_ACTIVE_LOW>; + }; + }; + + leds { + compatible = "gpio-leds"; + pinctrl-0 = <&pm8916_mpps_leds>; + pinctrl-names = "default"; + + led-1 { + function = LED_FUNCTION_WLAN; + color = ; + gpios = <&pm8916_mpps 2 GPIO_ACTIVE_HIGH>; + linux,default-trigger = "phy0tx"; + default-state = "off"; + }; + + led-2 { + function = LED_FUNCTION_BLUETOOTH; + color = ; + gpios = <&pm8916_mpps 3 GPIO_ACTIVE_HIGH>; + linux,default-trigger = "bluetooth-power"; + default-state = "off"; + }; + }; + + memory@8000 { + reg = <0 0x8000 0 0x4000>; + }; + + reserved-memory { + ramoops@bff0 { + compatible = "ramoops"; + reg = <0x0 0xbff0 0x0 0x10>; + record-size = <0x2>; + console-size = <0x2>; + ftrace-size = <0x2>; + ecc-size = <16>; + }; + }; + + usb-hub { + compatible = "smsc,usb3503"; + reset-gpios = <&pm8916_gpios 1 GPIO_ACTIVE_LOW>; + initial-mode = <1>; + }; + + usb_id: usb-id { + compatible = "linux,extcon-usb-gpio"; + id-gpios = <&tlmm 110 GPIO_ACTIVE_HIGH>; + pinctrl-0 = <&usb_id_default>; + pinctrl-names = "default"; + }; +}; + +&blsp_i2c3 { + status = "okay"; + + eeprom@50 { + compatible = "atmel,24c32"; + reg = <0x50>; + }; +}; + +&blsp_i2c4 { + status = "okay"; + + adv_bridge: bridge@39 { + compatible = "adi,adv7533"; + reg = <0x39>; + interrupts-extended = <&tlmm 31 IRQ_TYPE_EDGE_FALLING>; + + adi,dsi-lanes = <4>; + clocks = <&rpmcc RPM_SMD_BB_CLK2>; +