[PATCH] tools: Fix patman launcher script.
There is no "run_patman" procedure in patman's __main__.py file, which would cause the following error at execution: "AttributeError: module 'patman.__main__' has no attribute 'run_patman'" Signed-off-by: Maxim Cournoyer --- tools/patman/pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/patman/pyproject.toml b/tools/patman/pyproject.toml index c5dc7c7e27..a54211f706 100644 --- a/tools/patman/pyproject.toml +++ b/tools/patman/pyproject.toml @@ -23,7 +23,7 @@ classifiers = [ "Bug Tracker" = "https://source.denx.de/groups/u-boot/-/issues; [project.scripts] -patman = "patman.__main__:run_patman" +patman = "patman.__main__" [tool.setuptools.package-data] patman = ["*.rst"] base-commit: 0fe0395922d859730eb7ddfcff6ed8d3ac4b2937 -- 2.41.0
[PATCH RESEND 1/2] arm: dts: rockchip: rk3588, rk3588s: sync with Linux
Sync the devicetree with linux-next tag: next-20230831
Signed-off-by: FUKAUMI Naoki
---
arch/arm/dts/rk3588.dtsi | 215 +++
arch/arm/dts/rk3588s.dtsi | 367 +
include/dt-bindings/ata/ahci.h | 20 ++
3 files changed, 602 insertions(+)
create mode 100644 include/dt-bindings/ata/ahci.h
diff --git a/arch/arm/dts/rk3588.dtsi b/arch/arm/dts/rk3588.dtsi
index 8be75556af..5519c1430c 100644
--- a/arch/arm/dts/rk3588.dtsi
+++ b/arch/arm/dts/rk3588.dtsi
@@ -7,6 +7,16 @@
#include "rk3588-pinctrl.dtsi"
/ {
+ pcie30_phy_grf: syscon@fd5b8000 {
+ compatible = "rockchip,rk3588-pcie3-phy-grf", "syscon";
+ reg = <0x0 0xfd5b8000 0x0 0x1>;
+ };
+
+ pipe_phy1_grf: syscon@fd5c {
+ compatible = "rockchip,rk3588-pipe-phy-grf", "syscon";
+ reg = <0x0 0xfd5c 0x0 0x100>;
+ };
+
i2s8_8ch: i2s@fddc8000 {
compatible = "rockchip,rk3588-i2s-tdm";
reg = <0x0 0xfddc8000 0x0 0x1000>;
@@ -75,6 +85,159 @@
status = "disabled";
};
+ pcie3x4: pcie@fe15 {
+ compatible = "rockchip,rk3588-pcie", "rockchip,rk3568-pcie";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ bus-range = <0x00 0x0f>;
+ clocks = < ACLK_PCIE_4L_MSTR>, < ACLK_PCIE_4L_SLV>,
+< ACLK_PCIE_4L_DBI>, < PCLK_PCIE_4L>,
+< CLK_PCIE_AUX0>, < CLK_PCIE4L_PIPE>;
+ clock-names = "aclk_mst", "aclk_slv",
+ "aclk_dbi", "pclk",
+ "aux", "pipe";
+ device_type = "pci";
+ interrupts = ,
+,
+,
+,
+;
+ interrupt-names = "sys", "pmc", "msg", "legacy", "err";
+ #interrupt-cells = <1>;
+ interrupt-map-mask = <0 0 0 7>;
+ interrupt-map = <0 0 0 1 _intc 0>,
+ <0 0 0 2 _intc 1>,
+ <0 0 0 3 _intc 2>,
+ <0 0 0 4 _intc 3>;
+ linux,pci-domain = <0>;
+ max-link-speed = <3>;
+ msi-map = <0x 0x 0x1000>;
+ num-lanes = <4>;
+ phys = <>;
+ phy-names = "pcie-phy";
+ power-domains = < RK3588_PD_PCIE>;
+ ranges = <0x0100 0x0 0xf010 0x0 0xf010 0x0
0x0010>,
+<0x0200 0x0 0xf020 0x0 0xf020 0x0
0x00e0>,
+<0x0300 0x0 0x4000 0x9 0x 0x0
0x4000>;
+ reg = <0xa 0x4000 0x0 0x0040>,
+ <0x0 0xfe15 0x0 0x0001>,
+ <0x0 0xf000 0x0 0x0010>;
+ reg-names = "dbi", "apb", "config";
+ resets = < SRST_PCIE0_POWER_UP>, < SRST_P_PCIE0>;
+ reset-names = "pwr", "pipe";
+ status = "disabled";
+
+ pcie3x4_intc: legacy-interrupt-controller {
+ interrupt-controller;
+ #address-cells = <0>;
+ #interrupt-cells = <1>;
+ interrupt-parent = <>;
+ interrupts = ;
+ };
+ };
+
+ pcie3x2: pcie@fe16 {
+ compatible = "rockchip,rk3588-pcie", "rockchip,rk3568-pcie";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ bus-range = <0x10 0x1f>;
+ clocks = < ACLK_PCIE_2L_MSTR>, < ACLK_PCIE_2L_SLV>,
+< ACLK_PCIE_2L_DBI>, < PCLK_PCIE_2L>,
+< CLK_PCIE_AUX1>, < CLK_PCIE2L_PIPE>;
+ clock-names = "aclk_mst", "aclk_slv",
+ "aclk_dbi", "pclk",
+ "aux", "pipe";
+ device_type = "pci";
+ interrupts = ,
+,
+,
+,
+;
+ interrupt-names = "sys", "pmc", "msg", "l
[PATCH RESEND 2/2] arm: dts: rockchip: rock-5b: add support for PCIe3 and NVMe
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
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 1b2fcbb0bb..c790894170 100644
--- a/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi
+++ b/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi
@@ -27,6 +27,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";
@@ -87,6 +100,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 {
@@ -97,6 +122,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 {
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
--
2.39.2
[PATCH 2/2] arm: dts: rockchip: rock-5b: add support for PCIe3 and NVMe
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
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 1b2fcbb0bb..c790894170 100644
--- a/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi
+++ b/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi
@@ -27,6 +27,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";
@@ -87,6 +100,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 {
@@ -97,6 +122,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 {
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
--
2.39.2
[PATCH 1/2] arm: dts: rockchip: rk3588, rk3588s: sync with Linux
Sync the devicetree with linux-next tag: next-20230831
Signed-off-by: FUKAUMI Naoki
---
arch/arm/dts/rk3588.dtsi | 215 +++
arch/arm/dts/rk3588s.dtsi | 367 +
include/dt-bindings/ata/ahci.h | 20 ++
3 files changed, 602 insertions(+)
create mode 100644 include/dt-bindings/ata/ahci.h
diff --git a/arch/arm/dts/rk3588.dtsi b/arch/arm/dts/rk3588.dtsi
index 8be75556af..5519c1430c 100644
--- a/arch/arm/dts/rk3588.dtsi
+++ b/arch/arm/dts/rk3588.dtsi
@@ -7,6 +7,16 @@
#include "rk3588-pinctrl.dtsi"
/ {
+ pcie30_phy_grf: syscon@fd5b8000 {
+ compatible = "rockchip,rk3588-pcie3-phy-grf", "syscon";
+ reg = <0x0 0xfd5b8000 0x0 0x1>;
+ };
+
+ pipe_phy1_grf: syscon@fd5c {
+ compatible = "rockchip,rk3588-pipe-phy-grf", "syscon";
+ reg = <0x0 0xfd5c 0x0 0x100>;
+ };
+
i2s8_8ch: i2s@fddc8000 {
compatible = "rockchip,rk3588-i2s-tdm";
reg = <0x0 0xfddc8000 0x0 0x1000>;
@@ -75,6 +85,159 @@
status = "disabled";
};
+ pcie3x4: pcie@fe15 {
+ compatible = "rockchip,rk3588-pcie", "rockchip,rk3568-pcie";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ bus-range = <0x00 0x0f>;
+ clocks = < ACLK_PCIE_4L_MSTR>, < ACLK_PCIE_4L_SLV>,
+< ACLK_PCIE_4L_DBI>, < PCLK_PCIE_4L>,
+< CLK_PCIE_AUX0>, < CLK_PCIE4L_PIPE>;
+ clock-names = "aclk_mst", "aclk_slv",
+ "aclk_dbi", "pclk",
+ "aux", "pipe";
+ device_type = "pci";
+ interrupts = ,
+,
+,
+,
+;
+ interrupt-names = "sys", "pmc", "msg", "legacy", "err";
+ #interrupt-cells = <1>;
+ interrupt-map-mask = <0 0 0 7>;
+ interrupt-map = <0 0 0 1 _intc 0>,
+ <0 0 0 2 _intc 1>,
+ <0 0 0 3 _intc 2>,
+ <0 0 0 4 _intc 3>;
+ linux,pci-domain = <0>;
+ max-link-speed = <3>;
+ msi-map = <0x 0x 0x1000>;
+ num-lanes = <4>;
+ phys = <>;
+ phy-names = "pcie-phy";
+ power-domains = < RK3588_PD_PCIE>;
+ ranges = <0x0100 0x0 0xf010 0x0 0xf010 0x0
0x0010>,
+<0x0200 0x0 0xf020 0x0 0xf020 0x0
0x00e0>,
+<0x0300 0x0 0x4000 0x9 0x 0x0
0x4000>;
+ reg = <0xa 0x4000 0x0 0x0040>,
+ <0x0 0xfe15 0x0 0x0001>,
+ <0x0 0xf000 0x0 0x0010>;
+ reg-names = "dbi", "apb", "config";
+ resets = < SRST_PCIE0_POWER_UP>, < SRST_P_PCIE0>;
+ reset-names = "pwr", "pipe";
+ status = "disabled";
+
+ pcie3x4_intc: legacy-interrupt-controller {
+ interrupt-controller;
+ #address-cells = <0>;
+ #interrupt-cells = <1>;
+ interrupt-parent = <>;
+ interrupts = ;
+ };
+ };
+
+ pcie3x2: pcie@fe16 {
+ compatible = "rockchip,rk3588-pcie", "rockchip,rk3568-pcie";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ bus-range = <0x10 0x1f>;
+ clocks = < ACLK_PCIE_2L_MSTR>, < ACLK_PCIE_2L_SLV>,
+< ACLK_PCIE_2L_DBI>, < PCLK_PCIE_2L>,
+< CLK_PCIE_AUX1>, < CLK_PCIE2L_PIPE>;
+ clock-names = "aclk_mst", "aclk_slv",
+ "aclk_dbi", "pclk",
+ "aux", "pipe";
+ device_type = "pci";
+ interrupts = ,
+,
+,
+,
+;
+ interrupt-names = "sys", "pmc", "msg", "l
[PATCH v2 14/14] fs: Fix a confusing error about overlapping regions
When lmb runs out of space in its internal tables, it gives errors on
every fs load operation. This is horribly confusing, as the poor user
tries different memory regions one at a time.
Use the updated API to check the error code and print a helpful message.
Also, allow the operation to proceed.
Update the tests accordingly.
Signed-off-by: Simon Glass
---
(no changes since v1)
fs/fs.c| 7 ++-
include/lmb.h | 19 ++-
lib/lmb.c | 20 +++-
test/lib/lmb.c | 42 --
4 files changed, 55 insertions(+), 33 deletions(-)
diff --git a/fs/fs.c b/fs/fs.c
index 2b815b1db0fe..1a84cb410bf9 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -569,8 +569,13 @@ static int fs_read_lmb_check(const char *filename, ulong
addr, loff_t offset,
lmb_init_and_reserve(, gd->bd, (void *)gd->fdt_blob);
lmb_dump_all();
- if (lmb_alloc_addr(, addr, read_len) == addr)
+ ret = lmb_alloc_addr(, addr, read_len, NULL);
+ if (!ret)
return 0;
+ if (ret == -E2BIG) {
+ log_warning("Reservation tables exhausted: see
CONFIG_LMB_USE_MAX_REGIONS\n");
+ return 0;
+ }
log_err("** Reading file would overwrite reserved memory **\n");
return -ENOSPC;
diff --git a/include/lmb.h b/include/lmb.h
index c81716b3f1d1..f28a872c78d0 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -114,7 +114,24 @@ phys_addr_t lmb_alloc_base(struct lmb *lmb, phys_size_t
size, ulong align,
phys_addr_t max_addr);
phys_addr_t __lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align,
phys_addr_t max_addr);
-phys_addr_t lmb_alloc_addr(struct lmb *lmb, phys_addr_t base, phys_size_t
size);
+
+/**
+ * lmb_alloc_addr() - Allocate memory within a region
+ *
+ * Try to allocate a specific address range: must be in defined memory but not
+ * reserved
+ *
+ * @lmb: the logical memory block struct
+ * @base: base address of the memory region
+ * @size: size of the memory region
+ * @addrp: if non-NULL, returns the allocated address, on success
+ * Return: 0 if OK, -EPERM if the memory is already allocated, -E2BIG if
+ * there is not enough room in the reservation tables, so
+ * CONFIG_LMB_USE_MAX_REGIONS should be increased
+ */
+int lmb_alloc_addr(struct lmb *lmb, phys_addr_t base, phys_size_t size,
+ phys_addr_t *addrp);
+
phys_size_t lmb_get_free_size(struct lmb *lmb, phys_addr_t addr);
/**
diff --git a/lib/lmb.c b/lib/lmb.c
index fd6326cbf7fe..c3d14db07e45 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -252,7 +252,7 @@ void lmb_init_and_reserve_range(struct lmb *lmb,
phys_addr_t base,
* @size: size of the memory region to add
* @flags: flags for this new memory region
* Returns: 0 if OK, -EBUSY if an existing enveloping region has different
- * flags, -EPERM if there is an existing non-adjacent region, -ENOSPC if there
+ * flags, -EPERM if there is an existing non-adjacent region, -E2BIG if there
* is no more room in the list of regions, ekse region number that was
coalesced
* with this one
**/
@@ -318,7 +318,7 @@ static int lmb_add_area_flags(struct lmb_region *rgn,
phys_addr_t base,
if (coalesced)
return coalesced;
if (rgn->cnt >= rgn->max)
- return -ENOSPC;
+ return -E2BIG;
/* Couldn't coalesce the LMB, so add it to the sorted table. */
for (i = rgn->cnt-1; i >= 0; i--) {
@@ -511,11 +511,8 @@ phys_addr_t __lmb_alloc_base(struct lmb *lmb, phys_size_t
size, ulong align,
return 0;
}
-/*
- * Try to allocate a specific address range: must be in defined memory but not
- * reserved
- */
-phys_addr_t lmb_alloc_addr(struct lmb *lmb, phys_addr_t base, phys_size_t size)
+int lmb_alloc_addr(struct lmb *lmb, phys_addr_t base, phys_size_t size,
+ phys_addr_t *addrp)
{
int area;
@@ -529,9 +526,14 @@ phys_addr_t lmb_alloc_addr(struct lmb *lmb, phys_addr_t
base, phys_size_t size)
if (lmb_addrs_overlap(lmb->memory.area[area].base,
lmb->memory.area[area].size,
base + size - 1, 1)) {
+ int ret;
+
/* ok, reserve the memory */
- if (lmb_reserve(lmb, base, size) >= 0)
- return base;
+ ret = lmb_reserve(lmb, base, size);
+ if (ret < 0)
+ return ret;
+ if (addrp)
+ *addrp = base;
}
}
diff --git a/test/lib/lmb.c b/test/lib/lmb.c
index b665d8dcdeb4..c8dbdb3b73eb 100644
--- a/test/lib/lmb.c
+++ b/test/lib/lmb.c
@@ -497,22 +497,22 @@ static int test_alloc_addr(struct unit_test_state *uts,
const phys_addr_t ram)
[PATCH v2 13/14] lmb: Document and tidy lmb_add_region_flags()
Update this to return an error code so we can tell when it just ran out of
space in its lmb list. Make lmb_addrs_overlap() return a bool.
Add a few function comments while we are here.
Signed-off-by: Simon Glass
---
(no changes since v1)
lib/lmb.c | 47 +--
1 file changed, 41 insertions(+), 6 deletions(-)
diff --git a/lib/lmb.c b/lib/lmb.c
index 8a299c97b3ad..fd6326cbf7fe 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -53,8 +53,17 @@ void lmb_dump_all(struct lmb *lmb)
#endif
}
-static int lmb_addrs_overlap(phys_addr_t base1, phys_size_t size1,
-phys_addr_t base2, phys_size_t size2)
+/**
+ * lmb_addrs_overlap() - Compare two address ranges for overlap
+ *
+ * @base1: base address of region 1
+ * @size1: size of the region 1
+ * @base2: base address of region 2
+ * @size2: size of the region 2
+ * Returns: true if the regions overlap, else false
+ */
+static bool lmb_addrs_overlap(phys_addr_t base1, phys_size_t size1,
+ phys_addr_t base2, phys_size_t size2)
{
const phys_addr_t base1_end = base1 + size1 - 1;
const phys_addr_t base2_end = base2 + size2 - 1;
@@ -62,6 +71,17 @@ static int lmb_addrs_overlap(phys_addr_t base1, phys_size_t
size1,
return base1 <= base2_end && base2 <= base1_end;
}
+/**
+ * lmb_addrs_adjacent() - Compare two address ranges for adjacency
+ *
+ * @base1: base address of region 1
+ * @size1: size of the region 1
+ * @base2: base address of region 2
+ * @size2: size of the region 2
+ * Returns: 1 if region 2 start at the end of region 1,
+ * -1 if region 1 starts at the end of region 2,
+ * else 0
+ */
static int lmb_addrs_adjacent(phys_addr_t base1, phys_size_t size1,
phys_addr_t base2, phys_size_t size2)
{
@@ -220,7 +240,22 @@ void lmb_init_and_reserve_range(struct lmb *lmb,
phys_addr_t base,
lmb_reserve_common(lmb, fdt_blob);
}
-/* This routine called with relocation disabled. */
+/**
+ * lmb_add_area_flags() - Add a region or coalesce with another similar one
+ *
+ * This will coalesce with an existing regions so long as @flags is the same.
+ *
+ * This routine is called with relocation disabled.
+ *
+ * @rgn: Region to process
+ * @base: base address of the memory region to add
+ * @size: size of the memory region to add
+ * @flags: flags for this new memory region
+ * Returns: 0 if OK, -EBUSY if an existing enveloping region has different
+ * flags, -EPERM if there is an existing non-adjacent region, -ENOSPC if there
+ * is no more room in the list of regions, ekse region number that was
coalesced
+ * with this one
+ **/
static int lmb_add_area_flags(struct lmb_region *rgn, phys_addr_t base,
phys_size_t size, enum lmb_flags flags)
{
@@ -250,7 +285,7 @@ static int lmb_add_area_flags(struct lmb_region *rgn,
phys_addr_t base,
/* Already have this area, so we're done */
return 0;
else
- return -1; /* areas with new flags */
+ return -EBUSY; /* areas with new flags */
}
adjacent = lmb_addrs_adjacent(base, size, abase, asize);
@@ -269,7 +304,7 @@ static int lmb_add_area_flags(struct lmb_region *rgn,
phys_addr_t base,
break;
} else if (lmb_addrs_overlap(base, size, abase, asize)) {
/* areas overlap */
- return -1;
+ return -EPERM;
}
}
@@ -283,7 +318,7 @@ static int lmb_add_area_flags(struct lmb_region *rgn,
phys_addr_t base,
if (coalesced)
return coalesced;
if (rgn->cnt >= rgn->max)
- return -1;
+ return -ENOSPC;
/* Couldn't coalesce the LMB, so add it to the sorted table. */
for (i = rgn->cnt-1; i >= 0; i--) {
--
2.42.0.283.g2d96d420d3-goog
[PATCH v2 12/14] lmb: Tidy up lmb_overlaps_region()
Add a comment to define what this returns. Return a specific error when
no overlap is found.
Signed-off-by: Simon Glass
---
(no changes since v1)
lib/lmb.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/lib/lmb.c b/lib/lmb.c
index 0c6244ca47f8..8a299c97b3ad 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -386,6 +386,13 @@ int lmb_reserve(struct lmb *lmb, phys_addr_t base,
phys_size_t size)
return lmb_reserve_flags(lmb, base, size, LMB_NONE);
}
+/**
+ * lmb_overlaps_region() - Check if a region overlaps a given base/size
+ *
+ * @base: base address of the memory region
+ * @size: size of the memory region
+ * Returns: Region number of overlapping region, if found, else -ENOENT
+ */
static int lmb_overlaps_region(struct lmb_region *rgn, phys_addr_t base,
phys_size_t size)
{
@@ -396,10 +403,10 @@ static int lmb_overlaps_region(struct lmb_region *rgn,
phys_addr_t base,
phys_size_t asize = rgn->area[i].size;
if (lmb_addrs_overlap(base, size, abase, asize))
- break;
+ return i;
}
- return i < rgn->cnt ? i : -1;
+ return -ENOENT;
}
phys_addr_t lmb_alloc(struct lmb *lmb, phys_size_t size, ulong align)
--
2.42.0.283.g2d96d420d3-goog
[PATCH v2 11/14] lmb: Change functions returning long to return int
It makes no sense to return long when an int is plenty to provide an error
code and a region position. It is just confusing.
Update the return-value types to keep to this rule.
Signed-off-by: Simon Glass
---
(no changes since v1)
include/lmb.h | 10 +-
lib/lmb.c | 10 +-
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/lmb.h b/include/lmb.h
index 6f91df43d5d5..c81716b3f1d1 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -96,8 +96,8 @@ void lmb_init(struct lmb *lmb);
void lmb_init_and_reserve(struct lmb *lmb, struct bd_info *bd, void *fdt_blob);
void lmb_init_and_reserve_range(struct lmb *lmb, phys_addr_t base,
phys_size_t size, void *fdt_blob);
-long lmb_add(struct lmb *lmb, phys_addr_t base, phys_size_t size);
-long lmb_reserve(struct lmb *lmb, phys_addr_t base, phys_size_t size);
+int lmb_add(struct lmb *lmb, phys_addr_t base, phys_size_t size);
+int lmb_reserve(struct lmb *lmb, phys_addr_t base, phys_size_t size);
/**
* lmb_reserve_flags - Reserve one area with a specific flags bitfield.
*
@@ -107,8 +107,8 @@ long lmb_reserve(struct lmb *lmb, phys_addr_t base,
phys_size_t size);
* @flags: flags for the memory area
* Return: 0 if OK, > 0 for coalesced area or a negative error code.
*/
-long lmb_reserve_flags(struct lmb *lmb, phys_addr_t base,
- phys_size_t size, enum lmb_flags flags);
+int lmb_reserve_flags(struct lmb *lmb, phys_addr_t base, phys_size_t size,
+ enum lmb_flags flags);
phys_addr_t lmb_alloc(struct lmb *lmb, phys_size_t size, ulong align);
phys_addr_t lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align,
phys_addr_t max_addr);
@@ -141,7 +141,7 @@ int lmb_is_reserved(struct lmb *lmb, phys_addr_t addr);
*/
int lmb_is_reserved_flags(struct lmb *lmb, phys_addr_t addr, int flags);
-long lmb_free(struct lmb *lmb, phys_addr_t base, phys_size_t size);
+int lmb_free(struct lmb *lmb, phys_addr_t base, phys_size_t size);
void lmb_dump_all(struct lmb *lmb);
void lmb_dump_all_force(struct lmb *lmb);
diff --git a/lib/lmb.c b/lib/lmb.c
index 579f98baef87..0c6244ca47f8 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -315,14 +315,14 @@ static int lmb_add_area(struct lmb_region *rgn,
phys_addr_t base,
}
/* This routine may be called with relocation disabled. */
-long lmb_add(struct lmb *lmb, phys_addr_t base, phys_size_t size)
+int lmb_add(struct lmb *lmb, phys_addr_t base, phys_size_t size)
{
struct lmb_region *_rgn = &(lmb->memory);
return lmb_add_area(_rgn, base, size);
}
-long lmb_free(struct lmb *lmb, phys_addr_t base, phys_size_t size)
+int lmb_free(struct lmb *lmb, phys_addr_t base, phys_size_t size)
{
struct lmb_region *rgn = &(lmb->reserved);
phys_addr_t rgnbegin, rgnend;
@@ -373,15 +373,15 @@ long lmb_free(struct lmb *lmb, phys_addr_t base,
phys_size_t size)
rgn->area[i].flags);
}
-long lmb_reserve_flags(struct lmb *lmb, phys_addr_t base, phys_size_t size,
- enum lmb_flags flags)
+int lmb_reserve_flags(struct lmb *lmb, phys_addr_t base, phys_size_t size,
+ enum lmb_flags flags)
{
struct lmb_region *_rgn = &(lmb->reserved);
return lmb_add_area_flags(_rgn, base, size, flags);
}
-long lmb_reserve(struct lmb *lmb, phys_addr_t base, phys_size_t size)
+int lmb_reserve(struct lmb *lmb, phys_addr_t base, phys_size_t size)
{
return lmb_reserve_flags(lmb, base, size, LMB_NONE);
}
--
2.42.0.283.g2d96d420d3-goog
[PATCH v2 10/14] lmb: Avoid long for loop counters and function returns
Use int for loop counters since it is more common. Do the same with the
return value of some internal functions.
Signed-off-by: Simon Glass
---
(no changes since v1)
lib/lmb.c | 32
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/lib/lmb.c b/lib/lmb.c
index 862b1cd04953..579f98baef87 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -53,8 +53,8 @@ void lmb_dump_all(struct lmb *lmb)
#endif
}
-static long lmb_addrs_overlap(phys_addr_t base1, phys_size_t size1,
- phys_addr_t base2, phys_size_t size2)
+static int lmb_addrs_overlap(phys_addr_t base1, phys_size_t size1,
+phys_addr_t base2, phys_size_t size2)
{
const phys_addr_t base1_end = base1 + size1 - 1;
const phys_addr_t base2_end = base2 + size2 - 1;
@@ -62,8 +62,8 @@ static long lmb_addrs_overlap(phys_addr_t base1, phys_size_t
size1,
return base1 <= base2_end && base2 <= base1_end;
}
-static long lmb_addrs_adjacent(phys_addr_t base1, phys_size_t size1,
- phys_addr_t base2, phys_size_t size2)
+static int lmb_addrs_adjacent(phys_addr_t base1, phys_size_t size1,
+ phys_addr_t base2, phys_size_t size2)
{
if (base2 == base1 + size1)
return 1;
@@ -73,7 +73,7 @@ static long lmb_addrs_adjacent(phys_addr_t base1, phys_size_t
size1,
return 0;
}
-static long lmb_areas_adjacent(struct lmb_region *rgn, ulong a1, ulong a2)
+static int lmb_areas_adjacent(struct lmb_region *rgn, ulong a1, ulong a2)
{
phys_addr_t base1 = rgn->area[a1].base;
phys_size_t size1 = rgn->area[a1].size;
@@ -85,7 +85,7 @@ static long lmb_areas_adjacent(struct lmb_region *rgn, ulong
a1, ulong a2)
static void lmb_remove_area(struct lmb_region *rgn, unsigned long area)
{
- unsigned long i;
+ uint i;
for (i = area; i < rgn->cnt - 1; i++)
rgn->area[i] = rgn->area[i + 1];
@@ -221,11 +221,11 @@ void lmb_init_and_reserve_range(struct lmb *lmb,
phys_addr_t base,
}
/* This routine called with relocation disabled. */
-static long lmb_add_area_flags(struct lmb_region *rgn, phys_addr_t base,
- phys_size_t size, enum lmb_flags flags)
+static int lmb_add_area_flags(struct lmb_region *rgn, phys_addr_t base,
+ phys_size_t size, enum lmb_flags flags)
{
- unsigned long coalesced = 0;
- long adjacent, i;
+ uint coalesced = 0;
+ int adjacent, i;
if (rgn->cnt == 0) {
rgn->area[0].base = base;
@@ -308,8 +308,8 @@ static long lmb_add_area_flags(struct lmb_region *rgn,
phys_addr_t base,
return 0;
}
-static long lmb_add_area(struct lmb_region *rgn, phys_addr_t base,
-phys_size_t size)
+static int lmb_add_area(struct lmb_region *rgn, phys_addr_t base,
+ phys_size_t size)
{
return lmb_add_area_flags(rgn, base, size, LMB_NONE);
}
@@ -386,10 +386,10 @@ long lmb_reserve(struct lmb *lmb, phys_addr_t base,
phys_size_t size)
return lmb_reserve_flags(lmb, base, size, LMB_NONE);
}
-static long lmb_overlaps_region(struct lmb_region *rgn, phys_addr_t base,
- phys_size_t size)
+static int lmb_overlaps_region(struct lmb_region *rgn, phys_addr_t base,
+ phys_size_t size)
{
- unsigned long i;
+ uint i;
for (i = 0; i < rgn->cnt; i++) {
phys_addr_t abase = rgn->area[i].base;
@@ -429,7 +429,7 @@ static phys_addr_t lmb_align_down(phys_addr_t addr,
phys_size_t size)
phys_addr_t __lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align,
phys_addr_t max_addr)
{
- long i, area;
+ int i, area;
phys_addr_t base = 0;
phys_addr_t res_base;
--
2.42.0.283.g2d96d420d3-goog
[PATCH v2 09/14] lmb: Tidy up structure access
In some cases it helps to define a local variable pointing to the
structure being accessed. This avoids lots of repeated code.
There is no need to individually assign each struct member, so use a
structure assignment instead.
Signed-off-by: Simon Glass
---
(no changes since v1)
lib/lmb.c | 58 ++-
1 file changed, 27 insertions(+), 31 deletions(-)
diff --git a/lib/lmb.c b/lib/lmb.c
index 0b03cc4d1c23..862b1cd04953 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -23,20 +23,19 @@ DECLARE_GLOBAL_DATA_PTR;
static void lmb_dump_region(struct lmb_region *rgn, char *name)
{
- unsigned long long base, size, end;
- enum lmb_flags flags;
int i;
printf(" %s.cnt = 0x%lx / max = 0x%lx\n", name, rgn->cnt, rgn->max);
for (i = 0; i < rgn->cnt; i++) {
- base = rgn->area[i].base;
- size = rgn->area[i].size;
- end = base + size - 1;
- flags = rgn->area[i].flags;
+ struct lmb_area *area = >area[i];
+ unsigned long long end;
+
+ end = area->base + area->size - 1;
printf(" %s[%d]\t[0x%llx-0x%llx], 0x%08llx bytes flags: %x\n",
- name, i, base, end, size, flags);
+ name, i, (unsigned long long)area->base, end,
+ (unsigned long long)area->size, area->flags);
}
}
@@ -88,11 +87,8 @@ static void lmb_remove_area(struct lmb_region *rgn, unsigned
long area)
{
unsigned long i;
- for (i = area; i < rgn->cnt - 1; i++) {
- rgn->area[i].base = rgn->area[i + 1].base;
- rgn->area[i].size = rgn->area[i + 1].size;
- rgn->area[i].flags = rgn->area[i + 1].flags;
- }
+ for (i = area; i < rgn->cnt - 1; i++)
+ rgn->area[i] = rgn->area[i + 1];
rgn->cnt--;
}
@@ -120,6 +116,7 @@ void lmb_init(struct lmb *lmb)
void arch_lmb_reserve_generic(struct lmb *lmb, ulong sp, ulong end, ulong
align)
{
+ struct bd_info *bd = gd->bd;
ulong bank_end;
int bank;
@@ -133,12 +130,10 @@ void arch_lmb_reserve_generic(struct lmb *lmb, ulong sp,
ulong end, ulong align)
/* adjust sp by 4K to be safe */
sp -= align;
for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
- if (!gd->bd->bi_dram[bank].size ||
- sp < gd->bd->bi_dram[bank].start)
+ if (!bd->bi_dram[bank].size || sp < bd->bi_dram[bank].start)
continue;
/* Watch out for RAM at end of address space! */
- bank_end = gd->bd->bi_dram[bank].start +
- gd->bd->bi_dram[bank].size - 1;
+ bank_end = bd->bi_dram[bank].start + bd->bi_dram[bank].size - 1;
if (sp > bank_end)
continue;
if (bank_end > end)
@@ -242,13 +237,15 @@ static long lmb_add_area_flags(struct lmb_region *rgn,
phys_addr_t base,
/* First try and coalesce this LMB with another. */
for (i = 0; i < rgn->cnt; i++) {
- phys_addr_t abase = rgn->area[i].base;
- phys_size_t asize = rgn->area[i].size;
- phys_size_t aflags = rgn->area[i].flags;
+ struct lmb_area *area = >area[i];
+
+ phys_addr_t abase = area->base;
+ phys_size_t asize = area->size;
+ phys_size_t aflags = area->flags;
phys_addr_t end = base + size - 1;
- phys_addr_t rgnend = abase + asize - 1;
+ phys_addr_t aend = abase + asize - 1;
- if (abase <= base && end <= rgnend) {
+ if (abase <= base && end <= aend) {
if (flags == aflags)
/* Already have this area, so we're done */
return 0;
@@ -260,14 +257,14 @@ static long lmb_add_area_flags(struct lmb_region *rgn,
phys_addr_t base,
if (adjacent > 0) {
if (flags != aflags)
break;
- rgn->area[i].base -= size;
- rgn->area[i].size += size;
+ area->base -= size;
+ area->size += size;
coalesced++;
break;
} else if (adjacent < 0) {
if (flags != aflags)
break;
- rgn->area[i].size += size;
+ area->size += size;
coalesced++;
break;
} else if (lmb_addrs_overlap(base, size, abase, asize)) {
@@ -291,9 +288,7 @@ static long lmb_add_area_flags(struct lmb_region *rgn,
phys_addr_t base,
/* Couldn't coalesce the LMB, so add it to the sorted table. */
for (i = rgn->cnt-1; i >= 0; i--) {
[PATCH v2 08/14] lmb: Update use of region in the C file
Change places that should say area to do so. Change the name of the 'rgn'
variable to 'area' in places where it refers to an area number.
Fix the 'beginging' typo while here.
Signed-off-by: Simon Glass
---
Changes in v2:
- Add new patch to update use of region in the C file
lib/lmb.c | 126 +++---
1 file changed, 62 insertions(+), 64 deletions(-)
diff --git a/lib/lmb.c b/lib/lmb.c
index 6061c6f361c6..0b03cc4d1c23 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -74,22 +74,21 @@ static long lmb_addrs_adjacent(phys_addr_t base1,
phys_size_t size1,
return 0;
}
-static long lmb_regions_adjacent(struct lmb_region *rgn, unsigned long r1,
-unsigned long r2)
+static long lmb_areas_adjacent(struct lmb_region *rgn, ulong a1, ulong a2)
{
- phys_addr_t base1 = rgn->area[r1].base;
- phys_size_t size1 = rgn->area[r1].size;
- phys_addr_t base2 = rgn->area[r2].base;
- phys_size_t size2 = rgn->area[r2].size;
+ phys_addr_t base1 = rgn->area[a1].base;
+ phys_size_t size1 = rgn->area[a1].size;
+ phys_addr_t base2 = rgn->area[a2].base;
+ phys_size_t size2 = rgn->area[a2].size;
return lmb_addrs_adjacent(base1, size1, base2, size2);
}
-static void lmb_remove_region(struct lmb_region *rgn, unsigned long r)
+static void lmb_remove_area(struct lmb_region *rgn, unsigned long area)
{
unsigned long i;
- for (i = r; i < rgn->cnt - 1; i++) {
+ for (i = area; i < rgn->cnt - 1; i++) {
rgn->area[i].base = rgn->area[i + 1].base;
rgn->area[i].size = rgn->area[i + 1].size;
rgn->area[i].flags = rgn->area[i + 1].flags;
@@ -97,12 +96,11 @@ static void lmb_remove_region(struct lmb_region *rgn,
unsigned long r)
rgn->cnt--;
}
-/* Assumption: base addr of region 1 < base addr of region 2 */
-static void lmb_coalesce_regions(struct lmb_region *rgn, unsigned long r1,
-unsigned long r2)
+/* Assumption: base addr of area 1 < base addr of area 2 */
+static void lmb_coalesce_areas(struct lmb_region *rgn, uint a1, uint a2)
{
- rgn->area[r1].size += rgn->area[r2].size;
- lmb_remove_region(rgn, r2);
+ rgn->area[a1].size += rgn->area[a2].size;
+ lmb_remove_area(rgn, a2);
}
void lmb_init(struct lmb *lmb)
@@ -228,8 +226,8 @@ void lmb_init_and_reserve_range(struct lmb *lmb,
phys_addr_t base,
}
/* This routine called with relocation disabled. */
-static long lmb_add_region_flags(struct lmb_region *rgn, phys_addr_t base,
-phys_size_t size, enum lmb_flags flags)
+static long lmb_add_area_flags(struct lmb_region *rgn, phys_addr_t base,
+ phys_size_t size, enum lmb_flags flags)
{
unsigned long coalesced = 0;
long adjacent, i;
@@ -244,43 +242,43 @@ static long lmb_add_region_flags(struct lmb_region *rgn,
phys_addr_t base,
/* First try and coalesce this LMB with another. */
for (i = 0; i < rgn->cnt; i++) {
- phys_addr_t rgnbase = rgn->area[i].base;
- phys_size_t rgnsize = rgn->area[i].size;
- phys_size_t rgnflags = rgn->area[i].flags;
+ phys_addr_t abase = rgn->area[i].base;
+ phys_size_t asize = rgn->area[i].size;
+ phys_size_t aflags = rgn->area[i].flags;
phys_addr_t end = base + size - 1;
- phys_addr_t rgnend = rgnbase + rgnsize - 1;
+ phys_addr_t rgnend = abase + asize - 1;
- if (rgnbase <= base && end <= rgnend) {
- if (flags == rgnflags)
- /* Already have this region, so we're done */
+ if (abase <= base && end <= rgnend) {
+ if (flags == aflags)
+ /* Already have this area, so we're done */
return 0;
else
- return -1; /* regions with new flags */
+ return -1; /* areas with new flags */
}
- adjacent = lmb_addrs_adjacent(base, size, rgnbase, rgnsize);
+ adjacent = lmb_addrs_adjacent(base, size, abase, asize);
if (adjacent > 0) {
- if (flags != rgnflags)
+ if (flags != aflags)
break;
rgn->area[i].base -= size;
rgn->area[i].size += size;
coalesced++;
break;
} else if (adjacent < 0) {
- if (flags != rgnflags)
+ if (flags != aflags)
break;
rgn->area[i].size += size;
coalesced++;
break;
- } else if
[PATCH v2 07/14] lmb: Update use of region in the header file
Tidy up places in the header file where it says region but now means area.
Signed-off-by: Simon Glass
---
Changes in v2:
- Add new patch to update use of region in the header file
include/lmb.h | 44 ++--
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/include/lmb.h b/include/lmb.h
index 3cf23d2f2346..6f91df43d5d5 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -13,7 +13,7 @@
*/
/**
- * enum lmb_flags - definition of memory region attributes
+ * enum lmb_flags - definition of memory area attributes
* @LMB_NONE: no special request
* @LMB_NOMAP: don't add to mmu configuration
*/
@@ -40,24 +40,24 @@ struct lmb_area {
* all the #if test are done with CONFIG_LMB_USE_MAX_AREAS (boolean)
*
* case 1. CONFIG_LMB_USE_MAX_AREAS is defined (legacy mode)
- * => CONFIG_LMB_MAX_AREAS is used to configure the region size,
- * directly in the array lmb_region.region[], with the same
- * configuration for memory and reserved regions.
+ * => CONFIG_LMB_MAX_AREAS is used to configure the maximum number of
+ *areas, directly in the array lmb_region.area[], with the same
+ * configuration for memory and reserved areas.
*
- * case 2. CONFIG_LMB_USE_MAX_AREAS is not defined, the size of each
- * region is configurated *independently* with
- * => CONFIG_LMB_MEMORY_AREAS: struct lmb.memory_regions
- * => CONFIG_LMB_RESERVED_AREAS: struct lmb.reserved_regions
- * lmb_region.region is only a pointer to the correct buffer,
+ * case 2. CONFIG_LMB_USE_MAX_AREAS is not defined, the maximum number of
+ * areas is configured *independently* with
+ * => CONFIG_LMB_MEMORY_AREAS: struct lmb.memory_areas
+ * => CONFIG_LMB_RESERVED_AREAS: struct lmb.reserved_areas
+ * lmb_region.area is only a pointer to the correct buffer,
* initialized in lmb_init(). This configuration is useful to manage
- * more reserved memory regions with CONFIG_LMB_RESERVED_AREAS.
+ * more reserved memory areas with CONFIG_LMB_RESERVED_AREAS.
*/
/**
- * struct lmb_region - Description of a set of region.
+ * struct lmb_region - Description of a set of areas.
*
- * @cnt: Number of regions.
- * @max: Size of the region array, max value of cnt.
+ * @cnt: Number of areas
+ * @max: Size of the area array, max value of cnt
* @area: Array of the areas within the region
*/
struct lmb_region {
@@ -99,13 +99,13 @@ void lmb_init_and_reserve_range(struct lmb *lmb,
phys_addr_t base,
long lmb_add(struct lmb *lmb, phys_addr_t base, phys_size_t size);
long lmb_reserve(struct lmb *lmb, phys_addr_t base, phys_size_t size);
/**
- * lmb_reserve_flags - Reserve one region with a specific flags bitfield.
+ * lmb_reserve_flags - Reserve one area with a specific flags bitfield.
*
* @lmb: the logical memory block struct
- * @base: base address of the memory region
- * @size: size of the memory region
- * @flags: flags for the memory region
- * Return: 0 if OK, > 0 for coalesced region or a negative error code.
+ * @base: base address of the memory area
+ * @size: size of the memory area
+ * @flags: flags for the memory area
+ * Return: 0 if OK, > 0 for coalesced area or a negative error code.
*/
long lmb_reserve_flags(struct lmb *lmb, phys_addr_t base,
phys_size_t size, enum lmb_flags flags);
@@ -118,9 +118,9 @@ phys_addr_t lmb_alloc_addr(struct lmb *lmb, phys_addr_t
base, phys_size_t size);
phys_size_t lmb_get_free_size(struct lmb *lmb, phys_addr_t addr);
/**
- * lmb_is_reserved() - test if address is in reserved region
+ * lmb_is_reserved() - test if address is in reserved area
*
- * The function checks if a reserved region comprising @addr exists.
+ * The function checks if a reserved area comprising @addr exists.
*
* @lmb: the logical memory block struct
* @addr: address to be tested
@@ -129,9 +129,9 @@ phys_size_t lmb_get_free_size(struct lmb *lmb, phys_addr_t
addr);
int lmb_is_reserved(struct lmb *lmb, phys_addr_t addr);
/**
- * lmb_is_reserved_flags() - test if address is in reserved region with flag
bits set
+ * lmb_is_reserved_flags() - test if address is in reserved area with flag
bits set
*
- * The function checks if a reserved region comprising @addr exists which has
+ * The function checks if a reserved area comprising @addr exists which has
* all flag bits set which are set in @flags.
*
* @lmb: the logical memory block struct
--
2.42.0.283.g2d96d420d3-goog
[PATCH v2 06/14] lmb: Rename LMB_MAX_REGIONS and LMB_USE_MAX_REGIONS
These refer to the maximum number of areas, so rename them. Signed-off-by: Simon Glass --- Changes in v2: - Add new patch to rename LMB_MAX_REGIONS and LMB_USE_MAX_REGIONS configs/a3y17lte_defconfig | 2 +- configs/a5y17lte_defconfig | 2 +- configs/a7y17lte_defconfig | 2 +- configs/apple_m1_defconfig | 2 +- configs/dragonboard845c_defconfig| 2 +- configs/mt7981_emmc_rfb_defconfig| 2 +- configs/mt7981_rfb_defconfig | 2 +- configs/mt7981_sd_rfb_defconfig | 2 +- configs/mt7986_rfb_defconfig | 2 +- configs/mt7986a_bpir3_emmc_defconfig | 2 +- configs/mt7986a_bpir3_sd_defconfig | 2 +- configs/qcs404evb_defconfig | 2 +- configs/starqltechn_defconfig| 2 +- configs/stm32mp13_defconfig | 2 +- configs/stm32mp15_basic_defconfig| 2 +- configs/stm32mp15_defconfig | 2 +- configs/stm32mp15_trusted_defconfig | 2 +- configs/th1520_lpi4a_defconfig | 2 +- include/lmb.h| 14 - lib/Kconfig | 24 +++ lib/lmb.c| 6 ++-- test/lib/lmb.c | 44 ++-- 22 files changed, 62 insertions(+), 62 deletions(-) diff --git a/configs/a3y17lte_defconfig b/configs/a3y17lte_defconfig index 42fcd2a3d317..4db2274731ad 100644 --- a/configs/a3y17lte_defconfig +++ b/configs/a3y17lte_defconfig @@ -24,4 +24,4 @@ CONFIG_SYS_BOOTM_LEN=0x200 CONFIG_CMD_GPIO=y CONFIG_CMD_I2C=y CONFIG_DM_I2C_GPIO=y -CONFIG_LMB_MAX_REGIONS=64 +CONFIG_LMB_MAX_AREAS=64 diff --git a/configs/a5y17lte_defconfig b/configs/a5y17lte_defconfig index 3b80536c12c8..4bcd8313630c 100644 --- a/configs/a5y17lte_defconfig +++ b/configs/a5y17lte_defconfig @@ -24,4 +24,4 @@ CONFIG_SYS_BOOTM_LEN=0x200 CONFIG_CMD_GPIO=y CONFIG_CMD_I2C=y CONFIG_DM_I2C_GPIO=y -CONFIG_LMB_MAX_REGIONS=64 +CONFIG_LMB_MAX_AREAS=64 diff --git a/configs/a7y17lte_defconfig b/configs/a7y17lte_defconfig index 9390e35057eb..7236b70e4f88 100644 --- a/configs/a7y17lte_defconfig +++ b/configs/a7y17lte_defconfig @@ -24,4 +24,4 @@ CONFIG_SYS_BOOTM_LEN=0x200 CONFIG_CMD_GPIO=y CONFIG_CMD_I2C=y CONFIG_DM_I2C_GPIO=y -CONFIG_LMB_MAX_REGIONS=64 +CONFIG_LMB_MAX_AREAS=64 diff --git a/configs/apple_m1_defconfig b/configs/apple_m1_defconfig index d58a9030dbd0..618baee132fa 100644 --- a/configs/apple_m1_defconfig +++ b/configs/apple_m1_defconfig @@ -22,4 +22,4 @@ CONFIG_SYS_WHITE_ON_BLACK=y CONFIG_NO_FB_CLEAR=y CONFIG_VIDEO_SIMPLE=y # CONFIG_GENERATE_SMBIOS_TABLE is not set -CONFIG_LMB_MAX_REGIONS=64 +CONFIG_LMB_MAX_AREAS=64 diff --git a/configs/dragonboard845c_defconfig b/configs/dragonboard845c_defconfig index a69d82761a8d..17b1e4ffd7df 100644 --- a/configs/dragonboard845c_defconfig +++ b/configs/dragonboard845c_defconfig @@ -26,4 +26,4 @@ CONFIG_DM_PMIC=y CONFIG_PMIC_QCOM=y CONFIG_MSM_GENI_SERIAL=y CONFIG_SPMI_MSM=y -CONFIG_LMB_MAX_REGIONS=64 +CONFIG_LMB_MAX_AREAS=64 diff --git a/configs/mt7981_emmc_rfb_defconfig b/configs/mt7981_emmc_rfb_defconfig index b3b37b6e5ed4..44dbfa86e9b1 100644 --- a/configs/mt7981_emmc_rfb_defconfig +++ b/configs/mt7981_emmc_rfb_defconfig @@ -62,4 +62,4 @@ CONFIG_MTK_SERIAL=y CONFIG_FAT_WRITE=y CONFIG_HEXDUMP=y # CONFIG_EFI_LOADER is not set -CONFIG_LMB_MAX_REGIONS=64 +CONFIG_LMB_MAX_AREAS=64 diff --git a/configs/mt7981_rfb_defconfig b/configs/mt7981_rfb_defconfig index b7ffb4dfa74d..4fc9dd92fe43 100644 --- a/configs/mt7981_rfb_defconfig +++ b/configs/mt7981_rfb_defconfig @@ -64,4 +64,4 @@ CONFIG_SPI=y CONFIG_DM_SPI=y CONFIG_MTK_SPIM=y CONFIG_HEXDUMP=y -CONFIG_LMB_MAX_REGIONS=64 +CONFIG_LMB_MAX_AREAS=64 diff --git a/configs/mt7981_sd_rfb_defconfig b/configs/mt7981_sd_rfb_defconfig index 85be9bbc5030..2cbac6adde83 100644 --- a/configs/mt7981_sd_rfb_defconfig +++ b/configs/mt7981_sd_rfb_defconfig @@ -62,4 +62,4 @@ CONFIG_MTK_SERIAL=y CONFIG_FAT_WRITE=y CONFIG_HEXDUMP=y # CONFIG_EFI_LOADER is not set -CONFIG_LMB_MAX_REGIONS=64 +CONFIG_LMB_MAX_AREAS=64 diff --git a/configs/mt7986_rfb_defconfig b/configs/mt7986_rfb_defconfig index ac91c93efb42..73b2cad4275e 100644 --- a/configs/mt7986_rfb_defconfig +++ b/configs/mt7986_rfb_defconfig @@ -64,4 +64,4 @@ CONFIG_SPI=y CONFIG_DM_SPI=y CONFIG_MTK_SPIM=y CONFIG_HEXDUMP=y -CONFIG_LMB_MAX_REGIONS=64 +CONFIG_LMB_MAX_AREAS=64 diff --git a/configs/mt7986a_bpir3_emmc_defconfig b/configs/mt7986a_bpir3_emmc_defconfig index 5b76512a46f6..ee8f58bdbe09 100644 --- a/configs/mt7986a_bpir3_emmc_defconfig +++ b/configs/mt7986a_bpir3_emmc_defconfig @@ -62,4 +62,4 @@ CONFIG_MTK_SERIAL=y CONFIG_FAT_WRITE=y CONFIG_HEXDUMP=y # CONFIG_EFI_LOADER is not set -CONFIG_LMB_MAX_REGIONS=64 +CONFIG_LMB_MAX_AREAS=64 diff --git a/configs/mt7986a_bpir3_sd_defconfig b/configs/mt7986a_bpir3_sd_defconfig index 36547db91423..b301a626a946 100644 --- a/configs/mt7986a_bpir3_sd_defconfig +++ b/configs/mt7986a_bpir3_sd_defconfig @@ -62,4 +62,4 @@ CONFIG_MTK_SERIAL=y
[PATCH v2 05/14] lmb: Rename LMB_MAX_REGIONS and LMB_USE_MAX_REGIONS
These are the number of areas within a region, so rename them.
Signed-off-by: Simon Glass
---
Changes in v2:
- Add new patch to rename LMB_MAX_REGIONS and LMB_USE_MAX_REGIONS
configs/stm32mp13_defconfig | 4 ++--
configs/stm32mp15_basic_defconfig | 4 ++--
configs/stm32mp15_defconfig | 4 ++--
configs/stm32mp15_trusted_defconfig | 4 ++--
include/lmb.h | 10 +-
lib/Kconfig | 6 +++---
lib/lmb.c | 4 ++--
7 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/configs/stm32mp13_defconfig b/configs/stm32mp13_defconfig
index 82b62744f6db..98fcfeb5fa61 100644
--- a/configs/stm32mp13_defconfig
+++ b/configs/stm32mp13_defconfig
@@ -74,5 +74,5 @@ CONFIG_OPTEE=y
# CONFIG_OPTEE_TA_AVB is not set
CONFIG_ERRNO_STR=y
# CONFIG_LMB_USE_MAX_REGIONS is not set
-CONFIG_LMB_MEMORY_REGIONS=2
-CONFIG_LMB_RESERVED_REGIONS=16
+CONFIG_LMB_MEMORY_AREAS=2
+CONFIG_LMB_RESERVED_AREAS=16
diff --git a/configs/stm32mp15_basic_defconfig
b/configs/stm32mp15_basic_defconfig
index 9ea5aaa7145a..7b5655d957d9 100644
--- a/configs/stm32mp15_basic_defconfig
+++ b/configs/stm32mp15_basic_defconfig
@@ -189,5 +189,5 @@ CONFIG_WDT_STM32MP=y
# CONFIG_BINMAN_FDT is not set
CONFIG_ERRNO_STR=y
# CONFIG_LMB_USE_MAX_REGIONS is not set
-CONFIG_LMB_MEMORY_REGIONS=2
-CONFIG_LMB_RESERVED_REGIONS=16
+CONFIG_LMB_MEMORY_AREAS=2
+CONFIG_LMB_RESERVED_AREAS=16
diff --git a/configs/stm32mp15_defconfig b/configs/stm32mp15_defconfig
index 4d0a81f8a871..643ec201c644 100644
--- a/configs/stm32mp15_defconfig
+++ b/configs/stm32mp15_defconfig
@@ -165,5 +165,5 @@ CONFIG_WDT_STM32MP=y
# CONFIG_BINMAN_FDT is not set
CONFIG_ERRNO_STR=y
# CONFIG_LMB_USE_MAX_REGIONS is not set
-CONFIG_LMB_MEMORY_REGIONS=2
-CONFIG_LMB_RESERVED_REGIONS=16
+CONFIG_LMB_MEMORY_AREAS=2
+CONFIG_LMB_RESERVED_AREAS=16
diff --git a/configs/stm32mp15_trusted_defconfig
b/configs/stm32mp15_trusted_defconfig
index 0a7d8624858d..2a8162a870c5 100644
--- a/configs/stm32mp15_trusted_defconfig
+++ b/configs/stm32mp15_trusted_defconfig
@@ -165,5 +165,5 @@ CONFIG_WDT_STM32MP=y
# CONFIG_BINMAN_FDT is not set
CONFIG_ERRNO_STR=y
# CONFIG_LMB_USE_MAX_REGIONS is not set
-CONFIG_LMB_MEMORY_REGIONS=2
-CONFIG_LMB_RESERVED_REGIONS=16
+CONFIG_LMB_MEMORY_AREAS=2
+CONFIG_LMB_RESERVED_AREAS=16
diff --git a/include/lmb.h b/include/lmb.h
index 92332eb63069..d963ac28d9ac 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -46,11 +46,11 @@ struct lmb_area {
*
* case 2. CONFIG_LMB_USE_MAX_REGIONS is not defined, the size of each
* region is configurated *independently* with
- * => CONFIG_LMB_MEMORY_REGIONS: struct lmb.memory_regions
- * => CONFIG_LMB_RESERVED_REGIONS: struct lmb.reserved_regions
+ * => CONFIG_LMB_MEMORY_AREAS: struct lmb.memory_regions
+ * => CONFIG_LMB_RESERVED_AREAS: struct lmb.reserved_regions
* lmb_region.region is only a pointer to the correct buffer,
* initialized in lmb_init(). This configuration is useful to manage
- * more reserved memory regions with CONFIG_LMB_RESERVED_REGIONS.
+ * more reserved memory regions with CONFIG_LMB_RESERVED_AREAS.
*/
/**
@@ -87,8 +87,8 @@ struct lmb {
struct lmb_region memory;
struct lmb_region reserved;
#if !IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
- struct lmb_area memory_areas[CONFIG_LMB_MEMORY_REGIONS];
- struct lmb_area reserved_areas[CONFIG_LMB_RESERVED_REGIONS];
+ struct lmb_area memory_areas[CONFIG_LMB_MEMORY_AREAS];
+ struct lmb_area reserved_areas[CONFIG_LMB_RESERVED_AREAS];
#endif
};
diff --git a/lib/Kconfig b/lib/Kconfig
index 42e559ad0b51..53f1332a8f83 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -1082,7 +1082,7 @@ config LMB_USE_MAX_REGIONS
Define the number of supported memory regions in the library logical
memory blocks.
This feature allow to reduce the lmb library size by using compiler
- optimization when LMB_MEMORY_REGIONS == LMB_RESERVED_REGIONS.
+ optimization when LMB_MEMORY_AREAS == LMB_RESERVED_AREAS.
config LMB_MAX_REGIONS
int "Number of memory and reserved regions in lmb lib"
@@ -1092,7 +1092,7 @@ config LMB_MAX_REGIONS
Define the number of supported regions, memory and reserved, in the
library logical memory blocks.
-config LMB_MEMORY_REGIONS
+config LMB_MEMORY_AREAS
int "Number of memory regions in lmb lib"
depends on !LMB_USE_MAX_REGIONS
default 8
@@ -1101,7 +1101,7 @@ config LMB_MEMORY_REGIONS
memory blocks.
The minimal value is CONFIG_NR_DRAM_BANKS.
-config LMB_RESERVED_REGIONS
+config LMB_RESERVED_AREAS
int "Number of reserved regions in lmb lib"
depends on !LMB_USE_MAX_REGIONS
default 8
diff --git a/lib/lmb.c b/lib/lmb.c
index 2669868f0dda..f4321d10118b 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -111,8 +111,8 @@ void lmb_init(struct lmb
[PATCH v2 04/14] lmb: Rename memory/reserved_regions to area
Rename these two fields since they are areas, not regions.
Signed-off-by: Simon Glass
---
Changes in v2:
- Add new patch to rename memory/reserved_regions to area
include/lmb.h | 8
lib/lmb.c | 4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/lmb.h b/include/lmb.h
index 9d4a05670c23..92332eb63069 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -80,15 +80,15 @@ struct lmb_region {
*
* @memory: Description of memory regions.
* @reserved: Description of reserved regions.
- * @memory_regions: Array of the memory regions (statically allocated)
- * @reserved_regions: Array of the reserved regions (statically allocated)
+ * @memory_areas: Array of the memory areas (statically allocated)
+ * @reserved_areas: Array of the reserved areas (statically allocated)
*/
struct lmb {
struct lmb_region memory;
struct lmb_region reserved;
#if !IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
- struct lmb_area memory_regions[CONFIG_LMB_MEMORY_REGIONS];
- struct lmb_area reserved_regions[CONFIG_LMB_RESERVED_REGIONS];
+ struct lmb_area memory_areas[CONFIG_LMB_MEMORY_REGIONS];
+ struct lmb_area reserved_areas[CONFIG_LMB_RESERVED_REGIONS];
#endif
};
diff --git a/lib/lmb.c b/lib/lmb.c
index 9336792df8ba..2669868f0dda 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -113,8 +113,8 @@ void lmb_init(struct lmb *lmb)
#else
lmb->memory.max = CONFIG_LMB_MEMORY_REGIONS;
lmb->reserved.max = CONFIG_LMB_RESERVED_REGIONS;
- lmb->memory.region = lmb->memory_regions;
- lmb->reserved.region = lmb->reserved_regions;
+ lmb->memory.area = lmb->memory_areas;
+ lmb->reserved.area = lmb->reserved_areas;
#endif
lmb->memory.cnt = 0;
lmb->reserved.cnt = 0;
--
2.42.0.283.g2d96d420d3-goog
[PATCH v2 03/14] lmb: Rename region array to area
Adjust this so that the struct member matches its new name. For now, some
of the comments are wrong, but it difficult to do everything at once.
Signed-off-by: Simon Glass
---
Changes in v2:
- Add new patch to rename region array to area
include/lmb.h | 14 +++---
lib/lmb.c | 112 +++---
test/cmd/bdinfo.c | 6 +--
test/lib/lmb.c| 62 -
4 files changed, 97 insertions(+), 97 deletions(-)
diff --git a/include/lmb.h b/include/lmb.h
index 4b7664f22c1d..9d4a05670c23 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -23,11 +23,11 @@ enum lmb_flags {
};
/**
- * struct lmb_area - Description of one region.
+ * struct lmb_area - Description of one area within a region
*
- * @base: Base address of the region.
- * @size: Size of the region
- * @flags: memory region attributes
+ * @base: Base address of the area
+ * @size: Size of the area
+ * @flags: memory-area attributes
*/
struct lmb_area {
phys_addr_t base;
@@ -58,15 +58,15 @@ struct lmb_area {
*
* @cnt: Number of regions.
* @max: Size of the region array, max value of cnt.
- * @region: Array of the region properties
+ * @area: Array of the areas within the region
*/
struct lmb_region {
unsigned long cnt;
unsigned long max;
#if IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
- struct lmb_area region[CONFIG_LMB_MAX_REGIONS];
+ struct lmb_area area[CONFIG_LMB_MAX_REGIONS];
#else
- struct lmb_area *region;
+ struct lmb_area *area;
#endif
};
diff --git a/lib/lmb.c b/lib/lmb.c
index ae1969893f00..9336792df8ba 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -30,10 +30,10 @@ static void lmb_dump_region(struct lmb_region *rgn, char
*name)
printf(" %s.cnt = 0x%lx / max = 0x%lx\n", name, rgn->cnt, rgn->max);
for (i = 0; i < rgn->cnt; i++) {
- base = rgn->region[i].base;
- size = rgn->region[i].size;
+ base = rgn->area[i].base;
+ size = rgn->area[i].size;
end = base + size - 1;
- flags = rgn->region[i].flags;
+ flags = rgn->area[i].flags;
printf(" %s[%d]\t[0x%llx-0x%llx], 0x%08llx bytes flags: %x\n",
name, i, base, end, size, flags);
@@ -77,10 +77,10 @@ static long lmb_addrs_adjacent(phys_addr_t base1,
phys_size_t size1,
static long lmb_regions_adjacent(struct lmb_region *rgn, unsigned long r1,
unsigned long r2)
{
- phys_addr_t base1 = rgn->region[r1].base;
- phys_size_t size1 = rgn->region[r1].size;
- phys_addr_t base2 = rgn->region[r2].base;
- phys_size_t size2 = rgn->region[r2].size;
+ phys_addr_t base1 = rgn->area[r1].base;
+ phys_size_t size1 = rgn->area[r1].size;
+ phys_addr_t base2 = rgn->area[r2].base;
+ phys_size_t size2 = rgn->area[r2].size;
return lmb_addrs_adjacent(base1, size1, base2, size2);
}
@@ -90,9 +90,9 @@ static void lmb_remove_region(struct lmb_region *rgn,
unsigned long r)
unsigned long i;
for (i = r; i < rgn->cnt - 1; i++) {
- rgn->region[i].base = rgn->region[i + 1].base;
- rgn->region[i].size = rgn->region[i + 1].size;
- rgn->region[i].flags = rgn->region[i + 1].flags;
+ rgn->area[i].base = rgn->area[i + 1].base;
+ rgn->area[i].size = rgn->area[i + 1].size;
+ rgn->area[i].flags = rgn->area[i + 1].flags;
}
rgn->cnt--;
}
@@ -101,7 +101,7 @@ static void lmb_remove_region(struct lmb_region *rgn,
unsigned long r)
static void lmb_coalesce_regions(struct lmb_region *rgn, unsigned long r1,
unsigned long r2)
{
- rgn->region[r1].size += rgn->region[r2].size;
+ rgn->area[r1].size += rgn->area[r2].size;
lmb_remove_region(rgn, r2);
}
@@ -235,18 +235,18 @@ static long lmb_add_region_flags(struct lmb_region *rgn,
phys_addr_t base,
long adjacent, i;
if (rgn->cnt == 0) {
- rgn->region[0].base = base;
- rgn->region[0].size = size;
- rgn->region[0].flags = flags;
+ rgn->area[0].base = base;
+ rgn->area[0].size = size;
+ rgn->area[0].flags = flags;
rgn->cnt = 1;
return 0;
}
/* First try and coalesce this LMB with another. */
for (i = 0; i < rgn->cnt; i++) {
- phys_addr_t rgnbase = rgn->region[i].base;
- phys_size_t rgnsize = rgn->region[i].size;
- phys_size_t rgnflags = rgn->region[i].flags;
+ phys_addr_t rgnbase = rgn->area[i].base;
+ phys_size_t rgnsize = rgn->area[i].size;
+ phys_size_t rgnflags = rgn->area[i].flags;
phys_addr_t end = base + size - 1;
phys_addr_t rgnend = rgnbase + rgnsize - 1;
[PATCH v2 02/14] lmb: Rename interior piece to area
The lmb data structures use the word 'region' to describe the region in
which the allocations happen, as well as the individual allocations in
that region. The interior structure is called lmb_property but is
described as a region.
This is quite confusing. One of the reviewers in v1 asked why we are using
a property to describe a region!
It seems better to adopt the word 'area' for the internal pieces, since an
area is smaller than a region.
As a first step to improve this, rename struct lmb_property to lmb_area.
Signed-off-by: Simon Glass
---
(no changes since v1)
include/lmb.h | 12 ++--
test/lib/lmb.c | 2 +-
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/lmb.h b/include/lmb.h
index 231b68b27d91..4b7664f22c1d 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -23,13 +23,13 @@ enum lmb_flags {
};
/**
- * struct lmb_property - Description of one region.
+ * struct lmb_area - Description of one region.
*
* @base: Base address of the region.
* @size: Size of the region
* @flags: memory region attributes
*/
-struct lmb_property {
+struct lmb_area {
phys_addr_t base;
phys_size_t size;
enum lmb_flags flags;
@@ -64,9 +64,9 @@ struct lmb_region {
unsigned long cnt;
unsigned long max;
#if IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
- struct lmb_property region[CONFIG_LMB_MAX_REGIONS];
+ struct lmb_area region[CONFIG_LMB_MAX_REGIONS];
#else
- struct lmb_property *region;
+ struct lmb_area *region;
#endif
};
@@ -87,8 +87,8 @@ struct lmb {
struct lmb_region memory;
struct lmb_region reserved;
#if !IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
- struct lmb_property memory_regions[CONFIG_LMB_MEMORY_REGIONS];
- struct lmb_property reserved_regions[CONFIG_LMB_RESERVED_REGIONS];
+ struct lmb_area memory_regions[CONFIG_LMB_MEMORY_REGIONS];
+ struct lmb_area reserved_regions[CONFIG_LMB_RESERVED_REGIONS];
#endif
};
diff --git a/test/lib/lmb.c b/test/lib/lmb.c
index 162887503588..59b140fde4ce 100644
--- a/test/lib/lmb.c
+++ b/test/lib/lmb.c
@@ -12,7 +12,7 @@
#include
#include
-static inline bool lmb_is_nomap(struct lmb_property *m)
+static inline bool lmb_is_nomap(struct lmb_area *m)
{
return m->flags & LMB_NOMAP;
}
--
2.42.0.283.g2d96d420d3-goog
[PATCH v2 01/14] lmb: Drop surplus brackets and fix code style
Use a blank line before the final return. Avoid using too many brackets
to avoid confusion about precedence. Fix some overly long lines.
Signed-off-by: Simon Glass
---
(no changes since v1)
lib/lmb.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/lib/lmb.c b/lib/lmb.c
index b2c233edb64e..ae1969893f00 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -60,7 +60,7 @@ static long lmb_addrs_overlap(phys_addr_t base1, phys_size_t
size1,
const phys_addr_t base1_end = base1 + size1 - 1;
const phys_addr_t base2_end = base2 + size2 - 1;
- return ((base1 <= base2_end) && (base2 <= base1_end));
+ return base1 <= base2_end && base2 <= base1_end;
}
static long lmb_addrs_adjacent(phys_addr_t base1, phys_size_t size1,
@@ -278,7 +278,7 @@ static long lmb_add_region_flags(struct lmb_region *rgn,
phys_addr_t base,
}
}
- if ((i < rgn->cnt - 1) && lmb_regions_adjacent(rgn, i, i + 1)) {
+ if (i < rgn->cnt - 1 && lmb_regions_adjacent(rgn, i, i + 1)) {
if (rgn->region[i].flags == rgn->region[i + 1].flags) {
lmb_coalesce_regions(rgn, i, i + 1);
coalesced++;
@@ -375,6 +375,7 @@ long lmb_free(struct lmb *lmb, phys_addr_t base,
phys_size_t size)
* beginging of the hole and add the region after hole.
*/
rgn->region[i].size = base - rgn->region[i].base;
+
return lmb_add_region_flags(rgn, end + 1, rgnend - end,
rgn->region[i].flags);
}
@@ -404,7 +405,7 @@ static long lmb_overlaps_region(struct lmb_region *rgn,
phys_addr_t base,
break;
}
- return (i < rgn->cnt) ? i : -1;
+ return i < rgn->cnt ? i : -1;
}
phys_addr_t lmb_alloc(struct lmb *lmb, phys_size_t size, ulong align)
@@ -412,7 +413,8 @@ phys_addr_t lmb_alloc(struct lmb *lmb, phys_size_t size,
ulong align)
return lmb_alloc_base(lmb, size, align, LMB_ALLOC_ANYWHERE);
}
-phys_addr_t lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align,
phys_addr_t max_addr)
+phys_addr_t lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align,
+ phys_addr_t max_addr)
{
phys_addr_t alloc;
@@ -430,7 +432,8 @@ static phys_addr_t lmb_align_down(phys_addr_t addr,
phys_size_t size)
return addr & ~(size - 1);
}
-phys_addr_t __lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align,
phys_addr_t max_addr)
+phys_addr_t __lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align,
+phys_addr_t max_addr)
{
long i, rgn;
phys_addr_t base = 0;
@@ -468,6 +471,7 @@ phys_addr_t __lmb_alloc_base(struct lmb *lmb, phys_size_t
size, ulong align, phy
base = lmb_align_down(res_base - size, align);
}
}
+
return 0;
}
@@ -494,6 +498,7 @@ phys_addr_t lmb_alloc_addr(struct lmb *lmb, phys_addr_t
base, phys_size_t size)
return base;
}
}
+
return 0;
}
@@ -521,6 +526,7 @@ phys_size_t lmb_get_free_size(struct lmb *lmb, phys_addr_t
addr)
return lmb->memory.region[lmb->memory.cnt - 1].base +
lmb->memory.region[lmb->memory.cnt - 1].size - addr;
}
+
return 0;
}
@@ -534,6 +540,7 @@ int lmb_is_reserved_flags(struct lmb *lmb, phys_addr_t
addr, int flags)
if ((addr >= lmb->reserved.region[i].base) && (addr <= upper))
return (lmb->reserved.region[i].flags & flags) == flags;
}
+
return 0;
}
--
2.42.0.283.g2d96d420d3-goog
[PATCH v2 00/14] Correct confusing lmb error message
I ran into a very confusing error message about overlapping memory. I found that the message is not correct, so this series refactors the lmb code a little to permit the real message to be displayed, which is that the internal lmb table has overflowed. It also tidies up the code a little. Feedback on the initial series showed some confusion between one type of region and another. I had the same confusion, so this series renames the inner 'region' to 'area'. Changes in v2: - Add new patch to rename region array to area - Add new patch to rename memory/reserved_regions to area - Add new patch to rename LMB_MAX_REGIONS and LMB_USE_MAX_REGIONS - Add new patch to rename LMB_MAX_REGIONS and LMB_USE_MAX_REGIONS - Add new patch to update use of region in the header file - Add new patch to update use of region in the C file Simon Glass (14): lmb: Drop surplus brackets and fix code style lmb: Rename interior piece to area lmb: Rename region array to area lmb: Rename memory/reserved_regions to area lmb: Rename LMB_MAX_REGIONS and LMB_USE_MAX_REGIONS lmb: Rename LMB_MAX_REGIONS and LMB_USE_MAX_REGIONS lmb: Update use of region in the header file lmb: Update use of region in the C file lmb: Tidy up structure access lmb: Avoid long for loop counters and function returns lmb: Change functions returning long to return int lmb: Tidy up lmb_overlaps_region() lmb: Document and tidy lmb_add_region_flags() fs: Fix a confusing error about overlapping regions configs/a3y17lte_defconfig | 2 +- configs/a5y17lte_defconfig | 2 +- configs/a7y17lte_defconfig | 2 +- configs/apple_m1_defconfig | 2 +- configs/dragonboard845c_defconfig| 2 +- configs/mt7981_emmc_rfb_defconfig| 2 +- configs/mt7981_rfb_defconfig | 2 +- configs/mt7981_sd_rfb_defconfig | 2 +- configs/mt7986_rfb_defconfig | 2 +- configs/mt7986a_bpir3_emmc_defconfig | 2 +- configs/mt7986a_bpir3_sd_defconfig | 2 +- configs/qcs404evb_defconfig | 2 +- configs/starqltechn_defconfig| 2 +- configs/stm32mp13_defconfig | 6 +- configs/stm32mp15_basic_defconfig| 6 +- configs/stm32mp15_defconfig | 6 +- configs/stm32mp15_trusted_defconfig | 6 +- configs/th1520_lpi4a_defconfig | 2 +- fs/fs.c | 7 +- include/lmb.h| 105 lib/Kconfig | 30 +-- lib/lmb.c| 349 +++ test/cmd/bdinfo.c| 6 +- test/lib/lmb.c | 150 ++-- 24 files changed, 382 insertions(+), 317 deletions(-) -- 2.42.0.283.g2d96d420d3-goog
Re: [PATCH v3 1/2] schemas: Add a schema for memory map
Hi Ard, On Thu, 31 Aug 2023 at 16:39, Ard Biesheuvel wrote: > > On Fri, 1 Sept 2023 at 00:17, Simon Glass wrote: > > > > Hi Ard, > > > > On Thu, 31 Aug 2023 at 15:48, Ard Biesheuvel wrote: > > > > > > On Thu, 31 Aug 2023 at 21:03, Simon Glass wrote: > > > > > > > > Hi Ard, > > > > > > > > On Thu, 31 Aug 2023 at 06:28, Ard Biesheuvel wrote: > > > > > > > > > > On Wed, 30 Aug 2023 at 23:11, Simon Glass wrote: > > > > > > > > > > > > Hi Ard, > > > > > > > > > > > > On Tue, 29 Aug 2023 at 15:32, Ard Biesheuvel > > > > > > wrote: > > > > > > > > > > > > > > On Tue, 29 Aug 2023 at 21:18, Simon Glass > > > > > > > wrote: > > > > > > > > > > > > > > > > Hi Ard, > > > > > > > > > > > > > > > > On Thu, 24 Aug 2023 at 03:10, Ard Biesheuvel > > > > > > > > wrote: > > > > > ... > > > > > > > > > In summary, I don't see why a non-UEFI payload would care > > > > > > > > > about UEFI > > > > > > > > > semantics for pre-existing memory reservations, or vice > > > > > > > > > versa. Note > > > > > > > > > that EDK2 will manage its own memory map, and expose it via > > > > > > > > > UEFI boot > > > > > > > > > services and not via DT. > > > > > > > > > > > > > > > > Bear in mind that one or both sides of this interface may be > > > > > > > > UEFI. > > > > > > > > There is no boot-services link between the two parts that I have > > > > > > > > outlined. > > > > > > > > > > > > > > > > > > > > > > I don't understand what this means. > > > > > > > > > > > > > > UEFI specifies how one component invokes another, and it is not > > > > > > > based > > > > > > > on a DT binding. If the second component calls UEFI boot or > > > > > > > runtime > > > > > > > services, it should be invoked in this manner. If it doesn't, > > > > > > > then it > > > > > > > doesn't care about these memory reservations (and the OS will not > > > > > > > be > > > > > > > booted via UEFI either) > > > > > > > > > > > > > > So I feel I am missing something here. Perhaps a practical example > > > > > > > would be helpful? > > > > > > > > > > > > Let's say we want to support these combinations: > > > > > > > > > > > > Platform Init -> Payload > > > > > > > > > > > > U-Boot -> Tianocore > > > > > > coreboot -> U-Boot > > > > > > Tianocore -> U-Boot > > > > > > Tianocore -> Tianocore > > > > > > U-Boot -> U-Boot > > > > > > > > > > > > Some of the above things have UEFI interfaces, some don't. But in > > > > > > the > > > > > > case of Tianocore -> Tianocore we want things to work as if it were > > > > > > Tianocore -> (its own handoff mechanism) Tiancore. > > > > > > > > > > > > > > > > If Tianocore is the payload, it is either implemented as a EFI app, in > > > > > which case it has access to EFI services, or it is not, in which case > > > > > it doesn't care about UEFI semantics of the existing reserved regions, > > > > > and it only needs to know which regions exist and which of those are > > > > > reserved. > > > > > > > > > > And I think the same applies to all other rows in your table: either > > > > > the existence of UEFI needs to be carried forward, which needs to be > > > > > done via EFI services, or it doesn't, in which case the UEFI specific > > > > > reservations can be dropped, and only reserved and available memory is > > > > > relevant. > > > > > > > > > > > Some Platform Init may create runtime code which needs to > > > > > > accessible later. > > > > > > > > > > > > > > > > But not UEFI runtime code, right? If the payload is not UEFI based, > > > > > the OS would never be able to call that runtime code unless it is > > > > > described in a different, non-UEFI way. This is fine, but it is not > > > > > UEFI so we shouldn't call it UEFI runtime memory. > > > > > > > > > > > The way I think of it is that we need to generalise the memory map a > > > > > > bit. Saying that you must use UEFI boot services to discover it is > > > > > > too > > > > > > UEFI-specific. > > > > > > > > > > > > > > > > What I am questioning is why a memory map with UEFI semantics is even > > > > > relevant when those boot services do not exist. > > > > > > > > > > Could you be more specific about why a payload would have to be aware > > > > > of the existence of UEFI boot/runtime service regions if it does not > > > > > consume the UEFI interfaces of the platform init? And if the payload > > > > > exposes UEFI services to the OS, why would it consume a memory map > > > > > with UEFI semantics rather than a simple list of memblocks and memory > > > > > reservations? > > > > > > > > It seems like you are thinking of the Payload as grub, or something > > > > like that? This is not about grub. If there are EFI boot services to > > > > be provided, they are provided by the Payload, not Platform Init. I am > > > > not that familiar with Tianocore, but if you are, perhaps think of it > > > > as splitting Tianocore into two pieces, one of which inits the silicon > > > > and the other which provides the EFI boot services.
Re: [PATCH 0/6] Attempt to enforce standard extensions for build output
Hi Tom, On Thu, 31 Aug 2023 at 13:24, Tom Rini wrote: > > On Thu, Aug 31, 2023 at 01:22:13PM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Thu, 31 Aug 2023 at 13:07, Tom Rini wrote: > > > > > > On Thu, Aug 31, 2023 at 01:01:59PM -0600, Simon Glass wrote: > > > > Hi Alper, > > > > > > > > On Thu, 31 Aug 2023 at 04:20, Alper Nebi Yasak > > > > wrote: > > > > > > > > > > On 2023-08-28 20:54 +03:00, Simon Glass wrote: > > > > > > Hi Alper, > > > > > > > > > > > > On Sun, 27 Aug 2023 at 13:17, Alper Nebi Yasak > > > > > > wrote: > > > > > >> > > > > > >> On 2023-08-24 06:02 +03:00, Simon Glass wrote: > > > > > >>> In this early stage of using binman to produce output files, we > > > > > >>> are mostly > > > > > >>> seeing people using common extensions such as '.bin' and '.rom' > > > > > >>> > > > > > >>> But unusual extensions appear in some places. > > > > > >>> > > > > > >>> We would like 'buildman -k' to keep the build outputs, but this > > > > > >>> is hard if > > > > > >>> there is no consistency as to the extension used. > > > > > >>> > > > > > >>> This series adjusts binman to enforce just 4 extensions for > > > > > >>> output images: > > > > > >>> > > > > > >>>.bin > > > > > >>>.rom > > > > > >>>.itb > > > > > >>>.img > > > > > >>> > > > > > >>> Other extensions will produce an error. With this rule observed, > > > > > >>> buildman > > > > > >>> can keep the required files. > > > > > >> > > > > > >> I dislike this limitation. We know what files we will generate, > > > > > >> they are > > > > > >> listed in binman dtb, so we can add something like `binman build > > > > > >> --ls` > > > > > >> to print their names/paths for buildman to preserve them. > > > > > > > > > > > > Yes, it would be good to have that... > > > > > > > > > > > > But why do you dislike the limitation? Do you think extensions > > > > > > provide > > > > > > useful information? I suppose one problem is that *.bin might pick > > > > > > up > > > > > > private blobs that happen to be in the source directory? > > > > > > > > > > I guess my strongest argument is that people already/will have > > > > > workflows > > > > > that depend on certain filenames or extensions, and shouldn't have to > > > > > modify binman code (consider that people may be using the PyPI wheels) > > > > > to accommodate those when we are already putting the name in the dtb. > > > > > > > > > > I do want some standardization to the U-Boot build outputs though, but > > > > > more like a global binman.dts with like u-boot.rom, u-boot-sdmmc.img > > > > > images with well-defined TPL, SPL, U-Boot sections that arch-dtsi > > > > > files > > > > > can fill in. > > > > > > > > > > >> Regarding the output directory suggestion, I think the binman > > > > > >> outputs > > > > > >> (not temporary/intermediate files) should be in the same directory > > > > > >> as > > > > > >> make outputs > > > > > > > > > > > > Agreed > > > > > > > > > > > >> , and the Makefile should default to O=build to achieve the > > > > > >> "output dir". I'm not sure if that's going to happen. > > > > > > > > > > > > I would quite like the 'non-output' file (i.e. things that are not a > > > > > > binman image) to appear in a 'binman-work' subdir of the output dir. > > > > > > What do you think? > > > > > > > > > > I'd prefer them to go in a tempfile.TemporaryDirectory() and discarded > > > > > at the end of a binman run, and a "--tmpdir " option to use a > > > > > given directory instead and preserve things for debugging. > > > > > > > > Actually, that was the original purpose of the output directory in the > > > > u_boot_pylib.tools module. But with all the files binman creates, that > > > > has been lost. I think we should restore that purpose (with an output > > > > dir as a temp dir) and then I think what you have written here will > > > > work. > > > > > > Just please note the rest of the feedback that's been given to this > > > series too. > > > > Yes. It is a bit hard to know what to do. I suppose we need binman to > > publish a list of the image files it writes, so buildman can use that > > to save them. Or we could just add some more special cases to the rule > > in buildman? > > Extend the list of files that we do keep today, so that at least more > cases could work, set aside the "everything else is an error" notion for > now. OK got it, thanks. - Simon
Re: [PATCH 7/7] fs: Fix a confusing error about overlapping regions
Hi Tom,
On Wed, 23 Aug 2023 at 09:14, Tom Rini wrote:
>
> On Wed, Aug 23, 2023 at 07:42:03AM -0600, Simon Glass wrote:
>
> > When lmb runs out of space in its internal tables, it gives errors on
> > every fs load operation. This is horribly confusing, as the poor user
> > tries different memory regions one at a time.
> >
> > Use the updated API to check the error code and print a helpful message.
> > Also, allow the operation to proceed.
> >
> > Update the tests accordingly.
> >
> > Signed-off-by: Simon Glass
> [snip]
> > + if (ret == -E2BIG) {
> > + log_warning("Reservation tables exhausted: see
> > CONFIG_LMB_USE_MAX_REGIONS\n");
> > + return 0;
> > + }
>
> This isn't the right option. Everyone has CONFIG_LMB_USE_MAX_REGIONS
> set. You would want to increase CONFIG_LMB_MAX_REGIONS.
>
> But it sounds like what you've found and fixed is an underlying problem
> as to why 16 regions isn't enough in some common cases now? So we could
I don't think I have fixed anything. But I'll send v2 and perhaps it
will be clearer what is going on here.
> possibly avoid the string size growth here and have a comment that also
> matches up with the help under LMB_MAX_REGIONS?
I don't know, sorry. The size of struct(lmb) on 64-bit sandbox is
about 400 bytes. There seems to be a lot of code to save not much
memory.
Regards,
Simom
[PATCH v2 3/3] phy: Refactor generic_{setup, shutdown}_phy() to reduce complexity
Refactor generic_{setup,shutdown}_phy() to reduce complexity and
indentation. This have no intended functional change.
Signed-off-by: Jonas Karlman
---
v2:
- Split code refactor into own patch
drivers/phy/phy-uclass.c | 41
1 file changed, 16 insertions(+), 25 deletions(-)
diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c
index 343c23cead0c..190108e04c7f 100644
--- a/drivers/phy/phy-uclass.c
+++ b/drivers/phy/phy-uclass.c
@@ -510,44 +510,35 @@ int generic_phy_power_off_bulk(struct phy_bulk *bulk)
int generic_setup_phy(struct udevice *dev, struct phy *phy, int index)
{
- int ret = 0;
-
- if (!phy)
- return 0;
+ int ret;
ret = generic_phy_get_by_index(dev, index, phy);
- if (ret) {
- if (ret == -ENOENT)
- return 0;
- } else {
- ret = generic_phy_init(phy);
- if (ret)
- return ret;
+ if (ret)
+ return ret == -ENOENT ? 0 : ret;
- ret = generic_phy_power_on(phy);
- if (ret)
- generic_phy_exit(phy);
- }
+ ret = generic_phy_init(phy);
+ if (ret)
+ return ret;
+
+ ret = generic_phy_power_on(phy);
+ if (ret)
+ generic_phy_exit(phy);
return ret;
}
int generic_shutdown_phy(struct phy *phy)
{
- int ret = 0;
+ int ret;
- if (!phy)
+ if (!generic_phy_valid(phy))
return 0;
- if (generic_phy_valid(phy)) {
- ret = generic_phy_power_off(phy);
- if (ret)
- return ret;
-
- ret = generic_phy_exit(phy);
- }
+ ret = generic_phy_power_off(phy);
+ if (ret)
+ return ret;
- return ret;
+ return generic_phy_exit(phy);
}
UCLASS_DRIVER(phy) = {
--
2.42.0
[PATCH v2 2/3] phy: Return success from generic_setup_phy() when phy is not found
Restore the old behavior of ehci_setup_phy() and ohci_setup_phy() to
return success when generic_phy_get_by_index() return -ENOENT.
Fixes: 84e561407a5f ("phy: Add generic_{setup,shutdown}_phy() helpers")
Fixes: 10005004db73 ("usb: ohci: Make usage of generic_{setup,shutdown}_phy()
helpers")
Fixes: 083f8aa978a8 ("usb: ehci: Make usage of generic_{setup,shutdown}_phy()
helpers")
Fixes: 75341e9c16aa ("usb: ehci: Remove unused ehci_{setup,shutdown}_phy()
helpers")
Signed-off-by: Jonas Karlman
---
v2:
- New patch to fix regression introduced in v2023.01-rc1
drivers/phy/phy-uclass.c | 4 ++--
test/dm/phy.c| 5 +
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c
index d745e7babc12..343c23cead0c 100644
--- a/drivers/phy/phy-uclass.c
+++ b/drivers/phy/phy-uclass.c
@@ -517,8 +517,8 @@ int generic_setup_phy(struct udevice *dev, struct phy *phy,
int index)
ret = generic_phy_get_by_index(dev, index, phy);
if (ret) {
- if (ret != -ENOENT)
- return ret;
+ if (ret == -ENOENT)
+ return 0;
} else {
ret = generic_phy_init(phy);
if (ret)
diff --git a/test/dm/phy.c b/test/dm/phy.c
index 4da4841f40f7..4f91abca3a02 100644
--- a/test/dm/phy.c
+++ b/test/dm/phy.c
@@ -255,6 +255,11 @@ static int dm_test_phy_setup(struct unit_test_state *uts)
ut_asserteq(-EIO, generic_setup_phy(parent, , 2));
ut_assertok(generic_shutdown_phy());
+ /* generic_phy_get_by_index fail with -ENOENT */
+ ut_asserteq(-ENOENT, generic_phy_get_by_index(parent, 3, ));
+ ut_assertok(generic_setup_phy(parent, , 3));
+ ut_assertok(generic_shutdown_phy());
+
return 0;
}
DM_TEST(dm_test_phy_setup, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
--
2.42.0
[PATCH v2 1/3] phy: Fix generic_setup_phy() return value on power on failure
generic_phy_exit() typically return 0 for a struct phy that has been
initialized with a generic_phy_init() call.
generic_setup_phy() returns the value from a generic_phy_exit() call
when generic_phy_power_on() fails. This hides the failed state of the
power_on ops from the caller of generic_setup_phy().
Fix this by ignoring the return value of the generic_phy_exit() call and
return the value from the generic_phy_power_on() call.
Fixes: 84e561407a5f ("phy: Add generic_{setup,shutdown}_phy() helpers")
Signed-off-by: Jonas Karlman
---
v2:
- Move refactor to own patch
- Add unit test
drivers/phy/phy-uclass.c | 2 +-
test/dm/phy.c| 24
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c
index 7d707c022934..d745e7babc12 100644
--- a/drivers/phy/phy-uclass.c
+++ b/drivers/phy/phy-uclass.c
@@ -526,7 +526,7 @@ int generic_setup_phy(struct udevice *dev, struct phy *phy,
int index)
ret = generic_phy_power_on(phy);
if (ret)
- ret = generic_phy_exit(phy);
+ generic_phy_exit(phy);
}
return ret;
diff --git a/test/dm/phy.c b/test/dm/phy.c
index 2abd27b22d6d..4da4841f40f7 100644
--- a/test/dm/phy.c
+++ b/test/dm/phy.c
@@ -234,3 +234,27 @@ static int dm_test_phy_multi_exit(struct unit_test_state
*uts)
return 0;
}
DM_TEST(dm_test_phy_multi_exit, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+
+static int dm_test_phy_setup(struct unit_test_state *uts)
+{
+ struct phy phy;
+ struct udevice *parent;
+
+ ut_assertok(uclass_get_device_by_name(UCLASS_SIMPLE_BUS,
+ "gen_phy_user", ));
+
+ /* normal */
+ ut_assertok(generic_setup_phy(parent, , 0));
+ ut_assertok(generic_shutdown_phy());
+
+ /* power_off fail with -EIO */
+ ut_assertok(generic_setup_phy(parent, , 1));
+ ut_asserteq(-EIO, generic_shutdown_phy());
+
+ /* power_on fail with -EIO */
+ ut_asserteq(-EIO, generic_setup_phy(parent, , 2));
+ ut_assertok(generic_shutdown_phy());
+
+ return 0;
+}
+DM_TEST(dm_test_phy_setup, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
--
2.42.0
[PATCH v2 0/3] phy: Fix generic_setup_phy return value on power on failure
This series fixes two issues related to the return value in case of
error from generic_setup_phy().
Patch 1 fixes that the error code from power_on gets returned.
Patch 2 fixes an regression so that success is returned instead of ENOENT.
Patch 3 refactor generic_{setup,shutdown}_phy() to improve readability.
Changes in v2:
- Split patch in two
- Restore old behavior for ENOENT
- Add unit tests
Jonas Karlman (3):
phy: Fix generic_setup_phy() return value on power on failure
phy: Return success from generic_setup_phy() when phy is not found
phy: Refactor generic_{setup,shutdown}_phy() to reduce complexity
drivers/phy/phy-uclass.c | 41
test/dm/phy.c| 29
2 files changed, 45 insertions(+), 25 deletions(-)
--
2.42.0
Re: [PATCH v3 1/2] schemas: Add a schema for memory map
On Fri, 1 Sept 2023 at 00:17, Simon Glass wrote: > > Hi Ard, > > On Thu, 31 Aug 2023 at 15:48, Ard Biesheuvel wrote: > > > > On Thu, 31 Aug 2023 at 21:03, Simon Glass wrote: > > > > > > Hi Ard, > > > > > > On Thu, 31 Aug 2023 at 06:28, Ard Biesheuvel wrote: > > > > > > > > On Wed, 30 Aug 2023 at 23:11, Simon Glass wrote: > > > > > > > > > > Hi Ard, > > > > > > > > > > On Tue, 29 Aug 2023 at 15:32, Ard Biesheuvel wrote: > > > > > > > > > > > > On Tue, 29 Aug 2023 at 21:18, Simon Glass wrote: > > > > > > > > > > > > > > Hi Ard, > > > > > > > > > > > > > > On Thu, 24 Aug 2023 at 03:10, Ard Biesheuvel > > > > > > > wrote: > > > > ... > > > > > > > > In summary, I don't see why a non-UEFI payload would care about > > > > > > > > UEFI > > > > > > > > semantics for pre-existing memory reservations, or vice versa. > > > > > > > > Note > > > > > > > > that EDK2 will manage its own memory map, and expose it via > > > > > > > > UEFI boot > > > > > > > > services and not via DT. > > > > > > > > > > > > > > Bear in mind that one or both sides of this interface may be UEFI. > > > > > > > There is no boot-services link between the two parts that I have > > > > > > > outlined. > > > > > > > > > > > > > > > > > > > I don't understand what this means. > > > > > > > > > > > > UEFI specifies how one component invokes another, and it is not > > > > > > based > > > > > > on a DT binding. If the second component calls UEFI boot or runtime > > > > > > services, it should be invoked in this manner. If it doesn't, then > > > > > > it > > > > > > doesn't care about these memory reservations (and the OS will not be > > > > > > booted via UEFI either) > > > > > > > > > > > > So I feel I am missing something here. Perhaps a practical example > > > > > > would be helpful? > > > > > > > > > > Let's say we want to support these combinations: > > > > > > > > > > Platform Init -> Payload > > > > > > > > > > U-Boot -> Tianocore > > > > > coreboot -> U-Boot > > > > > Tianocore -> U-Boot > > > > > Tianocore -> Tianocore > > > > > U-Boot -> U-Boot > > > > > > > > > > Some of the above things have UEFI interfaces, some don't. But in the > > > > > case of Tianocore -> Tianocore we want things to work as if it were > > > > > Tianocore -> (its own handoff mechanism) Tiancore. > > > > > > > > > > > > > If Tianocore is the payload, it is either implemented as a EFI app, in > > > > which case it has access to EFI services, or it is not, in which case > > > > it doesn't care about UEFI semantics of the existing reserved regions, > > > > and it only needs to know which regions exist and which of those are > > > > reserved. > > > > > > > > And I think the same applies to all other rows in your table: either > > > > the existence of UEFI needs to be carried forward, which needs to be > > > > done via EFI services, or it doesn't, in which case the UEFI specific > > > > reservations can be dropped, and only reserved and available memory is > > > > relevant. > > > > > > > > > Some Platform Init may create runtime code which needs to accessible > > > > > later. > > > > > > > > > > > > > But not UEFI runtime code, right? If the payload is not UEFI based, > > > > the OS would never be able to call that runtime code unless it is > > > > described in a different, non-UEFI way. This is fine, but it is not > > > > UEFI so we shouldn't call it UEFI runtime memory. > > > > > > > > > The way I think of it is that we need to generalise the memory map a > > > > > bit. Saying that you must use UEFI boot services to discover it is too > > > > > UEFI-specific. > > > > > > > > > > > > > What I am questioning is why a memory map with UEFI semantics is even > > > > relevant when those boot services do not exist. > > > > > > > > Could you be more specific about why a payload would have to be aware > > > > of the existence of UEFI boot/runtime service regions if it does not > > > > consume the UEFI interfaces of the platform init? And if the payload > > > > exposes UEFI services to the OS, why would it consume a memory map > > > > with UEFI semantics rather than a simple list of memblocks and memory > > > > reservations? > > > > > > It seems like you are thinking of the Payload as grub, or something > > > like that? This is not about grub. If there are EFI boot services to > > > be provided, they are provided by the Payload, not Platform Init. I am > > > not that familiar with Tianocore, but if you are, perhaps think of it > > > as splitting Tianocore into two pieces, one of which inits the silicon > > > and the other which provides the EFI boot services. > > > > > > Again, if you are familiar with Tianocore, it can be built either as a > > > monolithic whole, or as a coreboot Payload. The Payload part of the > > > code is (roughly) the same in each case. But the Platform Init is > > > different[1] > > > > > > > I co-maintain OVMF [including the ARM port that I created from > > scratch] as well as the ARM architecture support in
Re: [PATCH v3 1/2] schemas: Add a schema for memory map
Hi Ard, On Thu, 31 Aug 2023 at 15:48, Ard Biesheuvel wrote: > > On Thu, 31 Aug 2023 at 21:03, Simon Glass wrote: > > > > Hi Ard, > > > > On Thu, 31 Aug 2023 at 06:28, Ard Biesheuvel wrote: > > > > > > On Wed, 30 Aug 2023 at 23:11, Simon Glass wrote: > > > > > > > > Hi Ard, > > > > > > > > On Tue, 29 Aug 2023 at 15:32, Ard Biesheuvel wrote: > > > > > > > > > > On Tue, 29 Aug 2023 at 21:18, Simon Glass wrote: > > > > > > > > > > > > Hi Ard, > > > > > > > > > > > > On Thu, 24 Aug 2023 at 03:10, Ard Biesheuvel > > > > > > wrote: > > > ... > > > > > > > In summary, I don't see why a non-UEFI payload would care about > > > > > > > UEFI > > > > > > > semantics for pre-existing memory reservations, or vice versa. > > > > > > > Note > > > > > > > that EDK2 will manage its own memory map, and expose it via UEFI > > > > > > > boot > > > > > > > services and not via DT. > > > > > > > > > > > > Bear in mind that one or both sides of this interface may be UEFI. > > > > > > There is no boot-services link between the two parts that I have > > > > > > outlined. > > > > > > > > > > > > > > > > I don't understand what this means. > > > > > > > > > > UEFI specifies how one component invokes another, and it is not based > > > > > on a DT binding. If the second component calls UEFI boot or runtime > > > > > services, it should be invoked in this manner. If it doesn't, then it > > > > > doesn't care about these memory reservations (and the OS will not be > > > > > booted via UEFI either) > > > > > > > > > > So I feel I am missing something here. Perhaps a practical example > > > > > would be helpful? > > > > > > > > Let's say we want to support these combinations: > > > > > > > > Platform Init -> Payload > > > > > > > > U-Boot -> Tianocore > > > > coreboot -> U-Boot > > > > Tianocore -> U-Boot > > > > Tianocore -> Tianocore > > > > U-Boot -> U-Boot > > > > > > > > Some of the above things have UEFI interfaces, some don't. But in the > > > > case of Tianocore -> Tianocore we want things to work as if it were > > > > Tianocore -> (its own handoff mechanism) Tiancore. > > > > > > > > > > If Tianocore is the payload, it is either implemented as a EFI app, in > > > which case it has access to EFI services, or it is not, in which case > > > it doesn't care about UEFI semantics of the existing reserved regions, > > > and it only needs to know which regions exist and which of those are > > > reserved. > > > > > > And I think the same applies to all other rows in your table: either > > > the existence of UEFI needs to be carried forward, which needs to be > > > done via EFI services, or it doesn't, in which case the UEFI specific > > > reservations can be dropped, and only reserved and available memory is > > > relevant. > > > > > > > Some Platform Init may create runtime code which needs to accessible > > > > later. > > > > > > > > > > But not UEFI runtime code, right? If the payload is not UEFI based, > > > the OS would never be able to call that runtime code unless it is > > > described in a different, non-UEFI way. This is fine, but it is not > > > UEFI so we shouldn't call it UEFI runtime memory. > > > > > > > The way I think of it is that we need to generalise the memory map a > > > > bit. Saying that you must use UEFI boot services to discover it is too > > > > UEFI-specific. > > > > > > > > > > What I am questioning is why a memory map with UEFI semantics is even > > > relevant when those boot services do not exist. > > > > > > Could you be more specific about why a payload would have to be aware > > > of the existence of UEFI boot/runtime service regions if it does not > > > consume the UEFI interfaces of the platform init? And if the payload > > > exposes UEFI services to the OS, why would it consume a memory map > > > with UEFI semantics rather than a simple list of memblocks and memory > > > reservations? > > > > It seems like you are thinking of the Payload as grub, or something > > like that? This is not about grub. If there are EFI boot services to > > be provided, they are provided by the Payload, not Platform Init. I am > > not that familiar with Tianocore, but if you are, perhaps think of it > > as splitting Tianocore into two pieces, one of which inits the silicon > > and the other which provides the EFI boot services. > > > > Again, if you are familiar with Tianocore, it can be built either as a > > monolithic whole, or as a coreboot Payload. The Payload part of the > > code is (roughly) the same in each case. But the Platform Init is > > different[1] > > > > I co-maintain OVMF [including the ARM port that I created from > scratch] as well as the ARM architecture support in Tianocore, along > with a couple of platform ports for ARM boards, some of which could by > now be characterized as 'historical' (AMD Seattle, Socionext SynQuacer > and Raspberry Pi 3/4). So I think I have a pretty good handle on how > Tianocore based firmware is put together. > > Tianocore as a
[PATCH 6/6] video: rockchip: dw_mipi_dsi: Use generic_phy_valid() helper
The documentation for struct phy state that "The content of the
structure is managed solely by the PHY API and PHY drivers".
Change to use the generic_phy_valid() helper to check if phy is valid.
Fixes: b7d8d40346f2 ("video: rockchip: dw_mipi_dsi: Fix external phy existence
check")
Signed-off-by: Jonas Karlman
---
drivers/video/rockchip/dw_mipi_dsi_rockchip.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/video/rockchip/dw_mipi_dsi_rockchip.c
b/drivers/video/rockchip/dw_mipi_dsi_rockchip.c
index 0852b53ebed5..1a5ab781e3f1 100644
--- a/drivers/video/rockchip/dw_mipi_dsi_rockchip.c
+++ b/drivers/video/rockchip/dw_mipi_dsi_rockchip.c
@@ -377,7 +377,7 @@ static int dsi_phy_init(void *priv_data)
struct dw_rockchip_dsi_priv *dsi = dev_get_priv(dev);
int ret, i, vco;
- if (dsi->phy.dev) {
+ if (generic_phy_valid(>phy)) {
ret = generic_phy_configure(>phy, >phy_opts);
if (ret) {
dev_err(dsi->dsi_host,
@@ -559,7 +559,7 @@ dw_mipi_dsi_get_lane_mbps(void *priv_data, struct
display_timing *timings,
}
/* for external phy only the mipi_dphy_config is necessary */
- if (dsi->phy.dev) {
+ if (generic_phy_valid(>phy)) {
phy_mipi_dphy_get_default_config(timings->pixelclock.typ * 10
/ 8,
bpp, lanes,
>phy_opts);
@@ -859,7 +859,7 @@ static int dw_mipi_dsi_rockchip_probe(struct udevice *dev)
}
/* Get a ref clock only if not using an external phy. */
- if (priv->phy.dev) {
+ if (generic_phy_valid(>phy)) {
dev_dbg(dev, "setting priv->ref to NULL\n");
priv->ref = NULL;
--
2.42.0
[PATCH 5/6] net: zynq: Use generic_phy_valid() helper
The documentation for struct phy state that "The content of the
structure is managed solely by the PHY API and PHY drivers".
Change to use the generic_phy_valid() helper to check if phy is valid.
Fixes: 10c50b1facbf ("net: zynq: Add support for PHY configuration in SGMII
mode")
Signed-off-by: Jonas Karlman
---
drivers/net/zynq_gem.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c
index f3cdfb0275d0..3377e669f2f6 100644
--- a/drivers/net/zynq_gem.c
+++ b/drivers/net/zynq_gem.c
@@ -890,7 +890,8 @@ static int zynq_gem_probe(struct udevice *dev)
if (ret)
goto err3;
- if (priv->interface == PHY_INTERFACE_MODE_SGMII && phy.dev) {
+ if (priv->interface == PHY_INTERFACE_MODE_SGMII &&
+ generic_phy_valid()) {
if (IS_ENABLED(CONFIG_DM_ETH_PHY)) {
if (device_is_compatible(dev, "cdns,zynqmp-gem") ||
device_is_compatible(dev, "xlnx,zynqmp-gem")) {
--
2.42.0
[PATCH 4/6] scsi: ceva: Use generic_phy_valid() helper
The documentation for struct phy state that "The content of the
structure is managed solely by the PHY API and PHY drivers".
Change to use the generic_phy_valid() helper to check if phy is valid.
Fixes: f6f5451d469b ("scsi: ceva: Enable PHY and reset support")
Signed-off-by: Jonas Karlman
---
drivers/ata/sata_ceva.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/ata/sata_ceva.c b/drivers/ata/sata_ceva.c
index 47366438fdfd..7769d4f99efd 100644
--- a/drivers/ata/sata_ceva.c
+++ b/drivers/ata/sata_ceva.c
@@ -217,7 +217,7 @@ static int sata_ceva_probe(struct udevice *dev)
}
}
- if (phy.dev) {
+ if (generic_phy_valid()) {
dev_dbg(dev, "Perform PHY power on\n");
ret = generic_phy_power_on();
if (ret) {
--
2.42.0
[PATCH 3/6] usb: dwc3: Use generic_phy_valid() helper
The documentation for struct phy state that "The content of the
structure is managed solely by the PHY API and PHY drivers".
Change to use the generic_phy_valid() helper to check if phy is valid.
Also remove setting phy->dev to NULL now that generic_phy_get_by_name()
properly initialize phy->dev to NULL.
Fixes: 142d50fbce7c ("usb: dwc3: Add support for usb3-phy PHY configuration")
Signed-off-by: Jonas Karlman
---
drivers/usb/dwc3/dwc3-generic.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
index 7f0af05855ab..3997b9dbff43 100644
--- a/drivers/usb/dwc3/dwc3-generic.c
+++ b/drivers/usb/dwc3/dwc3-generic.c
@@ -541,8 +541,6 @@ int dwc3_glue_probe(struct udevice *dev)
} else if (ret != -ENOENT && ret != -ENODATA) {
debug("could not get phy (err %d)\n", ret);
return ret;
- } else {
- phy.dev = NULL;
}
glue->regs = dev_read_addr_size_index(dev, 0, >size);
@@ -555,7 +553,7 @@ int dwc3_glue_probe(struct udevice *dev)
if (ret)
return ret;
- if (phy.dev) {
+ if (generic_phy_valid()) {
ret = generic_phy_power_on();
if (ret)
return ret;
--
2.42.0
[PATCH 2/6] phy: Set phy->dev to NULL when generic_phy_get_by_index_nodev() fails
Generic phy helpers typically use generic_phy_valid() to determine if
the helper should perform its function on a passed struct phy.
generic_phy_valid() treat any struct phy having phy->dev set as valid.
With generic_phy_get_by_index_nodev() setting phy->dev to a valid struct
udevice early, there can be situations where the struct phy is returned
as valid when initialization in fact failed and returned an error.
Fix this by setting phy->dev back to NULL when any of the calls to
of_xlate ops, device_get_supply_regulator or phy_alloc_counts fail. Also
extend the dm_test_phy_base test with a test where of_xlate ops fail.
Fixes: 72e5016f878d ("drivers: phy: add generic PHY framework")
Fixes: b9688df3cbf4 ("drivers: phy: Set phy->dev to NULL when
generic_phy_get_by_index() fails")
Signed-off-by: Jonas Karlman
---
arch/sandbox/dts/test.dts | 11 +++
drivers/phy/phy-uclass.c | 1 +
test/dm/phy.c | 12 +++-
3 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index f351d5cb84b0..6be212403473 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -431,6 +431,11 @@
#phy-cells = <0>;
};
+ phy_provider3: gen_phy@3 {
+ compatible = "sandbox,phy";
+ #phy-cells = <2>;
+ };
+
gen_phy_user: gen_phy_user {
compatible = "simple-bus";
phys = <_provider0 0>, <_provider0 1>, <_provider1>;
@@ -443,6 +448,12 @@
phy-names = "phy1", "phy2";
};
+ gen_phy_user2: gen_phy_user2 {
+ compatible = "simple-bus";
+ phys = <_provider3 0 0>;
+ phy-names = "phy1";
+ };
+
some-bus {
#address-cells = <1>;
#size-cells = <0>;
diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c
index 0baf314a3471..7d707c022934 100644
--- a/drivers/phy/phy-uclass.c
+++ b/drivers/phy/phy-uclass.c
@@ -195,6 +195,7 @@ int generic_phy_get_by_index_nodev(ofnode node, int index,
struct phy *phy)
return 0;
err:
+ phy->dev = NULL;
return ret;
}
diff --git a/test/dm/phy.c b/test/dm/phy.c
index 09329b9f71f6..2abd27b22d6d 100644
--- a/test/dm/phy.c
+++ b/test/dm/phy.c
@@ -49,7 +49,7 @@ static int dm_test_phy_base(struct unit_test_state *uts)
ut_assert(phy2.dev != phy3.dev);
/* Try to get a non-existing phy */
- ut_asserteq(-ENODEV, uclass_get_device(UCLASS_PHY, 4, ));
+ ut_asserteq(-ENODEV, uclass_get_device(UCLASS_PHY, 5, ));
ut_asserteq(-ENODATA, generic_phy_get_by_name(parent,
"phy_not_existing", _method1));
ut_assert(!generic_phy_valid(_method1));
@@ -57,6 +57,16 @@ static int dm_test_phy_base(struct unit_test_state *uts)
_method2));
ut_assert(!generic_phy_valid(_method2));
+ /* Try to get a phy where of_xlate fail */
+ ut_assertok(uclass_get_device_by_name(UCLASS_SIMPLE_BUS,
+ "gen_phy_user2", ));
+ ut_asserteq(-EINVAL, generic_phy_get_by_name(parent, "phy1",
+_method1));
+ ut_assert(!generic_phy_valid(_method1));
+ ut_asserteq(-EINVAL, generic_phy_get_by_index(parent, 0,
+ _method2));
+ ut_assert(!generic_phy_valid(_method2));
+
return 0;
}
DM_TEST(dm_test_phy_base, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
--
2.42.0
[PATCH 1/6] phy: Set phy->dev to NULL when generic_phy_get_by_name() fails
generic_phy_get_by_name() does not initialize phy->dev to NULL before
returning when dev_read_stringlist_search() fails. This can lead to an
uninitialized or reused struct phy erroneously be report as valid by
generic_phy_valid().
Fix this issue by initializing phy->dev to NULL, also extend the
dm_test_phy_base test with calls to generic_phy_valid().
Fixes: b9688df3cbf4 ("drivers: phy: Set phy->dev to NULL when
generic_phy_get_by_index() fails")
Fixes: 868d58f69c7c ("usb: dwc3: Fix non-usb3 configurations")
Signed-off-by: Jonas Karlman
---
drivers/phy/phy-uclass.c | 3 +++
test/dm/phy.c| 6 ++
2 files changed, 9 insertions(+)
diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c
index 629ef3aa3de5..0baf314a3471 100644
--- a/drivers/phy/phy-uclass.c
+++ b/drivers/phy/phy-uclass.c
@@ -211,6 +211,9 @@ int generic_phy_get_by_name(struct udevice *dev, const char
*phy_name,
debug("%s(dev=%p, name=%s, phy=%p)\n", __func__, dev, phy_name, phy);
+ assert(phy);
+ phy->dev = NULL;
+
index = dev_read_stringlist_search(dev, "phy-names", phy_name);
if (index < 0) {
debug("dev_read_stringlist_search() failed: %d\n", index);
diff --git a/test/dm/phy.c b/test/dm/phy.c
index 4d4a083dd0fd..09329b9f71f6 100644
--- a/test/dm/phy.c
+++ b/test/dm/phy.c
@@ -29,7 +29,9 @@ static int dm_test_phy_base(struct unit_test_state *uts)
* Get the same phy port in 2 different ways and compare.
*/
ut_assertok(generic_phy_get_by_name(parent, "phy1", _method1));
+ ut_assert(generic_phy_valid(_method1));
ut_assertok(generic_phy_get_by_index(parent, 0, _method2));
+ ut_assert(generic_phy_valid(_method2));
ut_asserteq(phy1_method1.id, phy1_method2.id);
/*
@@ -50,6 +52,10 @@ static int dm_test_phy_base(struct unit_test_state *uts)
ut_asserteq(-ENODEV, uclass_get_device(UCLASS_PHY, 4, ));
ut_asserteq(-ENODATA, generic_phy_get_by_name(parent,
"phy_not_existing", _method1));
+ ut_assert(!generic_phy_valid(_method1));
+ ut_asserteq(-ENOENT, generic_phy_get_by_index(parent, 3,
+ _method2));
+ ut_assert(!generic_phy_valid(_method2));
return 0;
}
--
2.42.0
[PATCH 0/6] phy: Fix use of generic_phy_valid() helper
The documentation for struct phy state that "The content of the structure is managed solely by the PHY API and PHY drivers". Change to use the generic_phy_valid() helper to check if phy is valid. Patch 1-2 fixes issues where generic_phy_valid() erroneously report a phy that has failed to be initialized as valid. Patch 3-6 replace direct access to phy->dev with generic_phy_valid(). Jonas Karlman (6): phy: Set phy->dev to NULL when generic_phy_get_by_name() fails phy: Set phy->dev to NULL when generic_phy_get_by_index_nodev() fails usb: dwc3: Use generic_phy_valid() helper scsi: ceva: Use generic_phy_valid() helper net: zynq: Use generic_phy_valid() helper video: rockchip: dw_mipi_dsi: Use generic_phy_valid() helper arch/sandbox/dts/test.dts | 11 +++ drivers/ata/sata_ceva.c | 2 +- drivers/net/zynq_gem.c| 3 ++- drivers/phy/phy-uclass.c | 4 drivers/usb/dwc3/dwc3-generic.c | 4 +--- drivers/video/rockchip/dw_mipi_dsi_rockchip.c | 6 +++--- test/dm/phy.c | 18 +- 7 files changed, 39 insertions(+), 9 deletions(-) -- 2.42.0
Re: [PATCH v3 1/2] schemas: Add a schema for memory map
On Thu, 31 Aug 2023 at 21:03, Simon Glass wrote: > > Hi Ard, > > On Thu, 31 Aug 2023 at 06:28, Ard Biesheuvel wrote: > > > > On Wed, 30 Aug 2023 at 23:11, Simon Glass wrote: > > > > > > Hi Ard, > > > > > > On Tue, 29 Aug 2023 at 15:32, Ard Biesheuvel wrote: > > > > > > > > On Tue, 29 Aug 2023 at 21:18, Simon Glass wrote: > > > > > > > > > > Hi Ard, > > > > > > > > > > On Thu, 24 Aug 2023 at 03:10, Ard Biesheuvel wrote: > > ... > > > > > > In summary, I don't see why a non-UEFI payload would care about UEFI > > > > > > semantics for pre-existing memory reservations, or vice versa. Note > > > > > > that EDK2 will manage its own memory map, and expose it via UEFI > > > > > > boot > > > > > > services and not via DT. > > > > > > > > > > Bear in mind that one or both sides of this interface may be UEFI. > > > > > There is no boot-services link between the two parts that I have > > > > > outlined. > > > > > > > > > > > > > I don't understand what this means. > > > > > > > > UEFI specifies how one component invokes another, and it is not based > > > > on a DT binding. If the second component calls UEFI boot or runtime > > > > services, it should be invoked in this manner. If it doesn't, then it > > > > doesn't care about these memory reservations (and the OS will not be > > > > booted via UEFI either) > > > > > > > > So I feel I am missing something here. Perhaps a practical example > > > > would be helpful? > > > > > > Let's say we want to support these combinations: > > > > > > Platform Init -> Payload > > > > > > U-Boot -> Tianocore > > > coreboot -> U-Boot > > > Tianocore -> U-Boot > > > Tianocore -> Tianocore > > > U-Boot -> U-Boot > > > > > > Some of the above things have UEFI interfaces, some don't. But in the > > > case of Tianocore -> Tianocore we want things to work as if it were > > > Tianocore -> (its own handoff mechanism) Tiancore. > > > > > > > If Tianocore is the payload, it is either implemented as a EFI app, in > > which case it has access to EFI services, or it is not, in which case > > it doesn't care about UEFI semantics of the existing reserved regions, > > and it only needs to know which regions exist and which of those are > > reserved. > > > > And I think the same applies to all other rows in your table: either > > the existence of UEFI needs to be carried forward, which needs to be > > done via EFI services, or it doesn't, in which case the UEFI specific > > reservations can be dropped, and only reserved and available memory is > > relevant. > > > > > Some Platform Init may create runtime code which needs to accessible > > > later. > > > > > > > But not UEFI runtime code, right? If the payload is not UEFI based, > > the OS would never be able to call that runtime code unless it is > > described in a different, non-UEFI way. This is fine, but it is not > > UEFI so we shouldn't call it UEFI runtime memory. > > > > > The way I think of it is that we need to generalise the memory map a > > > bit. Saying that you must use UEFI boot services to discover it is too > > > UEFI-specific. > > > > > > > What I am questioning is why a memory map with UEFI semantics is even > > relevant when those boot services do not exist. > > > > Could you be more specific about why a payload would have to be aware > > of the existence of UEFI boot/runtime service regions if it does not > > consume the UEFI interfaces of the platform init? And if the payload > > exposes UEFI services to the OS, why would it consume a memory map > > with UEFI semantics rather than a simple list of memblocks and memory > > reservations? > > It seems like you are thinking of the Payload as grub, or something > like that? This is not about grub. If there are EFI boot services to > be provided, they are provided by the Payload, not Platform Init. I am > not that familiar with Tianocore, but if you are, perhaps think of it > as splitting Tianocore into two pieces, one of which inits the silicon > and the other which provides the EFI boot services. > > Again, if you are familiar with Tianocore, it can be built either as a > monolithic whole, or as a coreboot Payload. The Payload part of the > code is (roughly) the same in each case. But the Platform Init is > different[1] > I co-maintain OVMF [including the ARM port that I created from scratch] as well as the ARM architecture support in Tianocore, along with a couple of platform ports for ARM boards, some of which could by now be characterized as 'historical' (AMD Seattle, Socionext SynQuacer and Raspberry Pi 3/4). So I think I have a pretty good handle on how Tianocore based firmware is put together. Tianocore as a payload will expose boot services to the OS, and will provide the OS with a memory map using the UEFI APIs. But you still haven't explained why the memory description this Tianocore inherits from the Platform Init would include any UEFI boot or runtime service regions, or any of the other memory regions with UEFI semantics. TIanocore
Re: [PATCH 00/14] event: Replace some more init hooks
On Mon, 21 Aug 2023 21:16:47 -0600, Simon Glass wrote: > This series replaces some more of the init hooks in board_f.c and > board_r.c with events. Notably it converts last_state_init() over. > > It also provides a 'simple' event spy, which takes no arguments. It turns > out that this is quite a common case, so it is worth optimising for this, > to reduce code size, before events become too commonly used. > > [...] Applied to u-boot/next, thanks! -- Tom
[PATCH RESEND] sunxi: add axp313a support
There is no response from previous post, I think this patch is not reached to
suitable person and not reviewed yet.
axp313a code is a part of power/sunxi. This is needed for Mango Pi MQ-Quad
and Orange Pi Zero3.
From 9127de42691f86ef5ed22dcf5fa0b44b03288a07 Mon Sep 17 00:00:00 2001
From: SASANO Takayoshi
Date: Thu, 13 Jul 2023 20:41:40 +0900
Subject: [PATCH] add axp313a support
Signed-off-by: SASANO Takayoshi
---
arch/arm/mach-sunxi/pmic_bus.c | 4 +-
board/sunxi/board.c| 9 +-
drivers/power/Kconfig | 16 ++-
drivers/power/Makefile | 1 +
drivers/power/axp313a.c| 171 +
drivers/power/pmic/axp.c | 1 +
include/axp313a.h | 31 ++
include/axp_pmic.h | 2 +
8 files changed, 226 insertions(+), 9 deletions(-)
create mode 100644 drivers/power/axp313a.c
create mode 100644 include/axp313a.h
diff --git a/arch/arm/mach-sunxi/pmic_bus.c b/arch/arm/mach-sunxi/pmic_bus.c
index c090840637..3e7bb5a5d1 100644
--- a/arch/arm/mach-sunxi/pmic_bus.c
+++ b/arch/arm/mach-sunxi/pmic_bus.c
@@ -21,7 +21,7 @@
#define AXP209_I2C_ADDR0x34
-#define AXP305_I2C_ADDR0x36
+#define AXP305_I2C_ADDR0x36 /* AXP305 and AXP313A */
#define AXP221_CHIP_ADDR 0x68
@@ -32,7 +32,7 @@ static int pmic_i2c_address(void)
{
if (IS_ENABLED(CONFIG_AXP152_POWER))
return AXP152_I2C_ADDR;
- if (IS_ENABLED(CONFIG_AXP305_POWER))
+ if (IS_ENABLED(CONFIG_AXP305_POWER) || IS_ENABLED(CONFIG_AXP313A_POWER))
return AXP305_I2C_ADDR;
/* Other AXP2xx and AXP8xx variants */
diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index f321cd58a6..9b0069cd52 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -584,6 +584,7 @@ void sunxi_board_init(void)
#if defined CONFIG_AXP152_POWER || defined CONFIG_AXP209_POWER || \
defined CONFIG_AXP221_POWER || defined CONFIG_AXP305_POWER || \
+ defined CONFIG_AXP313A_POWER || \
defined CONFIG_AXP809_POWER || defined CONFIG_AXP818_POWER
power_failed = axp_init();
@@ -605,7 +606,8 @@ void sunxi_board_init(void)
power_failed |= axp_set_dcdc2(CONFIG_AXP_DCDC2_VOLT);
power_failed |= axp_set_dcdc3(CONFIG_AXP_DCDC3_VOLT);
#endif
-#if !defined(CONFIG_AXP209_POWER) && !defined(CONFIG_AXP818_POWER)
+#if !defined(CONFIG_AXP209_POWER) && !defined(CONFIG_AXP818_POWER) && \
+ !defined(CONFIG_AXP313A_POWER)
power_failed |= axp_set_dcdc4(CONFIG_AXP_DCDC4_VOLT);
#endif
#if defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || \
@@ -617,10 +619,11 @@ void sunxi_board_init(void)
defined CONFIG_AXP818_POWER
power_failed |= axp_set_aldo1(CONFIG_AXP_ALDO1_VOLT);
#endif
-#if !defined(CONFIG_AXP305_POWER)
+#if !defined(CONFIG_AXP305_POWER) && !defined(CONFIG_AXP313A_POWER)
power_failed |= axp_set_aldo2(CONFIG_AXP_ALDO2_VOLT);
#endif
-#if !defined(CONFIG_AXP152_POWER) && !defined(CONFIG_AXP305_POWER)
+#if !defined(CONFIG_AXP152_POWER) && !defined(CONFIG_AXP305_POWER) && \
+ !defined(CONFIG_AXP313A_POWER)
power_failed |= axp_set_aldo3(CONFIG_AXP_ALDO3_VOLT);
#endif
#ifdef CONFIG_AXP209_POWER
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 7f3b990d23..12189eec9f 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -101,6 +101,14 @@ config AXP305_POWER
Select this to enable support for the axp305 pmic found on most
H616 boards.
+config AXP313A_POWER
+ bool "axp313a pmic support"
+ select AXP_PMIC_BUS
+ select CMD_POWEROFF
+ ---help---
+ Select this to enable support for the axp313a pmic found on some
+ H616 boards.
+
config AXP809_POWER
bool "axp809 pmic support"
depends on MACH_SUN9I
@@ -143,8 +151,8 @@ config AXP_DCDC1_VOLT
config AXP_DCDC2_VOLT
int "axp pmic dcdc2 voltage"
- depends on AXP152_POWER || AXP209_POWER || AXP221_POWER || AXP809_POWER
|| AXP818_POWER
- default 900 if AXP818_POWER
+ depends on AXP152_POWER || AXP209_POWER || AXP221_POWER || AXP809_POWER
|| AXP818_POWER || AXP313A_POWER
+ default 900 if AXP818_POWER || AXP313A_POWER
default 1400 if AXP152_POWER || AXP209_POWER
default 1200 if MACH_SUN6I
default 1100 if MACH_SUN8I
@@ -161,8 +169,8 @@ config AXP_DCDC2_VOLT
config AXP_DCDC3_VOLT
int "axp pmic dcdc3 voltage"
- depends on AXP152_POWER || AXP209_POWER || AXP221_POWER || AXP809_POWER
|| AXP818_POWER
- default 900 if AXP809_POWER || AXP818_POWER
+ depends on AXP152_POWER || AXP209_POWER || AXP221_POWER || AXP809_POWER
|| AXP818_POWER || AXP313A_POWER
+ default 900 if AXP809_POWER || AXP818_POWER || AXP313A_POWER
default 1500 if AXP152_POWER
default 1250 if AXP209_POWER
default 1100 if MACH_SUN8I_R40
diff --git
Re: [PATCH v1 0/4] Port gen_compile_commands.py from Linux to U-Boot
On Thu, Aug 31, 2023 at 09:21:19PM +0200, João Marcos Costa wrote: > Hello Simon, > > Em seg., 21 de ago. de 2023 às 21:13, Simon Glass > escreveu: > > > Hi Joao, > > > > Can you also please bring over the documentation for this feature? > > > > Actually, I couldn't find any documentation per se (e.g. in > linux/Documentation) besides what > is already documented in the actual code, as in the help message, > docstrings, and comments. > > Would you have any suggestions? Can you write up a little something? It'd probably be good to have it documented in the kernel as well too, so something that can be contributed upstream (and we try and be good neighbors). -- Tom signature.asc Description: PGP signature
Re: [PATCH 0/6] Attempt to enforce standard extensions for build output
On Thu, Aug 31, 2023 at 01:22:13PM -0600, Simon Glass wrote: > Hi Tom, > > On Thu, 31 Aug 2023 at 13:07, Tom Rini wrote: > > > > On Thu, Aug 31, 2023 at 01:01:59PM -0600, Simon Glass wrote: > > > Hi Alper, > > > > > > On Thu, 31 Aug 2023 at 04:20, Alper Nebi Yasak > > > wrote: > > > > > > > > On 2023-08-28 20:54 +03:00, Simon Glass wrote: > > > > > Hi Alper, > > > > > > > > > > On Sun, 27 Aug 2023 at 13:17, Alper Nebi Yasak > > > > > wrote: > > > > >> > > > > >> On 2023-08-24 06:02 +03:00, Simon Glass wrote: > > > > >>> In this early stage of using binman to produce output files, we are > > > > >>> mostly > > > > >>> seeing people using common extensions such as '.bin' and '.rom' > > > > >>> > > > > >>> But unusual extensions appear in some places. > > > > >>> > > > > >>> We would like 'buildman -k' to keep the build outputs, but this is > > > > >>> hard if > > > > >>> there is no consistency as to the extension used. > > > > >>> > > > > >>> This series adjusts binman to enforce just 4 extensions for output > > > > >>> images: > > > > >>> > > > > >>>.bin > > > > >>>.rom > > > > >>>.itb > > > > >>>.img > > > > >>> > > > > >>> Other extensions will produce an error. With this rule observed, > > > > >>> buildman > > > > >>> can keep the required files. > > > > >> > > > > >> I dislike this limitation. We know what files we will generate, they > > > > >> are > > > > >> listed in binman dtb, so we can add something like `binman build > > > > >> --ls` > > > > >> to print their names/paths for buildman to preserve them. > > > > > > > > > > Yes, it would be good to have that... > > > > > > > > > > But why do you dislike the limitation? Do you think extensions provide > > > > > useful information? I suppose one problem is that *.bin might pick up > > > > > private blobs that happen to be in the source directory? > > > > > > > > I guess my strongest argument is that people already/will have workflows > > > > that depend on certain filenames or extensions, and shouldn't have to > > > > modify binman code (consider that people may be using the PyPI wheels) > > > > to accommodate those when we are already putting the name in the dtb. > > > > > > > > I do want some standardization to the U-Boot build outputs though, but > > > > more like a global binman.dts with like u-boot.rom, u-boot-sdmmc.img > > > > images with well-defined TPL, SPL, U-Boot sections that arch-dtsi files > > > > can fill in. > > > > > > > > >> Regarding the output directory suggestion, I think the binman outputs > > > > >> (not temporary/intermediate files) should be in the same directory as > > > > >> make outputs > > > > > > > > > > Agreed > > > > > > > > > >> , and the Makefile should default to O=build to achieve the > > > > >> "output dir". I'm not sure if that's going to happen. > > > > > > > > > > I would quite like the 'non-output' file (i.e. things that are not a > > > > > binman image) to appear in a 'binman-work' subdir of the output dir. > > > > > What do you think? > > > > > > > > I'd prefer them to go in a tempfile.TemporaryDirectory() and discarded > > > > at the end of a binman run, and a "--tmpdir " option to use a > > > > given directory instead and preserve things for debugging. > > > > > > Actually, that was the original purpose of the output directory in the > > > u_boot_pylib.tools module. But with all the files binman creates, that > > > has been lost. I think we should restore that purpose (with an output > > > dir as a temp dir) and then I think what you have written here will > > > work. > > > > Just please note the rest of the feedback that's been given to this > > series too. > > Yes. It is a bit hard to know what to do. I suppose we need binman to > publish a list of the image files it writes, so buildman can use that > to save them. Or we could just add some more special cases to the rule > in buildman? Extend the list of files that we do keep today, so that at least more cases could work, set aside the "everything else is an error" notion for now. -- Tom signature.asc Description: PGP signature
Re: [PATCH 0/6] Attempt to enforce standard extensions for build output
Hi Tom, On Thu, 31 Aug 2023 at 13:07, Tom Rini wrote: > > On Thu, Aug 31, 2023 at 01:01:59PM -0600, Simon Glass wrote: > > Hi Alper, > > > > On Thu, 31 Aug 2023 at 04:20, Alper Nebi Yasak > > wrote: > > > > > > On 2023-08-28 20:54 +03:00, Simon Glass wrote: > > > > Hi Alper, > > > > > > > > On Sun, 27 Aug 2023 at 13:17, Alper Nebi Yasak > > > > wrote: > > > >> > > > >> On 2023-08-24 06:02 +03:00, Simon Glass wrote: > > > >>> In this early stage of using binman to produce output files, we are > > > >>> mostly > > > >>> seeing people using common extensions such as '.bin' and '.rom' > > > >>> > > > >>> But unusual extensions appear in some places. > > > >>> > > > >>> We would like 'buildman -k' to keep the build outputs, but this is > > > >>> hard if > > > >>> there is no consistency as to the extension used. > > > >>> > > > >>> This series adjusts binman to enforce just 4 extensions for output > > > >>> images: > > > >>> > > > >>>.bin > > > >>>.rom > > > >>>.itb > > > >>>.img > > > >>> > > > >>> Other extensions will produce an error. With this rule observed, > > > >>> buildman > > > >>> can keep the required files. > > > >> > > > >> I dislike this limitation. We know what files we will generate, they > > > >> are > > > >> listed in binman dtb, so we can add something like `binman build --ls` > > > >> to print their names/paths for buildman to preserve them. > > > > > > > > Yes, it would be good to have that... > > > > > > > > But why do you dislike the limitation? Do you think extensions provide > > > > useful information? I suppose one problem is that *.bin might pick up > > > > private blobs that happen to be in the source directory? > > > > > > I guess my strongest argument is that people already/will have workflows > > > that depend on certain filenames or extensions, and shouldn't have to > > > modify binman code (consider that people may be using the PyPI wheels) > > > to accommodate those when we are already putting the name in the dtb. > > > > > > I do want some standardization to the U-Boot build outputs though, but > > > more like a global binman.dts with like u-boot.rom, u-boot-sdmmc.img > > > images with well-defined TPL, SPL, U-Boot sections that arch-dtsi files > > > can fill in. > > > > > > >> Regarding the output directory suggestion, I think the binman outputs > > > >> (not temporary/intermediate files) should be in the same directory as > > > >> make outputs > > > > > > > > Agreed > > > > > > > >> , and the Makefile should default to O=build to achieve the > > > >> "output dir". I'm not sure if that's going to happen. > > > > > > > > I would quite like the 'non-output' file (i.e. things that are not a > > > > binman image) to appear in a 'binman-work' subdir of the output dir. > > > > What do you think? > > > > > > I'd prefer them to go in a tempfile.TemporaryDirectory() and discarded > > > at the end of a binman run, and a "--tmpdir " option to use a > > > given directory instead and preserve things for debugging. > > > > Actually, that was the original purpose of the output directory in the > > u_boot_pylib.tools module. But with all the files binman creates, that > > has been lost. I think we should restore that purpose (with an output > > dir as a temp dir) and then I think what you have written here will > > work. > > Just please note the rest of the feedback that's been given to this > series too. Yes. It is a bit hard to know what to do. I suppose we need binman to publish a list of the image files it writes, so buildman can use that to save them. Or we could just add some more special cases to the rule in buildman? Regards, Simon
Re: [PATCH v1 0/4] Port gen_compile_commands.py from Linux to U-Boot
Hello Yannic, Em seg., 28 de ago. de 2023 às 10:09, Yannic Moog escreveu: > > Can you also add a patch to add the compile_commands.json to > .gitignore, please? > > Yannic > Absolutely. I will add such patch in the v2 series. Thanks for the suggestion. -- Atenciosamente, João Marcos Costa www.linkedin.com/in/jmarcoscosta/ https://github.com/jmarcoscosta
Re: [PATCH v1 0/4] Port gen_compile_commands.py from Linux to U-Boot
Hello Simon, Em seg., 21 de ago. de 2023 às 21:13, Simon Glass escreveu: > Hi Joao, > > Can you also please bring over the documentation for this feature? > Actually, I couldn't find any documentation per se (e.g. in linux/Documentation) besides what is already documented in the actual code, as in the help message, docstrings, and comments. Would you have any suggestions? -- Atenciosamente, João Marcos Costa www.linkedin.com/in/jmarcoscosta/ https://github.com/jmarcoscosta
Re: [PATCH 0/6] Attempt to enforce standard extensions for build output
On Thu, Aug 31, 2023 at 01:01:59PM -0600, Simon Glass wrote: > Hi Alper, > > On Thu, 31 Aug 2023 at 04:20, Alper Nebi Yasak > wrote: > > > > On 2023-08-28 20:54 +03:00, Simon Glass wrote: > > > Hi Alper, > > > > > > On Sun, 27 Aug 2023 at 13:17, Alper Nebi Yasak > > > wrote: > > >> > > >> On 2023-08-24 06:02 +03:00, Simon Glass wrote: > > >>> In this early stage of using binman to produce output files, we are > > >>> mostly > > >>> seeing people using common extensions such as '.bin' and '.rom' > > >>> > > >>> But unusual extensions appear in some places. > > >>> > > >>> We would like 'buildman -k' to keep the build outputs, but this is hard > > >>> if > > >>> there is no consistency as to the extension used. > > >>> > > >>> This series adjusts binman to enforce just 4 extensions for output > > >>> images: > > >>> > > >>>.bin > > >>>.rom > > >>>.itb > > >>>.img > > >>> > > >>> Other extensions will produce an error. With this rule observed, > > >>> buildman > > >>> can keep the required files. > > >> > > >> I dislike this limitation. We know what files we will generate, they are > > >> listed in binman dtb, so we can add something like `binman build --ls` > > >> to print their names/paths for buildman to preserve them. > > > > > > Yes, it would be good to have that... > > > > > > But why do you dislike the limitation? Do you think extensions provide > > > useful information? I suppose one problem is that *.bin might pick up > > > private blobs that happen to be in the source directory? > > > > I guess my strongest argument is that people already/will have workflows > > that depend on certain filenames or extensions, and shouldn't have to > > modify binman code (consider that people may be using the PyPI wheels) > > to accommodate those when we are already putting the name in the dtb. > > > > I do want some standardization to the U-Boot build outputs though, but > > more like a global binman.dts with like u-boot.rom, u-boot-sdmmc.img > > images with well-defined TPL, SPL, U-Boot sections that arch-dtsi files > > can fill in. > > > > >> Regarding the output directory suggestion, I think the binman outputs > > >> (not temporary/intermediate files) should be in the same directory as > > >> make outputs > > > > > > Agreed > > > > > >> , and the Makefile should default to O=build to achieve the > > >> "output dir". I'm not sure if that's going to happen. > > > > > > I would quite like the 'non-output' file (i.e. things that are not a > > > binman image) to appear in a 'binman-work' subdir of the output dir. > > > What do you think? > > > > I'd prefer them to go in a tempfile.TemporaryDirectory() and discarded > > at the end of a binman run, and a "--tmpdir " option to use a > > given directory instead and preserve things for debugging. > > Actually, that was the original purpose of the output directory in the > u_boot_pylib.tools module. But with all the files binman creates, that > has been lost. I think we should restore that purpose (with an output > dir as a temp dir) and then I think what you have written here will > work. Just please note the rest of the feedback that's been given to this series too. -- Tom signature.asc Description: PGP signature
Re: [PATCH v3 1/2] schemas: Add a schema for memory map
Hi Ard, On Thu, 31 Aug 2023 at 06:28, Ard Biesheuvel wrote: > > On Wed, 30 Aug 2023 at 23:11, Simon Glass wrote: > > > > Hi Ard, > > > > On Tue, 29 Aug 2023 at 15:32, Ard Biesheuvel wrote: > > > > > > On Tue, 29 Aug 2023 at 21:18, Simon Glass wrote: > > > > > > > > Hi Ard, > > > > > > > > On Thu, 24 Aug 2023 at 03:10, Ard Biesheuvel wrote: > ... > > > > > In summary, I don't see why a non-UEFI payload would care about UEFI > > > > > semantics for pre-existing memory reservations, or vice versa. Note > > > > > that EDK2 will manage its own memory map, and expose it via UEFI boot > > > > > services and not via DT. > > > > > > > > Bear in mind that one or both sides of this interface may be UEFI. > > > > There is no boot-services link between the two parts that I have > > > > outlined. > > > > > > > > > > I don't understand what this means. > > > > > > UEFI specifies how one component invokes another, and it is not based > > > on a DT binding. If the second component calls UEFI boot or runtime > > > services, it should be invoked in this manner. If it doesn't, then it > > > doesn't care about these memory reservations (and the OS will not be > > > booted via UEFI either) > > > > > > So I feel I am missing something here. Perhaps a practical example > > > would be helpful? > > > > Let's say we want to support these combinations: > > > > Platform Init -> Payload > > > > U-Boot -> Tianocore > > coreboot -> U-Boot > > Tianocore -> U-Boot > > Tianocore -> Tianocore > > U-Boot -> U-Boot > > > > Some of the above things have UEFI interfaces, some don't. But in the > > case of Tianocore -> Tianocore we want things to work as if it were > > Tianocore -> (its own handoff mechanism) Tiancore. > > > > If Tianocore is the payload, it is either implemented as a EFI app, in > which case it has access to EFI services, or it is not, in which case > it doesn't care about UEFI semantics of the existing reserved regions, > and it only needs to know which regions exist and which of those are > reserved. > > And I think the same applies to all other rows in your table: either > the existence of UEFI needs to be carried forward, which needs to be > done via EFI services, or it doesn't, in which case the UEFI specific > reservations can be dropped, and only reserved and available memory is > relevant. > > > Some Platform Init may create runtime code which needs to accessible later. > > > > But not UEFI runtime code, right? If the payload is not UEFI based, > the OS would never be able to call that runtime code unless it is > described in a different, non-UEFI way. This is fine, but it is not > UEFI so we shouldn't call it UEFI runtime memory. > > > The way I think of it is that we need to generalise the memory map a > > bit. Saying that you must use UEFI boot services to discover it is too > > UEFI-specific. > > > > What I am questioning is why a memory map with UEFI semantics is even > relevant when those boot services do not exist. > > Could you be more specific about why a payload would have to be aware > of the existence of UEFI boot/runtime service regions if it does not > consume the UEFI interfaces of the platform init? And if the payload > exposes UEFI services to the OS, why would it consume a memory map > with UEFI semantics rather than a simple list of memblocks and memory > reservations? It seems like you are thinking of the Payload as grub, or something like that? This is not about grub. If there are EFI boot services to be provided, they are provided by the Payload, not Platform Init. I am not that familiar with Tianocore, but if you are, perhaps think of it as splitting Tianocore into two pieces, one of which inits the silicon and the other which provides the EFI boot services. Again, if you are familiar with Tianocore, it can be built either as a monolithic whole, or as a coreboot Payload. The Payload part of the code is (roughly) the same in each case. But the Platform Init is different[1] > > Again, I am inclined to treat this as a firmware implementation > detail, and the OS must never consume this binding. But I am still > puzzled about what exact purpose it is expected to serve. It really is purely so we can mix and match Platform Init (perhaps silicon init is a more familiar term?) and the Payload. Regards, Simon [1] Of course, coreboot uses blobs which are chunks of UEFI, but that is a separate issue
Re: [PATCH 30/32] fdt: Allow the devicetree to come from a bloblist
Hi Ilias,
On Thu, 31 Aug 2023 at 01:06, Ilias Apalodimas
wrote:
>
> Hi Simon,
>
> On Wed, Aug 30, 2023 at 12:05:01PM -0600, Simon Glass wrote:
> > Standard passage provides for a bloblist to be passed from one firmware
> > phase to the next. That can be used to pass the devicetree along as well.
> > Add an option to support this.
> >
> > Tests for this will be added as part of the Universal Payload work.
> >
> > Signed-off-by: Simon Glass
> > ---
> >
> > common/bloblist.c | 2 ++
> > doc/develop/devicetree/control.rst | 3 ++
> > dts/Kconfig| 8 ++
> > include/bloblist.h | 5
> > include/fdtdec.h | 3 +-
> > lib/fdtdec.c | 44 ++
> > 6 files changed, 52 insertions(+), 13 deletions(-)
> >
> > diff --git a/common/bloblist.c b/common/bloblist.c
> > index 6f2a4577708..b07ede11cfe 100644
> > --- a/common/bloblist.c
> > +++ b/common/bloblist.c
> > @@ -48,9 +48,11 @@ static struct tag_name {
> > { BLOBLISTT_ACPI_TABLES, "ACPI tables for x86" },
> > { BLOBLISTT_SMBIOS_TABLES, "SMBIOS tables for x86" },
> > { BLOBLISTT_VBOOT_CTX, "Chrome OS vboot context" },
> > + { BLOBLISTT_CONTROL_FDT, "Control FDT" },
> >
> > /* BLOBLISTT_PROJECT_AREA */
> > { BLOBLISTT_U_BOOT_SPL_HANDOFF, "SPL hand-off" },
> > + { BLOBLISTT_VBE, "VBE" },
> > { BLOBLISTT_U_BOOT_VIDEO, "SPL video handoff" },
> >
> > /* BLOBLISTT_VENDOR_AREA */
> > diff --git a/doc/develop/devicetree/control.rst
> > b/doc/develop/devicetree/control.rst
> > index cbb65c9b177..56e00090166 100644
> > --- a/doc/develop/devicetree/control.rst
> > +++ b/doc/develop/devicetree/control.rst
> > @@ -108,6 +108,9 @@ If CONFIG_OF_BOARD is defined, a board-specific routine
> > will provide the
> > devicetree at runtime, for example if an earlier bootloader stage creates
> > it and passes it to U-Boot.
> >
> > +If CONFIG_OF_BLOBLIST is defined, the devicetree comes from a bloblist
> > passed
> > +from a previous stage.
> > +
>
> Is this bloblist in the format described in the firmware handoff [0] ?
Yes
>
> > +config OF_BLOBLIST
> > + bool "DTB is provided by a bloblist"
> > + help
> > + Select this to read the devicetree from the bloblist. This allows
> > + using a bloblist to transfer the devicetree between U-Boot phases.
> > + The devicetree is stored in the bloblist by an early phase so that
> > + U-Boot can read it.
> > +
>
> I dont think this is a good idea. We used to have 4-5 different options
> here, which we tried to clean up and ended up with two very discrete
> options. Why do we have to reintroduce a new one? Doesn't that bloblist
> have a header of some sort? The bloblist literally comes from a previous
> stage bootloader which is what OF_BOARD is here for. So why can't we just
> read the header and figure out if the magic of the bloblist matches?
No, OF_BOARD is a hack to allow boards to do what they like with the FDT.
This patch is a standard mechanism to pass the DT from one firmware
phase to the next. We have spent quite a bit of time creating a spec
for it, and we should use it.
The patches to align bloblist with the spec have been sent, but there
is a late-breaking change that we are trying to resolve. Once that is
sorted out, I will send v2 of those patches.
>
> > config OF_BOARD
> > bool "Provided by the board (e.g a previous loader) at runtime"
> > default y if SANDBOX || OF_HAS_PRIOR_STAGE
> > diff --git a/include/bloblist.h b/include/bloblist.h
> > index 080cc46a126..e16d122f4fb 100644
> > --- a/include/bloblist.h
> > +++ b/include/bloblist.h
> > @@ -103,6 +103,11 @@ enum bloblist_tag_t {
> > BLOBLISTT_ACPI_TABLES = 0x104, /* ACPI tables for x86 */
> > BLOBLISTT_SMBIOS_TABLES = 0x105, /* SMBIOS tables for x86 */
> > BLOBLISTT_VBOOT_CTX = 0x106,/* Chromium OS verified boot context
> > */
> > + /*
>
> [...]
>
> >
> [0] https://github.com/FirmwareHandoff/firmware_handoff
Regards,
Simon
Re: [PATCH 0/6] Attempt to enforce standard extensions for build output
Hi Alper, On Thu, 31 Aug 2023 at 04:20, Alper Nebi Yasak wrote: > > On 2023-08-28 20:54 +03:00, Simon Glass wrote: > > Hi Alper, > > > > On Sun, 27 Aug 2023 at 13:17, Alper Nebi Yasak > > wrote: > >> > >> On 2023-08-24 06:02 +03:00, Simon Glass wrote: > >>> In this early stage of using binman to produce output files, we are mostly > >>> seeing people using common extensions such as '.bin' and '.rom' > >>> > >>> But unusual extensions appear in some places. > >>> > >>> We would like 'buildman -k' to keep the build outputs, but this is hard if > >>> there is no consistency as to the extension used. > >>> > >>> This series adjusts binman to enforce just 4 extensions for output images: > >>> > >>>.bin > >>>.rom > >>>.itb > >>>.img > >>> > >>> Other extensions will produce an error. With this rule observed, buildman > >>> can keep the required files. > >> > >> I dislike this limitation. We know what files we will generate, they are > >> listed in binman dtb, so we can add something like `binman build --ls` > >> to print their names/paths for buildman to preserve them. > > > > Yes, it would be good to have that... > > > > But why do you dislike the limitation? Do you think extensions provide > > useful information? I suppose one problem is that *.bin might pick up > > private blobs that happen to be in the source directory? > > I guess my strongest argument is that people already/will have workflows > that depend on certain filenames or extensions, and shouldn't have to > modify binman code (consider that people may be using the PyPI wheels) > to accommodate those when we are already putting the name in the dtb. > > I do want some standardization to the U-Boot build outputs though, but > more like a global binman.dts with like u-boot.rom, u-boot-sdmmc.img > images with well-defined TPL, SPL, U-Boot sections that arch-dtsi files > can fill in. > > >> Regarding the output directory suggestion, I think the binman outputs > >> (not temporary/intermediate files) should be in the same directory as > >> make outputs > > > > Agreed > > > >> , and the Makefile should default to O=build to achieve the > >> "output dir". I'm not sure if that's going to happen. > > > > I would quite like the 'non-output' file (i.e. things that are not a > > binman image) to appear in a 'binman-work' subdir of the output dir. > > What do you think? > > I'd prefer them to go in a tempfile.TemporaryDirectory() and discarded > at the end of a binman run, and a "--tmpdir " option to use a > given directory instead and preserve things for debugging. Actually, that was the original purpose of the output directory in the u_boot_pylib.tools module. But with all the files binman creates, that has been lost. I think we should restore that purpose (with an output dir as a temp dir) and then I think what you have written here will work. Regards, Simon
Re: [PATCH v2 2/2] xilinx: board: Add support to pick bootscr flash offset/size from DT
On Thu, 31 Aug 2023 at 01:04, Michal Simek wrote:
>
> Location of bootscript in flash can be specified via /options/u-boot DT
> node by using bootscr-flash-offset and bootscr-flash-size properties.
> Values should be saved to script_offset_f and script_size_f variables.
> Variables are described in doc/develop/bootstd.rst as:
> script_offset_f
> SPI flash offset from which to load the U-Boot script, e.g. 0xffe000
>
> script_size_f
> Size of the script to load, e.g. 0x2000
>
> Both of them are used by sf_get_bootflow() in drivers/mtd/spi/sf_bootdev.c
> to identify bootscript location inside flash.
>
> Signed-off-by: Michal Simek
> ---
>
> Changes in v2:
> - Change message from printf to debug not to disturb current users
>
> board/xilinx/common/board.c | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass
>
> diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
> index b3fb6acc25a3..126f0d81a9ed 100644
> --- a/board/xilinx/common/board.c
> +++ b/board/xilinx/common/board.c
> @@ -411,6 +411,7 @@ int board_late_init_xilinx(void)
> int i, id, macid = 0;
> struct xilinx_board_description *desc;
> phys_size_t bootm_size = gd->ram_top - gd->ram_base;
> + u64 bootscr_flash_offset, bootscr_flash_size;
>
> if (!IS_ENABLED(CONFIG_MICROBLAZE)) {
> ulong scriptaddr;
> @@ -435,11 +436,19 @@ int board_late_init_xilinx(void)
> }
> }
>
> + if (!ofnode_read_bootscript_flash(_flash_offset,
> + _flash_size)) {
> + ret |= env_set_hex("script_offset_f", bootscr_flash_offset);
> + ret |= env_set_hex("script_size_f", bootscr_flash_size);
> + } else {
> + debug("!!! Please define bootscr-flash-offset via DT !!!\n");
> + ret |= env_set_hex("script_offset_f",
> + CONFIG_BOOT_SCRIPT_OFFSET);
> + }
> +
> if (IS_ENABLED(CONFIG_ARCH_ZYNQ) || IS_ENABLED(CONFIG_MICROBLAZE))
> bootm_size = min(bootm_size, (phys_size_t)(SZ_512M +
> SZ_256M));
>
> - ret |= env_set_hex("script_offset_f", CONFIG_BOOT_SCRIPT_OFFSET);
> -
> ret |= env_set_addr("bootm_low", (void *)gd->ram_base);
> ret |= env_set_addr("bootm_size", (void *)bootm_size);
>
> --
> 2.36.1
>
Re: [PATCH v2 4/4] doc: qemu: arm: Add a section on booting Linux distros
Hi Alper, On Thu, 31 Aug 2023 at 03:57, Alper Nebi Yasak wrote: > > On 2023-08-28 20:54 +03:00, Simon Glass wrote: > > Hi Alper, > > > > On Sun, 27 Aug 2023 at 09:49, Alper Nebi Yasak > > wrote: > >> > >> On 2023-08-15 01:43 +03:00, Simon Glass wrote: > >>> Hi Alper, > >>> > >>> On Mon, 14 Aug 2023 at 11:40, Alper Nebi Yasak > >>> wrote: > I actually want to put the root.img device first so that the VM can boot > into the installed system when it reboots, but U-Boot can't find the > bootflow on the second drive. I tried e.g. `bootdev list -p; bootflow > scan -lab`, but it seems to only ever search the first virtio-blk. > However, `eficonfig; bootefi bootmgr` makes it boot somehow. > >>> > >>> Yes, this is annoying. > >>> > >>> The problem is that the scanning is getting so complicated. The > >>> boot_targets var lists things which can either be a uclass, or a > >>> uclass with a device number. The currently implementation sees > >>> "virtio" and moves on to the next thing in boot_targets once it has > >>> processed one virtio device. That is obviously not correct, but... > >>> > >>> Would it be possible to just drop the 'boot_targets' var? That is what > >>> is stuffing it up, but we don't actually need it now. The defaults end > >>> up doing the same thing, apart (perhaps) from the strangeness of > >>> virtio which can be both a network and a blk device. > >>> > >>> I believe it is possible to do the right thing, but I'll need to > >>> create a better test mechanism to handle all the cases. > >> > >> I think some kind of a "priority score" could help -- iterate over > >> boot_targets first just to generate bootdev priorities, then carry them > >> over as bootflows are discovered (adjusted for bootmeth order), then use > >> those scores as the order to boot / to show a sorted menu / to test? > > > > We sort-of have that, in that bootdevs have a priority. We could add > > something to struct bootflow which takes that but adds more > > fine-grained information, e.g. based on some sort of filter function > > that can decide? > > > > Bear in mind that we normally want to boot the first thing we find, > > and we also want to use lazy init (so no USB unless we need it). But > > yes for the case where we display a menu I think we could make more of > > the priority feature. What are you thinking of? > > For the priorities, I'm thinking of ChromiumOS boot flow where there is > an in-method order that's different from the order we encounter things > in. And I'm also thinking of customizing boot order for same-uclass > device (I guess boot_targets="usb1 usb0" works?). And a bit worried Yes > about if we would want to mix and match orders. If we say methA > methB > and dev0 > dev1, do we say methA-on-dev1 > methB-on-dev0 or the other > way? Not sure if I make sense here at all, need to think more on it. I hope not, but at least we have the data structures now and can do these sorts of things. > > In general, what I want is a graphical boot menu like rEFInd [1], but > more specifically with a better theme more like [2]. Or, at least to > support rEFInd better. (More bugs here, upstream builds don't start at > all with U-Boot, Debian builds boot on arm64 but not x86). Could you implement that as a theme on the existing 'bootflow menu'? It looks like you might be able to. > > Regarding lazy init. If we don't init USB, rEFInd can't search USB > drives to boot things from it. Not sure if that's a fundamental problem > or there's some way for EFI apps to tell firmware to init USB etc. EFI sort-of has a fundamental problem here, I believe. > > [1] https://www.rodsbooks.com/refind/ > [2] https://github.com/bobafetthotmail/refind-theme-regular > > > BTW, even for the menu, my original vision was that the menu would > > display immediately, with new bootflows appearing as they are > > discovered. > > Shifting list elements around while people are choosing from the list is > definitely not good, only way that could be OK is if the list was > append-only and not centered-in-append-direction. > > We should be displaying a priority-sorted list, I'm not sure we can > guarantee we will add things in priority order, and I would like things > centered as personal preference... Yes that is annoying, but all UIs do it these days. It is better to display something, even if you have to move an existing thing. Two other points: 1. The good news is that you are almost always adding at the end 2. This is often using the keyboard, so you can move the highlight when you insert an earlier item Regards, Simon
Re: [PATCH v2 1/4] fdt: common API to populate kaslr seed
Hi Sean,
On Tue, 29 Aug 2023 at 14:37, wrote:
>
> From: Dhananjay Phadke
>
> fdt_fixup_kaslr_seed() will update given ofnode with random seed value.
> Source for random seed can be TPM or RNG driver in u-boot or sec
> firmware (ARM).
>
> Signed-off-by: Dhananjay Phadke
> Signed-off-by: Sean Edmond
> ---
> arch/arm/cpu/armv8/sec_firmware.c | 39 +++
> common/fdt_support.c | 19 +++
> drivers/core/ofnode.c | 17 ++
> include/dm/ofnode.h | 12 ++
> include/fdt_support.h | 9 +++
> 5 files changed, 71 insertions(+), 25 deletions(-)
>
> diff --git a/arch/arm/cpu/armv8/sec_firmware.c
> b/arch/arm/cpu/armv8/sec_firmware.c
> index c0e8726346..5f04cd8aec 100644
> --- a/arch/arm/cpu/armv8/sec_firmware.c
> +++ b/arch/arm/cpu/armv8/sec_firmware.c
> @@ -411,46 +411,35 @@ int sec_firmware_init(const void *sec_firmware_img,
> /*
> * fdt_fix_kaslr - Add kalsr-seed node in Device tree
> * @fdt: Device tree
> - * @eret: 0 in case of error, 1 for success
> + * @eret: 0 for success
> */
> int fdt_fixup_kaslr(void *fdt)
Is it possible to put this code in an EVT_FT_FIXUP spy? I was rather
hoping not to add new fixup functions.
> {
> - int nodeoffset;
> - int err, ret = 0;
> - u8 rand[8];
> + int ret = 0;
>
> #if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT)
> + u8 rand[8];
> + ofnode root;
> +
> /* Check if random seed generation is supported */
> if (sec_firmware_support_hwrng() == false) {
> printf("WARNING: SEC firmware not running, no kaslr-seed\n");
> - return 0;
> + return -EOPNOTSUPP;
> }
>
> - err = sec_firmware_get_random(rand, 8);
> - if (err < 0) {
> + ret = sec_firmware_get_random(rand, 8);
> + if (ret < 0) {
> printf("WARNING: No random number to set kaslr-seed\n");
> - return 0;
> + return ret;
> }
>
> - err = fdt_check_header(fdt);
> - if (err < 0) {
> - printf("fdt_chosen: %s\n", fdt_strerror(err));
> - return 0;
> + ret = root_ofnode_from_fdt(fdt, );
> + if (ret < 0) {
> + printf("WARNING: Unable to get root ofnode\n");
> + return ret;
> }
>
> - /* find or create "/chosen" node. */
> - nodeoffset = fdt_find_or_add_subnode(fdt, 0, "chosen");
> - if (nodeoffset < 0)
> - return 0;
> -
> - err = fdt_setprop(fdt, nodeoffset, "kaslr-seed", rand,
> - sizeof(rand));
> - if (err < 0) {
> - printf("WARNING: can't set kaslr-seed %s.\n",
> - fdt_strerror(err));
> - return 0;
> - }
> - ret = 1;
> + ret = fdt_fixup_kaslr_seed(root, rand, sizeof(rand));
> #endif
>
> return ret;
> diff --git a/common/fdt_support.c b/common/fdt_support.c
> index 5e49078f8c..52be4375b4 100644
> --- a/common/fdt_support.c
> +++ b/common/fdt_support.c
> @@ -631,6 +631,25 @@ void fdt_fixup_ethernet(void *fdt)
> }
> }
>
> +int fdt_fixup_kaslr_seed(ofnode node, const u8 *seed, int len)
> +{
> + ofnode chosen;
> + int ret;
> +
> + /* find or create "/chosen" node. */
> + ret = ofnode_add_subnode(node, "chosen", );
> + if (ret && ret != -EEXIST)
> + return -ENOENT;
> +
> + ret = ofnode_write_prop(chosen, "kaslr-seed", seed, len, true);
> + if (ret) {
> + printf("WARNING: can't set kaslr-seed\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> int fdt_record_loadable(void *blob, u32 index, const char *name,
> uintptr_t load_addr, u32 size, uintptr_t entry_point,
> const char *type, const char *os, const char *arch)
> diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
> index 8df16e56af..4be21133b8 100644
> --- a/drivers/core/ofnode.c
> +++ b/drivers/core/ofnode.c
> @@ -870,6 +870,23 @@ ofnode oftree_path(oftree tree, const char *path)
> }
> }
>
> +int root_ofnode_from_fdt(void *fdt, ofnode *root_node)
> +{
> + oftree tree;
> + /* If OFNODE_MULTI_TREE is not set, and if fdt is not the control FDT,
> +* oftree_from_fdt() will return NULL
> +*/
> + tree = oftree_from_fdt(fdt);
> +
> + if (!oftree_valid(tree)) {
> + printf("Cannot create oftree\n");
> + return -EINVAL;
> + }
> + *root_node = oftree_root(tree);
> +
> + return 0;
> +}
> +
> const void *ofnode_read_chosen_prop(const char *propname, int *sizep)
> {
> ofnode chosen_node;
> diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
> index 0f38b3e736..e79bb62be8 100644
> --- a/include/dm/ofnode.h
> +++ b/include/dm/ofnode.h
> @@ -901,6 +901,18 @@
Re: [PATCH] driver: rng: Add DM_RNG interface for ARMv8.5 RNDR registers
Hi Andre,
On Thu, 31 Aug 2023 at 06:43, Andre Przywara wrote:
>
> On Wed, 30 Aug 2023 20:49:18 -0600
> Simon Glass wrote:
>
> Hi Simon,
>
> > On Wed, 30 Aug 2023 at 05:32, Andre Przywara wrote:
> > >
> > > The ARMv8.5 architecture extension defines architectural RNDR/RNDRRS
> > > system registers, that provide 64 bits worth of randomness on every
> > > read. Since it's an extension, and implementing it is optional, there is
> > > a field in the ID_AA64ISAR0_EL1 ID register to query the availability
> > > of those registers.
> > >
> > > Add a UCLASS_RNG driver that returns entropy via repeated reads from
> > > those system registers, if the extension is implemented.
> > > The driver always binds, but checks the availability in the probe()
> > > routine.
> > >
> > > This helps systems which suffer from low boot entropy, since U-Boot can
> > > provide entropy via the generic UEFI entropy gathering protocol to the OS,
> > > at an early stage.
> > >
> > > Signed-off-by: Andre Przywara
> > > ---
> > > Hi Simon,
> > >
> > > not sure if this is the expected way to model a driver which autodetects
> > > its device at runtime. It somewhat works, but always lists the bound
> > > device, even if the registers are not supported. If I do the check in bind
> > > instead, it fails booting when this returns -ENODEV, since it expects
> > > the fixed device to always bind successfully, I guess?
> > > Is there any other way of modeling this? And before you say your
> > > favourite two letters: this is not a DT job, since it can be safely
> > > auto-detected in an architectural way.
> > >
> > > Thanks,
> > > Andre
> > >
> > > arch/arm/include/asm/system.h | 1 +
> > > drivers/rng/Kconfig | 6 +++
> > > drivers/rng/Makefile | 1 +
> > > drivers/rng/arm_rndr.c| 82 +++
> > > 4 files changed, 90 insertions(+)
> > > create mode 100644 drivers/rng/arm_rndr.c
> >
> > Reviewed-by: Simon Glass
>
> thanks!
>
> > nit below
> >
> > >
> > > diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
> > > index 87d1c77e8b1..0eae857e73a 100644
> > > --- a/arch/arm/include/asm/system.h
> > > +++ b/arch/arm/include/asm/system.h
> > > @@ -84,6 +84,7 @@
> > > #define HCR_EL2_HCD_DIS(1 << 29) /* Hypervisor Call
> > > disabled */
> > > #define HCR_EL2_AMO_EL2(1 << 5) /* Route SErrors to EL2
> > > */
> > >
> > > +#define ID_AA64ISAR0_EL1_RNDR (0xFUL << 60) /* RNDR random registers */
> > > /*
> > > * ID_AA64ISAR1_EL1 bits definitions
> > > */
> > > diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig
> > > index 5deb5db5b71..72788d479cc 100644
> > > --- a/drivers/rng/Kconfig
> > > +++ b/drivers/rng/Kconfig
> > > @@ -76,6 +76,12 @@ config RNG_SMCCC_TRNG
> > > Enable random number generator for platforms that support Arm
> > > SMCCC TRNG interface.
> > >
> > > +config RNG_ARM_RNDR
> > > + bool "Generic ARMv8.5 RNDR register"
> > > + depends on DM_RNG && ARM64
> > > + help
> > > + Use the ARMv8.5 RNDR register to provide random numbers.
> > > +
> > > config TPM_RNG
> > > bool "Enable random number generator on TPM device"
> > > depends on TPM
> > > diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile
> > > index 78f61051acf..572fa7a0c66 100644
> > > --- a/drivers/rng/Makefile
> > > +++ b/drivers/rng/Makefile
> > > @@ -13,4 +13,5 @@ obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o
> > > obj-$(CONFIG_RNG_ROCKCHIP) += rockchip_rng.o
> > > obj-$(CONFIG_RNG_IPROC200) += iproc_rng200.o
> > > obj-$(CONFIG_RNG_SMCCC_TRNG) += smccc_trng.o
> > > +obj-$(CONFIG_RNG_ARM_RNDR) += arm_rndr.o
> > > obj-$(CONFIG_TPM_RNG) += tpm_rng.o
> > > diff --git a/drivers/rng/arm_rndr.c b/drivers/rng/arm_rndr.c
> > > new file mode 100644
> > > index 000..55989743eae
> > > --- /dev/null
> > > +++ b/drivers/rng/arm_rndr.c
> > > @@ -0,0 +1,82 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (c) 2023, Arm Ltd.
> > > + *
> > > + * Use the (optional) ARMv8.5 RNDR register to provide random numbers to
> > > + * U-Boot's UCLASS_RNG users.
> > > + * Detection is done at runtime using the CPU ID registers.
> > > + */
> > > +
> > > +#define LOG_CATEGORY UCLASS_RNG
> > > +
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +
> > > +#define DRIVER_NAME"arm-rndr"
> > > +
> > > +static bool cpu_has_rndr(void)
> > > +{
> > > + uint64_t reg;
> > > +
> > > + __asm__ volatile("mrs %0, ID_AA64ISAR0_EL1\n" : "=r" (reg));
> > > + return !!(reg & ID_AA64ISAR0_EL1_RNDR);
> > > +}
> > > +
> > > +/*
> > > + * The system register name is RNDR, but this isn't widely known among
> > > older
> > > + * toolchains, and also triggers errors because of it being an
> > > architecture
> > > + * extension. Since we check the availability of the register before,
> > > it's
> > > + * fine to use here,
Re: [PATCH v2 3/4] cmd: kaslrseed: Use common API to fixup FDT
Hi Sean,
On Tue, 29 Aug 2023 at 14:37, wrote:
>
> From: Sean Edmond
>
> Use the newly introduced common API fdt_fixup_kaslr_seed() in the
> kaslrseed command.
>
> Signed-off-by: Sean Edmond
> ---
> cmd/kaslrseed.c | 22 --
> 1 file changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/cmd/kaslrseed.c b/cmd/kaslrseed.c
> index 8a1d8120cd..c65607619b 100644
> --- a/cmd/kaslrseed.c
> +++ b/cmd/kaslrseed.c
> @@ -19,7 +19,7 @@ static int do_kaslr_seed(struct cmd_tbl *cmdtp, int flag,
> int argc, char *const
> size_t n = 0x8;
> struct udevice *dev;
> u64 *buf;
> - int nodeoffset;
> + ofnode root;
> int ret = CMD_RET_SUCCESS;
>
> if (uclass_get_device(UCLASS_RNG, 0, ) || !dev) {
> @@ -45,21 +45,15 @@ static int do_kaslr_seed(struct cmd_tbl *cmdtp, int flag,
> int argc, char *const
> return CMD_RET_FAILURE;
> }
>
> - ret = fdt_check_header(working_fdt);
> - if (ret < 0) {
> - printf("fdt_chosen: %s\n", fdt_strerror(ret));
> - return CMD_RET_FAILURE;
> - }
> -
> - nodeoffset = fdt_find_or_add_subnode(working_fdt, 0, "chosen");
> - if (nodeoffset < 0) {
> - printf("Reading chosen node failed\n");
> - return CMD_RET_FAILURE;
> + ret = root_ofnode_from_fdt(working_fdt, );
> + if (ret) {
> + printf("ERROR: Unable to get root ofnode\n");
> + goto CMD_RET_FAILURE;
> }
>
> - ret = fdt_setprop(working_fdt, nodeoffset, "kaslr-seed", buf,
> sizeof(buf));
> - if (ret < 0) {
> - printf("Unable to set kaslr-seed on chosen node: %s\n",
> fdt_strerror(ret));
> + ret = fdt_fixup_kaslr_seed(root, buf, sizeof(buf));
> + if (ret) {
> + printf("ERROR: failed to add kaslr-seed to fdt\n");
> return CMD_RET_FAILURE;
> }
Reviewed-by: Simon Glass
So this command is intended to be used in a script? I am just trying
to understand why we have the fixup code as well as this.
Regards,
Simon
Re: [PATCH v2 2/4] fdt: kaslr seed from tpm entropy
Hi Sean,
On Tue, 29 Aug 2023 at 14:37, wrote:
>
> From: Dhananjay Phadke
>
> Add support for KASLR seed from TPM device. Invokes tpm_get_random()
> API to read 8-bytes of random bytes for KASLR.
>
> Signed-off-by: Dhananjay Phadke
> Signed-off-by: Drew Kluemke
> Signed-off-by: Sean Edmond
> ---
> boot/image-fdt.c | 15 +++
> common/fdt_support.c | 30 ++
> include/fdt_support.h | 8
> lib/Kconfig | 9 +
> 4 files changed, 62 insertions(+)
>
> diff --git a/boot/image-fdt.c b/boot/image-fdt.c
> index f10200f647..ed38ed77b9 100644
> --- a/boot/image-fdt.c
> +++ b/boot/image-fdt.c
> @@ -624,6 +624,21 @@ int image_setup_libfdt(struct bootm_headers *images,
> void *blob,
> goto err;
> }
>
> + if (IS_ENABLED(CONFIG_KASLR_TPM_SEED)) {
> + ofnode root;
> +
> + ret = root_ofnode_from_fdt(blob, );
But can't you drop all this code and use an event spy?
> + if (ret) {
> + printf("ERROR: Unable to get root ofnode\n");
> + goto err;
> + }
> + ret = fdt_tpm_kaslr_seed(root);
This function can have a test.
> + if (ret) {
> + printf("ERROR: fdt fixup KASLR failed: %d\n", ret);
> + goto err;
> + }
> + }
> +
> fdt_ret = optee_copy_fdt_nodes(blob);
> if (fdt_ret) {
> printf("ERROR: transfer of optee nodes to new fdt failed:
> %s\n",
> diff --git a/common/fdt_support.c b/common/fdt_support.c
> index 52be4375b4..d338fcde54 100644
> --- a/common/fdt_support.c
> +++ b/common/fdt_support.c
> @@ -13,6 +13,9 @@
> #include
> #include
> #include
> +#include
> +#include
> +#include
> #include
> #include
> #include
> @@ -650,6 +653,33 @@ int fdt_fixup_kaslr_seed(ofnode node, const u8 *seed,
> int len)
> return 0;
> }
>
> +int fdt_tpm_kaslr_seed(ofnode node)
> +{
> + u8 rand[8] = {0};
> + struct udevice *dev;
> + int ret;
> +
> + ret = uclass_first_device_err(UCLASS_TPM, );
> + if (ret) {
> + printf("ERROR: Failed to find TPM device\n");
> + return ret;
> + }
> +
> + ret = tpm_get_random(dev, rand, sizeof(rand));
> + if (ret) {
> + printf("ERROR: TPM GetRandom failed, ret=%d\n", ret);
> + return ret;
> + }
> +
> + ret = fdt_fixup_kaslr_seed(node, rand, sizeof(rand));
> + if (ret) {
> + printf("ERROR: failed to add kaslr-seed to fdt\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> int fdt_record_loadable(void *blob, u32 index, const char *name,
> uintptr_t load_addr, u32 size, uintptr_t entry_point,
> const char *type, const char *os, const char *arch)
> diff --git a/include/fdt_support.h b/include/fdt_support.h
> index d967118bed..117ca14ca5 100644
> --- a/include/fdt_support.h
> +++ b/include/fdt_support.h
> @@ -130,6 +130,14 @@ void fdt_fixup_ethernet(void *fdt);
> */
> int fdt_fixup_kaslr_seed(ofnode node, const u8 *seed, int len);
>
> +/*
> + * fdt_add_tpm_kaslr_seed - Add kalsr-seed node in Device tree with random
> + * bytes from TPM device
> + * @node: ofnode
> + * @eret: 0 for success
> + */
> +int fdt_tpm_kaslr_seed(ofnode node);
> +
> int fdt_find_and_setprop(void *fdt, const char *node, const char *prop,
> const void *val, int len, int create);
> void fdt_fixup_qe_firmware(void *fdt);
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 3926652db6..1530ef7c86 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -465,6 +465,15 @@ config VPL_TPM
> for the low-level TPM interface, but only one TPM is supported at
> a time by the TPM library.
>
> +config KASLR_TPM_SEED
> + bool "Use TPM for KASLR random seed"
> + depends on TPM_V1 || TPM_V2
> + help
> + This enables support for using TPMs as entropy source for KASLR seed
> + populated in kernel's device tree. Both TPMv1 and TPMv2 are
> supported
> + for the low-level TPM interface, but only one TPM is supported at
> + a time by the library.
> +
> endmenu
>
> menu "Android Verified Boot"
> --
> 2.40.0
>
Regards,
Simon
Re: [PATCH 1/2] Allow Python packages to be dropped
Hi Tom, On Thu, 31 Aug 2023 at 11:48, Tom Rini wrote: > > On Thu, Aug 31, 2023 at 11:20:52AM -0600, Simon Glass wrote: > > > When building in a portage chroot, we do not have the environment needed > > to build pylibfdt. It is instead build as a separate package. > > > > Provide a build option to tell U-Boot to skip this part of the build. We > > still need it to use binman, etc. but don't need it to build its > > dependencies. > > > > Signed-off-by: Simon Glass > > Reviewed-by: Mike Frysinger > > --- > > > > Makefile | 8 > > Can we do this via Kconfig instead? It looks like we don't need to do > this part of the build for coreboot, yes? And so if we can drop it from > other builds as well that might help speed up CI. Yes it could. But I really don't like changing the board config on the fly - we have things like 'select BINMAN' now and it just gets messy. In the normal course of events we want to build the python things and the build won't work without it (e.g. binman will fail). But where the build environment has installed binman separate, it knows better. So I don't think this is a board config thing, > > [snip] > > +The tools-only build bytes pylibfdt by default. To disable this, use the > > "build bytes" should probably be "builds" I suspect. Yes, thanks. Regards, Simon
Re: [PATCH] efi: Correct handling of frame buffer
Hi Heinrich,
On Fri, 25 Aug 2023 at 16:06, Heinrich Schuchardt wrote:
>
> On 8/25/23 21:28, Simon Glass wrote:
> > The efi_gop driver uses private fields from the video uclass to obtain a
> > pointer to the frame buffer. Use the platform data instead.
> >
> > Check the VIDEO_COPY setting to determine which frame buffer to use. Once
> > the next stage is running (and making use of U-Boot's EFI boot services)
> > U-Boot does not handle copying from priv->fb to the hardware framebuffer,
> > so we must allow EFI to write directly to the hardware framebuffer.
>
> Let's think of a EFI application running:
>
> With your patch GOP API calls will use the hardware buffer. But text
> output by calling OutputString() and messages generated in the U-Boot
> code would write to the copy buffer. This will lead to unexpected output.
I haven't seen an EFI app writing text output and graphics output at
the same time. Probably with good reason!
>
> What triggers copying from the copy buffer to the hardware buffer?
>
> Where is the copy function implemented?
It is in vidconsole_sync_copy() at present, but Alper's series is
going to expand things a bit.
Regards,
Simon
>
> >
> > Signed-off-by: Simon Glass
> > ---
> >
> > lib/efi_loader/efi_gop.c | 12 +++-
> > 1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c
> > index 778b693f983..a09db31eb46 100644
> > --- a/lib/efi_loader/efi_gop.c
> > +++ b/lib/efi_loader/efi_gop.c
> > @@ -10,6 +10,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> >
> > @@ -467,10 +468,10 @@ efi_status_t efi_gop_register(void)
> > struct efi_gop_obj *gopobj;
> > u32 bpix, format, col, row;
> > u64 fb_base, fb_size;
> > - void *fb;
> > efi_status_t ret;
> > struct udevice *vdev;
> > struct video_priv *priv;
> > + struct video_uc_plat *plat;
> >
> > /* We only support a single video output device for now */
> > if (uclass_first_device_err(UCLASS_VIDEO, )) {
> > @@ -483,9 +484,10 @@ efi_status_t efi_gop_register(void)
> > format = priv->format;
> > col = video_get_xsize(vdev);
> > row = video_get_ysize(vdev);
> > - fb_base = (uintptr_t)priv->fb;
> > - fb_size = priv->fb_size;
> > - fb = priv->fb;
> > +
> > + plat = dev_get_uclass_plat(vdev);
> > + fb_base = IS_ENABLED(CONFIG_VIDEO_COPY) ? plat->copy_base :
> > plat->base;
> > + fb_size = plat->size;
>
> This logic and the map_sysmem() conversion would better be placed in
> video-uclass.c or video.h as a function video_get_hw_fb(void *fb_base,
> size_t *fb_size).
> >
> > switch (bpix) {
> > case VIDEO_BPP16:
> > @@ -547,7 +549,7 @@ efi_status_t efi_gop_register(void)
> > }
> > gopobj->info.pixels_per_scanline = col;
> > gopobj->bpix = bpix;
> > - gopobj->fb = fb;
> > + gopobj->fb = map_sysmem(fb_base, fb_size);
>
> Could you, please, add a description of the side effects of the 'len'
> parameter of map_sysmem() in doc/arch/sandbox/sandbox.rst. It seems to
> be ignored, see
>
> include/mapmem.h:16
> arch/sandbox/include/asm/io.h:40
>
> So maybe just remove it?
>
> Best regards
>
> Heinrich
>
> >
> > return EFI_SUCCESS;
> > }
>
Re: [PATCH] efi: Correct handling of frame buffer
Hi, On Sun, 27 Aug 2023 at 12:41, Alper Nebi Yasak wrote: > > On 2023-08-27 19:32 +03:00, Heinrich Schuchardt wrote: > > Am 27. August 2023 16:56:51 MESZ schrieb Alper Nebi Yasak > > : > >> On 2023-08-25 22:28 +03:00, Simon Glass wrote: > >>> so we must allow EFI to write directly to the hardware framebuffer. > >> > >> If you want a fix independent of that series, I think the proper > >> approach here is having EFI draw to fb as it already does, then copying > >> from that to copy_fb at the end of gop_blt_int(). > > > > An EFI app can write directly to the framebuffer and mix that with API > > calls. > > Ack. Can EFI apps write to framebuffer without informing U-Boot at all? > What I meant is, copying whenever EFI asks us to modify something or > tells us it modified something. > > If it can write without telling us, how are cache flushes and hardware > syncs handled? Asking these because I'm doing some weird experiments > with video damage and cyclic video_sync() calls and VIDEO_COPY... Yay! I'd like sync to happen there. In fact I am wondering if cyclic should have a flag indicating whether the callee has enough time to do whatever it wants (e.g. several milliseconds) or should be very fast. > > > blt functions causing a full copy would be much slower than what you get > > when eliminating the copy fb. > > When we expose copy_fb to EFI, don't we need to keep fb up to date, > incurring the same cost? The VIDEO_COPY description mentions "reading > from the frame buffer is slow", so whatever code is "reading" would use > fb and would get bad data otherwise. But I can't exactly point to any > examples other than the video damage series copying fb onto copy_fb. Look, the EFI API is not going to work so well here. but it is what it is. The purpose of this patch is to make it work. Of course it will be slow, but it isn't really noticeable to me. Regards, Simon
Re: [PATCH v2] cmd: dm: allow for selecting uclass and device
Hi,
On Wed, 30 Aug 2023 at 22:34, AKASHI Takahiro
wrote:
>
> On Wed, Aug 30, 2023 at 09:48:48PM -0600, Simon Glass wrote:
> > Hi AKASHI,
> >
> > On Tue, 22 Aug 2023 at 19:50, AKASHI Takahiro
> > wrote:
> > >
> > > The output from "dm tree" or "dm uclass" is a bit annoying
> > > if the number of devices available on the system is huge.
> > > (This is especially true on sandbox when I debug some DM code.)
> > >
> > > With this patch, we can specify the uclass name or the device
> > > name that we are interested in in order to limit the output.
> > >
> > > For instance,
> > >
> > > => dm uclass usb
> > > uclass 121: usb
> > > 0 usb@1 @ 0bcff8b0, seq 1
> > >
> > > uclass 124: usb
> > >
> > > => dm tree usb:usb@1
> > > Class Index Probed DriverName
> > > ---
> > > usb 0 [ ] usb_sandbox usb@1
> > > usb_hub 0 [ ] usb_hub `-- hub
> > > usb_emul 0 [ ] usb_sandbox_hub `-- hub-emul
> > > usb_emul 1 [ ] usb_sandbox_flash |-- flash-stick@0
> > > usb_emul 2 [ ] usb_sandbox_flash |-- flash-stick@1
> > > usb_emul 3 [ ] usb_sandbox_flash |-- flash-stick@2
> > > usb_emul 4 [ ] usb_sandbox_keyb `-- keyb@3
> > >
> > > If you want forward-matching against a uclass or udevice name,
> > > you can specify "-e" option.
> >
> > I don't really know what 'forward matching' means.
>
> Really? I believed that forward-matching was a common word.
> I meant that it searches for any string starting with
> a specific sub-string. In other words, it would be
> "^" in a regular expression.
> So, for example, "usb" should match with "usbABC", "usb-DEF",
> "usb_GHI" and so on, but not match with "ABCusb".
>
> > Please use forward instead of forword in the code
Well let's just go with what you have. We can always tweak it when
people start using it, if needed.
> >
> > >
> > > => dm uclass -e usb
> > > uclass 15: usb_emul
> > > 0 hub-emul @ 0bcffb00, seq 0
> > > 1 flash-stick@0 @ 0bcffc30, seq 1
> > > 2 flash-stick@1 @ 0bcffdc0, seq 2
> > > 3 flash-stick@2 @ 0bcfff50, seq 3
> > > 4 keyb@3 @ 0bd000e0, seq 4
> > >
> > > uclass 64: usb_mass_storage
> > >
> > > uclass 121: usb
> > > 0 usb@1 @ 0bcff8b0, seq 1
> > >
> > > uclass 122: usb_dev_generic
> > >
> > > uclass 123: usb_hub
> > > 0 hub @ 0bcff9b0, seq 0
> > >
> > > uclass 124: usb
> > >
> > > => dm tree -e usb
> > > Class Index Probed DriverName
> > > ---
> > > usb 0 [ ] usb_sandbox usb@1
> > > usb_hub 0 [ ] usb_hub `-- hub
> > > usb_emul 0 [ ] usb_sandbox_hub `-- hub-emul
> > > usb_emul 1 [ ] usb_sandbox_flash |-- flash-stick@0
> > > usb_emul 2 [ ] usb_sandbox_flash |-- flash-stick@1
> > > usb_emul 3 [ ] usb_sandbox_flash |-- flash-stick@2
> > > usb_emul 4 [ ] usb_sandbox_keyb `-- keyb@3
> > >
> > > Signed-off-by: AKASHI Takahiro
> > > ---
> > > v2
> > > - allow for forward-matching against the name
> > > - update command doc
> > > ---
> > > cmd/dm.c | 48 ++
> > > doc/usage/cmd/dm.rst | 30 ++-
> > > drivers/core/dump.c | 116 ---
> > > include/dm/util.h| 15 --
> > > 4 files changed, 165 insertions(+), 44 deletions(-)
> >
> > Reviewed-by: Simon Glass
> >
> > Thanks for doing this. It will be a big time-saver. I also wonder if
> > it would be better if the default were to do a substring search and
>
> Initially, I implemented so, but felt it is annoying to see
> (sometimes many) unexpected matched devices, especially
> when you know the exact name of device.
> See my example "dm uclass -e usb".
>
> > you have to add a flag to search for a single device?
>
> Does 'single' mean the first matched word or exactly-same one?
>
> Either way, I don't have a strong opinion here, though.
>
> Thanks,
> -Takahiro Akashi
>
> >
> > See below
> >
> > >
> > > diff --git a/cmd/dm.c b/cmd/dm.c
> > > index 3263547cbec6..1aa86aab9c1c 100644
> > > --- a/cmd/dm.c
> > > +++ b/cmd/dm.c
> > > @@ -59,11 +59,26 @@ static int do_dm_dump_static_driver_info(struct
> > > cmd_tbl *cmdtp, int flag,
> > > static int do_dm_dump_tree(struct cmd_tbl *cmdtp, int flag, int argc,
> > >char *const argv[])
> > > {
> > > - bool sort;
> > > -
> > > - sort = argc > 1 && !strcmp(argv[1], "-s");
> > > -
> > > - dm_dump_tree(sort);
> > > + bool extended = false, sort = false;
> > > + char *device = NULL;
> > > +
> > > + for (; argc > 1; argc--, argv++) {
> > > + if (argv[1][0] != '-')
> > > + break;
> > > +
> > > + if
Re: [PATCH 05/32] spl: mx6: powerpc: Drop the condition on timer_init()
On Wed, Aug 30, 2023 at 12:04:36PM -0600, Simon Glass wrote: > It doesn't make sense to have some boards do this differently. Drop the > condition in the hope that the maintainers can figure out any run-time > problems. > > Signed-off-by: Simon Glass > --- > > common/spl/spl.c | 6 -- > 1 file changed, 6 deletions(-) > > diff --git a/common/spl/spl.c b/common/spl/spl.c > index 78db9ef5318..3f513b0563a 100644 > --- a/common/spl/spl.c > +++ b/common/spl/spl.c > @@ -766,13 +766,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2) > if (spl_init()) > hang(); > } > -#if !defined(CONFIG_PPC) && !defined(CONFIG_ARCH_MX6) > - /* > - * timer_init() does not exist on PPC systems. The timer is initialized > - * and enabled (decrementer) in interrupt_init() here. > - */ > timer_init(); > -#endif PowerPC might be a little tricky, did qemu-ppce500 run? And please reach out to some of the iMX folks instead of just dropping this and hoping it works. -- Tom signature.asc Description: PGP signature
Re: [PATCH 1/2] Allow Python packages to be dropped
On Thu, Aug 31, 2023 at 11:20:52AM -0600, Simon Glass wrote: > When building in a portage chroot, we do not have the environment needed > to build pylibfdt. It is instead build as a separate package. > > Provide a build option to tell U-Boot to skip this part of the build. We > still need it to use binman, etc. but don't need it to build its > dependencies. > > Signed-off-by: Simon Glass > Reviewed-by: Mike Frysinger > --- > > Makefile | 8 Can we do this via Kconfig instead? It looks like we don't need to do this part of the build for coreboot, yes? And so if we can drop it from other builds as well that might help speed up CI. [snip] > +The tools-only build bytes pylibfdt by default. To disable this, use the "build bytes" should probably be "builds" I suspect. -- Tom signature.asc Description: PGP signature
[PATCH 2/2] x86: coreboot: Avoid a declaration after a label
Declare the global_data pointer at the top of the file, to avoid an
error:
arch/x86/include/asm/global_data.h:143:35: error: a label can
only be part of a statement and a declaration is not a statement
board/coreboot/coreboot/coreboot.c:60:2: note: in expansion of macro
‘DECLARE_GLOBAL_DATA_PTR’
Signed-off-by: Simon Glass
Reviewed-by: Mike Frysinger
---
board/coreboot/coreboot/coreboot.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/board/coreboot/coreboot/coreboot.c
b/board/coreboot/coreboot/coreboot.c
index 3b90ae75386..db855c11ae6 100644
--- a/board/coreboot/coreboot/coreboot.c
+++ b/board/coreboot/coreboot/coreboot.c
@@ -10,6 +10,8 @@
#include
#include
+DECLARE_GLOBAL_DATA_PTR;
+
int board_early_init_r(void)
{
/*
@@ -54,14 +56,12 @@ int show_board_info(void)
return 0;
fallback:
-#ifdef CONFIG_OF_CONTROL
- DECLARE_GLOBAL_DATA_PTR;
-
- model = fdt_getprop(gd->fdt_blob, 0, "model", NULL);
+ if (IS_ENABLED(CONFIG_OF_CONTROL)) {
+ model = fdt_getprop(gd->fdt_blob, 0, "model", NULL);
- if (model)
- printf("Model: %s\n", model);
-#endif
+ if (model)
+ printf("Model: %s\n", model);
+ }
return checkboard();
}
--
2.42.0.rc2.253.gd59a3bf2b4-goog
[PATCH 1/2] Allow Python packages to be dropped
When building in a portage chroot, we do not have the environment needed to build pylibfdt. It is instead build as a separate package. Provide a build option to tell U-Boot to skip this part of the build. We still need it to use binman, etc. but don't need it to build its dependencies. Signed-off-by: Simon Glass Reviewed-by: Mike Frysinger --- Makefile | 8 doc/build/tools.rst | 9 + scripts/dtc/Makefile | 2 ++ 3 files changed, 19 insertions(+) diff --git a/Makefile b/Makefile index 9b90204bfe6..25be94ef20f 100644 --- a/Makefile +++ b/Makefile @@ -646,12 +646,20 @@ export CFLAGS_NON_EFI # Compiler flags to remove when building EFI app export EFI_TARGET # binutils target if EFI is natively supported export LTO_ENABLE +export PYTHON_ENABLE # This is y if LTO is enabled for this build. See NO_LTO=1 to disable LTO ifeq ($(NO_LTO),) LTO_ENABLE=$(if $(CONFIG_LTO),y) endif +# This is y if U-Boot should not build any Python tools or libraries. Typically +# you would need to set this if those tools/libraries (typically binman and +# pylibfdt) cannot be built by your environment and are provided separately. +ifeq ($(NO_PYTHON),) +PYTHON_ENABLE=y +endif + # If board code explicitly specified LDSCRIPT or CONFIG_SYS_LDSCRIPT, use # that (or fail if absent). Otherwise, search for a linker script in a # standard location. diff --git a/doc/build/tools.rst b/doc/build/tools.rst index ec017229258..418f0abb236 100644 --- a/doc/build/tools.rst +++ b/doc/build/tools.rst @@ -45,3 +45,12 @@ Launch the MSYS2 shell of the MSYS2 environment, and do the following:: $ make tools-only_defconfig $ make tools-only + + +Building without Python +--- + +The tools-only build bytes pylibfdt by default. To disable this, use the +NO_PYTHON variable:: + + NO_PYTHON=1 make tools-only_defconfig tools-only diff --git a/scripts/dtc/Makefile b/scripts/dtc/Makefile index 58d879dd11f..faa72d95e28 100644 --- a/scripts/dtc/Makefile +++ b/scripts/dtc/Makefile @@ -19,4 +19,6 @@ HOSTCFLAGS_dtc-parser.tab.o := -I$(src) $(obj)/dtc-lexer.lex.o: $(obj)/dtc-parser.tab.h # Added for U-Boot +ifeq ($(PYTHON_ENABLE),y) subdir-$(CONFIG_PYLIBFDT) += pylibfdt +endif -- 2.42.0.rc2.253.gd59a3bf2b4-goog
[PATCH v5 8/8] cmd: gpt: Add command to swap partition order
Adds a command called "gpt transpose" which will swap the order two
partition table entries in the GPT partition table (but leaves them
pointing to the same locations on disk).
This can be useful for swapping bootloaders in systems that use an A/B
partitioning scheme where the bootrom is hard coded to look for the
bootloader in a specific index in the GPT partition table.
Signed-off-by: Joshua Watt
---
cmd/gpt.c | 46 ---
doc/usage/cmd/gpt.rst | 25 +
test/py/tests/test_gpt.py | 19
3 files changed, 87 insertions(+), 3 deletions(-)
diff --git a/cmd/gpt.c b/cmd/gpt.c
index 927b6afa68..58b2c904a1 100644
--- a/cmd/gpt.c
+++ b/cmd/gpt.c
@@ -858,8 +858,9 @@ static int do_rename_gpt_parts(struct blk_desc *dev_desc,
char *subcomm,
u8 part_count = 0;
int partlistlen, ret, numparts = 0, partnum, i = 1, ctr1 = 0, ctr2 = 0;
- if ((subcomm == NULL) || (name1 == NULL) || (name2 == NULL) ||
- (strcmp(subcomm, "swap") && (strcmp(subcomm, "rename"
+ if (!subcomm || !name1 || !name2 ||
+ (strcmp(subcomm, "swap") && strcmp(subcomm, "rename") &&
+strcmp(subcomm, "transpose")))
return -EINVAL;
ret = get_disk_guid(dev_desc, disk_guid);
@@ -920,6 +921,41 @@ static int do_rename_gpt_parts(struct blk_desc *dev_desc,
char *subcomm,
ret = -EINVAL;
goto out;
}
+ } else if (!strcmp(subcomm, "transpose")) {
+ int idx1, idx2;
+ struct disk_partition* first = NULL;
+ struct disk_partition* second= NULL;
+ struct disk_partition tmp_part;
+
+ idx1 = simple_strtoul(name1, NULL, 10);
+ idx2 = simple_strtoul(name2, NULL, 10);
+ if (idx1 == idx2) {
+ printf("Cannot swap partition with itself\n");
+ ret = -EINVAL;
+ goto out;
+ }
+
+ list_for_each(pos, _partitions) {
+ curr = list_entry(pos, struct disk_part, list);
+ if (curr->partnum == idx1)
+ first = >gpt_part_info;
+ else if (curr->partnum == idx2)
+ second = >gpt_part_info;
+ }
+ if (!first) {
+ printf("Illegal partition number %s\n", name1);
+ ret = -EINVAL;
+ goto out;
+ }
+ if (!second) {
+ printf("Illegal partition number %s\n", name2);
+ ret = -EINVAL;
+ goto out;
+ }
+
+ tmp_part = *first;
+ *first = *second;
+ *second = tmp_part;
} else { /* rename */
if (strlen(name2) > PART_NAME_LEN) {
printf("Names longer than %d characters are
truncated.\n", PART_NAME_LEN);
@@ -1123,7 +1159,8 @@ static int do_gpt(struct cmd_tbl *cmdtp, int flag, int
argc, char *const argv[])
} else if (strcmp(argv[1], "read") == 0) {
ret = do_get_gpt_info(blk_dev_desc, (argc == 5) ? argv[4] :
NULL);
} else if ((strcmp(argv[1], "swap") == 0) ||
- (strcmp(argv[1], "rename") == 0)) {
+ (strcmp(argv[1], "rename") == 0) ||
+ (strcmp(argv[1], "transpose") == 0)) {
ret = do_rename_gpt_parts(blk_dev_desc, argv[1], argv[4],
argv[5]);
} else if ((strcmp(argv[1], "set-bootable") == 0)) {
ret = gpt_set_bootable(blk_dev_desc, argv[4]);
@@ -1176,6 +1213,8 @@ U_BOOT_CMD(gpt, CONFIG_SYS_MAXARGS, 1, do_gpt,
" gpt swap\n"
"- change all partitions named name1 to name2\n"
" and vice-versa\n"
+ " gpt transpose\n"
+ "- Swap the order of the entries for part1 and part2 in the
partition table\n"
" gpt rename\n"
"- rename the specified partition\n"
" gpt set-bootable \n"
@@ -1184,5 +1223,6 @@ U_BOOT_CMD(gpt, CONFIG_SYS_MAXARGS, 1, do_gpt,
" gpt swap mmc 0 foo bar\n"
" gpt rename mmc 0 3 foo\n"
" gpt set-bootable mmc 0 boot_a,boot_b\n"
+ " gpt transpose mmc 0 1 2\n"
#endif
);
diff --git a/doc/usage/cmd/gpt.rst b/doc/usage/cmd/gpt.rst
index 288dd365c0..f6115ecb0e 100644
--- a/doc/usage/cmd/gpt.rst
+++ b/doc/usage/cmd/gpt.rst
@@ -16,6 +16,7 @@ Synopsis
gpt set-bootable
gpt setenv
gpt swap
+gpt transpose
gpt verify []
gpt write
@@ -126,6 +127,13 @@ Changes the names of all partitions that are named 'name1'
to be 'name2', and
all partitions named 'name2' to be 'name1'. CONFIG_CMD_GPT_RENAME=y is
required.
+gpt transpose
+~
+
+Swaps the order of two partition table entries
[PATCH v5 7/8] cmd: gpt: Preserve bootable flag
Sets the bootable flag when constructing the partition string from the
current partition configuration. This ensures that when the partitions
are written back (for example, when renaming a partition), the flag is
preserved.
Signed-off-by: Joshua Watt
---
cmd/gpt.c | 3 +++
test/py/tests/test_gpt.py | 1 +
2 files changed, 4 insertions(+)
diff --git a/cmd/gpt.c b/cmd/gpt.c
index 0090e02110..927b6afa68 100644
--- a/cmd/gpt.c
+++ b/cmd/gpt.c
@@ -176,6 +176,7 @@ static int calc_parts_list_len(int numparts)
#ifdef CONFIG_PARTITION_TYPE_GUID
partlistlen += numparts * (strlen("type=,") + UUID_STR_LEN + 1);
#endif
+ partlistlen += numparts * strlen("bootable,");
partlistlen += numparts * (strlen("uuid=;") + UUID_STR_LEN + 1);
/* for the terminating null */
partlistlen++;
@@ -318,6 +319,8 @@ static int create_gpt_partitions_list(int numparts, const
char *guid,
strcat(partitions_list, ",uuid=");
strncat(partitions_list, curr->gpt_part_info.uuid,
UUID_STR_LEN + 1);
+ if (curr->gpt_part_info.bootable & PART_BOOTABLE)
+ strcat(partitions_list, ",bootable");
strcat(partitions_list, ";");
}
return 0;
diff --git a/test/py/tests/test_gpt.py b/test/py/tests/test_gpt.py
index 93007dee9a..b4c03bc3a2 100644
--- a/test/py/tests/test_gpt.py
+++ b/test/py/tests/test_gpt.py
@@ -143,6 +143,7 @@ def test_gpt_read_var(state_disk_image, u_boot_console):
"size": "0x10",
"type": "0fc63daf-8483-4772-8e79-3d69d8477de4",
"uuid": "33194895-67f6-4561-8457-6fdeed4f50a3",
+"bootable": True,
},
{
"name": "part2",
--
2.34.1
[PATCH v5 6/8] cmd: gpt: Preserve type GUID if enabled
If CONFIG_PARTITION_TYPE_GUID is enabled, the type GUID will be
preserved when writing out the partition string. It was already
respected when writing out partitions; this ensures that if you capture
the current partition layout and write it back (such as when renaming),
the type GUIDs are preserved.
Signed-off-by: Joshua Watt
---
cmd/gpt.c | 16 ++
test/py/tests/test_gpt.py | 65 +++
2 files changed, 81 insertions(+)
diff --git a/cmd/gpt.c b/cmd/gpt.c
index 45fbe07404..0090e02110 100644
--- a/cmd/gpt.c
+++ b/cmd/gpt.c
@@ -173,6 +173,9 @@ static int calc_parts_list_len(int numparts)
/* see part.h for definition of struct disk_partition */
partlistlen += numparts * (strlen("start=MiB,") + sizeof(lbaint_t) + 1);
partlistlen += numparts * (strlen("size=MiB,") + sizeof(lbaint_t) + 1);
+#ifdef CONFIG_PARTITION_TYPE_GUID
+ partlistlen += numparts * (strlen("type=,") + UUID_STR_LEN + 1);
+#endif
partlistlen += numparts * (strlen("uuid=;") + UUID_STR_LEN + 1);
/* for the terminating null */
partlistlen++;
@@ -211,6 +214,11 @@ static struct disk_part *allocate_disk_part(struct
disk_partition *info,
PART_TYPE_LEN);
newpart->gpt_part_info.type[PART_TYPE_LEN - 1] = '\0';
newpart->gpt_part_info.bootable = info->bootable;
+#ifdef CONFIG_PARTITION_TYPE_GUID
+ strncpy(newpart->gpt_part_info.type_guid, (const char *)info->type_guid,
+ UUID_STR_LEN);
+ newpart->gpt_part_info.type_guid[UUID_STR_LEN] = '\0';
+#endif
#ifdef CONFIG_PARTITION_UUIDS
strncpy(newpart->gpt_part_info.uuid, (const char *)info->uuid,
UUID_STR_LEN);
@@ -252,6 +260,9 @@ static void print_gpt_info(void)
curr->gpt_part_info.name);
printf("Type %s, bootable %d\n", curr->gpt_part_info.type,
curr->gpt_part_info.bootable & PART_BOOTABLE);
+#ifdef CONFIG_PARTITION_TYPE_GUID
+ printf("Type GUID %s\n", curr->gpt_part_info.type_guid);
+#endif
#ifdef CONFIG_PARTITION_UUIDS
printf("UUID %s\n", curr->gpt_part_info.uuid);
#endif
@@ -299,6 +310,11 @@ static int create_gpt_partitions_list(int numparts, const
char *guid,
curr->gpt_part_info.blksz);
strncat(partitions_list, partstr, PART_NAME_LEN + 1);
+#ifdef CONFIG_PARTITION_TYPE_GUID
+ strcat(partitions_list, ",type=");
+ strncat(partitions_list, curr->gpt_part_info.type_guid,
+ UUID_STR_LEN + 1);
+#endif
strcat(partitions_list, ",uuid=");
strncat(partitions_list, curr->gpt_part_info.uuid,
UUID_STR_LEN + 1);
diff --git a/test/py/tests/test_gpt.py b/test/py/tests/test_gpt.py
index 5d23f9b292..93007dee9a 100644
--- a/test/py/tests/test_gpt.py
+++ b/test/py/tests/test_gpt.py
@@ -16,6 +16,35 @@ the test.
# Mark all tests here as slow
pytestmark = pytest.mark.slow
+def parse_gpt_parts(disk_str):
+"""Parser a partition string into a list of partitions.
+
+Args:
+disk_str: The disk description string, as returned by `gpt read`
+
+Returns:
+A list of parsed partitions. Each partition is a dictionary with the
+string value from each specified key in the partition description, or a
+key with with the value True for a boolean flag
+"""
+parts = []
+for part_str in disk_str.split(';'):
+part = {}
+for option in part_str.split(","):
+if not option:
+continue
+
+if "=" in option:
+key, value = option.split("=")
+part[key] = value
+else:
+part[option] = True
+
+if part:
+parts.append(part)
+
+return parts
+
class GptTestDiskImage(object):
"""Disk Image used by the GPT tests."""
@@ -49,11 +78,13 @@ class GptTestDiskImage(object):
u_boot_utils.run_and_log(u_boot_console, cmd)
# part1 offset 1MB size 1MB
cmd = ('sgdisk', '--new=1:2048:4095', '--change-name=1:part1',
+'--partition-guid=1:33194895-67f6-4561-8457-6fdeed4f50a3',
'-A 1:set:2',
persistent)
# part2 offset 2MB size 1.5MB
u_boot_utils.run_and_log(u_boot_console, cmd)
cmd = ('sgdisk', '--new=2:4096:7167', '--change-name=2:part2',
+'--partition-guid=2:cc9c6e4a-6551-4cb5-87be-3210f96c86fb',
persistent)
u_boot_utils.run_and_log(u_boot_console, cmd)
cmd = ('sgdisk', '--load-backup=' + persistent)
@@ -88,6 +119,40 @@ def test_gpt_read(state_disk_image, u_boot_console):
assert '0x0800 0x0fff "part1"' in output
assert '0x1000 0x1bff "part2"' in output
[PATCH v5 5/8] cmd: gpt: Add command to set bootable flags
Adds a command that can be used to modify the GPT partition table to
indicate which partitions should have the bootable flag set
Signed-off-by: Joshua Watt
---
cmd/gpt.c | 80 +++
doc/usage/cmd/gpt.rst | 12 ++
test/py/tests/test_gpt.py | 22 +++
3 files changed, 114 insertions(+)
diff --git a/cmd/gpt.c b/cmd/gpt.c
index c6c8282ac9..45fbe07404 100644
--- a/cmd/gpt.c
+++ b/cmd/gpt.c
@@ -972,6 +972,81 @@ static int do_rename_gpt_parts(struct blk_desc *dev_desc,
char *subcomm,
free(partitions_list);
return ret;
}
+
+/**
+ * gpt_set_bootable() - Set bootable flags for partitions
+ *
+ * Sets the bootable flag for any partition names in the comma separated list
of
+ * partition names. Any partitions not in the list have their bootable flag
+ * cleared
+ *
+ * @desc: block device descriptor
+ * @name: Comma separated list of partition names
+ *
+ * @Return: '0' on success and -ve error on failure
+ */
+static int gpt_set_bootable(struct blk_desc *blk_dev_desc, char *const
part_list)
+{
+ char *name;
+ char disk_guid[UUID_STR_LEN + 1];
+ struct list_head *pos;
+ struct disk_part *curr;
+ struct disk_partition *partitions = NULL;
+ int part_count = 0;
+ int ret = get_disk_guid(blk_dev_desc, disk_guid);
+
+ if (ret < 0)
+ return ret;
+
+ ret = get_gpt_info(blk_dev_desc);
+ if (ret <= 0)
+ goto out;
+
+ part_count = ret;
+ partitions = malloc(sizeof(*partitions) * part_count);
+ if (!partitions) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ /* Copy partitions and clear bootable flag */
+ part_count = 0;
+ list_for_each(pos, _partitions) {
+ curr = list_entry(pos, struct disk_part, list);
+ partitions[part_count] = curr->gpt_part_info;
+ partitions[part_count].bootable &= ~PART_BOOTABLE;
+ part_count++;
+ }
+
+ name = strtok(part_list, ",");
+ while (name) {
+ bool found = false;
+
+ for (int i = 0; i < part_count; i++) {
+ if (strcmp((char *)partitions[i].name, name) == 0) {
+ partitions[i].bootable |= PART_BOOTABLE;
+ found = true;
+ }
+ }
+
+ if (!found) {
+ printf("Warning: No partition matching '%s' found\n",
+ name);
+ }
+
+ name = strtok(NULL, ",");
+ }
+
+ ret = gpt_restore(blk_dev_desc, disk_guid, partitions, part_count);
+
+out:
+ del_gpt_info();
+
+ if (partitions)
+ free(partitions);
+
+ return ret;
+}
#endif
/**
@@ -1031,6 +1106,8 @@ static int do_gpt(struct cmd_tbl *cmdtp, int flag, int
argc, char *const argv[])
} else if ((strcmp(argv[1], "swap") == 0) ||
(strcmp(argv[1], "rename") == 0)) {
ret = do_rename_gpt_parts(blk_dev_desc, argv[1], argv[4],
argv[5]);
+ } else if ((strcmp(argv[1], "set-bootable") == 0)) {
+ ret = gpt_set_bootable(blk_dev_desc, argv[4]);
#endif
} else {
return CMD_RET_USAGE;
@@ -1082,8 +1159,11 @@ U_BOOT_CMD(gpt, CONFIG_SYS_MAXARGS, 1, do_gpt,
" and vice-versa\n"
" gpt rename\n"
"- rename the specified partition\n"
+ " gpt set-bootable \n"
+ "- make partition names in list bootable\n"
" Example usage:\n"
" gpt swap mmc 0 foo bar\n"
" gpt rename mmc 0 3 foo\n"
+ " gpt set-bootable mmc 0 boot_a,boot_b\n"
#endif
);
diff --git a/doc/usage/cmd/gpt.rst b/doc/usage/cmd/gpt.rst
index b505b159d0..288dd365c0 100644
--- a/doc/usage/cmd/gpt.rst
+++ b/doc/usage/cmd/gpt.rst
@@ -13,6 +13,7 @@ Synopsis
gpt read []
gpt rename
gpt repair
+gpt set-bootable
gpt setenv
gpt swap
gpt verify []
@@ -90,6 +91,13 @@ gpt repair
Repairs the GPT partition tables if it they become corrupted.
+gpt set-bootable
+
+
+Sets the bootable flag for all partitions in the table. If the partition name
+is in 'partition list' (separated by ','), the bootable flag is set, otherwise
+it is cleared. CONFIG_CMD_GPT_RENAME=y is required.
+
gpt setenv
~~
@@ -187,3 +195,7 @@ Get the GUID for a disk::
=> gpt guid mmc gpt_disk_uuid
=> echo ${gpt_disk_uuid}
bec9fc2a-86c1-483d-8a0e-0109732277d7
+
+Set the bootable flag for the 'boot' partition and clear it for all others::
+
+=> gpt set-bootable mmc 0 boot
diff --git a/test/py/tests/test_gpt.py b/test/py/tests/test_gpt.py
index 946858800d..5d23f9b292 100644
--- a/test/py/tests/test_gpt.py
+++ b/test/py/tests/test_gpt.py
@@ -222,6 +222,28 @@ def test_gpt_swap_partitions(state_disk_image,
u_boot_console):
[PATCH v5 4/8] cmd: gpt: Add gpt_partition_bootable variable
Adds an additional variable called gpt_partition_bootable that indicates
if the given partition is bootable or not.
Signed-off-by: Joshua Watt
---
cmd/gpt.c | 9 +++--
doc/usage/cmd/gpt.rst | 5 +
test/py/tests/test_gpt.py | 33 +
3 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/cmd/gpt.c b/cmd/gpt.c
index fe9e06681c..c6c8282ac9 100644
--- a/cmd/gpt.c
+++ b/cmd/gpt.c
@@ -725,7 +725,7 @@ static int gpt_enumerate(struct blk_desc *desc)
* gpt_setenv_part_variables() - setup partition environmental variables
*
* Setup the gpt_partition_name, gpt_partition_entry, gpt_partition_addr
- * and gpt_partition_size environment variables.
+ * and gpt_partition_size, gpt_partition_bootable environment variables.
*
* @pinfo: pointer to disk partition
* @i: partition entry
@@ -752,6 +752,10 @@ static int gpt_setenv_part_variables(struct disk_partition
*pinfo, int i)
if (ret)
goto fail;
+ ret = env_set_ulong("gpt_partition_bootable", !!(pinfo->bootable &
PART_BOOTABLE));
+ if (ret)
+ goto fail;
+
return 0;
fail:
@@ -1057,7 +1061,8 @@ U_BOOT_CMD(gpt, CONFIG_SYS_MAXARGS, 1, do_gpt,
" gpt setenv mmc 0 $name\n"
"- setup environment variables for partition $name:\n"
" gpt_partition_addr, gpt_partition_size,\n"
- " gpt_partition_name, gpt_partition_entry\n"
+ " gpt_partition_name, gpt_partition_entry,\n"
+ " gpt_partition_bootable\n"
" gpt enumerate mmc 0\n"
"- store list of partitions to gpt_partition_list environment
variable\n"
" gpt guid \n"
diff --git a/doc/usage/cmd/gpt.rst b/doc/usage/cmd/gpt.rst
index 6387c8116f..b505b159d0 100644
--- a/doc/usage/cmd/gpt.rst
+++ b/doc/usage/cmd/gpt.rst
@@ -108,6 +108,9 @@ gpt_partition_name
gpt_partition_entry
the partition number in the table, e.g. 1, 2, 3, etc.
+gpt_partition_bootable
+1 if the partition is marked as bootable, 0 if not
+
gpt swap
@@ -167,6 +170,8 @@ Get the information about the partition named 'rootfs'::
rootfs
=> echo ${gpt_partition_entry}
2
+=> echo ${gpt_partition_bootable}
+0
Get the list of partition names on the disk::
diff --git a/test/py/tests/test_gpt.py b/test/py/tests/test_gpt.py
index 339468bc12..946858800d 100644
--- a/test/py/tests/test_gpt.py
+++ b/test/py/tests/test_gpt.py
@@ -49,6 +49,7 @@ class GptTestDiskImage(object):
u_boot_utils.run_and_log(u_boot_console, cmd)
# part1 offset 1MB size 1MB
cmd = ('sgdisk', '--new=1:2048:4095', '--change-name=1:part1',
+'-A 1:set:2',
persistent)
# part2 offset 2MB size 1.5MB
u_boot_utils.run_and_log(u_boot_console, cmd)
@@ -117,6 +118,38 @@ def test_gpt_guid(state_disk_image, u_boot_console):
output = u_boot_console.run_command('gpt guid host 0')
assert '375a56f7-d6c9-4e81-b5f0-09d41ca89efe' in output
+@pytest.mark.boardspec('sandbox')
+@pytest.mark.buildconfigspec('cmd_gpt')
+@pytest.mark.requiredtool('sgdisk')
+def test_gpt_setenv(state_disk_image, u_boot_console):
+"""Test the gpt setenv command."""
+u_boot_console.run_command('host bind 0 ' + state_disk_image.path)
+output = u_boot_console.run_command('gpt setenv host 0 part1')
+assert 'success!' in output
+output = u_boot_console.run_command('echo ${gpt_partition_addr}')
+assert output.rstrip() == '800'
+output = u_boot_console.run_command('echo ${gpt_partition_size}')
+assert output.rstrip() == '800'
+output = u_boot_console.run_command('echo ${gpt_partition_name}')
+assert output.rstrip() == 'part1'
+output = u_boot_console.run_command('echo ${gpt_partition_entry}')
+assert output.rstrip() == '1'
+output = u_boot_console.run_command('echo ${gpt_partition_bootable}')
+assert output.rstrip() == '1'
+
+output = u_boot_console.run_command('gpt setenv host 0 part2')
+assert 'success!' in output
+output = u_boot_console.run_command('echo ${gpt_partition_addr}')
+assert output.rstrip() == '1000'
+output = u_boot_console.run_command('echo ${gpt_partition_size}')
+assert output.rstrip() == 'c00'
+output = u_boot_console.run_command('echo ${gpt_partition_name}')
+assert output.rstrip() == 'part2'
+output = u_boot_console.run_command('echo ${gpt_partition_entry}')
+assert output.rstrip() == '2'
+output = u_boot_console.run_command('echo ${gpt_partition_bootable}')
+assert output.rstrip() == '0'
+
@pytest.mark.boardspec('sandbox')
@pytest.mark.buildconfigspec('cmd_gpt')
@pytest.mark.requiredtool('sgdisk')
--
2.34.1
[PATCH v5 3/8] tests: gpt: Remove test order dependency
Re-create a clean disk image for each test to prevent modifications from
one test affecting another
Signed-off-by: Joshua Watt
---
test/py/tests/test_gpt.py | 20
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/test/py/tests/test_gpt.py b/test/py/tests/test_gpt.py
index 73bfbf77a2..339468bc12 100644
--- a/test/py/tests/test_gpt.py
+++ b/test/py/tests/test_gpt.py
@@ -61,18 +61,14 @@ class GptTestDiskImage(object):
cmd = ('cp', persistent, self.path)
u_boot_utils.run_and_log(u_boot_console, cmd)
-gtdi = None
@pytest.fixture(scope='function')
def state_disk_image(u_boot_console):
"""pytest fixture to provide a GptTestDiskImage object to tests.
This is function-scoped because it uses u_boot_console, which is also
-function-scoped. However, we don't need to actually do any function-scope
-work, so this simply returns the same object over and over each time."""
+function-scoped. A new disk is returned each time to prevent tests from
+interfering with each other."""
-global gtdi
-if not gtdi:
-gtdi = GptTestDiskImage(u_boot_console)
-return gtdi
+return GptTestDiskImage(u_boot_console)
@pytest.mark.boardspec('sandbox')
@pytest.mark.buildconfigspec('cmd_gpt')
@@ -186,12 +182,12 @@ def test_gpt_swap_partitions(state_disk_image,
u_boot_console):
u_boot_console.run_command('host bind 0 ' + state_disk_image.path)
output = u_boot_console.run_command('part list host 0')
-assert '0x0800 0x0fff "first"' in output
-assert '0x1000 0x1bff "second"' in output
-u_boot_console.run_command('gpt swap host 0 first second')
+assert '0x0800 0x0fff "part1"' in output
+assert '0x1000 0x1bff "part2"' in output
+u_boot_console.run_command('gpt swap host 0 part1 part2')
output = u_boot_console.run_command('part list host 0')
-assert '0x0800 0x0fff "second"' in output
-assert '0x1000 0x1bff "first"' in output
+assert '0x0800 0x0fff "part2"' in output
+assert '0x1000 0x1bff "part1"' in output
@pytest.mark.boardspec('sandbox')
@pytest.mark.buildconfigspec('cmd_gpt')
--
2.34.1
[PATCH v5 2/8] doc: Add gpt command documentation
Adds initial documentation for the gpt command Signed-off-by: Joshua Watt --- doc/usage/cmd/gpt.rst | 184 ++ doc/usage/index.rst | 1 + 2 files changed, 185 insertions(+) create mode 100644 doc/usage/cmd/gpt.rst diff --git a/doc/usage/cmd/gpt.rst b/doc/usage/cmd/gpt.rst new file mode 100644 index 00..6387c8116f --- /dev/null +++ b/doc/usage/cmd/gpt.rst @@ -0,0 +1,184 @@ +.. SPDX-License-Identifier: GPL-2.0+ + +gpt command +=== + +Synopsis + + +:: + +gpt enumerate +gpt guid [] +gpt read [] +gpt rename +gpt repair +gpt setenv +gpt swap +gpt verify [] +gpt write + +Description +--- + +The gpt command lets users read, create, modify, or verify the GPT (GUID +Partition Table) partition layout. + +Common arguments: + +interface +interface for accessing the block device (mmc, sata, scsi, usb, ) + +dev +device number + +partition string +Describes the GPT partition layout for a disk. The syntax is similar to +the one used by the :doc:`mbr command ` command. The string contains +one or more partition descriptors, each separated by a ";". Each descriptor +contains one or more fields, with each field separated by a ",". Fields are +either of the form "key=value" to set a specific value, or simple "flag" to +set a boolean flag + +The first descriptor can optionally be used to describe parameters for the +whole disk with the following fields: + +* uuid_disk=UUID - Set the UUID for the disk + +Partition descriptors can have the following fields: + +* name= - The partition name, required +* start= - The partition start offset in bytes, required +* size= - The partition size in bytes or "-" to expand it to the whole free area +* bootable - Set the legacy bootable flag +* uuid= - The partition UUID, optional if CONFIG_RANDOM_UUID=y is enabled +* type= - The partition type GUID, requires CONFIG_PARTITION_TYPE_GUID=y + + +If 'uuid' is not specified, but CONFIG_RANDOM_UUID is enabled, a random UUID +will be generated for the partition + +gpt enumerate +~ + +Sets the variable 'gpt_partition_list' to be a list of all the partition names +on the device. + +gpt guid + + +Report the GUID of a disk. If 'varname' is specified, the command will set the +variable to the GUID, otherwise it will be printed out. + +gpt read + + +Prints the current state of the GPT partition table. If 'varname' is specified, +the variable will be filled with a partition string in the same format as a +'', suitable for passing to other 'gpt' commands. If the +argument is omitted, a human readable description is printed out. +CONFIG_CMD_GPT_RENAME=y is required. + +gpt rename +~~ + +Renames all partitions named 'part' to be 'name'. CONFIG_CMD_GPT_RENAME=y is +required. + +gpt repair +~~ + +Repairs the GPT partition tables if it they become corrupted. + +gpt setenv +~~ + +The 'gpt setenv' command will set a series of environment variables with +information about the partition named ''. The variables are: + +gpt_partition_addr +the starting offset of the partition in blocks as a hexadecimal number + +gpt_partition_size +the size of the partition in blocks as a hexadecimal number + +gpt_partition_name +the name of the partition + +gpt_partition_entry +the partition number in the table, e.g. 1, 2, 3, etc. + +gpt swap + + +Changes the names of all partitions that are named 'name1' to be 'name2', and +all partitions named 'name2' to be 'name1'. CONFIG_CMD_GPT_RENAME=y is +required. + +gpt verify +~~ + +Sets return value $? to 0 (true) if the partition layout on the +specified disk matches the one in the provided partition string, and 1 (false) +if it does not match. If no partition string is specified, the command will +check if the disk is partitioned or not. + +gpt write +~ + +(Re)writes the partition table on the disk to match the provided +partition string. It returns 0 on success or 1 on failure. + +Configuration +- + +To use the 'gpt' command you must specify CONFIG_CMD_GPT=y. To enable 'gpt +read', 'gpt swap' and 'gpt rename', you must specify CONFIG_CMD_GPT_RENAME=y. + +Examples + +Create 6 partitions on a disk:: + +=> setenv gpt_parts 'uuid_disk=bec9fc2a-86c1-483d-8a0e-0109732277d7; + name=boot,start=4M,size=128M,bootable,type=ebd0a0a2-b9e5-4433-87c0-68b6b72699c7, +name=rootfs,size=3072M,type=0fc63daf-8483-4772-8e79-3d69d8477de4; +name=system-data,size=512M,type=0fc63daf-8483-4772-8e79-3d69d8477de4; +name=[ext],size=-,type=0fc63daf-8483-4772-8e79-3d69d8477de4; +name=user,size=-,type=0fc63daf-8483-4772-8e79-3d69d8477de4; +name=modules,size=100M,type=0fc63daf-8483-4772-8e79-3d69d8477de4; +name=ramdisk,size=8M,type=0fc63daf-8483-4772-8e79-3d69d8477de4 +=>
[PATCH v5 1/8] cmd: gpt: Remove confusing help text
This help text appears to be a fragment of the text shown when CONFIG_CMD_GPT_RENAME is enabled, but is confusing so remove it. Signed-off-by: Joshua Watt --- cmd/gpt.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/cmd/gpt.c b/cmd/gpt.c index 964056bd28..fe9e06681c 100644 --- a/cmd/gpt.c +++ b/cmd/gpt.c @@ -1060,8 +1060,6 @@ U_BOOT_CMD(gpt, CONFIG_SYS_MAXARGS, 1, do_gpt, " gpt_partition_name, gpt_partition_entry\n" " gpt enumerate mmc 0\n" "- store list of partitions to gpt_partition_list environment variable\n" - " read \n" - "- read GPT into a data structure for manipulation\n" " gpt guid \n" "- print disk GUID\n" " gpt guid \n" -- 2.34.1
[PATCH v5 0/8] cmd: gpt: GPT manipulation improvements
Adds several improvements and additions to the gpt command processing, specifically (although not exclusively) for the purpose of supporting "ping-pong" booting when doing A/B boot partitions with u-boot itself. In this mechanism, u-boot must boot up, and then check if the correct boot partition is active, and if not switch the GPT partition table to the other boot partition and reboot to activate the other u-boot. In order to facilitate this, the gpt command needs to be better at preserving entry attributes when manipulating the partition table. It also learns two new commands, one which can swap the order of partitions in the table, and another that lets it change which partitions have the bootable flag. V2: Add documentation and tests V3: Review Feedback V4: More review feedback. Fixed 'gpt swap-position' to work with missing partition indexes. V5: Rename 'gpt swap-position' -> 'gpt transpose', taking inspiration from `sgdisk --transpose` Joshua Watt (8): cmd: gpt: Remove confusing help text doc: Add gpt command documentation tests: gpt: Remove test order dependency cmd: gpt: Add gpt_partition_bootable variable cmd: gpt: Add command to set bootable flags cmd: gpt: Preserve type GUID if enabled cmd: gpt: Preserve bootable flag cmd: gpt: Add command to swap partition order cmd/gpt.c | 156 -- doc/usage/cmd/gpt.rst | 226 ++ doc/usage/index.rst | 1 + test/py/tests/test_gpt.py | 160 +-- 4 files changed, 524 insertions(+), 19 deletions(-) create mode 100644 doc/usage/cmd/gpt.rst -- 2.34.1
Re: [PATCH] net: wget: Avoid packet queue overflow
On Thu, Aug 31, 2023 at 12:27:59PM +0200, Richard Weinberger wrote:
> - Ursprüngliche Mail -
> > Von: "richard"
> > An: u-boot@lists.denx.de
> > CC: "richard" , "Joe Hershberger" ,
> > "Ramon Fried"
> > Gesendet: Donnerstag, 20. Juli 2023 14:51:56
> > Betreff: [PATCH] net: wget: Avoid packet queue overflow
>
> > Make sure to stay within bounds, as a misbehaving HTTP server
> > can trigger a buffer overflow if not properly handled.
> >
> > Cc: Joe Hershberger
> > Cc: Ramon Fried
> > Signed-off-by: Richard Weinberger
> > ---
> > net/wget.c | 10 +-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/wget.c b/net/wget.c
> > index 2dbfeb1a1d5b..8bb4d72db1ae 100644
> > --- a/net/wget.c
> > +++ b/net/wget.c
> > @@ -35,7 +35,8 @@ struct pkt_qd {
> > * The actual packet bufers are in the kernel space, and are
> > * expected to be overwritten by the downloaded image.
> > */
> > -static struct pkt_qd pkt_q[PKTBUFSRX / 4];
> > +#define PKTQ_SZ (PKTBUFSRX / 4)
> > +static struct pkt_qd pkt_q[PKTQ_SZ];
> > static int pkt_q_idx;
> > static unsigned long content_length;
> > static unsigned int packets;
> > @@ -202,6 +203,13 @@ static void wget_connected(uchar *pkt, unsigned int
> > tcp_seq_num,
> > pkt_q[pkt_q_idx].tcp_seq_num = tcp_seq_num;
> > pkt_q[pkt_q_idx].len = len;
> > pkt_q_idx++;
> > +
> > + if (pkt_q_idx >= PKTQ_SZ) {
> > + printf("wget: Fatal error, queue overrun!\n");
> > + net_set_state(NETLOOP_FAIL);
> > +
> > + return;
> > + }
> > } else {
> > debug_cond(DEBUG_WGET, "wget: Connected HTTP Header %p\n", pkt);
> > /* sizeof(http_eom) - 1 is the string length of (http_eom) */
This seems fine and I'll pick it up soon. Thanks!
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH] fwu: Initialize global fwu library state during CI test
On Wed, Aug 23, 2023 at 02:16:52AM +0200, Marek Vasut wrote: > The current CI test worked by sheer luck, the g_dev global pointer > in the fwu library was never initialized and the test equally well > failed on sandbox64. Trigger the main loop in sandbox tests too to > initialize that global state, and move the sandbox specific exit > from fwu_boottime_checks after g_dev is initialized. > > Signed-off-by: Marek Vasut > Acked-by: Sughosh Ganu > Reviewed-by: Simon Glass Applied to u-boot/next, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] tools: image-host: print error messages to stderr
On Thu, Aug 17, 2023 at 06:36:10PM +0300, Oleksandr Suvorov wrote: > The make by default cuts off the stdout output from external tools, > so all error messages from the image-host are not shown in a make > output. Besides that, it is a common approach to use stderr stream > for error messages. > Use stderr for all error messages in image-host. > > Signed-off-by: Oleksandr Suvorov > Reviewed-by: Simon Glass Applied to u-boot/next, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 5/5] configs: sandbox: Enable NVMXIP QSPI driver
On Wed, Aug 23, 2023 at 02:18:21AM +0200, Marek Vasut wrote: > Enable NVMXIP QSPI driver on sandbox, since it is already enabled > on sandbox64. > > Signed-off-by: Marek Vasut Applied to u-boot/next, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 4/5] drivers/mtd/nvmxip: Move sandbox_set_enable_memio() to test
On Wed, Aug 23, 2023 at 02:18:20AM +0200, Marek Vasut wrote: > The sandbox_set_enable_memio() should only ever be set during > sandbox testing, not within driver itself, move it back to test/ . > > Signed-off-by: Marek Vasut Applied to u-boot/next, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 3/5] drivers/mtd/nvmxip: Print phys_addr_t without warnings on both 32bit and 64bit systems
On Wed, Aug 23, 2023 at 02:18:19AM +0200, Marek Vasut wrote: > Cast the address such that it can be printed without warnings > on both 32bit and 64bit systems. This really should use some > better print formatter, but for the lack of it, do it this way. > > Signed-off-by: Marek Vasut Applied to u-boot/next, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 2/5] drivers/mtd/nvmxip: Rework the read accessor to support 32bit systems
On Wed, Aug 23, 2023 at 02:18:18AM +0200, Marek Vasut wrote: > Get rid of nvmxip_mmio_rawread() and just implement the readl()/readq() > reader loop within nvmxip_blk_read(). Cast the destination buffer as > needed and increment the read by either 4 or 8 bytes depending on if > this is systemd with 32bit or 64bit physical address. > > Signed-off-by: Marek Vasut Applied to u-boot/next, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 1/5] drivers/mtd/nvmxip: Trigger post bind as probe on driver level
On Wed, Aug 23, 2023 at 02:18:17AM +0200, Marek Vasut wrote: > Perform all the block device creation only once, after the driver itself > successfully bound. Do not do this in uclass post bind, as this might be > triggered multiple times. For example the ut_dm_host test triggers this > and triggers a memory leak that way, since there are now multiple block > devices created using the blk_create_devicef() . > > To retain the old probe-on-boot behavior, set DM_FLAG_PROBE_AFTER_BIND > flag in uclass post_bind callback, so the driver model would probe the > driver at the right time. > > Rename the function as well, to match similar functions in > other block-related subsystems, like the mmc one. > > Signed-off-by: Marek Vasut Applied to u-boot/next, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v1] serial-uclass: reset gd->cur_serial_dev to NULL if serial not found
On Fri, Aug 18, 2023 at 12:34:30PM +0300, Maksim Kiselev wrote:
> Reset gd->cur_serial_dev pointer to avoid calling non-relocated code
> from relocated code if a serial driver is not found and
> CONFIG_REQUIRE_SERIAL_CONSOLE is disabled.
>
> Here is detailed explanation of what this patch is trying to fix.
>
> U-boot calls the serial_find_console_or_panic() function twice.
> The first console setup occurs before U-boot relocation in
> the serial_init(). This stage uses simple FDT parsing and
> assigns gd->cur_serial_dev to a "serial" device that lives in
> non-relocated code too.
>
> The second console setup after U-boot relocation(from serial_initialize())
> may use full live DT (if OF_LIVE enabled) probe sequence with buses,
> clocks, resets, etc... And if the console setup fails at this step,
> than we should be caught by panic_str("No serial driver found").
>
> But... If we disable CONFIG_REQUIRE_SERIAL_CONSOLE, than we
> return from serial_init() with gd->cur_serial_dev pointing
> to the "old"(non-relocated) serial device.
>
> And if this area, where "old" serial device is placed, is changed
> (e.g. Linux kernel may be relocated at this address), than we will get
> an unexpected crash on the next call of printf().
>
> Signed-off-by: Maksim Kiselev
Applied to u-boot/next, thanks!
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH] pci: pcie-brcmstb: do not rely on CLKREQ# signal
On Wed, Aug 16, 2023 at 03:27:53PM -0700, Sam Edwards wrote: > When the Broadcom STB PCIe controller is initialized, it must be set > into one of three CLKREQ# modes: "none"/"aspm"/"l1ss". The Linux driver, > through today, hard-codes "aspm" since the vast majority of boards using > this driver have a fixed PCIe bus with the CLKREQ# signal wired up. > > The Raspberry Pi CM4, however, can be connected to a plethora of PCIe > devices, some of which do not connect the CLKREQ# line (they just leave > it floating). So "aspm" mode is no longer appropriate in all cases. In > Linux, there is a proposed patchset [1] to determine the proper mode. > This doesn't really make sense in U-Boot's case, so we just change the > assumption from "aspm" to "none" (which is always safe). > > This patch DOES resolve a real-world crash that occurs when U-Boot is > running on a Raspberry Pi CM4 installed in slot 3 of a Turing Pi 2 > cluster board. > > [1]: https://lore.kernel.org/all/20230428223500.23337-1-jim2101...@gmail.com/ > > Signed-off-by: Sam Edwards Applied to u-boot/next, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] pci: pcie-brcmstb: bring over some robustness improvements from Linux
On Mon, Aug 14, 2023 at 04:34:13PM -0600, Sam Edwards wrote: > Since the initial U-Boot driver was ported here from Linux, the latter > has had a few changes for robustness/stability. This patch brings over > two of them: > - Do not attempt to access the configuration space of a PCIe device if > the link has gone down, as that will result in an asynchronous SError > interrupt which will crash U-Boot. > - Wait for the recommended 100ms after PERST# is deasserted. > > I sent this patch while debugging a crash involving PCIe, but these > are unrelated improvements. I do not believe that this patch fixes any > real-world bug. > > Signed-off-by: Sam Edwards Applied to u-boot/next, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] imx: hab: Use CONFIG_SPL_LOAD_FIT_ADDRESS in the CSF example
On 31/08/2023 11:56, Marek Vasut wrote: The SPL authenticates image starting from CONFIG_SPL_LOAD_FIT_ADDRESS address, update the csf_fit.txt to match. Signed-off-by: Marek Vasut --- Cc: "NXP i.MX U-Boot Team" Cc: Fabio Estevam Cc: Peng Fan Cc: Rasmus Villemoes Cc: Stefano Babic Cc: Tim Harvey --- doc/imx/habv4/csf_examples/mx8m/csf_fit.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/imx/habv4/csf_examples/mx8m/csf_fit.txt b/doc/imx/habv4/csf_examples/mx8m/csf_fit.txt index 3d79edf2813..97f3eea573b 100644 --- a/doc/imx/habv4/csf_examples/mx8m/csf_fit.txt +++ b/doc/imx/habv4/csf_examples/mx8m/csf_fit.txt @@ -27,4 +27,4 @@ Verification index = 2 # FIXME: # Line 1 -- fitImage - Blocks = 0x401fcdc0 0x57c00 0x "flash.bin" + Blocks = CONFIG_SPL_LOAD_FIT_ADDRESS 0x57c00 0x "flash.bin" Reviewed-by: Fabio Estevam Thanks
Re: [PATCH v2] tools: ensure zeroed padding in external FIT images
On Thu, Aug 31, 2023 at 09:39:52AM +, Roman Azarenko wrote:
> > On Mon, 28 Aug 2023 at 02:00, Roman Azarenko
> wrote:
> > >
> > > On Fri, 2023-08-25 at 12:06 -0600, Simon Glass wrote:
> > > > > @@ -564,9 +564,13 @@ static int fit_extract_data(struct
> > > > > image_tool_params *params, const char *fname)
> > > > > /* Pack the FDT and place the data after it */
> > > > > fdt_pack(fdt);
> > > > >
> > > > > - new_size = fdt_totalsize(fdt);
> > > > > - new_size = ALIGN(new_size, align_size);
> > > > > + unpadded_size = fdt_totalsize(fdt);
> > > > > + new_size = ALIGN(unpadded_size, align_size);
> > > > > fdt_set_totalsize(fdt, new_size);
> > > >
> > > > I didn't know that was allowed...I thought it needed fdt_open_into() ?
> > >
> > > The introduction of fdt_set_totalsize() comes from commit ebfe611be91e
> > > ("mkimage: fit_image: Add option to make fit header align"). The commit
> > > message doesn't describe the choice of this function vs fdt_open_into().
> > >
> > > Personally I'm unable to definitively comment on it. I can only blindly
> > > guess, that because we're only changing the total length of the fdt
> > > struct, and keeping all other fields the same, we don't need to allocate
> > > a new fdt struct with a different size.
> >
> > I am not sure if it would cause problems. I do understand that you
> > didn't write the code. You could copy the people who did (and those
> > that reviewed it) for their input.
> >
>
> @Kever, @Punit, @Tom, could you please comment on this remark by Simon?
> Thank you!
I assume that we're given an already well-ordered device tree at this
point and so that's why we don't use fdt_open_into to move things and
just handle it ourselves at that point in the tool.
--
Tom
signature.asc
Description: PGP signature
Re: [RFC] make sandbox UT more generic
Hi, On Wed, 30 Aug 2023 at 23:28, AKASHI Takahiro wrote: > > Hi Simon, > > On Wed, Aug 30, 2023 at 08:49:05PM -0600, Simon Glass wrote: > > Hi, > > > > On Wed, 30 Aug 2023 at 18:38, AKASHI Takahiro > > wrote: > > > > > > Hi, > > > > > > I'm working on implementing SCMI-based pinctrl/gpio driver, > > > and want to re-use sandbox UT to test the code. However, > > > It is somehow sandbox-specific (with additional DT nodes). > > > How can/should we make it more generic for other targets/drivers > > > rather than just by copying the test code? > > > (I have already created a test for pinmux since there is only > > > one existing scenario, but gpio test has many.) > > > > > > Even if I say 'generic', my case may be special since real > > > hardware (device drivers) cannot always run all the test cases, > > > while SCMI-based drivers potentially can with a dummy SCMI server > > > for sandbox. > > > See: > > > drivers/firmware/scmi/sandbox-scmi_agent.c > > > > We don't have a good way to test drivers that talk to hardware, in general. > > > > For I2C, SPI and some PCI devices you can sometimes write an emulator > > for the chip and then your driver can talk to the emulator as if it > > were talking to the hardware. Sandbox does actually support that with > > memory-mapped I/O too, although it is fairly rarely used. > > Well, I don't want or need to emulate some *real* hardware. > Instead, I would like to emulate what the current sandbox drivers > (pinctrl-sandbox.c and gpio/sandbox.c) emulate so that we can re-use > (some portion of) test cases for sandbox (test/dm/pinmux.c and gpio.c). > > As you might know, SCMI protocol with associated drivers on U-Boot is > so generic that it would be able to talk to any of real pinctrl/gpio > drivers/firmware (say, run on OPTEE or SCP). > By implementing/mimicking protocol messages in sandbox-scmi_agent.c, > SCMI drivers are expected to provide *virtual* pinctrl/gpio devices > similar to what sandbox does. I actually know almost nothing about SCMI. > > I have already implemented pinmux test with some tweaks by copying > test/dm/pinmux.c and duplicating almost the same DT nodes as "pinctrl-gpio" > in test.dts. > But I'm looking for any other means without test code duplication. > > Did I clarify my question a bit? Well you should be able to factor out the test code into a function, then call it from two places with the two different devices (or other params) that are needed. For the DT, copying a few nodes is not the end of the world, IMO. BTW have you seen this talk? [2] It seems that you are moving pieces into firmware which should be OS drivers? Anyway, if you place a sandbox pinmux device under the SCMI node in the DT, then you should end up with a pinmux device you can use likely normal. Then if that device uses the sandbox emulator, you can run the existing tests on it with little modification, I suspect. But if I am still missing the point, a diagram or patch might help me understand! Regards, Simon > > -Takahiro Akashi > > > > We have done this a lot with Zephyr, as well[1] and achieved 90% code > > coverage on some boards. > > > > But I'm not quite sure I am answering the right question, so I will stop > > here. > > > > Regards, > > Simon > > > > [1] https://www.youtube.com/watch?v=usXCAXR2G_c [2] https://www.usenix.org/conference/osdi21/presentation/fri-keynote
[PATCH] imx: hab: Use size parameter
The current code works by sheer coincidence, because (see HABv4 API
documentation, section 3.4) the RVT authenticate_image call updates
the size that is passed in with the actual size ROM code pulls from
IVT/CSF . So if the input size is larger, that is "fine" . Pass in
size instead to make this really correct.
Signed-off-by: Marek Vasut
---
NOTE: Rasmus, can you please double-check this one ? Thanks
---
Cc: "NXP i.MX U-Boot Team"
Cc: Fabio Estevam
Cc: Peng Fan
Cc: Rasmus Villemoes
Cc: Stefano Babic
Cc: Tim Harvey
---
arch/arm/mach-imx/spl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
index 6c13b00ef10..b30cd962553 100644
--- a/arch/arm/mach-imx/spl.c
+++ b/arch/arm/mach-imx/spl.c
@@ -353,7 +353,7 @@ void *spl_load_simple_fit_fix_load(const void *fit)
debug("%s: ivt: %p offset: %lx size: %lx\n", __func__, ivt, offset,
size);
debug("%s: ivt self: %x\n", __func__, ivt->self);
- if (imx_hab_authenticate_image((uintptr_t)fit, (uintptr_t)ivt, offset))
+ if (imx_hab_authenticate_image((uintptr_t)fit, (uintptr_t)size, offset))
panic("spl: ERROR: image authentication unsuccessful\n");
return (void *)fit;
--
2.40.1
[PATCH] imx: hab: Use CONFIG_SPL_LOAD_FIT_ADDRESS in the CSF example
The SPL authenticates image starting from CONFIG_SPL_LOAD_FIT_ADDRESS address, update the csf_fit.txt to match. Signed-off-by: Marek Vasut --- Cc: "NXP i.MX U-Boot Team" Cc: Fabio Estevam Cc: Peng Fan Cc: Rasmus Villemoes Cc: Stefano Babic Cc: Tim Harvey --- doc/imx/habv4/csf_examples/mx8m/csf_fit.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/imx/habv4/csf_examples/mx8m/csf_fit.txt b/doc/imx/habv4/csf_examples/mx8m/csf_fit.txt index 3d79edf2813..97f3eea573b 100644 --- a/doc/imx/habv4/csf_examples/mx8m/csf_fit.txt +++ b/doc/imx/habv4/csf_examples/mx8m/csf_fit.txt @@ -27,4 +27,4 @@ Verification index = 2 # FIXME: # Line 1 -- fitImage - Blocks = 0x401fcdc0 0x57c00 0x "flash.bin" + Blocks = CONFIG_SPL_LOAD_FIT_ADDRESS 0x57c00 0x "flash.bin" -- 2.40.1
[PATCH 2/2] ARM: imx: Use default SAVED_DRAM_TIMING_BASE on Data Modul i.MX8M Plus eDM SBC
Use default SAVED_DRAM_TIMING_BASE as that is what upstream TFA expects. Without this change, the board will fail to suspend/resume e.g. in Linux. Signed-off-by: Marek Vasut --- Cc: Fabio Estevam Cc: Peng Fan Cc: Stefano Babic --- configs/imx8mp_data_modul_edm_sbc_defconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/configs/imx8mp_data_modul_edm_sbc_defconfig b/configs/imx8mp_data_modul_edm_sbc_defconfig index 2ff577f2d8a..8b9d4b6c089 100644 --- a/configs/imx8mp_data_modul_edm_sbc_defconfig +++ b/configs/imx8mp_data_modul_edm_sbc_defconfig @@ -164,7 +164,6 @@ CONFIG_SPL_CLK_COMPOSITE_CCF=y CONFIG_CLK_COMPOSITE_CCF=y CONFIG_SPL_CLK_IMX8MP=y CONFIG_CLK_IMX8MP=y -CONFIG_SAVED_DRAM_TIMING_BASE=0x4000 CONFIG_DFU_TFTP=y CONFIG_DFU_TIMEOUT=y CONFIG_DFU_MMC=y -- 2.40.1
[PATCH 1/2] ARM: imx: Use default SAVED_DRAM_TIMING_BASE on Data Modul i.MX8M Mini eDM SBC
Use default SAVED_DRAM_TIMING_BASE as that is what upstream TFA expects. Without this change, the board will fail to suspend/resume e.g. in Linux. Signed-off-by: Marek Vasut --- Cc: Fabio Estevam Cc: Peng Fan Cc: Stefano Babic --- configs/imx8mm_data_modul_edm_sbc_defconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/configs/imx8mm_data_modul_edm_sbc_defconfig b/configs/imx8mm_data_modul_edm_sbc_defconfig index d457f1e3c36..2de336e564c 100644 --- a/configs/imx8mm_data_modul_edm_sbc_defconfig +++ b/configs/imx8mm_data_modul_edm_sbc_defconfig @@ -157,7 +157,6 @@ CONFIG_SPL_CLK_COMPOSITE_CCF=y CONFIG_CLK_COMPOSITE_CCF=y CONFIG_SPL_CLK_IMX8MM=y CONFIG_CLK_IMX8MM=y -CONFIG_SAVED_DRAM_TIMING_BASE=0x4000 CONFIG_DFU_TFTP=y CONFIG_DFU_TIMEOUT=y CONFIG_DFU_MMC=y -- 2.40.1
[PATCH] ARM: imx: Use default SAVED_DRAM_TIMING_BASE on DH i.MX8M Plus DHCOM
Use default SAVED_DRAM_TIMING_BASE as that is what upstream TFA expects. Without this change, the board will fail to suspend/resume e.g. in Linux. Signed-off-by: Marek Vasut --- Cc: Fabio Estevam Cc: Peng Fan Cc: Stefano Babic --- configs/imx8mp_dhcom_pdk2_defconfig | 1 - configs/imx8mp_dhcom_pdk3_defconfig | 1 - 2 files changed, 2 deletions(-) diff --git a/configs/imx8mp_dhcom_pdk2_defconfig b/configs/imx8mp_dhcom_pdk2_defconfig index 14ac499e587..a920ffe09bf 100644 --- a/configs/imx8mp_dhcom_pdk2_defconfig +++ b/configs/imx8mp_dhcom_pdk2_defconfig @@ -160,7 +160,6 @@ CONFIG_SPL_CLK_COMPOSITE_CCF=y CONFIG_CLK_COMPOSITE_CCF=y CONFIG_SPL_CLK_IMX8MP=y CONFIG_CLK_IMX8MP=y -CONFIG_SAVED_DRAM_TIMING_BASE=0x4000 CONFIG_DFU_TFTP=y CONFIG_DFU_TIMEOUT=y CONFIG_DFU_MMC=y diff --git a/configs/imx8mp_dhcom_pdk3_defconfig b/configs/imx8mp_dhcom_pdk3_defconfig index 3b26f315bba..9463456798e 100644 --- a/configs/imx8mp_dhcom_pdk3_defconfig +++ b/configs/imx8mp_dhcom_pdk3_defconfig @@ -161,7 +161,6 @@ CONFIG_SPL_CLK_COMPOSITE_CCF=y CONFIG_CLK_COMPOSITE_CCF=y CONFIG_SPL_CLK_IMX8MP=y CONFIG_CLK_IMX8MP=y -CONFIG_SAVED_DRAM_TIMING_BASE=0x4000 CONFIG_DFU_TFTP=y CONFIG_DFU_TIMEOUT=y CONFIG_DFU_MMC=y -- 2.40.1
Re: [PATCH v2 2/2] armv8: Disable SYS_DCACHE_OFF & SYS_ICACHE_OFF for ARM64
On Thu, 31 Aug 2023 at 19:01, Tom Rini wrote: > > On Thu, Aug 31, 2023 at 10:24:07AM +0200, Michal Simek wrote: > > > > > > On 8/30/23 20:52, Tom Rini wrote: > > > On Tue, Aug 22, 2023 at 01:21:12PM +0530, Bhupesh Sharma wrote: > > > > > > > Ideally SYS_ICACHE_OFF and SYS_DCACHE_OFF should never be set for > > > > ARM64 (ARMv8) platforms, as it makes booting u-boot slower on these > > > > platforms. > > > > > > > > However, if some platforms require ICACHE or DCACHE to be disabled > > > > only for the smaller SPL stage, we should support such configurations > > > > in u-boot as well. > > > > I am missing the reason why this change should be accepted. > > > > > > > > > > Compile-tested for: > > > > - qemu arm64 > > > > qemu doesn't model caches that's why I don't think it is relevant here. > > > > > > - imx8 > > > > - stm32 > > > > > > > > and run-tested on: > > > > - Qualcomm RB3 platform > > > > > > > > Cc: Tom Rini > > > > Cc: Simon Glass > > > > Cc: Peng Fan > > > > Signed-off-by: Bhupesh Sharma > > > > Reviewed-by: Tom Rini > > > > --- > > > > arch/arm/Kconfig| 2 ++ > > > > arch/arm/cpu/armv8/Makefile | 4 ++-- > > > > 2 files changed, 4 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > > > > index 36ee1e9a3c..92bff715e3 100644 > > > > --- a/arch/arm/Kconfig > > > > +++ b/arch/arm/Kconfig > > > > @@ -141,6 +141,7 @@ config THUMB2_KERNEL > > > > config SYS_ICACHE_OFF > > > > bool "Do not enable icache" > > > > + depends on !ARM64 > > > > help > > > > Do not enable instruction cache in U-Boot. > > > > @@ -153,6 +154,7 @@ config SPL_SYS_ICACHE_OFF > > > > config SYS_DCACHE_OFF > > > > bool "Do not enable dcache" > > > > + depends on !ARM64 > > > > help > > > > Do not enable data cache in U-Boot. > > > > diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile > > > > index bba4f570db..0d0c1728e4 100644 > > > > --- a/arch/arm/cpu/armv8/Makefile > > > > +++ b/arch/arm/cpu/armv8/Makefile > > > > @@ -5,13 +5,13 @@ > > > > extra-y := start.o > > > > +obj-y+= cache_v8.o > > > > obj-y += cpu.o > > > > ifndef CONFIG_$(SPL_TPL_)TIMER > > > > obj-$(CONFIG_SYS_ARCH_TIMER) += generic_timer.o > > > > endif > > > > ifndef CONFIG_$(SPL_)SYS_DCACHE_OFF > > > > -obj-y+= cache_v8.o > > > > -obj-y+= cache.o > > > > +obj-y += cache.o > > > > endif > > > > ifdef CONFIG_SPL_BUILD > > > > obj-$(CONFIG_ARMV8_SPL_EXCEPTION_VECTORS) += exceptions.o > > > > > > I'm adding Michal here because this changes the behavior on xilinx > > > platforms that had the icache off. I was under the impression that at > > > the levels U-Boot runs at on aarch64, we just couldn't have > > > icache/dcache off, but I guess that's wrong? > > > > > > > yes. We are using icache off for mini u-boot configurations for quite a long > > time. I tried to find out any commit why we started to use it but didn't > > find any record. But will ask team to retest that use cases to see if that > > is working or not. > > In our case we are using small full U-Boot not any SPL variant. > > > > My biggest issue with the patch is that it is not clear what issue it is > > trying to solve as I have mentioned above. > > It just says that ideally icache/dcache should be on. If you want to enforce > > it on your platform you can select it when that platform is selected and > > don't need enforce it for other platforms. > > Thanks Michal. Bhupesh, I was wrong in v1, having caches off is allowed, > so you just need to fix issues like part one does where code doesn't > compile when the options are disabled, and then are there further > changes needed for your usecase? Sure Tom. I will go back to sending v1 as v3 with some minor changes. Thanks, Bhupesh
[PATCH v2] arm64: zynqmp: Add output-enable pins to SOMs
From: Neal Frager
Now that the zynqmp pinctrl driver supports the tri-state registers, make
sure that the pins requiring output-enable are configured appropriately for
SOMs.
Without it, all tristate setting for MIOs, which are not related to SOM
itself, are using default configuration which is not correct setting.
It means SDs, USBs, ethernet, etc. are not working properly.
In past it was fixed through calling tristate configuration via bootcmd:
usb_init=mw 0xFF180208 2020
kv260_gem3=mw 0xFF18020C 0xFC0 && gpio toggle gpio@ff0a38 && \
gpio toggle gpio@ff0a38
Signed-off-by: Neal Frager
Signed-off-by: Michal Simek
---
Changes in v2:
- update commit message
- add also fixes for kr260-revB and kv260-revA/B
arch/arm/dts/zynqmp-sck-kr-g-revA.dts | 6 ++
arch/arm/dts/zynqmp-sck-kr-g-revB.dts | 6 ++
arch/arm/dts/zynqmp-sck-kv-g-revA.dts | 5 +
arch/arm/dts/zynqmp-sck-kv-g-revB.dts | 5 +
4 files changed, 22 insertions(+)
diff --git a/arch/arm/dts/zynqmp-sck-kr-g-revA.dts
b/arch/arm/dts/zynqmp-sck-kr-g-revA.dts
index d318773bd9d6..30a0230d4767 100644
--- a/arch/arm/dts/zynqmp-sck-kr-g-revA.dts
+++ b/arch/arm/dts/zynqmp-sck-kr-g-revA.dts
@@ -250,6 +250,7 @@
conf-tx {
pins = "MIO36";
bias-disable;
+ output-enable;
};
mux {
@@ -301,6 +302,7 @@
conf-bootstrap {
pins = "MIO45", "MIO47", "MIO49";
bias-disable;
+ output-enable;
low-power-disable;
};
@@ -308,6 +310,7 @@
pins = "MIO38", "MIO39", "MIO40",
"MIO41", "MIO42", "MIO43";
bias-disable;
+ output-enable;
low-power-enable;
};
@@ -316,6 +319,7 @@
slew-rate = ;
power-source = ;
bias-disable;
+ output-enable;
};
mux-mdio {
@@ -346,6 +350,7 @@
pins = "MIO54", "MIO56", "MIO57", "MIO58", "MIO59",
"MIO60", "MIO61", "MIO62", "MIO63";
bias-disable;
+ output-enable;
drive-strength = <4>;
slew-rate = ;
};
@@ -373,6 +378,7 @@
pins = "MIO66", "MIO68", "MIO69", "MIO70", "MIO71",
"MIO72", "MIO73", "MIO74", "MIO75";
bias-disable;
+ output-enable;
drive-strength = <4>;
slew-rate = ;
};
diff --git a/arch/arm/dts/zynqmp-sck-kr-g-revB.dts
b/arch/arm/dts/zynqmp-sck-kr-g-revB.dts
index 69dba0761b37..8f4c52d6d643 100644
--- a/arch/arm/dts/zynqmp-sck-kr-g-revB.dts
+++ b/arch/arm/dts/zynqmp-sck-kr-g-revB.dts
@@ -250,6 +250,7 @@
conf-tx {
pins = "MIO36";
bias-disable;
+ output-enable;
};
mux {
@@ -301,6 +302,7 @@
conf-bootstrap {
pins = "MIO45", "MIO47", "MIO49";
bias-disable;
+ output-enable;
low-power-disable;
};
@@ -308,6 +310,7 @@
pins = "MIO38", "MIO39", "MIO40",
"MIO41", "MIO42", "MIO43";
bias-disable;
+ output-enable;
low-power-enable;
};
@@ -316,6 +319,7 @@
slew-rate = ;
power-source = ;
bias-disable;
+ output-enable;
};
mux-mdio {
@@ -346,6 +350,7 @@
pins = "MIO54", "MIO56", "MIO57", "MIO58", "MIO59",
"MIO60", "MIO61", "MIO62", "MIO63";
bias-disable;
+ output-enable;
drive-strength = <4>;
slew-rate = ;
};
@@ -373,6 +378,7 @@
pins = "MIO66", "MIO68", "MIO69", "MIO70", "MIO71",
"MIO72", "MIO73", "MIO74", "MIO75";
bias-disable;
+ output-enable;
drive-strength = <4>;
slew-rate = ;
};
diff --git a/arch/arm/dts/zynqmp-sck-kv-g-revA.dts
b/arch/arm/dts/zynqmp-sck-kv-g-revA.dts
index a81b3f6f51ad..55bef1df75d0 100644
--- a/arch/arm/dts/zynqmp-sck-kv-g-revA.dts
+++ b/arch/arm/dts/zynqmp-sck-kv-g-revA.dts
@@ -205,6 +205,7 @@
conf-tx {
pins = "MIO36";
Re: [PATCH v2 17/19] Mark DISTRO_DEFAULTS as deprecated
On Wed, Aug 30, 2023 at 09:53:11PM -0600, Simon Glass wrote: > Standard boot has been in place for a while now. Quite a few problems > have been found and fixed. It seems like a good time to mark the > script-based approach as deprecated and encourage people to use standard > boot. > > Update the DISTRO_DEFAULTS Kconfig to encourage people to move to > standard boot, which is able to boot Linux distributions automatically. > > Add a short migration guide to make this easier. > > Signed-off-by: Simon Glass > --- > > Changes in v2: > - Mention in the DISTRO_DEFAULTS option that it is script-based > - Expand and rewrite the commit message > - Use the word 'Mark' instead of 'Make' to improve the English > > boot/Kconfig| 7 ++- > doc/develop/bootstd.rst | 23 +++ > 2 files changed, 29 insertions(+), 1 deletion(-) > > diff --git a/boot/Kconfig b/boot/Kconfig > index 458512a4ade2..a7dea0a0623b 100644 > --- a/boot/Kconfig > +++ b/boot/Kconfig > @@ -784,7 +784,7 @@ config SYS_BOOT_RAMDISK_HIGH > endmenu # Boot images > > config DISTRO_DEFAULTS > - bool "Select defaults suitable for booting general purpose Linux > distributions" > + bool "(deprecated) 'Script-based booting of Linux distributions" Extra ' ? > select BOOT_DEFAULTS > select AUTO_COMPLETE > select CMDLINE_EDITING > @@ -792,6 +792,11 @@ config DISTRO_DEFAULTS > select HUSH_PARSER > select SYS_LONGHELP > help > + Note: These scripts have been replaced by Standard Boot. Do not use > + them on new boards. See 'Migrating from distro_boot' at > + doc/develop/bootstd.rst > + > + Extra newline. -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 14/19] spl: Tidy up load address in spl_ram
On Wed, Aug 30, 2023 at 09:53:08PM -0600, Simon Glass wrote:
> This CONFIG is used but is not given a value by some boards. Use
> a default value of 0 explicitly, rather than relying on the 0 value
> provided by CONFIG_SPL_LOAD_FIT_ADDRESS
>
> This will allow us to make SPL_LOAD_FIT_ADDRESS depend on SPL_LOAD_FIT
> as it should.
>
> Signed-off-by: Simon Glass
> ---
>
> (no changes since v1)
>
> common/spl/spl_ram.c | 19 +--
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/common/spl/spl_ram.c b/common/spl/spl_ram.c
> index 93cf420d810a..4158ed1c32d7 100644
> --- a/common/spl/spl_ram.c
> +++ b/common/spl/spl_ram.c
> @@ -20,12 +20,16 @@
> static ulong spl_ram_load_read(struct spl_load_info *load, ulong sector,
> ulong count, void *buf)
> {
> - ulong addr;
> + ulong addr = 0;
>
> debug("%s: sector %lx, count %lx, buf %lx\n",
> __func__, sector, count, (ulong)buf);
>
> - addr = (ulong)CONFIG_SPL_LOAD_FIT_ADDRESS + sector;
> + if (IS_ENABLED(CONFIG_SPL_LOAD_FIT)) {
> + addr = IF_ENABLED_INT(CONFIG_SPL_LOAD_FIT,
> + CONFIG_SPL_LOAD_FIT_ADDRESS);
> + }
> + addr += sector;
> if (CONFIG_IS_ENABLED(IMAGE_PRE_LOAD))
> addr += image_load_offset;
>
> @@ -38,20 +42,23 @@ static int spl_ram_load_image(struct spl_image_info
> *spl_image,
> struct spl_boot_device *bootdev)
> {
> struct legacy_img_hdr *header;
> + ulong addr = 0;
> int ret;
>
> - header = (struct legacy_img_hdr *)CONFIG_SPL_LOAD_FIT_ADDRESS;
> + if (IS_ENABLED(CONFIG_SPL_LOAD_FIT)) {
> + addr = IF_ENABLED_INT(CONFIG_SPL_LOAD_FIT,
> + CONFIG_SPL_LOAD_FIT_ADDRESS);
> + }
>
> if (CONFIG_IS_ENABLED(IMAGE_PRE_LOAD)) {
> - unsigned long addr = (unsigned long)header;
> ret = image_pre_load(addr);
>
> if (ret)
> return ret;
>
> addr += image_load_offset;
> - header = (struct legacy_img_hdr *)addr;
> }
> + header = map_sysmem(addr, 0);
>
> #if CONFIG_IS_ENABLED(DFU)
> if (bootdev->boot_device == BOOT_DEVICE_DFU)
> @@ -84,7 +91,7 @@ static int spl_ram_load_image(struct spl_image_info
> *spl_image,
> u_boot_pos =
> (ulong)spl_get_load_buffer(-sizeof(*header),
>
> sizeof(*header));
> }
> - header = (struct legacy_img_hdr *)map_sysmem(u_boot_pos, 0);
> + header = map_sysmem(u_boot_pos, 0);
>
> ret = spl_parse_image_header(spl_image, bootdev, header);
> }
This makes the code less readable. Perhaps it needs to be refactored in
to other files instead.
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v2 00/19] Kconfig: Tidy up some options
On Wed, Aug 30, 2023 at 09:52:54PM -0600, Simon Glass wrote:
> The view from 'make menuconfig' is confusing in places. This series aims
> to improve the top-level menu and also the boot menu.
>
> It also groups FDT-fixup options tegether, at least the ones I could fine.
>
> Finally this series marks the distro scripts as deprecated, so people have
> a pointer to standard boot.
>
> Changes in v2:
> - Fix FMU typo in the subject
> - Drop now-unnecessary depends on FWU_MULTI_BANK_UPDATE
> - Mention in the DISTRO_DEFAULTS option that it is script-based
> - Expand and rewrite the commit message
> - Use the word 'Mark' instead of 'Make' to improve the English
>
> Simon Glass (19):
> lib: rational: Move the Kconfigs into the correct place
> Kconfig: Move API into general setup
> video: Hide the BMP options
> video: Move BMP options and code to video directory
> net: Move SYS_RX_ETH_BUFFER into the network menu
> FWU: Avoid showing an unselectable menu option
> test: Move POST under a renamed Testing section
> boot: Move fdt_support to boot/
> Move fdt_simplefb to boot/
> boot: Move some other fdt-fixup options to the same menu
> boot: Rename Android-boot text
> Kconfig: Create a menu for FIT
> Kconfig: Move SPL_FIT under FIT
> spl: Tidy up load address in spl_ram
> boot: Make standard boot a menu
> Kconfig: Move TEXT_BASE et al under general setup
> Mark DISTRO_DEFAULTS as deprecated
> boot: Join FDT_FIXUP_PARTITIONS with related options
> boot: Join ARCH_FIXUP_FDT_MEMORY with related options
>
> Kconfig | 67 ++-
> boot/Kconfig| 202 +---
> boot/Makefile | 4 +
> {common => boot}/fdt_simplefb.c | 0
> {common => boot}/fdt_support.c | 0
> common/Kconfig | 20
> common/Makefile | 4 -
> common/spl/spl_ram.c| 19 ++-
> doc/develop/bootstd.rst | 23
> drivers/video/Kconfig | 11 ++
> drivers/video/Makefile | 2 +
> {common => drivers/video}/bmp.c | 0
> include/net.h | 9 +-
> lib/Kconfig | 17 +--
> lib/fwu_updates/Kconfig | 9 +-
> net/Kconfig | 4 +-
> test/Kconfig| 12 +-
> 17 files changed, 220 insertions(+), 183 deletions(-)
> rename {common => boot}/fdt_simplefb.c (100%)
> rename {common => boot}/fdt_support.c (100%)
> rename {common => drivers/video}/bmp.c (100%)
Please run a world build before/after and check for both size changes
and Kconfig unmet dependency issues (which CI does not catch). Based on
at least the BMP patch I suspect there are problems and unintended
behavior changes.
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v2 06/19] FWU: Avoid showing an unselectable menu option
On Wed, Aug 30, 2023 at 09:53:00PM -0600, Simon Glass wrote: > Use a menuconfig to avoid showing a menu which cannot be selected in many > cases. > > This option should really go with the other 'Update support'. > > Perhaps we should even consider a top-level update/ directory? I think this is where it should be, so please drop these comments from v3 of the series. The rest is fine: Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 05/19] net: Move SYS_RX_ETH_BUFFER into the network menu
On Wed, Aug 30, 2023 at 09:52:59PM -0600, Simon Glass wrote: > Move this Kconfig option into the correct place, so it doesn't show up > in the top-level menu. > > Unfortunately this makes the CONFIG undefined on some boards which don't > enable CONFIG_NET but do include the net.h header file. In fact that > header is included from a lot of common places. So use a fallback in the > header file. > > Signed-off-by: Simon Glass NAK, we leave the option where it is until someone comes and cleans up net.h usage. -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 03/19] video: Hide the BMP options
On Thu, Aug 31, 2023 at 09:58:46AM -0400, Tom Rini wrote: > On Thu, Aug 31, 2023 at 06:05:22AM +0200, Heinrich Schuchardt wrote: > > > > > > Am 31. August 2023 05:52:57 MESZ schrieb Simon Glass : > > >These appear prominently in the main menu at present. However they are > > >selected when needed so do not need to be visible. > > > > > >Make them hidden. > > > > > >Signed-off-by: Simon Glass > > >--- > > > > > >(no changes since v1) > > > > > > common/Kconfig | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > >diff --git a/common/Kconfig b/common/Kconfig > > >index 0b09bd68bd13..844531a59eda 100644 > > >--- a/common/Kconfig > > >+++ b/common/Kconfig > > >@@ -1169,12 +1169,12 @@ config IO_TRACE > > > bool > > > > > > config BMP > > >- bool "Enable bmp image display" > > >+ bool # Enable bmp image display > > Just change this to "bool" and don't add non-prompt text. The help > should be made clearer if it's not already. > > > This must depend on video. > > Since it's only select'd, no, it should be selected with "if VIDEO" if > the options that actually use them don't already depend on video > > > > > > help > > > Enable bmp functions to display bmp image and get bmp info. > > > > > > config SPL_BMP > > >- bool "Enable bmp image display at SPL" > > >+ bool # Enable bmp image display at SPL > > > > Why should I not be able to deactivate the logo display? > > That's SPLASH_SCREEN not BMP. Sorry, you're right Heinrich, the further options here depend on, not select SPL_BMP. -- Tom signature.asc Description: PGP signature
[PATCH v2] mach-imx: bootaux: fix building with disabled bootelf
If CMD_ELF disabled and IMX_BOOTAUX enabled, the u-boot building ends
up with a linking error [1]. Select LIB_ELF to fix the building
issue.
[1]
ld: /tmp/ccaF1rpv.ltrans0.ltrans.o: in function `do_bootaux':
arch/arm/mach-imx/imx_bootaux.c:108: undefined reference to `valid_elf_image'
Fixes: c0f037f6a2a ("mach-imx: bootaux: elf firmware support")
Signed-off-by: Oleksandr Suvorov
---
Changes in v2:
- select LIB_ELF unconditionally
arch/arm/mach-imx/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index d94b5828d0d..fda762426ef 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -32,6 +32,7 @@ config IMX_RDC
config IMX_BOOTAUX
bool "Support boot auxiliary core"
depends on ARCH_MX7 || ARCH_MX6 || ARCH_VF610 || ARCH_IMX8 || ARCH_IMX8M
+ select LIB_ELF
help
bootaux [addr] to boot auxiliary core.
--
2.41.0
Re: [PATCH v2 19/19] boot: Join ARCH_FIXUP_FDT_MEMORY with related options
On Wed, Aug 30, 2023 at 09:53:13PM -0600, Simon Glass wrote: > Move this to be with the other devicetree-fixup options. > > Signed-off-by: Simon Glass > --- > > (no changes since v1) > > boot/Kconfig | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/boot/Kconfig b/boot/Kconfig > index 023446130282..257f4cc085e1 100644 > --- a/boot/Kconfig > +++ b/boot/Kconfig > @@ -693,14 +693,6 @@ config SUPPORT_RAW_INITRD > address of the initrd must be augmented by it's size, in the following > format: ":". > > -config ARCH_FIXUP_FDT_MEMORY > - bool "Enable arch_fixup_memory_banks() call" > - default y > - help > - Enable FDT memory map syncup before OS boot. This feature can be > - used for booting OS with different memory setup where the part of > - the memory location should be used for different purpose. > - > config CHROMEOS > bool "Support booting Chrome OS" > help > @@ -1492,6 +1484,14 @@ config FDT_SIMPLEFB > the presence of the simple frame buffer with associated reserved > memory > > +config ARCH_FIXUP_FDT_MEMORY > + bool "Enable arch_fixup_memory_banks() call" > + default y > + help > + Enable FDT memory map syncup before OS boot. This feature can be > + used for booting OS with different memory setup where the part of > + the memory location should be used for different purpose. > + > endmenu > > endif # OF_LIBFDT It seems like fixups are mixed in with other changes in this menu when we should probably keep the fixups lumped together at least. -- Tom signature.asc Description: PGP signature
