Re: [RFC PATCH 1/3] dt-bindings: arm64: bcmbca: Merge BCM4908 into BCMBCA

["Rafał Miłecki" <rafal@milecki.pl>]

On 2022-07-13 12:50, Rafał Miłecki wrote:
> On 2022-07-13 02:57, William Zhang wrote:
>> On 7/12/22 11:18, Krzysztof Kozlowski wrote:
>>> On 12/07/2022 19:37, William Zhang wrote:
>>>>>> +      - description: BCM4908 Family based boards
>>>>>> +        items:
>>>>>> +          - enum:
>>>>>> +              # BCM4908 SoC based boards
>>>>>> +              - brcm,bcm94908
>>>>>> +              - asus,gt-ac5300
>>>>>> +              - netgear,raxe500
>>>>>> +              # BCM4906 SoC based boards
>>>>>> +              - brcm,bcm94906
>>>>>> +              - netgear,r8000p
>>>>>> +              - tplink,archer-c2300-v1
>>>>>> +          - enum:
>>>>>> +              - brcm,bcm4908
>>>>>> +              - brcm,bcm4906
>>>>>> +              - brcm,bcm49408
>>>>> 
>>>>> This is wrong.  brcm,bcm94908 followed by brcm,bcm4906 does not 
>>>>> look
>>>>> like valid list of compatibles.
>>>>> 
>>>> For 4908 board variant, it will need to be followed by 4908 chip. 
>>>> Sorry
>>>> for the basic question but is there any requirement to enforce this 
>>>> kind
>>>> of rule?  I would assume dts writer know what he/she is doing and 
>>>> select
>>>> the right combination.
>>> 
>>> The entire point of DT schema is to validate DTS. Combination like 
>>> above
>>> prevents that goal.
>>> 
>>> Best regards,
>>> Krzysztof
>> Understand the DT schema purpose. But items property allows multiple
>> enums in the list which gives a lot of flexibility but make it hard to
>> validate. I am not familiar with DT schema, is there any directive to
>> specify one enum value depending on another so dts validation tool can
>> report error if combination is wrong?
>> 
>> This is our preferred format of all bcmbca compatible string
>> especially when we could have more than 10 chip variants for the same
>> chip family and we really want to work on the chip family id.  We will
>> make sure they are in the right combination in our own patch and patch
>> from other contributors. Would this work? If not, I will probably have
>> to revert the change of 4908(maybe append brcm,bcmbca as this chip
>> belongs to the same bca group) and use "enum board variant", "const
>> main chip id", "brcm,bca" for all other chips as our secondary choice.
> 
> I'm not sure why I didn't even receive 1/3 and half of discussion
> e-mails.
> 
> You can't just put all strings into a single bag and allow mixing them
> in any combos. Please check how it's properly handled in the current
> existing binding:
> Documentation/devicetree/bindings/arm/bcm/brcm,bcm4908.yaml
> 
> Above binding enforces that non-matching compatible strings are not 
> used
> together.

I just noticed you're actually removing brcm,bcm4908.yaml in the 2/3 so
you must be aware of that file.

So you see a cleanly working binding in the brcm,bcm4908.yaml but
instead copying it you decided to wrote your own one from scratch.
Incorrectly.

This smells of NIH (not invented here). Please just use that binding I
wrote and move if it needed.