[PATCH] cmd: mbr: Allow 4 MBR partitions without need for extended

2023-10-06 Thread Alexander Gendin
Current code allows up to 3 MBR partitions without extended one.
If more than 3 partitions are required, then extended partition(s)
must be used.
This commit allows up to 4 primary MBR partitions without the
need for extended partition.

Add mbr test unit. In order to use the test, mmc1.img file of size
12 MiB or greater is required in the same directory as u-boot.
Running mbr test is only supported in sandbox mode.

Signed-off-by: Alex Gendin 
---
 disk/part_dos.c   |   2 +-
 include/test/suites.h |   1 +
 test/cmd/Makefile |   1 +
 test/cmd/mbr.c| 440 ++
 test/cmd_ut.c |   4 +
 5 files changed, 447 insertions(+), 1 deletion(-)
 create mode 100644 test/cmd/mbr.c

diff --git a/disk/part_dos.c b/disk/part_dos.c
index 3337438437..567ead7511 100644
--- a/disk/part_dos.c
+++ b/disk/part_dos.c
@@ -466,7 +466,7 @@ int layout_mbr_partitions(struct disk_partition *p, int 
count,
ext = [i];
}
 
-   if (count < 4)
+   if (count <= 4)
return 0;
 
if (!ext) {
diff --git a/include/test/suites.h b/include/test/suites.h
index 1c7dc65966..51acbe47b2 100644
--- a/include/test/suites.h
+++ b/include/test/suites.h
@@ -45,6 +45,7 @@ int do_ut_font(struct cmd_tbl *cmdtp, int flag, int argc, 
char *const argv[]);
 int do_ut_lib(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
 int do_ut_loadm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
 int do_ut_log(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[]);
+int do_ut_mbr(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
 int do_ut_mem(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
 int do_ut_optee(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
 int do_ut_overlay(struct cmd_tbl *cmdtp, int flag, int argc,
diff --git a/test/cmd/Makefile b/test/cmd/Makefile
index 6e3d7e919e..2f251e07b4 100644
--- a/test/cmd/Makefile
+++ b/test/cmd/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_CMD_PINMUX) += pinmux.o
 obj-$(CONFIG_CMD_PWM) += pwm.o
 obj-$(CONFIG_CMD_SEAMA) += seama.o
 ifdef CONFIG_SANDBOX
+obj-$(CONFIG_CMD_MBR) += mbr.o
 obj-$(CONFIG_CMD_READ) += rw.o
 obj-$(CONFIG_CMD_SETEXPR) += setexpr.o
 obj-$(CONFIG_ARM_FFA_TRANSPORT) += armffa.o
diff --git a/test/cmd/mbr.c b/test/cmd/mbr.c
new file mode 100644
index 00..bedcef0638
--- /dev/null
+++ b/test/cmd/mbr.c
@@ -0,0 +1,440 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Tests for mbr command
+ *
+ * Copyright 2023 Matrox Video
+ * Written by Alex Gendin 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+DECLARE_GLOBAL_DATA_PTR;
+/*
+ * Test requirements:
+ * mmc1.img - File size needs to be at least 12 MiB
+ *
+ * Command to create mmc1.img:
+ * $ dd if=/dev/zero of=mmc1.img bs=12M count=1
+ *
+ * Place mmc1.img into the same directory as sandboxed u-boot
+ *
+ * To run this test manually:
+ * $ ./u-boot -Tc 'ut mbr'
+ */
+
+static char * mbr_parts_header = "setenv mbr_parts '";
+static char * mbr_parts_p1 = 
"uuid_disk=0x12345678;name=p1,start=8M,bootable,size=1M,id=0x0e";
+static char * mbr_parts_p2 = ";name=p2,size=1M,id=0x0e";
+static char * mbr_parts_p3 = ";name=p3,size=1M,id=0x0e";
+static char * mbr_parts_p4 = ";name=p4,size=1M,id=0x0e";
+static char * mbr_parts_p5 = 
";name=[ext],size=2M,id=0x05;name=p5,size=1M,id=0x0e";
+static char * mbr_parts_tail = "'";
+
+/*
+ * One MBR partition
+01b0  00 00 00 00 00 00 00 00  78 56 34 12 00 00 80 05  |xV4.|
+01c0  05 01 0e 25 24 01 00 40  00 00 00 08 00 00 00 00  |...%$..@|
+01d0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ||
+01e0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ||
+01f0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 55 aa  |..U.|
+*/
+static unsigned mbr_cmp_start = 0x1B8;
+static unsigned mbr_cmp_size = 0x48;
+static unsigned char mbr_parts_ref_p1[] = {
+0x78, 0x56, 0x34, 0x12, 0x00, 
0x00, 0x80, 0x05,
+0x05, 0x01, 0x0e, 0x25, 0x24, 0x01, 0x00, 0x40, 0x00, 0x00, 0x00, 0x08, 0x00, 
0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 
0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 
0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 
0x00, 0x55, 0xaa
+};
+
+/*
+ * Two MBR partitions
+01b0  00 00 00 00 00 00 00 00  78 56 34 12 00 00 80 05  |xV4.|
+01c0  05 01 0e 25 24 01 00 40  00 00 00 08 00 00 00 25  |...%$..@...%|
+01d0  25 01 0e 46 05 01 00 48  00 00 00 08 00 00 00 00  |%..F...H|
+01e0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ||
+01f0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 55 aa  |..U.|
+*/
+static unsigned char mbr_parts_ref_p2[] = {
+   

[PATCH 1/1] [u-boot][master][PATCH v5] pico-imx7d: add baseboard SD card boot detect

2023-10-06 Thread egyszeregy
From: Benjamin Szőke 

Take over codes from Techenxion to support
mmc autodetect boot for pico-imx7d.

Signed-off-by: Benjamin Szőke 
---
 board/technexion/pico-imx7d/pico-imx7d.c | 57 +++
 board/technexion/pico-imx7d/spl.c| 91 ++--
 include/configs/pico-imx7d.h |  4 +-
 3 files changed, 144 insertions(+), 8 deletions(-)

diff --git a/board/technexion/pico-imx7d/pico-imx7d.c 
b/board/technexion/pico-imx7d/pico-imx7d.c
index 6e98b85b28..a3fa915b49 100644
--- a/board/technexion/pico-imx7d/pico-imx7d.c
+++ b/board/technexion/pico-imx7d/pico-imx7d.c
@@ -5,6 +5,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -13,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -129,6 +131,49 @@ int board_phy_config(struct phy_device *phydev)
 }
 #endif
 
+#if CONFIG_IS_ENABLED(FSL_ESDHC_IMX)
+int check_mmc_autodetect(void)
+{
+   char *autodetect_str = env_get("mmcautodetect");
+
+   if ((autodetect_str) &&
+   (strcmp(autodetect_str, "yes") == 0)) {
+   return 1;
+   }
+
+   return 0;
+}
+
+void board_late_mmc_init(void)
+{
+   int dev_no = 0;
+   char cmd[32];
+
+   if (!check_mmc_autodetect())
+   return;
+
+   switch (get_boot_device()) {
+   case SD3_BOOT:
+   case MMC3_BOOT:
+   env_set("bootdev", "MMC3");
+   dev_no = 2;
+   break;
+   case SD1_BOOT:
+   env_set("bootdev", "SD1");
+   dev_no = 0;
+   break;
+   default:
+   printf("Wrong boot device!");
+   }
+
+   /* Set mmcdev env */
+   env_set_ulong("mmcdev", dev_no);
+
+   sprintf(cmd, "mmc dev %d", dev_no);
+   run_command(cmd, 0);
+}
+#endif
+
 static void setup_iomux_uart(void)
 {
imx_iomux_v3_setup_multiple_pads(uart5_pads, ARRAY_SIZE(uart5_pads));
@@ -176,6 +221,12 @@ int board_late_init(void)
 
set_wdog_reset(wdog);
 
+#if CONFIG_IS_ENABLED(FSL_ESDHC_IMX)
+#if defined(CONFIG_ENV_IS_IN_MMC) || defined(CONFIG_ENV_IS_NOWHERE)
+   board_late_mmc_init();
+#endif /* CONFIG_ENV_IS_IN_MMC or CONFIG_ENV_IS_NOWHERE */
+#endif
+
/*
 * Do not assert internal WDOG_RESET_B_DEB(controlled by bit 4),
 * since we use PMIC_PWRON to reset the board.
@@ -210,3 +261,9 @@ int board_ehci_hcd_init(int port)
}
return 0;
 }
+
+/* This should be defined for each board */
+__weak int mmc_map_to_kernel_blk(int dev_no)
+{
+   return dev_no;
+}
diff --git a/board/technexion/pico-imx7d/spl.c 
b/board/technexion/pico-imx7d/spl.c
index c6b21aaa42..2fe76145be 100644
--- a/board/technexion/pico-imx7d/spl.c
+++ b/board/technexion/pico-imx7d/spl.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -159,7 +160,20 @@ void reset_cpu(void)
 #define USDHC_PAD_CTRL (PAD_CTL_DSE_3P3V_32OHM | PAD_CTL_SRE_SLOW | \
PAD_CTL_HYS | PAD_CTL_PUE | PAD_CTL_PUS_PU47KOHM)
 
-static iomux_v3_cfg_t const usdhc3_pads[] = {
+#define USDHC1_CD_GPIO IMX_GPIO_NR(5, 0)
+/* EMMC/SD */
+static const iomux_v3_cfg_t usdhc1_pads[] = {
+   MX7D_PAD_SD1_CLK__SD1_CLK | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+   MX7D_PAD_SD1_CMD__SD1_CMD | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+   MX7D_PAD_SD1_DATA0__SD1_DATA0 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+   MX7D_PAD_SD1_DATA1__SD1_DATA1 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+   MX7D_PAD_SD1_DATA2__SD1_DATA2 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+   MX7D_PAD_SD1_DATA3__SD1_DATA3 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+   MX7D_PAD_SD1_CD_B__GPIO5_IO0  | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+};
+
+#define USDHC3_CD_GPIO IMX_GPIO_NR(1, 14)
+static const iomux_v3_cfg_t usdhc3_emmc_pads[] = {
MX7D_PAD_SD3_CLK__SD3_CLK | MUX_PAD_CTRL(USDHC_PAD_CTRL),
MX7D_PAD_SD3_CMD__SD3_CMD | MUX_PAD_CTRL(USDHC_PAD_CTRL),
MX7D_PAD_SD3_DATA0__SD3_DATA0 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
@@ -173,20 +187,83 @@ static iomux_v3_cfg_t const usdhc3_pads[] = {
MX7D_PAD_GPIO1_IO14__GPIO1_IO14 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
 };
 
-static struct fsl_esdhc_cfg usdhc_cfg[1] = {
+static struct fsl_esdhc_cfg usdhc_cfg[2] = {
{USDHC3_BASE_ADDR},
+   {USDHC1_BASE_ADDR},
 };
 
 int board_mmc_getcd(struct mmc *mmc)
 {
-   /* Assume uSDHC3 emmc is always present */
-   return 1;
+   struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
+   int ret = 0;
+
+   switch (cfg->esdhc_base) {
+   case USDHC1_BASE_ADDR:
+   ret = !gpio_get_value(USDHC1_CD_GPIO); /* Assume uSDHC1 sd is 
always present */
+   break;
+   case USDHC3_BASE_ADDR:
+   ret = !gpio_get_value(USDHC3_CD_GPIO); /* Assume uSDHC3 emmc is 
always present */
+   break;
+   }
+
+   return ret;
 }
 
 int board_mmc_init(struct bd_info *bis)
 {
-   imx_iomux_v3_setup_multiple_pads(usdhc3_pads, ARRAY_SIZE(usdhc3_pads));
-   

[RESEND PATCH] bootstd: sata: bootdev scanning for ahci sata with no drive attached

2023-10-06 Thread Tony Dinh
It's normal to have no SATA drive attached to the controller, so return a
successful status when there is no block device found after probing.

Note: this patch depends on the previous patch
https://patchwork.ozlabs.org/project/uboot/patch/20230917230649.30357-1-mibo...@gmail.com/

Resend the right patch.

Signed-off-by: Tony Dinh 
---

 drivers/ata/sata.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/ata/sata.c b/drivers/ata/sata.c
index f126b84e05..dcb5fcf476 100644
--- a/drivers/ata/sata.c
+++ b/drivers/ata/sata.c
@@ -79,6 +79,12 @@ int sata_rescan(bool verbose)
 
ret = uclass_probe_all(UCLASS_AHCI);
 
+   if (ret == -ENODEV) {
+   if (verbose)
+   printf("No SATA block device found\n");
+   return 0;
+   }
+
return ret;
 }
 
-- 
2.39.2



Re: [PATCH 2/2] configs: rockchip: rk3308: enable CONFIG_OF_LIBFDT_OVERLAY

2023-10-06 Thread Kever Yang



On 2023/9/11 18:01, FUKAUMI Naoki wrote:

enable CONFIG_OF_LIBFDT_OVERLAY and use it on Radxa ROCK Pi S.

Signed-off-by: FUKAUMI Naoki 

Reviewed-by: Kever Yang 

Thanks,
- Kever

---
  configs/rock-pi-s-rk3308_defconfig | 1 +
  include/configs/rk3308_common.h| 1 +
  2 files changed, 2 insertions(+)

diff --git a/configs/rock-pi-s-rk3308_defconfig 
b/configs/rock-pi-s-rk3308_defconfig
index e2abe4b4f7..ca4a1800e7 100644
--- a/configs/rock-pi-s-rk3308_defconfig
+++ b/configs/rock-pi-s-rk3308_defconfig
@@ -9,6 +9,7 @@ CONFIG_SPL_LIBGENERIC_SUPPORT=y
  CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
  CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x80
  CONFIG_DEFAULT_DEVICE_TREE="rk3308-rock-pi-s"
+CONFIG_OF_LIBFDT_OVERLAY=y
  CONFIG_DM_RESET=y
  CONFIG_ROCKCHIP_RK3308=y
  CONFIG_ROCKCHIP_SPL_RESERVE_IRAM=0x0
diff --git a/include/configs/rk3308_common.h b/include/configs/rk3308_common.h
index a413af1bd4..861154fbeb 100644
--- a/include/configs/rk3308_common.h
+++ b/include/configs/rk3308_common.h
@@ -17,6 +17,7 @@
"scriptaddr=0x0050\0" \
"pxefile_addr_r=0x0060\0" \
"fdt_addr_r=0x03e0\0" \
+   "fdtoverlay_addr_r=0x03f0\0" \
"kernel_addr_r=0x0068\0" \
"ramdisk_addr_r=0x0400\0"
  


Re: [PATCH 1/2] configs: rockchip: rk3308: use CONFIG_DEFAULT_FDT_FILE

2023-10-06 Thread Kever Yang



On 2023/9/11 18:01, FUKAUMI Naoki wrote:

all rk3308 boards should use their own dtb file.

also, change fdt_addr_r to avoid following error:
  "ERROR: Did not find a cmdline Flattened Device Tree"
it happens on Radxa ROCK Pi S (256MB/512MB) with kernel built from
Radxa BSP.

Signed-off-by: FUKAUMI Naoki 

Reviewed-by: Kever Yang 

Thanks,
- Kever

---
  configs/evb-rk3308_defconfig   | 1 +
  configs/roc-cc-rk3308_defconfig| 1 +
  configs/rock-pi-s-rk3308_defconfig | 1 +
  include/configs/rk3308_common.h| 3 ++-
  4 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/configs/evb-rk3308_defconfig b/configs/evb-rk3308_defconfig
index a13a809c1e..2c7ac88ed9 100644
--- a/configs/evb-rk3308_defconfig
+++ b/configs/evb-rk3308_defconfig
@@ -24,6 +24,7 @@ CONFIG_ANDROID_BOOT_IMAGE=y
  CONFIG_FIT=y
  CONFIG_FIT_VERBOSE=y
  CONFIG_BOOTDELAY=0
+CONFIG_DEFAULT_FDT_FILE="rockchip/rk3308-evb.dtb"
  CONFIG_SYS_CONSOLE_INFO_QUIET=y
  # CONFIG_DISPLAY_CPUINFO is not set
  CONFIG_SPL_MAX_SIZE=0x2
diff --git a/configs/roc-cc-rk3308_defconfig b/configs/roc-cc-rk3308_defconfig
index 9a789b212f..9cb90f6ee6 100644
--- a/configs/roc-cc-rk3308_defconfig
+++ b/configs/roc-cc-rk3308_defconfig
@@ -24,6 +24,7 @@ CONFIG_ANDROID_BOOT_IMAGE=y
  CONFIG_FIT=y
  CONFIG_FIT_VERBOSE=y
  CONFIG_BOOTDELAY=0
+CONFIG_DEFAULT_FDT_FILE="rockchip/rk3308-roc-cc.dtb"
  CONFIG_SYS_CONSOLE_INFO_QUIET=y
  # CONFIG_DISPLAY_CPUINFO is not set
  CONFIG_SPL_MAX_SIZE=0x2
diff --git a/configs/rock-pi-s-rk3308_defconfig 
b/configs/rock-pi-s-rk3308_defconfig
index cc3274a98b..e2abe4b4f7 100644
--- a/configs/rock-pi-s-rk3308_defconfig
+++ b/configs/rock-pi-s-rk3308_defconfig
@@ -25,6 +25,7 @@ CONFIG_ANDROID_BOOT_IMAGE=y
  CONFIG_FIT=y
  CONFIG_FIT_VERBOSE=y
  CONFIG_BOOTDELAY=0
+CONFIG_DEFAULT_FDT_FILE="rockchip/rk3308-rock-pi-s.dtb"
  CONFIG_SYS_CONSOLE_INFO_QUIET=y
  # CONFIG_DISPLAY_CPUINFO is not set
  CONFIG_SPL_MAX_SIZE=0x2
diff --git a/include/configs/rk3308_common.h b/include/configs/rk3308_common.h
index 7d55fcd975..a413af1bd4 100644
--- a/include/configs/rk3308_common.h
+++ b/include/configs/rk3308_common.h
@@ -16,11 +16,12 @@
  #define ENV_MEM_LAYOUT_SETTINGS \
"scriptaddr=0x0050\0" \
"pxefile_addr_r=0x0060\0" \
-   "fdt_addr_r=0x0280\0" \
+   "fdt_addr_r=0x03e0\0" \
"kernel_addr_r=0x0068\0" \
"ramdisk_addr_r=0x0400\0"
  
  #define CFG_EXTRA_ENV_SETTINGS \

+   "fdtfile=" CONFIG_DEFAULT_FDT_FILE "\0" \
ENV_MEM_LAYOUT_SETTINGS \
"partitions=" PARTS_DEFAULT \
ROCKCHIP_DEVICE_SETTINGS \


Re: [PATCH v2 2/2] arm: dts: rockchip: rock-5b: add support for PCIe3 and NVMe

2023-10-06 Thread Kever Yang



On 2023/9/5 19:47, FUKAUMI Naoki wrote:

this patch adds support for PCIe3 (M.2 M key) and enables NVMe.

  => pci
  BusDevFun  VendorId   DeviceId   Device Class   Sub-Class
  _
  00.00.00   0x1d87 0x3588 Bridge device   0x04
  01.00.00   0x10ec 0x8125 Network controller  0x00
  02.00.00   0x1d87 0x3588 Bridge device   0x04
  03.00.00   0x1179 0x011a Mass storage controller 0x08
  => nvme scan
  => nvme info
  Device 0: Vendor: 0x1179 Rev: AGHA4101 Prod: 79CA20WPKRYN
  Type: Hard Disk
  Capacity: 488386.3 MB = 476.9 GB (1000215216 x 512)

Signed-off-by: FUKAUMI Naoki 

Reviewed-by: Kever Yang 

Thanks,
- Kever


this patch depends:
- "rockchip: rk3568: Fix use of PCIe bifurcation" [1]
[1] https://patchwork.ozlabs.org/project/uboot/list/?series=366997
---
  arch/arm/dts/rk3588-rock-5b-u-boot.dtsi | 33 +
  configs/rock5b-rk3588_defconfig |  1 +
  2 files changed, 34 insertions(+)

diff --git a/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi 
b/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi
index 03626e71ea..96cc84e5aa 100644
--- a/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi
+++ b/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi
@@ -23,6 +23,19 @@
regulator-max-microvolt = <1200>;
};
  
+	vcc3v3_pcie30: vcc3v3-pcie30-regulator {

+   compatible = "regulator-fixed";
+   regulator-name = "vcc3v3_pcie30";
+   regulator-min-microvolt = <330>;
+   regulator-max-microvolt = <330>;
+   enable-active-high;
+   gpios = < RK_PA4 GPIO_ACTIVE_HIGH>;
+   startup-delay-us = <5000>;
+   vin-supply = <_sys>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_vcc3v3_en>;
+   };
+
vcc5v0_usbdcin: vcc5v0-usbdcin {
compatible = "regulator-fixed";
regulator-name = "vcc5v0_usbdcin";
@@ -71,6 +84,18 @@
status = "okay";
  };
  
+ {

+   status = "okay";
+};
+
+ {
+   reset-gpios = < RK_PB6 GPIO_ACTIVE_HIGH>;
+   vpcie3v3-supply = <_pcie30>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_rst>;
+   status = "okay";
+};
+
   {
pcie {
pcie_reset_h: pcie-reset-h {
@@ -81,6 +106,14 @@
rockchip,pins = <3 RK_PC7 4 _pull_none>,
<3 RK_PD0 4 _pull_none>;
};
+
+   pcie3_rst: pcie3-rst {
+   rockchip,pins = <4 RK_PB6 RK_FUNC_GPIO _pull_none>;
+   };
+
+   pcie3_vcc3v3_en: pcie3-vcc3v3-en {
+   rockchip,pins = <1 RK_PA4 RK_FUNC_GPIO _pull_none>;
+   };
};
  
  	usb-typec {

diff --git a/configs/rock5b-rk3588_defconfig b/configs/rock5b-rk3588_defconfig
index 3fa65cbf9b..50551c70f2 100644
--- a/configs/rock5b-rk3588_defconfig
+++ b/configs/rock5b-rk3588_defconfig
@@ -81,6 +81,7 @@ CONFIG_SPI_FLASH_XTX=y
  CONFIG_ETH_DESIGNWARE=y
  CONFIG_RTL8169=y
  CONFIG_GMAC_ROCKCHIP=y
+CONFIG_NVME_PCI=y
  CONFIG_PCIE_DW_ROCKCHIP=y
  CONFIG_PHY_ROCKCHIP_INNO_USB2=y
  CONFIG_PHY_ROCKCHIP_NANENG_COMBOPHY=y


Re: [PATCH v2 1/2] arm: dts: rockchip: sync DT for RK3588 series with Linux

2023-10-06 Thread Kever Yang



On 2023/9/5 19:47, FUKAUMI Naoki wrote:

Sync the device tree for RK3588 series with Linux 6.6-rc1.

Signed-off-by: FUKAUMI Naoki 

Reviewed-by: Kever Yang 

Thanks,
- Kever

---
  .../dts/rk3588-edgeble-neu6a-io-u-boot.dtsi   |   1 -
  arch/arm/dts/rk3588-edgeble-neu6a.dtsi|   1 -
  .../dts/rk3588-edgeble-neu6b-io-u-boot.dtsi   |   6 -
  arch/arm/dts/rk3588-edgeble-neu6b-io.dts  |  66 ++
  arch/arm/dts/rk3588-edgeble-neu6b.dtsi| 359 -
  arch/arm/dts/rk3588-evb1-v10.dts  | 720 +-
  arch/arm/dts/rk3588-rock-5b-u-boot.dtsi   | 113 +--
  arch/arm/dts/rk3588-rock-5b.dts   | 448 ++-
  arch/arm/dts/rk3588.dtsi  | 215 ++
  arch/arm/dts/rk3588s-rock-5a-u-boot.dtsi  |  12 -
  arch/arm/dts/rk3588s-rock-5a.dts  | 665 +++-
  arch/arm/dts/rk3588s-u-boot.dtsi  | 162 
  arch/arm/dts/rk3588s.dtsi | 367 +
  include/dt-bindings/ata/ahci.h|  20 +
  14 files changed, 2866 insertions(+), 289 deletions(-)
  create mode 100644 include/dt-bindings/ata/ahci.h

diff --git a/arch/arm/dts/rk3588-edgeble-neu6a-io-u-boot.dtsi 
b/arch/arm/dts/rk3588-edgeble-neu6a-io-u-boot.dtsi
index 373f369c65..dd0058262b 100644
--- a/arch/arm/dts/rk3588-edgeble-neu6a-io-u-boot.dtsi
+++ b/arch/arm/dts/rk3588-edgeble-neu6a-io-u-boot.dtsi
@@ -11,7 +11,6 @@
};
  
  	chosen {

-   stdout-path = 
u-boot,spl-boot-order = 
};
  };
diff --git a/arch/arm/dts/rk3588-edgeble-neu6a.dtsi 
b/arch/arm/dts/rk3588-edgeble-neu6a.dtsi
index 38e1a1e25f..727580aaa1 100644
--- a/arch/arm/dts/rk3588-edgeble-neu6a.dtsi
+++ b/arch/arm/dts/rk3588-edgeble-neu6a.dtsi
@@ -25,7 +25,6 @@
no-sdio;
no-sd;
non-removable;
-   max-frequency = <2>;
mmc-hs400-1_8v;
mmc-hs400-enhanced-strobe;
status = "okay";
diff --git a/arch/arm/dts/rk3588-edgeble-neu6b-io-u-boot.dtsi 
b/arch/arm/dts/rk3588-edgeble-neu6b-io-u-boot.dtsi
index cd7626b24b..a45b3f5e86 100644
--- a/arch/arm/dts/rk3588-edgeble-neu6b-io-u-boot.dtsi
+++ b/arch/arm/dts/rk3588-edgeble-neu6b-io-u-boot.dtsi
@@ -11,12 +11,6 @@
};
  
  	chosen {

-   stdout-path = 
u-boot,spl-boot-order = 
};
  };
-
- {
-   bus-width = <4>;
-   status = "okay";
-};
diff --git a/arch/arm/dts/rk3588-edgeble-neu6b-io.dts 
b/arch/arm/dts/rk3588-edgeble-neu6b-io.dts
index e9d5a8bab5..9933765e40 100644
--- a/arch/arm/dts/rk3588-edgeble-neu6b-io.dts
+++ b/arch/arm/dts/rk3588-edgeble-neu6b-io.dts
@@ -21,7 +21,73 @@
};
  };
  
+_ps {

+   status = "okay";
+};
+
+ {
+   status = "okay";
+
+   hym8563: rtc@51 {
+   compatible = "haoyu,hym8563";
+   reg = <0x51>;
+   interrupt-parent = <>;
+   interrupts = ;
+   #clock-cells = <0>;
+   clock-output-names = "hym8563";
+   pinctrl-names = "default";
+   pinctrl-0 = <_int>;
+   wakeup-source;
+   };
+};
+
+ {
+   hym8563 {
+   hym8563_int: hym8563-int {
+   rockchip,pins = <0 RK_PB0 RK_FUNC_GPIO _pull_none>;
+   };
+   };
+};
+
+/* FAN */
+ {
+   pinctrl-0 = <_pins>;
+   pinctrl-names = "default";
+   status = "okay";
+};
+
+ {
+   status = "okay";
+};
+
+ {
+   bus-width = <4>;
+   cap-mmc-highspeed;
+   cap-sd-highspeed;
+   disable-wp;
+   no-sdio;
+   no-mmc;
+   sd-uhs-sdr104;
+   vmmc-supply = <_3v3_s3>;
+   vqmmc-supply = <_sd_s0>;
+   status = "okay";
+};
+
   {
pinctrl-0 = <_xfer>;
status = "okay";
  };
+
+/* RS232 */
+ {
+   pinctrl-0 = <_xfer>;
+   pinctrl-names = "default";
+   status = "okay";
+};
+
+/* RS485 */
+ {
+   pinctrl-0 = <_xfer>;
+   pinctrl-names = "default";
+   status = "okay";
+};
diff --git a/arch/arm/dts/rk3588-edgeble-neu6b.dtsi 
b/arch/arm/dts/rk3588-edgeble-neu6b.dtsi
index 1c5bcf1280..017559bba3 100644
--- a/arch/arm/dts/rk3588-edgeble-neu6b.dtsi
+++ b/arch/arm/dts/rk3588-edgeble-neu6b.dtsi
@@ -18,6 +18,42 @@
regulator-min-microvolt = <1200>;
regulator-max-microvolt = <1200>;
};
+
+   vcc5v0_sys: vcc5v0-sys-regulator {
+   compatible = "regulator-fixed";
+   regulator-name = "vcc5v0_sys";
+   regulator-always-on;
+   regulator-boot-on;
+   regulator-min-microvolt = <500>;
+   regulator-max-microvolt = <500>;
+   vin-supply = <_dcin>;
+   };
+
+   vcc_1v1_nldo_s3: vcc-1v1-nldo-s3-regulator {
+   compatible = "regulator-fixed";
+   regulator-name = "vcc_1v1_nldo_s3";
+   regulator-always-on;
+   regulator-boot-on;
+   regulator-min-microvolt = <110>;
+   

Re: [PATCH] board: rockchip: add Pine64 QuartzPro64 RK3588 board

2023-10-06 Thread Kever Yang

Hi Tom,

    Please add the info about where is the dts from in you v2, eg. a 
tag in mainline kernel.



Thanks,

- Kever

On 2023/10/3 23:34, Tom Fitzhenry wrote:

Tom Fitzhenry  writes:


QuartzPro64 is a Rockchip RK3588 based SBC by Pine64.

UART and booting over SD card are tested to work.

I've found a few issues with this patch, incl. confusion over the
relationship between rk3588-quartzpro64.dts and
rk3588-quartzpro64-u-boot.dtsi .

I will rework this patch as a v2, and include eMMC support too.


Re: [PATCH] bootstd: sata: bootdev scanning for ahci sata with no drive attached

2023-10-06 Thread Tony Dinh
On Fri, Oct 6, 2023 at 5:49 PM Tony Dinh  wrote:
>
> It's normal to have no SATA drive attached to the controller, so return a
> successful status when there is no block device found after probing.
>
> Note: this patch depends on the previous patch
> https://patchwork.ozlabs.org/project/uboot/patch/20230917230649.30357-1-mibo...@gmail.com/
>
> Signed-off-by: Tony Dinh 
> ---
>
>  drivers/ata/sata.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/ata/sata.c b/drivers/ata/sata.c
> index f126b84e05..59c40a829f 100644
> --- a/drivers/ata/sata.c
> +++ b/drivers/ata/sata.c
> @@ -79,6 +79,11 @@ int sata_rescan(bool verbose)
>
> ret = uclass_probe_all(UCLASS_AHCI);
>
> +   if (verbose && ret == -ENODEV) {
> +   printf("No SATA block device found\n");
> +   return 0;
> +   }
> +
> return ret;
>  }
>
> --
> 2.39.2
>

Sorry, I need to resend this patch (fat finger).

All the best,
Tony


Re: [PATCH] configs: rockchip: rock-pi-s: use default bootdelay (2s)

2023-10-06 Thread Kever Yang



On 2023/9/11 18:05, FUKAUMI Naoki wrote:

align with other boards.

Signed-off-by: FUKAUMI Naoki 

Reviewed-by: Kever Yang 

Thanks,
- Kever

---
  configs/rock-pi-s-rk3308_defconfig | 1 -
  1 file changed, 1 deletion(-)

diff --git a/configs/rock-pi-s-rk3308_defconfig 
b/configs/rock-pi-s-rk3308_defconfig
index cc3274a98b..cd0a996ee7 100644
--- a/configs/rock-pi-s-rk3308_defconfig
+++ b/configs/rock-pi-s-rk3308_defconfig
@@ -24,7 +24,6 @@ CONFIG_DEBUG_UART=y
  CONFIG_ANDROID_BOOT_IMAGE=y
  CONFIG_FIT=y
  CONFIG_FIT_VERBOSE=y
-CONFIG_BOOTDELAY=0
  CONFIG_SYS_CONSOLE_INFO_QUIET=y
  # CONFIG_DISPLAY_CPUINFO is not set
  CONFIG_SPL_MAX_SIZE=0x2


Re: [PATCH v2] configs: rockchip: add DOS_PARTITION to RK3308 boards defconfig

2023-10-06 Thread Kever Yang



On 2023/10/1 22:15, Massimo Pegorer wrote:

Without DOS_PARTITION support U-Boot is not able to boot an OS stored
into an SD card with MBR partitions table. This is still a quite common
case so add DOS_PARTITION (only for U-Boot proper build) to Rockchip
RK3308 EVB, Radxa ROCK Pi S and Firefly roc-rk3308-cc boards: they are
the only RK boards missing of DOS_PARTITION.

Reported-by: Jayantajit Gogoi 
Signed-off-by: Massimo Pegorer 

Reviewed-by: Kever Yang 

Thanks,
- Kever

---
  configs/evb-rk3308_defconfig   | 2 +-
  configs/roc-cc-rk3308_defconfig| 2 +-
  configs/rock-pi-s-rk3308_defconfig | 2 +-
  3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/configs/evb-rk3308_defconfig b/configs/evb-rk3308_defconfig
index a13a809c1e..c6472a2c9c 100644
--- a/configs/evb-rk3308_defconfig
+++ b/configs/evb-rk3308_defconfig
@@ -47,7 +47,7 @@ CONFIG_CMD_USB_MASS_STORAGE=y
  # CONFIG_CMD_ITEST is not set
  # CONFIG_CMD_SETEXPR is not set
  # CONFIG_CMD_SLEEP is not set
-# CONFIG_DOS_PARTITION is not set
+# CONFIG_SPL_DOS_PARTITION is not set
  # CONFIG_ISO_PARTITION is not set
  CONFIG_EFI_PARTITION_ENTRIES_NUMBERS=64
  CONFIG_SPL_OF_CONTROL=y
diff --git a/configs/roc-cc-rk3308_defconfig b/configs/roc-cc-rk3308_defconfig
index 9a789b212f..ca92b8f744 100644
--- a/configs/roc-cc-rk3308_defconfig
+++ b/configs/roc-cc-rk3308_defconfig
@@ -47,7 +47,7 @@ CONFIG_CMD_USB_MASS_STORAGE=y
  # CONFIG_CMD_ITEST is not set
  # CONFIG_CMD_SETEXPR is not set
  # CONFIG_CMD_SLEEP is not set
-# CONFIG_DOS_PARTITION is not set
+# CONFIG_SPL_DOS_PARTITION is not set
  # CONFIG_ISO_PARTITION is not set
  CONFIG_EFI_PARTITION_ENTRIES_NUMBERS=64
  CONFIG_SPL_OF_CONTROL=y
diff --git a/configs/rock-pi-s-rk3308_defconfig 
b/configs/rock-pi-s-rk3308_defconfig
index cc3274a98b..1c1fdc6611 100644
--- a/configs/rock-pi-s-rk3308_defconfig
+++ b/configs/rock-pi-s-rk3308_defconfig
@@ -48,7 +48,7 @@ CONFIG_CMD_USB_MASS_STORAGE=y
  # CONFIG_CMD_ITEST is not set
  # CONFIG_CMD_SETEXPR is not set
  # CONFIG_CMD_SLEEP is not set
-# CONFIG_DOS_PARTITION is not set
+# CONFIG_SPL_DOS_PARTITION is not set
  # CONFIG_ISO_PARTITION is not set
  CONFIG_EFI_PARTITION_ENTRIES_NUMBERS=64
  CONFIG_SPL_OF_CONTROL=y


Re: [PATCH 05/25] treewide: Correct use of long help

2023-10-06 Thread Tom Rini
On Fri, Oct 06, 2023 at 04:42:44PM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Fri, 6 Oct 2023 at 10:55, Tom Rini  wrote:
> >
> > On Fri, Oct 06, 2023 at 07:03:17AM -0600, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Thu, 5 Oct 2023 at 20:16, Tom Rini  wrote:
> > > >
> > > > On Thu, Oct 05, 2023 at 07:41:49PM -0600, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Thu, 5 Oct 2023 at 08:53, Tom Rini  wrote:
> > > > > >
> > > > > > On Wed, Oct 04, 2023 at 07:23:47PM -0600, Simon Glass wrote:
> > > > > > > Hi Tom,
> > > > > > >
> > > > > > > On Sun, 24 Sept 2023 at 17:27, Tom Rini  
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > On Sun, Sep 24, 2023 at 02:39:23PM -0600, Simon Glass wrote:
> > > > > > > > > Some commands assume that CONFIG_SYS_LONGHELP is always 
> > > > > > > > > defined.
> > > > > > > > > Declaration of long help should be bracketed by an #ifdef to 
> > > > > > > > > avoid an
> > > > > > > > > 'unused variable' warning.
> > > > > > > > >
> > > > > > > > > Fix this treewide.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Simon Glass 
> > > > > > > > [snip]
> > > > > > > > > diff --git a/arch/arm/mach-imx/cmd_dek.c 
> > > > > > > > > b/arch/arm/mach-imx/cmd_dek.c
> > > > > > > > > index 6fa5b41fcd38..25ea7d3b37da 100644
> > > > > > > > > --- a/arch/arm/mach-imx/cmd_dek.c
> > > > > > > > > +++ b/arch/arm/mach-imx/cmd_dek.c
> > > > > > > > > @@ -393,11 +393,12 @@ static int do_dek_blob(struct cmd_tbl 
> > > > > > > > > *cmdtp, int flag, int argc,
> > > > > > > > >   return blob_encap_dek(src_addr, dst_addr, len);
> > > > > > > > >  }
> > > > > > > > >
> > > > > > > > > -/***/
> > > > > > > > > +#if IS_ENABLED(CONFIG_SYS_LONGHELP)
> > > > > > > > >  static char dek_blob_help_text[] =
> > > > > > > > >   "src dst len- Encapsulate and create blob 
> > > > > > > > > of data\n"
> > > > > > > > >   " $len bits long at address 
> > > > > > > > > $src and\n"
> > > > > > > > >   " store the result at address 
> > > > > > > > > $dst.\n";
> > > > > > > > > +#endif
> > > > > > > > >
> > > > > > > > >  U_BOOT_CMD(
> > > > > > > > >   dek_blob, 4, 1, do_dek_blob,
> > > > > > > >
> > > > > > > > This really doesn't read nicely.  I would rather (globally and 
> > > > > > > > fix
> > > > > > > > existing users) __maybe_unused this instead.  I think there's 
> > > > > > > > just one
> > > > > > > > example today that isn't "foo_help_text".
> > > > > > >
> > > > > > > Hmm, what do you think about adding a __longhelp symbol to cause 
> > > > > > > the
> > > > > > > linker to discard it when not needed?
> > > > > >
> > > > > > Well, I don't think we need linker list magic when __maybe_unused 
> > > > > > will
> > > > > > just have them be discarded normally.
> > > > >
> > > > > Yes, perhaps things are in a better state than they used to be, but
> > > > > there is a linker discard for commands at present.
> > > >
> > > > Yes, but __maybe_unused means we don't get a warning about it, and it's
> > > > literally discarded as part of --gc-sections as it done everywhere with
> > > > maybe 3 exceptions?
> > >
> > > Actually with this series we get a lot closer to that. The problem
> > > with the status quo is that disabling CMDLINE doesn't disable most
> > > commands, relying instead on discarding them at link time.
> >
> > I don't follow you here.  We're talking only about the long help.
> 
> I was actually talking about how the command code is removed. This
> series allows that to happen via Kconfig rather than needing a
> linker-script discard rule, something I only just fully appreciated.

OK.  But this series as-is has a lot of problems and I don't see it as
more than a proof of concept.

> > There's already an option to enable/disable it.  When disabled all of
> > the long help text should be discarded, because we reference it via
> > U_BOOT_CMD macro which in turn cares about it, or not.  What's missing
> > is a U_BOOT_LONGHELP macro so that instead of:
> > #ifdef CONFIG_SYS_LONGHELP
> > static char cat_help_text[] =
> > "  \n"
> > "  - Print file from 'dev' on 'interface' to standard output\n";
> > #endif
> >
> > We do:
> > U_BOOT_LONGHELP(cat,
> > "  \n"
> > "  - Print file from 'dev' on 'interface' to standard output\n"
> > );
> >
> > Which then does:
> > static __maybe_unused char cat_help_text[] =
> > ...
> > ;
> >
> > And we discard the text automatically due to --gc-sections.  We possibly
> > haven't already been doing this since back when we first started using
> > --gc-sections there was a bug in binutils that caused text like the
> > above to still get combined in to other objects and not discarded.
> > That's been fixed for ages.
> >
> > And the above macro would also let us clean up U_BOOT_CMD macro slightly
> > to just omit the longhelp option and instead always do _CMD_HELP(_name)
> > inside the macros.  It'll also make 

[PATCH] bootstd: sata: bootdev scanning for ahci sata with no drive attached

2023-10-06 Thread Tony Dinh
It's normal to have no SATA drive attached to the controller, so return a
successful status when there is no block device found after probing.

Note: this patch depends on the previous patch
https://patchwork.ozlabs.org/project/uboot/patch/20230917230649.30357-1-mibo...@gmail.com/

Signed-off-by: Tony Dinh 
---

 drivers/ata/sata.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/ata/sata.c b/drivers/ata/sata.c
index f126b84e05..59c40a829f 100644
--- a/drivers/ata/sata.c
+++ b/drivers/ata/sata.c
@@ -79,6 +79,11 @@ int sata_rescan(bool verbose)
 
ret = uclass_probe_all(UCLASS_AHCI);
 
+   if (verbose && ret == -ENODEV) {
+   printf("No SATA block device found\n");
+   return 0;
+   }
+
return ret;
 }
 
-- 
2.39.2



Re: [PATCH v7 2/2] schemas: Add some common reserved-memory usages

2023-10-06 Thread Simon Glass
Hi Ard,

On Fri, 6 Oct 2023 at 17:00, Ard Biesheuvel  wrote:
>
> On Fri, 6 Oct 2023 at 20:17, Simon Glass  wrote:
> >
> > Hi Ard,
> >
> > On Fri, 6 Oct 2023 at 11:33, Ard Biesheuvel  wrote:
> > >
> > > On Mon, 2 Oct 2023 at 19:54, Simon Glass  wrote:
> > > >
> > > > Hi Rob,
> > > >
> > > > On Tue, 26 Sept 2023 at 13:42, Simon Glass  wrote:
> > > > >
> > > > > It is common to split firmware into 'Platform Init', which does the
> > > > > initial hardware setup and a "Payload" which selects the OS to be 
> > > > > booted.
> > > > > Thus an handover interface is required between these two pieces.
> > > > >
> > > > > Where UEFI boot-time services are not available, but UEFI firmware is
> > > > > present on either side of this interface, information about memory 
> > > > > usage
> > > > > and attributes must be presented to the "Payload" in some form.
> > > > >
> > > > > This aims to provide an small schema addition for the memory mapping
> > > > > needed to keep these two pieces working together well.
> > > > >
> > > > > Signed-off-by: Simon Glass 
> > > > > ---
> > > > >
> > > > > Changes in v7:
> > > > > - Rename acpi-reclaim to acpi
> > > > > - Drop individual mention of when memory can be reclaimed
> > > > > - Rewrite the item descriptions
> > > > > - Add back the UEFI text (with trepidation)
> > > >
> > > > I am again checking on this series. Can it be applied, please?
> > > >
> > >
> > > Apologies for the delay in response. I have been away.
> >
> > OK, I hope you had a nice trip.
> >
>
> Thanks, it was wonderful!
>
> > >
> > > >
> > > > >
> > > > > Changes in v6:
> > > > > - Drop mention of UEFI
> > > > > - Use compatible strings instead of node names
> > > > >
> > > > > Changes in v5:
> > > > > - Drop the memory-map node (should have done that in v4)
> > > > > - Tidy up schema a bit
> > > > >
> > > > > Changes in v4:
> > > > > - Make use of the reserved-memory node instead of creating a new one
> > > > >
> > > > > Changes in v3:
> > > > > - Reword commit message again
> > > > > - cc a lot more people, from the FFI patch
> > > > > - Split out the attributes into the /memory nodes
> > > > >
> > > > > Changes in v2:
> > > > > - Reword commit message
> > > > >
> > > > >  .../reserved-memory/common-reserved.yaml  | 71 
> > > > > +++
> > > > >  1 file changed, 71 insertions(+)
> > > > >  create mode 100644 
> > > > > dtschema/schemas/reserved-memory/common-reserved.yaml
> > > > >
> > > > > diff --git a/dtschema/schemas/reserved-memory/common-reserved.yaml 
> > > > > b/dtschema/schemas/reserved-memory/common-reserved.yaml
> > > > > new file mode 100644
> > > > > index 000..f7fbdfd
> > > > > --- /dev/null
> > > > > +++ b/dtschema/schemas/reserved-memory/common-reserved.yaml
> > > > > @@ -0,0 +1,71 @@
> > > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: 
> > > > > http://devicetree.org/schemas/reserved-memory/common-reserved.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: Common memory reservations
> > > > > +
> > > > > +description: |
> > > > > +  Specifies that the reserved memory region can be used for the 
> > > > > purpose
> > > > > +  indicated by its compatible string.
> > > > > +
> > > > > +  Clients may reuse this reserved memory if they understand what it 
> > > > > is for,
> > > > > +  subject to the notes below.
> > > > > +
> > > > > +maintainers:
> > > > > +  - Simon Glass 
> > > > > +
> > > > > +allOf:
> > > > > +  - $ref: reserved-memory.yaml
> > > > > +
> > > > > +properties:
> > > > > +  compatible:
> > > > > +description: |
> > > > > +  This describes some common memory reservations, with the 
> > > > > compatible
> > > > > +  string indicating what it is used for:
> > > > > +
> > > > > + acpi: Advanced Configuration and Power Interface (ACPI) 
> > > > > tables
> > > > > + acpi-nvs: ACPI Non-Volatile-Sleeping Memory (NVS). This is 
> > > > > reserved by
> > > > > +   the firmware for its use and is required to be saved and 
> > > > > restored
> > > > > +   across an NVS sleep
> > > > > + boot-code: Contains code used for booting which is not 
> > > > > needed by the OS
> > > > > + boot-code: Contains data used for booting which is not 
> > > > > needed by the OS
> > > > > + runtime-code: Contains code used for interacting with the 
> > > > > system when
> > > > > +   running the OS
> > > > > + runtime-data: Contains data used for interacting with the 
> > > > > system when
> > > > > +   running the OS
> > > > > +
> > > > > +enum:
> > > > > +  - acpi
> > > > > +  - acpi-nvs
> > > > > +  - boot-code
> > > > > +  - boot-data
> > > > > +  - runtime-code
> > > > > +  - runtime-data
> > > > > +
> > >
> > > As I mentioned a few times already, I don't think these compatibles
> > > should be introduced here.
> > >
> > > A reserved region has a 

Re: [PATCH v7 2/2] schemas: Add some common reserved-memory usages

2023-10-06 Thread Ard Biesheuvel
On Fri, 6 Oct 2023 at 20:17, Simon Glass  wrote:
>
> Hi Ard,
>
> On Fri, 6 Oct 2023 at 11:33, Ard Biesheuvel  wrote:
> >
> > On Mon, 2 Oct 2023 at 19:54, Simon Glass  wrote:
> > >
> > > Hi Rob,
> > >
> > > On Tue, 26 Sept 2023 at 13:42, Simon Glass  wrote:
> > > >
> > > > It is common to split firmware into 'Platform Init', which does the
> > > > initial hardware setup and a "Payload" which selects the OS to be 
> > > > booted.
> > > > Thus an handover interface is required between these two pieces.
> > > >
> > > > Where UEFI boot-time services are not available, but UEFI firmware is
> > > > present on either side of this interface, information about memory usage
> > > > and attributes must be presented to the "Payload" in some form.
> > > >
> > > > This aims to provide an small schema addition for the memory mapping
> > > > needed to keep these two pieces working together well.
> > > >
> > > > Signed-off-by: Simon Glass 
> > > > ---
> > > >
> > > > Changes in v7:
> > > > - Rename acpi-reclaim to acpi
> > > > - Drop individual mention of when memory can be reclaimed
> > > > - Rewrite the item descriptions
> > > > - Add back the UEFI text (with trepidation)
> > >
> > > I am again checking on this series. Can it be applied, please?
> > >
> >
> > Apologies for the delay in response. I have been away.
>
> OK, I hope you had a nice trip.
>

Thanks, it was wonderful!

> >
> > >
> > > >
> > > > Changes in v6:
> > > > - Drop mention of UEFI
> > > > - Use compatible strings instead of node names
> > > >
> > > > Changes in v5:
> > > > - Drop the memory-map node (should have done that in v4)
> > > > - Tidy up schema a bit
> > > >
> > > > Changes in v4:
> > > > - Make use of the reserved-memory node instead of creating a new one
> > > >
> > > > Changes in v3:
> > > > - Reword commit message again
> > > > - cc a lot more people, from the FFI patch
> > > > - Split out the attributes into the /memory nodes
> > > >
> > > > Changes in v2:
> > > > - Reword commit message
> > > >
> > > >  .../reserved-memory/common-reserved.yaml  | 71 +++
> > > >  1 file changed, 71 insertions(+)
> > > >  create mode 100644 
> > > > dtschema/schemas/reserved-memory/common-reserved.yaml
> > > >
> > > > diff --git a/dtschema/schemas/reserved-memory/common-reserved.yaml 
> > > > b/dtschema/schemas/reserved-memory/common-reserved.yaml
> > > > new file mode 100644
> > > > index 000..f7fbdfd
> > > > --- /dev/null
> > > > +++ b/dtschema/schemas/reserved-memory/common-reserved.yaml
> > > > @@ -0,0 +1,71 @@
> > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: 
> > > > http://devicetree.org/schemas/reserved-memory/common-reserved.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Common memory reservations
> > > > +
> > > > +description: |
> > > > +  Specifies that the reserved memory region can be used for the purpose
> > > > +  indicated by its compatible string.
> > > > +
> > > > +  Clients may reuse this reserved memory if they understand what it is 
> > > > for,
> > > > +  subject to the notes below.
> > > > +
> > > > +maintainers:
> > > > +  - Simon Glass 
> > > > +
> > > > +allOf:
> > > > +  - $ref: reserved-memory.yaml
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +description: |
> > > > +  This describes some common memory reservations, with the 
> > > > compatible
> > > > +  string indicating what it is used for:
> > > > +
> > > > + acpi: Advanced Configuration and Power Interface (ACPI) tables
> > > > + acpi-nvs: ACPI Non-Volatile-Sleeping Memory (NVS). This is 
> > > > reserved by
> > > > +   the firmware for its use and is required to be saved and 
> > > > restored
> > > > +   across an NVS sleep
> > > > + boot-code: Contains code used for booting which is not needed 
> > > > by the OS
> > > > + boot-code: Contains data used for booting which is not needed 
> > > > by the OS
> > > > + runtime-code: Contains code used for interacting with the 
> > > > system when
> > > > +   running the OS
> > > > + runtime-data: Contains data used for interacting with the 
> > > > system when
> > > > +   running the OS
> > > > +
> > > > +enum:
> > > > +  - acpi
> > > > +  - acpi-nvs
> > > > +  - boot-code
> > > > +  - boot-data
> > > > +  - runtime-code
> > > > +  - runtime-data
> > > > +
> >
> > As I mentioned a few times already, I don't think these compatibles
> > should be introduced here.
> >
> > A reserved region has a specific purpose, and the compatible should be
> > more descriptive than the enum above. If the consumer does not
> > understand this purpose, it should simply treat the memory as reserved
> > and not touch it. Alternatively, these regions can be referenced from
> > other DT nodes using phandles if needed.
>
> We still need some description of what these regions are used for, 

Re: [PATCH 05/25] treewide: Correct use of long help

2023-10-06 Thread Simon Glass
Hi Tom,

On Fri, 6 Oct 2023 at 10:55, Tom Rini  wrote:
>
> On Fri, Oct 06, 2023 at 07:03:17AM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Thu, 5 Oct 2023 at 20:16, Tom Rini  wrote:
> > >
> > > On Thu, Oct 05, 2023 at 07:41:49PM -0600, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Thu, 5 Oct 2023 at 08:53, Tom Rini  wrote:
> > > > >
> > > > > On Wed, Oct 04, 2023 at 07:23:47PM -0600, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Sun, 24 Sept 2023 at 17:27, Tom Rini  wrote:
> > > > > > >
> > > > > > > On Sun, Sep 24, 2023 at 02:39:23PM -0600, Simon Glass wrote:
> > > > > > > > Some commands assume that CONFIG_SYS_LONGHELP is always defined.
> > > > > > > > Declaration of long help should be bracketed by an #ifdef to 
> > > > > > > > avoid an
> > > > > > > > 'unused variable' warning.
> > > > > > > >
> > > > > > > > Fix this treewide.
> > > > > > > >
> > > > > > > > Signed-off-by: Simon Glass 
> > > > > > > [snip]
> > > > > > > > diff --git a/arch/arm/mach-imx/cmd_dek.c 
> > > > > > > > b/arch/arm/mach-imx/cmd_dek.c
> > > > > > > > index 6fa5b41fcd38..25ea7d3b37da 100644
> > > > > > > > --- a/arch/arm/mach-imx/cmd_dek.c
> > > > > > > > +++ b/arch/arm/mach-imx/cmd_dek.c
> > > > > > > > @@ -393,11 +393,12 @@ static int do_dek_blob(struct cmd_tbl 
> > > > > > > > *cmdtp, int flag, int argc,
> > > > > > > >   return blob_encap_dek(src_addr, dst_addr, len);
> > > > > > > >  }
> > > > > > > >
> > > > > > > > -/***/
> > > > > > > > +#if IS_ENABLED(CONFIG_SYS_LONGHELP)
> > > > > > > >  static char dek_blob_help_text[] =
> > > > > > > >   "src dst len- Encapsulate and create blob of 
> > > > > > > > data\n"
> > > > > > > >   " $len bits long at address $src 
> > > > > > > > and\n"
> > > > > > > >   " store the result at address 
> > > > > > > > $dst.\n";
> > > > > > > > +#endif
> > > > > > > >
> > > > > > > >  U_BOOT_CMD(
> > > > > > > >   dek_blob, 4, 1, do_dek_blob,
> > > > > > >
> > > > > > > This really doesn't read nicely.  I would rather (globally and fix
> > > > > > > existing users) __maybe_unused this instead.  I think there's 
> > > > > > > just one
> > > > > > > example today that isn't "foo_help_text".
> > > > > >
> > > > > > Hmm, what do you think about adding a __longhelp symbol to cause the
> > > > > > linker to discard it when not needed?
> > > > >
> > > > > Well, I don't think we need linker list magic when __maybe_unused will
> > > > > just have them be discarded normally.
> > > >
> > > > Yes, perhaps things are in a better state than they used to be, but
> > > > there is a linker discard for commands at present.
> > >
> > > Yes, but __maybe_unused means we don't get a warning about it, and it's
> > > literally discarded as part of --gc-sections as it done everywhere with
> > > maybe 3 exceptions?
> >
> > Actually with this series we get a lot closer to that. The problem
> > with the status quo is that disabling CMDLINE doesn't disable most
> > commands, relying instead on discarding them at link time.
>
> I don't follow you here.  We're talking only about the long help.

I was actually talking about how the command code is removed. This
series allows that to happen via Kconfig rather than needing a
linker-script discard rule, something I only just fully appreciated.

> There's already an option to enable/disable it.  When disabled all of
> the long help text should be discarded, because we reference it via
> U_BOOT_CMD macro which in turn cares about it, or not.  What's missing
> is a U_BOOT_LONGHELP macro so that instead of:
> #ifdef CONFIG_SYS_LONGHELP
> static char cat_help_text[] =
> "  \n"
> "  - Print file from 'dev' on 'interface' to standard output\n";
> #endif
>
> We do:
> U_BOOT_LONGHELP(cat,
> "  \n"
> "  - Print file from 'dev' on 'interface' to standard output\n"
> );
>
> Which then does:
> static __maybe_unused char cat_help_text[] =
> ...
> ;
>
> And we discard the text automatically due to --gc-sections.  We possibly
> haven't already been doing this since back when we first started using
> --gc-sections there was a bug in binutils that caused text like the
> above to still get combined in to other objects and not discarded.
> That's been fixed for ages.
>
> And the above macro would also let us clean up U_BOOT_CMD macro slightly
> to just omit the longhelp option and instead always do _CMD_HELP(_name)
> inside the macros.  It'll also make it harder to add new commands
> without a long help by accident.

Gosh this is complicated.

At present the U_BOOT_CMD() macro just drops the strings...the problem
with the unused var only happens in a small number of cases where an
explicit var is used. I don't see why creating a var (when none is
there today) helps anything? It doesn't detect missing longhelp, since
this is already an error (missing argument). Sure,  "" can be
provided, but your macro 

Re: [RESEND PATCH v3 0/3] rpi: Convert to standard boot

2023-10-06 Thread Simon Glass
Hi Peter,

On Fri, 6 Oct 2023 at 03:22, Peter Robinson  wrote:
>
> Hi Simon,
>
> So with more testing of 2023.10 in Fedora we found a regression where
> the display dies when the vc4 module loads in the kernel. With further
> debug it was found that it was due to the new U-Boot and with
> bisecting it myself I have found this series is the cause of the
> regression.
>
> The testing I have done is with recent RPi firmware, U-Boot 2023.10
> and kernel 6.5.x, but also seen with 6.4.x, with a minimal text only
> image. I can also reproduce it by using the F-38 GA image, no updates
> and purely just changing the U-Boot component. I've done my testing on
> the RPi4.
>
> Let me know if you need more information, I believe you have a RPi you
> can test with.

Thanks for the report. Which rpi should I test with? I take it that
vc4 is a kernel module? Do you have the error message?

Regards,
Simon


>
> Regards,
> Peter
>
> On Thu, Jul 27, 2023 at 10:54 PM Simon Glass  wrote:
> >
> > This series moves Raspberry Pi boards over to use standard boot.
> >
> > It also moves rpi over to use a text-based environment. Unfortunately it
> > is not possible to empty the header file due to several CFG options.
> >
> > Fix the repeated "and and" while we are here.
> >
> > Note that this reduces rodata size by about 4.5KB. We could get another
> >
> > for a total image-size saving of about 15KB. This is mostly because
> > HUSH_PARSER is not enabled anymore and the environment shrinks down by
> > about 3.5K. Hush is not actually needed anymore, since standard boot does
> > not use it. Also CMD_SYSBOOT is dropped since standard boot calls the
> > pxe_utils code directly in that case.
> >
> > Changes in v3:
> > - Rebase to -master
> >
> > Changes in v2:
> > - Rebase to -next
> > - Add new patch to disable DISTRO_DEFAULTS
> >
> > Simon Glass (3):
> >   arm: rpi: Switch to standard boot
> >   rpi: Disable DISTRO_DEFAULTS
> >   arm: rpi: Switch to a text environment
> >
> >  board/raspberrypi/rpi/rpi.env  |  77 +++
> >  configs/rpi_0_w_defconfig  |   2 +-
> >  configs/rpi_2_defconfig|   2 +-
> >  configs/rpi_3_32b_defconfig|   2 +-
> >  configs/rpi_3_b_plus_defconfig |   2 +-
> >  configs/rpi_3_defconfig|   2 +-
> >  configs/rpi_4_32b_defconfig|   2 +-
> >  configs/rpi_4_defconfig|   2 +-
> >  configs/rpi_arm64_defconfig|   2 +-
> >  configs/rpi_defconfig  |   2 +-
> >  include/configs/rpi.h  | 134 -
> >  11 files changed, 86 insertions(+), 143 deletions(-)
> >  create mode 100644 board/raspberrypi/rpi/rpi.env
> >
> > --
> > 2.41.0.487.g6d72f3e995-goog
> >


[PATCH] cli: Consume invalid escape sequences early

2023-10-06 Thread Yurii Monakov
This commit fixes some issues with extra 'Esc' keys entered by user:

1. Sequence  right after autoboot stop gives:
=>
nknown command 'ry 'help'
=>
2. Sequence  gives:
=> ri
Unknown command 'ri' - try 'help'
=>
3. Extra 'Esc' key presses break backspace functionality.

Signed-off-by: Yurii Monakov 
---
 common/cli_getch.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/common/cli_getch.c b/common/cli_getch.c
index 61d4cb261b..0ee7908777 100644
--- a/common/cli_getch.c
+++ b/common/cli_getch.c
@@ -46,6 +46,8 @@ static int cli_ch_esc(struct cli_ch_state *cch, int ichar,
case 1:
if (ichar == '[' || ichar == 'O')
act = ESC_SAVE;
+   else
+   act = ESC_CONVERTED;
break;
case 2:
switch (ichar) {
-- 
2.34.1



Re: [PATCH 5/7] net: dwc_eth_qos_rockchip: Add support for RK3588

2023-10-06 Thread Tom Fitzhenry
Tested to work (DHCP, ping) on RK3588 QuartzPro64 (DTS coming soon!),
thanks!

Tested-by: Tom Fitzhenry 


[PATCH] net: phy: TI DP83869 fix invalid clock delay configuration

2023-10-06 Thread Frank de Brabander
Setting the clock delay from the device tree settings
rx-internal-delay-ps and tx-internal-delay-ps was broken:

 - The expected value in the device tree is suppose to be a
   delay in picoseconds, but the driver only allowed an array index.
 - Driver converted this array index to the actual delay in
   picoseconds and tried to apply this in the device register. This
   however is not a valid register value. The actual logic here was
   reversed, it converted an register representation of the delay to
   the device tree delay in picoseconds.

Only when the internal delays were NOT configured in the device tree
and they default value of 7 (=2000ps) was used, a valid value was
loaded in the register.

Signed-off-by: Frank de Brabander 
---
 drivers/net/phy/dp83869.c | 53 +++
 1 file changed, 32 insertions(+), 21 deletions(-)

diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c
index 8d32d73b07..f9d4782580 100644
--- a/drivers/net/phy/dp83869.c
+++ b/drivers/net/phy/dp83869.c
@@ -81,7 +81,10 @@
 
 /* RGMIIDCTL bits */
 #define DP83869_RGMII_TX_CLK_DELAY_SHIFT   4
-#define DP83869_CLK_DELAY_DEF  7
+#define DP83869_CLK_DELAY_STEP 250
+#define DP83869_CLK_DELAY_MIN  250
+#define DP83869_CLK_DELAY_MAX  4000
+#define DP83869_CLK_DELAY_DEFAULT  2000
 
 /* CFG2 bits */
 #define MII_DP83869_CFG2_SPEEDOPT_10EN 0x0040
@@ -157,10 +160,6 @@ static int dp83869_config_port_mirroring(struct phy_device 
*phydev)
return 0;
 }
 
-static const int dp83869_internal_delay[] = {250, 500, 750, 1000, 1250, 1500,
-1750, 2000, 2250, 2500, 2750, 3000,
-3250, 3500, 3750, 4000};
-
 static int dp83869_set_strapped_mode(struct phy_device *phydev)
 {
struct dp83869_private *dp83869 = phydev->priv;
@@ -183,7 +182,6 @@ static int dp83869_set_strapped_mode(struct phy_device 
*phydev)
 static int dp83869_of_init(struct phy_device *phydev)
 {
struct dp83869_private * const dp83869 = phydev->priv;
-   const int delay_entries = ARRAY_SIZE(dp83869_internal_delay);
int ret;
ofnode node;
 
@@ -238,32 +236,45 @@ static int dp83869_of_init(struct phy_device *phydev)
dp83869->tx_fifo_depth = ofnode_read_s32_default(node, "tx-fifo-depth",
 
DP83869_PHYCR_FIFO_DEPTH_4_B_NIB);
 
+   /* Internal clock delay values can be configured in steps of
+* 250ps (0.25ns). The register field for clock delay is 4-bits wide,
+* the values range from 0b for 0.25ns to 0b for 4ns.
+*/
+
/* RX delay *must* be specified if internal delay of RX is used. */
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
-   dp83869->rx_int_delay = ofnode_read_u32_default(node, 
"rx-internal-delay-ps",
-   
DP83869_CLK_DELAY_DEF);
-   if (dp83869->rx_int_delay > delay_entries) {
-   dp83869->rx_int_delay = DP83869_CLK_DELAY_DEF;
-   pr_debug("rx-internal-delay-ps not set/invalid, default 
to %ups\n",
-dp83869_internal_delay[dp83869->rx_int_delay]);
+   dp83869->rx_int_delay = ofnode_read_u32_default(node,
+   "rx-internal-delay-ps", DP83869_CLK_DELAY_DEFAULT);
+   if (dp83869->rx_int_delay > DP83869_CLK_DELAY_MAX ||
+   dp83869->rx_int_delay < DP83869_CLK_DELAY_MIN ||
+   dp83869->rx_int_delay % DP83869_CLK_DELAY_STEP) {
+   dp83869->rx_int_delay = DP83869_CLK_DELAY_DEFAULT;
+   pr_warn("rx-internal-delay-ps not set/invalid, default"
+   " to %ups\n", DP83869_CLK_DELAY_DEFAULT);
}
 
-   dp83869->rx_int_delay = 
dp83869_internal_delay[dp83869->rx_int_delay];
+   dp83869->rx_int_delay =
+   (dp83869->rx_int_delay - DP83869_CLK_DELAY_STEP)
+   / DP83869_CLK_DELAY_STEP;
}
 
-   /* TX delay *must* be specified if internal delay of RX is used. */
+   /* TX delay *must* be specified if internal delay of TX is used. */
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
-   dp83869->tx_int_delay = ofnode_read_u32_default(node, 
"tx-internal-delay-ps",
-   
DP83869_CLK_DELAY_DEF);
-   if (dp83869->tx_int_delay > delay_entries) {
-   dp83869->tx_int_delay = DP83869_CLK_DELAY_DEF;
-   pr_debug("tx-internal-delay-ps not set/invalid, default 
to %ups\n",
-

Re: [PATCH] sphinx: Bump urllib3 version

2023-10-06 Thread Tom Rini
On Fri, Oct 06, 2023 at 09:50:20PM +0200, Heinrich Schuchardt wrote:
> On 10/6/23 03:41, Simon Glass wrote:
> > On Thu, 5 Oct 2023 at 10:27, Tom Rini  wrote:
> > > 
> > > While not a direct issue for us, urllib3 before 1.26.17 is vulnerable to
> > > CVE-2023-43804 to bump our version up.
> 
> The same bug is also fixed in 2.0.6. Why should we stick with the old
> series? I could not see any issues building the documentation locally
> and on Github with 2.0.6.

There's probably a number of packages we could bump for similar reasons,
if you'd like to unfreeze, build, check the output and refreeze.  I'm
just posting something to get Dependabot to be silenced since I get this
whenever I push a branch.

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 18/25] net: Depend on CONFIG_CMDLINE

2023-10-06 Thread Ramon Fried
On Sun, Sep 24, 2023 at 11:40 PM Simon Glass  wrote:
>
> At present it isn't possible to use networking without the command line
> enabled. Add this as a condition.
>
> Signed-off-by: Simon Glass 
> ---
>
>  cmd/Kconfig | 1 +
>  net/Kconfig | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 5f6834b335dc..c3428d19f31d 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1687,6 +1687,7 @@ if NET
>
>  menuconfig CMD_NET
> bool "Network commands"
> +   depends on CMDLINE
> default y
> imply NETDEVICES
>
> diff --git a/net/Kconfig b/net/Kconfig
> index 4215889127c9..25d494e1db46 100644
> --- a/net/Kconfig
> +++ b/net/Kconfig
> @@ -4,6 +4,7 @@
>
>  menuconfig NET
> bool "Networking support"
> +   depends on CMDLINE
> default y
>
>  if NET
> --
> 2.42.0.515.g380fc7ccd1-goog
>
Reviewed-by: Ramon Fried 


Re: [PATCH v5 2/7] net: wget: add wget with dns utility function

2023-10-06 Thread Ramon Fried
On Wed, Sep 27, 2023 at 12:37 PM Masahisa Kojima
 wrote:
>
> Current wget takes the target uri in this format:
>  ":"  e.g.) 192.168.1.1:/bar
> The http server ip address must be resolved before
> calling wget.
>
> This commit adds the utility function runs wget with dhs.
> User can call wget with the uri like "http://foo/bar;.
>
> Signed-off-by: Masahisa Kojima 
> Reviewed-by: Ilias Apalodimas 
> ---
>  include/net.h |  9 +
>  net/wget.c| 54 +++
>  2 files changed, 63 insertions(+)
>
> diff --git a/include/net.h b/include/net.h
> index e254df7d7f..57889d8b7a 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -926,4 +926,13 @@ void eth_set_enable_bootdevs(bool enable);
>  static inline void eth_set_enable_bootdevs(bool enable) {}
>  #endif
>
> +/**
> + * wget_with_dns() - runs dns host IP address resulution before wget
> + *
> + * @dst_addr:  destination address to download the file
> + * @uri:   uri string of target file of wget
> + * Return: downloaded file size, negative if failed
> + */
> +int wget_with_dns(ulong dst_addr, char *uri);
> +
>  #endif /* __NET_H__ */
> diff --git a/net/wget.c b/net/wget.c
> index a48a8cb624..4801e28eb9 100644
> --- a/net/wget.c
> +++ b/net/wget.c
> @@ -15,6 +15,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> @@ -504,3 +505,56 @@ void wget_start(void)
>
> wget_send(TCP_SYN, 0, 0, 0);
>  }
> +
> +#if (IS_ENABLED(CONFIG_CMD_DNS))
> +int wget_with_dns(ulong dst_addr, char *uri)
> +{
> +   int ret;
> +   char *s, *host_name, *file_name, *str_copy;
> +
> +   /*
> +* Download file using wget.
> +*
> +* U-Boot wget takes the target uri in this format.
> +*  ":"  e.g.) 192.168.1.1:/sample/test.iso
> +* Need to resolve the http server ip address before starting wget.
> +*/
> +   str_copy = strdup(uri);
> +   if (!str_copy)
> +   return -ENOMEM;
> +
> +   s = str_copy + strlen("http://;);
> +   host_name = strsep(, "/");
> +   if (!s) {
> +   log_err("Error: invalied uri, no file path\n");
> +   ret = -EINVAL;
> +   goto out;
> +   }
> +   file_name = s;
> +
> +   /* TODO: If the given uri has ip address for the http server, skip 
> dns */
> +   net_dns_resolve = host_name;
> +   net_dns_env_var = "httpserverip";
> +   if (net_loop(DNS) < 0) {
> +   log_err("Error: dns lookup of %s failed, check setup\n", 
> net_dns_resolve);
> +   ret = -EINVAL;
> +   goto out;
> +   }
> +   s = env_get("httpserverip");
> +   if (!s) {
> +   ret = -EINVAL;
> +   goto out;
> +   }
> +
> +   strlcpy(net_boot_file_name, s, sizeof(net_boot_file_name));
> +   strlcat(net_boot_file_name, ":/", sizeof(net_boot_file_name)); /* 
> append '/' which is removed by strsep() */
> +   strlcat(net_boot_file_name, file_name, sizeof(net_boot_file_name));
> +   image_load_addr = dst_addr;
> +   ret = net_loop(WGET);
> +
> +out:
> +   free(str_copy);
> +
> +   return ret;
> +}
> +#endif
> --
> 2.34.1
>
Reviewed-by: Ramon Fried 


Re: [PATCH v5 1/7] net: wget: prevent overwriting reserved memory

2023-10-06 Thread Ramon Fried
On Wed, Sep 27, 2023 at 12:37 PM Masahisa Kojima
 wrote:
>
> This introduces the valid range check to store the received
> blocks using lmb. The same logic is implemented in tftp.
>
> Signed-off-by: Masahisa Kojima 
> Acked-by: Ilias Apalodimas 
> Reviewed-by: Simon Glass 
> ---
>  net/wget.c | 80 +-
>  1 file changed, 73 insertions(+), 7 deletions(-)
>
> diff --git a/net/wget.c b/net/wget.c
> index 2dbfeb1a1d..a48a8cb624 100644
> --- a/net/wget.c
> +++ b/net/wget.c
> @@ -4,16 +4,20 @@
>   * Copyright Duncan Hare  2017
>   */
>
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
>
> +DECLARE_GLOBAL_DATA_PTR;
> +
>  static const char bootfile1[] = "GET ";
>  static const char bootfile3[] = " HTTP/1.0\r\n\r\n";
>  static const char http_eom[] = "\r\n\r\n";
> @@ -55,6 +59,29 @@ static unsigned int retry_tcp_ack_num;   /* TCP retry 
> acknowledge number*/
>  static unsigned int retry_tcp_seq_num; /* TCP retry sequence number */
>  static int retry_len;  /* TCP retry length */
>
> +static ulong wget_load_size;
> +
> +/**
> + * wget_init_max_size() - initialize maximum load size
> + *
> + * Return: 0 if success, -1 if fails
> + */
> +static int wget_init_load_size(void)
> +{
> +   struct lmb lmb;
> +   phys_size_t max_size;
> +
> +   lmb_init_and_reserve(, gd->bd, (void *)gd->fdt_blob);
> +
> +   max_size = lmb_get_free_size(, image_load_addr);
> +   if (!max_size)
> +   return -1;
> +
> +   wget_load_size = max_size;
> +
> +   return 0;
> +}
> +
>  /**
>   * store_block() - store block in memory
>   * @src: source of data
> @@ -63,10 +90,25 @@ static int retry_len;   /* TCP retry 
> length */
>   */
>  static inline int store_block(uchar *src, unsigned int offset, unsigned int 
> len)
>  {
> +   ulong store_addr = image_load_addr + offset;
> ulong newsize = offset + len;
> uchar *ptr;
>
> -   ptr = map_sysmem(image_load_addr + offset, len);
> +   if (IS_ENABLED(CONFIG_LMB)) {
> +   ulong end_addr = image_load_addr + wget_load_size;
> +
> +   if (!end_addr)
> +   end_addr = ULONG_MAX;
> +
> +   if (store_addr < image_load_addr ||
> +   store_addr + len > end_addr) {
> +   printf("\nwget error: ");
> +   printf("trying to overwrite reserved memory...\n");
> +   return -1;
> +   }
> +   }
> +
> +   ptr = map_sysmem(store_addr, len);
> memcpy(ptr, src, len);
> unmap_sysmem(ptr);
>
> @@ -240,25 +282,39 @@ static void wget_connected(uchar *pkt, unsigned int 
> tcp_seq_num,
>
> net_boot_file_size = 0;
>
> -   if (len > hlen)
> -   store_block(pkt + hlen, 0, len - hlen);
> +   if (len > hlen) {
> +   if (store_block(pkt + hlen, 0, len - hlen) != 
> 0) {
> +   wget_loop_state = NETLOOP_FAIL;
> +   wget_fail("wget: store error\n", 
> tcp_seq_num, tcp_ack_num, action);
> +   net_set_state(NETLOOP_FAIL);
> +   return;
> +   }
> +   }
>
> debug_cond(DEBUG_WGET,
>"wget: Connected Pkt %p hlen %x\n",
>pkt, hlen);
>
> for (i = 0; i < pkt_q_idx; i++) {
> +   int err;
> +
> ptr1 = map_sysmem(
> (phys_addr_t)(pkt_q[i].pkt),
> pkt_q[i].len);
> -   store_block(ptr1,
> -   pkt_q[i].tcp_seq_num -
> -   initial_data_seq_num,
> -   pkt_q[i].len);
> +   err = store_block(ptr1,
> + pkt_q[i].tcp_seq_num -
> + initial_data_seq_num,
> + pkt_q[i].len);
> unmap_sysmem(ptr1);
> debug_cond(DEBUG_WGET,
>"wget: Connctd pkt Q %p len %x\n",
>pkt_q[i].pkt, pkt_q[i].len);
> +   if (err) {
> +   wget_loop_state = NETLOOP_FAIL;
> +   wget_fail("wget: store error\n", 
> tcp_seq_num, tcp_ack_num, action);
> +  

Re: [PATCH v2 5/7] net: dwc_eth_qos_rockchip: Add support for RK3588

2023-10-06 Thread Ramon Fried
On Sun, Oct 1, 2023 at 10:17 PM Jonas Karlman  wrote:
>
> Add rk_gmac_ops and other special handling that is needed for GMAC to
> work on RK3588.
>
> rk_gmac_ops was ported from linux commits:
> 2f2b60a0ec28 ("net: ethernet: stmmac: dwmac-rk: Add gmac support for rk3588")
> 88619e77b33d ("net: stmmac: rk3588: Allow multiple gmac controller")
>
> Signed-off-by: Jonas Karlman 
> Reviewed-by: Kever Yang 
> ---
> Cc: David Wu 
> Cc: Sebastian Reichel 
> Cc: Benjamin Gaignard 
> ---
> v2:
> - Collect r-b tag
>
>  drivers/net/dwc_eth_qos.c  |   4 +
>  drivers/net/dwc_eth_qos_rockchip.c | 182 -
>  2 files changed, 182 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c
> index 9fb98a2c3c74..dc04416865dd 100644
> --- a/drivers/net/dwc_eth_qos.c
> +++ b/drivers/net/dwc_eth_qos.c
> @@ -1712,6 +1712,10 @@ static const struct udevice_id eqos_ids[] = {
> .compatible = "rockchip,rk3568-gmac",
> .data = (ulong)_rockchip_config
> },
> +   {
> +   .compatible = "rockchip,rk3588-gmac",
> +   .data = (ulong)_rockchip_config
> +   },
>  #endif
>  #if IS_ENABLED(CONFIG_DWC_ETH_QOS_QCOM)
> {
> diff --git a/drivers/net/dwc_eth_qos_rockchip.c 
> b/drivers/net/dwc_eth_qos_rockchip.c
> index 05edc098f50d..20fd3a25c3dd 100644
> --- a/drivers/net/dwc_eth_qos_rockchip.c
> +++ b/drivers/net/dwc_eth_qos_rockchip.c
> @@ -28,6 +28,7 @@ struct rk_gmac_ops {
> int tx_delay, int rx_delay);
> int (*set_to_rmii)(struct udevice *dev);
> int (*set_gmac_speed)(struct udevice *dev);
> +   void (*set_clock_selection)(struct udevice *dev, bool enable);
> u32 regs[3];
>  };
>
> @@ -35,7 +36,9 @@ struct rockchip_platform_data {
> struct reset_ctl_bulk resets;
> const struct rk_gmac_ops *ops;
> int id;
> +   bool clock_input;
> struct regmap *grf;
> +   struct regmap *php_grf;
>  };
>
>  #define HIWORD_UPDATE(val, mask, shift) \
> @@ -129,6 +132,137 @@ static int rk3568_set_gmac_speed(struct udevice *dev)
> return 0;
>  }
>
> +/* sys_grf */
> +#define RK3588_GRF_GMAC_CON7   0x031c
> +#define RK3588_GRF_GMAC_CON8   0x0320
> +#define RK3588_GRF_GMAC_CON9   0x0324
> +
> +#define RK3588_GMAC_RXCLK_DLY_ENABLE(id)   GRF_BIT(2 * (id) + 3)
> +#define RK3588_GMAC_RXCLK_DLY_DISABLE(id)  GRF_CLR_BIT(2 * (id) + 3)
> +#define RK3588_GMAC_TXCLK_DLY_ENABLE(id)   GRF_BIT(2 * (id) + 2)
> +#define RK3588_GMAC_TXCLK_DLY_DISABLE(id)  GRF_CLR_BIT(2 * (id) + 2)
> +
> +#define RK3588_GMAC_CLK_RX_DL_CFG(val) HIWORD_UPDATE(val, 0xFF, 8)
> +#define RK3588_GMAC_CLK_TX_DL_CFG(val) HIWORD_UPDATE(val, 0xFF, 0)
> +
> +/* php_grf */
> +#define RK3588_GRF_GMAC_CON0   0x0008
> +#define RK3588_GRF_CLK_CON10x0070
> +
> +#define RK3588_GMAC_PHY_INTF_SEL_RGMII(id) \
> +   (GRF_BIT(3 + (id) * 6) | GRF_CLR_BIT(4 + (id) * 6) | GRF_CLR_BIT(5 + 
> (id) * 6))
> +#define RK3588_GMAC_PHY_INTF_SEL_RMII(id)  \
> +   (GRF_CLR_BIT(3 + (id) * 6) | GRF_CLR_BIT(4 + (id) * 6) | GRF_BIT(5 + 
> (id) * 6))
> +
> +#define RK3588_GMAC_CLK_RMII_MODE(id)  GRF_BIT(5 * (id))
> +#define RK3588_GMAC_CLK_RGMII_MODE(id) GRF_CLR_BIT(5 * (id))
> +
> +#define RK3588_GMAC_CLK_SELET_CRU(id)  GRF_BIT(5 * (id) + 4)
> +#define RK3588_GMAC_CLK_SELET_IO(id)   GRF_CLR_BIT(5 * (id) + 4)
> +
> +#define RK3588_GMAC_CLK_RMII_DIV2(id)  GRF_BIT(5 * (id) + 2)
> +#define RK3588_GMAC_CLK_RMII_DIV20(id) GRF_CLR_BIT(5 * (id) + 2)
> +
> +#define RK3588_GMAC_CLK_RGMII_DIV1(id) \
> +   (GRF_CLR_BIT(5 * (id) + 2) | GRF_CLR_BIT(5 * (id) + 
> 3))
> +#define RK3588_GMAC_CLK_RGMII_DIV5(id) \
> +   (GRF_BIT(5 * (id) + 2) | GRF_BIT(5 * (id) + 3))
> +#define RK3588_GMAC_CLK_RGMII_DIV50(id)\
> +   (GRF_CLR_BIT(5 * (id) + 2) | GRF_BIT(5 * (id) + 3))
> +
> +#define RK3588_GMAC_CLK_RMII_GATE(id)  GRF_BIT(5 * (id) + 1)
> +#define RK3588_GMAC_CLK_RMII_NOGATE(id)GRF_CLR_BIT(5 * (id) 
> + 1)
> +
> +static int rk3588_set_to_rgmii(struct udevice *dev,
> +  int tx_delay, int rx_delay)
> +{
> +   struct eth_pdata *pdata = dev_get_plat(dev);
> +   struct rockchip_platform_data *data = pdata->priv_pdata;
> +   u32 offset_con, id = data->id;
> +
> +   offset_con = data->id == 1 ? RK3588_GRF_GMAC_CON9 :
> +RK3588_GRF_GMAC_CON8;
> +
> +   regmap_write(data->php_grf, RK3588_GRF_GMAC_CON0,
> +RK3588_GMAC_PHY_INTF_SEL_RGMII(id));
> +
> +   regmap_write(data->php_grf, RK3588_GRF_CLK_CON1,
> +RK3588_GMAC_CLK_RGMII_MODE(id));
> +
> +   regmap_write(data->grf, RK3588_GRF_GMAC_CON7,
> + 

Re: [PATCH v2 4/7] net: dwc_eth_qos: Add glue driver for GMAC on Rockchip RK3568

2023-10-06 Thread Ramon Fried
On Sun, Oct 1, 2023 at 10:17 PM Jonas Karlman  wrote:
>
> Add a new glue driver for Rockchip SoCs, i.e RK3568, with a GMAC based
> on Synopsys DWC Ethernet QoS IP.
>
> rk_gmac_ops was ported from linux commit:
> 3bb3d6b1c195 ("net: stmmac: Add RK3566/RK3568 SoC support")
>
> Signed-off-by: Jonas Karlman 
> Reviewed-by: Kever Yang 
> ---
> Cc: David Wu 
> Cc: Ezequiel Garcia 
> ---
> v2:
> - Add comment about ported code intentionally left close to 1:1 with linux
> - Collect r-b tag
>
>  drivers/net/Kconfig|   8 +
>  drivers/net/Makefile   |   1 +
>  drivers/net/dwc_eth_qos.c  |   8 +-
>  drivers/net/dwc_eth_qos.h  |   2 +
>  drivers/net/dwc_eth_qos_rockchip.c | 357 +
>  5 files changed, 374 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/net/dwc_eth_qos_rockchip.c
>
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index 0ed39a61e4de..29304fd77759 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -225,6 +225,14 @@ config DWC_ETH_QOS_IMX
>   The Synopsys Designware Ethernet QOS IP block with the specific
>   configuration used in IMX soc.
>
> +config DWC_ETH_QOS_ROCKCHIP
> +   bool "Synopsys DWC Ethernet QOS device support for Rockchip SoCs"
> +   depends on DWC_ETH_QOS
> +   select DM_ETH_PHY
> +   help
> + The Synopsys Designware Ethernet QOS IP block with specific
> + configuration used in Rockchip SoCs.
> +
>  config DWC_ETH_QOS_STM32
> bool "Synopsys DWC Ethernet QOS device support for STM32"
> depends on DWC_ETH_QOS
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> index d4af253b6f28..1d444f5b4a69 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -20,6 +20,7 @@ obj-$(CONFIG_DRIVER_DM9000) += dm9000x.o
>  obj-$(CONFIG_DSA_SANDBOX) += dsa_sandbox.o
>  obj-$(CONFIG_DWC_ETH_QOS) += dwc_eth_qos.o
>  obj-$(CONFIG_DWC_ETH_QOS_IMX) += dwc_eth_qos_imx.o
> +obj-$(CONFIG_DWC_ETH_QOS_ROCKCHIP) += dwc_eth_qos_rockchip.o
>  obj-$(CONFIG_DWC_ETH_QOS_QCOM) += dwc_eth_qos_qcom.o
>  obj-$(CONFIG_DWC_ETH_QOS_STARFIVE) += dwc_eth_qos_starfive.o
>  obj-$(CONFIG_E1000) += e1000.o
> diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c
> index 24fb3fac1f12..9fb98a2c3c74 100644
> --- a/drivers/net/dwc_eth_qos.c
> +++ b/drivers/net/dwc_eth_qos.c
> @@ -1707,7 +1707,12 @@ static const struct udevice_id eqos_ids[] = {
> .data = (ulong)_imx_config
> },
>  #endif
> -
> +#if IS_ENABLED(CONFIG_DWC_ETH_QOS_ROCKCHIP)
> +   {
> +   .compatible = "rockchip,rk3568-gmac",
> +   .data = (ulong)_rockchip_config
> +   },
> +#endif
>  #if IS_ENABLED(CONFIG_DWC_ETH_QOS_QCOM)
> {
> .compatible = "qcom,qcs404-ethqos",
> @@ -1720,7 +1725,6 @@ static const struct udevice_id eqos_ids[] = {
> .data = (ulong)_jh7110_config
> },
>  #endif
> -
> { }
>  };
>
> diff --git a/drivers/net/dwc_eth_qos.h b/drivers/net/dwc_eth_qos.h
> index 06a082da72ef..e3222e1e17e5 100644
> --- a/drivers/net/dwc_eth_qos.h
> +++ b/drivers/net/dwc_eth_qos.h
> @@ -82,6 +82,7 @@ struct eqos_mac_regs {
>  #define EQOS_MAC_MDIO_ADDRESS_PA_SHIFT 21
>  #define EQOS_MAC_MDIO_ADDRESS_RDA_SHIFT16
>  #define EQOS_MAC_MDIO_ADDRESS_CR_SHIFT 8
> +#define EQOS_MAC_MDIO_ADDRESS_CR_100_150   1
>  #define EQOS_MAC_MDIO_ADDRESS_CR_20_35 2
>  #define EQOS_MAC_MDIO_ADDRESS_CR_250_300   5
>  #define EQOS_MAC_MDIO_ADDRESS_SKAP BIT(4)
> @@ -287,5 +288,6 @@ void eqos_flush_buffer_generic(void *buf, size_t size);
>  int eqos_null_ops(struct udevice *dev);
>
>  extern struct eqos_config eqos_imx_config;
> +extern struct eqos_config eqos_rockchip_config;
>  extern struct eqos_config eqos_qcom_config;
>  extern struct eqos_config eqos_jh7110_config;
> diff --git a/drivers/net/dwc_eth_qos_rockchip.c 
> b/drivers/net/dwc_eth_qos_rockchip.c
> new file mode 100644
> index ..05edc098f50d
> --- /dev/null
> +++ b/drivers/net/dwc_eth_qos_rockchip.c
> @@ -0,0 +1,357 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright Contributors to the U-Boot project.
> + *
> + * rk_gmac_ops ported from linux 
> drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> + *
> + * Ported code is intentionally left as close as possible with linux counter
> + * part in order to simplify future porting of fixes and support for other 
> SoCs.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "dwc_eth_qos.h"
> +
> +struct rk_gmac_ops {
> +   const char *compatible;
> +   int (*set_to_rgmii)(struct udevice *dev,
> +   int tx_delay, int rx_delay);
> +   int (*set_to_rmii)(struct udevice *dev);
> +   int 

Re: [PATCH v2 3/7] net: dwc_eth_qos: Stop spam of RX packet not available message

2023-10-06 Thread Ramon Fried
On Sun, Oct 1, 2023 at 10:17 PM Jonas Karlman  wrote:
>
> Remove spam of RX packet not available debug messages when waiting to
> receive a packet.
>
> Signed-off-by: Jonas Karlman 
> Reviewed-by: Kever Yang 
> ---
> v2:
> - Collect r-b tag
>
>  drivers/net/dwc_eth_qos.c | 8 +++-
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c
> index 7565e716823a..24fb3fac1f12 100644
> --- a/drivers/net/dwc_eth_qos.c
> +++ b/drivers/net/dwc_eth_qos.c
> @@ -1193,14 +1193,12 @@ static int eqos_recv(struct udevice *dev, int flags, 
> uchar **packetp)
> struct eqos_desc *rx_desc;
> int length;
>
> -   debug("%s(dev=%p, flags=%x):\n", __func__, dev, flags);
> -
> rx_desc = eqos_get_desc(eqos, eqos->rx_desc_idx, true);
> eqos->config->ops->eqos_inval_desc(rx_desc);
> -   if (rx_desc->des3 & EQOS_DESC3_OWN) {
> -   debug("%s: RX packet not available\n", __func__);
> +   if (rx_desc->des3 & EQOS_DESC3_OWN)
> return -EAGAIN;
> -   }
> +
> +   debug("%s(dev=%p, flags=%x):\n", __func__, dev, flags);
>
> *packetp = eqos->rx_dma_buf +
> (eqos->rx_desc_idx * EQOS_MAX_PACKET_SIZE);
> --
> 2.42.0
>
Reviewed-by: Ramon Fried 


Re: [PATCH v2 2/7] net: dwc_eth_qos: Return error code when start fails

2023-10-06 Thread Ramon Fried
On Sun, Oct 1, 2023 at 10:17 PM Jonas Karlman  wrote:
>
> Return error code when phy_connect fails or no link can be established.
>
> Signed-off-by: Jonas Karlman 
> Reviewed-by: Kever Yang 
> ---
> v2:
> - Collect r-b tag
>
>  drivers/net/dwc_eth_qos.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c
> index 555eaee3bbc3..7565e716823a 100644
> --- a/drivers/net/dwc_eth_qos.c
> +++ b/drivers/net/dwc_eth_qos.c
> @@ -811,6 +811,7 @@ static int eqos_start(struct udevice *dev)
>
> if (!eqos->phy) {
> pr_err("phy_connect() failed");
> +   ret = -ENODEV;
> goto err_stop_resets;
> }
>
> @@ -838,6 +839,7 @@ static int eqos_start(struct udevice *dev)
>
> if (!eqos->phy->link) {
> pr_err("No link");
> +   ret = -EAGAIN;
> goto err_shutdown_phy;
> }
>
> --
> 2.42.0
>
Reviewed-by: Ramon Fried 


Re: [PATCH v2 1/7] net: dwc_eth_qos: Drop unused rx_pkt from eqos_priv

2023-10-06 Thread Ramon Fried
On Sun, Oct 1, 2023 at 10:17 PM Jonas Karlman  wrote:
>
> rx_pkt is allocated and not used for anything, remove it.
>
> Signed-off-by: Jonas Karlman 
> Reviewed-by: Kever Yang 
> ---
> v2:
> - Collect r-b tag
>
>  drivers/net/dwc_eth_qos.c | 11 ---
>  drivers/net/dwc_eth_qos.h |  1 -
>  2 files changed, 12 deletions(-)
>
> diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c
> index 1e92bd9ca9c0..555eaee3bbc3 100644
> --- a/drivers/net/dwc_eth_qos.c
> +++ b/drivers/net/dwc_eth_qos.c
> @@ -1314,22 +1314,12 @@ static int eqos_probe_resources_core(struct udevice 
> *dev)
> }
> debug("%s: rx_dma_buf=%p\n", __func__, eqos->rx_dma_buf);
>
> -   eqos->rx_pkt = malloc(EQOS_MAX_PACKET_SIZE);
> -   if (!eqos->rx_pkt) {
> -   debug("%s: malloc(rx_pkt) failed\n", __func__);
> -   ret = -ENOMEM;
> -   goto err_free_rx_dma_buf;
> -   }
> -   debug("%s: rx_pkt=%p\n", __func__, eqos->rx_pkt);
> -
> eqos->config->ops->eqos_inval_buffer(eqos->rx_dma_buf,
> EQOS_MAX_PACKET_SIZE * EQOS_DESCRIPTORS_RX);
>
> debug("%s: OK\n", __func__);
> return 0;
>
> -err_free_rx_dma_buf:
> -   free(eqos->rx_dma_buf);
>  err_free_tx_dma_buf:
> free(eqos->tx_dma_buf);
>  err_free_descs:
> @@ -1348,7 +1338,6 @@ static int eqos_remove_resources_core(struct udevice 
> *dev)
>
> debug("%s(dev=%p):\n", __func__, dev);
>
> -   free(eqos->rx_pkt);
> free(eqos->rx_dma_buf);
> free(eqos->tx_dma_buf);
> eqos_free_descs(eqos->rx_descs);
> diff --git a/drivers/net/dwc_eth_qos.h b/drivers/net/dwc_eth_qos.h
> index a6b719af809f..06a082da72ef 100644
> --- a/drivers/net/dwc_eth_qos.h
> +++ b/drivers/net/dwc_eth_qos.h
> @@ -273,7 +273,6 @@ struct eqos_priv {
> unsigned int desc_per_cacheline;
> void *tx_dma_buf;
> void *rx_dma_buf;
> -   void *rx_pkt;
> bool started;
> bool reg_access_ok;
> bool clk_ck_enabled;
> --
> 2.42.0
>
Reviewed-by: Ramon Fried 


Re: [PATCH] net: ftgmac100: Add reset control

2023-10-06 Thread Ramon Fried
On Thu, Jul 27, 2023 at 4:58 AM Dylan Hung  wrote:
>
> Add optional reset control, especially for the Aspeed SOC. For the
> hardware without a reset line, the reset assertion/deassertion will be
> skipped.
>
> Signed-off-by: Dylan Hung 
> ---
>  drivers/net/ftgmac100.c | 12 
>  1 file changed, 12 insertions(+)
>
> diff --git a/drivers/net/ftgmac100.c b/drivers/net/ftgmac100.c
> index a50cde338a..886b97119d 100644
> --- a/drivers/net/ftgmac100.c
> +++ b/drivers/net/ftgmac100.c
> @@ -13,6 +13,7 @@
>
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -90,6 +91,7 @@ struct ftgmac100_data {
> u32 max_speed;
>
> struct clk_bulk clks;
> +   struct reset_ctl *reset_ctl;
>
> /* End of RX/TX ring buffer bits. Depend on model */
> u32 rxdes0_edorr_mask;
> @@ -568,6 +570,8 @@ static int ftgmac100_of_to_plat(struct udevice *dev)
> priv->txdes0_edotr_mask = BIT(15);
> }
>
> +   priv->reset_ctl = devm_reset_control_get_optional(dev, NULL);
> +
> return clk_get_bulk(dev, >clks);
>  }
>
> @@ -593,6 +597,12 @@ static int ftgmac100_probe(struct udevice *dev)
> if (ret)
> goto out;
>
> +   if (priv->reset_ctl) {
> +   ret = reset_deassert(priv->reset_ctl);
> +   if (ret)
> +   goto out;
> +   }
> +
> /*
>  * If DM MDIO is enabled, the MDIO bus will be initialized later in
>  * dm_eth_phy_connect
> @@ -628,6 +638,8 @@ static int ftgmac100_remove(struct udevice *dev)
> free(priv->phydev);
> mdio_unregister(priv->bus);
> mdio_free(priv->bus);
> +   if (priv->reset_ctl)
> +   reset_assert(priv->reset_ctl);
> clk_release_bulk(>clks);
>
> return 0;
> --
> 2.25.1
>
Reviewed-by: Ramon Fried 


Re: [PATCH] net: phy: TI DP83869 fix invalid clock delay configuration

2023-10-06 Thread Ramon Fried
On Fri, Oct 6, 2023 at 3:24 PM Frank de Brabander  wrote:
>
> Setting the clock delay from the device tree settings
> rx-internal-delay-ps and tx-internal-delay-ps was broken:
>
>  - The expected value in the device tree is suppose to be a
>delay in picoseconds, but the driver only allowed an array index.
>  - Driver converted this array index to the actual delay in
>picoseconds and tried to apply this in the device register. This
>however is not a valid register value. The actual logic here was
>reversed, it converted an register representation of the delay to
>the device tree delay in picoseconds.
>
> Only when the internal delays were NOT configured in the device tree
> and they default value of 7 (=2000ps) was used, a valid value was
> loaded in the register.
>
> Signed-off-by: Frank de Brabander 
> ---
>  drivers/net/phy/dp83869.c | 53 +++
>  1 file changed, 32 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c
> index 8d32d73b07..f9d4782580 100644
> --- a/drivers/net/phy/dp83869.c
> +++ b/drivers/net/phy/dp83869.c
> @@ -81,7 +81,10 @@
>
>  /* RGMIIDCTL bits */
>  #define DP83869_RGMII_TX_CLK_DELAY_SHIFT   4
> -#define DP83869_CLK_DELAY_DEF  7
> +#define DP83869_CLK_DELAY_STEP 250
> +#define DP83869_CLK_DELAY_MIN  250
> +#define DP83869_CLK_DELAY_MAX  4000
> +#define DP83869_CLK_DELAY_DEFAULT  2000
>
>  /* CFG2 bits */
>  #define MII_DP83869_CFG2_SPEEDOPT_10EN 0x0040
> @@ -157,10 +160,6 @@ static int dp83869_config_port_mirroring(struct 
> phy_device *phydev)
> return 0;
>  }
>
> -static const int dp83869_internal_delay[] = {250, 500, 750, 1000, 1250, 1500,
> -1750, 2000, 2250, 2500, 2750, 
> 3000,
> -3250, 3500, 3750, 4000};
> -
>  static int dp83869_set_strapped_mode(struct phy_device *phydev)
>  {
> struct dp83869_private *dp83869 = phydev->priv;
> @@ -183,7 +182,6 @@ static int dp83869_set_strapped_mode(struct phy_device 
> *phydev)
>  static int dp83869_of_init(struct phy_device *phydev)
>  {
> struct dp83869_private * const dp83869 = phydev->priv;
> -   const int delay_entries = ARRAY_SIZE(dp83869_internal_delay);
> int ret;
> ofnode node;
>
> @@ -238,32 +236,45 @@ static int dp83869_of_init(struct phy_device *phydev)
> dp83869->tx_fifo_depth = ofnode_read_s32_default(node, 
> "tx-fifo-depth",
>  
> DP83869_PHYCR_FIFO_DEPTH_4_B_NIB);
>
> +   /* Internal clock delay values can be configured in steps of
> +* 250ps (0.25ns). The register field for clock delay is 4-bits wide,
> +* the values range from 0b for 0.25ns to 0b for 4ns.
> +*/
> +
> /* RX delay *must* be specified if internal delay of RX is used. */
> if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
> -   dp83869->rx_int_delay = ofnode_read_u32_default(node, 
> "rx-internal-delay-ps",
> -   
> DP83869_CLK_DELAY_DEF);
> -   if (dp83869->rx_int_delay > delay_entries) {
> -   dp83869->rx_int_delay = DP83869_CLK_DELAY_DEF;
> -   pr_debug("rx-internal-delay-ps not set/invalid, 
> default to %ups\n",
> -
> dp83869_internal_delay[dp83869->rx_int_delay]);
> +   dp83869->rx_int_delay = ofnode_read_u32_default(node,
> +   "rx-internal-delay-ps", DP83869_CLK_DELAY_DEFAULT);
> +   if (dp83869->rx_int_delay > DP83869_CLK_DELAY_MAX ||
> +   dp83869->rx_int_delay < DP83869_CLK_DELAY_MIN ||
> +   dp83869->rx_int_delay % DP83869_CLK_DELAY_STEP) {
> +   dp83869->rx_int_delay = DP83869_CLK_DELAY_DEFAULT;
> +   pr_warn("rx-internal-delay-ps not set/invalid, 
> default"
> +   " to %ups\n", DP83869_CLK_DELAY_DEFAULT);
> }
>
> -   dp83869->rx_int_delay = 
> dp83869_internal_delay[dp83869->rx_int_delay];
> +   dp83869->rx_int_delay =
> +   (dp83869->rx_int_delay - DP83869_CLK_DELAY_STEP)
> +   / DP83869_CLK_DELAY_STEP;
> }
>
> -   /* TX delay *must* be specified if internal delay of RX is used. */
> +   /* TX delay *must* be specified if internal delay of TX is used. */
> if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
> -   dp83869->tx_int_delay = ofnode_read_u32_default(node, 
> "tx-internal-delay-ps",
> -  

Re: [PATCH] test/py: net: Add a test for 'pxe get' command

2023-10-06 Thread Ramon Fried
On Tue, Oct 3, 2023 at 4:12 PM Love Kumar  wrote:
>
> Execute the 'pxe get' command to download a pxe configuration file from
> the TFTP server and validate its interpretation.
>
> Signed-off-by: Love Kumar 
> ---
>  test/py/tests/test_net.py | 66 +++
>  1 file changed, 66 insertions(+)
>
> diff --git a/test/py/tests/test_net.py b/test/py/tests/test_net.py
> index cd4b4dc53cbc..b2241ae6a482 100644
> --- a/test/py/tests/test_net.py
> +++ b/test/py/tests/test_net.py
> @@ -6,6 +6,7 @@
>
>  import pytest
>  import u_boot_utils
> +import uuid
>
>  """
>  Note: This test relies on boardenv_* containing configuration values to 
> define
> @@ -61,6 +62,16 @@ env__net_nfs_readable_file = {
>  'crc32': 'c2244b26',
>  }
>
> +# Details regarding a file that may be read from a TFTP server. This variable
> +# may be omitted or set to None if PXE testing is not possible or desired.
> +env__net_pxe_readable_file = {
> +'fn': 'default',
> +'addr': 0x200,
> +'size': 74,
> +'timeout': 5,
> +'pattern': 'Linux',
> +}
> +
>  # True if a router advertisement service is connected to the network, and 
> should
>  # be tested. If router advertisement testing is not possible or desired, this
>  variable may be omitted or set to False.
> @@ -260,3 +271,58 @@ def test_net_nfs(u_boot_console):
>
>  output = u_boot_console.run_command('crc32 %x $filesize' % addr)
>  assert expected_crc in output
> +
> +@pytest.mark.buildconfigspec("cmd_net")
> +@pytest.mark.buildconfigspec("cmd_pxe")
> +def test_net_pxe_get(u_boot_console):
> +"""Test the pxe get command.
> +
> +A pxe configuration file is downloaded from the TFTP server and 
> interpreted
> +to boot the images mentioned in pxe configuration file.
> +
> +The details of the file to download are provided by the boardenv_* file;
> +see the comment at the beginning of this file.
> +"""
> +
> +if not net_set_up:
> +pytest.skip("Network not initialized")
> +
> +test_net_setup_static(u_boot_console)
> +
> +f = u_boot_console.config.env.get("env__net_pxe_readable_file", None)
> +if not f:
> +pytest.skip("No PXE readable file to read")
> +
> +addr = f.get("addr", None)
> +timeout = f.get("timeout", u_boot_console.p.timeout)
> +
> +pxeuuid = uuid.uuid1()
> +u_boot_console.run_command(f"setenv pxeuuid {pxeuuid}")
> +expected_text_uuid = f"Retrieving file: pxelinux.cfg/{pxeuuid}"
> +
> +ethaddr = u_boot_console.run_command("echo $ethaddr")
> +ethaddr = ethaddr.replace(':', '-')
> +expected_text_ethaddr = f"Retrieving file: pxelinux.cfg/01-{ethaddr}"
> +
> +ip = u_boot_console.run_command("echo $ipaddr")
> +ip = ip.split('.')
> +ipaddr_file = "".join(['%02x' % int(x) for x in ip]).upper()
> +expected_text_ipaddr = f"Retrieving file: pxelinux.cfg/{ipaddr_file}"
> +expected_text_default = f"Retrieving file: pxelinux.cfg/default"
> +
> +with u_boot_console.temporary_timeout(timeout):
> +output = u_boot_console.run_command("pxe get")
> +
> +assert "TIMEOUT" not in output
> +assert expected_text_uuid in output
> +assert expected_text_ethaddr in output
> +assert expected_text_ipaddr in output
> +
> +i = 1
> +for i in range(0, len(ipaddr_file) - 1):
> +expected_text_ip = f"Retrieving file: 
> pxelinux.cfg/{ipaddr_file[:-i]}"
> +assert expected_text_ip in output
> +i += 1
> +
> +assert expected_text_default in output
> +assert "Config file 'default.boot' found" in output
> --
> 2.25.1
>
Reviewed-by: Ramon Fried 


Re: [PATCH] test/py: net: Add dhcp abort test

2023-10-06 Thread Ramon Fried
On Tue, Oct 3, 2023 at 3:46 PM Love Kumar  wrote:
>
> Abort the dhcp request in the middle by pressing ctrl + c on u-boot
> prompt and validate the abort status.
>
> Signed-off-by: Love Kumar 
> ---
>  test/py/tests/test_net.py | 44 +++
>  1 file changed, 44 insertions(+)
>
> diff --git a/test/py/tests/test_net.py b/test/py/tests/test_net.py
> index cd4b4dc53cbc..1e8eb0357eef 100644
> --- a/test/py/tests/test_net.py
> +++ b/test/py/tests/test_net.py
> @@ -6,6 +6,7 @@
>
>  import pytest
>  import u_boot_utils
> +import re
>
>  """
>  Note: This test relies on boardenv_* containing configuration values to 
> define
> @@ -104,6 +105,49 @@ def test_net_dhcp(u_boot_console):
>  global net_set_up
>  net_set_up = True
>
> +@pytest.mark.buildconfigspec("cmd_dhcp")
> +def test_net_dhcp_abort(u_boot_console):
> +"""Test the dhcp command by pressing ctrl+c in the middle of dhcp request
> +
> +The boardenv_* file may be used to enable/disable this test; see the
> +comment at the beginning of this file.
> +"""
> +
> +test_dhcp = u_boot_console.config.env.get("env__net_dhcp_server", False)
> +if not test_dhcp:
> +pytest.skip("No DHCP server available")
> +
> +u_boot_console.run_command("setenv autoload no")
> +
> +# Phy reset before running dhcp command
> +output = u_boot_console.run_command("mii device")
> +eth_num = re.search(r"Current device: '(.+?)'", output).groups()[0]
> +u_boot_console.run_command(f"mii device {eth_num}")
> +output = u_boot_console.run_command("mii info")
> +eth_addr = hex(int(re.search(r"PHY (.+?):", output).groups()[0], 16))
> +u_boot_console.run_command(f"mii modify {eth_addr} 0 0x8000 0x8000")
> +
> +u_boot_console.run_command("dhcp", wait_for_prompt=False)
> +try:
> +u_boot_console.wait_for("Waiting for PHY auto negotiation to 
> complete")
> +except:
> +pytest.skip("Timeout waiting for PHY auto negotiation to complete")
> +
> +u_boot_console.wait_for("done")
> +
> +# Sending Ctrl-C
> +output = u_boot_console.run_command(
> +chr(3), wait_for_echo=False, send_nl=False
> +)
> +
> +assert "TIMEOUT" not in output
> +assert "DHCP client bound to address " not in output
> +assert "Abort" in output
> +
> +# Provide a time to recover from Abort - if it is not performed
> +# There is message like: ethernet@ff0e: No link.
> +u_boot_console.run_command("sleep 1")
> +
>  @pytest.mark.buildconfigspec('cmd_dhcp6')
>  def test_net_dhcp6(u_boot_console):
>  """Test the dhcp6 command.
> --
> 2.25.1
>
Reviewed-by: Ramon Fried 


Re: [PATCH] test/py: net: Add a TFTP put test

2023-10-06 Thread Ramon Fried
On Tue, Oct 3, 2023 at 12:02 PM Love Kumar  wrote:
>
> Execute tftpput command for uploading files to a server and validate its
> size & CRC32.
>
> Signed-off-by: Love Kumar 
> ---
>  test/py/tests/test_net.py | 69 +++
>  1 file changed, 69 insertions(+)
>
> diff --git a/test/py/tests/test_net.py b/test/py/tests/test_net.py
> index cd4b4dc53cbc..f69e3ea2dbba 100644
> --- a/test/py/tests/test_net.py
> +++ b/test/py/tests/test_net.py
> @@ -6,6 +6,7 @@
>
>  import pytest
>  import u_boot_utils
> +import datetime
>
>  """
>  Note: This test relies on boardenv_* containing configuration values to 
> define
> @@ -50,6 +51,7 @@ env__net_tftp_readable_file = {
>  'addr': 0x1000,
>  'size': 5058624,
>  'crc32': 'c2244b26',
> +'timeout': 5,
>  }
>
>  # Details regarding a file that may be read from a NFS server. This variable
> @@ -260,3 +262,70 @@ def test_net_nfs(u_boot_console):
>
>  output = u_boot_console.run_command('crc32 %x $filesize' % addr)
>  assert expected_crc in output
> +
> +@pytest.mark.buildconfigspec("cmd_crc32")
> +@pytest.mark.buildconfigspec("cmd_net")
> +def test_net_tftpput(u_boot_console):
> +"""Test the tftpput command.
> +A file is downloaded from the TFTP server and then uploaded to the TFTP
> +server, its size and its CRC32 are validated.
> +The details of the file to download are provided by the boardenv_* file;
> +see the comment at the beginning of this file.
> +"""
> +
> +if not net_set_up:
> +pytest.skip("Network not initialized")
> +
> +f = u_boot_console.config.env.get("env__net_tftp_readable_file", None)
> +if not f:
> +pytest.skip("No TFTP readable file to read")
> +
> +addr = f.get("addr", None)
> +if not addr:
> +addr = u_boot_utils.find_ram_base(u_boot_console)
> +
> +sz = f.get("size", None)
> +timeout = f.get("timeout", u_boot_console.p.timeout)
> +fn = f["fn"]
> +fnu = "_".join([datetime.datetime.now().strftime("%y%m%d%H%M%S"), fn])
> +expected_text = "Bytes transferred = "
> +if sz:
> +expected_text += "%d" % sz
> +
> +with u_boot_console.temporary_timeout(timeout):
> +output = u_boot_console.run_command("tftpboot %x %s" % (addr, fn))
> +
> +assert "TIMEOUT" not in output
> +assert expected_text in output
> +
> +expected_tftpb_crc = f.get("crc32", None)
> +
> +output = u_boot_console.run_command("crc32 $fileaddr $filesize")
> +assert expected_tftpb_crc in output
> +
> +with u_boot_console.temporary_timeout(timeout):
> +output = u_boot_console.run_command(
> +"tftpput $fileaddr $filesize $serverip:%s" % (fnu)
> +)
> +
> +expected_text = "Bytes transferred = "
> +if sz:
> +expected_text += "%d" % sz
> +addr = addr + sz
> +assert "TIMEOUT" not in output
> +assert "Access violation" not in output
> +assert expected_text in output
> +
> +with u_boot_console.temporary_timeout(timeout):
> +output = u_boot_console.run_command("tftpboot %x %s" % (addr, fnu))
> +
> +expected_text = "Bytes transferred = "
> +if sz:
> +expected_text += "%d" % sz
> +assert "TIMEOUT" not in output
> +assert expected_text in output
> +
> +expected_tftpp_crc = expected_tftpb_crc
> +
> +output = u_boot_console.run_command("crc32 $fileaddr $filesize")
> +assert expected_tftpp_crc in output
> --
> 2.25.1
>
Reviewed-by: Ramon Fried 


Re: [PATCH] sphinx: Bump urllib3 version

2023-10-06 Thread Heinrich Schuchardt

On 10/6/23 03:41, Simon Glass wrote:

On Thu, 5 Oct 2023 at 10:27, Tom Rini  wrote:


While not a direct issue for us, urllib3 before 1.26.17 is vulnerable to
CVE-2023-43804 to bump our version up.


The same bug is also fixed in 2.0.6. Why should we stick with the old
series? I could not see any issues building the documentation locally
and on Github with 2.0.6.

Best regards

Heinrich



Reported-by: GitHub dependabot
Signed-off-by: Tom Rini 
---
Cc: Heinrich Schuchardt 
---
  doc/sphinx/requirements.txt | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Simon Glass 




Re: [PATCH v7 2/2] schemas: Add some common reserved-memory usages

2023-10-06 Thread Simon Glass
Hi Ard,

On Fri, 6 Oct 2023 at 11:33, Ard Biesheuvel  wrote:
>
> On Mon, 2 Oct 2023 at 19:54, Simon Glass  wrote:
> >
> > Hi Rob,
> >
> > On Tue, 26 Sept 2023 at 13:42, Simon Glass  wrote:
> > >
> > > It is common to split firmware into 'Platform Init', which does the
> > > initial hardware setup and a "Payload" which selects the OS to be booted.
> > > Thus an handover interface is required between these two pieces.
> > >
> > > Where UEFI boot-time services are not available, but UEFI firmware is
> > > present on either side of this interface, information about memory usage
> > > and attributes must be presented to the "Payload" in some form.
> > >
> > > This aims to provide an small schema addition for the memory mapping
> > > needed to keep these two pieces working together well.
> > >
> > > Signed-off-by: Simon Glass 
> > > ---
> > >
> > > Changes in v7:
> > > - Rename acpi-reclaim to acpi
> > > - Drop individual mention of when memory can be reclaimed
> > > - Rewrite the item descriptions
> > > - Add back the UEFI text (with trepidation)
> >
> > I am again checking on this series. Can it be applied, please?
> >
>
> Apologies for the delay in response. I have been away.

OK, I hope you had a nice trip.

>
> >
> > >
> > > Changes in v6:
> > > - Drop mention of UEFI
> > > - Use compatible strings instead of node names
> > >
> > > Changes in v5:
> > > - Drop the memory-map node (should have done that in v4)
> > > - Tidy up schema a bit
> > >
> > > Changes in v4:
> > > - Make use of the reserved-memory node instead of creating a new one
> > >
> > > Changes in v3:
> > > - Reword commit message again
> > > - cc a lot more people, from the FFI patch
> > > - Split out the attributes into the /memory nodes
> > >
> > > Changes in v2:
> > > - Reword commit message
> > >
> > >  .../reserved-memory/common-reserved.yaml  | 71 +++
> > >  1 file changed, 71 insertions(+)
> > >  create mode 100644 dtschema/schemas/reserved-memory/common-reserved.yaml
> > >
> > > diff --git a/dtschema/schemas/reserved-memory/common-reserved.yaml 
> > > b/dtschema/schemas/reserved-memory/common-reserved.yaml
> > > new file mode 100644
> > > index 000..f7fbdfd
> > > --- /dev/null
> > > +++ b/dtschema/schemas/reserved-memory/common-reserved.yaml
> > > @@ -0,0 +1,71 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/reserved-memory/common-reserved.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Common memory reservations
> > > +
> > > +description: |
> > > +  Specifies that the reserved memory region can be used for the purpose
> > > +  indicated by its compatible string.
> > > +
> > > +  Clients may reuse this reserved memory if they understand what it is 
> > > for,
> > > +  subject to the notes below.
> > > +
> > > +maintainers:
> > > +  - Simon Glass 
> > > +
> > > +allOf:
> > > +  - $ref: reserved-memory.yaml
> > > +
> > > +properties:
> > > +  compatible:
> > > +description: |
> > > +  This describes some common memory reservations, with the compatible
> > > +  string indicating what it is used for:
> > > +
> > > + acpi: Advanced Configuration and Power Interface (ACPI) tables
> > > + acpi-nvs: ACPI Non-Volatile-Sleeping Memory (NVS). This is 
> > > reserved by
> > > +   the firmware for its use and is required to be saved and 
> > > restored
> > > +   across an NVS sleep
> > > + boot-code: Contains code used for booting which is not needed 
> > > by the OS
> > > + boot-code: Contains data used for booting which is not needed 
> > > by the OS
> > > + runtime-code: Contains code used for interacting with the 
> > > system when
> > > +   running the OS
> > > + runtime-data: Contains data used for interacting with the 
> > > system when
> > > +   running the OS
> > > +
> > > +enum:
> > > +  - acpi
> > > +  - acpi-nvs
> > > +  - boot-code
> > > +  - boot-data
> > > +  - runtime-code
> > > +  - runtime-data
> > > +
>
> As I mentioned a few times already, I don't think these compatibles
> should be introduced here.
>
> A reserved region has a specific purpose, and the compatible should be
> more descriptive than the enum above. If the consumer does not
> understand this purpose, it should simply treat the memory as reserved
> and not touch it. Alternatively, these regions can be referenced from
> other DT nodes using phandles if needed.

We still need some description of what these regions are used for, so
that the payload can use the correct regions. I do not have any other
solution to this problem. We are in v7 at present. At least explain
where you want the compatible strings to be introduced.

What sort of extra detail are you looking for? Please be specific and
preferably add some suggestions so I can close this out ASAP.

>
>
> > > +  reg:
> > > +description: 

Re: [PATCH v7 2/2] schemas: Add some common reserved-memory usages

2023-10-06 Thread Ard Biesheuvel
On Mon, 2 Oct 2023 at 19:54, Simon Glass  wrote:
>
> Hi Rob,
>
> On Tue, 26 Sept 2023 at 13:42, Simon Glass  wrote:
> >
> > It is common to split firmware into 'Platform Init', which does the
> > initial hardware setup and a "Payload" which selects the OS to be booted.
> > Thus an handover interface is required between these two pieces.
> >
> > Where UEFI boot-time services are not available, but UEFI firmware is
> > present on either side of this interface, information about memory usage
> > and attributes must be presented to the "Payload" in some form.
> >
> > This aims to provide an small schema addition for the memory mapping
> > needed to keep these two pieces working together well.
> >
> > Signed-off-by: Simon Glass 
> > ---
> >
> > Changes in v7:
> > - Rename acpi-reclaim to acpi
> > - Drop individual mention of when memory can be reclaimed
> > - Rewrite the item descriptions
> > - Add back the UEFI text (with trepidation)
>
> I am again checking on this series. Can it be applied, please?
>

Apologies for the delay in response. I have been away.

>
> >
> > Changes in v6:
> > - Drop mention of UEFI
> > - Use compatible strings instead of node names
> >
> > Changes in v5:
> > - Drop the memory-map node (should have done that in v4)
> > - Tidy up schema a bit
> >
> > Changes in v4:
> > - Make use of the reserved-memory node instead of creating a new one
> >
> > Changes in v3:
> > - Reword commit message again
> > - cc a lot more people, from the FFI patch
> > - Split out the attributes into the /memory nodes
> >
> > Changes in v2:
> > - Reword commit message
> >
> >  .../reserved-memory/common-reserved.yaml  | 71 +++
> >  1 file changed, 71 insertions(+)
> >  create mode 100644 dtschema/schemas/reserved-memory/common-reserved.yaml
> >
> > diff --git a/dtschema/schemas/reserved-memory/common-reserved.yaml 
> > b/dtschema/schemas/reserved-memory/common-reserved.yaml
> > new file mode 100644
> > index 000..f7fbdfd
> > --- /dev/null
> > +++ b/dtschema/schemas/reserved-memory/common-reserved.yaml
> > @@ -0,0 +1,71 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/reserved-memory/common-reserved.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Common memory reservations
> > +
> > +description: |
> > +  Specifies that the reserved memory region can be used for the purpose
> > +  indicated by its compatible string.
> > +
> > +  Clients may reuse this reserved memory if they understand what it is for,
> > +  subject to the notes below.
> > +
> > +maintainers:
> > +  - Simon Glass 
> > +
> > +allOf:
> > +  - $ref: reserved-memory.yaml
> > +
> > +properties:
> > +  compatible:
> > +description: |
> > +  This describes some common memory reservations, with the compatible
> > +  string indicating what it is used for:
> > +
> > + acpi: Advanced Configuration and Power Interface (ACPI) tables
> > + acpi-nvs: ACPI Non-Volatile-Sleeping Memory (NVS). This is 
> > reserved by
> > +   the firmware for its use and is required to be saved and 
> > restored
> > +   across an NVS sleep
> > + boot-code: Contains code used for booting which is not needed by 
> > the OS
> > + boot-code: Contains data used for booting which is not needed by 
> > the OS
> > + runtime-code: Contains code used for interacting with the system 
> > when
> > +   running the OS
> > + runtime-data: Contains data used for interacting with the system 
> > when
> > +   running the OS
> > +
> > +enum:
> > +  - acpi
> > +  - acpi-nvs
> > +  - boot-code
> > +  - boot-data
> > +  - runtime-code
> > +  - runtime-data
> > +

As I mentioned a few times already, I don't think these compatibles
should be introduced here.

A reserved region has a specific purpose, and the compatible should be
more descriptive than the enum above. If the consumer does not
understand this purpose, it should simply treat the memory as reserved
and not touch it. Alternatively, these regions can be referenced from
other DT nodes using phandles if needed.


> > +  reg:
> > +description: region of memory that is reserved for the purpose 
> > indicated
> > +  by the compatible string.
> > +
> > +required:
> > +  - reg
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +reserved-memory {
> > +#address-cells = <1>;
> > +#size-cells = <1>;
> > +
> > +reserved@1234 {
> > +compatible = "boot-code";
> > +reg = <0x1234 0x0080>;
> > +};
> > +
> > +reserved@4321 {
> > +compatible = "boot-data";
> > +reg = <0x4321 0x0080>;
> > +};
> > +};
> > --
> > 2.42.0.515.g380fc7ccd1-goog
> >
>
> Regards,
> Simon


Re: [PATCH 05/25] treewide: Correct use of long help

2023-10-06 Thread Tom Rini
On Fri, Oct 06, 2023 at 07:03:17AM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Thu, 5 Oct 2023 at 20:16, Tom Rini  wrote:
> >
> > On Thu, Oct 05, 2023 at 07:41:49PM -0600, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Thu, 5 Oct 2023 at 08:53, Tom Rini  wrote:
> > > >
> > > > On Wed, Oct 04, 2023 at 07:23:47PM -0600, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Sun, 24 Sept 2023 at 17:27, Tom Rini  wrote:
> > > > > >
> > > > > > On Sun, Sep 24, 2023 at 02:39:23PM -0600, Simon Glass wrote:
> > > > > > > Some commands assume that CONFIG_SYS_LONGHELP is always defined.
> > > > > > > Declaration of long help should be bracketed by an #ifdef to 
> > > > > > > avoid an
> > > > > > > 'unused variable' warning.
> > > > > > >
> > > > > > > Fix this treewide.
> > > > > > >
> > > > > > > Signed-off-by: Simon Glass 
> > > > > > [snip]
> > > > > > > diff --git a/arch/arm/mach-imx/cmd_dek.c 
> > > > > > > b/arch/arm/mach-imx/cmd_dek.c
> > > > > > > index 6fa5b41fcd38..25ea7d3b37da 100644
> > > > > > > --- a/arch/arm/mach-imx/cmd_dek.c
> > > > > > > +++ b/arch/arm/mach-imx/cmd_dek.c
> > > > > > > @@ -393,11 +393,12 @@ static int do_dek_blob(struct cmd_tbl 
> > > > > > > *cmdtp, int flag, int argc,
> > > > > > >   return blob_encap_dek(src_addr, dst_addr, len);
> > > > > > >  }
> > > > > > >
> > > > > > > -/***/
> > > > > > > +#if IS_ENABLED(CONFIG_SYS_LONGHELP)
> > > > > > >  static char dek_blob_help_text[] =
> > > > > > >   "src dst len- Encapsulate and create blob of 
> > > > > > > data\n"
> > > > > > >   " $len bits long at address $src 
> > > > > > > and\n"
> > > > > > >   " store the result at address 
> > > > > > > $dst.\n";
> > > > > > > +#endif
> > > > > > >
> > > > > > >  U_BOOT_CMD(
> > > > > > >   dek_blob, 4, 1, do_dek_blob,
> > > > > >
> > > > > > This really doesn't read nicely.  I would rather (globally and fix
> > > > > > existing users) __maybe_unused this instead.  I think there's just 
> > > > > > one
> > > > > > example today that isn't "foo_help_text".
> > > > >
> > > > > Hmm, what do you think about adding a __longhelp symbol to cause the
> > > > > linker to discard it when not needed?
> > > >
> > > > Well, I don't think we need linker list magic when __maybe_unused will
> > > > just have them be discarded normally.
> > >
> > > Yes, perhaps things are in a better state than they used to be, but
> > > there is a linker discard for commands at present.
> >
> > Yes, but __maybe_unused means we don't get a warning about it, and it's
> > literally discarded as part of --gc-sections as it done everywhere with
> > maybe 3 exceptions?
> 
> Actually with this series we get a lot closer to that. The problem
> with the status quo is that disabling CMDLINE doesn't disable most
> commands, relying instead on discarding them at link time.

I don't follow you here.  We're talking only about the long help.
There's already an option to enable/disable it.  When disabled all of
the long help text should be discarded, because we reference it via
U_BOOT_CMD macro which in turn cares about it, or not.  What's missing
is a U_BOOT_LONGHELP macro so that instead of:
#ifdef CONFIG_SYS_LONGHELP
static char cat_help_text[] =
"  \n"
"  - Print file from 'dev' on 'interface' to standard output\n";
#endif

We do:
U_BOOT_LONGHELP(cat,
"  \n"
"  - Print file from 'dev' on 'interface' to standard output\n"
);

Which then does:
static __maybe_unused char cat_help_text[] =
...
;

And we discard the text automatically due to --gc-sections.  We possibly
haven't already been doing this since back when we first started using
--gc-sections there was a bug in binutils that caused text like the
above to still get combined in to other objects and not discarded.
That's been fixed for ages.

And the above macro would also let us clean up U_BOOT_CMD macro slightly
to just omit the longhelp option and instead always do _CMD_HELP(_name)
inside the macros.  It'll also make it harder to add new commands
without a long help by accident.

> With this series, it looks like we can in fact do that, so I should
> remove the discards as well.
> 
> The one proviso is that this does drop a lot of things we want...e.g.
> BOOTSTD_DEFAULTS cannot be enabled due to its use of 'select', meaning
> that common filesystems are dropped. So we'll need more effort after
> this, to allow (for example) bootmeths that don't need commands to
> work correctly. But I think this series at least provides a better
> starting point for teasing things apart.
> 
> So OK I'll go with __maybe_unused for the ones that need it, which
> isn't too many given that many of the strings are just included
> directly in the macro. It is considerably easier than trying to be to
> clever about things.

Yes, this series itself has a lot of problems that need to be addressed
before reposting because I don't like "hiding" 

Re: [RFC PATCH 2/2] board: ti: am65x: Move to using Extension framework

2023-10-06 Thread Simon Glass
Hi Köry,

On Fri, 6 Oct 2023 at 07:26, Köry Maincent  wrote:
>
> On Wed, 4 Oct 2023 15:39:23 +0300
> Roger Quadros  wrote:
>
> Hello,
> Thanks for adding me in cc. Also it seems I forgot to add myself to 
> MAINTAINERS
> for the extension_board.c file.
>
> > >>> Before this goes too far I think this should move to using a linker
> > >>> list to declare the driver (or a driver-model driver if you prefer,
> > >>> but that might be overkill).
> >
> > So I've been working on this on the side and got linker list way working
> > with custom script booting but as soon as I move to standard boot flow
> > it no longer works. This is because there is no code in place to
> > apply the overlay and pass it to next stage e.g. EFI.
> >
> > I see the following note at
> > https://elixir.bootlin.com/u-boot/latest/source/boot/bootmeth_efi.c#L304
> >
> > "
> > /*
> >  * TODO: Apply extension overlay
> >  *
> >  * Here we need to load and apply the extension overlay. 
> > This
> > is
> >  * not implemented. See do_extension_apply(). The extension
> >  * stuff needs an implementation in boot/extension.c so it 
> > is
> >  * separate from the command code. Really the extension 
> > stuff
> >  * should use the device tree and a uclass / driver 
> > interface
> >  * rather than implementing its own list
> >  */
> > "
>
> I agreed that the extension implementation should move in boot/extension.c or
> common for general use. I am wondering what the advantage of creating an 
> uclass
> interface?
> I am not an uclass expert but there is no per driver ops and usage. What do 
> you
> dislike about using its own list?

I looked at this briefly for bootstd and left it alone, partly for this reason.

The operations I know about are:
- scan for extension boards to produce a list: needs to happen at
start of 'bootflow scan' I think)
- applying relevant overlays: needs to happen in the read_bootflow()
method perhaps
- listing available extensions

I think a uclass makes sense, along with separating out the
'extension' command from the actual functionality.

>
> > Another note at
> > https://elixir.bootlin.com/u-boot/latest/source/cmd/extension_board.c#L198
> >
> > "/* extensions should have a uclass - for now we use UCLASS_SIMPLE_BUS 
> > uclass
> > */"
> >
> > So are we better off implementing a class driver for extension stuff?
>
> I think the first point should be to move it in common or boot and makes it
> generic for using the extension function everywhere. I will let Simon answer
> about the uclass part.

I believe this relates to boot/

>
> Regards,

Regards,
Simon


Re: [PATCH v2 1/3] dt-bindings: mtd: fixed-partitions: Add binman compatible

2023-10-06 Thread Rob Herring
On Fri, Oct 06, 2023 at 10:37:41AM +0200, Michael Walle wrote:
> Hi,
> 
> > > I'm still not sure why that compatible is needed. Also I'd need to
> > > change
> > > the label which might break user space apps looking for that specific
> > > name.
> > > 
> > > Also, our board might have u-boot/spl or u-boot/spl/bl31/bl32, right
> > > now
> > > that's something which depends on an u-boot configuration variable,
> > > which
> > > then enables or disables binman nodes in the -u-boot.dtsi. So in linux
> > > we only have that "bootloader" partition, but there might be either
> > > u-boot+spl or u-boot+spl+bl31+bl32.
> > > 
> > > Honestly, I'm really not sure this should go into a device tree.
> > 
> > I think we might be getting a bit ahead of ourselves here. I thought
> > that the decision was that the label should indicate the contents.
> > If you have multiple things in a partition then it would become a
> > 'section' in Binman's terminology. Either the label programmatically
> > describes what is inside or it doesn't. We can't have it both ways.
> > What do you suggest?
> 
> As Rob pointed out earlier, it's just a user-facing string. I'm a bit
> reluctant to use it programatically.

In general, yes, but the partition stuff has long (and still) uses 
label. As long as the values the tools understand are documented (which 
we don't normally do for label), I don't care so much. That's my 
opinion as long as this is shared with fixed-partitions. If it is not 
and there's little reason to use label, then absolutely, I think 
'compatible' makes more sense. 

> Taking my example again, the string "bootloader" is sufficient for a
> user. He doesn't care if it's u-boot with spl or u-boot with tfa, or
> even coreboot. It just says, "in this partition is the bootloader".
> If you have an "bootloader" image you can flash it there.

These days, there's generally not just 1 bootloader in the boot flow. 
Maybe there's 1 image, maybe not. Being more specific is hardly ever a 
bad thing. Only when the number of specific things becomes multiple 10s 
or 100s of them does it become a problem.


> If it has a label "u-boot" and I want to switch to coreboot, will
> it have to change to "coreboot"? I really don't think this is practical,
> you are really putting software configuration into the device tree.

On the input side (to binman), yes it is config, but on the output side 
(to the running system) we are saying what's there.


> > At present it seems you have the image described in two places - one
> > is the binman node and the other is the partitions node. I would like
> > to unify these.
> 
> And I'm not sure that will work for all the corner cases :/
> 
> If you keep the binman section seperate from the flash partition
> definition you don't have any of these problems, although there is
> some redundancy:
>  - you only have compatible = "binman", "fixed-partition", no further
>compatibles are required
>  - you don't have any conflicts with the current partition descriptions
>  - you could even use the labels, because binman is the (only?) user
> 
> But of course you need to find a place where to put your node.

And remove it. We don't need 2 sources of truth in the DTB.

Rob


Re: [RFC PATCH 2/2] board: ti: am65x: Move to using Extension framework

2023-10-06 Thread Roger Quadros


On 06/10/2023 16:26, Köry Maincent wrote:
> On Wed, 4 Oct 2023 15:39:23 +0300
> Roger Quadros  wrote:
> 
> Hello,
> Thanks for adding me in cc. Also it seems I forgot to add myself to 
> MAINTAINERS
> for the extension_board.c file.
> 
> Before this goes too far I think this should move to using a linker
> list to declare the driver (or a driver-model driver if you prefer,
> but that might be overkill).  
>>
>> So I've been working on this on the side and got linker list way working
>> with custom script booting but as soon as I move to standard boot flow
>> it no longer works. This is because there is no code in place to
>> apply the overlay and pass it to next stage e.g. EFI.
>>
>> I see the following note at
>> https://elixir.bootlin.com/u-boot/latest/source/boot/bootmeth_efi.c#L304
>>
>> "
>> /*
>>  * TODO: Apply extension overlay
>>  *
>>  * Here we need to load and apply the extension overlay. This
>> is
>>  * not implemented. See do_extension_apply(). The extension
>>  * stuff needs an implementation in boot/extension.c so it is
>>  * separate from the command code. Really the extension stuff
>>  * should use the device tree and a uclass / driver interface
>>  * rather than implementing its own list
>>  */
>> "
> 
> I agreed that the extension implementation should move in boot/extension.c or
> common for general use. I am wondering what the advantage of creating an 
> uclass
> interface?
> I am not an uclass expert but there is no per driver ops and usage. What do 
> you
> dislike about using its own list?

There would be per platform ops for probing the extension cards in a platform
specific way.

I already have a linker list way of doing this but stumbled upon Simon's
comments about using uclass for this.

> 
>> Another note at
>> https://elixir.bootlin.com/u-boot/latest/source/cmd/extension_board.c#L198
>>
>> "/* extensions should have a uclass - for now we use UCLASS_SIMPLE_BUS uclass
>> */"
>>
>> So are we better off implementing a class driver for extension stuff?
> 
> I think the first point should be to move it in common or boot and makes it
> generic for using the extension function everywhere. I will let Simon answer
> about the uclass part.
> 
> Regards,

-- 
cheers,
-roger


Re: [RFC PATCH 2/2] board: ti: am65x: Move to using Extension framework

2023-10-06 Thread Köry Maincent
On Wed, 4 Oct 2023 15:39:23 +0300
Roger Quadros  wrote:

Hello,
Thanks for adding me in cc. Also it seems I forgot to add myself to MAINTAINERS
for the extension_board.c file.

> >>> Before this goes too far I think this should move to using a linker
> >>> list to declare the driver (or a driver-model driver if you prefer,
> >>> but that might be overkill).  
> 
> So I've been working on this on the side and got linker list way working
> with custom script booting but as soon as I move to standard boot flow
> it no longer works. This is because there is no code in place to
> apply the overlay and pass it to next stage e.g. EFI.
> 
> I see the following note at
> https://elixir.bootlin.com/u-boot/latest/source/boot/bootmeth_efi.c#L304
> 
> "
> /*
>  * TODO: Apply extension overlay
>  *
>  * Here we need to load and apply the extension overlay. This
> is
>  * not implemented. See do_extension_apply(). The extension
>  * stuff needs an implementation in boot/extension.c so it is
>  * separate from the command code. Really the extension stuff
>  * should use the device tree and a uclass / driver interface
>  * rather than implementing its own list
>  */
> "

I agreed that the extension implementation should move in boot/extension.c or
common for general use. I am wondering what the advantage of creating an uclass
interface?
I am not an uclass expert but there is no per driver ops and usage. What do you
dislike about using its own list?

> Another note at
> https://elixir.bootlin.com/u-boot/latest/source/cmd/extension_board.c#L198
> 
> "/* extensions should have a uclass - for now we use UCLASS_SIMPLE_BUS uclass
> */"
> 
> So are we better off implementing a class driver for extension stuff?

I think the first point should be to move it in common or boot and makes it
generic for using the extension function everywhere. I will let Simon answer
about the uclass part.

Regards,


Re: [PATCH 05/25] treewide: Correct use of long help

2023-10-06 Thread Simon Glass
Hi Tom,

On Thu, 5 Oct 2023 at 20:16, Tom Rini  wrote:
>
> On Thu, Oct 05, 2023 at 07:41:49PM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Thu, 5 Oct 2023 at 08:53, Tom Rini  wrote:
> > >
> > > On Wed, Oct 04, 2023 at 07:23:47PM -0600, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Sun, 24 Sept 2023 at 17:27, Tom Rini  wrote:
> > > > >
> > > > > On Sun, Sep 24, 2023 at 02:39:23PM -0600, Simon Glass wrote:
> > > > > > Some commands assume that CONFIG_SYS_LONGHELP is always defined.
> > > > > > Declaration of long help should be bracketed by an #ifdef to avoid 
> > > > > > an
> > > > > > 'unused variable' warning.
> > > > > >
> > > > > > Fix this treewide.
> > > > > >
> > > > > > Signed-off-by: Simon Glass 
> > > > > [snip]
> > > > > > diff --git a/arch/arm/mach-imx/cmd_dek.c 
> > > > > > b/arch/arm/mach-imx/cmd_dek.c
> > > > > > index 6fa5b41fcd38..25ea7d3b37da 100644
> > > > > > --- a/arch/arm/mach-imx/cmd_dek.c
> > > > > > +++ b/arch/arm/mach-imx/cmd_dek.c
> > > > > > @@ -393,11 +393,12 @@ static int do_dek_blob(struct cmd_tbl *cmdtp, 
> > > > > > int flag, int argc,
> > > > > >   return blob_encap_dek(src_addr, dst_addr, len);
> > > > > >  }
> > > > > >
> > > > > > -/***/
> > > > > > +#if IS_ENABLED(CONFIG_SYS_LONGHELP)
> > > > > >  static char dek_blob_help_text[] =
> > > > > >   "src dst len- Encapsulate and create blob of 
> > > > > > data\n"
> > > > > >   " $len bits long at address $src 
> > > > > > and\n"
> > > > > >   " store the result at address 
> > > > > > $dst.\n";
> > > > > > +#endif
> > > > > >
> > > > > >  U_BOOT_CMD(
> > > > > >   dek_blob, 4, 1, do_dek_blob,
> > > > >
> > > > > This really doesn't read nicely.  I would rather (globally and fix
> > > > > existing users) __maybe_unused this instead.  I think there's just one
> > > > > example today that isn't "foo_help_text".
> > > >
> > > > Hmm, what do you think about adding a __longhelp symbol to cause the
> > > > linker to discard it when not needed?
> > >
> > > Well, I don't think we need linker list magic when __maybe_unused will
> > > just have them be discarded normally.
> >
> > Yes, perhaps things are in a better state than they used to be, but
> > there is a linker discard for commands at present.
>
> Yes, but __maybe_unused means we don't get a warning about it, and it's
> literally discarded as part of --gc-sections as it done everywhere with
> maybe 3 exceptions?

Actually with this series we get a lot closer to that. The problem
with the status quo is that disabling CMDLINE doesn't disable most
commands, relying instead on discarding them at link time.

With this series, it looks like we can in fact do that, so I should
remove the discards as well.

The one proviso is that this does drop a lot of things we want...e.g.
BOOTSTD_DEFAULTS cannot be enabled due to its use of 'select', meaning
that common filesystems are dropped. So we'll need more effort after
this, to allow (for example) bootmeths that don't need commands to
work correctly. But I think this series at least provides a better
starting point for teasing things apart.

So OK I'll go with __maybe_unused for the ones that need it, which
isn't too many given that many of the strings are just included
directly in the macro. It is considerably easier than trying to be to
clever about things.

Regards,
Simon


RE: [PATCH 16/16] board: rzg2l: Add RZ/G2L SMARC EVK board

2023-10-06 Thread Biju Das
Hi Paul,

> Subject: Re: [PATCH 16/16] board: rzg2l: Add RZ/G2L SMARC EVK board
> 
> On 03/10/2023 14:36, Marek Vasut wrote:
> > On 9/20/23 14:42, Paul Barker wrote:
> >> The Renesas RZ/G2L SMARC Evaluation Board Kit consists of the RZ/G2L
> >> System-on-Module (SOM) based on the R9A07G044L2 SoC, and a common
> >> SMARC carrier board.
> >>
> >> The ARM TrustedFirmware code for the Renesas RZ/G2L SoC family passes
> >> a devicetree blob to the bootloader as an argument in the same was
> >> previous R-Car gen3/gen4 SoCs. This blob contains a compatible string
> >> which can be used to identify the particular SoC we are running on
> >> and this is used to select the appropriate device tree to load.
> >>
> >> The configuration renesas_rzg2l_smarc_defconfig is added to support
> >> building for this target. In the future this defconfig will be
> >> extended to support other SoCs and evaluation boards from the RZ/G2L
> family.
> >>
> >> Signed-off-by: Paul Barker 
> >> Reviewed-by: Biju Das 
> >> Reviewed-by: Lad Prabhakar 
> >> ---
> >>   arch/arm/mach-rmobile/Kconfig.rzg2l   | 14 +
> >>   board/renesas/rzg2l/Kconfig   | 18 +++
> >>   board/renesas/rzg2l/MAINTAINERS   |  6 +++
> >>   board/renesas/rzg2l/Makefile  |  4 ++
> >>   board/renesas/rzg2l/rzg2l.c   | 76 +++
> >>   configs/renesas_rzg2l_smarc_defconfig | 52 ++
> >>   include/configs/rzg2l-smarc.h | 14 +
> >>   7 files changed, 184 insertions(+)
> >>   create mode 100644 board/renesas/rzg2l/Kconfig
> >>   create mode 100644 board/renesas/rzg2l/MAINTAINERS
> >>   create mode 100644 board/renesas/rzg2l/Makefile
> >>   create mode 100644 board/renesas/rzg2l/rzg2l.c
> >>   create mode 100644 configs/renesas_rzg2l_smarc_defconfig
> >>   create mode 100644 include/configs/rzg2l-smarc.h
> >>
> >> diff --git a/arch/arm/mach-rmobile/Kconfig.rzg2l
> >> b/arch/arm/mach-rmobile/Kconfig.rzg2l
> >> index 7d268e8c366a..1fe49e323300 100644
> >> --- a/arch/arm/mach-rmobile/Kconfig.rzg2l
> >> +++ b/arch/arm/mach-rmobile/Kconfig.rzg2l
> >> @@ -9,6 +9,20 @@ config R9A07G044L
> >>help
> >>  Enable support for the R9A07G044L SoC used in the RZ/G2L.
> >>
> >> +choice
> >> +  prompt "Renesas RZ/G2L Family Board selection"
> >> +  default TARGET_RZG2L_SMARC_EVK
> >> +
> >> +config TARGET_RZG2L_SMARC_EVK
> >> +  bool "Renesas RZ/G2L SMARC EVK"
> >> +  imply R9A07G044L
> >> +  help
> >> +Enable support for the RZ/G2L SMARC evaluation board.
> >> +
> >> +source "board/renesas/rzg2l/Kconfig"
> >> +
> >> +endchoice
> >> +
> >>   config MULTI_DTB_FIT_UNCOMPRESS_SZ
> >>default 0x8 if TARGET_RZG2L_SMARC_EVK
> >>
> >> diff --git a/board/renesas/rzg2l/Kconfig
> >> b/board/renesas/rzg2l/Kconfig new file mode 100644 index
> >> ..1335fc7ae806
> >> --- /dev/null
> >> +++ b/board/renesas/rzg2l/Kconfig
> >> @@ -0,0 +1,18 @@
> >> +# Copyright (C) 2023 Renesas Electronics Corporation #
> >> +SPDX-License-Identifier: GPL-2.0+
> >> +
> >> +if TARGET_RZG2L_SMARC_EVK
> >> +
> >> +config SYS_SOC
> >> +  default "rmobile"
> >> +
> >> +config SYS_BOARD
> >> +  default "rzg2l"
> >> +
> >> +config SYS_VENDOR
> >> +  default "renesas"
> >> +
> >> +config SYS_CONFIG_NAME
> >> +  default "rzg2l-smarc"
> >> +
> >> +endif
> >> diff --git a/board/renesas/rzg2l/MAINTAINERS
> >> b/board/renesas/rzg2l/MAINTAINERS new file mode 100644 index
> >> ..0a51391c1fc9
> >> --- /dev/null
> >> +++ b/board/renesas/rzg2l/MAINTAINERS
> >> @@ -0,0 +1,6 @@
> >> +RENESAS RZG2L BOARD FAMILY
> >> +M:Paul Barker 
> >> +S:Supported
> >> +F:arch/arm/dts/rz-smarc-common.dtsi
> >
> > I suspect there should be more files here, right ?
> > You likely want to be CCed on things like rzg2l clock, scif, and so on.
> >
> >> +N:rzg2l
> >> +N:r9a07g044
> >> diff --git a/board/renesas/rzg2l/Makefile
> >> b/board/renesas/rzg2l/Makefile new file mode 100644 index
> >> ..466935fc8158
> >> --- /dev/null
> >> +++ b/board/renesas/rzg2l/Makefile
> >> @@ -0,0 +1,4 @@
> >> +# Copyright (C) 2023 Renesas Electronics Corporation #
> >> +SPDX-License-Identifier: GPL-2.0+
> >> +
> >> +obj-y := rzg2l.o
> >
> >> diff --git a/board/renesas/rzg2l/rzg2l.c
> >> b/board/renesas/rzg2l/rzg2l.c new file mode 100644 index
> >> ..2b1bb3546c26
> >> --- /dev/null
> >> +++ b/board/renesas/rzg2l/rzg2l.c
> >> @@ -0,0 +1,76 @@
> >> +// SPDX-License-Identifier: GPL-2.0+
> >> +/*
> >> + * RZ/G2L board support.
> >> + * Copyright (C) 2023 Renesas Electronics Corporation  */
> >> +
> >> +#include 
> >> +#include 
> >> +#include 
> >> +
> >> +#if IS_ENABLED(CONFIG_MULTI_DTB_FIT)
> >> +/* If the firmware passed a device tree, use it for board
> >> +identification. */ extern u64 rcar_atf_boot_args[];
> >> +
> >> +static bool is_rzg2l_board(const char *board_name) {
> >> +  void *atf_fdt_blob = (void *)(rcar_atf_boot_args[1]);
> >> +
> >> +  return fdt_node_check_compatible(atf_fdt_blob, 

Re: [PATCH v2 1/3] dt-bindings: mtd: fixed-partitions: Add binman compatible

2023-10-06 Thread Simon Glass
Hi Michael,

On Fri, 6 Oct 2023 at 02:37, Michael Walle  wrote:
>
> Hi,
>
> >> I'm still not sure why that compatible is needed. Also I'd need to
> >> change
> >> the label which might break user space apps looking for that specific
> >> name.
> >>
> >> Also, our board might have u-boot/spl or u-boot/spl/bl31/bl32, right
> >> now
> >> that's something which depends on an u-boot configuration variable,
> >> which
> >> then enables or disables binman nodes in the -u-boot.dtsi. So in linux
> >> we only have that "bootloader" partition, but there might be either
> >> u-boot+spl or u-boot+spl+bl31+bl32.
> >>
> >> Honestly, I'm really not sure this should go into a device tree.
> >
> > I think we might be getting a bit ahead of ourselves here. I thought
> > that the decision was that the label should indicate the contents.
> > If you have multiple things in a partition then it would become a
> > 'section' in Binman's terminology. Either the label programmatically
> > describes what is inside or it doesn't. We can't have it both ways.
> > What do you suggest?
>
> As Rob pointed out earlier, it's just a user-facing string. I'm a bit
> reluctant to use it programatically.
> Taking my example again, the string "bootloader" is sufficient for a
> user. He doesn't care if it's u-boot with spl or u-boot with tfa, or
> even coreboot. It just says, "in this partition is the bootloader".
> If you have an "bootloader" image you can flash it there.
>
> If it has a label "u-boot" and I want to switch to coreboot, will
> it have to change to "coreboot"? I really don't think this is practical,
> you are really putting software configuration into the device tree.

I thought Rob changed his mind on that?

I agree that compatible would make things easier. We could then
continue to use 'label' for whatever it currently has.

Note that some kernel drivers or user space will want to look at what
is there, e.g. to provide a way to extract, check or update a
particular part of the firmware.

>
> > At present it seems you have the image described in two places - one
> > is the binman node and the other is the partitions node. I would like
> > to unify these.
>
> And I'm not sure that will work for all the corner cases :/
>
> If you keep the binman section seperate from the flash partition
> definition you don't have any of these problems, although there is
> some redundancy:
>   - you only have compatible = "binman", "fixed-partition", no further
> compatibles are required
>   - you don't have any conflicts with the current partition descriptions
>   - you could even use the labels, because binman is the (only?) user
>
> But of course you need to find a place where to put your node.

Sure, but I was pointed to partitions as the place where this should live.

>
> > What does user space do with the partition labels?
>
> I'm not sure. Also I'm not sure if it really matters, I just wanted to
> point out, that you'll force users to change it.

OK I'll send a version that uses compatible strings and see if that
makes any progress.

Regards,
Simon

>
> -michael
>
> >> >> What if a board uses eMMC to store the firmware binaries? Will that
> >> >> then
> >> >> be a subnode to the eMMC device?
> >> >
> >> > I thought there was a way to link the partition nodes and the device
> >> > using a property, without having the partition info as a subnode of
> >> > the device. But I may have imagined it as I cannot find it now. So
> >> > yes, it will be a subnode of the eMMC device.
> >>
> >> Not sure if that will fly.
> >
> > I can't find it anyway. There is somelike like that in
> > simple-framebuffer with the 'display' property.
> >
> > Regards,
> > SImon


[PATCH] xilinx: Enable DNS/WGET and BLKMAP for http boot

2023-10-06 Thread Michal Simek
Enable DNS/WGET and BLKMAP to be able to download images over HTTP and map
them.

Signed-off-by: Michal Simek 
---

Can be applied on v5 version of this when this is fixed
https://lore.kernel.org/all/cc0cff36-359a-4c3b-9ba6-bce083435...@amd.com/
---
 configs/xilinx_versal_net_virt_defconfig | 3 +++
 configs/xilinx_versal_virt_defconfig | 3 +++
 configs/xilinx_zynqmp_virt_defconfig | 4 
 3 files changed, 10 insertions(+)

diff --git a/configs/xilinx_versal_net_virt_defconfig 
b/configs/xilinx_versal_net_virt_defconfig
index 62322af458d6..c190bf532705 100644
--- a/configs/xilinx_versal_net_virt_defconfig
+++ b/configs/xilinx_versal_net_virt_defconfig
@@ -46,6 +46,8 @@ CONFIG_CMD_USB=y
 CONFIG_BOOTP_MAY_FAIL=y
 CONFIG_BOOTP_BOOTFILESIZE=y
 CONFIG_CMD_TFTPPUT=y
+CONFIG_CMD_WGET=y
+CONFIG_CMD_DNS=y
 CONFIG_CMD_CACHE=y
 CONFIG_CMD_EFIDEBUG=y
 CONFIG_CMD_TIME=y
@@ -69,6 +71,7 @@ CONFIG_NETCONSOLE=y
 CONFIG_IP_DEFRAG=y
 CONFIG_SYS_FAULT_ECHO_LINK_DOWN=y
 CONFIG_TFTP_BLOCKSIZE=4096
+CONFIG_BLKMAP=y
 CONFIG_CLK_VERSAL=y
 CONFIG_DFU_RAM=y
 CONFIG_ZYNQ_GPIO=y
diff --git a/configs/xilinx_versal_virt_defconfig 
b/configs/xilinx_versal_virt_defconfig
index 89566c7d52c0..5ee694a4ff73 100644
--- a/configs/xilinx_versal_virt_defconfig
+++ b/configs/xilinx_versal_virt_defconfig
@@ -47,6 +47,8 @@ CONFIG_CMD_USB=y
 CONFIG_BOOTP_MAY_FAIL=y
 CONFIG_BOOTP_BOOTFILESIZE=y
 CONFIG_CMD_TFTPPUT=y
+CONFIG_CMD_WGET=y
+CONFIG_CMD_DNS=y
 CONFIG_CMD_CACHE=y
 CONFIG_CMD_EFIDEBUG=y
 CONFIG_CMD_TIME=y
@@ -71,6 +73,7 @@ CONFIG_NETCONSOLE=y
 CONFIG_IP_DEFRAG=y
 CONFIG_SYS_FAULT_ECHO_LINK_DOWN=y
 CONFIG_TFTP_BLOCKSIZE=4096
+CONFIG_BLKMAP=y
 CONFIG_CLK_VERSAL=y
 CONFIG_DFU_TIMEOUT=y
 CONFIG_DFU_RAM=y
diff --git a/configs/xilinx_zynqmp_virt_defconfig 
b/configs/xilinx_zynqmp_virt_defconfig
index 30e420951dad..d1bdea09ebab 100644
--- a/configs/xilinx_zynqmp_virt_defconfig
+++ b/configs/xilinx_zynqmp_virt_defconfig
@@ -82,6 +82,8 @@ CONFIG_CMD_USB_MASS_STORAGE=y
 CONFIG_BOOTP_MAY_FAIL=y
 CONFIG_BOOTP_BOOTFILESIZE=y
 CONFIG_CMD_TFTPPUT=y
+CONFIG_CMD_WGET=y
+CONFIG_CMD_DNS=y
 CONFIG_CMD_BMP=y
 CONFIG_CMD_CACHE=y
 CONFIG_CMD_EFIDEBUG=y
@@ -119,6 +121,8 @@ CONFIG_SPL_DM_SEQ_ALIAS=y
 CONFIG_SATA=y
 CONFIG_SCSI_AHCI=y
 CONFIG_SATA_CEVA=y
+# CONFIG_SPL_BLK is not set
+CONFIG_BLKMAP=y
 CONFIG_BUTTON=y
 CONFIG_BUTTON_GPIO=y
 CONFIG_CLK_ZYNQMP=y
-- 
2.36.1



Re: [PATCH 1/1] riscv: remove dram_init_banksize()

2023-10-06 Thread Anup Patel
On Tue, Sep 26, 2023 at 12:47 PM Heinrich Schuchardt
 wrote:
>
> Remove dram_init_banksize() on the architecture level.
>
> Limiting used RAM to under 4 GiB is only necessary for CPUs which have a
> DMA issue. SoC specific code already exists for FU540, FU740, JH7110.
>
> Not all RISC-V boards will have memory below 4 GiB.
>
> A weak implementation of dram_init_banksize() exists in common/board_f.c.
>
> See the discussion in
> https://lore.kernel.org/u-boot/545fe813-cb1e-469c-a131-0025c77ae...@canonical.com/T/
>
> Signed-off-by: Heinrich Schuchardt 

Looks good to me.

Reviewed-by: Anup Patel 

Regards,
Anup

> ---
>  arch/riscv/cpu/generic/dram.c | 16 
>  1 file changed, 16 deletions(-)
>
> diff --git a/arch/riscv/cpu/generic/dram.c b/arch/riscv/cpu/generic/dram.c
> index 94d8018407..1b51bae9b6 100644
> --- a/arch/riscv/cpu/generic/dram.c
> +++ b/arch/riscv/cpu/generic/dram.c
> @@ -20,19 +20,3 @@ int dram_init_banksize(void)
>  {
> return fdtdec_setup_memory_banksize();
>  }
> -
> -phys_addr_t board_get_usable_ram_top(phys_size_t total_size)
> -{
> -   /*
> -* Ensure that we run from first 4GB so that all
> -* addresses used by U-Boot are 32bit addresses.
> -*
> -* This in-turn ensures that 32bit DMA capable
> -* devices work fine because DMA mapping APIs will
> -* provide 32bit DMA addresses only.
> -*/
> -   if (gd->ram_top >= SZ_4G)
> -   return SZ_4G - 1;
> -
> -   return gd->ram_top;
> -}
> --
> 2.40.1
>


[PATCH RESEND 7/7] riscv: spl: andes: Move the DTB in front of kernel

2023-10-06 Thread Randolph
Originally, u-boot SPL will place the DTB directly after the kernel,
but the size of the kernel does not include the BSS section, This
means that u-boot SPL places the DTB in the kernel BSS section causing
the DTB to be cleared by the kernel BSS initialisation.

Moving the DTB in front of the kernel can avoid this error.

Signed-off-by: Randolph 
---
 board/AndesTech/ae350/ae350.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/board/AndesTech/ae350/ae350.c b/board/AndesTech/ae350/ae350.c
index 1c2288b6ce..d78ee403e6 100644
--- a/board/AndesTech/ae350/ae350.c
+++ b/board/AndesTech/ae350/ae350.c
@@ -19,6 +19,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -26,6 +28,29 @@ DECLARE_GLOBAL_DATA_PTR;
  * Miscellaneous platform dependent initializations
  */
 
+#if CONFIG_IS_ENABLED(LOAD_FIT) || CONFIG_IS_ENABLED(LOAD_FIT_FULL)
+#define ANDES_SPL_FDT_ADDR (CONFIG_TEXT_BASE - 0x10)
+void spl_perform_fixups(struct spl_image_info *spl_image)
+{
+   /*
+* Originally, u-boot-spl will place DTB directly after the kernel,
+* but the size of the kernel did not include the BSS section, which
+* means u-boot-spl will place the DTB in the kernel BSS section
+* causing the DTB to be cleared by kernel BSS initializtion.
+* Moving DTB in front of the kernel can avoid the error.
+*/
+   if (ANDES_SPL_FDT_ADDR < 0) {
+   printf("%s: CONFIG_TEXT_BASE needs to be larger than 
0x10\n",
+  __func__);
+   hang();
+   }
+
+   memcpy((void *)ANDES_SPL_FDT_ADDR, spl_image->fdt_addr,
+  fdt_totalsize(spl_image->fdt_addr));
+   spl_image->fdt_addr = map_sysmem(ANDES_SPL_FDT_ADDR, 0);
+}
+#endif
+
 int board_init(void)
 {
gd->bd->bi_boot_params = PHYS_SDRAM_0 + 0x400;
-- 
2.34.1



[PATCH RESEND 6/7] andes: config: add riscv falcon mode for ae350 platform

2023-10-06 Thread Randolph
Fork from ae350_rv[32/64]_spl_[xip]_defconfig and
append CONFIG_SPL_LOAD_FIT_OPENSBI_OS_BOOT=y

Signed-off-by: Randolph 
---
 configs/ae350_rv32_falcon_defconfig | 60 
 configs/ae350_rv32_falcon_xip_defconfig | 61 +
 configs/ae350_rv64_falcon_defconfig | 60 
 configs/ae350_rv64_falcon_xip_defconfig | 61 +
 4 files changed, 242 insertions(+)
 create mode 100644 configs/ae350_rv32_falcon_defconfig
 create mode 100644 configs/ae350_rv32_falcon_xip_defconfig
 create mode 100644 configs/ae350_rv64_falcon_defconfig
 create mode 100644 configs/ae350_rv64_falcon_xip_defconfig

diff --git a/configs/ae350_rv32_falcon_defconfig 
b/configs/ae350_rv32_falcon_defconfig
new file mode 100644
index 00..8f796d88e3
--- /dev/null
+++ b/configs/ae350_rv32_falcon_defconfig
@@ -0,0 +1,60 @@
+CONFIG_RISCV=y
+CONFIG_TEXT_BASE=0x0180
+CONFIG_SYS_MALLOC_LEN=0x8
+CONFIG_NR_DRAM_BANKS=2
+CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x1000
+CONFIG_ENV_SECT_SIZE=0x1000
+CONFIG_DEFAULT_DEVICE_TREE="ae350_32"
+CONFIG_SYS_PROMPT="RISC-V # "
+CONFIG_SYS_MONITOR_LEN=786432
+CONFIG_SPL_SYS_MALLOC_F_LEN=0x200
+CONFIG_SPL=y
+CONFIG_SYS_LOAD_ADDR=0x10
+CONFIG_TARGET_ANDES_AE350=y
+CONFIG_RISCV_SMODE=y
+# CONFIG_AVAILABLE_HARTS is not set
+CONFIG_FIT=y
+CONFIG_SPL_LOAD_FIT_ADDRESS=0x1000
+CONFIG_SYS_MONITOR_BASE=0x8800
+CONFIG_DISTRO_DEFAULTS=y
+CONFIG_BOOTDELAY=3
+CONFIG_DISPLAY_CPUINFO=y
+CONFIG_DISPLAY_BOARDINFO=y
+CONFIG_BOARD_EARLY_INIT_F=y
+CONFIG_SPL_MAX_SIZE=0x10
+CONFIG_SPL_BSS_START_ADDR=0x40
+CONFIG_SPL_BOARD_INIT=y
+CONFIG_SPL_CACHE=y
+CONFIG_SPL_OPENSBI_SCRATCH_OPTIONS=0x0
+CONFIG_SYS_PBSIZE=1050
+CONFIG_SYS_BOOTM_LEN=0x400
+CONFIG_CMD_IMLS=y
+CONFIG_CMD_MMC=y
+CONFIG_CMD_SF_TEST=y
+# CONFIG_CMD_SETEXPR is not set
+CONFIG_BOOTP_PREFER_SERVERIP=y
+CONFIG_CMD_CACHE=y
+CONFIG_ENV_OVERWRITE=y
+CONFIG_ENV_IS_IN_SPI_FLASH=y
+CONFIG_NET_RETRY_COUNT=50
+CONFIG_BOOTP_SEND_HOSTNAME=y
+CONFIG_NET_RANDOM_ETHADDR=y
+CONFIG_MMC=y
+CONFIG_FTSDC010=y
+CONFIG_FTSDC010_SDIO=y
+CONFIG_MTD_NOR_FLASH=y
+CONFIG_FLASH_CFI_DRIVER=y
+CONFIG_SYS_FLASH_CFI_WIDTH_16BIT=y
+CONFIG_FLASH_SHOW_PROGRESS=0
+CONFIG_SYS_CFI_FLASH_STATUS_POLL=y
+CONFIG_SYS_FLASH_USE_BUFFER_WRITE=y
+CONFIG_SYS_FLASH_CFI=y
+CONFIG_SPI_FLASH_MACRONIX=y
+CONFIG_FTMAC100=y
+CONFIG_BAUDRATE=38400
+CONFIG_SYS_NS16550=y
+CONFIG_SPI=y
+CONFIG_ATCSPI200_SPI=y
+# CONFIG_BINMAN_FDT is not set
+CONFIG_SPL_LOAD_FIT_OPENSBI_OS_BOOT=y
\ No newline at end of file
diff --git a/configs/ae350_rv32_falcon_xip_defconfig 
b/configs/ae350_rv32_falcon_xip_defconfig
new file mode 100644
index 00..e01dd6fc51
--- /dev/null
+++ b/configs/ae350_rv32_falcon_xip_defconfig
@@ -0,0 +1,61 @@
+CONFIG_RISCV=y
+CONFIG_TEXT_BASE=0x0180
+CONFIG_SYS_MALLOC_LEN=0x8
+CONFIG_NR_DRAM_BANKS=2
+CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x1000
+CONFIG_ENV_SECT_SIZE=0x1000
+CONFIG_DEFAULT_DEVICE_TREE="ae350_32"
+CONFIG_SPL_TEXT_BASE=0x8000
+CONFIG_SYS_PROMPT="RISC-V # "
+CONFIG_SYS_MONITOR_LEN=786432
+CONFIG_SPL_SYS_MALLOC_F_LEN=0x200
+CONFIG_SPL=y
+CONFIG_SYS_LOAD_ADDR=0x10
+CONFIG_TARGET_ANDES_AE350=y
+CONFIG_RISCV_SMODE=y
+CONFIG_SPL_XIP=y
+CONFIG_FIT=y
+CONFIG_SPL_LOAD_FIT_ADDRESS=0x8001
+CONFIG_SYS_MONITOR_BASE=0x8800
+CONFIG_DISTRO_DEFAULTS=y
+CONFIG_BOOTDELAY=3
+CONFIG_DISPLAY_CPUINFO=y
+CONFIG_DISPLAY_BOARDINFO=y
+CONFIG_BOARD_EARLY_INIT_F=y
+CONFIG_SPL_MAX_SIZE=0x10
+CONFIG_SPL_BSS_START_ADDR=0x40
+CONFIG_SPL_BOARD_INIT=y
+CONFIG_SPL_CACHE=y
+CONFIG_SPL_OPENSBI_SCRATCH_OPTIONS=0x0
+CONFIG_SYS_PBSIZE=1050
+CONFIG_SYS_BOOTM_LEN=0x400
+CONFIG_CMD_IMLS=y
+CONFIG_CMD_MMC=y
+CONFIG_CMD_SF_TEST=y
+# CONFIG_CMD_SETEXPR is not set
+CONFIG_BOOTP_PREFER_SERVERIP=y
+CONFIG_CMD_CACHE=y
+CONFIG_ENV_OVERWRITE=y
+CONFIG_ENV_IS_IN_SPI_FLASH=y
+CONFIG_NET_RETRY_COUNT=50
+CONFIG_BOOTP_SEND_HOSTNAME=y
+CONFIG_NET_RANDOM_ETHADDR=y
+CONFIG_MMC=y
+CONFIG_FTSDC010=y
+CONFIG_FTSDC010_SDIO=y
+CONFIG_MTD_NOR_FLASH=y
+CONFIG_FLASH_CFI_DRIVER=y
+CONFIG_SYS_FLASH_CFI_WIDTH_16BIT=y
+CONFIG_FLASH_SHOW_PROGRESS=0
+CONFIG_SYS_CFI_FLASH_STATUS_POLL=y
+CONFIG_SYS_FLASH_USE_BUFFER_WRITE=y
+CONFIG_SYS_FLASH_CFI=y
+CONFIG_SPI_FLASH_MACRONIX=y
+CONFIG_FTMAC100=y
+CONFIG_BAUDRATE=38400
+CONFIG_SYS_NS16550=y
+CONFIG_SPI=y
+CONFIG_ATCSPI200_SPI=y
+# CONFIG_BINMAN_FDT is not set
+CONFIG_SPL_LOAD_FIT_OPENSBI_OS_BOOT=y
\ No newline at end of file
diff --git a/configs/ae350_rv64_falcon_defconfig 
b/configs/ae350_rv64_falcon_defconfig
new file mode 100644
index 00..d11be976de
--- /dev/null
+++ b/configs/ae350_rv64_falcon_defconfig
@@ -0,0 +1,60 @@
+CONFIG_RISCV=y
+CONFIG_TEXT_BASE=0x0180
+CONFIG_SYS_MALLOC_LEN=0x8
+CONFIG_NR_DRAM_BANKS=2
+CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x1000
+CONFIG_ENV_SECT_SIZE=0x1000
+CONFIG_DEFAULT_DEVICE_TREE="ae350_64"

[PATCH RESEND 5/7] spl: riscv: add os type for next booting stage

2023-10-06 Thread Randolph
If SPL_LOAD_FIT_OPENSBI_OS_BOOT is enabled, the function
spl_invoke_opensbi should change the target OS type to IH_OS_LINUX.
OpenSBI will load the Linux image as the next boot stage.
The os_takes_devicetree function returns a value of true or false
depending on whether or not SPL_LOAD_FIT_OPENSBI_OS_BOOT is enabled.

Signed-off-by: Randolph 
---
 common/spl/spl_fit.c | 4 
 common/spl/spl_opensbi.c | 7 ++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 730639f756..750562721a 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -351,7 +351,11 @@ static bool os_takes_devicetree(uint8_t os)
case IH_OS_U_BOOT:
return true;
case IH_OS_LINUX:
+#ifdef CONFIG_RISCV
+   return IS_ENABLED(CONFIG_SPL_LOAD_FIT_OPENSBI_OS_BOOT);
+#else
return IS_ENABLED(CONFIG_SPL_OS_BOOT);
+#endif
default:
return false;
}
diff --git a/common/spl/spl_opensbi.c b/common/spl/spl_opensbi.c
index a0c5f35dab..708869ad48 100644
--- a/common/spl/spl_opensbi.c
+++ b/common/spl/spl_opensbi.c
@@ -58,9 +58,14 @@ void __noreturn spl_invoke_opensbi(struct spl_image_info 
*spl_image)
 
/*
 * Find next os image in /fit-images
-* The next os image default is u-boot proper
+* The next os image default is u-boot proper, once enable
+* OpenSBI OS boot mode, the OS image should be linux.
 */
+#if CONFIG_IS_ENABLED(LOAD_FIT_OPENSBI_OS_BOOT)
+   os_type = IH_OS_LINUX;
+#else
os_type = IH_OS_U_BOOT;
+#endif
ret = spl_opensbi_find_os_node(spl_image->fdt_addr, _node, os_type);
if (ret) {
pr_err("Can't find %s node for opensbi, %d\n",
-- 
2.34.1



[PATCH RESEND 4/7] riscv: dts: introduce SPL_LOAD_FIT_OPENSBI_OS_BOOT symbol

2023-10-06 Thread Randolph
Introduce common Kconfig symbol for riscv architecture.
This symbol SPL_LOAD_FIT_OPENSBI_OS_BOOT is like falcon mode on ARM,
the Falcon boot is a shortcut boot method for SD/eMMC targets. It
skips the loading the RAM version U-Boot. Instead, it will loads
the FIT image and boots directly to Linux.

When SPL_OPENSBI_OS_BOOT is enabled, linux.itb is created after
compilation instead of the default u-boot.itb. It initialises memory
with the U-Boot SPL at the first stage, just as a normal boot process
does at the beginning. Instead of jumping to the U-Boot proper from
OpenSBI before booting the Linux kernel, the RISC-V falcon mode
process jumps directly to the Linux kernel to gain shorter booting time.

When SPL_OPENSBI_OS_BOOT is enabled, it will change the default FIT
configure file "binman.dtsi" to "binman_linux.dtsi"
Default is not enabled.

Signed-off-by: Randolph 
---
 arch/riscv/Kconfig | 9 +
 1 file changed, 9 insertions(+)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index ec1cfcaaa7..4f104789a7 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -422,8 +422,17 @@ config TPL_USE_ARCH_MEMSET
 
 endmenu
 
+config SPL_LOAD_FIT_OPENSBI_OS_BOOT
+   bool "Enable SPL (OpenSBI OS boot mode) applying linux from FIT"
+   depends on SPL_LOAD_FIT
+   help
+ Use fw_dynamic from the FIT image, and u-boot SPL will invoke it 
directly.
+ This is a shortcut boot flow, from u-boot SPL -> OpenSBI -> u-boot 
proper
+ -> linux to u-boot SPL -> OpenSBI -> linux.
+
 config SPL_LOAD_FIT_CONFIG
string "Default FIT configuration for SPL"
+   default "binman_linux.dtsi" if SPL_LOAD_FIT_OPENSBI_OS_BOOT
default "binman.dtsi"
depends on SPL_LOAD_FIT
help
-- 
2.34.1



[PATCH RESEND 3/7] spl: riscv: opensbi: change the default os_type as varible

2023-10-06 Thread Randolph
In order to introduce the Opensbi OS boot mode, the next stage boot
image of OpenSBI should be configurable.

Signed-off-by: Randolph 
---
 common/spl/spl_opensbi.c | 26 --
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/common/spl/spl_opensbi.c b/common/spl/spl_opensbi.c
index e2aaa46046..a0c5f35dab 100644
--- a/common/spl/spl_opensbi.c
+++ b/common/spl/spl_opensbi.c
@@ -20,7 +20,7 @@ DECLARE_GLOBAL_DATA_PTR;
 
 struct fw_dynamic_info opensbi_info;
 
-static int spl_opensbi_find_uboot_node(void *blob, int *uboot_node)
+static int spl_opensbi_find_os_node(void *blob, int *uboot_node, int os_type)
 {
int fit_images_node, node;
const char *fit_os;
@@ -34,7 +34,7 @@ static int spl_opensbi_find_uboot_node(void *blob, int 
*uboot_node)
if (!fit_os)
continue;
 
-   if (genimg_get_os_id(fit_os) == IH_OS_U_BOOT) {
+   if (genimg_get_os_id(fit_os) == os_type) {
*uboot_node = node;
return 0;
}
@@ -45,8 +45,9 @@ static int spl_opensbi_find_uboot_node(void *blob, int 
*uboot_node)
 
 void __noreturn spl_invoke_opensbi(struct spl_image_info *spl_image)
 {
-   int ret, uboot_node;
-   ulong uboot_entry;
+   int ret, os_node;
+   ulong os_entry;
+   int os_type;
typedef void __noreturn (*opensbi_entry_t)(ulong hartid, ulong dtb, 
ulong info);
opensbi_entry_t opensbi_entry;
 
@@ -55,22 +56,27 @@ void __noreturn spl_invoke_opensbi(struct spl_image_info 
*spl_image)
hang();
}
 
-   /* Find U-Boot image in /fit-images */
-   ret = spl_opensbi_find_uboot_node(spl_image->fdt_addr, _node);
+   /*
+* Find next os image in /fit-images
+* The next os image default is u-boot proper
+*/
+   os_type = IH_OS_U_BOOT;
+   ret = spl_opensbi_find_os_node(spl_image->fdt_addr, _node, os_type);
if (ret) {
-   pr_err("Can't find U-Boot node, %d\n", ret);
+   pr_err("Can't find %s node for opensbi, %d\n",
+  genimg_get_os_name(os_type), ret);
hang();
}
 
/* Get U-Boot entry point */
-   ret = fit_image_get_entry(spl_image->fdt_addr, uboot_node, 
_entry);
+   ret = fit_image_get_entry(spl_image->fdt_addr, os_node, _entry);
if (ret)
-   ret = fit_image_get_load(spl_image->fdt_addr, uboot_node, 
_entry);
+   ret = fit_image_get_load(spl_image->fdt_addr, os_node, 
_entry);
 
/* Prepare opensbi_info object */
opensbi_info.magic = FW_DYNAMIC_INFO_MAGIC_VALUE;
opensbi_info.version = FW_DYNAMIC_INFO_VERSION;
-   opensbi_info.next_addr = uboot_entry;
+   opensbi_info.next_addr = os_entry;
opensbi_info.next_mode = FW_DYNAMIC_INFO_NEXT_MODE_S;
opensbi_info.options = CONFIG_SPL_OPENSBI_SCRATCH_OPTIONS;
opensbi_info.boot_hart = gd->arch.boot_hart;
-- 
2.34.1



[PATCH RESEND 2/7] riscv: dts: add binman_linux.dtsi for opensbi os boot mode

2023-10-06 Thread Randolph
The binman_linux.dtsi is a fork of binman.dtsi, just change the first
section image from the "u-boot" to "linux". Note that the filename
is also changed. In binman.dtsi, the filename of u-boot section filename
is called "u-boot-nodtb.bin". In binman_linux.dtsi, the filename should
be called "Image", which is located in linux/arch/riscv/boot.

Signed-off-by: Randolph 
---
 arch/riscv/dts/binman_linux.dtsi | 79 
 1 file changed, 79 insertions(+)
 create mode 100644 arch/riscv/dts/binman_linux.dtsi

diff --git a/arch/riscv/dts/binman_linux.dtsi b/arch/riscv/dts/binman_linux.dtsi
new file mode 100644
index 00..334d64bc40
--- /dev/null
+++ b/arch/riscv/dts/binman_linux.dtsi
@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2021, Bin Meng 
+ */
+
+#include 
+
+/ {
+   binman: binman {
+   multiple-images;
+   };
+};
+
+ {
+   itb {
+   filename = "linux.itb";
+
+   fit {
+   description = "Configuration to load OpenSBI before 
Linux";
+   #address-cells = <1>;
+   fit,fdt-list = "of-list";
+
+   images {
+   linux {
+   description = "Linux";
+   type = "standalone";
+   os = "Linux";
+   arch = "riscv";
+   compression = "none";
+   load = ;
+
+   linux_blob: blob-ext {
+   filename = "Image";
+   };
+   };
+
+   opensbi {
+   description = "OpenSBI fw_dynamic 
Firmware";
+   type = "firmware";
+   os = "opensbi";
+   arch = "riscv";
+   compression = "none";
+   load = ;
+   entry = ;
+
+   opensbi_blob: opensbi {
+   filename = "fw_dynamic.bin";
+   missing-msg = "opensbi";
+   };
+   };
+
+#ifndef CONFIG_OF_BOARD
+   @fdt-SEQ {
+   description = "NAME";
+   type = "flat_dt";
+   compression = "none";
+   };
+#endif
+   };
+
+   configurations {
+   default = "conf-1";
+
+#ifndef CONFIG_OF_BOARD
+   @conf-SEQ {
+#else
+   conf-1 {
+#endif
+   description = "NAME";
+   firmware = "opensbi";
+   loadables = "linux";
+#ifndef CONFIG_OF_BOARD
+   fdt = "fdt-SEQ";
+#endif
+   };
+   };
+   };
+   };
+};
-- 
2.34.1



[PATCH RESEND 1/7] riscv: dts: Introduce SPL_LOAD_FIT_CONFIG symbol

2023-10-06 Thread Randolph
Introduce common Kconfig symbol for riscv architecture
This symbol SPL_LOAD_FIT_CONFIG for binman itb layout selection
Default is using binman.dtsi

Signed-off-by: Randolph 
---
 arch/riscv/Kconfig   | 7 +++
 arch/riscv/dts/ae350-u-boot.dtsi | 1 +
 arch/riscv/dts/ae350_32.dts  | 1 -
 arch/riscv/dts/ae350_64.dts  | 1 -
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index aff1f33665..ec1cfcaaa7 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -422,4 +422,11 @@ config TPL_USE_ARCH_MEMSET
 
 endmenu
 
+config SPL_LOAD_FIT_CONFIG
+   string "Default FIT configuration for SPL"
+   default "binman.dtsi"
+   depends on SPL_LOAD_FIT
+   help
+ Specify corresponding FIT configuration for SPL modes.
+
 endmenu
diff --git a/arch/riscv/dts/ae350-u-boot.dtsi b/arch/riscv/dts/ae350-u-boot.dtsi
index aef9159b7a..ff5725501f 100644
--- a/arch/riscv/dts/ae350-u-boot.dtsi
+++ b/arch/riscv/dts/ae350-u-boot.dtsi
@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: (GPL-2.0 OR MIT)
+#include CONFIG_SPL_LOAD_FIT_CONFIG
 
 / {
cpus {
diff --git a/arch/riscv/dts/ae350_32.dts b/arch/riscv/dts/ae350_32.dts
index 61af6d5465..2caabad888 100644
--- a/arch/riscv/dts/ae350_32.dts
+++ b/arch/riscv/dts/ae350_32.dts
@@ -2,7 +2,6 @@
 
 /dts-v1/;
 
-#include "binman.dtsi"
 #include "ae350-u-boot.dtsi"
 
 / {
diff --git a/arch/riscv/dts/ae350_64.dts b/arch/riscv/dts/ae350_64.dts
index 8c7db29b4f..9d5f6c743c 100644
--- a/arch/riscv/dts/ae350_64.dts
+++ b/arch/riscv/dts/ae350_64.dts
@@ -2,7 +2,6 @@
 
 /dts-v1/;
 
-#include "binman.dtsi"
 #include "ae350-u-boot.dtsi"
 
 / {
-- 
2.34.1



[PATCH RESEND 0/7] riscv: spl: OpenSBI OS boot mode

2023-10-06 Thread Randolph
Introduce a shortcut boot mode for RISC-V. 

As we know, in ARM architecture has the Falcon mode to do the shortcut
boot to the Linux kernel. (by enabling CONFIG_SPL_OS_BOOT)
ARM Falcon mode boot flow would be as follows:
u-boot SPL -> Linux kernel

But for RISC-V, OpenSBI is required to allows the supervisor to execute
some privileged operations.
The RISC-V Normal boot flow as follows:
u-boot SPL -> OpenSBI -> u-boot proper -> Linux kernel

Quoting the same ideas as ARM's Falcon mode,
OpenSBI OS boot flow as follows:
u-boot SPL -> OpenSBI -> Linux kernel

An important part of OpenSBI OS boot mode is to prepare the device tree. 
A normal U-Boot does FDT fixups when booting Linux. 
For OpenSBI OS boot mode, Linux boots directly from SPL, 
skipping the normal U-Boot. 
The device tree has to be prepared in advance.
The device tree in memory is the one needed for OpenSBI OS boot mode.

The Linux kernel image will also need to be provided for the generation
of the FIT file.

Randolph (7):
  riscv: dts: Introduce SPL_LOAD_FIT_CONFIG symbol
  riscv: dts: add binman_linux.dtsi for opensbi os boot mode
  spl: riscv: opensbi: change the default os_type as varible
  riscv: dts: introduce SPL_LOAD_FIT_OPENSBI_OS_BOOT symbol
  spl: riscv: add os type for next booting stage
  andes: config: add riscv falcon mode for ae350 platform
  riscv: spl: andes: Move the DTB in front of kernel

 arch/riscv/Kconfig  | 16 +
 arch/riscv/dts/ae350-u-boot.dtsi|  1 +
 arch/riscv/dts/ae350_32.dts |  1 -
 arch/riscv/dts/ae350_64.dts |  1 -
 arch/riscv/dts/binman_linux.dtsi| 79 +
 board/AndesTech/ae350/ae350.c   | 25 
 common/spl/spl_fit.c|  4 ++
 common/spl/spl_opensbi.c| 31 ++
 configs/ae350_rv32_falcon_defconfig | 60 +++
 configs/ae350_rv32_falcon_xip_defconfig | 61 +++
 configs/ae350_rv64_falcon_defconfig | 60 +++
 configs/ae350_rv64_falcon_xip_defconfig | 61 +++
 12 files changed, 388 insertions(+), 12 deletions(-)
 create mode 100644 arch/riscv/dts/binman_linux.dtsi
 create mode 100644 configs/ae350_rv32_falcon_defconfig
 create mode 100644 configs/ae350_rv32_falcon_xip_defconfig
 create mode 100644 configs/ae350_rv64_falcon_defconfig
 create mode 100644 configs/ae350_rv64_falcon_xip_defconfig

-- 
2.34.1



Re: [PATCH v5 3/7] blk: blkmap: add ramdisk creation utility function

2023-10-06 Thread Michal Simek




On 9/27/23 11:36, Masahisa Kojima wrote:

User needs to call several functions to create the ramdisk
with blkmap.
This adds the utility function to create blkmap device and
mount the ramdisk.

Signed-off-by: Masahisa Kojima 
Reviewed-by: Simon Glass 
Reviewed-by: Ilias Apalodimas 
---
  drivers/block/Makefile|  1 +
  drivers/block/blkmap.c| 15 --
  drivers/block/blkmap_helper.c | 53 +++
  include/blkmap.h  | 29 +++
  4 files changed, 83 insertions(+), 15 deletions(-)
  create mode 100644 drivers/block/blkmap_helper.c

diff --git a/drivers/block/Makefile b/drivers/block/Makefile
index a161d145fd..c3ccfc03e5 100644
--- a/drivers/block/Makefile
+++ b/drivers/block/Makefile
@@ -15,6 +15,7 @@ endif
  obj-$(CONFIG_SANDBOX) += sandbox.o host-uclass.o host_dev.o
  obj-$(CONFIG_$(SPL_TPL_)BLOCK_CACHE) += blkcache.o
  obj-$(CONFIG_BLKMAP) += blkmap.o
+obj-$(CONFIG_BLKMAP) += blkmap_helper.o


you should actually build them only when it is enabled also for SPL/TPL

It means use style like this.

obj-$(CONFIG_$(SPL_TPL_)BLKMAP) += blkmap.o
obj-$(CONFIG_$(SPL_TPL_)BLKMAP) += blkmap_helper.o

Thanks,
Michal


Re: [PATCH v7 2/7] Revert "arm: dts: k3-j7*: ddr: Update to 0.10 version of DDR config tool"

2023-10-06 Thread Nishanth Menon
On 10:15-20231006, Manorit Chawdhry wrote:
> The update causes instability in am68-sk boards so revert the patch in
> the meantime till fix is available.
> 
> This reverts commit f1edf4bb6aa19732574ac23ca90cb9a0ba395ec1.
> 
> Signed-off-by: Manorit Chawdhry 
> ---
>  arch/arm/dts/k3-j721e-ddr-evm-lp4-4266.dtsi  |  98 +++---
>  arch/arm/dts/k3-j721s2-ddr-evm-lp4-4266.dtsi | 464 
> +--
>  2 files changed, 281 insertions(+), 281 deletions(-)
> 
> diff --git a/arch/arm/dts/k3-j721e-ddr-evm-lp4-4266.dtsi 
> b/arch/arm/dts/k3-j721e-ddr-evm-lp4-4266.dtsi
> index a0285ce0520e..5a6f9b11b8e3 100644
> --- a/arch/arm/dts/k3-j721e-ddr-evm-lp4-4266.dtsi
> +++ b/arch/arm/dts/k3-j721e-ddr-evm-lp4-4266.dtsi

Sigh. Hope to see a fix some time.

Reviewed-by: Nishanth Menon 
-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 
849D 1736 249D


Re: [PATCH v4 02/16] arm: mach-k3: Add basic support for J784S4 SoC definition

2023-10-06 Thread Nishanth Menon
On 09:46-20231006, Manorit Chawdhry wrote:
> Hi Nishanth,
> 
> On 06:26-20231005, Nishanth Menon wrote:
> > On 10:29-20231005, Manorit Chawdhry wrote:
> > > Hi Nishanth,
> > > 
> > > On 07:24-20231004, Nishanth Menon wrote:
> > > > On 10:43-20231004, Manorit Chawdhry wrote:
> > > > 
> > > > > These are required to remove the firewall configurations that are done
> > > > > by ROM, those are not the ones that are being handled by OIDs. The
> > > > 
> > > > I am not sure I understand this clearly. OIDs are setup to open up
> > > > firewalls or close firewalls as the system requires and since it
> > > > is authenticated, not compromiseable.- U-boot by itself (even if
> > > > authenticated), is not a secure entity for it to dictate the firewall
> > > > configuration (u-boot must be assumed to be compromised after
> > > > authentication is complete). So, doing firewall configuration via APIs
> > > > after boot, to me looks broken approach.
> > > > 
> > > 
> > > I know U-boot ain't that secure given the most trusted entity is always
> > > gonna be the software that starts up the system, we can't expect those
> > > to be doing all the work and based on that we have the secure boot
> > > designed to configure firewalls (that are not owned by anymore) and
> > > U-boot R5 being one of the early bootloaders do come as a part of it. 
> > > 
> > > Regarding the OIDs thing, I don't think the OID in question is looked by
> > > ROM and ROM always configures some firewalls for it's usecase that are
> > > present in those arrays. 
> > > 
> > > The OID that we are using in the series that you had shared is looked by
> > > TIFS instead of ROM and TIFS is the entity that is authenticating the
> > > binary along with setting up the firewalls.
> > > 
> > > > > current series that is being worked on is to add additional 
> > > > > firewalling
> > > > > support with OIDs that TIFS will be handling.
> > > > > The above patch is
> > > > > essentially added to have the same development experience on GP 
> > > > > devices
> > > > > similar to HS after the secure boot is done so that people don't end 
> > > > > up
> > > > 
> > > > huh? the code seems to blindly call the 
> > > > remove_fwl_configs(cbass_hc_cfg0_fwls, ARRAY_SIZE(cbass_hc_cfg0_fwls));
> > > > where is the distinction of HS vs GP here? This implementation looks
> > > > completely broken to me at least.. please correct what I missed here.
> > > 
> > > Since this call is used across all SoCs there wasn't any point to make
> > > the differentiation between GP and HS here, remove_fwl_configs
> > > internally handles looking at the firewalls and disabling them if they
> > > are enabled ( Which would be only in the case of HS devices ), for GP it
> > > would automatically by a noop.
> > 
> > Correct me if I understand the security chain here:
> > 
> > ROM sets up firewalls that are needed by itself
> > TIFS (in multicertificate will setup it's own firewalls)
> > R5 SPL comes along and opens up other firewalls
> > Each stage beyond this: such as tispl.bin containing TFA/OPTEE uses OIDs to
> > set up firewalls to protect themselves (enforced by TIFS)
> > A53 SPL and U-boot itself startups but has no ability to change the
> > protection firewalls enforced by x509 OIDs.
> > 
> > Further, firewalls have lockdown bit that enforces the setting (and
> > cannot be over-ridden) till system restart is requested
> > 
> > Is this correct? If so, needs to be clearly documented.
> 
> Yes, this seems correct. Will be updating it in the upstream
> documentation in another series.
> 

Thanks. I suggest:
a) Add documentation as part of firewall series[1] including information
as to how to customize the flow as needed.
b) once the firewall series is merged
c) am69/j784s4 series and add reference in code to the section of
documentation.

[1] 
https://lore.kernel.org/all/20231004-binman-firewalling-v3-0-e4a102324...@ti.com/
-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 
849D 1736 249D


[PATCH v3 2/2] DONOTMERGE: arm: dts: k3-j7200-binman: Enable split mode for MCU R5

2023-10-06 Thread Neha Malcom Francis
Set boot core-opts to enable split mode for MCU R5 cluster by default.
This patch serves to demonstrate how this can be done.

Signed-off-by: Neha Malcom Francis 
---
No change since v2

 arch/arm/dts/k3-j7200-binman.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/dts/k3-j7200-binman.dtsi 
b/arch/arm/dts/k3-j7200-binman.dtsi
index 14f7dea65e..025a0bd071 100644
--- a/arch/arm/dts/k3-j7200-binman.dtsi
+++ b/arch/arm/dts/k3-j7200-binman.dtsi
@@ -55,6 +55,7 @@
<_dm_cfg>, <_inner_cert>;
combined;
dm-data;
+   core-opts = <2>;
sysfw-inner-cert;
keyfile = "custMpk.pem";
sw-rev = <1>;
@@ -100,6 +101,7 @@
<_dm_cfg_fs>, <_inner_cert_fs>;
combined;
dm-data;
+   core-opts = <2>;
sysfw-inner-cert;
keyfile = "custMpk.pem";
sw-rev = <1>;
@@ -146,6 +148,7 @@
<_tifs_cfg_gp>, <_dm_cfg_gp>;
combined;
dm-data;
+   core-opts = <2>;
content-sbl = <_boot_spl_unsigned>;
load = <0x41c0>;
content-sysfw = <_fs_gp>;
-- 
2.34.1



[PATCH v3 1/2] binman: openssl: x509: ti_secure_rom: Add support for bootcore_opts

2023-10-06 Thread Neha Malcom Francis
According to the TRMs of K3 platform of devices, the ROM boot image
format specifies a "Core Options Field" that provides the capability to
set the boot core in lockstep when set to 0 or to split mode when set
to 2. Add support for providing the same from the binman DTS. Also
modify existing test case for ensuring future coverage.

Signed-off-by: Neha Malcom Francis 
---
Link to J721E TRM: https://www.ti.com/lit/zip/spruil1
Section 4.5.4.1 Boot Info

Changes in v3:
- updated function comments
- removed inconsistency in setting bootcore_opts to 32

Changes in v2:
- included TRM link in commit message

 tools/binman/btool/openssl.py   |  6 --
 tools/binman/entries.rst|  1 +
 tools/binman/etype/ti_secure_rom.py | 11 +--
 tools/binman/etype/x509_cert.py |  3 ++-
 tools/binman/test/297_ti_secure_rom.dts |  1 +
 5 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/tools/binman/btool/openssl.py b/tools/binman/btool/openssl.py
index aad3b61ae2..86cc56fbd7 100644
--- a/tools/binman/btool/openssl.py
+++ b/tools/binman/btool/openssl.py
@@ -155,6 +155,7 @@ authInPlace = INTEGER:2
 C, ST, L, O, OU, CN and emailAddress
 cert_type (int): Certification type
 bootcore (int): Booting core
+bootcore_opts(int): Booting core option (split/lockstep mode)
 load_addr (int): Load address of image
 sha (int): Hash function
 
@@ -225,7 +226,7 @@ emailAddress   = 
{req_dist_name_dict['emailAddress']}
   imagesize_sbl, hashval_sbl, load_addr_sysfw, imagesize_sysfw,
   hashval_sysfw, load_addr_sysfw_data, imagesize_sysfw_data,
   hashval_sysfw_data, sysfw_inner_cert_ext_boot_block,
-  dm_data_ext_boot_block):
+  dm_data_ext_boot_block, bootcore_opts):
 """Create a certificate
 
 Args:
@@ -241,6 +242,7 @@ emailAddress   = 
{req_dist_name_dict['emailAddress']}
 bootcore (int): Booting core
 load_addr (int): Load address of image
 sha (int): Hash function
+bootcore_opts (int): Boot core option (split/lockstep mode)
 
 Returns:
 str: Tool output
@@ -285,7 +287,7 @@ sysfw_data=SEQUENCE:sysfw_data
 [sbl]
 compType = INTEGER:1
 bootCore = INTEGER:16
-compOpts = INTEGER:0
+compOpts = INTEGER:{bootcore_opts}
 destAddr = FORMAT:HEX,OCT:{load_addr:08x}
 compSize = INTEGER:{imagesize_sbl}
 shaType  = OID:{sha_type}
diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
index 801bd94674..b401f9426a 100644
--- a/tools/binman/entries.rst
+++ b/tools/binman/entries.rst
@@ -1900,6 +1900,7 @@ Properties / Entry arguments:
 - core: core on which bootloader runs, valid cores are 'secure' and 
'public'
 - content: phandle of SPL in case of legacy bootflow or phandles of 
component binaries
   in case of combined bootflow
+- core-opts (optional): split-mode (0) or lockstep mode (1) set to 0 by 
default
 
 The following properties are only for generating a combined bootflow binary:
 - sysfw-inner-cert: boolean if binary contains sysfw inner certificate
diff --git a/tools/binman/etype/ti_secure_rom.py 
b/tools/binman/etype/ti_secure_rom.py
index 9a7ac9e9e0..17c50cefa1 100644
--- a/tools/binman/etype/ti_secure_rom.py
+++ b/tools/binman/etype/ti_secure_rom.py
@@ -32,6 +32,7 @@ class Entry_ti_secure_rom(Entry_x509_cert):
 - core: core on which bootloader runs, valid cores are 'secure' and 
'public'
 - content: phandle of SPL in case of legacy bootflow or phandles of 
component binaries
   in case of combined bootflow
+- core-opts (optional): split-mode (0) or lockstep mode (1) set to 0 
by default
 
 The following properties are only for generating a combined bootflow 
binary:
 - sysfw-inner-cert: boolean if binary contains sysfw inner certificate
@@ -69,6 +70,7 @@ class Entry_ti_secure_rom(Entry_x509_cert):
 self.sw_rev = fdt_util.GetInt(self._node, 'sw-rev', 1)
 self.sha = fdt_util.GetInt(self._node, 'sha', 512)
 self.core = fdt_util.GetString(self._node, 'core', 'secure')
+self.bootcore_opts = fdt_util.GetInt(self._node, 'core-opts')
 self.key_fname = self.GetEntryArgsOrProps([
 EntryArg('keyfile', str)], required=True)[0]
 if self.combined:
@@ -97,17 +99,19 @@ class Entry_ti_secure_rom(Entry_x509_cert):
 bytes content of the entry, which is the certificate binary for the
 provided data
 """
+if self.bootcore_opts is None:
+self.bootcore_opts = 0
+
 if self.core == 'secure':
 if self.countersign:
 self.cert_type = 3
 else:
 self.cert_type = 2
 self.bootcore = 0
-self.bootcore_opts = 32
 else:
 self.cert_type = 1
 

[PATCH v3 0/2] Enable split mode in binman

2023-10-06 Thread Neha Malcom Francis
This series extends the functionality of ti-secure-rom entry type in
binman to support enabling of split mode vs. the default lockstep mode
via changing the field in the x509 certificate. A DONOTMERGE patch is
added to give an example of how this can be done via the binman.dtsi

Changes in v3:
- Simon
- added entries.rst change
- updated function comments
- removed inconsistency in setting bootcore_opts to 32

Changes in v2:
- Udit:
- included TRM link in commit message
- added DONOTMERGE patch showing example

Neha Malcom Francis (2):
  binman: openssl: x509: ti_secure_rom: Add support for bootcore_opts
  DONOTMERGE: arm: dts: k3-j7200-binman: Enable split mode for MCU R5

 arch/arm/dts/k3-j7200-binman.dtsi   |  3 +++
 tools/binman/btool/openssl.py   |  6 --
 tools/binman/entries.rst|  1 +
 tools/binman/etype/ti_secure_rom.py | 11 +--
 tools/binman/etype/x509_cert.py |  3 ++-
 tools/binman/test/297_ti_secure_rom.dts |  1 +
 6 files changed, 20 insertions(+), 5 deletions(-)

-- 
2.34.1



Re: [PATCH 1/2] board: ti: am62x: am62x.env: Fix boot_targets

2023-10-06 Thread Roger Quadros
Hi Simon,

On 05/10/2023 20:22, Simon Glass wrote:
> Hi Nishanth,
> 
> On Thu, 5 Oct 2023 at 11:16, Nishanth Menon  wrote:
>>
>> On 12:10-20231005, Nishanth Menon wrote:
>>> On 12:36-20231005, Tom Rini wrote:
 On Thu, Oct 05, 2023 at 09:19:48AM -0500, Andrew Davis wrote:
> On 10/4/23 8:54 AM, Nishanth Menon wrote:
>> On 08:48-20231004, Andrew Davis wrote:
>>> On 10/4/23 8:23 AM, Roger Quadros wrote:
 ti_mmc is not a valid boot_target for standard boot flow so
>>>
>>> Is there some way to make it into a valid boot_target? Otherwise
>>> how do we use uEnv.txt files, or boot from FIT images with overlays?
>>
>> envboot takes care of uEnv.txt file (see
>> https://lore.kernel.org/all/20231004132324.44198-3-rog...@kernel.org/)
>>
>> Early remote proc loading and FIT image is a question for stdboot itself.
>>
>
> If stdboot is missing these features then we shouldn't switch until it
> has them. I'm all for switching to this, but only if it is complete.

 Depends on what you mean?  Did you mean an option to run scripts
 (exists) or an option to do what TI needs done, via
 boot/bootmeth_something.c ?  If the latter, someone from TI needs to
 figure out what that should be and do (but plumbing-wise everything it
 needs should exist).
>>>
>>> Andrew is generalizing here (on the wrong patch though).
>>>
>>> On am62x platforms, there is nothing regressing with this series. The
>>> challenge is early remote_proc loading which is done for J7* platforms.
>>>
>>> How that is initiated as part of bootmethods is something of a gap.
>>>
>>> The other gap has been support for uEnv.txt -> which we can workaround
>>> at the moment by using CONFIG_BOOTCOMMAND="run envboot; bootflow scan
>>> -lb" in defconfig (This series from Roger already does that - hence I am
>>> saying that Andrew is complaining on the wrong series).
>>>
>>> Ideally, we should just have CONFIG_BOOTCOMMAND="bootflow scan -lb" and
>>> uEnv.txt remoteproc loads and the various standard bootmethods should
>>> "just work".
>>
>>
>> I forgot to add: FIT image authenticated boot flow. That is really what
>> ti_mmc distroboot method was trying to solve.
>>
>> Maybe Simon or someone know how the stdboot flow handles authenticated
>> kernel image and dtb boot flow with FIT image?
> 
> Yes you can use FIT configuration verification and things should work as 
> normal.
> 

It there any reference to look at on how this is done? Thanks.

-- 
cheers,
-roger


Re: [PATCH v2 2/2] board: ti: am64x: Switch to standard boot flow

2023-10-06 Thread Roger Quadros
Hi Simon,

On 05/10/2023 20:22, Simon Glass wrote:
> Hi Tom,
> 
> On Thu, 5 Oct 2023 at 10:31, Tom Rini  wrote:
>>
>> On Thu, Oct 05, 2023 at 09:01:42AM -0600, Simon Glass wrote:
>>> Hi Roger,
>>>
>>> On Thu, 5 Oct 2023 at 07:07, Roger Quadros  wrote:

 Switch to using bootstd. Note with this change, we will stop using
 distro_bootcmd and instead depend entirely on bootflow method of
 starting the system up.

 Drop header files that are no longer needed in am64x_evm.h.
 k3_dfu.h is available via k3_dfu.env in am64x.env.

 Drop unused macro CFG_SYS_SDRAM_BASE1.

 Signed-off-by: Roger Quadros 
 ---
  board/ti/am64x/am64x.env| 1 +
  configs/am64x_evm_a53_defconfig | 5 +++--
  include/configs/am64x_evm.h | 9 -
  3 files changed, 4 insertions(+), 11 deletions(-)

 diff --git a/board/ti/am64x/am64x.env b/board/ti/am64x/am64x.env
 index 68e4b7..efd736b99b 100644
 --- a/board/ti/am64x/am64x.env
 +++ b/board/ti/am64x/am64x.env
 @@ -15,6 +15,7 @@ console=ttyS2,115200n8
  args_all=setenv optargs earlycon=ns16550a,mmio32,0x0280 ${mtdparts}
  run_kern=booti ${loadaddr} ${rd_spec} ${fdtaddr}

 +boot_targets=mmc1 mmc0 usb pxe dhcp
  boot=mmc
  mmcdev=1
  bootpart=1:2
 diff --git a/configs/am64x_evm_a53_defconfig 
 b/configs/am64x_evm_a53_defconfig
 index 718ad176cb..43bfcf957a 100644
 --- a/configs/am64x_evm_a53_defconfig
 +++ b/configs/am64x_evm_a53_defconfig
 @@ -31,8 +31,9 @@ CONFIG_SPL_SPI=y
  # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
  CONFIG_SPL_LOAD_FIT=y
  CONFIG_SPL_LOAD_FIT_ADDRESS=0x8100
 -CONFIG_DISTRO_DEFAULTS=y
 -CONFIG_BOOTCOMMAND="run envboot; run distro_bootcmd;"
 +CONFIG_BOOTSTD_FULL=y
 +CONFIG_BOOTSTD_DEFAULTS=y
 +CONFIG_BOOTCOMMAND="run envboot; bootflow scan -lb"
  CONFIG_BOARD_LATE_INIT=y
  CONFIG_SPL_MAX_SIZE=0x18
  CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
 diff --git a/include/configs/am64x_evm.h b/include/configs/am64x_evm.h
 index 062102a610..f9f8c7bc2f 100644
 --- a/include/configs/am64x_evm.h
 +++ b/include/configs/am64x_evm.h
 @@ -9,15 +9,6 @@
  #ifndef __CONFIG_AM642_EVM_H
  #define __CONFIG_AM642_EVM_H

 -#include 
 -#include 
 -#include 
 -#include 
 -#include 
 -
 -/* DDR Configuration */
 -#define CFG_SYS_SDRAM_BASE10x88000
 -
  /* Now for the remaining common defines */
  #include 
>>>
>>> It looks like this file still includes distro_bootcmd and defines all
>>> the BOOT_TARGET things. Can they be dropped? Perhaps for now they
>>> could be put behind an #ifdef if other boards need them?
>>>
>>> I suspect that with your patch as is, the environment is still full of 
>>> scripts?
>>
>> There's a lot of TI platforms, so I'm not sure what "#if" you're
>> thinking of might make it cleaner?  We could / should move some of the
>> still relevant content and comments from that file to
>> include/env/ti/ti_armv7_common.env as I do think some of the K3
>> platforms could just drop  at this point.
> 
> OK so if they are using text env then the header-file stuff doesn't
> matter since I believe it is ignored? I was thinking of something
> like:
> 
> #ifdef CONFIG_DISTRO_DEFAULTS
> 
> do all the distro #defines
> 
> #endif

This is already done, although within #ifdef CONFIG_ARM64 ...

All we are picking up is really CFG_SYS_SDRAM_BASE.

Cleaning up ti_armv7_common.h should be a separate series and
involve testing on all affected platforms (entire TI range) :P.

-- 
cheers,
-roger


Re: [RESEND PATCH v3 0/3] rpi: Convert to standard boot

2023-10-06 Thread Peter Robinson
Hi Simon,

So with more testing of 2023.10 in Fedora we found a regression where
the display dies when the vc4 module loads in the kernel. With further
debug it was found that it was due to the new U-Boot and with
bisecting it myself I have found this series is the cause of the
regression.

The testing I have done is with recent RPi firmware, U-Boot 2023.10
and kernel 6.5.x, but also seen with 6.4.x, with a minimal text only
image. I can also reproduce it by using the F-38 GA image, no updates
and purely just changing the U-Boot component. I've done my testing on
the RPi4.

Let me know if you need more information, I believe you have a RPi you
can test with.

Regards,
Peter

On Thu, Jul 27, 2023 at 10:54 PM Simon Glass  wrote:
>
> This series moves Raspberry Pi boards over to use standard boot.
>
> It also moves rpi over to use a text-based environment. Unfortunately it
> is not possible to empty the header file due to several CFG options.
>
> Fix the repeated "and and" while we are here.
>
> Note that this reduces rodata size by about 4.5KB. We could get another
>
> for a total image-size saving of about 15KB. This is mostly because
> HUSH_PARSER is not enabled anymore and the environment shrinks down by
> about 3.5K. Hush is not actually needed anymore, since standard boot does
> not use it. Also CMD_SYSBOOT is dropped since standard boot calls the
> pxe_utils code directly in that case.
>
> Changes in v3:
> - Rebase to -master
>
> Changes in v2:
> - Rebase to -next
> - Add new patch to disable DISTRO_DEFAULTS
>
> Simon Glass (3):
>   arm: rpi: Switch to standard boot
>   rpi: Disable DISTRO_DEFAULTS
>   arm: rpi: Switch to a text environment
>
>  board/raspberrypi/rpi/rpi.env  |  77 +++
>  configs/rpi_0_w_defconfig  |   2 +-
>  configs/rpi_2_defconfig|   2 +-
>  configs/rpi_3_32b_defconfig|   2 +-
>  configs/rpi_3_b_plus_defconfig |   2 +-
>  configs/rpi_3_defconfig|   2 +-
>  configs/rpi_4_32b_defconfig|   2 +-
>  configs/rpi_4_defconfig|   2 +-
>  configs/rpi_arm64_defconfig|   2 +-
>  configs/rpi_defconfig  |   2 +-
>  include/configs/rpi.h  | 134 -
>  11 files changed, 86 insertions(+), 143 deletions(-)
>  create mode 100644 board/raspberrypi/rpi/rpi.env
>
> --
> 2.41.0.487.g6d72f3e995-goog
>


Re: [PATCH] arm: dts: k3-j721e-sk/common-proc-board: Fix boot

2023-10-06 Thread Roger Quadros



On 05/10/2023 21:15, Nishanth Menon wrote:
> Since commit 9e644284ab81 ("dm: core: Report bootph-pre-ram/sram node
> as pre-reloc after relocation") A53 u-boot proper is broken. This is
> because nodes marked as 'bootph-pre-ram' are not available at u-boot
> proper before relocation.
> 
> To fix this we mark all nodes in u-boot.dtsi as 'bootph-all'.
> 
> Fixes: 69b19ca67bcb ("arm: dts: k3-j721e: Sync with v6.6-rc1")
> Cc: Neha Francis 
> Signed-off-by: Nishanth Menon 

Reviewed-by: Roger Quadros 


Re: [PATCH v2 1/2] binman: openssl: x509: ti_secure_rom: Add support for bootcore_opts

2023-10-06 Thread Neha Malcom Francis

Hi Simon

On 02/10/23 06:46, Simon Glass wrote:

Hi Neha,

On Tue, 26 Sept 2023 at 22:08, Neha Malcom Francis  wrote:


According to the TRMs of K3 platform of devices, the ROM boot image
format specifies a "Core Options Field" that provides the capability to
set the boot core in lockstep when set to 0 or to split mode when set
to 2. Add support for providing the same from the binman DTS. Also


[...]


  self.content = fdt_util.GetPhandleList(self._node, 'content-sbl')
  input_data_sbl = self.GetContents(required)
diff --git a/tools/binman/etype/x509_cert.py b/tools/binman/etype/x509_cert.py
index d028cfe38c..fc0bb12278 100644
--- a/tools/binman/etype/x509_cert.py
+++ b/tools/binman/etype/x509_cert.py
@@ -136,7 +136,8 @@ class Entry_x509_cert(Entry_collection):
  imagesize_sysfw_data=self.imagesize_sysfw_data,
  hashval_sysfw_data=self.hashval_sysfw_data,
  
sysfw_inner_cert_ext_boot_block=self.sysfw_inner_cert_ext_boot_block,
-dm_data_ext_boot_block=self.dm_data_ext_boot_block
+dm_data_ext_boot_block=self.dm_data_ext_boot_block,
+bootcore_opts=self.bootcore_opts


When you add args to a function, please update the function comment.



Before sending out the next version, just wanted to mention that this was a 
function call, the function comment has already been updated in 
tools/binman/btool/openssl.py


--
Thanking You
Neha Malcom Francis


Re: [PATCH v2 1/3] dt-bindings: mtd: fixed-partitions: Add binman compatible

2023-10-06 Thread Michael Walle

Hi,


I'm still not sure why that compatible is needed. Also I'd need to
change
the label which might break user space apps looking for that specific
name.

Also, our board might have u-boot/spl or u-boot/spl/bl31/bl32, right 
now

that's something which depends on an u-boot configuration variable,
which
then enables or disables binman nodes in the -u-boot.dtsi. So in linux
we only have that "bootloader" partition, but there might be either
u-boot+spl or u-boot+spl+bl31+bl32.

Honestly, I'm really not sure this should go into a device tree.


I think we might be getting a bit ahead of ourselves here. I thought
that the decision was that the label should indicate the contents.
If you have multiple things in a partition then it would become a
'section' in Binman's terminology. Either the label programmatically
describes what is inside or it doesn't. We can't have it both ways.
What do you suggest?


As Rob pointed out earlier, it's just a user-facing string. I'm a bit
reluctant to use it programatically.
Taking my example again, the string "bootloader" is sufficient for a
user. He doesn't care if it's u-boot with spl or u-boot with tfa, or
even coreboot. It just says, "in this partition is the bootloader".
If you have an "bootloader" image you can flash it there.

If it has a label "u-boot" and I want to switch to coreboot, will
it have to change to "coreboot"? I really don't think this is practical,
you are really putting software configuration into the device tree.


At present it seems you have the image described in two places - one
is the binman node and the other is the partitions node. I would like
to unify these.


And I'm not sure that will work for all the corner cases :/

If you keep the binman section seperate from the flash partition
definition you don't have any of these problems, although there is
some redundancy:
 - you only have compatible = "binman", "fixed-partition", no further
   compatibles are required
 - you don't have any conflicts with the current partition descriptions
 - you could even use the labels, because binman is the (only?) user

But of course you need to find a place where to put your node.


What does user space do with the partition labels?


I'm not sure. Also I'm not sure if it really matters, I just wanted to
point out, that you'll force users to change it.

-michael


>> What if a board uses eMMC to store the firmware binaries? Will that
>> then
>> be a subnode to the eMMC device?
>
> I thought there was a way to link the partition nodes and the device
> using a property, without having the partition info as a subnode of
> the device. But I may have imagined it as I cannot find it now. So
> yes, it will be a subnode of the eMMC device.

Not sure if that will fly.


I can't find it anyway. There is somelike like that in
simple-framebuffer with the 'display' property.

Regards,
SImon


Re: [PATCH] arm: mach-k3: Remove secure device makefile

2023-10-06 Thread Neha Malcom Francis

Hi Andrew

On 05/10/23 19:51, Andrew Davis wrote:

This is now done using binman but this file was leftover and is now
unused, remove it.

Signed-off-by: Andrew Davis 


Reviewed-by: Neha Malcom Francis 


---
  MAINTAINERS   |  1 -
  arch/arm/mach-k3/config_secure.mk | 44 ---
  2 files changed, 45 deletions(-)
  delete mode 100644 arch/arm/mach-k3/config_secure.mk

diff --git a/MAINTAINERS b/MAINTAINERS
index 4df79254dfe..de4711721b2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1548,7 +1548,6 @@ F:arch/arm/mach-omap2/omap5/sec_entry_cpu1.S
  F:arch/arm/mach-omap2/sec-common.c
  F:arch/arm/mach-omap2/config_secure.mk
  F:arch/arm/mach-k3/security.c
-F: arch/arm/mach-k3/config_secure.mk
  F:configs/am335x_hs_evm_defconfig
  F:configs/am335x_hs_evm_uart_defconfig
  F:configs/am43xx_hs_evm_defconfig
diff --git a/arch/arm/mach-k3/config_secure.mk 
b/arch/arm/mach-k3/config_secure.mk
deleted file mode 100644
index 9cc1f9eb24f..000
--- a/arch/arm/mach-k3/config_secure.mk
+++ /dev/null
@@ -1,44 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-#
-# Copyright (C) 2018 Texas Instruments, Incorporated - http://www.ti.com/
-#  Andrew F. Davis 
-
-quiet_cmd_k3secureimg = SECURE  $@
-ifneq ($(TI_SECURE_DEV_PKG),)
-ifneq ($(wildcard $(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh),)
-cmd_k3secureimg = $(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh \
-   $< $@ \
-   $(if $(KBUILD_VERBOSE:1=), >/dev/null)
-else
-cmd_k3secureimg = echo "WARNING:" \
-   "$(TI_SECURE_DEV_PKG)/scripts/secure-binary-image.sh not found." \
-   "$@ was NOT secured!"; cp $< $@
-endif
-else
-cmd_k3secureimg = echo "WARNING: TI_SECURE_DEV_PKG environment" \
-   "variable must be defined for TI secure devices." \
-   "$@ was NOT secured!"; cp $< $@
-endif
-
-%.dtb_HS: %.dtb FORCE
-   $(call if_changed,k3secureimg)
-
-$(obj)/u-boot-spl-nodtb.bin_HS: $(obj)/u-boot-spl-nodtb.bin FORCE
-   $(call if_changed,k3secureimg)
-
-tispl.bin_HS: $(obj)/u-boot-spl-nodtb.bin_HS $(patsubst 
%,$(obj)/dts/%.dtb_HS,$(subst ",,$(CONFIG_SPL_OF_LIST))) $(SPL_ITS) FORCE
-   $(call if_changed,mkfitimage)
-
-MKIMAGEFLAGS_u-boot.img_HS = -f auto -A $(ARCH) -T firmware -C none -O u-boot \
-   -a $(CONFIG_TEXT_BASE) -e $(CONFIG_SYS_UBOOT_START) \
-   -n "U-Boot $(UBOOTRELEASE) for $(BOARD) board" -E \
-   $(patsubst %,-b arch/$(ARCH)/dts/%.dtb_HS,$(subst ",,$(CONFIG_OF_LIST)))
-
-OF_LIST_TARGETS = $(patsubst %,arch/$(ARCH)/dts/%.dtb,$(subst 
",,$(CONFIG_OF_LIST)))
-$(OF_LIST_TARGETS): dtbs
-
-u-boot-nodtb.bin_HS: u-boot-nodtb.bin FORCE
-   $(call if_changed,k3secureimg)
-
-u-boot.img_HS: u-boot-nodtb.bin_HS u-boot.img $(patsubst 
%.dtb,%.dtb_HS,$(OF_LIST_TARGETS)) FORCE
-   $(call if_changed,mkimage)


--
Thanking You
Neha Malcom Francis