* [RESEND PATCH 0/4] Improve RaspBerry Pi 5 support part1: Fixes
@ 2025-11-05 16:35 Torsten Duwe
2025-11-05 16:44 ` [RESEND PATCH 1/4] macb: wrap asm/arch/clk.h with ifndef CLK Torsten Duwe
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Torsten Duwe @ 2025-11-05 16:35 UTC (permalink / raw)
To: Matthias Brugger, Simon Glass
Cc: Andrew Goodbody, Christian Marangi, Elaine Zhang,
Gabriel Fernandez, Heiko Stuebner, Huan Zhou, Ilias Apalodimas,
Jan Čermák, Jerome Forissier, Joe Hershberger,
Jonas Karlman, Joseph Chen, Kever Yang, Leo Yu-Chi Liang,
Marek Vasut, Patrice Chotard, Patrick Delaunay, Paul Barker,
Peter Robinson, Ramon Fried, Tom Rini, Volodymyr Babchuk,
Oleksii Moisieiev, Quentin Schulz, u-boot
Hi all,
here is the requested resend of the first mini-series with the fixes,
now hopefully with all interested people involved. I regenerated it
using get_maintainer.pl on part1 and part2, for the heads-up.
I also added Oleksii's refreshed review tag.
Part2 takes actually longer to rework than expected; documentation on the
PCIe bridges is not so opulent, mildly speaking.
This set should apply cleanly on 2025.10 and on master [1c250e444ad].
Torsten
^ permalink raw reply [flat|nested] 11+ messages in thread
* [RESEND PATCH 1/4] macb: wrap asm/arch/clk.h with ifndef CLK
2025-11-05 16:35 [RESEND PATCH 0/4] Improve RaspBerry Pi 5 support part1: Fixes Torsten Duwe
@ 2025-11-05 16:44 ` Torsten Duwe
2025-11-06 15:51 ` Matthias Brugger
2025-11-05 16:44 ` [RESEND PATCH 2/4] pci-uclass: respect ofnode_read_pci_vendev retval Torsten Duwe
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Torsten Duwe @ 2025-11-05 16:44 UTC (permalink / raw)
To: Matthias Brugger, Simon Glass
Cc: Andrew Goodbody, Christian Marangi, Elaine Zhang,
Gabriel Fernandez, Heiko Stuebner, Huan Zhou, Ilias Apalodimas,
Jan Čermák, Jerome Forissier, Joe Hershberger,
Jonas Karlman, Joseph Chen, Kever Yang, Leo Yu-Chi Liang,
Marek Vasut, Patrice Chotard, Patrick Delaunay, Paul Barker,
Peter Robinson, Ramon Fried, Tom Rini, Volodymyr Babchuk,
Oleksii Moisieiev, Quentin Schulz, u-boot
macb.c includes asm/arch/clk.h solely to declare get_macb_pclk_rate().
That function is platform specific and only required if CONFIG_CLK
is not set. In case the machine does not provide it, compilation fails
even when the MACB pclk is detected via the CLK framework.
Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Signed-off-by: Torsten Duwe <duwe@suse.de>
Reviewed-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
---
drivers/net/macb.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index cbf5f605518..a54a268036d 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -38,7 +38,9 @@
#include <linux/mii.h>
#include <asm/io.h>
#include <linux/dma-mapping.h>
+#ifndef CONFIG_CLK
#include <asm/arch/clk.h>
+#endif
#include <linux/errno.h>
#include "macb.h"
--
2.51.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [RESEND PATCH 2/4] pci-uclass: respect ofnode_read_pci_vendev retval
2025-11-05 16:35 [RESEND PATCH 0/4] Improve RaspBerry Pi 5 support part1: Fixes Torsten Duwe
2025-11-05 16:44 ` [RESEND PATCH 1/4] macb: wrap asm/arch/clk.h with ifndef CLK Torsten Duwe
@ 2025-11-05 16:44 ` Torsten Duwe
2025-11-06 15:51 ` Matthias Brugger
2025-11-19 16:47 ` Torsten Duwe
2025-11-05 16:44 ` [RESEND PATCH 3/4] Fix #cells lookup in of_get_dma_range Torsten Duwe
` (2 subsequent siblings)
4 siblings, 2 replies; 11+ messages in thread
From: Torsten Duwe @ 2025-11-05 16:44 UTC (permalink / raw)
To: Matthias Brugger, Simon Glass
Cc: Andrew Goodbody, Christian Marangi, Elaine Zhang,
Gabriel Fernandez, Heiko Stuebner, Huan Zhou, Ilias Apalodimas,
Jan Čermák, Jerome Forissier, Joe Hershberger,
Jonas Karlman, Joseph Chen, Kever Yang, Leo Yu-Chi Liang,
Marek Vasut, Patrice Chotard, Patrick Delaunay, Paul Barker,
Peter Robinson, Ramon Fried, Tom Rini, Volodymyr Babchuk,
Oleksii Moisieiev, Quentin Schulz, u-boot
The vendor:device read in pci_uclass_child_post_bind() might fail,
which is currently ignored. In that case e.g. the RP1 on the RasPi5
is erroneously being treated as a host bridge.
Mark the device's PCI info as invalid in case of failure.
Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
Signed-off-by: Torsten Duwe <duwe@suse.de>
Reviewed-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
---
drivers/pci/pci-uclass.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
index c370f8c6400..4092e8ff5a9 100644
--- a/drivers/pci/pci-uclass.c
+++ b/drivers/pci/pci-uclass.c
@@ -1200,6 +1200,7 @@ static int pci_uclass_post_probe(struct udevice *bus)
static int pci_uclass_child_post_bind(struct udevice *dev)
{
struct pci_child_plat *pplat;
+ int err;
if (!dev_has_ofnode(dev))
return 0;
@@ -1207,7 +1208,13 @@ static int pci_uclass_child_post_bind(struct udevice *dev)
pplat = dev_get_parent_plat(dev);
/* Extract vendor id and device id if available */
- ofnode_read_pci_vendev(dev_ofnode(dev), &pplat->vendor, &pplat->device);
+ err = ofnode_read_pci_vendev(dev_ofnode(dev), &pplat->vendor,
+ &pplat->device);
+ if (err) {
+ /* Mark PCI device structure as invalid */
+ pplat->devfn = -1;
+ return 0;
+ }
/* Extract the devfn from fdt_pci_addr */
pplat->devfn = pci_get_devfn(dev);
--
2.51.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [RESEND PATCH 3/4] Fix #cells lookup in of_get_dma_range
2025-11-05 16:35 [RESEND PATCH 0/4] Improve RaspBerry Pi 5 support part1: Fixes Torsten Duwe
2025-11-05 16:44 ` [RESEND PATCH 1/4] macb: wrap asm/arch/clk.h with ifndef CLK Torsten Duwe
2025-11-05 16:44 ` [RESEND PATCH 2/4] pci-uclass: respect ofnode_read_pci_vendev retval Torsten Duwe
@ 2025-11-05 16:44 ` Torsten Duwe
2025-11-28 14:44 ` Peter Robinson
2025-11-05 16:44 ` [RESEND PATCH 4/4] pcie_brcmstb: use bus_base for PCI config devices Torsten Duwe
2025-11-28 14:38 ` [RESEND PATCH 0/4] Improve RaspBerry Pi 5 support part1: Fixes Peter Robinson
4 siblings, 1 reply; 11+ messages in thread
From: Torsten Duwe @ 2025-11-05 16:44 UTC (permalink / raw)
To: Matthias Brugger, Simon Glass
Cc: Andrew Goodbody, Christian Marangi, Elaine Zhang,
Gabriel Fernandez, Heiko Stuebner, Huan Zhou, Ilias Apalodimas,
Jan Čermák, Jerome Forissier, Joe Hershberger,
Jonas Karlman, Joseph Chen, Kever Yang, Leo Yu-Chi Liang,
Marek Vasut, Patrice Chotard, Patrick Delaunay, Paul Barker,
Peter Robinson, Ramon Fried, Tom Rini, Volodymyr Babchuk,
Oleksii Moisieiev, Quentin Schulz, u-boot
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 5249 bytes --]
of_get_dma_range uses (of_bus *)->count_cells to look up the
#cells for addresses and sizes. It tries to do so for the current
device node and for the parent tree. count_cells defaults to
of_bus_default_count_cells, which _starts_ looking at the parent
node already, and then upwards. The correct way is to use
of_simple_*_cells for the current node, and count_cells(dev) for
the parent tree.
Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Signed-off-by: Torsten Duwe <duwe@suse.de>
Reviewed-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
---
Here is the original, very detailed analysis and explanation
[edited] :
According to the Section 2.3.9 of the Device Tree specification[0]
the format of the value of the dma-ranges property is an arbitrary
number of triplets of (child-bus-address,
parent-bus-address, length).
Where:
* The child-bus-address is a physical address within the child bus’
address space. The number of cells to represent the address depends on
the bus and can be determined from the #address-cells of this node
(the node in which the dma-ranges property appears).
* The parent-bus-address is a physical address within the parent bus’
address space. The number of cells to represent the parent address
is bus dependent and can be determined from the #address-cells
property of the node that defines the parent’s address space.
* The length specifies the size of the range in the child’s address
space. The number of cells to represent the size can be determined
from the #size-cells of this node (the node in which the dma-ranges
property appears)
This description matches comments in the of_addr source code. But the
actual implementation does not match the specification.
'of_match_bus' returns address-cells and size-cells of the parent
device. That's why the following calls were used to receive addr_cells
and size_cells of the current device:
- of_simple_addr_cells
- of_simple_size_cells
This fix was tested on RPI5 board with both device types: directly
connected (e.g Serial) and connected over PCIe (e.g ethernet).
For example when dma_ranges are parsing the following device tree
structure:
/ {
pcie2: {
#address-cells = <3>;
#size-cells = <2>;
// 64GB system RAM space at PCIe 10_00000000
dma-ranges =
<0x02000000 0x00 0x00000000
0x1f 0x00000000
0x00 0x00400000>,
<0x43000000 0x10 0x00000000
0x00 0x00000000
0x10 0x00000000>;
rp1: {
#address-cells = <2>;
#size-cells = <2>;
dma-ranges =
// inbound RP1 1x_xxxxxxxx -> PCIe 1x_xxxxxxxx
<0x10 0x00000000
0x43000000 0x10 0x00000000
0x10 0x00000000>,
// inbound RP1 c0_40xxxxxx -> PCIe 00_00xxxxxx
<0xc0 0x40000000
0x02000000 0x0 0x00000000
0x0 0x00400000>,
// inbound RP1 0x_xxxxxxxx -> PCIe 1x_xxxxxxxx
<0x00 0x00000000
0x02000000 0x10 0x00000000
0x10 0x00000000>;
};
};
};
So when we parse ranges on rp1, #address-cells of the rp1 node should
be taken into account.
In the provided example we have the following format:
pcie2:
#addres-cells = <0x3>;
#size-cells = <0x2>;
dma-ranges = < addr_hi addr_mid addr_lo
trans_addr_hi trans_addr_lo
size_hi size_lo>;
rp1:
#addres-cells = <0x2>;
#size-cells = <0x2>;
dma-ranges = < addr_hi addr_lo
trans_addr_hi trans_addr_mid trans_addr_lo
size_hi size_lo>;
of_get_dma_range searches for the first dev with dma_ranges
and then executes of_match_bus which follows by count_cells call
for both device and parent.
count_cells returns #address-cells for the parent device to
the caller.
So in the provided example address cells should be 2 for rp1,
but we get 3 when using count_cells.
This breaks address translation and is fixed in the provided patch
by using the of_simple_addr_cells call, which will return correct
address-cells for the node. This matches the requirements of the
device-tree specification.
[0] https://readthedocs.org/projects/devicetree-specification/downloads/pdf/latest/
Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
---
drivers/core/of_addr.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/core/of_addr.c b/drivers/core/of_addr.c
index 250dd175b55..823a4d70a6b 100644
--- a/drivers/core/of_addr.c
+++ b/drivers/core/of_addr.c
@@ -374,16 +374,16 @@ int of_get_dma_range(const struct device_node *dev, phys_addr_t *cpu,
}
/* Get the address sizes both for the bus and its parent */
- bus_node = of_match_bus((struct device_node*)dev);
- bus_node->count_cells(dev, &na, &ns);
+ na = of_simple_addr_cells(dev);
+ ns = of_simple_size_cells(dev);
if (!OF_CHECK_COUNTS(na, ns)) {
printf("Bad cell count for %s\n", of_node_full_name(dev));
ret = -EINVAL;
goto out_parent;
}
- bus_node = of_match_bus(parent);
- bus_node->count_cells(parent, &pna, &pns);
+ bus_node = of_match_bus((struct device_node *)dev);
+ bus_node->count_cells(dev, &pna, &pns);
if (!OF_CHECK_COUNTS(pna, pns)) {
printf("Bad cell count for %s\n", of_node_full_name(parent));
ret = -EINVAL;
--
2.51.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [RESEND PATCH 4/4] pcie_brcmstb: use bus_base for PCI config devices
2025-11-05 16:35 [RESEND PATCH 0/4] Improve RaspBerry Pi 5 support part1: Fixes Torsten Duwe
` (2 preceding siblings ...)
2025-11-05 16:44 ` [RESEND PATCH 3/4] Fix #cells lookup in of_get_dma_range Torsten Duwe
@ 2025-11-05 16:44 ` Torsten Duwe
2025-11-28 14:38 ` [RESEND PATCH 0/4] Improve RaspBerry Pi 5 support part1: Fixes Peter Robinson
4 siblings, 0 replies; 11+ messages in thread
From: Torsten Duwe @ 2025-11-05 16:44 UTC (permalink / raw)
To: Matthias Brugger, Simon Glass
Cc: Andrew Goodbody, Christian Marangi, Elaine Zhang,
Gabriel Fernandez, Heiko Stuebner, Huan Zhou, Ilias Apalodimas,
Jan Čermák, Jerome Forissier, Joe Hershberger,
Jonas Karlman, Joseph Chen, Kever Yang, Leo Yu-Chi Liang,
Marek Vasut, Patrice Chotard, Patrick Delaunay, Paul Barker,
Peter Robinson, Ramon Fried, Tom Rini, Volodymyr Babchuk,
Oleksii Moisieiev, Quentin Schulz, u-boot
BRCM PCIe controller has different means of accessing to its own
config space and to device's config space. To support this quirk,
its driver uses simple heuristic:
when PCI subsystem probes Bus 0, the driver assumes
that the subsystem tries to accesses PCIe controller config space,
when PCI subsystem probes other busses, the driver uses mechanism
for device config space access. This heuristic works well when
there is only one PCIe controller in the system.
But U-Boot does not support concept of "PCI Segment", so when there
are more than one PCI controller:
- first one gets Bus address 0,
- second one - Bus address 2 and so on.
This breaks the brcm driver, because it tries to access second PCIe
controller config space as if it is device. To fix this, we introduce
additional property bus_base that holds a PCIe controller own Bus
address.
We use this property to calculate "real" Bus address for
underlying devices.
Please note that this fix may not support a case when we have
PCI-to-PCI bridges behind the second PCIe controller.
Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Signed-off-by: Torsten Duwe <duwe@suse.de>
Reviewed-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
---
drivers/pci/pcie_brcmstb.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/pci/pcie_brcmstb.c b/drivers/pci/pcie_brcmstb.c
index f089c48f028..ca5951974ce 100644
--- a/drivers/pci/pcie_brcmstb.c
+++ b/drivers/pci/pcie_brcmstb.c
@@ -61,6 +61,7 @@ struct brcm_pcie {
int gen;
bool ssc;
+ uint16_t bus_base;
};
/**
@@ -134,6 +135,7 @@ static int brcm_pcie_config_address(const struct udevice *dev, pci_dev_t bdf,
* Busses 0 (host PCIe bridge) and 1 (its immediate child)
* are limited to a single device each
*/
+ pci_bus -= pcie->bus_base;
if (pci_bus < 2 && pci_dev > 0)
return -EINVAL;
@@ -360,6 +362,8 @@ static int brcm_pcie_probe(struct udevice *dev)
u16 nlw, cls, lnksta;
u32 tmp;
+ pcie->bus_base = hose->first_busno;
+
/*
* Reset the bridge, assert the fundamental reset. Note for some SoCs,
* e.g. BCM7278, the fundamental reset should not be asserted here.
--
2.51.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [RESEND PATCH 1/4] macb: wrap asm/arch/clk.h with ifndef CLK
2025-11-05 16:44 ` [RESEND PATCH 1/4] macb: wrap asm/arch/clk.h with ifndef CLK Torsten Duwe
@ 2025-11-06 15:51 ` Matthias Brugger
0 siblings, 0 replies; 11+ messages in thread
From: Matthias Brugger @ 2025-11-06 15:51 UTC (permalink / raw)
To: Torsten Duwe, Simon Glass
Cc: Andrew Goodbody, Christian Marangi, Elaine Zhang,
Gabriel Fernandez, Heiko Stuebner, Huan Zhou, Ilias Apalodimas,
Jan Čermák, Jerome Forissier, Joe Hershberger,
Jonas Karlman, Joseph Chen, Kever Yang, Leo Yu-Chi Liang,
Marek Vasut, Patrice Chotard, Patrick Delaunay, Paul Barker,
Peter Robinson, Ramon Fried, Tom Rini, Volodymyr Babchuk,
Oleksii Moisieiev, Quentin Schulz, u-boot
On 05/11/2025 17:44, Torsten Duwe wrote:
> macb.c includes asm/arch/clk.h solely to declare get_macb_pclk_rate().
> That function is platform specific and only required if CONFIG_CLK
> is not set. In case the machine does not provide it, compilation fails
> even when the MACB pclk is detected via the CLK framework.
>
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> Signed-off-by: Torsten Duwe <duwe@suse.de>
> Reviewed-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
It seems you forgot my reviewed-by tag, anyway here it comes again :)
Reviewed-by: Matthias Brugger <mbrugger@suse.com>
> ---
> drivers/net/macb.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> index cbf5f605518..a54a268036d 100644
> --- a/drivers/net/macb.c
> +++ b/drivers/net/macb.c
> @@ -38,7 +38,9 @@
> #include <linux/mii.h>
> #include <asm/io.h>
> #include <linux/dma-mapping.h>
> +#ifndef CONFIG_CLK
> #include <asm/arch/clk.h>
> +#endif
> #include <linux/errno.h>
>
> #include "macb.h"
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RESEND PATCH 2/4] pci-uclass: respect ofnode_read_pci_vendev retval
2025-11-05 16:44 ` [RESEND PATCH 2/4] pci-uclass: respect ofnode_read_pci_vendev retval Torsten Duwe
@ 2025-11-06 15:51 ` Matthias Brugger
2025-11-19 16:47 ` Torsten Duwe
1 sibling, 0 replies; 11+ messages in thread
From: Matthias Brugger @ 2025-11-06 15:51 UTC (permalink / raw)
To: Torsten Duwe, Simon Glass
Cc: Andrew Goodbody, Christian Marangi, Elaine Zhang,
Gabriel Fernandez, Heiko Stuebner, Huan Zhou, Ilias Apalodimas,
Jan Čermák, Jerome Forissier, Joe Hershberger,
Jonas Karlman, Joseph Chen, Kever Yang, Leo Yu-Chi Liang,
Marek Vasut, Patrice Chotard, Patrick Delaunay, Paul Barker,
Peter Robinson, Ramon Fried, Tom Rini, Volodymyr Babchuk,
Oleksii Moisieiev, Quentin Schulz, u-boot
On 05/11/2025 17:44, Torsten Duwe wrote:
> The vendor:device read in pci_uclass_child_post_bind() might fail,
> which is currently ignored. In that case e.g. the RP1 on the RasPi5
> is erroneously being treated as a host bridge.
>
> Mark the device's PCI info as invalid in case of failure.
>
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> Signed-off-by: Torsten Duwe <duwe@suse.de>
> Reviewed-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
Reviewed-by: Matthias Brugger <mbrugger@suse.com>
> ---
> drivers/pci/pci-uclass.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> index c370f8c6400..4092e8ff5a9 100644
> --- a/drivers/pci/pci-uclass.c
> +++ b/drivers/pci/pci-uclass.c
> @@ -1200,6 +1200,7 @@ static int pci_uclass_post_probe(struct udevice *bus)
> static int pci_uclass_child_post_bind(struct udevice *dev)
> {
> struct pci_child_plat *pplat;
> + int err;
>
> if (!dev_has_ofnode(dev))
> return 0;
> @@ -1207,7 +1208,13 @@ static int pci_uclass_child_post_bind(struct udevice *dev)
> pplat = dev_get_parent_plat(dev);
>
> /* Extract vendor id and device id if available */
> - ofnode_read_pci_vendev(dev_ofnode(dev), &pplat->vendor, &pplat->device);
> + err = ofnode_read_pci_vendev(dev_ofnode(dev), &pplat->vendor,
> + &pplat->device);
> + if (err) {
> + /* Mark PCI device structure as invalid */
> + pplat->devfn = -1;
> + return 0;
> + }
>
> /* Extract the devfn from fdt_pci_addr */
> pplat->devfn = pci_get_devfn(dev);
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RESEND PATCH 2/4] pci-uclass: respect ofnode_read_pci_vendev retval
2025-11-05 16:44 ` [RESEND PATCH 2/4] pci-uclass: respect ofnode_read_pci_vendev retval Torsten Duwe
2025-11-06 15:51 ` Matthias Brugger
@ 2025-11-19 16:47 ` Torsten Duwe
1 sibling, 0 replies; 11+ messages in thread
From: Torsten Duwe @ 2025-11-19 16:47 UTC (permalink / raw)
To: Peter Robinson
Cc: Matthias Brugger, Simon Glass, Andrew Goodbody, Christian Marangi,
Elaine Zhang, Gabriel Fernandez, Heiko Stuebner, Huan Zhou,
Ilias Apalodimas, Jan XXermXXk, Jerome Forissier, Joe Hershberger,
Jonas Karlman, Joseph Chen, Kever Yang, Leo Yu-Chi Liang,
Marek Vasut, Patrice Chotard, Patrick Delaunay, Paul Barker,
Ramon Fried, Tom Rini, Volodymyr Babchuk, Oleksii Moisieiev,
Quentin Schulz, u-boot
Hi Peter,
please don't apply this one yet.
On Wed, 5 Nov 2025 17:44:32 +0100 (CET)
Torsten Duwe <duwe@lst.de> wrote:
> The vendor:device read in pci_uclass_child_post_bind() might fail,
> which is currently ignored. In that case e.g. the RP1 on the RasPi5
> is erroneously being treated as a host bridge.
>
> Mark the device's PCI info as invalid in case of failure.
>
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> Signed-off-by: Torsten Duwe <duwe@suse.de>
> Reviewed-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> ---
> drivers/pci/pci-uclass.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> index c370f8c6400..4092e8ff5a9 100644
> --- a/drivers/pci/pci-uclass.c
> +++ b/drivers/pci/pci-uclass.c
> @@ -1200,6 +1200,7 @@ static int pci_uclass_post_probe(struct udevice *bus)
> static int pci_uclass_child_post_bind(struct udevice *dev)
> {
> struct pci_child_plat *pplat;
> + int err;
>
> if (!dev_has_ofnode(dev))
> return 0;
> @@ -1207,7 +1208,13 @@ static int pci_uclass_child_post_bind(struct udevice *dev)
> pplat = dev_get_parent_plat(dev);
>
> /* Extract vendor id and device id if available */
> - ofnode_read_pci_vendev(dev_ofnode(dev), &pplat->vendor, &pplat->device);
> + err = ofnode_read_pci_vendev(dev_ofnode(dev), &pplat->vendor,
> + &pplat->device);
> + if (err) {
> + /* Mark PCI device structure as invalid */
> + pplat->devfn = -1;
While I guess we all agree that an error return should not be ignored,
there might still be a better solution in this case. pplat->devfn = -1
might prevent the worst, but I assume the PCI dev
| 02.1f.07 0xffff 0xffff Does not fit any class 0xff
is a result of that.
> + return 0;
> + }
>
> /* Extract the devfn from fdt_pci_addr */
> pplat->devfn = pci_get_devfn(dev);
thanks,
Torsten
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RESEND PATCH 0/4] Improve RaspBerry Pi 5 support part1: Fixes
2025-11-05 16:35 [RESEND PATCH 0/4] Improve RaspBerry Pi 5 support part1: Fixes Torsten Duwe
` (3 preceding siblings ...)
2025-11-05 16:44 ` [RESEND PATCH 4/4] pcie_brcmstb: use bus_base for PCI config devices Torsten Duwe
@ 2025-11-28 14:38 ` Peter Robinson
2026-07-19 10:43 ` Mike Groves
4 siblings, 1 reply; 11+ messages in thread
From: Peter Robinson @ 2025-11-28 14:38 UTC (permalink / raw)
To: Torsten Duwe
Cc: Matthias Brugger, Simon Glass, Andrew Goodbody, Christian Marangi,
Elaine Zhang, Gabriel Fernandez, Heiko Stuebner, Huan Zhou,
Ilias Apalodimas, Jan Čermák, Jerome Forissier,
Joe Hershberger, Jonas Karlman, Joseph Chen, Kever Yang,
Leo Yu-Chi Liang, Marek Vasut, Patrice Chotard, Patrick Delaunay,
Paul Barker, Ramon Fried, Tom Rini, Volodymyr Babchuk,
Oleksii Moisieiev, Quentin Schulz, u-boot
Hi Torsten,
> here is the requested resend of the first mini-series with the fixes,
> now hopefully with all interested people involved. I regenerated it
> using get_maintainer.pl on part1 and part2, for the heads-up.
> I also added Oleksii's refreshed review tag.
>
> Part2 takes actually longer to rework than expected; documentation on the
> PCIe bridges is not so opulent, mildly speaking.
>
> This set should apply cleanly on 2025.10 and on master [1c250e444ad].
So generally these all need more work.
To begin with the commit messages in general, as well as the proper
sign offs and attributions, are a mess and definitely need a cleanup.
The set of 4 patches as a whole don't actually fix anything because
for example, there's PCIe fixes in there, but given there hasn't been
work done to add support for the PCIe IP that's in the bcm2712 SoC if
they do fix something to make that work without the enablement patches
they're basically a no-op. Also it seems you have an issue with your
mail service because I never got [1] your mail about not applying
something.
What I would like to see, which I believe I asked for when the
original series was sent, it is broken down into a few series:
1) Fixes for existing support without feature enablement
2) A PCIe enablement series, this would include, if necessary, and
PCIe fixes like from this series, enable NVME etc
3) A patch series for the RP1 chip, the support here is now upstream
so we know what this looks like, this would include any fixes for say
macb so it can work, any config updates to enabled ethernet/USB etc.
If you've got any clarifications feel free to either post here or
happy to have a chat separately via IRC/matrix/VC etc.
Cheers,
Peter
[1] https://lore.kernel.org/u-boot/20251119174740.00be0a78@rocky.lan/
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RESEND PATCH 3/4] Fix #cells lookup in of_get_dma_range
2025-11-05 16:44 ` [RESEND PATCH 3/4] Fix #cells lookup in of_get_dma_range Torsten Duwe
@ 2025-11-28 14:44 ` Peter Robinson
0 siblings, 0 replies; 11+ messages in thread
From: Peter Robinson @ 2025-11-28 14:44 UTC (permalink / raw)
To: Torsten Duwe
Cc: Matthias Brugger, Simon Glass, Andrew Goodbody, Christian Marangi,
Elaine Zhang, Gabriel Fernandez, Heiko Stuebner, Huan Zhou,
Ilias Apalodimas, Jan Čermák, Jerome Forissier,
Joe Hershberger, Jonas Karlman, Joseph Chen, Kever Yang,
Leo Yu-Chi Liang, Marek Vasut, Patrice Chotard, Patrick Delaunay,
Paul Barker, Ramon Fried, Tom Rini, Volodymyr Babchuk,
Oleksii Moisieiev, Quentin Schulz, u-boot
On Wed, 5 Nov 2025 at 16:44, Torsten Duwe <duwe@lst.de> wrote:
>
> of_get_dma_range uses (of_bus *)->count_cells to look up the
> #cells for addresses and sizes. It tries to do so for the current
> device node and for the parent tree. count_cells defaults to
> of_bus_default_count_cells, which _starts_ looking at the parent
> node already, and then upwards. The correct way is to use
> of_simple_*_cells for the current node, and count_cells(dev) for
> the parent tree.
>
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> Signed-off-by: Torsten Duwe <duwe@suse.de>
> Reviewed-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
There can't be the same person sign off and review, were the reviews
on the public list?
> ---
>
> Here is the original, very detailed analysis and explanation
It would likely be useful to make this more succinct and put the most
useful bits of it in the commit, in documentation or something
similar.
> [edited] :
>
> According to the Section 2.3.9 of the Device Tree specification[0]
> the format of the value of the dma-ranges property is an arbitrary
> number of triplets of (child-bus-address,
> parent-bus-address, length).
> Where:
> * The child-bus-address is a physical address within the child bus’
> address space. The number of cells to represent the address depends on
> the bus and can be determined from the #address-cells of this node
> (the node in which the dma-ranges property appears).
> * The parent-bus-address is a physical address within the parent bus’
> address space. The number of cells to represent the parent address
> is bus dependent and can be determined from the #address-cells
> property of the node that defines the parent’s address space.
> * The length specifies the size of the range in the child’s address
> space. The number of cells to represent the size can be determined
> from the #size-cells of this node (the node in which the dma-ranges
> property appears)
>
> This description matches comments in the of_addr source code. But the
> actual implementation does not match the specification.
> 'of_match_bus' returns address-cells and size-cells of the parent
> device. That's why the following calls were used to receive addr_cells
> and size_cells of the current device:
> - of_simple_addr_cells
> - of_simple_size_cells
>
> This fix was tested on RPI5 board with both device types: directly
> connected (e.g Serial) and connected over PCIe (e.g ethernet).
>
> For example when dma_ranges are parsing the following device tree
> structure:
> / {
> pcie2: {
> #address-cells = <3>;
> #size-cells = <2>;
> // 64GB system RAM space at PCIe 10_00000000
> dma-ranges =
> <0x02000000 0x00 0x00000000
> 0x1f 0x00000000
> 0x00 0x00400000>,
> <0x43000000 0x10 0x00000000
> 0x00 0x00000000
> 0x10 0x00000000>;
> rp1: {
> #address-cells = <2>;
> #size-cells = <2>;
> dma-ranges =
> // inbound RP1 1x_xxxxxxxx -> PCIe 1x_xxxxxxxx
> <0x10 0x00000000
> 0x43000000 0x10 0x00000000
> 0x10 0x00000000>,
>
> // inbound RP1 c0_40xxxxxx -> PCIe 00_00xxxxxx
> <0xc0 0x40000000
> 0x02000000 0x0 0x00000000
> 0x0 0x00400000>,
>
> // inbound RP1 0x_xxxxxxxx -> PCIe 1x_xxxxxxxx
> <0x00 0x00000000
> 0x02000000 0x10 0x00000000
> 0x10 0x00000000>;
> };
> };
> };
>
> So when we parse ranges on rp1, #address-cells of the rp1 node should
> be taken into account.
> In the provided example we have the following format:
> pcie2:
> #addres-cells = <0x3>;
> #size-cells = <0x2>;
> dma-ranges = < addr_hi addr_mid addr_lo
> trans_addr_hi trans_addr_lo
> size_hi size_lo>;
>
> rp1:
> #addres-cells = <0x2>;
> #size-cells = <0x2>;
> dma-ranges = < addr_hi addr_lo
> trans_addr_hi trans_addr_mid trans_addr_lo
> size_hi size_lo>;
>
> of_get_dma_range searches for the first dev with dma_ranges
> and then executes of_match_bus which follows by count_cells call
> for both device and parent.
>
> count_cells returns #address-cells for the parent device to
> the caller.
> So in the provided example address cells should be 2 for rp1,
> but we get 3 when using count_cells.
> This breaks address translation and is fixed in the provided patch
> by using the of_simple_addr_cells call, which will return correct
> address-cells for the node. This matches the requirements of the
> device-tree specification.
>
> [0] https://readthedocs.org/projects/devicetree-specification/downloads/pdf/latest/
>
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
There's more sign offs?
> ---
> drivers/core/of_addr.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/core/of_addr.c b/drivers/core/of_addr.c
> index 250dd175b55..823a4d70a6b 100644
> --- a/drivers/core/of_addr.c
> +++ b/drivers/core/of_addr.c
> @@ -374,16 +374,16 @@ int of_get_dma_range(const struct device_node *dev, phys_addr_t *cpu,
> }
>
> /* Get the address sizes both for the bus and its parent */
> - bus_node = of_match_bus((struct device_node*)dev);
> - bus_node->count_cells(dev, &na, &ns);
> + na = of_simple_addr_cells(dev);
> + ns = of_simple_size_cells(dev);
> if (!OF_CHECK_COUNTS(na, ns)) {
> printf("Bad cell count for %s\n", of_node_full_name(dev));
> ret = -EINVAL;
> goto out_parent;
> }
>
> - bus_node = of_match_bus(parent);
> - bus_node->count_cells(parent, &pna, &pns);
> + bus_node = of_match_bus((struct device_node *)dev);
> + bus_node->count_cells(dev, &pna, &pns);
> if (!OF_CHECK_COUNTS(pna, pns)) {
> printf("Bad cell count for %s\n", of_node_full_name(parent));
> ret = -EINVAL;
> --
> 2.51.0
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RESEND PATCH 0/4] Improve RaspBerry Pi 5 support part1: Fixes
2025-11-28 14:38 ` [RESEND PATCH 0/4] Improve RaspBerry Pi 5 support part1: Fixes Peter Robinson
@ 2026-07-19 10:43 ` Mike Groves
0 siblings, 0 replies; 11+ messages in thread
From: Mike Groves @ 2026-07-19 10:43 UTC (permalink / raw)
To: pbrobinson
Cc: Oleksii_Moisieiev, andrew.goodbody, ansuelsmth, chenjh, duwe,
gabriel.fernandez, heiko, ilias.apalodimas, jerome.forissier,
joe.hershberger, jonas, kever.yang, marek.vasut+renesas, mbrugger,
me, patrice.chotard, patrick.delaunay, paul.barker.ct,
quentin.schulz, rfried.dev, sairon, sjg, trini, u-boot,
volodymyr_babchuk, ycliang, zhangqing
Hi Peter, Torsten, all,
I have not been part of this thread so far — some context: I have been running a forward-port of this same work (full set) in production for several weeks now, across four RPi 5 boards (revision e04171 / D0 stepping), as Kubernetes worker nodes booting from NVMe via a custom Talos installer overlay. This is what prompted me to start following this thread and look at contributing.
The ask is understood, and tracks with what I ran into looking at this myself - happy to help with that cleanup if useful, deferring to Oleksii/Volodymyr/Torsten on attribution for anything I did not originally author.
The three-way split makes sense to me. One thing I want to make sure I understand correctly before anyone puts more work in: when you say RP1 support is now upstream, could you point me at exactly what's landed and where (kernel tree, specific commits/series)? I want to make sure part 3 gets built against whatever the actual current reference implementation is, rather than against the static BAR-configuration approach the original RFC used - my understanding was there was disagreement about that specific point, so if there is now a settled upstream answer I want to build on that correctly rather than repeat the same discussion.
Once there is an agreed shape for this, I am glad to help test against real D0-stepping hardware - that's one gap I can concretely help close if not already tested. Happy to also help with the actual restructuring work if that's useful, or just stay out of the way if Torsten's already got it in hand - whichever is more helpful.
Mike
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-07-19 13:11 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-05 16:35 [RESEND PATCH 0/4] Improve RaspBerry Pi 5 support part1: Fixes Torsten Duwe
2025-11-05 16:44 ` [RESEND PATCH 1/4] macb: wrap asm/arch/clk.h with ifndef CLK Torsten Duwe
2025-11-06 15:51 ` Matthias Brugger
2025-11-05 16:44 ` [RESEND PATCH 2/4] pci-uclass: respect ofnode_read_pci_vendev retval Torsten Duwe
2025-11-06 15:51 ` Matthias Brugger
2025-11-19 16:47 ` Torsten Duwe
2025-11-05 16:44 ` [RESEND PATCH 3/4] Fix #cells lookup in of_get_dma_range Torsten Duwe
2025-11-28 14:44 ` Peter Robinson
2025-11-05 16:44 ` [RESEND PATCH 4/4] pcie_brcmstb: use bus_base for PCI config devices Torsten Duwe
2025-11-28 14:38 ` [RESEND PATCH 0/4] Improve RaspBerry Pi 5 support part1: Fixes Peter Robinson
2026-07-19 10:43 ` Mike Groves
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.