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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 10/3/22 12:50, Krzysztof Kozlowski wrote:
On 03/10/2022 12:37, Michal Simek wrote:


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.

So you have a specific device? Why it is not part of name and compatible?

It means depends on SOC you have ZynqMP or Versal and based on that one or
another is taken.

Exactly. The names xlnx,clocking-wizard and xlnx,clk-wizard-1.0 are
therefore not specific enough and mixing different devices.

And just to be clear these IPs can be combined with systems where the main cpu can be Microblaze. I have also seen some vendors mixing RISC-V with Xilinx IPs.

Please look below.

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.

I don't think so. Let's skip for now "clk" and "clocking" differences
and assume both are "clocking". You have then compatibles:

xlnx,clocking-wizard and xlnx,clocking-wizard-1.0

and you said these are entirely different blocks.

There is no way this creates readable DTS.

And I really thank you for this discussion to do it properly and have proper compatible string and description for this block.

Shubhrajyoti: please correct me if I am wrong.

All Xilinx SOCs have programmable logic aligned with FPGAs. Zynq is based 28nm,
ZynqMP (Ultrascale MPSOC) is based on 16nm and Versal is based on 7nm.

I think these clocking IPs are using low level primitives available in PL logic.
Which means there is connection to fpga/pl technology instead of SOC family and main cpu. It can be of course said that if this is ZynqMP SOC that IP A is used. The same for Versal SOC. But for soft cores this can't be said.

Would it be better to reflect PL technology in compatible string?

xlnx,clocking-wizard-vX.X (used now for ZynqMP and previous families)
xlnx,clocking-wizard-7nm-vX.X (or find better name names which reflect PL logic type)

Thanks,
Michal



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux