public inbox for linux-snps-arc@lists.infradead.org
 help / color / mirror / Atom feed
From: Alexey.Brodkin@synopsys.com (Alexey Brodkin)
To: linux-snps-arc@lists.infradead.org
Subject: [RESEND PATCH v4] clk/axs10x: Add I2S PLL clock driver
Date: Wed, 20 Apr 2016 16:12:50 +0000	[thread overview]
Message-ID: <1461168765.3149.31.camel@synopsys.com> (raw)
In-Reply-To: <5717503F.7020806@synopsys.com>

Hi Jose, Stephen,

On Wed, 2016-04-20@10:47 +0100, Jose Abreu wrote:
> Hi Stephen,
> 
> 
> On 20-04-2016 02:54, Stephen Boyd wrote:
> > 
> > On 04/19, Jose Abreu wrote:
> > > 
> > > @Stephen: can you give some input so that I can submit a v6?
> > > 
> > I don't prefer putting the second register in the same DT node,
> > but that's really up to the DT reviewers to approve such a
> > design. The current binding has been acked by Rob right?
> Yes.
> 
> > 
> > Assuming the new binding is acked/reviewed then that solution is
> > fine.
> Ok, will then use the DT to pass the FPGA version register.

We won't need to know FPGA version at all I think.
Read my comment below.

> > 
> > Otherwise, I still prefer two DTS files for the two different FPGA
> > versions. At the least, please use ioremap for any pointers that
> > you readl/writel here.
> > 
> > Beyond that, we should have a fixed rate source clk somewhere in
> > the software view of the clk tree, because that reflects reality.
> > Hardcoding the parent rate in the structure works, but doesn't
> > properly express the clk tree.
> > 
> Can I use a property in the DT to pass this reference clock? something like this:
> ????snps,parent-freq = <0xFBED9 27000000>, <0x0 28224000>; /* Tuple
> <fpga-version reference-clock-freq>, fpga-version = 0 is default */
> 
> Or use a parent clock? like:
> ????clk {
> ????????compatible = "fixed-clock";
> ????????clock-frequency = <27000000>;
> ????????#clock-cells = <0>;
> ????????snps,fpga-version = <0xFBED9>;
> ????}
> 
> It is important to distinguish between the different versions automatically, is
> any of these solutions ok?

I do like that solution with a master clock but with some fine-tuning
for simplification.

We'll add master clock node for I2S as a fixed clock like that:
------------------->8------------------
	i2s_master_clock: clk {
		#clock-cells = <0>;
		compatible = "fixed-clock";
		clock-frequency = <27000000>;
	};
------------------->8------------------

Note there's no mention of MB version, just a value of the frequency.
And in the driver itself value of that master clock will be used for
population of "pll_clk->ref_clk" directly.

These are benefits we'll get with that approach:
?[1] We escape any IOs not related to our clock device (I mean
? ? ?"snps,i2s-pll-clock") itself.
?[2] We'll use whatever reference clock value is given.
? ? ?I.e. we'll be able to do a fix-up of that reference clock
? ? ?value early in platform code depending on HW we're running on.
? ? ?That's what people do here and there.
?[3] Remember another clock driver for AXS10x board is right around
? ? ?the corner. I mean the one for ARC PGU which uses exactly the same
? ? ?master clock. So one fixup as mentioned above will work
? ? ?at once for 2 clock drivers.

Let me know if above makes sense.

-Alexey

  reply	other threads:[~2016-04-20 16:12 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-04 18:00 [PATCH v4] clk/axs10x: Add I2S PLL clock driver Jose Abreu
2016-04-11 16:47 ` [RESEND PATCH " 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
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 [this message]
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=1461168765.3149.31.camel@synopsys.com \
    --to=alexey.brodkin@synopsys.com \
    --cc=linux-snps-arc@lists.infradead.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