Re: [PATCH 1/2] dt-bindings: clk: Add binding for versal clocking wizard

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

On 03/10/2022 09:58, Michal Simek wrote:
> 
> 
> On 10/3/22 09:23, Krzysztof Kozlowski wrote:
>> On 03/10/2022 09:15, Michal Simek wrote:
>>>>> And this is new IP. Not sure who has chosen similar name but this targets
>>>>> Xilinx Versal SOCs. Origin one was targeting previous families.
>>>>
>>>> Do we need a whole new schema doc?
>>>
>>> It is completely new IP with different logic compare to origin one.
>>>
>>>>
>>>> It is not ideal to define the same property, xlnx,nr-outputs, more than
>>>> once. And it's only a new compatible string.
>>>
>>> I can't see any issue with using dt binding for xlnx,clocking-wizard.yaml
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.yaml
>>
>> So we already have out of staging document:
>> devicetree/bindings/clock/xlnx,clocking-wizard.yaml
> 
> in 6.1 yes.
> 
>>
>> and author wants to add one more:
>> devicetree/bindings/clock/xlnx,clk-wizard.yaml
> 
> as I said it is completely different IP which requires complete different driver 
> but IP designers choose similar name which is out of developer control.
> 
>>
>> Shall we expect in two years, a third document like:
>> devicetree/bindings/clock/xlnx,clk-wzrd.yaml
>> ?
> 
> Developer definitely doesn't know. If new SoC requires for the same purpose 
> different IP with completely different driver is something out of developer 
> control. As of today I am not aware about such a requirement and need and 
> personally I can just hope that if they need to do such a change they will be 
> able to keep current SW driver compatible with new HW IP.

Then please start naming them reasonable, not two (and in future
x-times) the same names for entirely different blocks. And by name I
mean compatible, filename and device name.

>>> also for this IP if that's fine with you.
>>> Only xlnx,speed-grade can be defined for previous IP which is easy to mark.
>>
>> That old binding also explained nr-outputs as "Number of outputs".
>> Perfect... :(
> 
> Anyway if description should be improved let's just do it. I just want to get 
> guidance if we should update current dt binding for similar IP or just create 
> new one as this one is trying to do.

IMHO, new binding is extremely confusing. We already have support for
devices named "xlnx,clocking-wizard" and now you add exactly the same
(clk=clocking) with almost the same properties, named
"xlnx,clk-wizard-1.0". For a different IP?

How anyone (even Xilinx' customer) can understand which block is for
what if they have exactly the same name and (almost) the same
properties, but as you said - these are entirely different IP?

Best regards,
Krzysztof