diff mbox series

[08/10] ARM: dts: NSP: Add a default compatible for switch node

Message ID 20201110033113.31090-9-f.fainelli@gmail.com
State New
Headers show
Series Broadcom b53 YAML bindings | expand

Commit Message

Florian Fainelli Nov. 10, 2020, 3:31 a.m. UTC
Provide a default compatible string which is based on the 58522 SRAB
compatible, this allows us to have sane defaults and silences the
following warnings:

 arch/arm/boot/dts/bcm958522er.dt.yaml:
    ethernet-switch@36000: compatible: 'oneOf' conditional failed,
one
    must be fixed:
            ['brcm,bcm5301x-srab'] is too short
            'brcm,bcm5325' was expected
            'brcm,bcm53115' was expected
            'brcm,bcm53125' was expected
            'brcm,bcm53128' was expected
            'brcm,bcm5365' was expected
            'brcm,bcm5395' was expected
            'brcm,bcm5389' was expected
            'brcm,bcm5397' was expected
            'brcm,bcm5398' was expected
            'brcm,bcm11360-srab' was expected
            'brcm,bcm5301x-srab' is not one of ['brcm,bcm53010-srab',
    'brcm,bcm53011-srab', 'brcm,bcm53012-srab', 'brcm,bcm53018-srab',
    'brcm,bcm53019-srab']
            'brcm,bcm5301x-srab' is not one of ['brcm,bcm11404-srab',
    'brcm,bcm11407-srab', 'brcm,bcm11409-srab', 'brcm,bcm58310-srab',
    'brcm,bcm58311-srab', 'brcm,bcm58313-srab']
            'brcm,bcm5301x-srab' is not one of ['brcm,bcm58522-srab',
    'brcm,bcm58523-srab', 'brcm,bcm58525-srab', 'brcm,bcm58622-srab',
    'brcm,bcm58623-srab', 'brcm,bcm58625-srab', 'brcm,bcm88312-srab']
            'brcm,bcm5301x-srab' is not one of ['brcm,bcm3384-switch',
    'brcm,bcm6328-switch', 'brcm,bcm6368-switch']
            From schema:
    Documentation/devicetree/bindings/net/dsa/b53.yaml

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/arm/boot/dts/bcm-nsp.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Vladimir Oltean Nov. 10, 2020, 10:37 p.m. UTC | #1
On Mon, Nov 09, 2020 at 07:31:11PM -0800, Florian Fainelli wrote:
> Provide a default compatible string which is based on the 58522 SRAB
> compatible, this allows us to have sane defaults and silences the
> following warnings:
>
>  arch/arm/boot/dts/bcm958522er.dt.yaml:
>     ethernet-switch@36000: compatible: 'oneOf' conditional failed,
> one
>     must be fixed:
>             ['brcm,bcm5301x-srab'] is too short
>             'brcm,bcm5325' was expected
>             'brcm,bcm53115' was expected
>             'brcm,bcm53125' was expected
>             'brcm,bcm53128' was expected
>             'brcm,bcm5365' was expected
>             'brcm,bcm5395' was expected
>             'brcm,bcm5389' was expected
>             'brcm,bcm5397' was expected
>             'brcm,bcm5398' was expected
>             'brcm,bcm11360-srab' was expected
>             'brcm,bcm5301x-srab' is not one of ['brcm,bcm53010-srab',
>     'brcm,bcm53011-srab', 'brcm,bcm53012-srab', 'brcm,bcm53018-srab',
>     'brcm,bcm53019-srab']
>             'brcm,bcm5301x-srab' is not one of ['brcm,bcm11404-srab',
>     'brcm,bcm11407-srab', 'brcm,bcm11409-srab', 'brcm,bcm58310-srab',
>     'brcm,bcm58311-srab', 'brcm,bcm58313-srab']
>             'brcm,bcm5301x-srab' is not one of ['brcm,bcm58522-srab',
>     'brcm,bcm58523-srab', 'brcm,bcm58525-srab', 'brcm,bcm58622-srab',
>     'brcm,bcm58623-srab', 'brcm,bcm58625-srab', 'brcm,bcm88312-srab']
>             'brcm,bcm5301x-srab' is not one of ['brcm,bcm3384-switch',
>     'brcm,bcm6328-switch', 'brcm,bcm6368-switch']
>             From schema:
>     Documentation/devicetree/bindings/net/dsa/b53.yaml
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  arch/arm/boot/dts/bcm-nsp.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
> index 09fd7e55c069..8453865d1439 100644
> --- a/arch/arm/boot/dts/bcm-nsp.dtsi
> +++ b/arch/arm/boot/dts/bcm-nsp.dtsi
> @@ -386,7 +386,7 @@ ccbtimer1: timer@35000 {
>  		};
>
>  		srab: ethernet-switch@36000 {
> -			compatible = "brcm,nsp-srab";
> +			compatible = "brcm,bcm58522-srab", "brcm,nsp-srab";
>  			reg = <0x36000 0x1000>,
>  			      <0x3f308 0x8>,
>  			      <0x3f410 0xc>;
> --
> 2.25.1
>

I am not getting this.
The line:
#include "bcm-nsp.dtsi"

can be found in:

arch/arm/boot/dts/bcm988312hr.dts
arch/arm/boot/dts/bcm958625hr.dts
arch/arm/boot/dts/bcm958622hr.dts
arch/arm/boot/dts/bcm958625k.dts
arch/arm/boot/dts/bcm958522er.dts
arch/arm/boot/dts/bcm958525er.dts
arch/arm/boot/dts/bcm958623hr.dts
arch/arm/boot/dts/bcm958525xmc.dts


The pattern for the other DTS files that include this seems to be to
overwrite the compatible locally in bcm958522er.dts, like this:

&srab {
	compatible = "brcm,bcm58522-srab", "brcm,nsp-srab";
};

Is there a reason why you are choosing to put an SoC specific compatible
in the common bcm-nsp.dtsi?
Florian Fainelli Nov. 10, 2020, 10:40 p.m. UTC | #2
On 11/10/20 2:37 PM, Vladimir Oltean wrote:
> On Mon, Nov 09, 2020 at 07:31:11PM -0800, Florian Fainelli wrote:
>> Provide a default compatible string which is based on the 58522 SRAB
>> compatible, this allows us to have sane defaults and silences the
>> following warnings:
>>
>>  arch/arm/boot/dts/bcm958522er.dt.yaml:
>>     ethernet-switch@36000: compatible: 'oneOf' conditional failed,
>> one
>>     must be fixed:
>>             ['brcm,bcm5301x-srab'] is too short
>>             'brcm,bcm5325' was expected
>>             'brcm,bcm53115' was expected
>>             'brcm,bcm53125' was expected
>>             'brcm,bcm53128' was expected
>>             'brcm,bcm5365' was expected
>>             'brcm,bcm5395' was expected
>>             'brcm,bcm5389' was expected
>>             'brcm,bcm5397' was expected
>>             'brcm,bcm5398' was expected
>>             'brcm,bcm11360-srab' was expected
>>             'brcm,bcm5301x-srab' is not one of ['brcm,bcm53010-srab',
>>     'brcm,bcm53011-srab', 'brcm,bcm53012-srab', 'brcm,bcm53018-srab',
>>     'brcm,bcm53019-srab']
>>             'brcm,bcm5301x-srab' is not one of ['brcm,bcm11404-srab',
>>     'brcm,bcm11407-srab', 'brcm,bcm11409-srab', 'brcm,bcm58310-srab',
>>     'brcm,bcm58311-srab', 'brcm,bcm58313-srab']
>>             'brcm,bcm5301x-srab' is not one of ['brcm,bcm58522-srab',
>>     'brcm,bcm58523-srab', 'brcm,bcm58525-srab', 'brcm,bcm58622-srab',
>>     'brcm,bcm58623-srab', 'brcm,bcm58625-srab', 'brcm,bcm88312-srab']
>>             'brcm,bcm5301x-srab' is not one of ['brcm,bcm3384-switch',
>>     'brcm,bcm6328-switch', 'brcm,bcm6368-switch']
>>             From schema:
>>     Documentation/devicetree/bindings/net/dsa/b53.yaml
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>  arch/arm/boot/dts/bcm-nsp.dtsi | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
>> index 09fd7e55c069..8453865d1439 100644
>> --- a/arch/arm/boot/dts/bcm-nsp.dtsi
>> +++ b/arch/arm/boot/dts/bcm-nsp.dtsi
>> @@ -386,7 +386,7 @@ ccbtimer1: timer@35000 {
>>  		};
>>
>>  		srab: ethernet-switch@36000 {
>> -			compatible = "brcm,nsp-srab";
>> +			compatible = "brcm,bcm58522-srab", "brcm,nsp-srab";
>>  			reg = <0x36000 0x1000>,
>>  			      <0x3f308 0x8>,
>>  			      <0x3f410 0xc>;
>> --
>> 2.25.1
>>
> 
> I am not getting this.
> The line:
> #include "bcm-nsp.dtsi"
> 
> can be found in:
> 
> arch/arm/boot/dts/bcm988312hr.dts
> arch/arm/boot/dts/bcm958625hr.dts
> arch/arm/boot/dts/bcm958622hr.dts
> arch/arm/boot/dts/bcm958625k.dts
> arch/arm/boot/dts/bcm958522er.dts
> arch/arm/boot/dts/bcm958525er.dts
> arch/arm/boot/dts/bcm958623hr.dts
> arch/arm/boot/dts/bcm958525xmc.dts
> 
> 
> The pattern for the other DTS files that include this seems to be to
> overwrite the compatible locally in bcm958522er.dts, like this:
> 
> &srab {
> 	compatible = "brcm,bcm58522-srab", "brcm,nsp-srab";
> };
> 
> Is there a reason why you are choosing to put an SoC specific compatible
> in the common bcm-nsp.dtsi?

It is necessary to silence the warnings provided in the commit message
even when the srab node is disabled, since the dt_binding_check rule
will check all of the nodes matching the pattern. If there is a better
way to do this, I would gladly do it differently.
Vladimir Oltean Nov. 10, 2020, 10:48 p.m. UTC | #3
On Tue, Nov 10, 2020 at 02:40:43PM -0800, Florian Fainelli wrote:
> On 11/10/20 2:37 PM, Vladimir Oltean wrote:
> > On Mon, Nov 09, 2020 at 07:31:11PM -0800, Florian Fainelli wrote:
> >> Provide a default compatible string which is based on the 58522 SRAB
> >> compatible, this allows us to have sane defaults and silences the
> >> following warnings:
> >>
> >>  arch/arm/boot/dts/bcm958522er.dt.yaml:
> >>     ethernet-switch@36000: compatible: 'oneOf' conditional failed,
> >> one
> >>     must be fixed:
> >>             ['brcm,bcm5301x-srab'] is too short
> >>             'brcm,bcm5325' was expected
> >>             'brcm,bcm53115' was expected
> >>             'brcm,bcm53125' was expected
> >>             'brcm,bcm53128' was expected
> >>             'brcm,bcm5365' was expected
> >>             'brcm,bcm5395' was expected
> >>             'brcm,bcm5389' was expected
> >>             'brcm,bcm5397' was expected
> >>             'brcm,bcm5398' was expected
> >>             'brcm,bcm11360-srab' was expected
> >>             'brcm,bcm5301x-srab' is not one of ['brcm,bcm53010-srab',
> >>     'brcm,bcm53011-srab', 'brcm,bcm53012-srab', 'brcm,bcm53018-srab',
> >>     'brcm,bcm53019-srab']
> >>             'brcm,bcm5301x-srab' is not one of ['brcm,bcm11404-srab',
> >>     'brcm,bcm11407-srab', 'brcm,bcm11409-srab', 'brcm,bcm58310-srab',
> >>     'brcm,bcm58311-srab', 'brcm,bcm58313-srab']
> >>             'brcm,bcm5301x-srab' is not one of ['brcm,bcm58522-srab',
> >>     'brcm,bcm58523-srab', 'brcm,bcm58525-srab', 'brcm,bcm58622-srab',
> >>     'brcm,bcm58623-srab', 'brcm,bcm58625-srab', 'brcm,bcm88312-srab']
> >>             'brcm,bcm5301x-srab' is not one of ['brcm,bcm3384-switch',
> >>     'brcm,bcm6328-switch', 'brcm,bcm6368-switch']
> >>             From schema:
> >>     Documentation/devicetree/bindings/net/dsa/b53.yaml
> >>
> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> >> ---
> >>  arch/arm/boot/dts/bcm-nsp.dtsi | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
> >> index 09fd7e55c069..8453865d1439 100644
> >> --- a/arch/arm/boot/dts/bcm-nsp.dtsi
> >> +++ b/arch/arm/boot/dts/bcm-nsp.dtsi
> >> @@ -386,7 +386,7 @@ ccbtimer1: timer@35000 {
> >>  		};
> >>
> >>  		srab: ethernet-switch@36000 {
> >> -			compatible = "brcm,nsp-srab";
> >> +			compatible = "brcm,bcm58522-srab", "brcm,nsp-srab";
> >>  			reg = <0x36000 0x1000>,
> >>  			      <0x3f308 0x8>,
> >>  			      <0x3f410 0xc>;
> >> --
> >> 2.25.1
> >>
> > 
> > I am not getting this.
> > The line:
> > #include "bcm-nsp.dtsi"
> > 
> > can be found in:
> > 
> > arch/arm/boot/dts/bcm988312hr.dts
> > arch/arm/boot/dts/bcm958625hr.dts
> > arch/arm/boot/dts/bcm958622hr.dts
> > arch/arm/boot/dts/bcm958625k.dts
> > arch/arm/boot/dts/bcm958522er.dts
> > arch/arm/boot/dts/bcm958525er.dts
> > arch/arm/boot/dts/bcm958623hr.dts
> > arch/arm/boot/dts/bcm958525xmc.dts
> > 
> > 
> > The pattern for the other DTS files that include this seems to be to
> > overwrite the compatible locally in bcm958522er.dts, like this:
> > 
> > &srab {
> > 	compatible = "brcm,bcm58522-srab", "brcm,nsp-srab";
> > };
> > 
> > Is there a reason why you are choosing to put an SoC specific compatible
> > in the common bcm-nsp.dtsi?
> 
> It is necessary to silence the warnings provided in the commit message
> even when the srab node is disabled, since the dt_binding_check rule
> will check all of the nodes matching the pattern. If there is a better
> way to do this, I would gladly do it differently.
> -- 
> Florian

I am still not getting it. The exact 3 lines from above will not change
the "status" property from "disabled" to "okay", so I don't understand
why it matters whether it's enabled or not. The dt_binding_check error
isn't in the DTSI, it's in bcm958522er.dts. All that needs to be done is
that the bcm958522er.dts needs to override the compatible from the DTSI
and only the compatible, I believe. With no occurrence of an incomplete
list of compatibles in any final DTS, the dt_binding_check should not
complain about that single occurrence in the DTSI as far as I know (and
I did not test this).
Florian Fainelli Nov. 10, 2020, 10:52 p.m. UTC | #4
On 11/10/20 2:48 PM, Vladimir Oltean wrote:
> On Tue, Nov 10, 2020 at 02:40:43PM -0800, Florian Fainelli wrote:
>> On 11/10/20 2:37 PM, Vladimir Oltean wrote:
>>> On Mon, Nov 09, 2020 at 07:31:11PM -0800, Florian Fainelli wrote:
>>>> Provide a default compatible string which is based on the 58522 SRAB
>>>> compatible, this allows us to have sane defaults and silences the
>>>> following warnings:
>>>>
>>>>  arch/arm/boot/dts/bcm958522er.dt.yaml:
>>>>     ethernet-switch@36000: compatible: 'oneOf' conditional failed,
>>>> one
>>>>     must be fixed:
>>>>             ['brcm,bcm5301x-srab'] is too short
>>>>             'brcm,bcm5325' was expected
>>>>             'brcm,bcm53115' was expected
>>>>             'brcm,bcm53125' was expected
>>>>             'brcm,bcm53128' was expected
>>>>             'brcm,bcm5365' was expected
>>>>             'brcm,bcm5395' was expected
>>>>             'brcm,bcm5389' was expected
>>>>             'brcm,bcm5397' was expected
>>>>             'brcm,bcm5398' was expected
>>>>             'brcm,bcm11360-srab' was expected
>>>>             'brcm,bcm5301x-srab' is not one of ['brcm,bcm53010-srab',
>>>>     'brcm,bcm53011-srab', 'brcm,bcm53012-srab', 'brcm,bcm53018-srab',
>>>>     'brcm,bcm53019-srab']
>>>>             'brcm,bcm5301x-srab' is not one of ['brcm,bcm11404-srab',
>>>>     'brcm,bcm11407-srab', 'brcm,bcm11409-srab', 'brcm,bcm58310-srab',
>>>>     'brcm,bcm58311-srab', 'brcm,bcm58313-srab']
>>>>             'brcm,bcm5301x-srab' is not one of ['brcm,bcm58522-srab',
>>>>     'brcm,bcm58523-srab', 'brcm,bcm58525-srab', 'brcm,bcm58622-srab',
>>>>     'brcm,bcm58623-srab', 'brcm,bcm58625-srab', 'brcm,bcm88312-srab']
>>>>             'brcm,bcm5301x-srab' is not one of ['brcm,bcm3384-switch',
>>>>     'brcm,bcm6328-switch', 'brcm,bcm6368-switch']
>>>>             From schema:
>>>>     Documentation/devicetree/bindings/net/dsa/b53.yaml
>>>>
>>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>>>> ---
>>>>  arch/arm/boot/dts/bcm-nsp.dtsi | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
>>>> index 09fd7e55c069..8453865d1439 100644
>>>> --- a/arch/arm/boot/dts/bcm-nsp.dtsi
>>>> +++ b/arch/arm/boot/dts/bcm-nsp.dtsi
>>>> @@ -386,7 +386,7 @@ ccbtimer1: timer@35000 {
>>>>  		};
>>>>
>>>>  		srab: ethernet-switch@36000 {
>>>> -			compatible = "brcm,nsp-srab";
>>>> +			compatible = "brcm,bcm58522-srab", "brcm,nsp-srab";
>>>>  			reg = <0x36000 0x1000>,
>>>>  			      <0x3f308 0x8>,
>>>>  			      <0x3f410 0xc>;
>>>> --
>>>> 2.25.1
>>>>
>>>
>>> I am not getting this.
>>> The line:
>>> #include "bcm-nsp.dtsi"
>>>
>>> can be found in:
>>>
>>> arch/arm/boot/dts/bcm988312hr.dts
>>> arch/arm/boot/dts/bcm958625hr.dts
>>> arch/arm/boot/dts/bcm958622hr.dts
>>> arch/arm/boot/dts/bcm958625k.dts
>>> arch/arm/boot/dts/bcm958522er.dts
>>> arch/arm/boot/dts/bcm958525er.dts
>>> arch/arm/boot/dts/bcm958623hr.dts
>>> arch/arm/boot/dts/bcm958525xmc.dts
>>>
>>>
>>> The pattern for the other DTS files that include this seems to be to
>>> overwrite the compatible locally in bcm958522er.dts, like this:
>>>
>>> &srab {
>>> 	compatible = "brcm,bcm58522-srab", "brcm,nsp-srab";
>>> };
>>>
>>> Is there a reason why you are choosing to put an SoC specific compatible
>>> in the common bcm-nsp.dtsi?
>>
>> It is necessary to silence the warnings provided in the commit message
>> even when the srab node is disabled, since the dt_binding_check rule
>> will check all of the nodes matching the pattern. If there is a better
>> way to do this, I would gladly do it differently.
>> -- 
>> Florian
> 
> I am still not getting it. The exact 3 lines from above will not change
> the "status" property from "disabled" to "okay", so I don't understand
> why it matters whether it's enabled or not. The dt_binding_check error
> isn't in the DTSI, it's in bcm958522er.dts. All that needs to be done is
> that the bcm958522er.dts needs to override the compatible from the DTSI
> and only the compatible, I believe. With no occurrence of an incomplete
> list of compatibles in any final DTS, the dt_binding_check should not
> complain about that single occurrence in the DTSI as far as I know (and
> I did not test this).

There is not a switch being enabled in
arch/arm/boot/dts/bcm958522er.dts, so sure, I could add the 3 lines you
quote above and that would silence the warning, but that does not scale
at all across DTS files including bcm5301x.dtsi for instance, it sort of
does for those including bcm-nsp.dtsi.
Vladimir Oltean Nov. 10, 2020, 11:06 p.m. UTC | #5
On Tue, Nov 10, 2020 at 02:52:57PM -0800, Florian Fainelli wrote:
> On 11/10/20 2:48 PM, Vladimir Oltean wrote:
> > On Tue, Nov 10, 2020 at 02:40:43PM -0800, Florian Fainelli wrote:
> >> On 11/10/20 2:37 PM, Vladimir Oltean wrote:
> >>> On Mon, Nov 09, 2020 at 07:31:11PM -0800, Florian Fainelli wrote:
> >>>> Provide a default compatible string which is based on the 58522 SRAB
> >>>> compatible, this allows us to have sane defaults and silences the
> >>>> following warnings:
> >>>>
> >>>>  arch/arm/boot/dts/bcm958522er.dt.yaml:
> >>>>     ethernet-switch@36000: compatible: 'oneOf' conditional failed,
> >>>> one
> >>>>     must be fixed:
> >>>>             ['brcm,bcm5301x-srab'] is too short
> >>>>             'brcm,bcm5325' was expected
> >>>>             'brcm,bcm53115' was expected
> >>>>             'brcm,bcm53125' was expected
> >>>>             'brcm,bcm53128' was expected
> >>>>             'brcm,bcm5365' was expected
> >>>>             'brcm,bcm5395' was expected
> >>>>             'brcm,bcm5389' was expected
> >>>>             'brcm,bcm5397' was expected
> >>>>             'brcm,bcm5398' was expected
> >>>>             'brcm,bcm11360-srab' was expected
> >>>>             'brcm,bcm5301x-srab' is not one of ['brcm,bcm53010-srab',
> >>>>     'brcm,bcm53011-srab', 'brcm,bcm53012-srab', 'brcm,bcm53018-srab',
> >>>>     'brcm,bcm53019-srab']
> >>>>             'brcm,bcm5301x-srab' is not one of ['brcm,bcm11404-srab',
> >>>>     'brcm,bcm11407-srab', 'brcm,bcm11409-srab', 'brcm,bcm58310-srab',
> >>>>     'brcm,bcm58311-srab', 'brcm,bcm58313-srab']
> >>>>             'brcm,bcm5301x-srab' is not one of ['brcm,bcm58522-srab',
> >>>>     'brcm,bcm58523-srab', 'brcm,bcm58525-srab', 'brcm,bcm58622-srab',
> >>>>     'brcm,bcm58623-srab', 'brcm,bcm58625-srab', 'brcm,bcm88312-srab']
> >>>>             'brcm,bcm5301x-srab' is not one of ['brcm,bcm3384-switch',
> >>>>     'brcm,bcm6328-switch', 'brcm,bcm6368-switch']
> >>>>             From schema:
> >>>>     Documentation/devicetree/bindings/net/dsa/b53.yaml
> >>>>
> >>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> >>>> ---
> >>>>  arch/arm/boot/dts/bcm-nsp.dtsi | 2 +-
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
> >>>> index 09fd7e55c069..8453865d1439 100644
> >>>> --- a/arch/arm/boot/dts/bcm-nsp.dtsi
> >>>> +++ b/arch/arm/boot/dts/bcm-nsp.dtsi
> >>>> @@ -386,7 +386,7 @@ ccbtimer1: timer@35000 {
> >>>>  		};
> >>>>
> >>>>  		srab: ethernet-switch@36000 {
> >>>> -			compatible = "brcm,nsp-srab";
> >>>> +			compatible = "brcm,bcm58522-srab", "brcm,nsp-srab";
> >>>>  			reg = <0x36000 0x1000>,
> >>>>  			      <0x3f308 0x8>,
> >>>>  			      <0x3f410 0xc>;
> >>>> --
> >>>> 2.25.1
> >>>>
> >>>
> >>> I am not getting this.
> >>> The line:
> >>> #include "bcm-nsp.dtsi"
> >>>
> >>> can be found in:
> >>>
> >>> arch/arm/boot/dts/bcm988312hr.dts
> >>> arch/arm/boot/dts/bcm958625hr.dts
> >>> arch/arm/boot/dts/bcm958622hr.dts
> >>> arch/arm/boot/dts/bcm958625k.dts
> >>> arch/arm/boot/dts/bcm958522er.dts
> >>> arch/arm/boot/dts/bcm958525er.dts
> >>> arch/arm/boot/dts/bcm958623hr.dts
> >>> arch/arm/boot/dts/bcm958525xmc.dts
> >>>
> >>>
> >>> The pattern for the other DTS files that include this seems to be to
> >>> overwrite the compatible locally in bcm958522er.dts, like this:
> >>>
> >>> &srab {
> >>> 	compatible = "brcm,bcm58522-srab", "brcm,nsp-srab";
> >>> };
> >>>
> >>> Is there a reason why you are choosing to put an SoC specific compatible
> >>> in the common bcm-nsp.dtsi?
> >>
> >> It is necessary to silence the warnings provided in the commit message
> >> even when the srab node is disabled, since the dt_binding_check rule
> >> will check all of the nodes matching the pattern. If there is a better
> >> way to do this, I would gladly do it differently.
> >> -- 
> >> Florian
> > 
> > I am still not getting it. The exact 3 lines from above will not change
> > the "status" property from "disabled" to "okay", so I don't understand
> > why it matters whether it's enabled or not. The dt_binding_check error
> > isn't in the DTSI, it's in bcm958522er.dts. All that needs to be done is
> > that the bcm958522er.dts needs to override the compatible from the DTSI
> > and only the compatible, I believe. With no occurrence of an incomplete
> > list of compatibles in any final DTS, the dt_binding_check should not
> > complain about that single occurrence in the DTSI as far as I know (and
> > I did not test this).
> 
> There is not a switch being enabled in
> arch/arm/boot/dts/bcm958522er.dts, so sure, I could add the 3 lines you
> quote above and that would silence the warning, but that does not scale
> at all across DTS files including bcm5301x.dtsi for instance, it sort of
> does for those including bcm-nsp.dtsi.
> -- 
> Florian

I see only bcm47081.dtsi and bcm4708.dtsi to include bcm5301x.dtsi, and
you did point out in the other email that the BCM4708* SoCs always
contain a BCM53010 switch, so that would make the other patch not wrong.

Either way, it's up to you and Rafal.
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
index 09fd7e55c069..8453865d1439 100644
--- a/arch/arm/boot/dts/bcm-nsp.dtsi
+++ b/arch/arm/boot/dts/bcm-nsp.dtsi
@@ -386,7 +386,7 @@  ccbtimer1: timer@35000 {
 		};
 
 		srab: ethernet-switch@36000 {
-			compatible = "brcm,nsp-srab";
+			compatible = "brcm,bcm58522-srab", "brcm,nsp-srab";
 			reg = <0x36000 0x1000>,
 			      <0x3f308 0x8>,
 			      <0x3f410 0xc>;