linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vineet Gupta <Vineet.Gupta1@synopsys.com>
To: Jose Abreu <Jose.Abreu@synopsys.com>,
	Stephen Boyd <sboyd@codeaurora.org>
Cc: <linux-kernel@vger.kernel.org>, <linux-clk@vger.kernel.org>,
	<linux-snps-arc@lists.infradead.org>,
	<CARLOS.PALMINHA@synopsys.com>, <Alexey.Brodkin@synopsys.com>,
	<mturquette@baylibre.com>
Subject: Re: [RESEND PATCH v4] clk/axs10x: Add I2S PLL clock driver
Date: Mon, 18 Apr 2016 17:19:14 +0530	[thread overview]
Message-ID: <5714C9BA.8040004@synopsys.com> (raw)
In-Reply-To: <5714B763.8010109@synopsys.com>

On Monday 18 April 2016 04:00 PM, Jose Abreu wrote:
>>> +	if (readl((void *)FPGA_VER_INFO) <= FPGA_VER_27M) {
>> > Please don't readl directly from addresses. I think I mentioned
>> > that before and didn't get back to you when you replied asking
>> > for other solutions. I still think a proper DT is in order
>> > instead of doing this check for ref_clk.
> I think that the DT approach would be better but I also think that using two DT
> files with only one change between them is not viable. I can see some alternatives:
>     1) Pass the region of FPGA version in reg field of DT so that writel is not
> directly used;
>     2) Create a dummy parent clock driver that reads from FPGA version register
> and returns the rate;
>     3) Last resort: Use two DT files for each FPGA version.
> 
> @Vineet, @Alexey: Can you give some suggestions?
> 
> Some background:
> We are expecting a new firmware release for the AXS board that will change the
> reference clock value of the I2S PLL from 27MHz to 28.224MHz. Due to this change
> the dividers of this PLL will change. Right now I am directly reading from the
> FPGA version register but Stephen suggested to use a DT approach so that this
> rate is declared as parent clock. This would be a good solution but would
> require the usage of two different DT files (one for the current firmware and
> another for the new firmware), which I think is not ideal. What is your opinion?
> Some other solutions are listed above.

Consider this my ignorance of clk drivers, what exactly is the problem with that
readl() for FPGA ver. Having 2 versions of DT is annoyance for sure, but the
bigger headache is that it still won't help cases of users mixing and matching
boards and DT. IMO this runtime check is pretty nice and will support both types
of boards with exact same code/DT !

FWIW, both solutions #1 and #3 seem to imply a different DT - no ?

And I really don't see how #2 makes things more elegant/abstracted w.r.t clk
framework ?

So I prefer what you had before.
-Vineet

  reply	other threads:[~2016-04-18 11:49 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-11 10:41 [RESEND PATCH v4] clk/axs10x: Add I2S PLL clock driver Jose Abreu
2016-04-11 16:47 ` Alexey Brodkin
2016-04-11 22:03   ` sboyd
2016-04-15 12:08     ` Alexey Brodkin
2016-04-15 23:38       ` sboyd
2016-04-15 23:46 ` Stephen Boyd
2016-04-18 10:30   ` Jose Abreu
2016-04-18 11:49     ` Vineet Gupta [this message]
2016-04-19  9:13       ` Jose Abreu
2016-04-20  1:54         ` Stephen Boyd
2016-04-20  9:47           ` Jose Abreu
2016-04-20 16:12             ` Alexey Brodkin
2016-04-21  9:51               ` Jose Abreu
2016-04-21 12:18                 ` Alexey Brodkin
2016-04-21 13:10                   ` Jose Abreu
2016-04-21 14:15                     ` Alexey Brodkin
2016-04-22  5:50                   ` Vineet Gupta

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=5714C9BA.8040004@synopsys.com \
    --to=vineet.gupta1@synopsys.com \
    --cc=Alexey.Brodkin@synopsys.com \
    --cc=CARLOS.PALMINHA@synopsys.com \
    --cc=Jose.Abreu@synopsys.com \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-snps-arc@lists.infradead.org \
    --cc=mturquette@baylibre.com \
    --cc=sboyd@codeaurora.org \
    /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;
as well as URLs for NNTP newsgroup(s).