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

Michal Simek
2022-10-03T10:41:49Z

Hi again,

On 10/3/22 11:59, Michal Simek wrote:
> Hi Greg,
>
> On 10/3/22 10:16, Greg Kroah-Hartman wrote:
>> On Mon, Oct 03, 2022 at 10:10:38AM +0200, 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?
>>
>> Maybe we should just delete the staging one (and the staging driver),
>> and start over?  No one has taken the time to get the staging driver out
>> of there, so I have no objection to dropping it for 6.1.
>
> As I said it is be out of staging in linux-next. When CLK tree is merged in
> these 2 weeks we are done at least with this driver.

FYI: Here is link where I asked you for your ACK to get the driver out of staging.
https://lore.kernel.org/all/Ys%2F%2FaPLkLGaooYYw@kroah.com/

Thanks,
Michal