Re: [PATCH v3 1/2] dt-bindindgs: clock: support NXP i.MX95 BLK CTL module

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

On 05/03/2024 08:29, Krzysztof Kozlowski wrote:
> On 05/03/2024 08:18, Peng Fan wrote:
>>> Subject: Re: [PATCH v3 1/2] dt-bindindgs: clock: support NXP i.MX95 BLK CTL
>>> module
>>>
>>> On 05/03/2024 05:13, Peng Fan wrote:
>>>>>> +
>>>>>> +examples:
>>>>>> +  # Clock Control Module node:
>>>>>> +  - |
>>>>>> +    #include <dt-bindings/clock/nxp,imx95-clock.h>
>>>>>> +
>>>>>> +    syscon@4c410000 {
>>>>>
>>>>> clock-controller@...
>>>>
>>>> But this is a syscon, using clock-controller will trigger dt check
>>>> warning.
>>>
>>> Which warning?
>>
>> I just recalled that node with syscon in compatible string needs
>> has syscon as node, I maybe wrong.
> 
> Just paste the warning, so we can think about it.
> 
>>
>>>
>>>>>
>>>>> As that is the main feature/function.
>>>>>
>>>>>> +      compatible = "nxp,imx95-vpumix-csr", "syscon";
>>>>>> +      reg = <0x4c410000 0x10000>;
>>>>>> +      #clock-cells = <1>;
>>>>>
>>>>> Please make the example as full as possible. For example, add
>>>>> mux-controller node. Do some of the blocks not have mux ctrl?
>>>>
>>>> Yes. The blk ctrl is not just for clock, some registers has mux ctrl,
>>>> such as Pixel_link_sel.
>>>
>>> Then mux-controller should not be allowed for them.
>>
>> You mean I should not add mux-controller under the blk ctrl node?
> 
> mux-controller is already there, isn't it? I am saying your binding is
> not precise. Your binding implies that ALL OF THEM have mux controller.
> You told me it is not true, so you have change the meaning of binding
> and disallow the mux-controller for the cases it is not applicable.
> 

... or create separate binding/schema for the variants with mux-controller.

Best regards,
Krzysztof