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

Greg Kroah-Hartman
2022-10-03T08:42:01Z

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.

thanks,

greg k-h