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

Tue Dec 9 10:46:45 PST 2025

On Tue, Dec 9, 2025 at 12:24 PM Andrea della Porta
<andrea.porta at 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 at 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 at 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 at 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 at intel.com>
> > > > > >>> Closes: https://lore.kernel.org/oe-kbuild-all/202506041952.baJDYBT4-lkp@intel.com/
> > > > > >>> Signed-off-by: Andrea della Porta <andrea.porta at 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 at 0,0 or
> > > > > > dev at 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