public inbox for linux-clk@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Michal Simek <michal.simek@amd.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
	Rob Herring <robh@kernel.org>,
	Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>,
	linux-clk@vger.kernel.org, git@amd.com,
	devicetree@vger.kernel.org, krzysztof.kozlowski+dt@linaro.org,
	sboyd@kernel.org, mturquette@baylibre.com
Subject: Re: [PATCH 1/2] dt-bindings: clk: Add binding for versal clocking wizard
Date: Mon, 3 Oct 2022 16:50:25 +0200	[thread overview]
Message-ID: <Yzr2sbrt3uZwALnz@kroah.com> (raw)
In-Reply-To: <053c0831-6bce-ff0c-a511-6f292127ca63@amd.com>

On Mon, Oct 03, 2022 at 12:41:34PM +0200, Michal Simek wrote:
> 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/

Ah, good, I forgot about that, nevermind!

greg k-h

  reply	other threads:[~2022-10-03 14:50 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-30  8:03 [PATCH 0/2] clocking-wizard: Add versal clocking wizard support Shubhrajyoti Datta
2022-09-30  8:03 ` [PATCH 1/2] dt-bindings: clk: Add binding for versal clocking wizard Shubhrajyoti Datta
2022-09-30 11:19   ` Krzysztof Kozlowski
2022-09-30 12:25   ` Rob Herring
2022-09-30 13:00     ` Michal Simek
2022-09-30 21:39       ` Rob Herring
2022-10-03  7:15         ` Michal Simek
2022-10-03  7:23           ` Krzysztof Kozlowski
2022-10-03  7:58             ` Michal Simek
2022-10-03  8:10               ` Krzysztof Kozlowski
2022-10-03  8:16                 ` Greg Kroah-Hartman
2022-10-03  9:59                   ` Michal Simek
2022-10-03 10:41                     ` Michal Simek
2022-10-03 14:50                       ` Greg Kroah-Hartman [this message]
2022-10-03 10:37                 ` Michal Simek
2022-10-03 10:50                   ` Krzysztof Kozlowski
2022-10-03 15:27                     ` Michal Simek
2022-10-04 11:00                       ` Krzysztof Kozlowski
2022-10-04 11:04                         ` Michal Simek
2022-09-30  8:04 ` [PATCH 2/2] clocking-wizard: Add versal clocking wizard support Shubhrajyoti Datta

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Yzr2sbrt3uZwALnz@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=devicetree@vger.kernel.org \
    --cc=git@amd.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=krzysztof.kozlowski@linaro.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=michal.simek@amd.com \
    --cc=mturquette@baylibre.com \
    --cc=robh@kernel.org \
    --cc=sboyd@kernel.org \
    --cc=shubhrajyoti.datta@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox