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

[Michal Simek <michal.simek@amd.com>]

On 10/3/22 10:10, Krzysztof Kozlowski wrote:
> 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?

Let me start from last one. Xilinx has IP catalog in design tools called Vivado. 
You choose device you have and then you will find clk wizard and you get an IP.
It means depends on SOC you have ZynqMP or Versal and based on that one or 
another is taken.
And because this is fpga world none is really describing programmable logic by 
hand because it would take a look a lot of time. That's why I created long time 
ago device-tree generator (DTG) which gets design data and based on it generate 
device tree description. Newest version is available for example here.
https://github.com/Xilinx/device-tree-xlnx
There is also newer version called system device tree generato
https://github.com/Xilinx/system-device-tree-xlnx

Because of this infrastructure user will all the time get proper compatible 
string which is aligned with IP catalog.

If you think that we should use the name which is more closer to ZynqMP clocking 
wizard driver we can try to take a look at it.
We can also update description in dt binding to explain this better.

I understand that it is confusing but we just got this name from IP catalog 
which is created by HW guys and we won't be able to change it. We can't also 
enforce that they will rename existing IP.
But we can use different compatible string and wire it up in DTG that we will 
start to generate.
We tend to use IP name and also IP version for compatible string generation 
because HW guys are IP owner and when any change happen they should bump IP 
version.

Thanks,
Michal