Re: [PATCH] dt-bindings: clock: ti: Convert interface.txt to json-schema

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 03/12/2023 23:46, Andreas Kemnade wrote:
> On Fri, 1 Dec 2023 15:45:06 +0100
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> 
>> On 01/12/2023 15:41, Andreas Kemnade wrote:
>>> On Fri, 1 Dec 2023 15:17:46 +0100
>>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>>>   
>>>> On 01/12/2023 15:09, Andreas Kemnade wrote:  
>>>>> Am Wed, 29 Nov 2023 09:15:57 +0100
>>>>> schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>:
>>>>>     
>>>>>> On 28/11/2023 21:41, Andreas Kemnade wrote:    
>>>>>>> Am Tue, 28 Nov 2023 09:41:23 +0100
>>>>>>> schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>:      
>>>>>>>>> If the interface clock is not below a ti,clksel then we have reg.
>>>>>>>>>        
>>>>>>>>
>>>>>>>> This should be expressed in the bindings. It's fine to make the reg
>>>>>>>> optional (skip the description, it's confusing), but the ti,clksel
>>>>>>>> should reference this schema and enforce it on the children.
>>>>>>>>      
>>>>>>> Well there are other compatibles below ti,clksel, too, so should we
>>>>>>> rather add them when the other .txt files are converted?      
>>>>>>
>>>>>> This binding should already be referenced by ti,clksel. When the other
>>>>>> are ready, you will change additionalProperties from object to false.
>>>>>>    
>>>>> I played around with it:
>>>>>
>>>>> --- a/Documentation/devicetree/bindings/clock/ti/ti,clksel.yaml
>>>>> +++ b/Documentation/devicetree/bindings/clock/ti/ti,clksel.yaml
>>>>> @@ -33,6 +33,11 @@ properties:
>>>>>      const: 2
>>>>>      description: The CLKSEL register and bit offset
>>>>>  
>>>>> +patternProperties:
>>>>> +  "-ick$":
>>>>> +    $ref: /schemas/clock/ti/ti,interface-clock.yaml#
>>>>> +    type: object
>>>>> +
>>>>>  required:
>>>>>    - compatible
>>>>>    - reg
>>>>>
>>>>>  
>>>>> That generates warnings, which look more serious than just a
>>>>> non-converted compatible, so lowering the overall "signal-noise-ratio".
>>>>>
>>>>> e.g.
>>>>> from schema $id:
>>>>> http://devicetree.org/schemas/clock/ti/ti,clksel.yaml#
>>>>> /home/andi/linux-dtbs/arch/arm/boot/dts/ti/omap/omap3-overo-tobiduo.dtb:
>>>>> clock@c40: clock-rm-ick: 'ti,index-starts-at-one', 'ti,max-div' do not
>>>>> match any of the regexes: 'pinctrl-[0-9]+'
>>>>>
>>>>> I think we should rather postpone such referencing.    
>>>>
>>>> Are you sure in such case that your binding is correct? The warnings
>>>> suggest that not, therefore please do not postpone.
>>>>  
>>> well, there is not only stuff from clock/ti/ti,interface.yaml but also from
>>> clock/ti/divider.txt below ti,clksel. So I have one warning about the missing
>>> compatible there and also about the properties belonging to that compatible.  
>>
>> Ah, you have other bindings for the "-ick" nodes? Then you cannot match
>> by pattern now, indeed. Maybe skipping ref but adding "compatible" into
>> node, like we do for Qualcomm mdss bindings, would work. But in general
>> all these should be converted at the same time.
>>
> Yes, there are other bindings for the "-ick" nodes. But these bindings
> are not exclusive to the "-ick" nodes. I personally would prefer not
> having to do the whole clock/ti/*.txt directory at once.

This is what usually is expected for multiple schemas used together
(common for MFD). Don't convert part of device but everything needed for
the main node. It's not different here.

But sure, you can go with mdss approach.

Best regards,
Krzysztof