arm64: dts: imx8mm-venice: Fix PCI bus nodes
Commit Message
The imx8mm-venice boards PCI bus nodes are a complete mess. The
unit-addresses are wrong. The PCI bridge nodes are incomplete missing
"device_type" and "ranges" and just wrong for "#address-cells" and
"#size-cells" values.
All of these issues are reported warnings if anyone bothered to pay
attention. Sigh.
The kernel may have happened to work because it only looks at "reg"
to assign the DT nodes to PCI devices. Based on that, I'm assuming "reg"
is correctly matching the devices present, and the unit-addresses are
wrong. Presumably the bootloader fills in "local-mac-address" with
something valid. Hopefully it too uses "reg" rather than the path.
Fixes: afb424b99e0f ("arm64: dts: imx8mm-venice*: add PCIe support")
Signed-off-by: Rob Herring <robh@kernel.org>
---
.../dts/freescale/imx8mm-venice-gw72xx.dtsi | 28 +++++++++++--------
.../dts/freescale/imx8mm-venice-gw73xx.dtsi | 28 +++++++++++--------
.../dts/freescale/imx8mm-venice-gw7902.dts | 12 ++++----
3 files changed, 38 insertions(+), 30 deletions(-)
Comments
Hi Rob,
On Thu, Nov 30, 2023 at 4:18 PM Rob Herring <robh@kernel.org> wrote:
>
> The imx8mm-venice boards PCI bus nodes are a complete mess. The
> unit-addresses are wrong. The PCI bridge nodes are incomplete missing
> "device_type" and "ranges" and just wrong for "#address-cells" and
> "#size-cells" values.
>
> All of these issues are reported warnings if anyone bothered to pay
> attention. Sigh.
The warnings are gone in linux-next:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/arch/arm64/boot/dts/freescale?h=next-20231130&id=d61c5068729a76a6183a897bcad4bf26e8d3d674
On Thu, Nov 30, 2023 at 1:28 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Rob,
>
> On Thu, Nov 30, 2023 at 4:18 PM Rob Herring <robh@kernel.org> wrote:
> >
> > The imx8mm-venice boards PCI bus nodes are a complete mess. The
> > unit-addresses are wrong. The PCI bridge nodes are incomplete missing
> > "device_type" and "ranges" and just wrong for "#address-cells" and
> > "#size-cells" values.
> >
> > All of these issues are reported warnings if anyone bothered to pay
> > attention. Sigh.
>
> The warnings are gone in linux-next:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/arch/arm64/boot/dts/freescale?h=next-20231130&id=d61c5068729a76a6183a897bcad4bf26e8d3d674
Linux-next is wrong. The ethernet device should have a node name of
'ethernet'. The 'pcie' node name and 'device_type = "pci"' is for PCI
buses/bridges only.
Rob
On Thu, Nov 30, 2023 at 2:33 PM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Nov 30, 2023 at 1:28 PM Fabio Estevam <festevam@gmail.com> wrote:
> >
> > Hi Rob,
> >
> > On Thu, Nov 30, 2023 at 4:18 PM Rob Herring <robh@kernel.org> wrote:
> > >
> > > The imx8mm-venice boards PCI bus nodes are a complete mess. The
> > > unit-addresses are wrong. The PCI bridge nodes are incomplete missing
> > > "device_type" and "ranges" and just wrong for "#address-cells" and
> > > "#size-cells" values.
> > >
> > > All of these issues are reported warnings if anyone bothered to pay
> > > attention. Sigh.
Rob,
Sorry about that. At the time the dt was submitted there were still so
many dt warnings it wasn't very clear what was a legitimate issue and
the PCI bindings are not that easy to understand.
> >
> > The warnings are gone in linux-next:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/arch/arm64/boot/dts/freescale?h=next-20231130&id=d61c5068729a76a6183a897bcad4bf26e8d3d674
>
> Linux-next is wrong. The ethernet device should have a node name of
> 'ethernet'. The 'pcie' node name and 'device_type = "pci"' is for PCI
> buses/bridges only.
So as Fabio has tried to fix this with a patch that landed in
linux-next this patch won't apply. I'll submit one that covers your
changes.
It's always been unfortunate to have to have this level of detail in a
device-tree just to allow boot firmware to populate the mac address of
a PCI ethernet device.
Best Regards,
Tim
On Tue, Dec 5, 2023 at 12:13 PM Tim Harvey <tharvey@gateworks.com> wrote:
>
> On Thu, Nov 30, 2023 at 2:33 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Thu, Nov 30, 2023 at 1:28 PM Fabio Estevam <festevam@gmail.com> wrote:
> > >
> > > Hi Rob,
> > >
> > > On Thu, Nov 30, 2023 at 4:18 PM Rob Herring <robh@kernel.org> wrote:
> > > >
> > > > The imx8mm-venice boards PCI bus nodes are a complete mess. The
> > > > unit-addresses are wrong. The PCI bridge nodes are incomplete missing
> > > > "device_type" and "ranges" and just wrong for "#address-cells" and
> > > > "#size-cells" values.
> > > >
> > > > All of these issues are reported warnings if anyone bothered to pay
> > > > attention. Sigh.
>
> Rob,
>
> Sorry about that. At the time the dt was submitted there were still so
> many dt warnings it wasn't very clear what was a legitimate issue and
> the PCI bindings are not that easy to understand.
>
> > >
> > > The warnings are gone in linux-next:
> > >
> > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/arch/arm64/boot/dts/freescale?h=next-20231130&id=d61c5068729a76a6183a897bcad4bf26e8d3d674
> >
> > Linux-next is wrong. The ethernet device should have a node name of
> > 'ethernet'. The 'pcie' node name and 'device_type = "pci"' is for PCI
> > buses/bridges only.
>
> So as Fabio has tried to fix this with a patch that landed in
> linux-next this patch won't apply. I'll submit one that covers your
> changes.
Thanks.
> It's always been unfortunate to have to have this level of detail in a
> device-tree just to allow boot firmware to populate the mac address of
> a PCI ethernet device.
More unfortunate are incomplete h/w designs lacking the MAC address. :(
Not really any way around it I think if you want something that works
for any device and any number of devices.
Rob
@@ -150,25 +150,29 @@ &pcie0 {
<&clk IMX8MM_SYS_PLL2_250M>;
status = "okay";
- pcie@0,0 {
+ pcie@0 {
+ device_type = "pci";
reg = <0x0000 0 0 0 0>;
- #address-cells = <1>;
- #size-cells = <0>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges;
- pcie@1,0 {
+ pcie@0 {
+ device_type = "pci";
reg = <0x0000 0 0 0 0>;
- #address-cells = <1>;
- #size-cells = <0>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges;
- pcie@2,3 {
+ pcie@3 {
+ device_type = "pci";
reg = <0x1800 0 0 0 0>;
- #address-cells = <1>;
- #size-cells = <0>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges;
- eth1: pcie@5,0 {
+ eth1: ethernet@0 {
reg = <0x0000 0 0 0 0>;
- #address-cells = <1>;
- #size-cells = <0>;
local-mac-address = [00 00 00 00 00 00];
};
@@ -177,25 +177,29 @@ &pcie0 {
<&clk IMX8MM_SYS_PLL2_250M>;
status = "okay";
- pcie@0,0 {
+ pcie@0 {
+ device_type = "pci";
reg = <0x0000 0 0 0 0>;
- #address-cells = <1>;
- #size-cells = <0>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges;
- pcie@1,0 {
+ pcie@0 {
+ device_type = "pci";
reg = <0x0000 0 0 0 0>;
- #address-cells = <1>;
- #size-cells = <0>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges;
- pcie@2,4 {
+ pcie@4 {
+ device_type = "pci";
reg = <0x2000 0 0 0 0>;
- #address-cells = <1>;
- #size-cells = <0>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges;
- eth1: pcie@6,0 {
+ eth1: ethernet@0 {
reg = <0x0000 0 0 0 0>;
- #address-cells = <1>;
- #size-cells = <0>;
local-mac-address = [00 00 00 00 00 00];
};
@@ -633,15 +633,15 @@ &pcie0 {
<&clk IMX8MM_SYS_PLL2_250M>;
status = "okay";
- pcie@0,0 {
+ pcie@0 {
+ device_type = "pci";
reg = <0x0000 0 0 0 0>;
- #address-cells = <1>;
- #size-cells = <0>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges;
- eth1: pcie@1,0 {
+ eth1: ethernet@0 {
reg = <0x0000 0 0 0 0>;
- #address-cells = <1>;
- #size-cells = <0>;
local-mac-address = [00 00 00 00 00 00];
};