| Message ID | 20220930080400.15619-1-shubhrajyoti.datta@amd.com |
|---|---|
| Headers | show |
| Series | clocking-wizard: Add versal clocking wizard support | expand |
On Fri, Sep 30, 2022 at 03:00:28PM +0200, Michal Simek wrote: > Hi Rob, > > On 9/30/22 14:25, Rob Herring wrote: > > On Fri, Sep 30, 2022 at 3:04 AM Shubhrajyoti Datta > > <shubhrajyoti.datta@amd.com> wrote: > > > > > > The Clocking Wizard for Versal adaptive compute acceleration platforms > > > generates multiple configurable number of clock outputs. > > > Add device tree binding for Versal clocking wizard support. > > > > Really v1? I'm sure I heard of this wizard before. > > > > What about this?: > > > > drivers/staging/clocking-wizard/dt-binding.txt > > > > That needs to be moved out of staging rather than adding a 2nd one. > > > Let me clarify this. This is IP which is already moved out of staging. > Linux-next has these changes and waiting for MW to happen (already in clock > tree). Where does this series explain that? If the dependency is not the latest rc1, then state that. > 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 not ideal to define the same property, xlnx,nr-outputs, more than once. And it's only a new compatible string. Rob
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
On 10/4/22 13:00, Krzysztof Kozlowski wrote: > On 03/10/2022 17:27, Michal Simek wrote: >>> >>> 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. > > Then maybe the compatibles (and device names) should have that fpga/pl > technology used to differentiate between them? I am already trying to find out better generic description without mentioning sizes. >> 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? > > Yes, although we might misunderstand what PL technology is. 28/16/7 nm > is the size of transistor or the process. Even two different processes > can use same type of technology, e.g. FinFET: > https://en.wikipedia.org/wiki/14_nm_process > https://en.wikipedia.org/wiki/10_nm_process > > You could have very similar (or even the same) designs done in 28 nm and > 16 nm. Does it mean these are entirely different devices? Not > necessarily... Maybe they are, maybe not, but is the size of process > differentiating? I actually don't know what's there in 28/16/7, I am > just saying that number alone might not mean different technology. > Programming API could be the same, inputs/outputs could be the same. > Just the size of transistor is different... I agree. Will try to come up with better name without nm inside to uniquely identify PL logic type. Thanks, Michal