All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] Add SE HMBSC board support
@ 2023-12-18  7:24 Sumit Garg
  2023-12-18  7:24 ` [PATCH 1/7] clk: apq8016: Add support for UART1 clocks Sumit Garg
                   ` (7 more replies)
  0 siblings, 8 replies; 20+ messages in thread
From: Sumit Garg @ 2023-12-18  7:24 UTC (permalink / raw)
  To: u-boot
  Cc: caleb.connolly, neil.armstrong, lukma, seanga2, marex,
	laetitia.mariottini, pascal.eberhard, abdou.saker, jimmy.lalande,
	benjamin.missey, daniel.thompson, Sumit Garg

SE HMIBSC board is based on Qcom APQ8016 SoC. One of the major
difference from db410c is serial port where HMIBSC board uses UART1 as
the debug console with an RS232 port, patch #1 - #3 adds corresponding
driver support.

Patch #4 adds main HMIBSC board specific bits, 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

Patch #5 - #7 enables specific board features like RAUC support,
environment protection and USB networking support.

This patch series is based on top of Qcom maintainer tree [1] + the latest
PMIC patch-set [2]. Feedback is very much welcome.

[1] https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commits/u-boot-qcom-next?ref_type=heads
[2] https://patchwork.ozlabs.org/project/uboot/list/?series=385322

Sumit Garg (7):
  clk: apq8016: Add support for UART1 clocks
  serial_msm: Add support for RS232 GPIOs
  serial_msm: Enable RS232 flow control
  board: Add SE HMIBSC board support
  hmibsc: Enable RAUC support
  hmibsc: enable U-Boot Environment variables protection
  hmibsc: Enable LAN75XX USB ethernet driver

 arch/arm/dts/Makefile              |   1 +
 arch/arm/dts/hmibsc-uboot.dtsi     |  43 +++++++
 arch/arm/dts/hmibsc.dts            | 188 +++++++++++++++++++++++++++++
 arch/arm/mach-snapdragon/Kconfig   |  18 +++
 arch/arm/mach-snapdragon/Makefile  |   1 +
 board/schneider/hmibsc/Kconfig     |  15 +++
 board/schneider/hmibsc/MAINTAINERS |   6 +
 board/schneider/hmibsc/Makefile    |   5 +
 board/schneider/hmibsc/hmibsc.c    | 179 +++++++++++++++++++++++++++
 board/schneider/hmibsc/hmibsc.env  |  11 ++
 configs/hmibsc_defconfig           |  79 ++++++++++++
 drivers/clk/qcom/clock-apq8016.c   |  44 ++++++-
 drivers/serial/serial_msm.c        |  23 +++-
 drivers/usb/host/Kconfig           |   1 +
 include/configs/hmibsc.h           |  59 +++++++++
 15 files changed, 665 insertions(+), 8 deletions(-)
 create mode 100644 arch/arm/dts/hmibsc-uboot.dtsi
 create mode 100644 arch/arm/dts/hmibsc.dts
 create mode 100644 board/schneider/hmibsc/Kconfig
 create mode 100644 board/schneider/hmibsc/MAINTAINERS
 create mode 100644 board/schneider/hmibsc/Makefile
 create mode 100644 board/schneider/hmibsc/hmibsc.c
 create mode 100644 board/schneider/hmibsc/hmibsc.env
 create mode 100644 configs/hmibsc_defconfig
 create mode 100644 include/configs/hmibsc.h

-- 
2.34.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/7] clk: apq8016: Add support for UART1 clocks
  2023-12-18  7:24 [PATCH 0/7] Add SE HMBSC board support Sumit Garg
@ 2023-12-18  7:24 ` Sumit Garg
  2023-12-18  7:24 ` [PATCH 2/7] serial_msm: Add support for RS232 GPIOs Sumit Garg
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Sumit Garg @ 2023-12-18  7:24 UTC (permalink / raw)
  To: u-boot
  Cc: caleb.connolly, neil.armstrong, lukma, seanga2, marex,
	laetitia.mariottini, pascal.eberhard, abdou.saker, jimmy.lalande,
	benjamin.missey, daniel.thompson, Sumit Garg

SE HMIBSC board uses UART1 as the main debug console, so add
corresponding clocks support.

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
 drivers/clk/qcom/clock-apq8016.c | 44 ++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/qcom/clock-apq8016.c b/drivers/clk/qcom/clock-apq8016.c
index c0ce570edc7..fb663d2e92e 100644
--- a/drivers/clk/qcom/clock-apq8016.c
+++ b/drivers/clk/qcom/clock-apq8016.c
@@ -42,6 +42,14 @@
 #define BLSP1_UART2_APPS_N		(0x3040)
 #define BLSP1_UART2_APPS_D		(0x3044)
 
+#define BLSP1_UART1_BCR			(0x2038)
+#define BLSP1_UART1_APPS_CBCR		(0x203C)
+#define BLSP1_UART1_APPS_CMD_RCGR	(0x2044)
+#define BLSP1_UART1_APPS_CFG_RCGR	(0x2048)
+#define BLSP1_UART1_APPS_M		(0x204C)
+#define BLSP1_UART1_APPS_N		(0x2050)
+#define BLSP1_UART1_APPS_D		(0x2054)
+
 /* GPLL0 clock control registers */
 #define GPLL0_STATUS_ACTIVE BIT(17)
 
@@ -93,6 +101,33 @@ static int clk_init_sdc(struct msm_clk_priv *priv, int slot, uint rate)
 	return rate;
 }
 
+static const struct bcr_regs uart1_regs = {
+	.cfg_rcgr = BLSP1_UART1_APPS_CFG_RCGR,
+	.cmd_rcgr = BLSP1_UART1_APPS_CMD_RCGR,
+	.M = BLSP1_UART1_APPS_M,
+	.N = BLSP1_UART1_APPS_N,
+	.D = BLSP1_UART1_APPS_D,
+};
+
+/* UART: 115200 */
+static int clk_init_uart1(struct msm_clk_priv *priv)
+{
+	/* Enable AHB clock */
+	clk_enable_vote_clk(priv->base, &gcc_blsp1_ahb_clk);
+
+	/* 7372800 uart block clock @ GPLL0 */
+	clk_rcg_set_rate_mnd(priv->base, &uart1_regs, 1, 144, 15625,
+			     CFG_CLK_SRC_GPLL0, 16);
+
+	/* Vote for gpll0 clock */
+	clk_enable_gpll0(priv->base, &gpll0_vote_clk);
+
+	/* Enable core clk */
+	clk_enable_cbc(priv->base + BLSP1_UART1_APPS_CBCR);
+
+	return 0;
+}
+
 static const struct bcr_regs uart2_regs = {
 	.cfg_rcgr = BLSP1_UART2_APPS_CFG_RCGR,
 	.cmd_rcgr = BLSP1_UART2_APPS_CMD_RCGR,
@@ -102,7 +137,7 @@ static const struct bcr_regs uart2_regs = {
 };
 
 /* UART: 115200 */
-static int clk_init_uart(struct msm_clk_priv *priv)
+static int clk_init_uart2(struct msm_clk_priv *priv)
 {
 	/* Enable AHB clock */
 	clk_enable_vote_clk(priv->base, &gcc_blsp1_ahb_clk);
@@ -127,13 +162,12 @@ static ulong apq8016_clk_set_rate(struct clk *clk, ulong rate)
 	switch (clk->id) {
 	case 0: /* SDC1 */
 		return clk_init_sdc(priv, 0, rate);
-		break;
 	case 1: /* SDC2 */
 		return clk_init_sdc(priv, 1, rate);
-		break;
 	case 4: /* UART2 */
-		return clk_init_uart(priv);
-		break;
+		return clk_init_uart2(priv);
+	case 5: /* UART1 */
+		return clk_init_uart1(priv);
 	default:
 		return 0;
 	}
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/7] serial_msm: Add support for RS232 GPIOs
  2023-12-18  7:24 [PATCH 0/7] Add SE HMBSC board support Sumit Garg
  2023-12-18  7:24 ` [PATCH 1/7] clk: apq8016: Add support for UART1 clocks Sumit Garg
@ 2023-12-18  7:24 ` Sumit Garg
  2023-12-19 16:47   ` Caleb Connolly
  2023-12-18  7:24 ` [PATCH 3/7] serial_msm: Enable RS232 flow control Sumit Garg
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Sumit Garg @ 2023-12-18  7:24 UTC (permalink / raw)
  To: u-boot
  Cc: caleb.connolly, neil.armstrong, lukma, seanga2, marex,
	laetitia.mariottini, pascal.eberhard, abdou.saker, jimmy.lalande,
	benjamin.missey, daniel.thompson, Sumit Garg

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
 drivers/serial/serial_msm.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c
index a22623c316e..43e58595dc2 100644
--- a/drivers/serial/serial_msm.c
+++ b/drivers/serial/serial_msm.c
@@ -16,6 +16,7 @@
 #include <serial.h>
 #include <watchdog.h>
 #include <asm/global_data.h>
+#include <asm/gpio.h>
 #include <asm/io.h>
 #include <linux/compiler.h>
 #include <linux/delay.h>
@@ -210,6 +211,7 @@ static int msm_serial_probe(struct udevice *dev)
 {
 	int ret;
 	struct msm_serial_data *priv = dev_get_priv(dev);
+	struct gpio_desc rs232_0, rs232_1;
 
 	/* No need to reinitialize the UART after relocation */
 	if (gd->flags & GD_FLG_RELOC)
@@ -219,6 +221,11 @@ static int msm_serial_probe(struct udevice *dev)
 	if (ret)
 		return ret;
 
+	if (!gpio_request_by_name(dev, "gpios", 0, &rs232_0, GPIOD_IS_OUT))
+		dm_gpio_set_value(&rs232_0, 1);
+	if (!gpio_request_by_name(dev, "gpios", 1, &rs232_1, GPIOD_IS_OUT))
+		dm_gpio_set_value(&rs232_1, 0);
+
 	pinctrl_select_state(dev, "uart");
 	uart_dm_init(priv);
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/7] serial_msm: Enable RS232 flow control
  2023-12-18  7:24 [PATCH 0/7] Add SE HMBSC board support Sumit Garg
  2023-12-18  7:24 ` [PATCH 1/7] clk: apq8016: Add support for UART1 clocks Sumit Garg
  2023-12-18  7:24 ` [PATCH 2/7] serial_msm: Add support for RS232 GPIOs Sumit Garg
@ 2023-12-18  7:24 ` Sumit Garg
  2023-12-18  7:24 ` [PATCH 4/7] board: Add SE HMIBSC board support Sumit Garg
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Sumit Garg @ 2023-12-18  7:24 UTC (permalink / raw)
  To: u-boot
  Cc: caleb.connolly, neil.armstrong, lukma, seanga2, marex,
	laetitia.mariottini, pascal.eberhard, abdou.saker, jimmy.lalande,
	benjamin.missey, daniel.thompson, Sumit Garg

SE HMIBSC board debug console requires RS232 flow control, so enable
corresponding support if RS232 gpios are present.

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
 drivers/serial/serial_msm.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c
index 43e58595dc2..a38bf229f06 100644
--- a/drivers/serial/serial_msm.c
+++ b/drivers/serial/serial_msm.c
@@ -35,6 +35,8 @@
 #define UARTDM_MR2				 0x04
 #define UARTDM_CSR				 0xA0
 
+#define MSM_UART_MR1_RX_RDY_CTL		BIT(7)
+
 #define UARTDM_SR                0xA4 /* Status register */
 #define UARTDM_SR_RX_READY       (1 << 0) /* Word is the receiver FIFO */
 #define UARTDM_SR_TX_EMPTY       (1 << 3) /* Transmitter underrun */
@@ -193,13 +195,18 @@ static int msm_uart_clk_init(struct udevice *dev)
 	return 0;
 }
 
-static void uart_dm_init(struct msm_serial_data *priv)
+static void uart_dm_init(struct msm_serial_data *priv, bool is_rs232)
 {
 	/* Delay initialization for a bit to let pins stabilize if necessary */
 	mdelay(5);
 
 	writel(priv->clk_bit_rate, priv->base + UARTDM_CSR);
-	writel(0x0, priv->base + UARTDM_MR1);
+
+	if (is_rs232)
+		writel(MSM_UART_MR1_RX_RDY_CTL, priv->base + UARTDM_MR1);
+	else
+		writel(0x0, priv->base + UARTDM_MR1);
+
 	writel(MSM_BOOT_UART_DM_8_N_1_MODE, priv->base + UARTDM_MR2);
 	writel(MSM_BOOT_UART_DM_CMD_RESET_RX, priv->base + UARTDM_CR);
 	writel(MSM_BOOT_UART_DM_CMD_RESET_TX, priv->base + UARTDM_CR);
@@ -212,6 +219,7 @@ static int msm_serial_probe(struct udevice *dev)
 	int ret;
 	struct msm_serial_data *priv = dev_get_priv(dev);
 	struct gpio_desc rs232_0, rs232_1;
+	bool is_rs232 = false;
 
 	/* No need to reinitialize the UART after relocation */
 	if (gd->flags & GD_FLG_RELOC)
@@ -221,13 +229,15 @@ static int msm_serial_probe(struct udevice *dev)
 	if (ret)
 		return ret;
 
-	if (!gpio_request_by_name(dev, "gpios", 0, &rs232_0, GPIOD_IS_OUT))
+	if (!gpio_request_by_name(dev, "gpios", 0, &rs232_0, GPIOD_IS_OUT)) {
 		dm_gpio_set_value(&rs232_0, 1);
+		is_rs232 = true;
+	}
 	if (!gpio_request_by_name(dev, "gpios", 1, &rs232_1, GPIOD_IS_OUT))
 		dm_gpio_set_value(&rs232_1, 0);
 
 	pinctrl_select_state(dev, "uart");
-	uart_dm_init(priv);
+	uart_dm_init(priv, is_rs232);
 
 	return 0;
 }
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 4/7] board: Add SE HMIBSC board support
  2023-12-18  7:24 [PATCH 0/7] Add SE HMBSC board support Sumit Garg
                   ` (2 preceding siblings ...)
  2023-12-18  7:24 ` [PATCH 3/7] serial_msm: Enable RS232 flow control Sumit Garg
@ 2023-12-18  7:24 ` Sumit Garg
  2023-12-18  7:24 ` [PATCH 5/7] hmibsc: Enable RAUC support Sumit Garg
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Sumit Garg @ 2023-12-18  7:24 UTC (permalink / raw)
  To: u-boot
  Cc: caleb.connolly, neil.armstrong, lukma, seanga2, marex,
	laetitia.mariottini, pascal.eberhard, abdou.saker, jimmy.lalande,
	benjamin.missey, daniel.thompson, Sumit Garg

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

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
 arch/arm/dts/Makefile              |   1 +
 arch/arm/dts/hmibsc-uboot.dtsi     |  43 +++++++
 arch/arm/dts/hmibsc.dts            | 188 +++++++++++++++++++++++++++++
 arch/arm/mach-snapdragon/Kconfig   |  18 +++
 arch/arm/mach-snapdragon/Makefile  |   1 +
 board/schneider/hmibsc/Kconfig     |  15 +++
 board/schneider/hmibsc/MAINTAINERS |   6 +
 board/schneider/hmibsc/Makefile    |   5 +
 board/schneider/hmibsc/hmibsc.c    | 179 +++++++++++++++++++++++++++
 board/schneider/hmibsc/hmibsc.env  |  11 ++
 configs/hmibsc_defconfig           |  73 +++++++++++
 drivers/usb/host/Kconfig           |   1 +
 include/configs/hmibsc.h           |  19 +++
 13 files changed, 560 insertions(+)
 create mode 100644 arch/arm/dts/hmibsc-uboot.dtsi
 create mode 100644 arch/arm/dts/hmibsc.dts
 create mode 100644 board/schneider/hmibsc/Kconfig
 create mode 100644 board/schneider/hmibsc/MAINTAINERS
 create mode 100644 board/schneider/hmibsc/Makefile
 create mode 100644 board/schneider/hmibsc/hmibsc.c
 create mode 100644 board/schneider/hmibsc/hmibsc.env
 create mode 100644 configs/hmibsc_defconfig
 create mode 100644 include/configs/hmibsc.h

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 55aceb51cdb..59cfc318400 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -601,6 +601,7 @@ dtb-$(CONFIG_TARGET_SL28) += fsl-ls1028a-kontron-sl28.dtb \
 dtb-$(CONFIG_TARGET_TEN64) += fsl-ls1088a-ten64.dtb
 
 dtb-$(CONFIG_TARGET_DRAGONBOARD410C) += dragonboard410c.dtb
+dtb-$(CONFIG_TARGET_HMIBSC) += hmibsc.dtb
 dtb-$(CONFIG_TARGET_DRAGONBOARD820C) += dragonboard820c.dtb
 dtb-$(CONFIG_TARGET_STARQLTECHN) += starqltechn.dtb
 dtb-$(CONFIG_TARGET_QCS404EVB) += qcs404-evb.dtb
diff --git a/arch/arm/dts/hmibsc-uboot.dtsi b/arch/arm/dts/hmibsc-uboot.dtsi
new file mode 100644
index 00000000000..1629e9263f5
--- /dev/null
+++ b/arch/arm/dts/hmibsc-uboot.dtsi
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * U-Boot addition to handle HMIBSC pins
+ *
+ * (C) Copyright 2022 Sumit Garg <sumit.garg@linaro.org>
+ */
+
+/ {
+
+	smem {
+		bootph-all;
+	};
+
+	soc {
+		bootph-all;
+
+		pinctrl@1000000 {
+			bootph-all;
+
+			uart {
+				bootph-all;
+			};
+		};
+
+		qcom,gcc@1800000 {
+			bootph-all;
+		};
+
+		serial@78af000 {
+			bootph-all;
+		};
+	};
+};
+
+&pm8916_gpios {
+	usb_hub_reset_pm {
+		gpios = <&pm8916_gpios 2 0>;
+	};
+
+	usb_sw_sel_pm {
+		gpios = <&pm8916_gpios 3 0>;
+	};
+};
diff --git a/arch/arm/dts/hmibsc.dts b/arch/arm/dts/hmibsc.dts
new file mode 100644
index 00000000000..5b01870cb90
--- /dev/null
+++ b/arch/arm/dts/hmibsc.dts
@@ -0,0 +1,188 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Qualcomm APQ8016 based HMIBSC board device tree source
+ *
+ * (C) Copyright 2023 Sumit Garg <sumit.garg@linaro.org>
+ */
+
+/dts-v1/;
+
+#include "skeleton64.dtsi"
+#include <dt-bindings/gpio/gpio.h>
+
+/ {
+	model = "Schneider Electric HMIBSC";
+	compatible = "se,hmibsc", "qcom,apq8016-sbc";
+	qcom,msm-id = <0xce 0x0 0xf8 0x0 0xf9 0x0 0xfa 0x0 0xf7 0x0>;
+	qcom,board-id = <0x10018 0x0>;
+	#address-cells = <0x2>;
+	#size-cells = <0x2>;
+
+	aliases {
+		usb0 = "/soc/ehci@78d9000";
+	};
+
+	memory {
+		device_type = "memory";
+		reg = <0 0x80000000 0 0x3da00000>;
+	};
+
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		smem_mem: smem_region@86300000 {
+			reg = <0x0 0x86300000 0x0 0x100000>;
+			no-map;
+		};
+	};
+
+	chosen {
+		stdout-path = "/soc/serial@78af000";
+	};
+
+	smem {
+		compatible = "qcom,smem";
+		memory-region = <&smem_mem>;
+		qcom,rpm-msg-ram = <&rpm_msg_ram>;
+	};
+
+	soc {
+		#address-cells = <0x1>;
+		#size-cells = <0x1>;
+		ranges = <0x0 0x0 0x0 0xffffffff>;
+		compatible = "simple-bus";
+
+		rpm_msg_ram: memory@60000 {
+			compatible = "qcom,rpm-msg-ram";
+			reg = <0x60000 0x8000>;
+		};
+
+		soc_gpios: pinctrl@1000000 {
+			compatible = "qcom,msm8916-pinctrl";
+			reg = <0x1000000 0x400000>;
+			gpio-controller;
+			gpio-count = <122>;
+			gpio-bank-name="soc";
+			#gpio-cells = <2>;
+
+			blsp1_uart: uart {
+				function = "blsp1_uart";
+				pins = "GPIO_0", "GPIO_1", "GPIO_2", "GPIO_3";
+				drive-strength = <8>;
+				bias-disable;
+			};
+		};
+		clkc: qcom,gcc@1800000 {
+			compatible = "qcom,gcc-apq8016";
+			reg = <0x1800000 0x80000>;
+			#address-cells = <0x1>;
+			#size-cells = <0x0>;
+		};
+
+		serial@78af000 {
+			compatible = "qcom,msm-uartdm-v1.4";
+			reg = <0x78af000 0x200>;
+			clock = <&clkc 5>;
+			pinctrl-names = "uart";
+			pinctrl-0 = <&blsp1_uart>;
+			gpios = <&soc_gpios 99 GPIO_ACTIVE_HIGH>,
+				<&soc_gpios 100 GPIO_ACTIVE_HIGH>;
+		};
+
+		ehci@78d9000 {
+			compatible = "qcom,ehci-host";
+			reg = <0x78d9000 0x400>;
+			phys = <&ehci_phy>;
+		};
+
+		ehci_phy: ehci_phy@78d9000 {
+			compatible = "qcom,apq8016-usbphy";
+			reg = <0x78d9000 0x400>;
+			#phy-cells = <0>;
+		};
+
+		sdhci@07824000 {
+			compatible = "qcom,sdhci-msm-v4";
+			reg = <0x7824900 0x11c 0x7824000 0x800>;
+			bus-width = <0x8>;
+			index = <0x0>;
+			non-removable;
+			clock = <&clkc 0>;
+			clock-frequency = <100000000>;
+		};
+
+		sdhci@07864000 {
+			compatible = "qcom,sdhci-msm-v4";
+			reg = <0x7864900 0x11c 0x7864000 0x800>;
+			index = <0x1>;
+			bus-width = <0x4>;
+			clock = <&clkc 1>;
+			clock-frequency = <200000000>;
+			cd-gpios = <&soc_gpios 38 GPIO_ACTIVE_LOW>;
+		};
+
+		wcnss {
+			bt {
+				compatible="qcom,wcnss-bt";
+			};
+
+			wifi {
+				compatible="qcom,wcnss-wlan";
+			};
+		};
+
+		spmi_bus: spmi@200f000 {
+			compatible = "qcom,spmi-pmic-arb";
+			reg = <0x0200f000 0x001000>,
+			      <0x02400000 0x400000>,
+			      <0x02c00000 0x400000>,
+			      <0x03800000 0x200000>,
+			      <0x0200a000 0x002100>;
+			reg-names = "core", "chnls", "obsrvr", "intr", "cnfg";
+			#address-cells = <0x1>;
+			#size-cells = <0x1>;
+			pmic0: pm8916@0 {
+				compatible = "qcom,spmi-pmic";
+				reg = <0x0 0x1>;
+				#address-cells = <0x1>;
+				#size-cells = <0x1>;
+
+				pon@800 {
+					compatible = "qcom,pm8916-pon";
+					reg = <0x800 0x100>;
+					mode-bootloader = <0x2>;
+					mode-recovery = <0x1>;
+
+					pwrkey {
+						compatible = "qcom,pm8941-pwrkey";
+						debounce = <15625>;
+						bias-pull-up;
+					};
+
+					pm8916_resin: resin {
+						compatible = "qcom,pm8941-resin";
+						debounce = <15625>;
+						bias-pull-up;
+					};
+				};
+
+				pm8916_gpios: pm8916_gpios@c000 {
+					compatible = "qcom,pm8916-gpio";
+					reg = <0xc000 0x400>;
+					gpio-controller;
+					gpio-ranges = <&pm8916_gpios 0 0 4>;
+					#gpio-cells = <2>;
+				};
+			};
+
+			pmic1: pm8916@1 {
+				compatible = "qcom,spmi-pmic";
+				reg = <0x1 0x1>;
+			};
+		};
+	};
+};
+
+#include "hmibsc-uboot.dtsi"
diff --git a/arch/arm/mach-snapdragon/Kconfig b/arch/arm/mach-snapdragon/Kconfig
index ad667108191..d1312f1c2ab 100644
--- a/arch/arm/mach-snapdragon/Kconfig
+++ b/arch/arm/mach-snapdragon/Kconfig
@@ -43,6 +43,23 @@ config TARGET_DRAGONBOARD410C
 	  - HDMI
 	  - 20-pin low speed and 40-pin high speed expanders, 4 LED, 3 buttons
 
+config TARGET_HMIBSC
+	bool "Schneider Electric HMIBSC"
+	select BOARD_LATE_INIT
+	select ENABLE_ARM_SOC_BOOT0_HOOK
+	imply CLK_QCOM_APQ8016
+	imply PINCTRL_QCOM_APQ8016
+	imply BUTTON_QCOM_PMIC
+	help
+	  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
+
 config TARGET_DRAGONBOARD820C
 	bool "96Boards Dragonboard 820C"
 	imply CLK_QCOM_APQ8096
@@ -93,6 +110,7 @@ config TARGET_QCS404EVB
 endchoice
 
 source "board/qualcomm/dragonboard410c/Kconfig"
+source "board/schneider/hmibsc/Kconfig"
 source "board/qualcomm/dragonboard820c/Kconfig"
 source "board/qualcomm/dragonboard845c/Kconfig"
 source "board/samsung/starqltechn/Kconfig"
diff --git a/arch/arm/mach-snapdragon/Makefile b/arch/arm/mach-snapdragon/Makefile
index 3a3a297c176..971196942b7 100644
--- a/arch/arm/mach-snapdragon/Makefile
+++ b/arch/arm/mach-snapdragon/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_SDM845) += sysmap-sdm845.o
 obj-$(CONFIG_SDM845) += init_sdm845.o
 obj-$(CONFIG_TARGET_DRAGONBOARD820C) += sysmap-apq8096.o
 obj-$(CONFIG_TARGET_DRAGONBOARD410C) += sysmap-apq8016.o
+obj-$(CONFIG_TARGET_HMIBSC) += sysmap-apq8016.o
 obj-y += misc.o
 obj-y += dram.o
 obj-$(CONFIG_TARGET_QCS404EVB) += sysmap-qcs404.o
diff --git a/board/schneider/hmibsc/Kconfig b/board/schneider/hmibsc/Kconfig
new file mode 100644
index 00000000000..f093350b868
--- /dev/null
+++ b/board/schneider/hmibsc/Kconfig
@@ -0,0 +1,15 @@
+if TARGET_HMIBSC
+
+config SYS_BOARD
+	default "hmibsc"
+
+config SYS_VENDOR
+	default "schneider"
+
+config SYS_SOC
+	default "apq8016"
+
+config SYS_CONFIG_NAME
+	default "hmibsc"
+
+endif
diff --git a/board/schneider/hmibsc/MAINTAINERS b/board/schneider/hmibsc/MAINTAINERS
new file mode 100644
index 00000000000..0f31bbda966
--- /dev/null
+++ b/board/schneider/hmibsc/MAINTAINERS
@@ -0,0 +1,6 @@
+HMIBSC BOARD
+M:	Sumit Garg <sumit.garg@linaro.org>
+S:	Maintained
+F:	board/schneider/hmibsc/
+F:	include/configs/hmibsc.h
+F:	configs/hmibsc_defconfig
diff --git a/board/schneider/hmibsc/Makefile b/board/schneider/hmibsc/Makefile
new file mode 100644
index 00000000000..7c9edd6a90d
--- /dev/null
+++ b/board/schneider/hmibsc/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# (C) Copyright 2023 Sumit Garg <sumit.garg@linaro.org>
+
+obj-y	:= hmibsc.o
diff --git a/board/schneider/hmibsc/hmibsc.c b/board/schneider/hmibsc/hmibsc.c
new file mode 100644
index 00000000000..b2d506b528b
--- /dev/null
+++ b/board/schneider/hmibsc/hmibsc.c
@@ -0,0 +1,179 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Board init file for SE HMIBSC
+ *
+ * (C) Copyright 2023 Sumit Garg <sumit.garg@linaro.org>
+ */
+
+#include <button.h>
+#include <cpu_func.h>
+#include <dm.h>
+#include <env.h>
+#include <init.h>
+#include <net.h>
+#include <usb.h>
+#include <asm/cache.h>
+#include <asm/global_data.h>
+#include <asm/gpio.h>
+#include <fdt_support.h>
+#include <asm/arch/dram.h>
+#include <asm/arch/misc.h>
+#include <linux/delay.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+int dram_init(void)
+{
+	gd->ram_size = PHYS_SDRAM_1_SIZE;
+
+	return 0;
+}
+
+int dram_init_banksize(void)
+{
+	gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
+	gd->bd->bi_dram[0].size = PHYS_SDRAM_1_SIZE;
+
+	return 0;
+}
+
+int board_usb_init(int index, enum usb_init_type init)
+{
+	static struct udevice *pmic_gpio;
+	static struct gpio_desc hub_reset, usb_sel;
+	int ret = 0, node;
+
+	if (!pmic_gpio) {
+		ret = uclass_get_device_by_name(UCLASS_GPIO,
+						"pm8916_gpios@c000",
+						&pmic_gpio);
+		if (ret < 0) {
+			printf("Failed to find pm8916_gpios@c000 node.\n");
+			return ret;
+		}
+	}
+
+	/* Try to request gpios needed to start usb host on dragonboard */
+	if (!dm_gpio_is_valid(&hub_reset)) {
+		node = fdt_subnode_offset(gd->fdt_blob,
+					  dev_of_offset(pmic_gpio),
+					  "usb_hub_reset_pm");
+		if (node < 0) {
+			printf("Failed to find usb_hub_reset_pm dt node.\n");
+			return node;
+		}
+		ret = gpio_request_by_name_nodev(offset_to_ofnode(node),
+						 "gpios", 0, &hub_reset, 0);
+		if (ret < 0) {
+			printf("Failed to request usb_hub_reset_pm gpio.\n");
+			return ret;
+		}
+	}
+
+	if (!dm_gpio_is_valid(&usb_sel)) {
+		node = fdt_subnode_offset(gd->fdt_blob,
+					  dev_of_offset(pmic_gpio),
+					  "usb_sw_sel_pm");
+		if (node < 0) {
+			printf("Failed to find usb_sw_sel_pm dt node.\n");
+			return 0;
+		}
+		ret = gpio_request_by_name_nodev(offset_to_ofnode(node),
+						 "gpios", 0, &usb_sel, 0);
+		if (ret < 0) {
+			printf("Failed to request usb_sw_sel_pm gpio.\n");
+			return ret;
+		}
+	}
+
+	if (init == USB_INIT_HOST) {
+		/* Start USB Hub */
+		dm_gpio_set_dir_flags(&hub_reset,
+				      GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE);
+		mdelay(100);
+		/* Switch usb to host connectors */
+		dm_gpio_set_dir_flags(&usb_sel,
+				      GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE);
+		mdelay(100);
+	} else { /* Device */
+		/* Disable hub */
+		dm_gpio_set_dir_flags(&hub_reset, GPIOD_IS_OUT);
+		/* Switch back to device connector */
+		dm_gpio_set_dir_flags(&usb_sel, GPIOD_IS_OUT);
+	}
+
+	return 0;
+}
+
+/* Check for vol- button - if pressed - stop autoboot */
+int misc_init_r(void)
+{
+	struct udevice *btn;
+	int ret;
+	enum button_state_t state;
+
+	ret = button_get_by_label("vol_down", &btn);
+	if (ret < 0) {
+		printf("Couldn't find power button!\n");
+		return ret;
+	}
+
+	state = button_get_state(btn);
+	if (state == BUTTON_ON) {
+		env_set("preboot", "setenv preboot; fastboot 0");
+		printf("vol_down pressed - Starting fastboot.\n");
+	}
+
+	return 0;
+}
+
+int board_init(void)
+{
+	return 0;
+}
+
+int board_late_init(void)
+{
+	return 0;
+}
+
+/*
+ * Fixup of DTB for Linux Kernel
+ * 1. Fixup installed DRAM.
+ * 2. Fixup WLAN/BT Mac address:
+ *	First, check if MAC addresses for WLAN/BT exists as environemnt
+ *	variables wlanaddr,btaddr. if not, generate a unique address.
+ */
+
+int ft_board_setup(void *blob, struct bd_info *bd)
+{
+	u8 mac[ARP_HLEN];
+
+	msm_fixup_memory(blob);
+
+	if (!eth_env_get_enetaddr("wlanaddr", mac)) {
+		msm_generate_mac_addr(mac);
+	};
+
+	do_fixup_by_compat(blob, "qcom,wcnss-wlan",
+			   "local-mac-address", mac, ARP_HLEN, 1);
+
+	if (!eth_env_get_enetaddr("btaddr", mac)) {
+		msm_generate_mac_addr(mac);
+
+		/*
+		 * The BD address is same as WLAN MAC address but with
+		 * least significant bit flipped.
+		 */
+		mac[0] ^= 0x01;
+	};
+
+	do_fixup_by_compat(blob, "qcom,wcnss-bt",
+			   "local-bd-address", mac, ARP_HLEN, 1);
+	return 0;
+}
+
+void reset_cpu(void)
+{
+	psci_system_reset();
+}
diff --git a/board/schneider/hmibsc/hmibsc.env b/board/schneider/hmibsc/hmibsc.env
new file mode 100644
index 00000000000..692722058f5
--- /dev/null
+++ b/board/schneider/hmibsc/hmibsc.env
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+loadaddr=0x90000000
+initrd_high=0xffffffffffffffff
+linux_image=Image
+kernel_addr_r=0x81000000
+fdtfile=qcom/hmibsc.dtb
+fdt_addr_r=0x83000000
+ramdisk_addr_r=0x84000000
+scriptaddr=0x90000000
+pxefile_addr_r=0x90100000
diff --git a/configs/hmibsc_defconfig b/configs/hmibsc_defconfig
new file mode 100644
index 00000000000..cef9e304b3a
--- /dev/null
+++ b/configs/hmibsc_defconfig
@@ -0,0 +1,73 @@
+CONFIG_ARM=y
+CONFIG_COUNTER_FREQUENCY=19000000
+CONFIG_ARCH_SNAPDRAGON=y
+CONFIG_TEXT_BASE=0x8f600000
+CONFIG_SYS_MALLOC_LEN=0x802000
+CONFIG_NR_DRAM_BANKS=1
+CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x8007fff0
+CONFIG_ENV_SIZE=0x2000
+CONFIG_ENV_OFFSET=0x0
+CONFIG_TARGET_HMIBSC=y
+CONFIG_DEFAULT_DEVICE_TREE="hmibsc"
+CONFIG_OF_LIBFDT_OVERLAY=y
+CONFIG_IDENT_STRING="\nSchneider Electric-HMIBSC"
+CONFIG_SYS_LOAD_ADDR=0x80080000
+CONFIG_REMAKE_ELF=y
+# CONFIG_ANDROID_BOOT_IMAGE is not set
+CONFIG_FIT=y
+CONFIG_DISTRO_DEFAULTS=y
+CONFIG_OF_BOARD_SETUP=y
+CONFIG_USE_PREBOOT=y
+# CONFIG_DISPLAY_CPUINFO is not set
+# CONFIG_DISPLAY_BOARDINFO is not set
+CONFIG_MISC_INIT_R=y
+CONFIG_SYS_PROMPT="hmibsc => "
+CONFIG_SYS_MAXARGS=64
+CONFIG_SYS_CBSIZE=512
+CONFIG_SYS_PBSIZE=548
+# CONFIG_CMD_IMI is not set
+CONFIG_CMD_MD5SUM=y
+CONFIG_CMD_MEMINFO=y
+CONFIG_CMD_GPIO=y
+CONFIG_CMD_GPT=y
+CONFIG_CMD_MMC=y
+CONFIG_CMD_USB=y
+CONFIG_BOOTP_BOOTFILESIZE=y
+CONFIG_CMD_CACHE=y
+CONFIG_CMD_TIMER=y
+CONFIG_ENV_IS_IN_MMC=y
+CONFIG_SYS_RELOC_GD_ENV_ADDR=y
+CONFIG_SYS_MMC_ENV_PART=2
+CONFIG_CLK=y
+CONFIG_USB_FUNCTION_FASTBOOT=y
+CONFIG_FASTBOOT_BUF_ADDR=0x91000000
+CONFIG_FASTBOOT_FLASH=y
+CONFIG_FASTBOOT_FLASH_MMC_DEV=0
+CONFIG_MSM_GPIO=y
+CONFIG_QCOM_PMIC_GPIO=y
+CONFIG_LED=y
+CONFIG_LED_GPIO=y
+CONFIG_MMC_SDHCI=y
+CONFIG_MMC_SDHCI_MSM=y
+CONFIG_PHY=y
+CONFIG_PINCTRL=y
+CONFIG_PINCONF=y
+CONFIG_DM_PMIC=y
+CONFIG_PMIC_QCOM=y
+CONFIG_MSM_SERIAL=y
+CONFIG_SPMI_MSM=y
+CONFIG_USB=y
+CONFIG_USB_EHCI_HCD=y
+CONFIG_USB_EHCI_MSM=y
+CONFIG_USB_ULPI_VIEWPORT=y
+CONFIG_USB_ULPI=y
+CONFIG_USB_HOST_ETHER=y
+CONFIG_USB_ETHER_ASIX=y
+CONFIG_USB_ETHER_ASIX88179=y
+CONFIG_USB_ETHER_MCS7830=y
+CONFIG_USB_ETHER_SMSC95XX=y
+CONFIG_USB_GADGET=y
+CONFIG_USB_GADGET_VENDOR_NUM=0x18d1
+CONFIG_USB_GADGET_PRODUCT_NUM=0xd00d
+CONFIG_CI_UDC=y
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index b501ea514bc..776466978f7 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -266,6 +266,7 @@ config USB_EHCI_MSM
 	depends on DM_USB
 	select USB_ULPI_VIEWPORT
 	select MSM8916_USB_PHY
+	select EHCI_HCD_INIT_AFTER_RESET
 	---help---
 	  Enables support for the on-chip EHCI controller on Qualcomm
 	  Snapdragon SoCs.
diff --git a/include/configs/hmibsc.h b/include/configs/hmibsc.h
new file mode 100644
index 00000000000..04052ed6dee
--- /dev/null
+++ b/include/configs/hmibsc.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Board configuration file for HMIBSC
+ *
+ * (C) Copyright 2023 Sumit Garg <sumit.garg@linaro.org>
+ */
+
+#ifndef __CONFIGS_HMIBSC_H
+#define __CONFIGS_HMIBSC_H
+
+#include <linux/sizes.h>
+
+/* Physical Memory Map */
+#define PHYS_SDRAM_1			0x80000000
+/* Note: 8 MiB (0x86000000 - 0x86800000) are reserved for tz/smem/hyp/rmtfs/rfsa */
+#define PHYS_SDRAM_1_SIZE		SZ_1G
+#define CFG_SYS_SDRAM_BASE		PHYS_SDRAM_1
+
+#endif
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 5/7] hmibsc: Enable RAUC support
  2023-12-18  7:24 [PATCH 0/7] Add SE HMBSC board support Sumit Garg
                   ` (3 preceding siblings ...)
  2023-12-18  7:24 ` [PATCH 4/7] board: Add SE HMIBSC board support Sumit Garg
@ 2023-12-18  7:24 ` Sumit Garg
  2023-12-19 16:30   ` Caleb Connolly
  2023-12-18  7:24 ` [PATCH 6/7] hmibsc: enable U-Boot Environment variables protection Sumit Garg
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Sumit Garg @ 2023-12-18  7:24 UTC (permalink / raw)
  To: u-boot
  Cc: caleb.connolly, neil.armstrong, lukma, seanga2, marex,
	laetitia.mariottini, pascal.eberhard, abdou.saker, jimmy.lalande,
	benjamin.missey, daniel.thompson, Sumit Garg

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
 include/configs/hmibsc.h | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/include/configs/hmibsc.h b/include/configs/hmibsc.h
index 04052ed6dee..b614dec7870 100644
--- a/include/configs/hmibsc.h
+++ b/include/configs/hmibsc.h
@@ -16,4 +16,42 @@
 #define PHYS_SDRAM_1_SIZE		SZ_1G
 #define CFG_SYS_SDRAM_BASE		PHYS_SDRAM_1
 
+#undef CONFIG_BOOTCOMMAND
+# define CONFIG_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"
+
 #endif
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 6/7] hmibsc: enable U-Boot Environment variables protection
  2023-12-18  7:24 [PATCH 0/7] Add SE HMBSC board support Sumit Garg
                   ` (4 preceding siblings ...)
  2023-12-18  7:24 ` [PATCH 5/7] hmibsc: Enable RAUC support Sumit Garg
@ 2023-12-18  7:24 ` Sumit Garg
  2023-12-18  7:24 ` [PATCH 7/7] hmibsc: Enable LAN75XX USB ethernet driver Sumit Garg
  2023-12-18 15:01 ` [PATCH 0/7] Add SE HMBSC board support Simon Glass
  7 siblings, 0 replies; 20+ messages in thread
From: Sumit Garg @ 2023-12-18  7:24 UTC (permalink / raw)
  To: u-boot
  Cc: caleb.connolly, neil.armstrong, lukma, seanga2, marex,
	laetitia.mariottini, pascal.eberhard, abdou.saker, jimmy.lalande,
	benjamin.missey, daniel.thompson, Sumit Garg

Enable Environment protection with:

CONFIG_ENV_WRITEABLE_LIST=y
CONFIG_ENV_ACCESS_IGNORE_FORCE=y

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
 configs/hmibsc_defconfig | 4 ++++
 include/configs/hmibsc.h | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/configs/hmibsc_defconfig b/configs/hmibsc_defconfig
index cef9e304b3a..656c9c6d0d6 100644
--- a/configs/hmibsc_defconfig
+++ b/configs/hmibsc_defconfig
@@ -36,6 +36,10 @@ CONFIG_CMD_USB=y
 CONFIG_BOOTP_BOOTFILESIZE=y
 CONFIG_CMD_CACHE=y
 CONFIG_CMD_TIMER=y
+CONFIG_CMD_ENV_FLAGS=y
+CONFIG_CMD_NVEDIT_INFO=y
+CONFIG_ENV_WRITEABLE_LIST=y
+CONFIG_ENV_ACCESS_IGNORE_FORCE=y
 CONFIG_ENV_IS_IN_MMC=y
 CONFIG_SYS_RELOC_GD_ENV_ADDR=y
 CONFIG_SYS_MMC_ENV_PART=2
diff --git a/include/configs/hmibsc.h b/include/configs/hmibsc.h
index b614dec7870..b228e035429 100644
--- a/include/configs/hmibsc.h
+++ b/include/configs/hmibsc.h
@@ -54,4 +54,6 @@
 	"fi; " \
 	"load ${devtype} ${devnum}:${distro_bootpart} ${loadaddr} /boot/fitImage && bootm"
 
+#define CFG_ENV_FLAGS_LIST_STATIC "BOOT_A_LEFT:dw,BOOT_B_LEFT:dw,BOOT_ORDER:sw"
+
 #endif
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 7/7] hmibsc: Enable LAN75XX USB ethernet driver
  2023-12-18  7:24 [PATCH 0/7] Add SE HMBSC board support Sumit Garg
                   ` (5 preceding siblings ...)
  2023-12-18  7:24 ` [PATCH 6/7] hmibsc: enable U-Boot Environment variables protection Sumit Garg
@ 2023-12-18  7:24 ` Sumit Garg
  2023-12-18 15:01 ` [PATCH 0/7] Add SE HMBSC board support Simon Glass
  7 siblings, 0 replies; 20+ messages in thread
From: Sumit Garg @ 2023-12-18  7:24 UTC (permalink / raw)
  To: u-boot
  Cc: caleb.connolly, neil.armstrong, lukma, seanga2, marex,
	laetitia.mariottini, pascal.eberhard, abdou.saker, jimmy.lalande,
	benjamin.missey, daniel.thompson, Sumit Garg

HMIBSC board has hardwired support for 2 USB host LAN75XX ethernet
adaptors. So enable corresponding driver in u-boot.

Steps to test networking:

hmibsc => setenv bootfile fitImage
hmibsc => setenv serverip 192.168.1.24
hmibsc => usb start
starting USB...
Bus ehci@78d9000: USB EHCI 1.00
scanning bus ehci@78d9000 for devices... 5 USB Device(s) found
       scanning usb for storage devices... 1 Storage Device(s) found
hmibsc => dhcp
lan75xx_eth Waiting for PHY auto negotiation to complete......... TIMEOUT !
phy_startup failed
lan75xx_eth Waiting for PHY auto negotiation to complete....... done
BOOTP broadcast 1
BOOTP broadcast 2
*** Unhandled DHCP Option in OFFER/ACK: 125
*** Unhandled DHCP Option in OFFER/ACK: 125
DHCP client bound to address 192.168.1.22 (6220 ms)
Using lan75xx_eth device
TFTP from server 192.168.1.24; our IP address is 192.168.1.22
Filename 'fitImage'.
Load address: 0x90000000
Loading: #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         #################################################################
         ################################
         5.2 MiB/s
done
Bytes transferred = 30992680 (1d8e928 hex)

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
 configs/hmibsc_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configs/hmibsc_defconfig b/configs/hmibsc_defconfig
index 656c9c6d0d6..dd3d18a5953 100644
--- a/configs/hmibsc_defconfig
+++ b/configs/hmibsc_defconfig
@@ -71,6 +71,8 @@ CONFIG_USB_ETHER_ASIX=y
 CONFIG_USB_ETHER_ASIX88179=y
 CONFIG_USB_ETHER_MCS7830=y
 CONFIG_USB_ETHER_SMSC95XX=y
+CONFIG_PHYLIB=y
+CONFIG_USB_ETHER_LAN75XX=y
 CONFIG_USB_GADGET=y
 CONFIG_USB_GADGET_VENDOR_NUM=0x18d1
 CONFIG_USB_GADGET_PRODUCT_NUM=0xd00d
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/7] Add SE HMBSC board support
  2023-12-18  7:24 [PATCH 0/7] Add SE HMBSC board support Sumit Garg
                   ` (6 preceding siblings ...)
  2023-12-18  7:24 ` [PATCH 7/7] hmibsc: Enable LAN75XX USB ethernet driver Sumit Garg
@ 2023-12-18 15:01 ` Simon Glass
  2023-12-19  6:25   ` Sumit Garg
  7 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2023-12-18 15:01 UTC (permalink / raw)
  To: Sumit Garg
  Cc: u-boot, caleb.connolly, neil.armstrong, lukma, seanga2, marex,
	laetitia.mariottini, pascal.eberhard, abdou.saker, jimmy.lalande,
	benjamin.missey, daniel.thompson

Hi Sumit,

On Mon, 18 Dec 2023 at 00:24, Sumit Garg <sumit.garg@linaro.org> wrote:
>
> SE HMIBSC board is based on Qcom APQ8016 SoC. One of the major

Could you please add a doc/ file for this board and explain how to
build it and how to run U-Boot on it?

> difference from db410c is serial port where HMIBSC board uses UART1 as
> the debug console with an RS232 port, patch #1 - #3 adds corresponding
> driver support.
>
> Patch #4 adds main HMIBSC board specific bits, 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
>
> Patch #5 - #7 enables specific board features like RAUC support,
> environment protection and USB networking support.
>
> This patch series is based on top of Qcom maintainer tree [1] + the latest
> PMIC patch-set [2]. Feedback is very much welcome.
>
> [1] https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commits/u-boot-qcom-next?ref_type=heads
> [2] https://patchwork.ozlabs.org/project/uboot/list/?series=385322
>
> Sumit Garg (7):
>   clk: apq8016: Add support for UART1 clocks
>   serial_msm: Add support for RS232 GPIOs
>   serial_msm: Enable RS232 flow control
>   board: Add SE HMIBSC board support
>   hmibsc: Enable RAUC support
>   hmibsc: enable U-Boot Environment variables protection
>   hmibsc: Enable LAN75XX USB ethernet driver
>
>  arch/arm/dts/Makefile              |   1 +
>  arch/arm/dts/hmibsc-uboot.dtsi     |  43 +++++++
>  arch/arm/dts/hmibsc.dts            | 188 +++++++++++++++++++++++++++++
>  arch/arm/mach-snapdragon/Kconfig   |  18 +++
>  arch/arm/mach-snapdragon/Makefile  |   1 +
>  board/schneider/hmibsc/Kconfig     |  15 +++
>  board/schneider/hmibsc/MAINTAINERS |   6 +
>  board/schneider/hmibsc/Makefile    |   5 +
>  board/schneider/hmibsc/hmibsc.c    | 179 +++++++++++++++++++++++++++
>  board/schneider/hmibsc/hmibsc.env  |  11 ++
>  configs/hmibsc_defconfig           |  79 ++++++++++++
>  drivers/clk/qcom/clock-apq8016.c   |  44 ++++++-
>  drivers/serial/serial_msm.c        |  23 +++-
>  drivers/usb/host/Kconfig           |   1 +
>  include/configs/hmibsc.h           |  59 +++++++++
>  15 files changed, 665 insertions(+), 8 deletions(-)
>  create mode 100644 arch/arm/dts/hmibsc-uboot.dtsi
>  create mode 100644 arch/arm/dts/hmibsc.dts
>  create mode 100644 board/schneider/hmibsc/Kconfig
>  create mode 100644 board/schneider/hmibsc/MAINTAINERS
>  create mode 100644 board/schneider/hmibsc/Makefile
>  create mode 100644 board/schneider/hmibsc/hmibsc.c
>  create mode 100644 board/schneider/hmibsc/hmibsc.env
>  create mode 100644 configs/hmibsc_defconfig
>  create mode 100644 include/configs/hmibsc.h
>
> --
> 2.34.1
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/7] Add SE HMBSC board support
  2023-12-18 15:01 ` [PATCH 0/7] Add SE HMBSC board support Simon Glass
@ 2023-12-19  6:25   ` Sumit Garg
  2023-12-19 16:26     ` Caleb Connolly
  0 siblings, 1 reply; 20+ messages in thread
From: Sumit Garg @ 2023-12-19  6:25 UTC (permalink / raw)
  To: Simon Glass
  Cc: u-boot, caleb.connolly, neil.armstrong, lukma, seanga2, marex,
	laetitia.mariottini, pascal.eberhard, abdou.saker, jimmy.lalande,
	benjamin.missey, daniel.thompson

Hi Simon,

On Mon, 18 Dec 2023 at 20:32, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Sumit,
>
> On Mon, 18 Dec 2023 at 00:24, Sumit Garg <sumit.garg@linaro.org> wrote:
> >
> > SE HMIBSC board is based on Qcom APQ8016 SoC. One of the major
>
> Could you please add a doc/ file for this board and explain how to
> build it and how to run U-Boot on it?

Ah I forgot to add that since the build/boot instructions are quite
similar to db410c. BTW, I will add that in the next spin.

-Sumit

>
> > difference from db410c is serial port where HMIBSC board uses UART1 as
> > the debug console with an RS232 port, patch #1 - #3 adds corresponding
> > driver support.
> >
> > Patch #4 adds main HMIBSC board specific bits, 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
> >
> > Patch #5 - #7 enables specific board features like RAUC support,
> > environment protection and USB networking support.
> >
> > This patch series is based on top of Qcom maintainer tree [1] + the latest
> > PMIC patch-set [2]. Feedback is very much welcome.
> >
> > [1] https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commits/u-boot-qcom-next?ref_type=heads
> > [2] https://patchwork.ozlabs.org/project/uboot/list/?series=385322
> >
> > Sumit Garg (7):
> >   clk: apq8016: Add support for UART1 clocks
> >   serial_msm: Add support for RS232 GPIOs
> >   serial_msm: Enable RS232 flow control
> >   board: Add SE HMIBSC board support
> >   hmibsc: Enable RAUC support
> >   hmibsc: enable U-Boot Environment variables protection
> >   hmibsc: Enable LAN75XX USB ethernet driver
> >
> >  arch/arm/dts/Makefile              |   1 +
> >  arch/arm/dts/hmibsc-uboot.dtsi     |  43 +++++++
> >  arch/arm/dts/hmibsc.dts            | 188 +++++++++++++++++++++++++++++
> >  arch/arm/mach-snapdragon/Kconfig   |  18 +++
> >  arch/arm/mach-snapdragon/Makefile  |   1 +
> >  board/schneider/hmibsc/Kconfig     |  15 +++
> >  board/schneider/hmibsc/MAINTAINERS |   6 +
> >  board/schneider/hmibsc/Makefile    |   5 +
> >  board/schneider/hmibsc/hmibsc.c    | 179 +++++++++++++++++++++++++++
> >  board/schneider/hmibsc/hmibsc.env  |  11 ++
> >  configs/hmibsc_defconfig           |  79 ++++++++++++
> >  drivers/clk/qcom/clock-apq8016.c   |  44 ++++++-
> >  drivers/serial/serial_msm.c        |  23 +++-
> >  drivers/usb/host/Kconfig           |   1 +
> >  include/configs/hmibsc.h           |  59 +++++++++
> >  15 files changed, 665 insertions(+), 8 deletions(-)
> >  create mode 100644 arch/arm/dts/hmibsc-uboot.dtsi
> >  create mode 100644 arch/arm/dts/hmibsc.dts
> >  create mode 100644 board/schneider/hmibsc/Kconfig
> >  create mode 100644 board/schneider/hmibsc/MAINTAINERS
> >  create mode 100644 board/schneider/hmibsc/Makefile
> >  create mode 100644 board/schneider/hmibsc/hmibsc.c
> >  create mode 100644 board/schneider/hmibsc/hmibsc.env
> >  create mode 100644 configs/hmibsc_defconfig
> >  create mode 100644 include/configs/hmibsc.h
> >
> > --
> > 2.34.1
> >
>
> Regards,
> Simon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/7] Add SE HMBSC board support
  2023-12-19  6:25   ` Sumit Garg
@ 2023-12-19 16:26     ` Caleb Connolly
  2023-12-20 13:36       ` Sumit Garg
  0 siblings, 1 reply; 20+ messages in thread
From: Caleb Connolly @ 2023-12-19 16:26 UTC (permalink / raw)
  To: Sumit Garg, Simon Glass
  Cc: u-boot, neil.armstrong, lukma, seanga2, marex,
	laetitia.mariottini, pascal.eberhard, abdou.saker, jimmy.lalande,
	benjamin.missey, daniel.thompson



On 19/12/2023 06:25, Sumit Garg wrote:
> Hi Simon,
> 
> On Mon, 18 Dec 2023 at 20:32, Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi Sumit,
>>
>> On Mon, 18 Dec 2023 at 00:24, Sumit Garg <sumit.garg@linaro.org> wrote:
>>>
>>> SE HMIBSC board is based on Qcom APQ8016 SoC. One of the major
>>
>> Could you please add a doc/ file for this board and explain how to
>> build it and how to run U-Boot on it?
> 
> Ah I forgot to add that since the build/boot instructions are quite
> similar to db410c. BTW, I will add that in the next spin.

Probably just referencing that document or making the language generic
would be good.

Sumit, could you rebase this series on my generic board support? [1] in
it's current form this series conflicts, and includes some of the major
anti-patterns I'm trying to move away from in mach-snapdragon.

You should not have to introduce a new CONFIG_TARGET_XYZ variable, and
from what I can tell you shouldn't even need to add the board/schneider
directory at all, you can just set the following in your defconfig:

CONFIG_SYS_BOARD="dragonboard410c"
CONFIG_SYS_CONFIG_NAME="hmibsc"

This will use the db410c board code (which yours is just a copy/paste of
from what I can tell) and your custom include/configs/hmibsc.h header.

The addresses set in your environment file should be allocated
automatically at runtime too (see the ("mach-snapdragon: dynamic load
addresses") patch).

You should also switch to an upstream board DTS based on my series, and
drop the "-uboot.dtsi" file.

[1]:
https://lore.kernel.org/u-boot/20231219-b4-qcom-common-target-v2-0-b6dd9704219e@linaro.org/

Kind regards,

> 
> -Sumit
> 
>>
>>> difference from db410c is serial port where HMIBSC board uses UART1 as
>>> the debug console with an RS232 port, patch #1 - #3 adds corresponding
>>> driver support.
>>>
>>> Patch #4 adds main HMIBSC board specific bits, 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
>>>
>>> Patch #5 - #7 enables specific board features like RAUC support,
>>> environment protection and USB networking support.
>>>
>>> This patch series is based on top of Qcom maintainer tree [1] + the latest
>>> PMIC patch-set [2]. Feedback is very much welcome.
>>>
>>> [1] https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commits/u-boot-qcom-next?ref_type=heads
>>> [2] https://patchwork.ozlabs.org/project/uboot/list/?series=385322
>>>
>>> Sumit Garg (7):
>>>   clk: apq8016: Add support for UART1 clocks
>>>   serial_msm: Add support for RS232 GPIOs
>>>   serial_msm: Enable RS232 flow control
>>>   board: Add SE HMIBSC board support
>>>   hmibsc: Enable RAUC support
>>>   hmibsc: enable U-Boot Environment variables protection
>>>   hmibsc: Enable LAN75XX USB ethernet driver
>>>
>>>  arch/arm/dts/Makefile              |   1 +
>>>  arch/arm/dts/hmibsc-uboot.dtsi     |  43 +++++++
>>>  arch/arm/dts/hmibsc.dts            | 188 +++++++++++++++++++++++++++++
>>>  arch/arm/mach-snapdragon/Kconfig   |  18 +++
>>>  arch/arm/mach-snapdragon/Makefile  |   1 +
>>>  board/schneider/hmibsc/Kconfig     |  15 +++
>>>  board/schneider/hmibsc/MAINTAINERS |   6 +
>>>  board/schneider/hmibsc/Makefile    |   5 +
>>>  board/schneider/hmibsc/hmibsc.c    | 179 +++++++++++++++++++++++++++
>>>  board/schneider/hmibsc/hmibsc.env  |  11 ++
>>>  configs/hmibsc_defconfig           |  79 ++++++++++++
>>>  drivers/clk/qcom/clock-apq8016.c   |  44 ++++++-
>>>  drivers/serial/serial_msm.c        |  23 +++-
>>>  drivers/usb/host/Kconfig           |   1 +
>>>  include/configs/hmibsc.h           |  59 +++++++++
>>>  15 files changed, 665 insertions(+), 8 deletions(-)
>>>  create mode 100644 arch/arm/dts/hmibsc-uboot.dtsi
>>>  create mode 100644 arch/arm/dts/hmibsc.dts
>>>  create mode 100644 board/schneider/hmibsc/Kconfig
>>>  create mode 100644 board/schneider/hmibsc/MAINTAINERS
>>>  create mode 100644 board/schneider/hmibsc/Makefile
>>>  create mode 100644 board/schneider/hmibsc/hmibsc.c
>>>  create mode 100644 board/schneider/hmibsc/hmibsc.env
>>>  create mode 100644 configs/hmibsc_defconfig
>>>  create mode 100644 include/configs/hmibsc.h
>>>
>>> --
>>> 2.34.1
>>>
>>
>> Regards,
>> Simon

-- 
// Caleb (they/them)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/7] hmibsc: Enable RAUC support
  2023-12-18  7:24 ` [PATCH 5/7] hmibsc: Enable RAUC support Sumit Garg
@ 2023-12-19 16:30   ` Caleb Connolly
  0 siblings, 0 replies; 20+ messages in thread
From: Caleb Connolly @ 2023-12-19 16:30 UTC (permalink / raw)
  To: Sumit Garg, u-boot
  Cc: neil.armstrong, lukma, seanga2, marex, laetitia.mariottini,
	pascal.eberhard, abdou.saker, jimmy.lalande, benjamin.missey,
	daniel.thompson

Hi Sumit,

Please add a description to this patch (what is RAUC?).

On 18/12/2023 07:24, Sumit Garg wrote:
> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> ---
>  include/configs/hmibsc.h | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/include/configs/hmibsc.h b/include/configs/hmibsc.h
> index 04052ed6dee..b614dec7870 100644
> --- a/include/configs/hmibsc.h
> +++ b/include/configs/hmibsc.h
> @@ -16,4 +16,42 @@
>  #define PHYS_SDRAM_1_SIZE		SZ_1G
>  #define CFG_SYS_SDRAM_BASE		PHYS_SDRAM_1
>  
> +#undef CONFIG_BOOTCOMMAND
> +# define CONFIG_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"
> +
>  #endif

-- 
// Caleb (they/them)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/7] serial_msm: Add support for RS232 GPIOs
  2023-12-18  7:24 ` [PATCH 2/7] serial_msm: Add support for RS232 GPIOs Sumit Garg
@ 2023-12-19 16:47   ` Caleb Connolly
  2023-12-20 14:09     ` Sumit Garg
  0 siblings, 1 reply; 20+ messages in thread
From: Caleb Connolly @ 2023-12-19 16:47 UTC (permalink / raw)
  To: Sumit Garg, u-boot
  Cc: neil.armstrong, lukma, seanga2, marex, laetitia.mariottini,
	pascal.eberhard, abdou.saker, jimmy.lalande, benjamin.missey,
	daniel.thompson

I guess these GPIOs are RTS/CTS? Missing description...

Given that you just set them to their respective values, you should be
able to configure these using pinctrl with this patch [1], so we can
avoid adding these non-standard bindings.

I tested your next patch, setting "MSM_UART_MR1_RX_RDY_CTL" in
uart_dm_init() on db410c and db820c and I don't notice any issues with
setting that bit unconditionally. If it's definitely needed for your
board then I'd be ok with a patch to always set it with a comment
offering some context.

[1]:
https://lore.kernel.org/u-boot/20231219-b4-qcom-common-target-v2-10-b6dd9704219e@linaro.org/

On 18/12/2023 07:24, Sumit Garg wrote:
> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> ---
>  drivers/serial/serial_msm.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c
> index a22623c316e..43e58595dc2 100644
> --- a/drivers/serial/serial_msm.c
> +++ b/drivers/serial/serial_msm.c
> @@ -16,6 +16,7 @@
>  #include <serial.h>
>  #include <watchdog.h>
>  #include <asm/global_data.h>
> +#include <asm/gpio.h>
>  #include <asm/io.h>
>  #include <linux/compiler.h>
>  #include <linux/delay.h>
> @@ -210,6 +211,7 @@ static int msm_serial_probe(struct udevice *dev)
>  {
>  	int ret;
>  	struct msm_serial_data *priv = dev_get_priv(dev);
> +	struct gpio_desc rs232_0, rs232_1;
>  
>  	/* No need to reinitialize the UART after relocation */
>  	if (gd->flags & GD_FLG_RELOC)
> @@ -219,6 +221,11 @@ static int msm_serial_probe(struct udevice *dev)
>  	if (ret)
>  		return ret;
>  
> +	if (!gpio_request_by_name(dev, "gpios", 0, &rs232_0, GPIOD_IS_OUT))
> +		dm_gpio_set_value(&rs232_0, 1);
> +	if (!gpio_request_by_name(dev, "gpios", 1, &rs232_1, GPIOD_IS_OUT))
> +		dm_gpio_set_value(&rs232_1, 0);
> +
>  	pinctrl_select_state(dev, "uart");
>  	uart_dm_init(priv);
>  

-- 
// Caleb (they/them)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/7] Add SE HMBSC board support
  2023-12-19 16:26     ` Caleb Connolly
@ 2023-12-20 13:36       ` Sumit Garg
  2023-12-21 13:38         ` Caleb Connolly
  0 siblings, 1 reply; 20+ messages in thread
From: Sumit Garg @ 2023-12-20 13:36 UTC (permalink / raw)
  To: Caleb Connolly
  Cc: Simon Glass, u-boot, neil.armstrong, lukma, seanga2, marex,
	laetitia.mariottini, pascal.eberhard, abdou.saker, jimmy.lalande,
	benjamin.missey, daniel.thompson

On Tue, 19 Dec 2023 at 21:56, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>
>
>
> On 19/12/2023 06:25, Sumit Garg wrote:
> > Hi Simon,
> >
> > On Mon, 18 Dec 2023 at 20:32, Simon Glass <sjg@chromium.org> wrote:
> >>
> >> Hi Sumit,
> >>
> >> On Mon, 18 Dec 2023 at 00:24, Sumit Garg <sumit.garg@linaro.org> wrote:
> >>>
> >>> SE HMIBSC board is based on Qcom APQ8016 SoC. One of the major
> >>
> >> Could you please add a doc/ file for this board and explain how to
> >> build it and how to run U-Boot on it?
> >
> > Ah I forgot to add that since the build/boot instructions are quite
> > similar to db410c. BTW, I will add that in the next spin.
>
> Probably just referencing that document or making the language generic
> would be good.
>

I will rename that doc to be rather SoC specific with board specific
sub-sections.

> Sumit, could you rebase this series on my generic board support? [1] in
> it's current form this series conflicts, and includes some of the major
> anti-patterns I'm trying to move away from in mach-snapdragon.

Although, I haven't gone through your series but I was expecting those
conflicts. Let's work together to make this series compatible on top
of yours.

>
> You should not have to introduce a new CONFIG_TARGET_XYZ variable, and
> from what I can tell you shouldn't even need to add the board/schneider
> directory at all, you can just set the following in your defconfig:
>
> CONFIG_SYS_BOARD="dragonboard410c"

This is simply a misnomer, its HMIBSC board. I suppose we should
rather separate the SoC specific bits into mach-snapdragon and let
different boards use them.

> CONFIG_SYS_CONFIG_NAME="hmibsc"
>
> This will use the db410c board code (which yours is just a copy/paste of
> from what I can tell) and your custom include/configs/hmibsc.h header.
>
> The addresses set in your environment file should be allocated
> automatically at runtime too (see the ("mach-snapdragon: dynamic load
> addresses") patch).
>
> You should also switch to an upstream board DTS based on my series, and
> drop the "-uboot.dtsi" file.

Unfortunately, currently there isn't any upstream DTS for this board
but I will check with the SE team regarding their plans. Until then we
have to use a U-Boot specific DTS file.

-Sumit

>
> [1]:
> https://lore.kernel.org/u-boot/20231219-b4-qcom-common-target-v2-0-b6dd9704219e@linaro.org/
>
> Kind regards,
>
> >
> > -Sumit
> >
> >>
> >>> difference from db410c is serial port where HMIBSC board uses UART1 as
> >>> the debug console with an RS232 port, patch #1 - #3 adds corresponding
> >>> driver support.
> >>>
> >>> Patch #4 adds main HMIBSC board specific bits, 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
> >>>
> >>> Patch #5 - #7 enables specific board features like RAUC support,
> >>> environment protection and USB networking support.
> >>>
> >>> This patch series is based on top of Qcom maintainer tree [1] + the latest
> >>> PMIC patch-set [2]. Feedback is very much welcome.
> >>>
> >>> [1] https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commits/u-boot-qcom-next?ref_type=heads
> >>> [2] https://patchwork.ozlabs.org/project/uboot/list/?series=385322
> >>>
> >>> Sumit Garg (7):
> >>>   clk: apq8016: Add support for UART1 clocks
> >>>   serial_msm: Add support for RS232 GPIOs
> >>>   serial_msm: Enable RS232 flow control
> >>>   board: Add SE HMIBSC board support
> >>>   hmibsc: Enable RAUC support
> >>>   hmibsc: enable U-Boot Environment variables protection
> >>>   hmibsc: Enable LAN75XX USB ethernet driver
> >>>
> >>>  arch/arm/dts/Makefile              |   1 +
> >>>  arch/arm/dts/hmibsc-uboot.dtsi     |  43 +++++++
> >>>  arch/arm/dts/hmibsc.dts            | 188 +++++++++++++++++++++++++++++
> >>>  arch/arm/mach-snapdragon/Kconfig   |  18 +++
> >>>  arch/arm/mach-snapdragon/Makefile  |   1 +
> >>>  board/schneider/hmibsc/Kconfig     |  15 +++
> >>>  board/schneider/hmibsc/MAINTAINERS |   6 +
> >>>  board/schneider/hmibsc/Makefile    |   5 +
> >>>  board/schneider/hmibsc/hmibsc.c    | 179 +++++++++++++++++++++++++++
> >>>  board/schneider/hmibsc/hmibsc.env  |  11 ++
> >>>  configs/hmibsc_defconfig           |  79 ++++++++++++
> >>>  drivers/clk/qcom/clock-apq8016.c   |  44 ++++++-
> >>>  drivers/serial/serial_msm.c        |  23 +++-
> >>>  drivers/usb/host/Kconfig           |   1 +
> >>>  include/configs/hmibsc.h           |  59 +++++++++
> >>>  15 files changed, 665 insertions(+), 8 deletions(-)
> >>>  create mode 100644 arch/arm/dts/hmibsc-uboot.dtsi
> >>>  create mode 100644 arch/arm/dts/hmibsc.dts
> >>>  create mode 100644 board/schneider/hmibsc/Kconfig
> >>>  create mode 100644 board/schneider/hmibsc/MAINTAINERS
> >>>  create mode 100644 board/schneider/hmibsc/Makefile
> >>>  create mode 100644 board/schneider/hmibsc/hmibsc.c
> >>>  create mode 100644 board/schneider/hmibsc/hmibsc.env
> >>>  create mode 100644 configs/hmibsc_defconfig
> >>>  create mode 100644 include/configs/hmibsc.h
> >>>
> >>> --
> >>> 2.34.1
> >>>
> >>
> >> Regards,
> >> Simon
>
> --
> // Caleb (they/them)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/7] serial_msm: Add support for RS232 GPIOs
  2023-12-19 16:47   ` Caleb Connolly
@ 2023-12-20 14:09     ` Sumit Garg
  0 siblings, 0 replies; 20+ messages in thread
From: Sumit Garg @ 2023-12-20 14:09 UTC (permalink / raw)
  To: Caleb Connolly
  Cc: u-boot, neil.armstrong, lukma, seanga2, marex,
	laetitia.mariottini, pascal.eberhard, abdou.saker, jimmy.lalande,
	benjamin.missey, daniel.thompson

On Tue, 19 Dec 2023 at 22:17, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>
> I guess these GPIOs are RTS/CTS? Missing description...

No, these GPIOs (99 and 100) on HMIBSC board rather act as a mux to
control UART mode out of rs232/422/485. I will add a corresponding
description.

>
> Given that you just set them to their respective values, you should be
> able to configure these using pinctrl with this patch [1], so we can
> avoid adding these non-standard bindings.

Okay I will try to use them once we resolve the conflicts discussed in
the other thread.

>
> I tested your next patch, setting "MSM_UART_MR1_RX_RDY_CTL" in
> uart_dm_init() on db410c and db820c and I don't notice any issues with
> setting that bit unconditionally. If it's definitely needed for your
> board then I'd be ok with a patch to always set it with a comment
> offering some context.

Yeah it's needed for RS232 mode to operate correctly. I will make it
unconditional then.

-Sumit

>
> [1]:
> https://lore.kernel.org/u-boot/20231219-b4-qcom-common-target-v2-10-b6dd9704219e@linaro.org/
>
> On 18/12/2023 07:24, Sumit Garg wrote:
> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> > ---
> >  drivers/serial/serial_msm.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c
> > index a22623c316e..43e58595dc2 100644
> > --- a/drivers/serial/serial_msm.c
> > +++ b/drivers/serial/serial_msm.c
> > @@ -16,6 +16,7 @@
> >  #include <serial.h>
> >  #include <watchdog.h>
> >  #include <asm/global_data.h>
> > +#include <asm/gpio.h>
> >  #include <asm/io.h>
> >  #include <linux/compiler.h>
> >  #include <linux/delay.h>
> > @@ -210,6 +211,7 @@ static int msm_serial_probe(struct udevice *dev)
> >  {
> >       int ret;
> >       struct msm_serial_data *priv = dev_get_priv(dev);
> > +     struct gpio_desc rs232_0, rs232_1;
> >
> >       /* No need to reinitialize the UART after relocation */
> >       if (gd->flags & GD_FLG_RELOC)
> > @@ -219,6 +221,11 @@ static int msm_serial_probe(struct udevice *dev)
> >       if (ret)
> >               return ret;
> >
> > +     if (!gpio_request_by_name(dev, "gpios", 0, &rs232_0, GPIOD_IS_OUT))
> > +             dm_gpio_set_value(&rs232_0, 1);
> > +     if (!gpio_request_by_name(dev, "gpios", 1, &rs232_1, GPIOD_IS_OUT))
> > +             dm_gpio_set_value(&rs232_1, 0);
> > +
> >       pinctrl_select_state(dev, "uart");
> >       uart_dm_init(priv);
> >
>
> --
> // Caleb (they/them)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/7] Add SE HMBSC board support
  2023-12-20 13:36       ` Sumit Garg
@ 2023-12-21 13:38         ` Caleb Connolly
  2023-12-22  9:04           ` Sumit Garg
  0 siblings, 1 reply; 20+ messages in thread
From: Caleb Connolly @ 2023-12-21 13:38 UTC (permalink / raw)
  To: Sumit Garg
  Cc: Simon Glass, u-boot, neil.armstrong, lukma, seanga2, marex,
	laetitia.mariottini, pascal.eberhard, abdou.saker, jimmy.lalande,
	benjamin.missey, daniel.thompson



On 20/12/2023 13:36, Sumit Garg wrote:
> On Tue, 19 Dec 2023 at 21:56, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>>
>>
>>
>> On 19/12/2023 06:25, Sumit Garg wrote:
>>> Hi Simon,
>>>
>>> On Mon, 18 Dec 2023 at 20:32, Simon Glass <sjg@chromium.org> wrote:
>>>>
>>>> Hi Sumit,
>>>>
>>>> On Mon, 18 Dec 2023 at 00:24, Sumit Garg <sumit.garg@linaro.org> wrote:
>>>>>
>>>>> SE HMIBSC board is based on Qcom APQ8016 SoC. One of the major
>>>>
>>>> Could you please add a doc/ file for this board and explain how to
>>>> build it and how to run U-Boot on it?
>>>
>>> Ah I forgot to add that since the build/boot instructions are quite
>>> similar to db410c. BTW, I will add that in the next spin.
>>
>> Probably just referencing that document or making the language generic
>> would be good.
>>
> 
> I will rename that doc to be rather SoC specific with board specific
> sub-sections.
> 
>> Sumit, could you rebase this series on my generic board support? [1] in
>> it's current form this series conflicts, and includes some of the major
>> anti-patterns I'm trying to move away from in mach-snapdragon.
> 
> Although, I haven't gone through your series but I was expecting those
> conflicts. Let's work together to make this series compatible on top
> of yours.
> 
>>
>> You should not have to introduce a new CONFIG_TARGET_XYZ variable, and
>> from what I can tell you shouldn't even need to add the board/schneider
>> directory at all, you can just set the following in your defconfig:
>>
>> CONFIG_SYS_BOARD="dragonboard410c"
> 
> This is simply a misnomer, its HMIBSC board. I suppose we should
> rather separate the SoC specific bits into mach-snapdragon and let
> different boards use them.

Is there any reason why you can't just use the existing db410c board
code as-is? I don't see why you want to duplicate it.

The only reason the db410c and db820c have their board code is because
they're old platforms and already supported. For adding new support
there needs to be some very strong justification to have board-specific
C code.

I think it would be nice to make the db410c code go away, or be toggled
at runtime, probably most of it will just work and not break any other
boards anyway. The db820c code is just part of what should be in the
pinctrl driver...

Let's move away from this old model and towards having more generic
U-Boot images. This will snowball towards making future bringup even easier.
> 
>> CONFIG_SYS_CONFIG_NAME="hmibsc"
>>
>> This will use the db410c board code (which yours is just a copy/paste of
>> from what I can tell) and your custom include/configs/hmibsc.h header.
>>
>> The addresses set in your environment file should be allocated
>> automatically at runtime too (see the ("mach-snapdragon: dynamic load
>> addresses") patch).
>>
>> You should also switch to an upstream board DTS based on my series, and
>> drop the "-uboot.dtsi" file.
> 
> Unfortunately, currently there isn't any upstream DTS for this board
> but I will check with the SE team regarding their plans. Until then we
> have to use a U-Boot specific DTS file.

In that case, perhaps we can take whatever DTS they're using in
production, or a version of it, and at least split the U-Boot changes
(if any) out into a separate file as I've done with the other platforms.
This way we stay consistent and can keep track of what U-Boot specific
DTS changes we need.

I guess this is all new territory for us, but imho if we're adding
support for a board that doesn't have an upstream DTS, we should still
follow the same model, open to input here.
> 
> -Sumit
> 
>>
>> [1]:
>> https://lore.kernel.org/u-boot/20231219-b4-qcom-common-target-v2-0-b6dd9704219e@linaro.org/
>>
>> Kind regards,
>>
>>>
>>> -Sumit
>>>
>>>>
>>>>> difference from db410c is serial port where HMIBSC board uses UART1 as
>>>>> the debug console with an RS232 port, patch #1 - #3 adds corresponding
>>>>> driver support.
>>>>>
>>>>> Patch #4 adds main HMIBSC board specific bits, 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
>>>>>
>>>>> Patch #5 - #7 enables specific board features like RAUC support,
>>>>> environment protection and USB networking support.
>>>>>
>>>>> This patch series is based on top of Qcom maintainer tree [1] + the latest
>>>>> PMIC patch-set [2]. Feedback is very much welcome.
>>>>>
>>>>> [1] https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commits/u-boot-qcom-next?ref_type=heads
>>>>> [2] https://patchwork.ozlabs.org/project/uboot/list/?series=385322
>>>>>
>>>>> Sumit Garg (7):
>>>>>   clk: apq8016: Add support for UART1 clocks
>>>>>   serial_msm: Add support for RS232 GPIOs
>>>>>   serial_msm: Enable RS232 flow control
>>>>>   board: Add SE HMIBSC board support
>>>>>   hmibsc: Enable RAUC support
>>>>>   hmibsc: enable U-Boot Environment variables protection
>>>>>   hmibsc: Enable LAN75XX USB ethernet driver
>>>>>
>>>>>  arch/arm/dts/Makefile              |   1 +
>>>>>  arch/arm/dts/hmibsc-uboot.dtsi     |  43 +++++++
>>>>>  arch/arm/dts/hmibsc.dts            | 188 +++++++++++++++++++++++++++++
>>>>>  arch/arm/mach-snapdragon/Kconfig   |  18 +++
>>>>>  arch/arm/mach-snapdragon/Makefile  |   1 +
>>>>>  board/schneider/hmibsc/Kconfig     |  15 +++
>>>>>  board/schneider/hmibsc/MAINTAINERS |   6 +
>>>>>  board/schneider/hmibsc/Makefile    |   5 +
>>>>>  board/schneider/hmibsc/hmibsc.c    | 179 +++++++++++++++++++++++++++
>>>>>  board/schneider/hmibsc/hmibsc.env  |  11 ++
>>>>>  configs/hmibsc_defconfig           |  79 ++++++++++++
>>>>>  drivers/clk/qcom/clock-apq8016.c   |  44 ++++++-
>>>>>  drivers/serial/serial_msm.c        |  23 +++-
>>>>>  drivers/usb/host/Kconfig           |   1 +
>>>>>  include/configs/hmibsc.h           |  59 +++++++++
>>>>>  15 files changed, 665 insertions(+), 8 deletions(-)
>>>>>  create mode 100644 arch/arm/dts/hmibsc-uboot.dtsi
>>>>>  create mode 100644 arch/arm/dts/hmibsc.dts
>>>>>  create mode 100644 board/schneider/hmibsc/Kconfig
>>>>>  create mode 100644 board/schneider/hmibsc/MAINTAINERS
>>>>>  create mode 100644 board/schneider/hmibsc/Makefile
>>>>>  create mode 100644 board/schneider/hmibsc/hmibsc.c
>>>>>  create mode 100644 board/schneider/hmibsc/hmibsc.env
>>>>>  create mode 100644 configs/hmibsc_defconfig
>>>>>  create mode 100644 include/configs/hmibsc.h
>>>>>
>>>>> --
>>>>> 2.34.1
>>>>>
>>>>
>>>> Regards,
>>>> Simon
>>
>> --
>> // Caleb (they/them)

-- 
// Caleb (they/them)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/7] Add SE HMBSC board support
  2023-12-21 13:38         ` Caleb Connolly
@ 2023-12-22  9:04           ` Sumit Garg
  2023-12-22 17:34             ` Caleb Connolly
  0 siblings, 1 reply; 20+ messages in thread
From: Sumit Garg @ 2023-12-22  9:04 UTC (permalink / raw)
  To: Caleb Connolly
  Cc: Simon Glass, u-boot, neil.armstrong, lukma, seanga2, marex,
	laetitia.mariottini, pascal.eberhard, abdou.saker, jimmy.lalande,
	benjamin.missey, daniel.thompson

On Thu, 21 Dec 2023 at 19:09, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>
>
>
> On 20/12/2023 13:36, Sumit Garg wrote:
> > On Tue, 19 Dec 2023 at 21:56, Caleb Connolly <caleb.connolly@linaro.org> wrote:
> >>
> >>
> >>
> >> On 19/12/2023 06:25, Sumit Garg wrote:
> >>> Hi Simon,
> >>>
> >>> On Mon, 18 Dec 2023 at 20:32, Simon Glass <sjg@chromium.org> wrote:
> >>>>
> >>>> Hi Sumit,
> >>>>
> >>>> On Mon, 18 Dec 2023 at 00:24, Sumit Garg <sumit.garg@linaro.org> wrote:
> >>>>>
> >>>>> SE HMIBSC board is based on Qcom APQ8016 SoC. One of the major
> >>>>
> >>>> Could you please add a doc/ file for this board and explain how to
> >>>> build it and how to run U-Boot on it?
> >>>
> >>> Ah I forgot to add that since the build/boot instructions are quite
> >>> similar to db410c. BTW, I will add that in the next spin.
> >>
> >> Probably just referencing that document or making the language generic
> >> would be good.
> >>
> >
> > I will rename that doc to be rather SoC specific with board specific
> > sub-sections.
> >
> >> Sumit, could you rebase this series on my generic board support? [1] in
> >> it's current form this series conflicts, and includes some of the major
> >> anti-patterns I'm trying to move away from in mach-snapdragon.
> >
> > Although, I haven't gone through your series but I was expecting those
> > conflicts. Let's work together to make this series compatible on top
> > of yours.
> >
> >>
> >> You should not have to introduce a new CONFIG_TARGET_XYZ variable, and
> >> from what I can tell you shouldn't even need to add the board/schneider
> >> directory at all, you can just set the following in your defconfig:
> >>
> >> CONFIG_SYS_BOARD="dragonboard410c"
> >
> > This is simply a misnomer, its HMIBSC board. I suppose we should
> > rather separate the SoC specific bits into mach-snapdragon and let
> > different boards use them.
>
> Is there any reason why you can't just use the existing db410c board
> code as-is? I don't see why you want to duplicate it.

I don't want to duplicate it but rather we should make the common
parts as part of arch/arm/mach-snapdragon/ and let derivative boards
use them. The HMIBSC board is an industrial board which doesn't need
any generic distro boot but rather FIT booting with OTA updates via
RAUC [1] along with U-boot environment protection. Doesn't that make
it different from db410c?

[1] https://rauc.io/

>
> The only reason the db410c and db820c have their board code is because
> they're old platforms and already supported. For adding new support
> there needs to be some very strong justification to have board-specific
> C code.
>
> I think it would be nice to make the db410c code go away, or be toggled
> at runtime, probably most of it will just work and not break any other
> boards anyway. The db820c code is just part of what should be in the
> pinctrl driver...
>
> Let's move away from this old model and towards having more generic
> U-Boot images. This will snowball towards making future bringup even easier.

As I have mentioned in the other thread, we should give alternatives
to the board code as well. How would you handle the following?

- Fastboot mode entry on a button press.
- Configure MAC address for network support.
- Setup board serial number.

I suppose we don't need #ifdefry in the
arch/arm/mach-snapdragon/board.c file, right?

> >
> >> CONFIG_SYS_CONFIG_NAME="hmibsc"
> >>
> >> This will use the db410c board code (which yours is just a copy/paste of
> >> from what I can tell) and your custom include/configs/hmibsc.h header.
> >>
> >> The addresses set in your environment file should be allocated
> >> automatically at runtime too (see the ("mach-snapdragon: dynamic load
> >> addresses") patch).
> >>
> >> You should also switch to an upstream board DTS based on my series, and
> >> drop the "-uboot.dtsi" file.
> >
> > Unfortunately, currently there isn't any upstream DTS for this board
> > but I will check with the SE team regarding their plans. Until then we
> > have to use a U-Boot specific DTS file.
>
> In that case, perhaps we can take whatever DTS they're using in
> production, or a version of it, and at least split the U-Boot changes
> (if any) out into a separate file as I've done with the other platforms.
> This way we stay consistent and can keep track of what U-Boot specific
> DTS changes we need.

The first step for that is to land that DTS file upstream in the Linux
kernel and then we should make a switch. The middle ground here won't
be maintainable.

>
> I guess this is all new territory for us, but imho if we're adding
> support for a board that doesn't have an upstream DTS, we should still
> follow the same model, open to input here.

With the DT rebasing tree, we actually want to make these bits clear.
Either the board support is fully upstream and then we make a switch
to upstream or you can have a u-boot specific DTS subset compliant
with upstream bindings while upstreaming is in progress. There is
always ABI risk involved for using half baked board DTS files.

-Sumit

> >
> > -Sumit
> >
> >>
> >> [1]:
> >> https://lore.kernel.org/u-boot/20231219-b4-qcom-common-target-v2-0-b6dd9704219e@linaro.org/
> >>
> >> Kind regards,
> >>
> >>>
> >>> -Sumit
> >>>
> >>>>
> >>>>> difference from db410c is serial port where HMIBSC board uses UART1 as
> >>>>> the debug console with an RS232 port, patch #1 - #3 adds corresponding
> >>>>> driver support.
> >>>>>
> >>>>> Patch #4 adds main HMIBSC board specific bits, 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
> >>>>>
> >>>>> Patch #5 - #7 enables specific board features like RAUC support,
> >>>>> environment protection and USB networking support.
> >>>>>
> >>>>> This patch series is based on top of Qcom maintainer tree [1] + the latest
> >>>>> PMIC patch-set [2]. Feedback is very much welcome.
> >>>>>
> >>>>> [1] https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commits/u-boot-qcom-next?ref_type=heads
> >>>>> [2] https://patchwork.ozlabs.org/project/uboot/list/?series=385322
> >>>>>
> >>>>> Sumit Garg (7):
> >>>>>   clk: apq8016: Add support for UART1 clocks
> >>>>>   serial_msm: Add support for RS232 GPIOs
> >>>>>   serial_msm: Enable RS232 flow control
> >>>>>   board: Add SE HMIBSC board support
> >>>>>   hmibsc: Enable RAUC support
> >>>>>   hmibsc: enable U-Boot Environment variables protection
> >>>>>   hmibsc: Enable LAN75XX USB ethernet driver
> >>>>>
> >>>>>  arch/arm/dts/Makefile              |   1 +
> >>>>>  arch/arm/dts/hmibsc-uboot.dtsi     |  43 +++++++
> >>>>>  arch/arm/dts/hmibsc.dts            | 188 +++++++++++++++++++++++++++++
> >>>>>  arch/arm/mach-snapdragon/Kconfig   |  18 +++
> >>>>>  arch/arm/mach-snapdragon/Makefile  |   1 +
> >>>>>  board/schneider/hmibsc/Kconfig     |  15 +++
> >>>>>  board/schneider/hmibsc/MAINTAINERS |   6 +
> >>>>>  board/schneider/hmibsc/Makefile    |   5 +
> >>>>>  board/schneider/hmibsc/hmibsc.c    | 179 +++++++++++++++++++++++++++
> >>>>>  board/schneider/hmibsc/hmibsc.env  |  11 ++
> >>>>>  configs/hmibsc_defconfig           |  79 ++++++++++++
> >>>>>  drivers/clk/qcom/clock-apq8016.c   |  44 ++++++-
> >>>>>  drivers/serial/serial_msm.c        |  23 +++-
> >>>>>  drivers/usb/host/Kconfig           |   1 +
> >>>>>  include/configs/hmibsc.h           |  59 +++++++++
> >>>>>  15 files changed, 665 insertions(+), 8 deletions(-)
> >>>>>  create mode 100644 arch/arm/dts/hmibsc-uboot.dtsi
> >>>>>  create mode 100644 arch/arm/dts/hmibsc.dts
> >>>>>  create mode 100644 board/schneider/hmibsc/Kconfig
> >>>>>  create mode 100644 board/schneider/hmibsc/MAINTAINERS
> >>>>>  create mode 100644 board/schneider/hmibsc/Makefile
> >>>>>  create mode 100644 board/schneider/hmibsc/hmibsc.c
> >>>>>  create mode 100644 board/schneider/hmibsc/hmibsc.env
> >>>>>  create mode 100644 configs/hmibsc_defconfig
> >>>>>  create mode 100644 include/configs/hmibsc.h
> >>>>>
> >>>>> --
> >>>>> 2.34.1
> >>>>>
> >>>>
> >>>> Regards,
> >>>> Simon
> >>
> >> --
> >> // Caleb (they/them)
>
> --
> // Caleb (they/them)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/7] Add SE HMBSC board support
  2023-12-22  9:04           ` Sumit Garg
@ 2023-12-22 17:34             ` Caleb Connolly
  2023-12-26  9:27               ` Sumit Garg
  0 siblings, 1 reply; 20+ messages in thread
From: Caleb Connolly @ 2023-12-22 17:34 UTC (permalink / raw)
  To: Sumit Garg
  Cc: Simon Glass, u-boot, neil.armstrong, lukma, seanga2, marex,
	laetitia.mariottini, pascal.eberhard, abdou.saker, jimmy.lalande,
	benjamin.missey, daniel.thompson

[snip]

>>>> Sumit, could you rebase this series on my generic board support? [1] in
>>>> it's current form this series conflicts, and includes some of the major
>>>> anti-patterns I'm trying to move away from in mach-snapdragon.
>>>
>>> Although, I haven't gone through your series but I was expecting those
>>> conflicts. Let's work together to make this series compatible on top
>>> of yours.
>>>
>>>>
>>>> You should not have to introduce a new CONFIG_TARGET_XYZ variable, and
>>>> from what I can tell you shouldn't even need to add the board/schneider
>>>> directory at all, you can just set the following in your defconfig:
>>>>
>>>> CONFIG_SYS_BOARD="dragonboard410c"
>>>
>>> This is simply a misnomer, its HMIBSC board. I suppose we should
>>> rather separate the SoC specific bits into mach-snapdragon and let
>>> different boards use them.
>>
>> Is there any reason why you can't just use the existing db410c board
>> code as-is? I don't see why you want to duplicate it.
> 
> I don't want to duplicate it but rather we should make the common
> parts as part of arch/arm/mach-snapdragon/ and let derivative boards
> use them. The HMIBSC board is an industrial board which doesn't need
> any generic distro boot but rather FIT booting with OTA updates via
> RAUC [1] along with U-boot environment protection. Doesn't that make
> it different from db410c?

Right, your series does this board specific configuration in the board
header file (include/configs/hmibsc.h) and in the defconfig.

The actual board code in board/schneider/hmibsc/hmibsc.c is directly
copied from board/qualcomm/dragonboard410c/dragonboard410c.c with just
the copyright changed, the serial number code removed, and a style fix
to a comment. Ignoring for a moment the copyright issue which would
definitely need to be fixed, what I'm suggesting that you do is just use
the board code from db410c.

The way I have designed the generic board support is so that two similar
boards can share the same board code but with different config headers,
from what I can tell this would work just fine for you.

--- board/qualcomm/dragonboard410c/dragonboard410c.c
+++ board/schneider/hmibsc/hmibsc.c
@@ -2,5 +2,5 @@
 /*
- * Board init file for Dragonboard 410C
+ * Board init file for SE HMIBSC
  *
- * (C) Copyright 2015 Mateusz Kulikowski <mateusz.kulikowski@gmail.com>
+ * (C) Copyright 2023 Sumit Garg <sumit.garg@linaro.org>
  */
@@ -8,3 +8,2 @@
 #include <button.h>
-#include <common.h>
 #include <cpu_func.h>
@@ -137,7 +136,2 @@
 {
-       char serial[16];
-
-       memset(serial, 0, 16);
-       snprintf(serial, 13, "%x", msm_board_serial());
-       env_set("serial#", serial);
        return 0;
@@ -145,3 +139,4 @@

-/* Fixup of DTB for Linux Kernel
+/*
+ * Fixup of DTB for Linux Kernel
  * 1. Fixup installed DRAM.
@@ -165,3 +160,2 @@

-
        if (!eth_env_get_enetaddr("btaddr", mac)) {
@@ -169,5 +163,6 @@

-/* The BD address is same as WLAN MAC address but with
- * least significant bit flipped.
- */
+               /*
+                * The BD address is same as WLAN MAC address but with
+                * least significant bit flipped.
+                */
                mac[0] ^= 0x01;

> 
> [1] https://rauc.io/
> 
>>
>> The only reason the db410c and db820c have their board code is because
>> they're old platforms and already supported. For adding new support
>> there needs to be some very strong justification to have board-specific
>> C code.
>>
>> I think it would be nice to make the db410c code go away, or be toggled
>> at runtime, probably most of it will just work and not break any other
>> boards anyway. The db820c code is just part of what should be in the
>> pinctrl driver...
>>
>> Let's move away from this old model and towards having more generic
>> U-Boot images. This will snowball towards making future bringup even easier.
> 
> As I have mentioned in the other thread, we should give alternatives
> to the board code as well. How would you handle the following?
> 
> - Fastboot mode entry on a button press.

This can be nicely handled through CONFIG_AUTOBOOT. I currently use
AUTOBOOT_STOP_STR but that's quite limited (only works for the power
button with "\r"). Probably the right solution here is to use
CONFIG_AUTOBOOT_USE_MENUKEY and introduce support for
CONFIG_AUTOBOOT_MENUBUTTON to allow specifying a named button instead of
an ASCII code. One would then configure "menucmd" in the U-Boot
environment to launch fastboot.

This lets us drop all the weird non-standard keycode handling in board
code and just configure it in the defconfig instead. I plan to implement
this eventually but would for sure appreciate it if someone beat me to it.
> - Configure MAC address for network support.

I believe you mentioned elsewhere some board with the MAC address on an
i2c EEPROM? The NVMEM framework solves exactly this issue, and U-Boot
already supports it, just add support to your ethernet driver to
retrieve its MAC address via NVMEM.
> - Setup board serial number.

Same as above, the quick and dirty way to go would be to have
mach-snapdragon check for an alias defined in DT and expect that alias
to be an NVMEM cell which contains the serial number. On the Android
phones this is set in the kernel cmdline from ABL so I would probably
add some code to mach-snapdragon to check the bootargs for one.

> 
> I suppose we don't need #ifdefry in the
> arch/arm/mach-snapdragon/board.c file, right?

Nope, we aren't that limited on code size, and the runtime overhead of a
few extra branches is really not significant. If we do need to start
caring about either of those things then I would rather do this on a
per-feature basis than per-board.
> 
>>>
>>>> CONFIG_SYS_CONFIG_NAME="hmibsc"
>>>>
>>>> This will use the db410c board code (which yours is just a copy/paste of
>>>> from what I can tell) and your custom include/configs/hmibsc.h header.
>>>>
>>>> The addresses set in your environment file should be allocated
>>>> automatically at runtime too (see the ("mach-snapdragon: dynamic load
>>>> addresses") patch).
>>>>
>>>> You should also switch to an upstream board DTS based on my series, and
>>>> drop the "-uboot.dtsi" file.
>>>
>>> Unfortunately, currently there isn't any upstream DTS for this board
>>> but I will check with the SE team regarding their plans. Until then we
>>> have to use a U-Boot specific DTS file.
>>
>> In that case, perhaps we can take whatever DTS they're using in
>> production, or a version of it, and at least split the U-Boot changes
>> (if any) out into a separate file as I've done with the other platforms.
>> This way we stay consistent and can keep track of what U-Boot specific
>> DTS changes we need.
> 
> The first step for that is to land that DTS file upstream in the Linux
> kernel and then we should make a switch. The middle ground here won't
> be maintainable.
> 
>>
>> I guess this is all new territory for us, but imho if we're adding
>> support for a board that doesn't have an upstream DTS, we should still
>> follow the same model, open to input here.
> 
> With the DT rebasing tree, we actually want to make these bits clear.
> Either the board support is fully upstream and then we make a switch
> to upstream or you can have a u-boot specific DTS subset compliant
> with upstream bindings while upstreaming is in progress. There is
> always ABI risk involved for using half baked board DTS files.

The DTS file submitted in your series is a copy paste of
dragonboard410c.dts with the serial port adjusted, the LEDs removed, and
the copyright changed...

This will make it the only board still using the totally made up DTS
style that all the Qualcomm boards used to use, while everything else is
standardised on upstream (meaning they #include the SoC and PMIC dtsi
files).

It doesn't make a difference whether you put the board DT in dt-rebasing
or if it just goes in U-Boot, it should be a DT derived from upstream
not some hand-crafted thing which will never be supported. Just copy
apq8016-sbc.dts when rebasing on my series and it will be fine.

That said, as I'm writing this I've realised that the pinctrl-apq8016
driver is missing a patch to use the upstream GPIO naming. I'll fix this
in the next revision of the qualcomm generic support series, but just a
heads up if you run into that.
> 
> -Sumit
> 
>>>
>>> -Sumit
>>>
>>>>
>>>> [1]:
>>>> https://lore.kernel.org/u-boot/20231219-b4-qcom-common-target-v2-0-b6dd9704219e@linaro.org/
>>>>
>>>> Kind regards,
>>>>
>>>>>
>>>>> -Sumit
>>>>>
>>>>>>
>>>>>>> difference from db410c is serial port where HMIBSC board uses UART1 as
>>>>>>> the debug console with an RS232 port, patch #1 - #3 adds corresponding
>>>>>>> driver support.
>>>>>>>
>>>>>>> Patch #4 adds main HMIBSC board specific bits, 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
>>>>>>>
>>>>>>> Patch #5 - #7 enables specific board features like RAUC support,
>>>>>>> environment protection and USB networking support.
>>>>>>>
>>>>>>> This patch series is based on top of Qcom maintainer tree [1] + the latest
>>>>>>> PMIC patch-set [2]. Feedback is very much welcome.
>>>>>>>
>>>>>>> [1] https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commits/u-boot-qcom-next?ref_type=heads
>>>>>>> [2] https://patchwork.ozlabs.org/project/uboot/list/?series=385322
>>>>>>>
>>>>>>> Sumit Garg (7):
>>>>>>>   clk: apq8016: Add support for UART1 clocks
>>>>>>>   serial_msm: Add support for RS232 GPIOs
>>>>>>>   serial_msm: Enable RS232 flow control
>>>>>>>   board: Add SE HMIBSC board support
>>>>>>>   hmibsc: Enable RAUC support
>>>>>>>   hmibsc: enable U-Boot Environment variables protection
>>>>>>>   hmibsc: Enable LAN75XX USB ethernet driver
>>>>>>>
>>>>>>>  arch/arm/dts/Makefile              |   1 +
>>>>>>>  arch/arm/dts/hmibsc-uboot.dtsi     |  43 +++++++
>>>>>>>  arch/arm/dts/hmibsc.dts            | 188 +++++++++++++++++++++++++++++
>>>>>>>  arch/arm/mach-snapdragon/Kconfig   |  18 +++
>>>>>>>  arch/arm/mach-snapdragon/Makefile  |   1 +
>>>>>>>  board/schneider/hmibsc/Kconfig     |  15 +++
>>>>>>>  board/schneider/hmibsc/MAINTAINERS |   6 +
>>>>>>>  board/schneider/hmibsc/Makefile    |   5 +
>>>>>>>  board/schneider/hmibsc/hmibsc.c    | 179 +++++++++++++++++++++++++++
>>>>>>>  board/schneider/hmibsc/hmibsc.env  |  11 ++
>>>>>>>  configs/hmibsc_defconfig           |  79 ++++++++++++
>>>>>>>  drivers/clk/qcom/clock-apq8016.c   |  44 ++++++-
>>>>>>>  drivers/serial/serial_msm.c        |  23 +++-
>>>>>>>  drivers/usb/host/Kconfig           |   1 +
>>>>>>>  include/configs/hmibsc.h           |  59 +++++++++
>>>>>>>  15 files changed, 665 insertions(+), 8 deletions(-)
>>>>>>>  create mode 100644 arch/arm/dts/hmibsc-uboot.dtsi
>>>>>>>  create mode 100644 arch/arm/dts/hmibsc.dts
>>>>>>>  create mode 100644 board/schneider/hmibsc/Kconfig
>>>>>>>  create mode 100644 board/schneider/hmibsc/MAINTAINERS
>>>>>>>  create mode 100644 board/schneider/hmibsc/Makefile
>>>>>>>  create mode 100644 board/schneider/hmibsc/hmibsc.c
>>>>>>>  create mode 100644 board/schneider/hmibsc/hmibsc.env
>>>>>>>  create mode 100644 configs/hmibsc_defconfig
>>>>>>>  create mode 100644 include/configs/hmibsc.h
>>>>>>>
>>>>>>> --
>>>>>>> 2.34.1
>>>>>>>
>>>>>>
>>>>>> Regards,
>>>>>> Simon
>>>>
>>>> --
>>>> // Caleb (they/them)
>>
>> --
>> // Caleb (they/them)

-- 
// Caleb (they/them)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/7] Add SE HMBSC board support
  2023-12-22 17:34             ` Caleb Connolly
@ 2023-12-26  9:27               ` Sumit Garg
  2024-01-02  8:33                 ` Caleb Connolly
  0 siblings, 1 reply; 20+ messages in thread
From: Sumit Garg @ 2023-12-26  9:27 UTC (permalink / raw)
  To: Caleb Connolly
  Cc: Simon Glass, u-boot, neil.armstrong, lukma, seanga2, marex,
	laetitia.mariottini, pascal.eberhard, abdou.saker, jimmy.lalande,
	benjamin.missey, daniel.thompson

On Fri, 22 Dec 2023 at 23:04, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>
> [snip]
>
> >>>> Sumit, could you rebase this series on my generic board support? [1] in
> >>>> it's current form this series conflicts, and includes some of the major
> >>>> anti-patterns I'm trying to move away from in mach-snapdragon.
> >>>
> >>> Although, I haven't gone through your series but I was expecting those
> >>> conflicts. Let's work together to make this series compatible on top
> >>> of yours.
> >>>
> >>>>
> >>>> You should not have to introduce a new CONFIG_TARGET_XYZ variable, and
> >>>> from what I can tell you shouldn't even need to add the board/schneider
> >>>> directory at all, you can just set the following in your defconfig:
> >>>>
> >>>> CONFIG_SYS_BOARD="dragonboard410c"
> >>>
> >>> This is simply a misnomer, its HMIBSC board. I suppose we should
> >>> rather separate the SoC specific bits into mach-snapdragon and let
> >>> different boards use them.
> >>
> >> Is there any reason why you can't just use the existing db410c board
> >> code as-is? I don't see why you want to duplicate it.
> >
> > I don't want to duplicate it but rather we should make the common
> > parts as part of arch/arm/mach-snapdragon/ and let derivative boards
> > use them. The HMIBSC board is an industrial board which doesn't need
> > any generic distro boot but rather FIT booting with OTA updates via
> > RAUC [1] along with U-boot environment protection. Doesn't that make
> > it different from db410c?
>
> Right, your series does this board specific configuration in the board
> header file (include/configs/hmibsc.h) and in the defconfig.
>
> The actual board code in board/schneider/hmibsc/hmibsc.c is directly
> copied from board/qualcomm/dragonboard410c/dragonboard410c.c with just
> the copyright changed, the serial number code removed, and a style fix
> to a comment. Ignoring for a moment the copyright issue which would
> definitely need to be fixed,

Ah, I should have retained the copyright.

> what I'm suggesting that you do is just use
> the board code from db410c.

This is just initial board support, a direct copy would just limit any
further board support extensions.

>
> The way I have designed the generic board support is so that two similar
> boards can share the same board code but with different config headers,
> from what I can tell this would work just fine for you.

As I have mentioned earlier, the proper way to share code among boards
is to have something like following:

arch/arm/mach-snapdragon/board_apq8016.c

You can take examples from arch/arm/mach-meson/board-*.c. If you don't
want to do it as part of your series then let me know I will include
it in this series.

>
> --- board/qualcomm/dragonboard410c/dragonboard410c.c
> +++ board/schneider/hmibsc/hmibsc.c
> @@ -2,5 +2,5 @@
>  /*
> - * Board init file for Dragonboard 410C
> + * Board init file for SE HMIBSC
>   *
> - * (C) Copyright 2015 Mateusz Kulikowski <mateusz.kulikowski@gmail.com>
> + * (C) Copyright 2023 Sumit Garg <sumit.garg@linaro.org>
>   */
> @@ -8,3 +8,2 @@
>  #include <button.h>
> -#include <common.h>
>  #include <cpu_func.h>
> @@ -137,7 +136,2 @@
>  {
> -       char serial[16];
> -
> -       memset(serial, 0, 16);
> -       snprintf(serial, 13, "%x", msm_board_serial());
> -       env_set("serial#", serial);
>         return 0;
> @@ -145,3 +139,4 @@
>
> -/* Fixup of DTB for Linux Kernel
> +/*
> + * Fixup of DTB for Linux Kernel
>   * 1. Fixup installed DRAM.
> @@ -165,3 +160,2 @@
>
> -
>         if (!eth_env_get_enetaddr("btaddr", mac)) {
> @@ -169,5 +163,6 @@
>
> -/* The BD address is same as WLAN MAC address but with
> - * least significant bit flipped.
> - */
> +               /*
> +                * The BD address is same as WLAN MAC address but with
> +                * least significant bit flipped.
> +                */
>                 mac[0] ^= 0x01;
>
> >
> > [1] https://rauc.io/
> >
> >>
> >> The only reason the db410c and db820c have their board code is because
> >> they're old platforms and already supported. For adding new support
> >> there needs to be some very strong justification to have board-specific
> >> C code.
> >>
> >> I think it would be nice to make the db410c code go away, or be toggled
> >> at runtime, probably most of it will just work and not break any other
> >> boards anyway. The db820c code is just part of what should be in the
> >> pinctrl driver...
> >>
> >> Let's move away from this old model and towards having more generic
> >> U-Boot images. This will snowball towards making future bringup even easier.
> >
> > As I have mentioned in the other thread, we should give alternatives
> > to the board code as well. How would you handle the following?
> >
> > - Fastboot mode entry on a button press.
>
> This can be nicely handled through CONFIG_AUTOBOOT. I currently use
> AUTOBOOT_STOP_STR but that's quite limited (only works for the power
> button with "\r"). Probably the right solution here is to use
> CONFIG_AUTOBOOT_USE_MENUKEY and introduce support for
> CONFIG_AUTOBOOT_MENUBUTTON to allow specifying a named button instead of
> an ASCII code. One would then configure "menucmd" in the U-Boot
> environment to launch fastboot.

Won't this break existing users who relied on volume buttons to flash
their board and rather have to purchase a debug uart?

>
> This lets us drop all the weird non-standard keycode handling in board
> code and just configure it in the defconfig instead. I plan to implement
> this eventually but would for sure appreciate it if someone beat me to it.
> > - Configure MAC address for network support.
>
> I believe you mentioned elsewhere some board with the MAC address on an
> i2c EEPROM? The NVMEM framework solves exactly this issue, and U-Boot
> already supports it, just add support to your ethernet driver to
> retrieve its MAC address via NVMEM.

Good to know that.

> > - Setup board serial number.
>
> Same as above, the quick and dirty way to go would be to have
> mach-snapdragon check for an alias defined in DT and expect that alias
> to be an NVMEM cell which contains the serial number. On the Android
> phones this is set in the kernel cmdline from ABL so I would probably
> add some code to mach-snapdragon to check the bootargs for one.

And what about boards where U-Boot starts as the first stage
boot-loader? Is there corresponding DT binding?

>
> >
> > I suppose we don't need #ifdefry in the
> > arch/arm/mach-snapdragon/board.c file, right?
>
> Nope, we aren't that limited on code size, and the runtime overhead of a
> few extra branches is really not significant. If we do need to start
> caring about either of those things then I would rather do this on a
> per-feature basis than per-board.

The examples I gave were only limited to what are currently used by
Qcom boards. But we should be open to future board support extensions
too.

> >
> >>>
> >>>> CONFIG_SYS_CONFIG_NAME="hmibsc"
> >>>>
> >>>> This will use the db410c board code (which yours is just a copy/paste of
> >>>> from what I can tell) and your custom include/configs/hmibsc.h header.
> >>>>
> >>>> The addresses set in your environment file should be allocated
> >>>> automatically at runtime too (see the ("mach-snapdragon: dynamic load
> >>>> addresses") patch).
> >>>>
> >>>> You should also switch to an upstream board DTS based on my series, and
> >>>> drop the "-uboot.dtsi" file.
> >>>
> >>> Unfortunately, currently there isn't any upstream DTS for this board
> >>> but I will check with the SE team regarding their plans. Until then we
> >>> have to use a U-Boot specific DTS file.
> >>
> >> In that case, perhaps we can take whatever DTS they're using in
> >> production, or a version of it, and at least split the U-Boot changes
> >> (if any) out into a separate file as I've done with the other platforms.
> >> This way we stay consistent and can keep track of what U-Boot specific
> >> DTS changes we need.
> >
> > The first step for that is to land that DTS file upstream in the Linux
> > kernel and then we should make a switch. The middle ground here won't
> > be maintainable.
> >
> >>
> >> I guess this is all new territory for us, but imho if we're adding
> >> support for a board that doesn't have an upstream DTS, we should still
> >> follow the same model, open to input here.
> >
> > With the DT rebasing tree, we actually want to make these bits clear.
> > Either the board support is fully upstream and then we make a switch
> > to upstream or you can have a u-boot specific DTS subset compliant
> > with upstream bindings while upstreaming is in progress. There is
> > always ABI risk involved for using half baked board DTS files.
>
> The DTS file submitted in your series is a copy paste of
> dragonboard410c.dts with the serial port adjusted, the LEDs removed, and
> the copyright changed...
>
> This will make it the only board still using the totally made up DTS
> style that all the Qualcomm boards used to use, while everything else is
> standardised on upstream (meaning they #include the SoC and PMIC dtsi
> files).
>
> It doesn't make a difference whether you put the board DT in dt-rebasing
> or if it just goes in U-Boot, it should be a DT derived from upstream
> not some hand-crafted thing which will never be supported. Just copy
> apq8016-sbc.dts when rebasing on my series and it will be fine.

The major bit that I was concerned about previously is if people start
to pass that u-boot specific DT to the kernel directly. But with
dt-rebasing at least we can distinguish among them. So I am fine to
use the format that you have described.

>
> That said, as I'm writing this I've realised that the pinctrl-apq8016
> driver is missing a patch to use the upstream GPIO naming. I'll fix this
> in the next revision of the qualcomm generic support series, but just a
> heads up if you run into that.

Good to know.

-Sumit

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/7] Add SE HMBSC board support
  2023-12-26  9:27               ` Sumit Garg
@ 2024-01-02  8:33                 ` Caleb Connolly
  0 siblings, 0 replies; 20+ messages in thread
From: Caleb Connolly @ 2024-01-02  8:33 UTC (permalink / raw)
  To: Sumit Garg
  Cc: Simon Glass, u-boot, neil.armstrong, lukma, seanga2, marex,
	laetitia.mariottini, pascal.eberhard, abdou.saker, jimmy.lalande,
	benjamin.missey, daniel.thompson



On 26/12/2023 10:27, Sumit Garg wrote:
> On Fri, 22 Dec 2023 at 23:04, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>>
>> [snip]
>>
>>>>>> Sumit, could you rebase this series on my generic board support? [1] in
>>>>>> it's current form this series conflicts, and includes some of the major
>>>>>> anti-patterns I'm trying to move away from in mach-snapdragon.
>>>>>
>>>>> Although, I haven't gone through your series but I was expecting those
>>>>> conflicts. Let's work together to make this series compatible on top
>>>>> of yours.
>>>>>
>>>>>>
>>>>>> You should not have to introduce a new CONFIG_TARGET_XYZ variable, and
>>>>>> from what I can tell you shouldn't even need to add the board/schneider
>>>>>> directory at all, you can just set the following in your defconfig:
>>>>>>
>>>>>> CONFIG_SYS_BOARD="dragonboard410c"
>>>>>
>>>>> This is simply a misnomer, its HMIBSC board. I suppose we should
>>>>> rather separate the SoC specific bits into mach-snapdragon and let
>>>>> different boards use them.
>>>>
>>>> Is there any reason why you can't just use the existing db410c board
>>>> code as-is? I don't see why you want to duplicate it.
>>>
>>> I don't want to duplicate it but rather we should make the common
>>> parts as part of arch/arm/mach-snapdragon/ and let derivative boards
>>> use them. The HMIBSC board is an industrial board which doesn't need
>>> any generic distro boot but rather FIT booting with OTA updates via
>>> RAUC [1] along with U-boot environment protection. Doesn't that make
>>> it different from db410c?
>>
>> Right, your series does this board specific configuration in the board
>> header file (include/configs/hmibsc.h) and in the defconfig.
>>
>> The actual board code in board/schneider/hmibsc/hmibsc.c is directly
>> copied from board/qualcomm/dragonboard410c/dragonboard410c.c with just
>> the copyright changed, the serial number code removed, and a style fix
>> to a comment. Ignoring for a moment the copyright issue which would
>> definitely need to be fixed,
> 
> Ah, I should have retained the copyright.
> 
>> what I'm suggesting that you do is just use
>> the board code from db410c.
> 
> This is just initial board support, a direct copy would just limit any
> further board support extensions.

Could you elaborate a little? If there are technical reasons to 
duplicate the board code then they should be explained in the commit 
message - and we can avoid this whole back and forth.
> 
>>
>> The way I have designed the generic board support is so that two similar
>> boards can share the same board code but with different config headers,
>> from what I can tell this would work just fine for you.
> 
> As I have mentioned earlier, the proper way to share code among boards
> is to have something like following:
> 
> arch/arm/mach-snapdragon/board_apq8016.c
> 
> You can take examples from arch/arm/mach-meson/board-*.c. If you don't
> want to do it as part of your series then let me know I will include
> it in this series.

I'm open to moving some of the code from board/qualcomm/dragonboard410c/ 
back to mach-snapdragon, I think my requirements for moving some feature 
code back are as follows:

  * Multiple unrelated Qualcomm boards use it
  * There is no clear path forward for upstream DT bindings
  * (and/therefore) Duplicating the function across boards is silly

Then we should definitely move it to mach-snapdragon.

I again want to be clear that this kind of board specific code IS a 
hack, it gets things working where we don't have a proper DT-based 
solution yet.

I know doing things properly is more effort, but I think with just a bit 
of effort we can find much better solutions that let us get rid of this 
board specific code all-together.
> 
>>
>> --- board/qualcomm/dragonboard410c/dragonboard410c.c
>> +++ board/schneider/hmibsc/hmibsc.c
>> @@ -2,5 +2,5 @@
>>   /*
>> - * Board init file for Dragonboard 410C
>> + * Board init file for SE HMIBSC
>>    *
>> - * (C) Copyright 2015 Mateusz Kulikowski <mateusz.kulikowski@gmail.com>
>> + * (C) Copyright 2023 Sumit Garg <sumit.garg@linaro.org>
>>    */
>> @@ -8,3 +8,2 @@
>>   #include <button.h>
>> -#include <common.h>
>>   #include <cpu_func.h>
>> @@ -137,7 +136,2 @@
>>   {
>> -       char serial[16];
>> -
>> -       memset(serial, 0, 16);
>> -       snprintf(serial, 13, "%x", msm_board_serial());
>> -       env_set("serial#", serial);
>>          return 0;
>> @@ -145,3 +139,4 @@
>>
>> -/* Fixup of DTB for Linux Kernel
>> +/*
>> + * Fixup of DTB for Linux Kernel
>>    * 1. Fixup installed DRAM.
>> @@ -165,3 +160,2 @@
>>
>> -
>>          if (!eth_env_get_enetaddr("btaddr", mac)) {
>> @@ -169,5 +163,6 @@
>>
>> -/* The BD address is same as WLAN MAC address but with
>> - * least significant bit flipped.
>> - */
>> +               /*
>> +                * The BD address is same as WLAN MAC address but with
>> +                * least significant bit flipped.
>> +                */
>>                  mac[0] ^= 0x01;
>>
>>>
>>> [1] https://rauc.io/
>>>
>>>>
>>>> The only reason the db410c and db820c have their board code is because
>>>> they're old platforms and already supported. For adding new support
>>>> there needs to be some very strong justification to have board-specific
>>>> C code.
>>>>
>>>> I think it would be nice to make the db410c code go away, or be toggled
>>>> at runtime, probably most of it will just work and not break any other
>>>> boards anyway. The db820c code is just part of what should be in the
>>>> pinctrl driver...
>>>>
>>>> Let's move away from this old model and towards having more generic
>>>> U-Boot images. This will snowball towards making future bringup even easier.
>>>
>>> As I have mentioned in the other thread, we should give alternatives
>>> to the board code as well. How would you handle the following?
>>>
>>> - Fastboot mode entry on a button press.
>>
>> This can be nicely handled through CONFIG_AUTOBOOT. I currently use
>> AUTOBOOT_STOP_STR but that's quite limited (only works for the power
>> button with "\r"). Probably the right solution here is to use
>> CONFIG_AUTOBOOT_USE_MENUKEY and introduce support for
>> CONFIG_AUTOBOOT_MENUBUTTON to allow specifying a named button instead of
>> an ASCII code. One would then configure "menucmd" in the U-Boot
>> environment to launch fastboot.
> 
> Won't this break existing users who relied on volume buttons to flash
> their board and rather have to purchase a debug uart?

I don't think so(???). This would highly depend on the device and 
configuration. As a developer you should have ways to recover from a 
borked U-Boot build, and you should make sure that any binary releases 
you make don't result in bricked boards.
> 
>>
>> This lets us drop all the weird non-standard keycode handling in board
>> code and just configure it in the defconfig instead. I plan to implement
>> this eventually but would for sure appreciate it if someone beat me to it.
>>> - Configure MAC address for network support.
>>
>> I believe you mentioned elsewhere some board with the MAC address on an
>> i2c EEPROM? The NVMEM framework solves exactly this issue, and U-Boot
>> already supports it, just add support to your ethernet driver to
>> retrieve its MAC address via NVMEM.
> 
> Good to know that.
> 
>>> - Setup board serial number.
>>
>> Same as above, the quick and dirty way to go would be to have
>> mach-snapdragon check for an alias defined in DT and expect that alias
>> to be an NVMEM cell which contains the serial number. On the Android
>> phones this is set in the kernel cmdline from ABL so I would probably
>> add some code to mach-snapdragon to check the bootargs for one.
> 
> And what about boards where U-Boot starts as the first stage
> boot-loader? Is there corresponding DT binding?

I really don't know. Having some code to read the MMC serial number and 
use it as the fallback board serial sounds reasonable to me.
> 
>>
>>>
>>> I suppose we don't need #ifdefry in the
>>> arch/arm/mach-snapdragon/board.c file, right?
>>
>> Nope, we aren't that limited on code size, and the runtime overhead of a
>> few extra branches is really not significant. If we do need to start
>> caring about either of those things then I would rather do this on a
>> per-feature basis than per-board.
> 
> The examples I gave were only limited to what are currently used by
> Qcom boards. But we should be open to future board support extensions
> too.

Absolutely!
> 
>>>
>>>>>
>>>>>> CONFIG_SYS_CONFIG_NAME="hmibsc"
>>>>>>
>>>>>> This will use the db410c board code (which yours is just a copy/paste of
>>>>>> from what I can tell) and your custom include/configs/hmibsc.h header.
>>>>>>
>>>>>> The addresses set in your environment file should be allocated
>>>>>> automatically at runtime too (see the ("mach-snapdragon: dynamic load
>>>>>> addresses") patch).
>>>>>>
>>>>>> You should also switch to an upstream board DTS based on my series, and
>>>>>> drop the "-uboot.dtsi" file.
>>>>>
>>>>> Unfortunately, currently there isn't any upstream DTS for this board
>>>>> but I will check with the SE team regarding their plans. Until then we
>>>>> have to use a U-Boot specific DTS file.
>>>>
>>>> In that case, perhaps we can take whatever DTS they're using in
>>>> production, or a version of it, and at least split the U-Boot changes
>>>> (if any) out into a separate file as I've done with the other platforms.
>>>> This way we stay consistent and can keep track of what U-Boot specific
>>>> DTS changes we need.
>>>
>>> The first step for that is to land that DTS file upstream in the Linux
>>> kernel and then we should make a switch. The middle ground here won't
>>> be maintainable.
>>>
>>>>
>>>> I guess this is all new territory for us, but imho if we're adding
>>>> support for a board that doesn't have an upstream DTS, we should still
>>>> follow the same model, open to input here.
>>>
>>> With the DT rebasing tree, we actually want to make these bits clear.
>>> Either the board support is fully upstream and then we make a switch
>>> to upstream or you can have a u-boot specific DTS subset compliant
>>> with upstream bindings while upstreaming is in progress. There is
>>> always ABI risk involved for using half baked board DTS files.
>>
>> The DTS file submitted in your series is a copy paste of
>> dragonboard410c.dts with the serial port adjusted, the LEDs removed, and
>> the copyright changed...
>>
>> This will make it the only board still using the totally made up DTS
>> style that all the Qualcomm boards used to use, while everything else is
>> standardised on upstream (meaning they #include the SoC and PMIC dtsi
>> files).
>>
>> It doesn't make a difference whether you put the board DT in dt-rebasing
>> or if it just goes in U-Boot, it should be a DT derived from upstream
>> not some hand-crafted thing which will never be supported. Just copy
>> apq8016-sbc.dts when rebasing on my series and it will be fine.
> 
> The major bit that I was concerned about previously is if people start
> to pass that u-boot specific DT to the kernel directly. But with
> dt-rebasing at least we can distinguish among them. So I am fine to
> use the format that you have described.

Ok, great!
> 
>>
>> That said, as I'm writing this I've realised that the pinctrl-apq8016
>> driver is missing a patch to use the upstream GPIO naming. I'll fix this
>> in the next revision of the qualcomm generic support series, but just a
>> heads up if you run into that.
> 
> Good to know.
> 
> -Sumit

-- 
// Caleb (they/them)

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2024-01-02  8:33 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-18  7:24 [PATCH 0/7] Add SE HMBSC board support Sumit Garg
2023-12-18  7:24 ` [PATCH 1/7] clk: apq8016: Add support for UART1 clocks Sumit Garg
2023-12-18  7:24 ` [PATCH 2/7] serial_msm: Add support for RS232 GPIOs Sumit Garg
2023-12-19 16:47   ` Caleb Connolly
2023-12-20 14:09     ` Sumit Garg
2023-12-18  7:24 ` [PATCH 3/7] serial_msm: Enable RS232 flow control Sumit Garg
2023-12-18  7:24 ` [PATCH 4/7] board: Add SE HMIBSC board support Sumit Garg
2023-12-18  7:24 ` [PATCH 5/7] hmibsc: Enable RAUC support Sumit Garg
2023-12-19 16:30   ` Caleb Connolly
2023-12-18  7:24 ` [PATCH 6/7] hmibsc: enable U-Boot Environment variables protection Sumit Garg
2023-12-18  7:24 ` [PATCH 7/7] hmibsc: Enable LAN75XX USB ethernet driver Sumit Garg
2023-12-18 15:01 ` [PATCH 0/7] Add SE HMBSC board support Simon Glass
2023-12-19  6:25   ` Sumit Garg
2023-12-19 16:26     ` Caleb Connolly
2023-12-20 13:36       ` Sumit Garg
2023-12-21 13:38         ` Caleb Connolly
2023-12-22  9:04           ` Sumit Garg
2023-12-22 17:34             ` Caleb Connolly
2023-12-26  9:27               ` Sumit Garg
2024-01-02  8:33                 ` Caleb Connolly

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.