[PATCH] dt-bindings: pci: brcmstb: Add rp1-nexus node to fix DTC warning

[PATCH] dt-bindings: pci: brcmstb: Add rp1-nexus node to fix DTC warning

[Andrea della Porta]

The devicetree compiler is complaining as follows:

arch/arm64/boot/dts/broadcom/rp1-nexus.dtsi:3.11-14.3: Warning (unit_address_vs_reg): /axi/pcie@1000120000/rp1_nexus: node has a reg or ranges property, but no unit name
/home/andrea/linux-torvalds/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@1000120000: Unevaluated properties are not allowed ('rp1_nexus' was unexpected)

Add the optional node that fix this to the DT binding.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202506041952.baJDYBT4-lkp@intel.com/
Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
---
 Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
index 812ef5957cfc..7d8ba920b652 100644
--- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
@@ -126,6 +126,15 @@ required:
 allOf:
   - $ref: /schemas/pci/pci-host-bridge.yaml#
   - $ref: /schemas/interrupt-controller/msi-controller.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: brcm,bcm2712-pcie
+    then:
+      properties:
+        rp1_nexus:
+          $ref: /schemas/misc/pci1de4,1.yaml
   - if:
       properties:
         compatible:
-- 
2.35.3

Re: [PATCH] dt-bindings: pci: brcmstb: Add rp1-nexus node to fix DTC warning

[Krzysztof Kozlowski]

On 12/08/2025 10:50, Andrea della Porta wrote:
> The devicetree compiler is complaining as follows:
> 
> arch/arm64/boot/dts/broadcom/rp1-nexus.dtsi:3.11-14.3: Warning (unit_address_vs_reg): /axi/pcie@1000120000/rp1_nexus: node has a reg or ranges property, but no unit name
> /home/andrea/linux-torvalds/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@1000120000: Unevaluated properties are not allowed ('rp1_nexus' was unexpected)

Please trim the paths.

> 
> Add the optional node that fix this to the DT binding.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202506041952.baJDYBT4-lkp@intel.com/
> Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
> ---
>  Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> index 812ef5957cfc..7d8ba920b652 100644
> --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> @@ -126,6 +126,15 @@ required:
>  allOf:
>    - $ref: /schemas/pci/pci-host-bridge.yaml#
>    - $ref: /schemas/interrupt-controller/msi-controller.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: brcm,bcm2712-pcie
> +    then:
> +      properties:
> +        rp1_nexus:

No, you cannot document post-factum... This does not follow DTS coding
style.

Also:

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

... and nodes should be anyway defined in top-level and only customized
per variant. I am surprised that DTS patch carries a reviewed tag,
because it was never checked/tested :/

> +          $ref: /schemas/misc/pci1de4,1.yaml
>    - if:
>        properties:
>          compatible:


Best regards,
Krzysztof

Re: [PATCH] dt-bindings: pci: brcmstb: Add rp1-nexus node to fix DTC warning

[Andrea della Porta]

Hi Krzysztof,

On 10:55 Tue 12 Aug     , Krzysztof Kozlowski wrote:
> On 12/08/2025 10:50, Andrea della Porta wrote:
> > The devicetree compiler is complaining as follows:
> > 
> > arch/arm64/boot/dts/broadcom/rp1-nexus.dtsi:3.11-14.3: Warning (unit_address_vs_reg): /axi/pcie@1000120000/rp1_nexus: node has a reg or ranges property, but no unit name
> > /home/andrea/linux-torvalds/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@1000120000: Unevaluated properties are not allowed ('rp1_nexus' was unexpected)
> 
> Please trim the paths.

Ack.

> 
> > 
> > Add the optional node that fix this to the DT binding.
> > 
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202506041952.baJDYBT4-lkp@intel.com/
> > Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
> > ---
> >  Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > index 812ef5957cfc..7d8ba920b652 100644
> > --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > @@ -126,6 +126,15 @@ required:
> >  allOf:
> >    - $ref: /schemas/pci/pci-host-bridge.yaml#
> >    - $ref: /schemas/interrupt-controller/msi-controller.yaml#
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: brcm,bcm2712-pcie
> > +    then:
> > +      properties:
> > +        rp1_nexus:
> 
> No, you cannot document post-factum... This does not follow DTS coding
> style.

I think I didn't catch what you mean here: would that mean that
we cannot resolve that warning since we cannot add anything to the
binding?

Regarding rp1_nexus, you're right I guess it should be
rp1-nexus as per DTS coding style.

> 
> Also:
> 
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

In this case it could be difficult: we need to search for a DT node
starting from the DT root and using generic names like pci@0,0 or
dev@0,0 could possibly led to conflicts with other peripherals.
That's why I chose a specific name.

Many thanks,
Andrea

> 
> ... and nodes should be anyway defined in top-level and only customized
> per variant. I am surprised that DTS patch carries a reviewed tag,
> because it was never checked/tested :/
> 
> > +          $ref: /schemas/misc/pci1de4,1.yaml
> >    - if:
> >        properties:
> >          compatible:
> 
> 
> Best regards,
> Krzysztof

Re: [PATCH] dt-bindings: pci: brcmstb: Add rp1-nexus node to fix DTC warning

[Krzysztof Kozlowski]

On 21/08/2025 17:22, Andrea della Porta wrote:
> Hi Krzysztof,
> 
> On 10:55 Tue 12 Aug     , Krzysztof Kozlowski wrote:
>> On 12/08/2025 10:50, Andrea della Porta wrote:
>>> The devicetree compiler is complaining as follows:
>>>
>>> arch/arm64/boot/dts/broadcom/rp1-nexus.dtsi:3.11-14.3: Warning (unit_address_vs_reg): /axi/pcie@1000120000/rp1_nexus: node has a reg or ranges property, but no unit name
>>> /home/andrea/linux-torvalds/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@1000120000: Unevaluated properties are not allowed ('rp1_nexus' was unexpected)
>>
>> Please trim the paths.
> 
> Ack.
> 
>>
>>>
>>> Add the optional node that fix this to the DT binding.
>>>
>>> Reported-by: kernel test robot <lkp@intel.com>
>>> Closes: https://lore.kernel.org/oe-kbuild-all/202506041952.baJDYBT4-lkp@intel.com/
>>> Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
>>> ---
>>>  Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml | 9 +++++++++
>>>  1 file changed, 9 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
>>> index 812ef5957cfc..7d8ba920b652 100644
>>> --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
>>> +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
>>> @@ -126,6 +126,15 @@ required:
>>>  allOf:
>>>    - $ref: /schemas/pci/pci-host-bridge.yaml#
>>>    - $ref: /schemas/interrupt-controller/msi-controller.yaml#
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: brcm,bcm2712-pcie
>>> +    then:
>>> +      properties:
>>> +        rp1_nexus:
>>
>> No, you cannot document post-factum... This does not follow DTS coding
>> style.
> 
> I think I didn't catch what you mean here: would that mean that
> we cannot resolve that warning since we cannot add anything to the
> binding?

I meant, you cannot use a warning from the code you recently introduced
as a reason to use incorrect style.

Fixing warning is of course fine and correct, but for the code recently
introduced and which bypassed ABI review it is basically like new review
of new ABI.

This needs standard review practice, so you need to document WHY you
need such node. Warning is not the reason here why you are doing. If
this was part of original patchset, like it should have been, you would
not use some imaginary warning as reason, right?

So provide reason why you need here this dedicated child, what is that
child representing.

Otherwise I can suggest: drop the child and DTSO, this also solves the
warning...

> 
> Regarding rp1_nexus, you're right I guess it should be
> rp1-nexus as per DTS coding style.
> 
>>
>> Also:
>>
>> Node names should be generic. See also an explanation and list of
>> examples (not exhaustive) in DT specification:
>> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> 
> In this case it could be difficult: we need to search for a DT node

Search like in driver? That's wrong, you should be searching by compatible.

> starting from the DT root and using generic names like pci@0,0 or
> dev@0,0 could possibly led to conflicts with other peripherals.
> That's why I chose a specific name.

Dunno, depends what can be there, but you do not get a specific
(non-generic) device node name for a generic PCI device or endpoint.



Best regards,
Krzysztof

Re: [PATCH] dt-bindings: pci: brcmstb: Add rp1-nexus node to fix DTC warning

[Andrea della Porta]

Hi Krzysztof,

On 08:50 Fri 22 Aug     , Krzysztof Kozlowski wrote:
> On 21/08/2025 17:22, Andrea della Porta wrote:
> > Hi Krzysztof,
> > 
> > On 10:55 Tue 12 Aug     , Krzysztof Kozlowski wrote:
> >> On 12/08/2025 10:50, Andrea della Porta wrote:
> >>> The devicetree compiler is complaining as follows:
> >>>
> >>> arch/arm64/boot/dts/broadcom/rp1-nexus.dtsi:3.11-14.3: Warning (unit_address_vs_reg): /axi/pcie@1000120000/rp1_nexus: node has a reg or ranges property, but no unit name
> >>> /home/andrea/linux-torvalds/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@1000120000: Unevaluated properties are not allowed ('rp1_nexus' was unexpected)
> >>
> >> Please trim the paths.
> > 
> > Ack.
> > 
> >>
> >>>
> >>> Add the optional node that fix this to the DT binding.
> >>>
> >>> Reported-by: kernel test robot <lkp@intel.com>
> >>> Closes: https://lore.kernel.org/oe-kbuild-all/202506041952.baJDYBT4-lkp@intel.com/
> >>> Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
> >>> ---
> >>>  Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml | 9 +++++++++
> >>>  1 file changed, 9 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> >>> index 812ef5957cfc..7d8ba920b652 100644
> >>> --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> >>> +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> >>> @@ -126,6 +126,15 @@ required:
> >>>  allOf:
> >>>    - $ref: /schemas/pci/pci-host-bridge.yaml#
> >>>    - $ref: /schemas/interrupt-controller/msi-controller.yaml#
> >>> +  - if:
> >>> +      properties:
> >>> +        compatible:
> >>> +          contains:
> >>> +            const: brcm,bcm2712-pcie
> >>> +    then:
> >>> +      properties:
> >>> +        rp1_nexus:
> >>
> >> No, you cannot document post-factum... This does not follow DTS coding
> >> style.
> > 
> > I think I didn't catch what you mean here: would that mean that
> > we cannot resolve that warning since we cannot add anything to the
> > binding?
> 
> I meant, you cannot use a warning from the code you recently introduced
> as a reason to use incorrect style.
> 
> Fixing warning is of course fine and correct, but for the code recently
> introduced and which bypassed ABI review it is basically like new review
> of new ABI.
> 
> This needs standard review practice, so you need to document WHY you
> need such node. Warning is not the reason here why you are doing. If
> this was part of original patchset, like it should have been, you would
> not use some imaginary warning as reason, right?
> 
> So provide reason why you need here this dedicated child, what is that
> child representing.

Ack.

> 
> Otherwise I can suggest: drop the child and DTSO, this also solves the
> warning...

This would not fix the issue: it's the non overlay that needs the specific
node. But I got the point, and we have a solution for that (see below).

> 
> > 
> > Regarding rp1_nexus, you're right I guess it should be
> > rp1-nexus as per DTS coding style.
> > 
> >>
> >> Also:
> >>
> >> Node names should be generic. See also an explanation and list of
> >> examples (not exhaustive) in DT specification:
> >> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> > 
> > In this case it could be difficult: we need to search for a DT node
> 
> Search like in driver? That's wrong, you should be searching by compatible.

Thanks for the hint. Searching by compatble is the solution.

> 
> > starting from the DT root and using generic names like pci@0,0 or
> > dev@0,0 could possibly led to conflicts with other peripherals.
> > That's why I chose a specific name.
> 
> Dunno, depends what can be there, but you do not get a specific
> (non-generic) device node name for a generic PCI device or endpoint.

I would use 'port' instead of rp1-nexus. Would it work for you?

Many thanks,
Andrea

> 
> 
> 
> Best regards,
> Krzysztof

Re: [PATCH] dt-bindings: pci: brcmstb: Add rp1-nexus node to fix DTC warning

[Rob Herring]

On Mon, Sep 1, 2025 at 3:48 AM Andrea della Porta <andrea.porta@suse.com> wrote:
>
> Hi Krzysztof,
>
> On 08:50 Fri 22 Aug     , Krzysztof Kozlowski wrote:
> > On 21/08/2025 17:22, Andrea della Porta wrote:
> > > Hi Krzysztof,
> > >
> > > On 10:55 Tue 12 Aug     , Krzysztof Kozlowski wrote:
> > >> On 12/08/2025 10:50, Andrea della Porta wrote:
> > >>> The devicetree compiler is complaining as follows:
> > >>>
> > >>> arch/arm64/boot/dts/broadcom/rp1-nexus.dtsi:3.11-14.3: Warning (unit_address_vs_reg): /axi/pcie@1000120000/rp1_nexus: node has a reg or ranges property, but no unit name
> > >>> /home/andrea/linux-torvalds/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@1000120000: Unevaluated properties are not allowed ('rp1_nexus' was unexpected)
> > >>
> > >> Please trim the paths.
> > >
> > > Ack.
> > >
> > >>
> > >>>
> > >>> Add the optional node that fix this to the DT binding.
> > >>>
> > >>> Reported-by: kernel test robot <lkp@intel.com>
> > >>> Closes: https://lore.kernel.org/oe-kbuild-all/202506041952.baJDYBT4-lkp@intel.com/
> > >>> Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
> > >>> ---
> > >>>  Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml | 9 +++++++++
> > >>>  1 file changed, 9 insertions(+)
> > >>>
> > >>> diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > >>> index 812ef5957cfc..7d8ba920b652 100644
> > >>> --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > >>> +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > >>> @@ -126,6 +126,15 @@ required:
> > >>>  allOf:
> > >>>    - $ref: /schemas/pci/pci-host-bridge.yaml#
> > >>>    - $ref: /schemas/interrupt-controller/msi-controller.yaml#
> > >>> +  - if:
> > >>> +      properties:
> > >>> +        compatible:
> > >>> +          contains:
> > >>> +            const: brcm,bcm2712-pcie
> > >>> +    then:
> > >>> +      properties:
> > >>> +        rp1_nexus:
> > >>
> > >> No, you cannot document post-factum... This does not follow DTS coding
> > >> style.
> > >
> > > I think I didn't catch what you mean here: would that mean that
> > > we cannot resolve that warning since we cannot add anything to the
> > > binding?
> >
> > I meant, you cannot use a warning from the code you recently introduced
> > as a reason to use incorrect style.
> >
> > Fixing warning is of course fine and correct, but for the code recently
> > introduced and which bypassed ABI review it is basically like new review
> > of new ABI.
> >
> > This needs standard review practice, so you need to document WHY you
> > need such node. Warning is not the reason here why you are doing. If
> > this was part of original patchset, like it should have been, you would
> > not use some imaginary warning as reason, right?
> >
> > So provide reason why you need here this dedicated child, what is that
> > child representing.
>
> Ack.
>
> >
> > Otherwise I can suggest: drop the child and DTSO, this also solves the
> > warning...
>
> This would not fix the issue: it's the non overlay that needs the specific
> node. But I got the point, and we have a solution for that (see below).
>
> >
> > >
> > > Regarding rp1_nexus, you're right I guess it should be
> > > rp1-nexus as per DTS coding style.
> > >
> > >>
> > >> Also:
> > >>
> > >> Node names should be generic. See also an explanation and list of
> > >> examples (not exhaustive) in DT specification:
> > >> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> > >
> > > In this case it could be difficult: we need to search for a DT node
> >
> > Search like in driver? That's wrong, you should be searching by compatible.
>
> Thanks for the hint. Searching by compatble is the solution.

No, it is not.

> >
> > > starting from the DT root and using generic names like pci@0,0 or
> > > dev@0,0 could possibly led to conflicts with other peripherals.
> > > That's why I chose a specific name.
> >
> > Dunno, depends what can be there, but you do not get a specific
> > (non-generic) device node name for a generic PCI device or endpoint.
>
> I would use 'port' instead of rp1-nexus. Would it work for you?

Do you still plan to fix this? This is broken far worse than just the node name.

The 'rp1_nexus' node is applied to the PCI host bridge. That's wrong
unless this is PCI rather than PCIe. There's the root port device in
between.

The clue that things are wrong are start in the driver here:

rp1_node = of_find_node_by_name(NULL, "rp1_nexus");
if (!rp1_node) {
  rp1_node = dev_of_node(dev);
  skip_ovl = false;
}

You should not need to do this nor care what the node name is. The PCI
core should have populated pdev->dev.of_node for you. If not, your DT
is wrong. Turn on CONFIG_PCI_DYNAMIC_OF_NODES and look at the
resulting PCI nodes. They should also match what the hierarchy looks
like with lspci. I don't recommend you rely on
CONFIG_PCI_DYNAMIC_OF_NODES, but statically populate the nodes in the
DT. First, CONFIG_PCI_DYNAMIC_OF_NODES is an under development thing
and I hope to get rid of the config option. Second, your case is
static (i.e. not a PCIe card in a slot) so there is no issue
hardcoding the DT.

As far as the node name, I don't care so much as long as the driver
doesn't care and you don't use '_' or 'pcie?' (that's for PCI
bridges).

And why do we have drivers/misc/rp1/rp1-pci.dtso and a .dtso in
arch/arm64? There should not be both.

Rob

Re: [PATCH] dt-bindings: pci: brcmstb: Add rp1-nexus node to fix DTC warning

[Andrea della Porta]

Hi Rob,

On 11:09 Thu 04 Dec     , Rob Herring wrote:
> On Mon, Sep 1, 2025 at 3:48 AM Andrea della Porta <andrea.porta@suse.com> wrote:
> >
> > Hi Krzysztof,
> >
> > On 08:50 Fri 22 Aug     , Krzysztof Kozlowski wrote:
> > > On 21/08/2025 17:22, Andrea della Porta wrote:
> > > > Hi Krzysztof,
> > > >
> > > > On 10:55 Tue 12 Aug     , Krzysztof Kozlowski wrote:
> > > >> On 12/08/2025 10:50, Andrea della Porta wrote:
> > > >>> The devicetree compiler is complaining as follows:
> > > >>>
> > > >>> arch/arm64/boot/dts/broadcom/rp1-nexus.dtsi:3.11-14.3: Warning (unit_address_vs_reg): /axi/pcie@1000120000/rp1_nexus: node has a reg or ranges property, but no unit name
> > > >>> /home/andrea/linux-torvalds/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@1000120000: Unevaluated properties are not allowed ('rp1_nexus' was unexpected)
> > > >>
> > > >> Please trim the paths.
> > > >
> > > > Ack.
> > > >
> > > >>
> > > >>>
> > > >>> Add the optional node that fix this to the DT binding.
> > > >>>
> > > >>> Reported-by: kernel test robot <lkp@intel.com>
> > > >>> Closes: https://lore.kernel.org/oe-kbuild-all/202506041952.baJDYBT4-lkp@intel.com/
> > > >>> Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
> > > >>> ---
> > > >>>  Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml | 9 +++++++++
> > > >>>  1 file changed, 9 insertions(+)
> > > >>>
> > > >>> diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > > >>> index 812ef5957cfc..7d8ba920b652 100644
> > > >>> --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > > >>> +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > > >>> @@ -126,6 +126,15 @@ required:
> > > >>>  allOf:
> > > >>>    - $ref: /schemas/pci/pci-host-bridge.yaml#
> > > >>>    - $ref: /schemas/interrupt-controller/msi-controller.yaml#
> > > >>> +  - if:
> > > >>> +      properties:
> > > >>> +        compatible:
> > > >>> +          contains:
> > > >>> +            const: brcm,bcm2712-pcie
> > > >>> +    then:
> > > >>> +      properties:
> > > >>> +        rp1_nexus:
> > > >>
> > > >> No, you cannot document post-factum... This does not follow DTS coding
> > > >> style.
> > > >
> > > > I think I didn't catch what you mean here: would that mean that
> > > > we cannot resolve that warning since we cannot add anything to the
> > > > binding?
> > >
> > > I meant, you cannot use a warning from the code you recently introduced
> > > as a reason to use incorrect style.
> > >
> > > Fixing warning is of course fine and correct, but for the code recently
> > > introduced and which bypassed ABI review it is basically like new review
> > > of new ABI.
> > >
> > > This needs standard review practice, so you need to document WHY you
> > > need such node. Warning is not the reason here why you are doing. If
> > > this was part of original patchset, like it should have been, you would
> > > not use some imaginary warning as reason, right?
> > >
> > > So provide reason why you need here this dedicated child, what is that
> > > child representing.
> >
> > Ack.
> >
> > >
> > > Otherwise I can suggest: drop the child and DTSO, this also solves the
> > > warning...
> >
> > This would not fix the issue: it's the non overlay that needs the specific
> > node. But I got the point, and we have a solution for that (see below).
> >
> > >
> > > >
> > > > Regarding rp1_nexus, you're right I guess it should be
> > > > rp1-nexus as per DTS coding style.
> > > >
> > > >>
> > > >> Also:
> > > >>
> > > >> Node names should be generic. See also an explanation and list of
> > > >> examples (not exhaustive) in DT specification:
> > > >> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> > > >
> > > > In this case it could be difficult: we need to search for a DT node
> > >
> > > Search like in driver? That's wrong, you should be searching by compatible.
> >
> > Thanks for the hint. Searching by compatble is the solution.
> 
> No, it is not.

This is partly true, indeed. On one side there's the need to avoid a
specific node name ('rp1_nexus'), so the only other unique identifier would
be the compatible string ('pci1de4,1' in this case, which identifies that specific
device). Unfortunately, the same compatible string is also assigned to the pci
endpoint node filled automatically by enabling CONFIG_PCI_DYNAMIC_OF_NODES.
We would end up with two nodes with the same compatible, which is not unique
anymore. 
This applies only when using 'full' dtb (bcm2712-rpi-5-b.dtb) *and* you enable
CONFIG_PCI_DYNAMIC_OF_NODES, the latter being not necessary since the overlay dtb
(...-ovl-rp1.dtb) is not in use here. To overcome this problem, the solutions
I can think of are the following:

1- Just disable CONFIG_PCI_DYNAMIC_OF_NODES should work, but only when using the
   full dtb version. However, if the user enable that option for debug or to use
   the overlay dtb version, he better be sure not to use teh full dtb or it won't
   work.
   This solution seems really weak.

2- Add another compatible string other than 'pci1de4,1', so it will be really
   unique.

> 
> > >
> > > > starting from the DT root and using generic names like pci@0,0 or
> > > > dev@0,0 could possibly led to conflicts with other peripherals.
> > > > That's why I chose a specific name.
> > >
> > > Dunno, depends what can be there, but you do not get a specific
> > > (non-generic) device node name for a generic PCI device or endpoint.
> >
> > I would use 'port' instead of rp1-nexus. Would it work for you?
> 
> Do you still plan to fix this? This is broken far worse than just the node name.

Yes, if we want to get rid of that nasty warning and comply with DT guidelines,
I think I really need to fix that.

> 
> The 'rp1_nexus' node is applied to the PCI host bridge. That's wrong
> unless this is PCI rather than PCIe. There's the root port device in
> between.
> 
> The clue that things are wrong are start in the driver here:
> 
> rp1_node = of_find_node_by_name(NULL, "rp1_nexus");
> if (!rp1_node) {
>   rp1_node = dev_of_node(dev);
>   skip_ovl = false;
> }
> 
> You should not need to do this nor care what the node name is. The PCI
> core should have populated pdev->dev.of_node for you. If not, your DT
> is wrong. Turn on CONFIG_PCI_DYNAMIC_OF_NODES and look at the
> resulting PCI nodes. They should also match what the hierarchy looks
> like with lspci. I don't recommend you rely on
> CONFIG_PCI_DYNAMIC_OF_NODES, but statically populate the nodes in the
> DT. First, CONFIG_PCI_DYNAMIC_OF_NODES is an under development thing
> and I hope to get rid of the config option. Second, your case is
> static (i.e. not a PCIe card in a slot) so there is no issue
> hardcoding the DT.

The 'full' dtb (bcm2712-rpi-5-b.dtb) is indeed statically populated.
CONFIG_PCI_DYNAMIC_OF_NODES is needed only if you use the overlay approach
(bcm2712-rpi-5-b-ovl-rp1.dtb) and in that case the node will be
added to the correct device node at runtime, and there won't be any node
labeled as rp1_nexus.
That conditional just check for teh presence of the rp1_nexus node to
choose if the overlay should be loaded at runtime (if rp1_nexus is present,
then we are in the static case so no overlay need to be loaded).

> 
> As far as the node name, I don't care so much as long as the driver
> doesn't care and you don't use '_' or 'pcie?' (that's for PCI
> bridges).
> 
> And why do we have drivers/misc/rp1/rp1-pci.dtso and a .dtso in
> arch/arm64? There should not be both.

drivers/misc/rp1/rp1-pci.dtso is just a wrapper over rp1-common.dtsi,
which is to be compiled in as binary blob in the driver and loaded at
runtime.
rp1.dtbo is optional, it's there just for completeness in case anyone want
to load teh overlay from the fw, as explained here:
https://lore.kernel.org/all/ab9ab3536baf5fdf6016f2a01044f00034189291.1742418429.git.andrea.porta@suse.com/
If it causes confusion, we can probably get rid of it with no penalty.

Many thanks,
Andrea

> 
> Rob

Re: [PATCH] dt-bindings: pci: brcmstb: Add rp1-nexus node to fix DTC warning

[Rob Herring]

On Tue, Dec 9, 2025 at 11:56 AM Andrea della Porta
<andrea.porta@suse.com> wrote:
>
> Hi Rob,
>
> On 11:09 Thu 04 Dec     , Rob Herring wrote:
> > On Mon, Sep 1, 2025 at 3:48 AM Andrea della Porta <andrea.porta@suse.com> wrote:
> > >
> > > Hi Krzysztof,
> > >
> > > On 08:50 Fri 22 Aug     , Krzysztof Kozlowski wrote:
> > > > On 21/08/2025 17:22, Andrea della Porta wrote:
> > > > > Hi Krzysztof,
> > > > >
> > > > > On 10:55 Tue 12 Aug     , Krzysztof Kozlowski wrote:
> > > > >> On 12/08/2025 10:50, Andrea della Porta wrote:
> > > > >>> The devicetree compiler is complaining as follows:
> > > > >>>
> > > > >>> arch/arm64/boot/dts/broadcom/rp1-nexus.dtsi:3.11-14.3: Warning (unit_address_vs_reg): /axi/pcie@1000120000/rp1_nexus: node has a reg or ranges property, but no unit name
> > > > >>> /home/andrea/linux-torvalds/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@1000120000: Unevaluated properties are not allowed ('rp1_nexus' was unexpected)
> > > > >>
> > > > >> Please trim the paths.
> > > > >
> > > > > Ack.
> > > > >
> > > > >>
> > > > >>>
> > > > >>> Add the optional node that fix this to the DT binding.
> > > > >>>
> > > > >>> Reported-by: kernel test robot <lkp@intel.com>
> > > > >>> Closes: https://lore.kernel.org/oe-kbuild-all/202506041952.baJDYBT4-lkp@intel.com/
> > > > >>> Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
> > > > >>> ---
> > > > >>>  Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml | 9 +++++++++
> > > > >>>  1 file changed, 9 insertions(+)
> > > > >>>
> > > > >>> diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > > > >>> index 812ef5957cfc..7d8ba920b652 100644
> > > > >>> --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > > > >>> +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > > > >>> @@ -126,6 +126,15 @@ required:
> > > > >>>  allOf:
> > > > >>>    - $ref: /schemas/pci/pci-host-bridge.yaml#
> > > > >>>    - $ref: /schemas/interrupt-controller/msi-controller.yaml#
> > > > >>> +  - if:
> > > > >>> +      properties:
> > > > >>> +        compatible:
> > > > >>> +          contains:
> > > > >>> +            const: brcm,bcm2712-pcie
> > > > >>> +    then:
> > > > >>> +      properties:
> > > > >>> +        rp1_nexus:
> > > > >>
> > > > >> No, you cannot document post-factum... This does not follow DTS coding
> > > > >> style.
> > > > >
> > > > > I think I didn't catch what you mean here: would that mean that
> > > > > we cannot resolve that warning since we cannot add anything to the
> > > > > binding?
> > > >
> > > > I meant, you cannot use a warning from the code you recently introduced
> > > > as a reason to use incorrect style.
> > > >
> > > > Fixing warning is of course fine and correct, but for the code recently
> > > > introduced and which bypassed ABI review it is basically like new review
> > > > of new ABI.
> > > >
> > > > This needs standard review practice, so you need to document WHY you
> > > > need such node. Warning is not the reason here why you are doing. If
> > > > this was part of original patchset, like it should have been, you would
> > > > not use some imaginary warning as reason, right?
> > > >
> > > > So provide reason why you need here this dedicated child, what is that
> > > > child representing.
> > >
> > > Ack.
> > >
> > > >
> > > > Otherwise I can suggest: drop the child and DTSO, this also solves the
> > > > warning...
> > >
> > > This would not fix the issue: it's the non overlay that needs the specific
> > > node. But I got the point, and we have a solution for that (see below).
> > >
> > > >
> > > > >
> > > > > Regarding rp1_nexus, you're right I guess it should be
> > > > > rp1-nexus as per DTS coding style.
> > > > >
> > > > >>
> > > > >> Also:
> > > > >>
> > > > >> Node names should be generic. See also an explanation and list of
> > > > >> examples (not exhaustive) in DT specification:
> > > > >> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> > > > >
> > > > > In this case it could be difficult: we need to search for a DT node
> > > >
> > > > Search like in driver? That's wrong, you should be searching by compatible.
> > >
> > > Thanks for the hint. Searching by compatble is the solution.
> >
> > No, it is not.
>
> This is partly true, indeed. On one side there's the need to avoid a
> specific node name ('rp1_nexus'), so the only other unique identifier would
> be the compatible string ('pci1de4,1' in this case, which identifies that specific
> device). Unfortunately, the same compatible string is also assigned to the pci
> endpoint node filled automatically by enabling CONFIG_PCI_DYNAMIC_OF_NODES.
> We would end up with two nodes with the same compatible, which is not unique
> anymore.
> This applies only when using 'full' dtb (bcm2712-rpi-5-b.dtb) *and* you enable
> CONFIG_PCI_DYNAMIC_OF_NODES, the latter being not necessary since the overlay dtb
> (...-ovl-rp1.dtb) is not in use here. To overcome this problem, the solutions
> I can think of are the following:
>
> 1- Just disable CONFIG_PCI_DYNAMIC_OF_NODES should work, but only when using the
>    full dtb version. However, if the user enable that option for debug or to use
>    the overlay dtb version, he better be sure not to use teh full dtb or it won't
>    work.
>    This solution seems really weak.
>
> 2- Add another compatible string other than 'pci1de4,1', so it will be really
>    unique.

No! Neither one. If you end up with 2 nodes when you turn on
CONFIG_PCI_DYNAMIC_OF_NODES, then you have an error in your DT (the
node that CONFIG_PCI_DYNAMIC_OF_NODES *didn't* create is the one in
error).

You need to fix the structure.

> >
> > > >
> > > > > starting from the DT root and using generic names like pci@0,0 or
> > > > > dev@0,0 could possibly led to conflicts with other peripherals.
> > > > > That's why I chose a specific name.
> > > >
> > > > Dunno, depends what can be there, but you do not get a specific
> > > > (non-generic) device node name for a generic PCI device or endpoint.
> > >
> > > I would use 'port' instead of rp1-nexus. Would it work for you?
> >
> > Do you still plan to fix this? This is broken far worse than just the node name.
>
> Yes, if we want to get rid of that nasty warning and comply with DT guidelines,
> I think I really need to fix that.
>
> >
> > The 'rp1_nexus' node is applied to the PCI host bridge. That's wrong
> > unless this is PCI rather than PCIe. There's the root port device in
> > between.
> >
> > The clue that things are wrong are start in the driver here:
> >
> > rp1_node = of_find_node_by_name(NULL, "rp1_nexus");
> > if (!rp1_node) {
> >   rp1_node = dev_of_node(dev);
> >   skip_ovl = false;
> > }
> >
> > You should not need to do this nor care what the node name is. The PCI
> > core should have populated pdev->dev.of_node for you. If not, your DT
> > is wrong. Turn on CONFIG_PCI_DYNAMIC_OF_NODES and look at the
> > resulting PCI nodes. They should also match what the hierarchy looks
> > like with lspci. I don't recommend you rely on
> > CONFIG_PCI_DYNAMIC_OF_NODES, but statically populate the nodes in the
> > DT. First, CONFIG_PCI_DYNAMIC_OF_NODES is an under development thing
> > and I hope to get rid of the config option. Second, your case is
> > static (i.e. not a PCIe card in a slot) so there is no issue
> > hardcoding the DT.
>
> The 'full' dtb (bcm2712-rpi-5-b.dtb) is indeed statically populated.
> CONFIG_PCI_DYNAMIC_OF_NODES is needed only if you use the overlay approach
> (bcm2712-rpi-5-b-ovl-rp1.dtb) and in that case the node will be
> added to the correct device node at runtime, and there won't be any node
> labeled as rp1_nexus.
> That conditional just check for teh presence of the rp1_nexus node to
> choose if the overlay should be loaded at runtime (if rp1_nexus is present,
> then we are in the static case so no overlay need to be loaded).

Checking is fine, but you are checking the wrong thing.

> >
> > As far as the node name, I don't care so much as long as the driver
> > doesn't care and you don't use '_' or 'pcie?' (that's for PCI
> > bridges).
> >
> > And why do we have drivers/misc/rp1/rp1-pci.dtso and a .dtso in
> > arch/arm64? There should not be both.
>
> drivers/misc/rp1/rp1-pci.dtso is just a wrapper over rp1-common.dtsi,
> which is to be compiled in as binary blob in the driver and loaded at
> runtime.
> rp1.dtbo is optional, it's there just for completeness in case anyone want
> to load teh overlay from the fw, as explained here:
> https://lore.kernel.org/all/ab9ab3536baf5fdf6016f2a01044f00034189291.1742418429.git.andrea.porta@suse.com/
> If it causes confusion, we can probably get rid of it with no penalty.

Supporting both ways is fine, but you don't need to duplicate the
source. You can build in a .dtbo from the arch/arm64/boot/dts/...
directories.

Rob

Re: [PATCH] dt-bindings: pci: brcmstb: Add rp1-nexus node to fix DTC warning

[Andrea della Porta]

[+cc Herve]

On 18:58 Tue 09 Dec     , Andrea della Porta wrote:
> Hi Rob,
> 
> On 11:09 Thu 04 Dec     , Rob Herring wrote:
> > On Mon, Sep 1, 2025 at 3:48 AM Andrea della Porta <andrea.porta@suse.com> wrote:
> > >
> > > Hi Krzysztof,
> > >
> > > On 08:50 Fri 22 Aug     , Krzysztof Kozlowski wrote:
> > > > On 21/08/2025 17:22, Andrea della Porta wrote:
> > > > > Hi Krzysztof,
> > > > >
> > > > > On 10:55 Tue 12 Aug     , Krzysztof Kozlowski wrote:
> > > > >> On 12/08/2025 10:50, Andrea della Porta wrote:
> > > > >>> The devicetree compiler is complaining as follows:
> > > > >>>
> > > > >>> arch/arm64/boot/dts/broadcom/rp1-nexus.dtsi:3.11-14.3: Warning (unit_address_vs_reg): /axi/pcie@1000120000/rp1_nexus: node has a reg or ranges property, but no unit name
> > > > >>> /home/andrea/linux-torvalds/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@1000120000: Unevaluated properties are not allowed ('rp1_nexus' was unexpected)
> > > > >>
> > > > >> Please trim the paths.
> > > > >
> > > > > Ack.
> > > > >
> > > > >>
> > > > >>>
> > > > >>> Add the optional node that fix this to the DT binding.
> > > > >>>
> > > > >>> Reported-by: kernel test robot <lkp@intel.com>
> > > > >>> Closes: https://lore.kernel.org/oe-kbuild-all/202506041952.baJDYBT4-lkp@intel.com/
> > > > >>> Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
> > > > >>> ---
> > > > >>>  Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml | 9 +++++++++
> > > > >>>  1 file changed, 9 insertions(+)
> > > > >>>
> > > > >>> diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > > > >>> index 812ef5957cfc..7d8ba920b652 100644
> > > > >>> --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > > > >>> +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > > > >>> @@ -126,6 +126,15 @@ required:
> > > > >>>  allOf:
> > > > >>>    - $ref: /schemas/pci/pci-host-bridge.yaml#
> > > > >>>    - $ref: /schemas/interrupt-controller/msi-controller.yaml#
> > > > >>> +  - if:
> > > > >>> +      properties:
> > > > >>> +        compatible:
> > > > >>> +          contains:
> > > > >>> +            const: brcm,bcm2712-pcie
> > > > >>> +    then:
> > > > >>> +      properties:
> > > > >>> +        rp1_nexus:
> > > > >>
> > > > >> No, you cannot document post-factum... This does not follow DTS coding
> > > > >> style.
> > > > >
> > > > > I think I didn't catch what you mean here: would that mean that
> > > > > we cannot resolve that warning since we cannot add anything to the
> > > > > binding?
> > > >
> > > > I meant, you cannot use a warning from the code you recently introduced
> > > > as a reason to use incorrect style.
> > > >
> > > > Fixing warning is of course fine and correct, but for the code recently
> > > > introduced and which bypassed ABI review it is basically like new review
> > > > of new ABI.
> > > >
> > > > This needs standard review practice, so you need to document WHY you
> > > > need such node. Warning is not the reason here why you are doing. If
> > > > this was part of original patchset, like it should have been, you would
> > > > not use some imaginary warning as reason, right?
> > > >
> > > > So provide reason why you need here this dedicated child, what is that
> > > > child representing.
> > >
> > > Ack.
> > >
> > > >
> > > > Otherwise I can suggest: drop the child and DTSO, this also solves the
> > > > warning...
> > >
> > > This would not fix the issue: it's the non overlay that needs the specific
> > > node. But I got the point, and we have a solution for that (see below).
> > >
> > > >
> > > > >
> > > > > Regarding rp1_nexus, you're right I guess it should be
> > > > > rp1-nexus as per DTS coding style.
> > > > >
> > > > >>
> > > > >> Also:
> > > > >>
> > > > >> Node names should be generic. See also an explanation and list of
> > > > >> examples (not exhaustive) in DT specification:
> > > > >> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> > > > >
> > > > > In this case it could be difficult: we need to search for a DT node
> > > >
> > > > Search like in driver? That's wrong, you should be searching by compatible.
> > >
> > > Thanks for the hint. Searching by compatble is the solution.
> > 
> > No, it is not.
> 
> This is partly true, indeed. On one side there's the need to avoid a
> specific node name ('rp1_nexus'), so the only other unique identifier would
> be the compatible string ('pci1de4,1' in this case, which identifies that specific
> device). Unfortunately, the same compatible string is also assigned to the pci
> endpoint node filled automatically by enabling CONFIG_PCI_DYNAMIC_OF_NODES.
> We would end up with two nodes with the same compatible, which is not unique
> anymore. 
> This applies only when using 'full' dtb (bcm2712-rpi-5-b.dtb) *and* you enable
> CONFIG_PCI_DYNAMIC_OF_NODES, the latter being not necessary since the overlay dtb
> (...-ovl-rp1.dtb) is not in use here. To overcome this problem, the solutions
> I can think of are the following:
> 
> 1- Just disable CONFIG_PCI_DYNAMIC_OF_NODES should work, but only when using the
>    full dtb version. However, if the user enable that option for debug or to use
>    the overlay dtb version, he better be sure not to use teh full dtb or it won't
>    work.
>    This solution seems really weak.
> 
> 2- Add another compatible string other than 'pci1de4,1', so it will be really
>    unique.
> 
> > 
> > > >
> > > > > starting from the DT root and using generic names like pci@0,0 or
> > > > > dev@0,0 could possibly led to conflicts with other peripherals.
> > > > > That's why I chose a specific name.
> > > >
> > > > Dunno, depends what can be there, but you do not get a specific
> > > > (non-generic) device node name for a generic PCI device or endpoint.
> > >
> > > I would use 'port' instead of rp1-nexus. Would it work for you?
> > 
> > Do you still plan to fix this? This is broken far worse than just the node name.
> 
> Yes, if we want to get rid of that nasty warning and comply with DT guidelines,
> I think I really need to fix that.
> 
> > 
> > The 'rp1_nexus' node is applied to the PCI host bridge. That's wrong
> > unless this is PCI rather than PCIe. There's the root port device in
> > between.
> > 
> > The clue that things are wrong are start in the driver here:
> > 
> > rp1_node = of_find_node_by_name(NULL, "rp1_nexus");
> > if (!rp1_node) {
> >   rp1_node = dev_of_node(dev);
> >   skip_ovl = false;
> > }
> > 
> > You should not need to do this nor care what the node name is. The PCI
> > core should have populated pdev->dev.of_node for you. If not, your DT
> > is wrong. Turn on CONFIG_PCI_DYNAMIC_OF_NODES and look at the
> > resulting PCI nodes. They should also match what the hierarchy looks
> > like with lspci. I don't recommend you rely on
> > CONFIG_PCI_DYNAMIC_OF_NODES, but statically populate the nodes in the
> > DT. First, CONFIG_PCI_DYNAMIC_OF_NODES is an under development thing
> > and I hope to get rid of the config option. Second, your case is
> > static (i.e. not a PCIe card in a slot) so there is no issue
> > hardcoding the DT.

Are you planning to get rid of the CONFIG_PCI_DYNAMIC_OF_NODES features?
There could be other drivers relying on that besides RP1 in overlay mode,
e.g. LAN966x for instance.

Thanks,
Andrea

> 
> The 'full' dtb (bcm2712-rpi-5-b.dtb) is indeed statically populated.
> CONFIG_PCI_DYNAMIC_OF_NODES is needed only if you use the overlay approach
> (bcm2712-rpi-5-b-ovl-rp1.dtb) and in that case the node will be
> added to the correct device node at runtime, and there won't be any node
> labeled as rp1_nexus.
> That conditional just check for teh presence of the rp1_nexus node to
> choose if the overlay should be loaded at runtime (if rp1_nexus is present,
> then we are in the static case so no overlay need to be loaded).
> 
> > 
> > As far as the node name, I don't care so much as long as the driver
> > doesn't care and you don't use '_' or 'pcie?' (that's for PCI
> > bridges).
> > 
> > And why do we have drivers/misc/rp1/rp1-pci.dtso and a .dtso in
> > arch/arm64? There should not be both.
> 
> drivers/misc/rp1/rp1-pci.dtso is just a wrapper over rp1-common.dtsi,
> which is to be compiled in as binary blob in the driver and loaded at
> runtime.
> rp1.dtbo is optional, it's there just for completeness in case anyone want
> to load teh overlay from the fw, as explained here:
> https://lore.kernel.org/all/ab9ab3536baf5fdf6016f2a01044f00034189291.1742418429.git.andrea.porta@suse.com/
> If it causes confusion, we can probably get rid of it with no penalty.
> 
> Many thanks,
> Andrea
> 
> > 
> > Rob

Re: [PATCH] dt-bindings: pci: brcmstb: Add rp1-nexus node to fix DTC warning

[Rob Herring]

On Tue, Dec 9, 2025 at 12:24 PM Andrea della Porta
<andrea.porta@suse.com> wrote:
>
> [+cc Herve]
>
> On 18:58 Tue 09 Dec     , Andrea della Porta wrote:
> > Hi Rob,
> >
> > On 11:09 Thu 04 Dec     , Rob Herring wrote:
> > > On Mon, Sep 1, 2025 at 3:48 AM Andrea della Porta <andrea.porta@suse.com> wrote:
> > > >
> > > > Hi Krzysztof,
> > > >
> > > > On 08:50 Fri 22 Aug     , Krzysztof Kozlowski wrote:
> > > > > On 21/08/2025 17:22, Andrea della Porta wrote:
> > > > > > Hi Krzysztof,
> > > > > >
> > > > > > On 10:55 Tue 12 Aug     , Krzysztof Kozlowski wrote:
> > > > > >> On 12/08/2025 10:50, Andrea della Porta wrote:
> > > > > >>> The devicetree compiler is complaining as follows:
> > > > > >>>
> > > > > >>> arch/arm64/boot/dts/broadcom/rp1-nexus.dtsi:3.11-14.3: Warning (unit_address_vs_reg): /axi/pcie@1000120000/rp1_nexus: node has a reg or ranges property, but no unit name
> > > > > >>> /home/andrea/linux-torvalds/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@1000120000: Unevaluated properties are not allowed ('rp1_nexus' was unexpected)
> > > > > >>
> > > > > >> Please trim the paths.
> > > > > >
> > > > > > Ack.
> > > > > >
> > > > > >>
> > > > > >>>
> > > > > >>> Add the optional node that fix this to the DT binding.
> > > > > >>>
> > > > > >>> Reported-by: kernel test robot <lkp@intel.com>
> > > > > >>> Closes: https://lore.kernel.org/oe-kbuild-all/202506041952.baJDYBT4-lkp@intel.com/
> > > > > >>> Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
> > > > > >>> ---
> > > > > >>>  Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml | 9 +++++++++
> > > > > >>>  1 file changed, 9 insertions(+)
> > > > > >>>
> > > > > >>> diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > > > > >>> index 812ef5957cfc..7d8ba920b652 100644
> > > > > >>> --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > > > > >>> +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > > > > >>> @@ -126,6 +126,15 @@ required:
> > > > > >>>  allOf:
> > > > > >>>    - $ref: /schemas/pci/pci-host-bridge.yaml#
> > > > > >>>    - $ref: /schemas/interrupt-controller/msi-controller.yaml#
> > > > > >>> +  - if:
> > > > > >>> +      properties:
> > > > > >>> +        compatible:
> > > > > >>> +          contains:
> > > > > >>> +            const: brcm,bcm2712-pcie
> > > > > >>> +    then:
> > > > > >>> +      properties:
> > > > > >>> +        rp1_nexus:
> > > > > >>
> > > > > >> No, you cannot document post-factum... This does not follow DTS coding
> > > > > >> style.
> > > > > >
> > > > > > I think I didn't catch what you mean here: would that mean that
> > > > > > we cannot resolve that warning since we cannot add anything to the
> > > > > > binding?
> > > > >
> > > > > I meant, you cannot use a warning from the code you recently introduced
> > > > > as a reason to use incorrect style.
> > > > >
> > > > > Fixing warning is of course fine and correct, but for the code recently
> > > > > introduced and which bypassed ABI review it is basically like new review
> > > > > of new ABI.
> > > > >
> > > > > This needs standard review practice, so you need to document WHY you
> > > > > need such node. Warning is not the reason here why you are doing. If
> > > > > this was part of original patchset, like it should have been, you would
> > > > > not use some imaginary warning as reason, right?
> > > > >
> > > > > So provide reason why you need here this dedicated child, what is that
> > > > > child representing.
> > > >
> > > > Ack.
> > > >
> > > > >
> > > > > Otherwise I can suggest: drop the child and DTSO, this also solves the
> > > > > warning...
> > > >
> > > > This would not fix the issue: it's the non overlay that needs the specific
> > > > node. But I got the point, and we have a solution for that (see below).
> > > >
> > > > >
> > > > > >
> > > > > > Regarding rp1_nexus, you're right I guess it should be
> > > > > > rp1-nexus as per DTS coding style.
> > > > > >
> > > > > >>
> > > > > >> Also:
> > > > > >>
> > > > > >> Node names should be generic. See also an explanation and list of
> > > > > >> examples (not exhaustive) in DT specification:
> > > > > >> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> > > > > >
> > > > > > In this case it could be difficult: we need to search for a DT node
> > > > >
> > > > > Search like in driver? That's wrong, you should be searching by compatible.
> > > >
> > > > Thanks for the hint. Searching by compatble is the solution.
> > >
> > > No, it is not.
> >
> > This is partly true, indeed. On one side there's the need to avoid a
> > specific node name ('rp1_nexus'), so the only other unique identifier would
> > be the compatible string ('pci1de4,1' in this case, which identifies that specific
> > device). Unfortunately, the same compatible string is also assigned to the pci
> > endpoint node filled automatically by enabling CONFIG_PCI_DYNAMIC_OF_NODES.
> > We would end up with two nodes with the same compatible, which is not unique
> > anymore.
> > This applies only when using 'full' dtb (bcm2712-rpi-5-b.dtb) *and* you enable
> > CONFIG_PCI_DYNAMIC_OF_NODES, the latter being not necessary since the overlay dtb
> > (...-ovl-rp1.dtb) is not in use here. To overcome this problem, the solutions
> > I can think of are the following:
> >
> > 1- Just disable CONFIG_PCI_DYNAMIC_OF_NODES should work, but only when using the
> >    full dtb version. However, if the user enable that option for debug or to use
> >    the overlay dtb version, he better be sure not to use teh full dtb or it won't
> >    work.
> >    This solution seems really weak.
> >
> > 2- Add another compatible string other than 'pci1de4,1', so it will be really
> >    unique.
> >
> > >
> > > > >
> > > > > > starting from the DT root and using generic names like pci@0,0 or
> > > > > > dev@0,0 could possibly led to conflicts with other peripherals.
> > > > > > That's why I chose a specific name.
> > > > >
> > > > > Dunno, depends what can be there, but you do not get a specific
> > > > > (non-generic) device node name for a generic PCI device or endpoint.
> > > >
> > > > I would use 'port' instead of rp1-nexus. Would it work for you?
> > >
> > > Do you still plan to fix this? This is broken far worse than just the node name.
> >
> > Yes, if we want to get rid of that nasty warning and comply with DT guidelines,
> > I think I really need to fix that.
> >
> > >
> > > The 'rp1_nexus' node is applied to the PCI host bridge. That's wrong
> > > unless this is PCI rather than PCIe. There's the root port device in
> > > between.
> > >
> > > The clue that things are wrong are start in the driver here:
> > >
> > > rp1_node = of_find_node_by_name(NULL, "rp1_nexus");
> > > if (!rp1_node) {
> > >   rp1_node = dev_of_node(dev);
> > >   skip_ovl = false;
> > > }
> > >
> > > You should not need to do this nor care what the node name is. The PCI
> > > core should have populated pdev->dev.of_node for you. If not, your DT
> > > is wrong. Turn on CONFIG_PCI_DYNAMIC_OF_NODES and look at the
> > > resulting PCI nodes. They should also match what the hierarchy looks
> > > like with lspci. I don't recommend you rely on
> > > CONFIG_PCI_DYNAMIC_OF_NODES, but statically populate the nodes in the
> > > DT. First, CONFIG_PCI_DYNAMIC_OF_NODES is an under development thing
> > > and I hope to get rid of the config option. Second, your case is
> > > static (i.e. not a PCIe card in a slot) so there is no issue
> > > hardcoding the DT.
>
> Are you planning to get rid of the CONFIG_PCI_DYNAMIC_OF_NODES features?
> There could be other drivers relying on that besides RP1 in overlay mode,
> e.g. LAN966x for instance.

I only plan/want to get rid of the kconfig option, not the feature
itself! IOW, it will be enabled/disabled at runtime based on
something. I'm not sure what that something is.

Rob

Re: [PATCH] dt-bindings: pci: brcmstb: Add rp1-nexus node to fix DTC warning

[Herve Codina]

Hi Andrea,

On Tue, 9 Dec 2025 19:27:03 +0100
Andrea della Porta <andrea.porta@suse.com> wrote:

> [+cc Herve]
> 
> On 18:58 Tue 09 Dec     , Andrea della Porta wrote:
> > Hi Rob,
> > 
> > On 11:09 Thu 04 Dec     , Rob Herring wrote:  
> > > On Mon, Sep 1, 2025 at 3:48 AM Andrea della Porta <andrea.porta@suse.com> wrote:  
> > > >
> > > > Hi Krzysztof,
> > > >
> > > > On 08:50 Fri 22 Aug     , Krzysztof Kozlowski wrote:  
> > > > > On 21/08/2025 17:22, Andrea della Porta wrote:  
> > > > > > Hi Krzysztof,
> > > > > >
> > > > > > On 10:55 Tue 12 Aug     , Krzysztof Kozlowski wrote:  
> > > > > >> On 12/08/2025 10:50, Andrea della Porta wrote:  
> > > > > >>> The devicetree compiler is complaining as follows:
> > > > > >>>
> > > > > >>> arch/arm64/boot/dts/broadcom/rp1-nexus.dtsi:3.11-14.3: Warning (unit_address_vs_reg): /axi/pcie@1000120000/rp1_nexus: node has a reg or ranges property, but no unit name
> > > > > >>> /home/andrea/linux-torvalds/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@1000120000: Unevaluated properties are not allowed ('rp1_nexus' was unexpected)  
> > > > > >>
> > > > > >> Please trim the paths.  
> > > > > >
> > > > > > Ack.
> > > > > >  
> > > > > >>  
> > > > > >>>
> > > > > >>> Add the optional node that fix this to the DT binding.
> > > > > >>>
> > > > > >>> Reported-by: kernel test robot <lkp@intel.com>
> > > > > >>> Closes: https://lore.kernel.org/oe-kbuild-all/202506041952.baJDYBT4-lkp@intel.com/
> > > > > >>> Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
> > > > > >>> ---
> > > > > >>>  Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml | 9 +++++++++
> > > > > >>>  1 file changed, 9 insertions(+)
> > > > > >>>
> > > > > >>> diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > > > > >>> index 812ef5957cfc..7d8ba920b652 100644
> > > > > >>> --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > > > > >>> +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > > > > >>> @@ -126,6 +126,15 @@ required:
> > > > > >>>  allOf:
> > > > > >>>    - $ref: /schemas/pci/pci-host-bridge.yaml#
> > > > > >>>    - $ref: /schemas/interrupt-controller/msi-controller.yaml#
> > > > > >>> +  - if:
> > > > > >>> +      properties:
> > > > > >>> +        compatible:
> > > > > >>> +          contains:
> > > > > >>> +            const: brcm,bcm2712-pcie
> > > > > >>> +    then:
> > > > > >>> +      properties:
> > > > > >>> +        rp1_nexus:  
> > > > > >>
> > > > > >> No, you cannot document post-factum... This does not follow DTS coding
> > > > > >> style.  
> > > > > >
> > > > > > I think I didn't catch what you mean here: would that mean that
> > > > > > we cannot resolve that warning since we cannot add anything to the
> > > > > > binding?  
> > > > >
> > > > > I meant, you cannot use a warning from the code you recently introduced
> > > > > as a reason to use incorrect style.
> > > > >
> > > > > Fixing warning is of course fine and correct, but for the code recently
> > > > > introduced and which bypassed ABI review it is basically like new review
> > > > > of new ABI.
> > > > >
> > > > > This needs standard review practice, so you need to document WHY you
> > > > > need such node. Warning is not the reason here why you are doing. If
> > > > > this was part of original patchset, like it should have been, you would
> > > > > not use some imaginary warning as reason, right?
> > > > >
> > > > > So provide reason why you need here this dedicated child, what is that
> > > > > child representing.  
> > > >
> > > > Ack.
> > > >  
> > > > >
> > > > > Otherwise I can suggest: drop the child and DTSO, this also solves the
> > > > > warning...  
> > > >
> > > > This would not fix the issue: it's the non overlay that needs the specific
> > > > node. But I got the point, and we have a solution for that (see below).
> > > >  
> > > > >  
> > > > > >
> > > > > > Regarding rp1_nexus, you're right I guess it should be
> > > > > > rp1-nexus as per DTS coding style.
> > > > > >  
> > > > > >>
> > > > > >> Also:
> > > > > >>
> > > > > >> Node names should be generic. See also an explanation and list of
> > > > > >> examples (not exhaustive) in DT specification:
> > > > > >> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation  
> > > > > >
> > > > > > In this case it could be difficult: we need to search for a DT node  
> > > > >
> > > > > Search like in driver? That's wrong, you should be searching by compatible.  
> > > >
> > > > Thanks for the hint. Searching by compatble is the solution.  
> > > 
> > > No, it is not.  
> > 
> > This is partly true, indeed. On one side there's the need to avoid a
> > specific node name ('rp1_nexus'), so the only other unique identifier would
> > be the compatible string ('pci1de4,1' in this case, which identifies that specific
> > device). Unfortunately, the same compatible string is also assigned to the pci
> > endpoint node filled automatically by enabling CONFIG_PCI_DYNAMIC_OF_NODES.
> > We would end up with two nodes with the same compatible, which is not unique
> > anymore. 
> > This applies only when using 'full' dtb (bcm2712-rpi-5-b.dtb) *and* you enable
> > CONFIG_PCI_DYNAMIC_OF_NODES, the latter being not necessary since the overlay dtb
> > (...-ovl-rp1.dtb) is not in use here. To overcome this problem, the solutions
> > I can think of are the following:
> > 
> > 1- Just disable CONFIG_PCI_DYNAMIC_OF_NODES should work, but only when using the
> >    full dtb version. However, if the user enable that option for debug or to use
> >    the overlay dtb version, he better be sure not to use teh full dtb or it won't
> >    work.
> >    This solution seems really weak.
> > 
> > 2- Add another compatible string other than 'pci1de4,1', so it will be really
> >    unique.
> >   
> > >   
> > > > >  
> > > > > > starting from the DT root and using generic names like pci@0,0 or
> > > > > > dev@0,0 could possibly led to conflicts with other peripherals.
> > > > > > That's why I chose a specific name.  
> > > > >
> > > > > Dunno, depends what can be there, but you do not get a specific
> > > > > (non-generic) device node name for a generic PCI device or endpoint.  
> > > >
> > > > I would use 'port' instead of rp1-nexus. Would it work for you?  
> > > 
> > > Do you still plan to fix this? This is broken far worse than just the node name.  
> > 
> > Yes, if we want to get rid of that nasty warning and comply with DT guidelines,
> > I think I really need to fix that.
> >   
> > > 
> > > The 'rp1_nexus' node is applied to the PCI host bridge. That's wrong
> > > unless this is PCI rather than PCIe. There's the root port device in
> > > between.
> > > 
> > > The clue that things are wrong are start in the driver here:
> > > 
> > > rp1_node = of_find_node_by_name(NULL, "rp1_nexus");
> > > if (!rp1_node) {
> > >   rp1_node = dev_of_node(dev);
> > >   skip_ovl = false;
> > > }

I don't fully understand what you want to do.

This piece of code is present in the drivers/misc/rp1/rp1_pci.c driver.
This driver is used when a specific PCI device is available on the system.
This device is PCI_DEVICE(PCI_VENDOR_ID_RPI, PCI_DEVICE_ID_RPI_RP1_C0)

In order to work, the driver needs either CONFIG_PCI_DYNAMIC_OF_NODES or
having the full PCI hierarchy described in the DT leading to this device.
Indeed, it needs a DT node related to the PCI device.

I don't understand why rp1_nexus should be described at the PCI controller
root level.

Only PCI devices (or bridges as they are particular devices) at this level
as decribed in the pci-bus-common.yaml binding [1].

If the purpose of this 'rp1_nexus' node is to only avoid the application of
the overlay, even if I don't understand why, you should search for something
added by the overlay and not for the 'rp1_nexus' node in the whole DT.

Instead of:
  rp1_node = of_find_node_by_name(NULL, "rp1_nexus");

You should search for a sub-node added by the overlay. Something like the
following for instance:
--- 8< ---
int rp1_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
	struct device_node *np;

	np = dev_of_node(&pdev->dev);

	...
	if ( np && of_find_node_by_name(np, "pci-ep-bus@1");
        ...
}
--- 8< ---

If you really want to be sure that the "pci-ep-bus@1" comes from an overlay
you can check for the OF_OVERLAY flag.
--- 8< ---
  node = of_find_node_by_name(np, "pci-ep-bus@1");
  if (node && of_node_check_flag(node, OF_OVERLAY))
--- 8< ---

Also your overlay has to be applied on the DT node related to your PCI
device and not the rp1_nexus you search for in the whole DT. In other
word, in your driver, the following is not correct:

   err = of_overlay_fdt_apply(dtbo_start, dtbo_size, &rp1->ovcs_id,
                                           rp1_node);

It should be:

   err = of_overlay_fdt_apply(dtbo_start, dtbo_size, &rp1->ovcs_id,
                                           dev_of_node(&pdev->dev));
 
In your overlay, your have
	__overlay__ {
		compatible = "pci1de4,1";
		#address-cells = <3>;
		#size-cells = <2>;
		interrupt-controller;
		#interrupt-cells = <2>;

		#include "arm64/broadcom/rp1-common.dtsi"
       };

Only sub-nodes should be present (#address-cells = <3> and #size-cells = <2>)
can be there just to make DTC happy.

It is worth noting that any properties other than #address-cells and
#size-cells lead to memleaks. And so avoid them.

Memleaks are reported at runtime when the overlay is applied with this
kind of logs:
 OF: overlay: WARNING: memory leak will occur if overlay removed, property: ...

I am pretty sure that you have those kind of traces when you apply your
overlay.

compatible, interrupt-controller and #interrupt-cells should not be added by
the overlay. Either they are added by when PCI nodes are created by 
CONFIG_PCI_DYNAMIC_OF_NODES either they are already available in the base
DT if the PCI device is described in the base DT.

[1] https://github.com/devicetree-org/dt-schema/blob/aa859412ce8e38c63bb13fa55c5e1c6ba66a8e3b/dtschema/schemas/pci/pci-bus-common.yaml#L226

Best regards,
Hervé

Re: [PATCH] dt-bindings: pci: brcmstb: Add rp1-nexus node to fix DTC warning

[Andrea della Porta]

Hi Herve, Rob,

On 15:00 Wed 10 Dec     , Herve Codina wrote:
> Hi Andrea,
> 
> On Tue, 9 Dec 2025 19:27:03 +0100
> Andrea della Porta <andrea.porta@suse.com> wrote:
> 
> > [+cc Herve]
> > 
> > On 18:58 Tue 09 Dec     , Andrea della Porta wrote:
> > > Hi Rob,
> > > 
> > > On 11:09 Thu 04 Dec     , Rob Herring wrote:  
> > > > On Mon, Sep 1, 2025 at 3:48 AM Andrea della Porta <andrea.porta@suse.com> wrote:  
> > > > >
> > > > > Hi Krzysztof,
> > > > >
> > > > > On 08:50 Fri 22 Aug     , Krzysztof Kozlowski wrote:  
> > > > > > On 21/08/2025 17:22, Andrea della Porta wrote:  
> > > > > > > Hi Krzysztof,
> > > > > > >
> > > > > > > On 10:55 Tue 12 Aug     , Krzysztof Kozlowski wrote:  
> > > > > > >> On 12/08/2025 10:50, Andrea della Porta wrote:  
> > > > > > >>> The devicetree compiler is complaining as follows:
> > > > > > >>>
> > > > > > >>> arch/arm64/boot/dts/broadcom/rp1-nexus.dtsi:3.11-14.3: Warning (unit_address_vs_reg): /axi/pcie@1000120000/rp1_nexus: node has a reg or ranges property, but no unit name
> > > > > > >>> /home/andrea/linux-torvalds/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@1000120000: Unevaluated properties are not allowed ('rp1_nexus' was unexpected)  
> > > > > > >>
> > > > > > >> Please trim the paths.  
> > > > > > >
> > > > > > > Ack.
> > > > > > >  
> > > > > > >>  
> > > > > > >>>
> > > > > > >>> Add the optional node that fix this to the DT binding.
> > > > > > >>>
> > > > > > >>> Reported-by: kernel test robot <lkp@intel.com>
> > > > > > >>> Closes: https://lore.kernel.org/oe-kbuild-all/202506041952.baJDYBT4-lkp@intel.com/
> > > > > > >>> Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
> > > > > > >>> ---
> > > > > > >>>  Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml | 9 +++++++++
> > > > > > >>>  1 file changed, 9 insertions(+)
> > > > > > >>>
> > > > > > >>> diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > > > > > >>> index 812ef5957cfc..7d8ba920b652 100644
> > > > > > >>> --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > > > > > >>> +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > > > > > >>> @@ -126,6 +126,15 @@ required:
> > > > > > >>>  allOf:
> > > > > > >>>    - $ref: /schemas/pci/pci-host-bridge.yaml#
> > > > > > >>>    - $ref: /schemas/interrupt-controller/msi-controller.yaml#
> > > > > > >>> +  - if:
> > > > > > >>> +      properties:
> > > > > > >>> +        compatible:
> > > > > > >>> +          contains:
> > > > > > >>> +            const: brcm,bcm2712-pcie
> > > > > > >>> +    then:
> > > > > > >>> +      properties:
> > > > > > >>> +        rp1_nexus:  
> > > > > > >>
> > > > > > >> No, you cannot document post-factum... This does not follow DTS coding
> > > > > > >> style.  
> > > > > > >
> > > > > > > I think I didn't catch what you mean here: would that mean that
> > > > > > > we cannot resolve that warning since we cannot add anything to the
> > > > > > > binding?  
> > > > > >
> > > > > > I meant, you cannot use a warning from the code you recently introduced
> > > > > > as a reason to use incorrect style.
> > > > > >
> > > > > > Fixing warning is of course fine and correct, but for the code recently
> > > > > > introduced and which bypassed ABI review it is basically like new review
> > > > > > of new ABI.
> > > > > >
> > > > > > This needs standard review practice, so you need to document WHY you
> > > > > > need such node. Warning is not the reason here why you are doing. If
> > > > > > this was part of original patchset, like it should have been, you would
> > > > > > not use some imaginary warning as reason, right?
> > > > > >
> > > > > > So provide reason why you need here this dedicated child, what is that
> > > > > > child representing.  
> > > > >
> > > > > Ack.
> > > > >  
> > > > > >
> > > > > > Otherwise I can suggest: drop the child and DTSO, this also solves the
> > > > > > warning...  
> > > > >
> > > > > This would not fix the issue: it's the non overlay that needs the specific
> > > > > node. But I got the point, and we have a solution for that (see below).
> > > > >  
> > > > > >  
> > > > > > >
> > > > > > > Regarding rp1_nexus, you're right I guess it should be
> > > > > > > rp1-nexus as per DTS coding style.
> > > > > > >  
> > > > > > >>
> > > > > > >> Also:
> > > > > > >>
> > > > > > >> Node names should be generic. See also an explanation and list of
> > > > > > >> examples (not exhaustive) in DT specification:
> > > > > > >> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation  
> > > > > > >
> > > > > > > In this case it could be difficult: we need to search for a DT node  
> > > > > >
> > > > > > Search like in driver? That's wrong, you should be searching by compatible.  
> > > > >
> > > > > Thanks for the hint. Searching by compatble is the solution.  
> > > > 
> > > > No, it is not.  
> > > 
> > > This is partly true, indeed. On one side there's the need to avoid a
> > > specific node name ('rp1_nexus'), so the only other unique identifier would
> > > be the compatible string ('pci1de4,1' in this case, which identifies that specific
> > > device). Unfortunately, the same compatible string is also assigned to the pci
> > > endpoint node filled automatically by enabling CONFIG_PCI_DYNAMIC_OF_NODES.
> > > We would end up with two nodes with the same compatible, which is not unique
> > > anymore. 
> > > This applies only when using 'full' dtb (bcm2712-rpi-5-b.dtb) *and* you enable
> > > CONFIG_PCI_DYNAMIC_OF_NODES, the latter being not necessary since the overlay dtb
> > > (...-ovl-rp1.dtb) is not in use here. To overcome this problem, the solutions
> > > I can think of are the following:
> > > 
> > > 1- Just disable CONFIG_PCI_DYNAMIC_OF_NODES should work, but only when using the
> > >    full dtb version. However, if the user enable that option for debug or to use
> > >    the overlay dtb version, he better be sure not to use teh full dtb or it won't
> > >    work.
> > >    This solution seems really weak.
> > > 
> > > 2- Add another compatible string other than 'pci1de4,1', so it will be really
> > >    unique.
> > >   
> > > >   
> > > > > >  
> > > > > > > starting from the DT root and using generic names like pci@0,0 or
> > > > > > > dev@0,0 could possibly led to conflicts with other peripherals.
> > > > > > > That's why I chose a specific name.  
> > > > > >
> > > > > > Dunno, depends what can be there, but you do not get a specific
> > > > > > (non-generic) device node name for a generic PCI device or endpoint.  
> > > > >
> > > > > I would use 'port' instead of rp1-nexus. Would it work for you?  
> > > > 
> > > > Do you still plan to fix this? This is broken far worse than just the node name.  
> > > 
> > > Yes, if we want to get rid of that nasty warning and comply with DT guidelines,
> > > I think I really need to fix that.
> > >   
> > > > 
> > > > The 'rp1_nexus' node is applied to the PCI host bridge. That's wrong
> > > > unless this is PCI rather than PCIe. There's the root port device in
> > > > between.
> > > > 
> > > > The clue that things are wrong are start in the driver here:
> > > > 
> > > > rp1_node = of_find_node_by_name(NULL, "rp1_nexus");
> > > > if (!rp1_node) {
> > > >   rp1_node = dev_of_node(dev);
> > > >   skip_ovl = false;
> > > > }
> 
> I don't fully understand what you want to do.
> 
> This piece of code is present in the drivers/misc/rp1/rp1_pci.c driver.
> This driver is used when a specific PCI device is available on the system.
> This device is PCI_DEVICE(PCI_VENDOR_ID_RPI, PCI_DEVICE_ID_RPI_RP1_C0)
> 
> In order to work, the driver needs either CONFIG_PCI_DYNAMIC_OF_NODES or
> having the full PCI hierarchy described in the DT leading to this device.
> Indeed, it needs a DT node related to the PCI device.
> 
> I don't understand why rp1_nexus should be described at the PCI controller
> root level.
> 
> Only PCI devices (or bridges as they are particular devices) at this level
> as decribed in the pci-bus-common.yaml binding [1].

Ack. As you and Rob pointed out, I will amend the full DT hierarchy with the
PCI nodes in the way as they are dynamically created by CONFIG_PCI_DYNAMIC_OF_NODES.

> 
> If the purpose of this 'rp1_nexus' node is to only avoid the application of
> the overlay, even if I don't understand why, you should search for something
> added by the overlay and not for the 'rp1_nexus' node in the whole DT.

I cannot search for something added by the overlay if I need to choose whether
to apply the overlay or not in teh first place (iIOW the overlay is not yet loaded).
This is not of a concern however, since I'm planning to get rid of the overlay
for reasons already explained here [1]. This will not only solve all of this
problems but it will also address Rob's concern of not relying on 
CONFIG_PCI_DYNAMIC_OF_NODES.

> 
> Instead of:
>   rp1_node = of_find_node_by_name(NULL, "rp1_nexus");
> 
> You should search for a sub-node added by the overlay. Something like the
> following for instance:
> --- 8< ---
> int rp1_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> {
> 	struct device_node *np;
> 
> 	np = dev_of_node(&pdev->dev);
> 
> 	...
> 	if ( np && of_find_node_by_name(np, "pci-ep-bus@1");
>         ...
> }
> --- 8< ---
> 
> If you really want to be sure that the "pci-ep-bus@1" comes from an overlay
> you can check for the OF_OVERLAY flag.
> --- 8< ---
>   node = of_find_node_by_name(np, "pci-ep-bus@1");
>   if (node && of_node_check_flag(node, OF_OVERLAY))
> --- 8< ---
> 
> Also your overlay has to be applied on the DT node related to your PCI
> device and not the rp1_nexus you search for in the whole DT. In other
> word, in your driver, the following is not correct:
> 
>    err = of_overlay_fdt_apply(dtbo_start, dtbo_size, &rp1->ovcs_id,
>                                            rp1_node);
> 
> It should be:
> 
>    err = of_overlay_fdt_apply(dtbo_start, dtbo_size, &rp1->ovcs_id,
>                                            dev_of_node(&pdev->dev));
>  
> In your overlay, your have
> 	__overlay__ {
> 		compatible = "pci1de4,1";
> 		#address-cells = <3>;
> 		#size-cells = <2>;
> 		interrupt-controller;
> 		#interrupt-cells = <2>;
> 
> 		#include "arm64/broadcom/rp1-common.dtsi"
>        };
> 
> Only sub-nodes should be present (#address-cells = <3> and #size-cells = <2>)
> can be there just to make DTC happy.
> 
> It is worth noting that any properties other than #address-cells and
> #size-cells lead to memleaks. And so avoid them.
> 
> Memleaks are reported at runtime when the overlay is applied with this
> kind of logs:
>  OF: overlay: WARNING: memory leak will occur if overlay removed, property: ...
> 
> I am pretty sure that you have those kind of traces when you apply your
> overlay.

Indeed.

Many thanks,
Andrea

[1] - https://lore.kernel.org/all/aTvtr5v4DjuqctdY@apocalypse/

> 
> compatible, interrupt-controller and #interrupt-cells should not be added by
> the overlay. Either they are added by when PCI nodes are created by 
> CONFIG_PCI_DYNAMIC_OF_NODES either they are already available in the base
> DT if the PCI device is described in the base DT.
> 
> [1] https://github.com/devicetree-org/dt-schema/blob/aa859412ce8e38c63bb13fa55c5e1c6ba66a8e3b/dtschema/schemas/pci/pci-bus-common.yaml#L226
> 
> Best regards,
> Hervé