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

Posted by Andrea della Porta 1 month, 3 weeks ago

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

Posted by Krzysztof Kozlowski 1 month, 3 weeks ago

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

Posted by Andrea della Porta 1 month, 2 weeks ago

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

Posted by Krzysztof Kozlowski 1 month, 1 week ago

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

Posted by Andrea della Porta 1 month ago

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