Re: [PATCH v2 10/10] arch: arm: dts: k3-am62-sk-u-boot: Add missing "bootph-all" property to phy_gmii_sel node
On 20/05/24 17:43, Roger Quadros wrote:
On 20/05/2024 09:04, Chintan Vankar wrote:
On 25/04/24 18:06, Chintan Vankar wrote:
On 25/04/24 18:01, Roger Quadros wrote:
On 25/04/2024 15:08, Chintan Vankar wrote:
Add "bootph-all" property to CPSW MAC's PHY node phy_gmii_sel.
Signed-off-by: Chintan Vankar
---
Changes from v1 to v2:
- This patch is newly added in this series to enable CPSW MAC's PHY
node "phy_gmii_sel". As per discussion at here:
https://lore.kernel.org/r/20240112130127.rvvrhz7p4vmlyalh@smother/
changes made by this patch can be dropped in the future when the
DT-Sync is performed with am62-main.dtsi containing this change in
the Linux DT which will match U-Boot's DT.
I don't think bootph-all exists in am62-main.dtsi. It should come from
board.dts
Yes, I am having the same discussion at here:
https://lore.kernel.org/all/c13ac165-7cbd-4e53-914e-8c6bc2825...@ti.com/
Since I have posted patch which adds bootph-all property to
"k3-am62x-sk-common.dtsi" at here:
https://lore.kernel.org/r/20240430085048.3143665-1-c-van...@ti.com/
and it has no open comments and this series
also does not have any open comments, so can it be merged ?
Since bootph-all is being added to k3-am62x-sk-common.dtsi don't you have to
drop patch 10
from this series?
Yes you are correct, but that patch is not merged yet.
arch/arm/dts/k3-am625-sk-u-boot.dtsi | 4
1 file changed, 4 insertions(+)
diff --git a/arch/arm/dts/k3-am625-sk-u-boot.dtsi
b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
index fa778b0ff4..e9a1afde95 100644
--- a/arch/arm/dts/k3-am625-sk-u-boot.dtsi
+++ b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
@@ -46,3 +46,7 @@
_port2 {
status = "disabled";
};
+
+_gmii_sel {
+ bootph-all;
+};
Re: [PATCH v2 06/10] arm: mach-k3: am625_init: Probe AM65 CPSW NUSS
On 20/05/24 17:42, Roger Quadros wrote:
On 25/04/2024 15:59, Chintan Vankar wrote:
On 25/04/24 17:57, Roger Quadros wrote:
On 25/04/2024 15:08, Chintan Vankar wrote:
From: Kishon Vijay Abraham I
In order to support Ethernet boot on AM62x, probe AM65 CPSW NUSS
driver in board_init_f().
Signed-off-by: Kishon Vijay Abraham I
Signed-off-by: Siddharth Vadapalli
Signed-off-by: Chintan Vankar
---
Link to v1:
https://lore.kernel.org/r/20240112064759.1801600-8-s-vadapa...@ti.com/
Changes from v1 to v2:
- No changes.
arch/arm/mach-k3/am625_init.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/arch/arm/mach-k3/am625_init.c b/arch/arm/mach-k3/am625_init.c
index 668f9a51ef..9bf61b1e83 100644
--- a/arch/arm/mach-k3/am625_init.c
+++ b/arch/arm/mach-k3/am625_init.c
@@ -277,6 +277,16 @@ void board_init_f(ulong dummy)
if (ret)
panic("DRAM init failed: %d\n", ret);
}
+
+ if (IS_ENABLED(CONFIG_SPL_ETH) && IS_ENABLED(CONFIG_TI_AM65_CPSW_NUSS) &&
+ spl_boot_device() == BOOT_DEVICE_ETHERNET) {
+ struct udevice *cpswdev;
+
+ if (uclass_get_device_by_driver(UCLASS_MISC,
DM_DRIVER_GET(am65_cpsw_nuss),
+ ))
+ printf("Failed to probe am65_cpsw_nuss driver\n");
+ }
+
This looks like a HACK.
The network driver should be probed only when the networking feature is
actually required.
Driver is probed only when the condition above
"spl_boot_device() == BOOT_DEVICE_ETHERNET" is true, which says Boot
device is Ethernet, and here we are booting with Ethernet so driver is
getting probed.
I think you misunderstood what I was saying as am625_init.c is not using
any Ethernet function it should not cause the Ethernet driver to probe.
Instead it should be probed by the networking use case.
What happens if you don't use this patch? Where is it failing?
You are correct that it should be probed by the networking use case and
we are using networking functionality of "Booting via Ethernet".
Regarding its probing, I already had discussion that why do I have to
probe it explicitly at here:
https://lore.kernel.org/r/ee1d16fd-b99b-4b07-97bb-a896e1791...@ti.com/
Now if I don't use this patch then Ethernet will not get initialized at
SPL stage, and in "spl_net_load_image()" function, there will be an
error saying "No Ethernet Found", and since we selected booting via
Ethernet it will fail to boot.
spl_enable_cache();
fixup_a53_cpu_freq_by_speed_grade();
Re: [PATCH v2 10/10] arch: arm: dts: k3-am62-sk-u-boot: Add missing "bootph-all" property to phy_gmii_sel node
On 25/04/24 18:06, Chintan Vankar wrote:
On 25/04/24 18:01, Roger Quadros wrote:
On 25/04/2024 15:08, Chintan Vankar wrote:
Add "bootph-all" property to CPSW MAC's PHY node phy_gmii_sel.
Signed-off-by: Chintan Vankar
---
Changes from v1 to v2:
- This patch is newly added in this series to enable CPSW MAC's PHY
node "phy_gmii_sel". As per discussion at here:
https://lore.kernel.org/r/20240112130127.rvvrhz7p4vmlyalh@smother/
changes made by this patch can be dropped in the future when the
DT-Sync is performed with am62-main.dtsi containing this change in
the Linux DT which will match U-Boot's DT.
I don't think bootph-all exists in am62-main.dtsi. It should come from
board.dts
Yes, I am having the same discussion at here:
https://lore.kernel.org/all/c13ac165-7cbd-4e53-914e-8c6bc2825...@ti.com/
Since I have posted patch which adds bootph-all property to
"k3-am62x-sk-common.dtsi" at here:
https://lore.kernel.org/r/20240430085048.3143665-1-c-van...@ti.com/
and it has no open comments and this series
also does not have any open comments, so can it be merged ?
arch/arm/dts/k3-am625-sk-u-boot.dtsi | 4
1 file changed, 4 insertions(+)
diff --git a/arch/arm/dts/k3-am625-sk-u-boot.dtsi
b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
index fa778b0ff4..e9a1afde95 100644
--- a/arch/arm/dts/k3-am625-sk-u-boot.dtsi
+++ b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
@@ -46,3 +46,7 @@
_port2 {
status = "disabled";
};
+
+_gmii_sel {
+ bootph-all;
+};
Re: [PATCH v2 06/10] arm: mach-k3: am625_init: Probe AM65 CPSW NUSS
On 25/04/24 17:57, Roger Quadros wrote:
On 25/04/2024 15:08, Chintan Vankar wrote:
From: Kishon Vijay Abraham I
In order to support Ethernet boot on AM62x, probe AM65 CPSW NUSS
driver in board_init_f().
Signed-off-by: Kishon Vijay Abraham I
Signed-off-by: Siddharth Vadapalli
Signed-off-by: Chintan Vankar
---
Link to v1:
https://lore.kernel.org/r/20240112064759.1801600-8-s-vadapa...@ti.com/
Changes from v1 to v2:
- No changes.
arch/arm/mach-k3/am625_init.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/arch/arm/mach-k3/am625_init.c b/arch/arm/mach-k3/am625_init.c
index 668f9a51ef..9bf61b1e83 100644
--- a/arch/arm/mach-k3/am625_init.c
+++ b/arch/arm/mach-k3/am625_init.c
@@ -277,6 +277,16 @@ void board_init_f(ulong dummy)
if (ret)
panic("DRAM init failed: %d\n", ret);
}
+
+ if (IS_ENABLED(CONFIG_SPL_ETH) && IS_ENABLED(CONFIG_TI_AM65_CPSW_NUSS)
&&
+ spl_boot_device() == BOOT_DEVICE_ETHERNET) {
+ struct udevice *cpswdev;
+
+ if (uclass_get_device_by_driver(UCLASS_MISC,
DM_DRIVER_GET(am65_cpsw_nuss),
+ ))
+ printf("Failed to probe am65_cpsw_nuss driver\n");
+ }
+
This looks like a HACK.
The network driver should be probed only when the networking feature is
actually required.
Driver is probed only when the condition above
"spl_boot_device() == BOOT_DEVICE_ETHERNET" is true, which says Boot
device is Ethernet, and here we are booting with Ethernet so driver is
getting probed.
spl_enable_cache();
fixup_a53_cpu_freq_by_speed_grade();
Re: [PATCH v2 10/10] arch: arm: dts: k3-am62-sk-u-boot: Add missing "bootph-all" property to phy_gmii_sel node
On 25/04/24 18:01, Roger Quadros wrote:
On 25/04/2024 15:08, Chintan Vankar wrote:
Add "bootph-all" property to CPSW MAC's PHY node phy_gmii_sel.
Signed-off-by: Chintan Vankar
---
Changes from v1 to v2:
- This patch is newly added in this series to enable CPSW MAC's PHY
node "phy_gmii_sel". As per discussion at here:
https://lore.kernel.org/r/20240112130127.rvvrhz7p4vmlyalh@smother/
changes made by this patch can be dropped in the future when the
DT-Sync is performed with am62-main.dtsi containing this change in
the Linux DT which will match U-Boot's DT.
I don't think bootph-all exists in am62-main.dtsi. It should come from
board.dts
Yes, I am having the same discussion at here:
https://lore.kernel.org/all/c13ac165-7cbd-4e53-914e-8c6bc2825...@ti.com/
arch/arm/dts/k3-am625-sk-u-boot.dtsi | 4
1 file changed, 4 insertions(+)
diff --git a/arch/arm/dts/k3-am625-sk-u-boot.dtsi
b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
index fa778b0ff4..e9a1afde95 100644
--- a/arch/arm/dts/k3-am625-sk-u-boot.dtsi
+++ b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
@@ -46,3 +46,7 @@
_port2 {
status = "disabled";
};
+
+_gmii_sel {
+ bootph-all;
+};
[PATCH v2 10/10] arch: arm: dts: k3-am62-sk-u-boot: Add missing "bootph-all" property to phy_gmii_sel node
Add "bootph-all" property to CPSW MAC's PHY node phy_gmii_sel.
Signed-off-by: Chintan Vankar
---
Changes from v1 to v2:
- This patch is newly added in this series to enable CPSW MAC's PHY
node "phy_gmii_sel". As per discussion at here:
https://lore.kernel.org/r/20240112130127.rvvrhz7p4vmlyalh@smother/
changes made by this patch can be dropped in the future when the
DT-Sync is performed with am62-main.dtsi containing this change in
the Linux DT which will match U-Boot's DT.
arch/arm/dts/k3-am625-sk-u-boot.dtsi | 4
1 file changed, 4 insertions(+)
diff --git a/arch/arm/dts/k3-am625-sk-u-boot.dtsi
b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
index fa778b0ff4..e9a1afde95 100644
--- a/arch/arm/dts/k3-am625-sk-u-boot.dtsi
+++ b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
@@ -46,3 +46,7 @@
_port2 {
status = "disabled";
};
+
+_gmii_sel {
+ bootph-all;
+};
--
2.34.1
[PATCH v2 08/10] configs: am62: Enable configs required for Ethboot
From: Kishon Vijay Abraham I Enable config options needed to support Ethernet boot on AM62x SK. Signed-off-by: Kishon Vijay Abraham I Signed-off-by: Siddharth Vadapalli Signed-off-by: Chintan Vankar --- Link to v1: https://lore.kernel.org/r/20240112064759.1801600-10-s-vadapa...@ti.com/ Changes from v1 to v2: - Removed independent defconfig files for A53 configs and created a config fragment for them as suggested by Nishanth. configs/am62x_evm_a53_ethboot_defconfig | 11 +++ 1 file changed, 11 insertions(+) create mode 100644 configs/am62x_evm_a53_ethboot_defconfig diff --git a/configs/am62x_evm_a53_ethboot_defconfig b/configs/am62x_evm_a53_ethboot_defconfig new file mode 100644 index 00..e49221ee23 --- /dev/null +++ b/configs/am62x_evm_a53_ethboot_defconfig @@ -0,0 +1,11 @@ +#include + +CONFIG_SPL_STACK_R_ADDR=0x8300 +CONFIG_SPL_DRIVERS_MISC=y +CONFIG_SPL_BOARD_INIT=y +CONFIG_SPL_DMA=y +CONFIG_SPL_ENV_SUPPORT=y +CONFIG_SPL_ETH=y +CONFIG_SPL_NET=y +CONFIG_SPL_NET_VCI_STRING="AM62X U-Boot A53 SPL" +CONFIG_SPL_SYSCON=y -- 2.34.1
[PATCH v2 09/10] arm: dts: k3-am625-r5-sk: Enable DM services for main_pktdma
From: Siddharth Vadapalli
Enable DM services for main_pktdma during R5 SPL stage.
Signed-off-by: Siddharth Vadapalli
Signed-off-by: Chintan Vankar
---
Link to v1:
https://lore.kernel.org/r/20240112064759.1801600-11-s-vadapa...@ti.com/
Changes from v1 to v2:
- No changes.
arch/arm/dts/k3-am625-r5-sk.dts | 5 +
1 file changed, 5 insertions(+)
diff --git a/arch/arm/dts/k3-am625-r5-sk.dts b/arch/arm/dts/k3-am625-r5-sk.dts
index 6b9f40e555..0912b953db 100644
--- a/arch/arm/dts/k3-am625-r5-sk.dts
+++ b/arch/arm/dts/k3-am625-r5-sk.dts
@@ -83,3 +83,8 @@
reg = <0x00 0x0fc4 0x00 0x100>,
<0x00 0x6000 0x00 0x0800>;
};
+
+_pktdma {
+ ti,sci = <_tifs>;
+ bootph-all;
+};
--
2.34.1
[PATCH v2 07/10] configs: am62: Add configs for enabling ETHBOOT in R5SPL
From: Kishon Vijay Abraham I Add configs for enabling ETHBOOT in R5SPL. Signed-off-by: Kishon Vijay Abraham I Signed-off-by: Andreas Dannenberg Signed-off-by: Siddharth Vadapalli Signed-off-by: Chintan Vankar --- Link to v1: https://lore.kernel.org/r/20240112064759.1801600-9-s-vadapa...@ti.com/ Changes from v1 to v2: - Removed independent defconfig files for R5 configs and created a config fragment for them as suggested by Nishanth. configs/am62x_evm_r5_ethboot_defconfig | 19 +++ 1 file changed, 19 insertions(+) create mode 100644 configs/am62x_evm_r5_ethboot_defconfig diff --git a/configs/am62x_evm_r5_ethboot_defconfig b/configs/am62x_evm_r5_ethboot_defconfig new file mode 100644 index 00..5100165aa2 --- /dev/null +++ b/configs/am62x_evm_r5_ethboot_defconfig @@ -0,0 +1,19 @@ +#include + +CONFIG_SPL_GPIO=y +CONFIG_SPL_MMC=n +CONFIG_SPL_BOARD_INIT=y +CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x20 +CONFIG_SPL_DMA=y +CONFIG_SPL_ENV_SUPPORT=y +CONFIG_SPL_ETH=y +CONFIG_SPL_I2C=y +CONFIG_SPL_NET=y +CONFIG_SPL_NET_VCI_STRING="AM62X U-Boot R5 SPL" +CONFIG_CMD_DHCP=y +CONFIG_SPL_SYSCON=y +CONFIG_DMA_CHANNELS=y +CONFIG_TI_K3_NAVSS_UDMA=y +CONFIG_DM_I2C=y +CONFIG_PHY_TI_DP83867=y +CONFIG_TI_AM65_CPSW_NUSS=y -- 2.34.1
[PATCH v2 06/10] arm: mach-k3: am625_init: Probe AM65 CPSW NUSS
From: Kishon Vijay Abraham I
In order to support Ethernet boot on AM62x, probe AM65 CPSW NUSS
driver in board_init_f().
Signed-off-by: Kishon Vijay Abraham I
Signed-off-by: Siddharth Vadapalli
Signed-off-by: Chintan Vankar
---
Link to v1:
https://lore.kernel.org/r/20240112064759.1801600-8-s-vadapa...@ti.com/
Changes from v1 to v2:
- No changes.
arch/arm/mach-k3/am625_init.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/arch/arm/mach-k3/am625_init.c b/arch/arm/mach-k3/am625_init.c
index 668f9a51ef..9bf61b1e83 100644
--- a/arch/arm/mach-k3/am625_init.c
+++ b/arch/arm/mach-k3/am625_init.c
@@ -277,6 +277,16 @@ void board_init_f(ulong dummy)
if (ret)
panic("DRAM init failed: %d\n", ret);
}
+
+ if (IS_ENABLED(CONFIG_SPL_ETH) && IS_ENABLED(CONFIG_TI_AM65_CPSW_NUSS)
&&
+ spl_boot_device() == BOOT_DEVICE_ETHERNET) {
+ struct udevice *cpswdev;
+
+ if (uclass_get_device_by_driver(UCLASS_MISC,
DM_DRIVER_GET(am65_cpsw_nuss),
+ ))
+ printf("Failed to probe am65_cpsw_nuss driver\n");
+ }
+
spl_enable_cache();
fixup_a53_cpu_freq_by_speed_grade();
--
2.34.1
[PATCH v2 04/10] soc: ti: k3-navss-ringacc: Fix reset ring API
From: Vignesh Raghavendra
Expectation of k3_ringacc_ring_reset_raw() is to reset the ring to
requested size and not to 0. Fix this.
Signed-off-by: Vignesh Raghavendra
Signed-off-by: Siddharth Vadapalli
Signed-off-by: Chintan Vankar
---
Link to v1:
https://lore.kernel.org/r/20240112064759.1801600-5-s-vadapa...@ti.com/
Changes from v1 to v2:
- No changes.
drivers/soc/ti/k3-navss-ringacc-u-boot.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/ti/k3-navss-ringacc-u-boot.c
b/drivers/soc/ti/k3-navss-ringacc-u-boot.c
index f958239c2a..5d650b9de7 100644
--- a/drivers/soc/ti/k3-navss-ringacc-u-boot.c
+++ b/drivers/soc/ti/k3-navss-ringacc-u-boot.c
@@ -25,9 +25,16 @@ struct k3_nav_ring_cfg_regs {
#define KNAV_RINGACC_CFG_RING_SIZE_ELSIZE_MASK GENMASK(26, 24)
#define KNAV_RINGACC_CFG_RING_SIZE_ELSIZE_SHIFT(24)
+#define KNAV_RINGACC_CFG_RING_SIZE_MASKGENMASK(15, 0)
+
static void k3_ringacc_ring_reset_raw(struct k3_nav_ring *ring)
{
- writel(0, >cfg->size);
+ u32 reg;
+
+ reg = readl(>cfg->size);
+ reg &= KNAV_RINGACC_CFG_RING_SIZE_MASK;
+ reg |= ring->size;
+ writel(reg, >cfg->size);
}
static void k3_ringacc_ring_reconfig_qmode_raw(struct k3_nav_ring *ring, enum
k3_nav_ring_mode mode)
--
2.34.1
[PATCH v2 05/10] dma: ti: k3-udma: Add support for native configuration of chan/flow
From: Kishon Vijay Abraham I In absence of Device Manager (DM) services such as at R5 SPL stage, driver will have to natively setup TCHAN/RCHAN/RFLOW cfg registers. Existing UDMA driver performed the above mentioned configuration for UDMA. Add similar configuration for PKTDMA here. Signed-off-by: Kishon Vijay Abraham I Signed-off-by: Siddharth Vadapalli Signed-off-by: Chintan Vankar --- Link to v1: https://lore.kernel.org/r/20240112064759.1801600-6-s-vadapa...@ti.com/ Changes from v1 to v2: - No changes. drivers/dma/ti/k3-udma.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c index ef3074aa13..ad3fa4a515 100644 --- a/drivers/dma/ti/k3-udma.c +++ b/drivers/dma/ti/k3-udma.c @@ -2119,6 +2119,9 @@ static int bcdma_tisci_tx_channel_config(struct udma_chan *uc) if (ret) dev_err(ud->dev, "tchan%d cfg failed %d\n", tchan->id, ret); + if (IS_ENABLED(CONFIG_K3_DM_FW)) + udma_alloc_tchan_raw(uc); + return ret; } @@ -2167,6 +2170,9 @@ static int pktdma_tisci_rx_channel_config(struct udma_chan *uc) dev_err(ud->dev, "flow%d config failed: %d\n", uc->rflow->id, ret); + if (IS_ENABLED(CONFIG_K3_DM_FW)) + udma_alloc_rchan_raw(uc); + return ret; } -- 2.34.1
[PATCH v2 03/10] soc: ti: k3-navss-ringacc: Initialize base address of ring cfg registers
From: Kishon Vijay Abraham I
Initialize base address of ring config registers required to natively
setup ring cfg registers in the absence of Device Manager (DM) services
at R5 SPL stage.
Signed-off-by: Kishon Vijay Abraham I
Signed-off-by: Siddharth Vadapalli
Signed-off-by: Chintan Vankar
---
Link to v1:
https://lore.kernel.org/r/20240112064759.1801600-4-s-vadapa...@ti.com/
Changes from v1 to v2:
- No changes.
drivers/soc/ti/k3-navss-ringacc.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/ti/k3-navss-ringacc.c
b/drivers/soc/ti/k3-navss-ringacc.c
index ed39ff2fa4..0a96ed666c 100644
--- a/drivers/soc/ti/k3-navss-ringacc.c
+++ b/drivers/soc/ti/k3-navss-ringacc.c
@@ -1030,8 +1030,8 @@ static int k3_nav_ringacc_init(struct udevice *dev,
struct k3_nav_ringacc *ringa
struct k3_nav_ringacc *k3_ringacc_dmarings_init(struct udevice *dev,
struct k3_ringacc_init_data
*data)
{
+ void __iomem *base_rt, *base_cfg;
struct k3_nav_ringacc *ringacc;
- void __iomem *base_rt;
int i;
ringacc = devm_kzalloc(dev, sizeof(*ringacc), GFP_KERNEL);
@@ -1049,6 +1049,10 @@ struct k3_nav_ringacc *k3_ringacc_dmarings_init(struct
udevice *dev,
if (!base_rt)
return ERR_PTR(-EINVAL);
+ base_cfg = dev_read_addr_name_ptr(dev, "cfg");
+ if (!base_cfg)
+ return ERR_PTR(-EINVAL);
+
ringacc->rings = devm_kzalloc(dev,
sizeof(*ringacc->rings) *
ringacc->num_rings * 2,
@@ -1063,6 +1067,7 @@ struct k3_nav_ringacc *k3_ringacc_dmarings_init(struct
udevice *dev,
for (i = 0; i < ringacc->num_rings; i++) {
struct k3_nav_ring *ring = >rings[i];
+ ring->cfg = base_cfg + KNAV_RINGACC_CFG_REGS_STEP * i;
ring->rt = base_rt + K3_DMARING_RING_RT_REGS_STEP * i;
ring->parent = ringacc;
ring->ring_id = i;
--
2.34.1
[PATCH v2 02/10] firmware: ti_sci: Add No-OP for "RX_FL_CFG"
From: Kishon Vijay Abraham I
RX_FL_CFG message should not be forwarded to TIFS and should be
handled within R5 SPL (when DM services are not available). Add
a no-op function to not handle RX_FL_CFG messages.
Signed-off-by: Kishon Vijay Abraham I
Signed-off-by: Siddharth Vadapalli
Signed-off-by: Chintan Vankar
---
Link to v1:
https://lore.kernel.org/r/20240112064759.1801600-3-s-vadapa...@ti.com/
Changes from v1 to v2:
- No changes.
drivers/firmware/ti_sci.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 6c581b9df9..2e9cc1d86c 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -2452,6 +2452,12 @@ fail:
return ret;
}
+static int ti_sci_cmd_rm_udmap_rx_flow_cfg_noop(const struct ti_sci_handle
*handle,
+ const struct
ti_sci_msg_rm_udmap_flow_cfg *params)
+{
+ return 0;
+}
+
/**
* ti_sci_cmd_set_fwl_region() - Request for configuring a firewall region
* @handle:pointer to TI SCI handle
@@ -2897,7 +2903,7 @@ static __maybe_unused int ti_sci_dm_probe(struct udevice
*dev)
udmap_ops = >rm_udmap_ops;
udmap_ops->tx_ch_cfg = ti_sci_cmd_rm_udmap_tx_ch_cfg;
udmap_ops->rx_ch_cfg = ti_sci_cmd_rm_udmap_rx_ch_cfg;
- udmap_ops->rx_flow_cfg = ti_sci_cmd_rm_udmap_rx_flow_cfg;
+ udmap_ops->rx_flow_cfg = ti_sci_cmd_rm_udmap_rx_flow_cfg_noop;
return ret;
}
--
2.34.1
[PATCH v2 00/10] Add support for Ethernet Boot on SK-AM62
This series enables Ethernet Boot on SK-AM62 device. Link to v1: https://lore.kernel.org/all/20240112064759.1801600-1-s-vadapa...@ti.com/ Changes from v1 to v2: - Updated dram_init_banksize() function to be called from common section instead of board specific function call as suggested by Tom. - Dropped [PATCH 06/10] from this version since it was a clean up patch and not required for AM62x Ethernet Boot functionality. - Removed independent defconfig files for R5 and A53 configs and created a config fragment for them as suggested by Nishanth. - Added new patch [PATCH v2 10/10] in this series to enable CPSW MAC's PHY node "phy_gmii_sel" which is required for Ethernet Boot. This changes are done as per discussion here: https://lore.kernel.org/r/20240112064759.1801600-1-s-vadapa...@ti.com/ Logs: https://gist.github.com/chintanv133/07bcb1473301581aab6d4e951d9c6572 Chintan Vankar (2): common: spl: spl: Init DRAM size in R5/A53 SPL arch: arm: dts: k3-am62-sk-u-boot: Add missing "bootph-all" property to phy_gmii_sel node Kishon Vijay Abraham I (6): firmware: ti_sci: Add No-OP for "RX_FL_CFG" soc: ti: k3-navss-ringacc: Initialize base address of ring cfg registers dma: ti: k3-udma: Add support for native configuration of chan/flow arm: mach-k3: am625_init: Probe AM65 CPSW NUSS configs: am62: Add configs for enabling ETHBOOT in R5SPL configs: am62: Enable configs required for Ethboot Siddharth Vadapalli (1): arm: dts: k3-am625-r5-sk: Enable DM services for main_pktdma Vignesh Raghavendra (1): soc: ti: k3-navss-ringacc: Fix reset ring API arch/arm/dts/k3-am625-r5-sk.dts | 5 + arch/arm/dts/k3-am625-sk-u-boot.dtsi | 4 arch/arm/mach-k3/am625_init.c| 10 ++ common/spl/spl.c | 2 +- configs/am62x_evm_a53_ethboot_defconfig | 11 +++ configs/am62x_evm_r5_ethboot_defconfig | 19 +++ drivers/dma/ti/k3-udma.c | 6 ++ drivers/firmware/ti_sci.c| 8 +++- drivers/soc/ti/k3-navss-ringacc-u-boot.c | 9 - drivers/soc/ti/k3-navss-ringacc.c| 7 ++- 10 files changed, 77 insertions(+), 4 deletions(-) create mode 100644 configs/am62x_evm_a53_ethboot_defconfig create mode 100644 configs/am62x_evm_r5_ethboot_defconfig -- 2.34.1
[PATCH v2 01/10] common: spl: spl: Init DRAM size in R5/A53 SPL
Initialize DRAM size in SPL stage since networking requires DDR
to be initialized.
Signed-off-by: Chintan Vankar
---
Link to v1:
https://lore.kernel.org/r/20240112064759.1801600-2-s-vadapa...@ti.com/
Changes from v1 to v2:
- Updated dram_init_banksize() function to be called from common section
instead of board specific function call as suggested by Tom.
common/spl/spl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c
index e06bc75d36..2a9d58195c 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -718,7 +718,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
initr_watchdog();
if (IS_ENABLED(CONFIG_SPL_OS_BOOT) || CONFIG_IS_ENABLED(HANDOFF) ||
- IS_ENABLED(CONFIG_SPL_ATF))
+ IS_ENABLED(CONFIG_SPL_ATF) || IS_ENABLED(CONFIG_SPL_NET))
dram_init_banksize();
if (CONFIG_IS_ENABLED(PCI) && !(gd->flags & GD_FLG_DM_DEAD)) {
--
2.34.1
Re: [PATCH 03/10] soc: ti: k3-navss-ringacc: Initialize base address of ring cfg registers
On 16/01/24 17:13, Roger Quadros wrote:
Hi,
On 12/01/2024 08:47, Siddharth Vadapalli wrote:
From: Kishon Vijay Abraham I
Initialize base address of ring config registers required to natively
setup ring cfg registers in the absence of Device Manager (DM) services
at R5 SPL stage.
Signed-off-by: Kishon Vijay Abraham I
Signed-off-by: Siddharth Vadapalli
---
drivers/soc/ti/k3-navss-ringacc.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/ti/k3-navss-ringacc.c
b/drivers/soc/ti/k3-navss-ringacc.c
index 7a2fbb0db6..31e9b372ee 100644
--- a/drivers/soc/ti/k3-navss-ringacc.c
+++ b/drivers/soc/ti/k3-navss-ringacc.c
@@ -1030,8 +1030,8 @@ static int k3_nav_ringacc_init(struct udevice *dev,
struct k3_nav_ringacc *ringa
struct k3_nav_ringacc *k3_ringacc_dmarings_init(struct udevice *dev,
struct k3_ringacc_init_data
*data)
{
+ void __iomem *base_rt, *base_cfg;
struct k3_nav_ringacc *ringacc;
- void __iomem *base_rt;
int i;
ringacc = devm_kzalloc(dev, sizeof(*ringacc), GFP_KERNEL);
@@ -1049,6 +1049,10 @@ struct k3_nav_ringacc *k3_ringacc_dmarings_init(struct
udevice *dev,
if (!base_rt)
return ERR_PTR(-EINVAL);
+ base_cfg = dev_read_addr_name_ptr(dev, "cfg");
+ if (!base_cfg)
+ return ERR_PTR(-EINVAL);
+
Should this be restricted only for R5 SPL case? else we conflict with
Device Manager services?
I don't see any conflict in it, you can go through this:
https://github.com/u-boot/u-boot/commit/86e58800fd7cdba4fa9229aeee3a54a2ccece406
ringacc->rings = devm_kzalloc(dev,
sizeof(*ringacc->rings) *
ringacc->num_rings * 2,
@@ -1063,6 +1067,7 @@ struct k3_nav_ringacc *k3_ringacc_dmarings_init(struct
udevice *dev,
for (i = 0; i < ringacc->num_rings; i++) {
struct k3_nav_ring *ring = >rings[i];
+ ring->cfg = base_cfg + KNAV_RINGACC_CFG_REGS_STEP * i;
ring->rt = base_rt + K3_DMARING_RING_RT_REGS_STEP * i;
ring->parent = ringacc;
ring->ring_id = i;
Re: [PATCH 01/10] board: ti: am62x: Init DRAM size in R5/A53 SPL
On 17/04/24 21:34, Tom Rini wrote: On Wed, Apr 17, 2024 at 05:48:31PM +0530, Sughosh Ganu wrote: hi Chintan, On Wed, 17 Apr 2024 at 13:21, Chintan Vankar wrote: On 16/04/24 22:30, Tom Rini wrote: On Tue, Apr 16, 2024 at 05:52:58PM +0530, Chintan Vankar wrote: On 12/04/24 03:37, Tom Rini wrote: On Wed, Apr 03, 2024 at 06:18:01PM +0530, Chintan Vankar wrote: On 22/01/24 10:11, Siddharth Vadapalli wrote: On 20/01/24 22:11, Tom Rini wrote: On Mon, Jan 15, 2024 at 01:42:51PM +0530, Siddharth Vadapalli wrote: Hello Tom, On 12/01/24 18:56, Tom Rini wrote: ... The list of conditionals in common/spl/spl.c::board_init_r() should be updated and probably use SPL_NET as the option to check for. Thank you for reviewing the patch and pointing this out. I wasn't aware of it. I assume that you are referring to the following change: if (IS_ENABLED(CONFIG_SPL_OS_BOOT) || CONFIG_IS_ENABLED(HANDOFF) || - IS_ENABLED(CONFIG_SPL_ATF)) + IS_ENABLED(CONFIG_SPL_ATF) || IS_ENABLED(CONFIG_SPL_NET)) dram_init_banksize(); I shall replace the current patch with the above change in the v2 series. Since this is in the common section, is there a generic reason I could provide in the commit message rather than the existing commit message which seems to be board specific? Also, I hope that the above change will not cause regressions for other non-TI devices. Please let me know. Yes, that's the area, and just note that networking also requires the DDR to be initialized. Thank you for confirming and providing your suggestion for the contents of the commit message. Following Tom's Suggestion of adding CONFIG_SPL_NET in common/spl/spl.c "dram_init_banksize()", the issue of fetching a file at SPL stage seemed to be fixed. However the commit "ba20b2443c29", which sets gd->ram_top for the very first time in "spl_enable_cache()" results in "arch_lmb_reserve()" function reserving memory region from Stack pointer at "0x81FFB820" to gd->ram_top pointing to "0x1". Previously when gd->ram_top was zero "arch_lmb_reserve()" was noop. Now using TFTP to fetch U-Boot image at SPL stage results in "tftp_init_load_addr()" function call that invokes "arch_lmb_reserve()" function, which reserves entire memory starting from Stack Pointer to gd->ram_top leaving no space to load U-Boot image via TFTP since TFTP loads files at pre configured memory address at "0x8200". As a workaround for this issue, one solution we can propose is to disable the checks "lmb_get_free_size()" at SPL and U-Boot stage. For that we can define a new config option for LMB reserve checks as "SPL_LMB". This config will be enable by default for the backword compatibility and disable for our use case at SPL and U-Boot stage. The problem here is that we need LMB for booting an OS, which is something we'll want in SPL in non-cortex-R cases too, which means this platform, so that's a no-go. I think you need to dig harder and see if you can correct the logic somewhere so that we don't over reserve? Since this issue is due to function call "lmb_init_and_reserve()" function invoked from "tftp_init_load_addr()" function. This function is defined by Simon in commit "a156c47e39ad", which fixes "CVE-2018-18439" to prevent overwriting reserved memory. Simon, can you explain why do we need to call "lmb_init_and_reserve()" function here ? This is indeed a tricky area which is why Sughosh is looking in to trying to re-work the LMB mechanic and we've had a few long threads about it as well. I've honestly forgotten the use case you have here, can you please remind us? We are trying to boot AM62x using Ethernet for which we need to load binary files at SPL and U-Boot stage using TFTP. To store the file we need a free memory in RAM, specifically we are storing these files at 0x8200. But we are facing an issue while loading the file since the memory area having an address 0x8200 is reserved due to "lmb_init_and_reserve()" function call. This function is called in "tftp_init_load_addr()" function which is getting called exactly before we are trying to get the free memory area by calling "lmb_get_free_size()". I have no idea about your platform but I was wondering if there is any particular importance of the load address of 0x8200? It looks as though the current location of the SP when arch_lmb_reserve() gets called means that the load address is getting reserved for the U-Boot image. Do you not have the option of loading the image at a lower address instead? Or using a higher address for SPL stack? You might be able to solve this just by re-examining which addresses (and RAM size limitations) need to be considered here. Tom, We changed SPL_STACK_R_ADDR to high
Re: [PATCH 01/10] board: ti: am62x: Init DRAM size in R5/A53 SPL
On 19/04/24 17:04, Sughosh Ganu wrote: On Fri, 19 Apr 2024 at 16:04, Chintan Vankar wrote: On 18/04/24 17:30, Sughosh Ganu wrote: On Thu, 18 Apr 2024 at 16:08, Chintan Vankar wrote: On 17/04/24 21:34, Tom Rini wrote: On Wed, Apr 17, 2024 at 05:48:31PM +0530, Sughosh Ganu wrote: hi Chintan, On Wed, 17 Apr 2024 at 13:21, Chintan Vankar wrote: On 16/04/24 22:30, Tom Rini wrote: On Tue, Apr 16, 2024 at 05:52:58PM +0530, Chintan Vankar wrote: On 12/04/24 03:37, Tom Rini wrote: On Wed, Apr 03, 2024 at 06:18:01PM +0530, Chintan Vankar wrote: On 22/01/24 10:11, Siddharth Vadapalli wrote: On 20/01/24 22:11, Tom Rini wrote: On Mon, Jan 15, 2024 at 01:42:51PM +0530, Siddharth Vadapalli wrote: Hello Tom, On 12/01/24 18:56, Tom Rini wrote: ... The list of conditionals in common/spl/spl.c::board_init_r() should be updated and probably use SPL_NET as the option to check for. Thank you for reviewing the patch and pointing this out. I wasn't aware of it. I assume that you are referring to the following change: if (IS_ENABLED(CONFIG_SPL_OS_BOOT) || CONFIG_IS_ENABLED(HANDOFF) || - IS_ENABLED(CONFIG_SPL_ATF)) + IS_ENABLED(CONFIG_SPL_ATF) || IS_ENABLED(CONFIG_SPL_NET)) dram_init_banksize(); I shall replace the current patch with the above change in the v2 series. Since this is in the common section, is there a generic reason I could provide in the commit message rather than the existing commit message which seems to be board specific? Also, I hope that the above change will not cause regressions for other non-TI devices. Please let me know. Yes, that's the area, and just note that networking also requires the DDR to be initialized. Thank you for confirming and providing your suggestion for the contents of the commit message. Following Tom's Suggestion of adding CONFIG_SPL_NET in common/spl/spl.c "dram_init_banksize()", the issue of fetching a file at SPL stage seemed to be fixed. However the commit "ba20b2443c29", which sets gd->ram_top for the very first time in "spl_enable_cache()" results in "arch_lmb_reserve()" function reserving memory region from Stack pointer at "0x81FFB820" to gd->ram_top pointing to "0x1". Previously when gd->ram_top was zero "arch_lmb_reserve()" was noop. Now using TFTP to fetch U-Boot image at SPL stage results in "tftp_init_load_addr()" function call that invokes "arch_lmb_reserve()" function, which reserves entire memory starting from Stack Pointer to gd->ram_top leaving no space to load U-Boot image via TFTP since TFTP loads files at pre configured memory address at "0x8200". As a workaround for this issue, one solution we can propose is to disable the checks "lmb_get_free_size()" at SPL and U-Boot stage. For that we can define a new config option for LMB reserve checks as "SPL_LMB". This config will be enable by default for the backword compatibility and disable for our use case at SPL and U-Boot stage. The problem here is that we need LMB for booting an OS, which is something we'll want in SPL in non-cortex-R cases too, which means this platform, so that's a no-go. I think you need to dig harder and see if you can correct the logic somewhere so that we don't over reserve? Since this issue is due to function call "lmb_init_and_reserve()" function invoked from "tftp_init_load_addr()" function. This function is defined by Simon in commit "a156c47e39ad", which fixes "CVE-2018-18439" to prevent overwriting reserved memory. Simon, can you explain why do we need to call "lmb_init_and_reserve()" function here ? This is indeed a tricky area which is why Sughosh is looking in to trying to re-work the LMB mechanic and we've had a few long threads about it as well. I've honestly forgotten the use case you have here, can you please remind us? We are trying to boot AM62x using Ethernet for which we need to load binary files at SPL and U-Boot stage using TFTP. To store the file we need a free memory in RAM, specifically we are storing these files at 0x8200. But we are facing an issue while loading the file since the memory area having an address 0x8200 is reserved due to "lmb_init_and_reserve()" function call. This function is called in "tftp_init_load_addr()" function which is getting called exactly before we are trying to get the free memory area by calling "lmb_get_free_size()". I have no idea about your platform but I was wondering if there is any particular importance of the load address of 0x8200? It looks as though the current location of the SP when arch_lmb_reserve() gets called means that the load address is getting reserved for the U-Boot image. Do you not have the option of loading the image at a lower address instead? Sughosh, I think my e
Re: [PATCH 01/10] board: ti: am62x: Init DRAM size in R5/A53 SPL
On 18/04/24 17:30, Sughosh Ganu wrote: On Thu, 18 Apr 2024 at 16:08, Chintan Vankar wrote: On 17/04/24 21:34, Tom Rini wrote: On Wed, Apr 17, 2024 at 05:48:31PM +0530, Sughosh Ganu wrote: hi Chintan, On Wed, 17 Apr 2024 at 13:21, Chintan Vankar wrote: On 16/04/24 22:30, Tom Rini wrote: On Tue, Apr 16, 2024 at 05:52:58PM +0530, Chintan Vankar wrote: On 12/04/24 03:37, Tom Rini wrote: On Wed, Apr 03, 2024 at 06:18:01PM +0530, Chintan Vankar wrote: On 22/01/24 10:11, Siddharth Vadapalli wrote: On 20/01/24 22:11, Tom Rini wrote: On Mon, Jan 15, 2024 at 01:42:51PM +0530, Siddharth Vadapalli wrote: Hello Tom, On 12/01/24 18:56, Tom Rini wrote: ... The list of conditionals in common/spl/spl.c::board_init_r() should be updated and probably use SPL_NET as the option to check for. Thank you for reviewing the patch and pointing this out. I wasn't aware of it. I assume that you are referring to the following change: if (IS_ENABLED(CONFIG_SPL_OS_BOOT) || CONFIG_IS_ENABLED(HANDOFF) || - IS_ENABLED(CONFIG_SPL_ATF)) + IS_ENABLED(CONFIG_SPL_ATF) || IS_ENABLED(CONFIG_SPL_NET)) dram_init_banksize(); I shall replace the current patch with the above change in the v2 series. Since this is in the common section, is there a generic reason I could provide in the commit message rather than the existing commit message which seems to be board specific? Also, I hope that the above change will not cause regressions for other non-TI devices. Please let me know. Yes, that's the area, and just note that networking also requires the DDR to be initialized. Thank you for confirming and providing your suggestion for the contents of the commit message. Following Tom's Suggestion of adding CONFIG_SPL_NET in common/spl/spl.c "dram_init_banksize()", the issue of fetching a file at SPL stage seemed to be fixed. However the commit "ba20b2443c29", which sets gd->ram_top for the very first time in "spl_enable_cache()" results in "arch_lmb_reserve()" function reserving memory region from Stack pointer at "0x81FFB820" to gd->ram_top pointing to "0x1". Previously when gd->ram_top was zero "arch_lmb_reserve()" was noop. Now using TFTP to fetch U-Boot image at SPL stage results in "tftp_init_load_addr()" function call that invokes "arch_lmb_reserve()" function, which reserves entire memory starting from Stack Pointer to gd->ram_top leaving no space to load U-Boot image via TFTP since TFTP loads files at pre configured memory address at "0x8200". As a workaround for this issue, one solution we can propose is to disable the checks "lmb_get_free_size()" at SPL and U-Boot stage. For that we can define a new config option for LMB reserve checks as "SPL_LMB". This config will be enable by default for the backword compatibility and disable for our use case at SPL and U-Boot stage. The problem here is that we need LMB for booting an OS, which is something we'll want in SPL in non-cortex-R cases too, which means this platform, so that's a no-go. I think you need to dig harder and see if you can correct the logic somewhere so that we don't over reserve? Since this issue is due to function call "lmb_init_and_reserve()" function invoked from "tftp_init_load_addr()" function. This function is defined by Simon in commit "a156c47e39ad", which fixes "CVE-2018-18439" to prevent overwriting reserved memory. Simon, can you explain why do we need to call "lmb_init_and_reserve()" function here ? This is indeed a tricky area which is why Sughosh is looking in to trying to re-work the LMB mechanic and we've had a few long threads about it as well. I've honestly forgotten the use case you have here, can you please remind us? We are trying to boot AM62x using Ethernet for which we need to load binary files at SPL and U-Boot stage using TFTP. To store the file we need a free memory in RAM, specifically we are storing these files at 0x8200. But we are facing an issue while loading the file since the memory area having an address 0x8200 is reserved due to "lmb_init_and_reserve()" function call. This function is called in "tftp_init_load_addr()" function which is getting called exactly before we are trying to get the free memory area by calling "lmb_get_free_size()". I have no idea about your platform but I was wondering if there is any particular importance of the load address of 0x8200? It looks as though the current location of the SP when arch_lmb_reserve() gets called means that the load address is getting reserved for the U-Boot image. Do you not have the option of loading the image at a lower address instead? Sughosh, I think my explanation was not clear at: "We are trying to boot AM62x using Ethernet for which we need
Re: [PATCH 01/10] board: ti: am62x: Init DRAM size in R5/A53 SPL
On 17/04/24 21:34, Tom Rini wrote: On Wed, Apr 17, 2024 at 05:48:31PM +0530, Sughosh Ganu wrote: hi Chintan, On Wed, 17 Apr 2024 at 13:21, Chintan Vankar wrote: On 16/04/24 22:30, Tom Rini wrote: On Tue, Apr 16, 2024 at 05:52:58PM +0530, Chintan Vankar wrote: On 12/04/24 03:37, Tom Rini wrote: On Wed, Apr 03, 2024 at 06:18:01PM +0530, Chintan Vankar wrote: On 22/01/24 10:11, Siddharth Vadapalli wrote: On 20/01/24 22:11, Tom Rini wrote: On Mon, Jan 15, 2024 at 01:42:51PM +0530, Siddharth Vadapalli wrote: Hello Tom, On 12/01/24 18:56, Tom Rini wrote: ... The list of conditionals in common/spl/spl.c::board_init_r() should be updated and probably use SPL_NET as the option to check for. Thank you for reviewing the patch and pointing this out. I wasn't aware of it. I assume that you are referring to the following change: if (IS_ENABLED(CONFIG_SPL_OS_BOOT) || CONFIG_IS_ENABLED(HANDOFF) || - IS_ENABLED(CONFIG_SPL_ATF)) + IS_ENABLED(CONFIG_SPL_ATF) || IS_ENABLED(CONFIG_SPL_NET)) dram_init_banksize(); I shall replace the current patch with the above change in the v2 series. Since this is in the common section, is there a generic reason I could provide in the commit message rather than the existing commit message which seems to be board specific? Also, I hope that the above change will not cause regressions for other non-TI devices. Please let me know. Yes, that's the area, and just note that networking also requires the DDR to be initialized. Thank you for confirming and providing your suggestion for the contents of the commit message. Following Tom's Suggestion of adding CONFIG_SPL_NET in common/spl/spl.c "dram_init_banksize()", the issue of fetching a file at SPL stage seemed to be fixed. However the commit "ba20b2443c29", which sets gd->ram_top for the very first time in "spl_enable_cache()" results in "arch_lmb_reserve()" function reserving memory region from Stack pointer at "0x81FFB820" to gd->ram_top pointing to "0x1". Previously when gd->ram_top was zero "arch_lmb_reserve()" was noop. Now using TFTP to fetch U-Boot image at SPL stage results in "tftp_init_load_addr()" function call that invokes "arch_lmb_reserve()" function, which reserves entire memory starting from Stack Pointer to gd->ram_top leaving no space to load U-Boot image via TFTP since TFTP loads files at pre configured memory address at "0x8200". As a workaround for this issue, one solution we can propose is to disable the checks "lmb_get_free_size()" at SPL and U-Boot stage. For that we can define a new config option for LMB reserve checks as "SPL_LMB". This config will be enable by default for the backword compatibility and disable for our use case at SPL and U-Boot stage. The problem here is that we need LMB for booting an OS, which is something we'll want in SPL in non-cortex-R cases too, which means this platform, so that's a no-go. I think you need to dig harder and see if you can correct the logic somewhere so that we don't over reserve? Since this issue is due to function call "lmb_init_and_reserve()" function invoked from "tftp_init_load_addr()" function. This function is defined by Simon in commit "a156c47e39ad", which fixes "CVE-2018-18439" to prevent overwriting reserved memory. Simon, can you explain why do we need to call "lmb_init_and_reserve()" function here ? This is indeed a tricky area which is why Sughosh is looking in to trying to re-work the LMB mechanic and we've had a few long threads about it as well. I've honestly forgotten the use case you have here, can you please remind us? We are trying to boot AM62x using Ethernet for which we need to load binary files at SPL and U-Boot stage using TFTP. To store the file we need a free memory in RAM, specifically we are storing these files at 0x8200. But we are facing an issue while loading the file since the memory area having an address 0x8200 is reserved due to "lmb_init_and_reserve()" function call. This function is called in "tftp_init_load_addr()" function which is getting called exactly before we are trying to get the free memory area by calling "lmb_get_free_size()". I have no idea about your platform but I was wondering if there is any particular importance of the load address of 0x8200? It looks as though the current location of the SP when arch_lmb_reserve() gets called means that the load address is getting reserved for the U-Boot image. Do you not have the option of loading the image at a lower address instead? Sughosh, I think my explanation was not clear at: "We are trying to boot AM62x using Ethernet for which we need to load binary files at SPL and U-Boot stage using TFTP." - In Ethernet Booting we are fetching U-
Re: [PATCH 01/10] board: ti: am62x: Init DRAM size in R5/A53 SPL
On 16/04/24 22:30, Tom Rini wrote: On Tue, Apr 16, 2024 at 05:52:58PM +0530, Chintan Vankar wrote: On 12/04/24 03:37, Tom Rini wrote: On Wed, Apr 03, 2024 at 06:18:01PM +0530, Chintan Vankar wrote: On 22/01/24 10:11, Siddharth Vadapalli wrote: On 20/01/24 22:11, Tom Rini wrote: On Mon, Jan 15, 2024 at 01:42:51PM +0530, Siddharth Vadapalli wrote: Hello Tom, On 12/01/24 18:56, Tom Rini wrote: ... The list of conditionals in common/spl/spl.c::board_init_r() should be updated and probably use SPL_NET as the option to check for. Thank you for reviewing the patch and pointing this out. I wasn't aware of it. I assume that you are referring to the following change: if (IS_ENABLED(CONFIG_SPL_OS_BOOT) || CONFIG_IS_ENABLED(HANDOFF) || - IS_ENABLED(CONFIG_SPL_ATF)) + IS_ENABLED(CONFIG_SPL_ATF) || IS_ENABLED(CONFIG_SPL_NET)) dram_init_banksize(); I shall replace the current patch with the above change in the v2 series. Since this is in the common section, is there a generic reason I could provide in the commit message rather than the existing commit message which seems to be board specific? Also, I hope that the above change will not cause regressions for other non-TI devices. Please let me know. Yes, that's the area, and just note that networking also requires the DDR to be initialized. Thank you for confirming and providing your suggestion for the contents of the commit message. Following Tom's Suggestion of adding CONFIG_SPL_NET in common/spl/spl.c "dram_init_banksize()", the issue of fetching a file at SPL stage seemed to be fixed. However the commit "ba20b2443c29", which sets gd->ram_top for the very first time in "spl_enable_cache()" results in "arch_lmb_reserve()" function reserving memory region from Stack pointer at "0x81FFB820" to gd->ram_top pointing to "0x1". Previously when gd->ram_top was zero "arch_lmb_reserve()" was noop. Now using TFTP to fetch U-Boot image at SPL stage results in "tftp_init_load_addr()" function call that invokes "arch_lmb_reserve()" function, which reserves entire memory starting from Stack Pointer to gd->ram_top leaving no space to load U-Boot image via TFTP since TFTP loads files at pre configured memory address at "0x8200". As a workaround for this issue, one solution we can propose is to disable the checks "lmb_get_free_size()" at SPL and U-Boot stage. For that we can define a new config option for LMB reserve checks as "SPL_LMB". This config will be enable by default for the backword compatibility and disable for our use case at SPL and U-Boot stage. The problem here is that we need LMB for booting an OS, which is something we'll want in SPL in non-cortex-R cases too, which means this platform, so that's a no-go. I think you need to dig harder and see if you can correct the logic somewhere so that we don't over reserve? Since this issue is due to function call "lmb_init_and_reserve()" function invoked from "tftp_init_load_addr()" function. This function is defined by Simon in commit "a156c47e39ad", which fixes "CVE-2018-18439" to prevent overwriting reserved memory. Simon, can you explain why do we need to call "lmb_init_and_reserve()" function here ? This is indeed a tricky area which is why Sughosh is looking in to trying to re-work the LMB mechanic and we've had a few long threads about it as well. I've honestly forgotten the use case you have here, can you please remind us? We are trying to boot AM62x using Ethernet for which we need to load binary files at SPL and U-Boot stage using TFTP. To store the file we need a free memory in RAM, specifically we are storing these files at 0x8200. But we are facing an issue while loading the file since the memory area having an address 0x8200 is reserved due to "lmb_init_and_reserve()" function call. This function is called in "tftp_init_load_addr()" function which is getting called exactly before we are trying to get the free memory area by calling "lmb_get_free_size()".
Re: [PATCH 01/10] board: ti: am62x: Init DRAM size in R5/A53 SPL
On 12/04/24 03:37, Tom Rini wrote: On Wed, Apr 03, 2024 at 06:18:01PM +0530, Chintan Vankar wrote: On 22/01/24 10:11, Siddharth Vadapalli wrote: On 20/01/24 22:11, Tom Rini wrote: On Mon, Jan 15, 2024 at 01:42:51PM +0530, Siddharth Vadapalli wrote: Hello Tom, On 12/01/24 18:56, Tom Rini wrote: ... The list of conditionals in common/spl/spl.c::board_init_r() should be updated and probably use SPL_NET as the option to check for. Thank you for reviewing the patch and pointing this out. I wasn't aware of it. I assume that you are referring to the following change: if (IS_ENABLED(CONFIG_SPL_OS_BOOT) || CONFIG_IS_ENABLED(HANDOFF) || - IS_ENABLED(CONFIG_SPL_ATF)) + IS_ENABLED(CONFIG_SPL_ATF) || IS_ENABLED(CONFIG_SPL_NET)) dram_init_banksize(); I shall replace the current patch with the above change in the v2 series. Since this is in the common section, is there a generic reason I could provide in the commit message rather than the existing commit message which seems to be board specific? Also, I hope that the above change will not cause regressions for other non-TI devices. Please let me know. Yes, that's the area, and just note that networking also requires the DDR to be initialized. Thank you for confirming and providing your suggestion for the contents of the commit message. Following Tom's Suggestion of adding CONFIG_SPL_NET in common/spl/spl.c "dram_init_banksize()", the issue of fetching a file at SPL stage seemed to be fixed. However the commit "ba20b2443c29", which sets gd->ram_top for the very first time in "spl_enable_cache()" results in "arch_lmb_reserve()" function reserving memory region from Stack pointer at "0x81FFB820" to gd->ram_top pointing to "0x1". Previously when gd->ram_top was zero "arch_lmb_reserve()" was noop. Now using TFTP to fetch U-Boot image at SPL stage results in "tftp_init_load_addr()" function call that invokes "arch_lmb_reserve()" function, which reserves entire memory starting from Stack Pointer to gd->ram_top leaving no space to load U-Boot image via TFTP since TFTP loads files at pre configured memory address at "0x8200". As a workaround for this issue, one solution we can propose is to disable the checks "lmb_get_free_size()" at SPL and U-Boot stage. For that we can define a new config option for LMB reserve checks as "SPL_LMB". This config will be enable by default for the backword compatibility and disable for our use case at SPL and U-Boot stage. The problem here is that we need LMB for booting an OS, which is something we'll want in SPL in non-cortex-R cases too, which means this platform, so that's a no-go. I think you need to dig harder and see if you can correct the logic somewhere so that we don't over reserve? Since this issue is due to function call "lmb_init_and_reserve()" function invoked from "tftp_init_load_addr()" function. This function is defined by Simon in commit "a156c47e39ad", which fixes "CVE-2018-18439" to prevent overwriting reserved memory. Simon, can you explain why do we need to call "lmb_init_and_reserve()" function here ?
Re: [PATCH 01/10] board: ti: am62x: Init DRAM size in R5/A53 SPL
On 22/01/24 10:11, Siddharth Vadapalli wrote: On 20/01/24 22:11, Tom Rini wrote: On Mon, Jan 15, 2024 at 01:42:51PM +0530, Siddharth Vadapalli wrote: Hello Tom, On 12/01/24 18:56, Tom Rini wrote: ... The list of conditionals in common/spl/spl.c::board_init_r() should be updated and probably use SPL_NET as the option to check for. Thank you for reviewing the patch and pointing this out. I wasn't aware of it. I assume that you are referring to the following change: if (IS_ENABLED(CONFIG_SPL_OS_BOOT) || CONFIG_IS_ENABLED(HANDOFF) || - IS_ENABLED(CONFIG_SPL_ATF)) + IS_ENABLED(CONFIG_SPL_ATF) || IS_ENABLED(CONFIG_SPL_NET)) dram_init_banksize(); I shall replace the current patch with the above change in the v2 series. Since this is in the common section, is there a generic reason I could provide in the commit message rather than the existing commit message which seems to be board specific? Also, I hope that the above change will not cause regressions for other non-TI devices. Please let me know. Yes, that's the area, and just note that networking also requires the DDR to be initialized. Thank you for confirming and providing your suggestion for the contents of the commit message. Following Tom's Suggestion of adding CONFIG_SPL_NET in common/spl/spl.c "dram_init_banksize()", the issue of fetching a file at SPL stage seemed to be fixed. However the commit "ba20b2443c29", which sets gd->ram_top for the very first time in "spl_enable_cache()" results in "arch_lmb_reserve()" function reserving memory region from Stack pointer at "0x81FFB820" to gd->ram_top pointing to "0x1". Previously when gd->ram_top was zero "arch_lmb_reserve()" was noop. Now using TFTP to fetch U-Boot image at SPL stage results in "tftp_init_load_addr()" function call that invokes "arch_lmb_reserve()" function, which reserves entire memory starting from Stack Pointer to gd->ram_top leaving no space to load U-Boot image via TFTP since TFTP loads files at pre configured memory address at "0x8200". As a workaround for this issue, one solution we can propose is to disable the checks "lmb_get_free_size()" at SPL and U-Boot stage. For that we can define a new config option for LMB reserve checks as "SPL_LMB". This config will be enable by default for the backword compatibility and disable for our use case at SPL and U-Boot stage.
Re: [PATCH 07/10] arm: mach-k3: am625_init: Probe AM65 CPSW NUSS
On 24/01/24 02:27, Nishanth Menon wrote: On 15:49-20240122, Chintan Vankar wrote: On 12/01/24 18:00, Nishanth Menon wrote: On 12:17-20240112, Siddharth Vadapalli wrote: From: Kishon Vijay Abraham I In order to support Ethernet boot on AM62x, probe AM65 CPSW NUSS driver in board_init_f(). Why? doesn't the DM framework handle this? Hello, In board_init_f(), the ESM driver which is modeled as UClASS_MISC driver seems to be probed explicitly. So it appears to me that the DM framework is probably not capable of handling probe of UCLASS_MISC. The am65-cpsw driver is also a UCLASS_MISC driver because of which it has to be probed similar to the ESM driver. Can someone suggest me if there is a way to trigger probe of all UCLASS_MISC drivers using the DM framework or am65-cpsw should be probed in this way only ? Can you suggest how can we do this ? Did'nt you guys just discuss this in https://lore.kernel.org/all/48c63fc4-9f06-4066-b206-a0a548936...@ti.com/ ?
Re: [PATCH 07/10] arm: mach-k3: am625_init: Probe AM65 CPSW NUSS
On 12/01/24 18:00, Nishanth Menon wrote:
On 12:17-20240112, Siddharth Vadapalli wrote:
From: Kishon Vijay Abraham I
In order to support Ethernet boot on AM62x, probe AM65 CPSW NUSS driver
in board_init_f().
Why? doesn't the DM framework handle this?
Can you suggest how can we do this ?
Signed-off-by: Kishon Vijay Abraham I
Signed-off-by: Siddharth Vadapalli
---
arch/arm/mach-k3/am625_init.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/arch/arm/mach-k3/am625_init.c b/arch/arm/mach-k3/am625_init.c
index 6c96e88114..b89dd206e5 100644
--- a/arch/arm/mach-k3/am625_init.c
+++ b/arch/arm/mach-k3/am625_init.c
@@ -209,6 +209,16 @@ void board_init_f(ulong dummy)
if (ret)
panic("DRAM init failed: %d\n", ret);
}
+
+ if (IS_ENABLED(CONFIG_SPL_ETH) && IS_ENABLED(CONFIG_TI_AM65_CPSW_NUSS)
&&
+ spl_boot_device() == BOOT_DEVICE_ETHERNET) {
+ struct udevice *cpswdev;
+
+ if (uclass_get_device_by_driver(UCLASS_MISC,
DM_DRIVER_GET(am65_cpsw_nuss),
+ ))
+ printf("Failed to probe am65_cpsw_nuss driver\n");
+ }
+
