Re: [PATCH v2 3/6] dt-bindings: soc: starfive: syscon: Add optional patternProperties

[Krzysztof Kozlowski]

On 20/03/2023 09:26, Xingyu Wu wrote:
> On 2023/3/20 15:40, Krzysztof Kozlowski wrote:
>> On 20/03/2023 08:29, Xingyu Wu wrote:
>>> On 2023/3/20 14:37, Krzysztof Kozlowski wrote:
>>>> On 20/03/2023 04:54, Xingyu Wu wrote:
>>>>> On 2023/3/19 20:28, Krzysztof Kozlowski wrote:
>>>>>> On 16/03/2023 04:05, Xingyu Wu wrote:
>>>>>>> Add optional compatible and patternProperties.
>>>>>>>
>>>>>>> Signed-off-by: Xingyu Wu <xingyu.wu@xxxxxxxxxxxxxxxx>
>>>>>>> ---
>>>>>>>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 39 ++++++++++++++++---
>>>>>>>  1 file changed, 33 insertions(+), 6 deletions(-)
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>>>> index ae7f1d6916af..b61d8921ef42 100644
>>>>>>> --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>>>> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>>>> @@ -15,16 +15,31 @@ description: |
>>>>>>>  
>>>>>>>  properties:
>>>>>>>    compatible:
>>>>>>> -    items:
>>>>>>> -      - enum:
>>>>>>> -          - starfive,jh7110-aon-syscon
>>>>>>> -          - starfive,jh7110-stg-syscon
>>>>>>> -          - starfive,jh7110-sys-syscon
>>>>>>> -      - const: syscon
>>>>>>> +    oneOf:
>>>>>>> +      - items:
>>>>>>> +          - enum:
>>>>>>> +              - starfive,jh7110-aon-syscon
>>>>>>> +              - starfive,jh7110-stg-syscon
>>>>>>> +              - starfive,jh7110-sys-syscon
>>>>>>> +          - const: syscon
>>>>>>> +      - items:
>>>>>>> +          - enum:
>>>>>>> +              - starfive,jh7110-aon-syscon
>>>>>>> +              - starfive,jh7110-stg-syscon
>>>>>>> +              - starfive,jh7110-sys-syscon
>>>>>>> +          - const: syscon
>>>>>>> +          - const: simple-mfd
>>
>> BTW, this also looks wrong. You just said that clock controller exists
>> only in few variants. Also, why sometimes the same device  goes with
>> simple-mfd and sometimies without? It's the same device.
> 
> Oh yes, If modified to:
> 
> oneOf:
>       - items:
>           - enum:
>               - starfive,jh7110-aon-syscon
>               - starfive,jh7110-stg-syscon
>           - const: syscon
>       - items:
>           - const: starfive,jh7110-sys-syscon
>           - const: syscon
>           - const: simple-mfd
> 
> Or:
> 
>      - minItems: 2
>        items:
>          - enum:
>              - starfive,jh7110-aon-syscon
>              - starfive,jh7110-stg-syscon
>              - starfive,jh7110-sys-syscon
>          - const: syscon
>          - const: simple-mfd
> 
> 
> Which one is better?

If aon and stg are not supposed to have children, then only the first is
correct. It's not which is better, the second is not really correct in
such case.

Best regards,
Krzysztof