lkml.org 
[lkml]   [2016]   [Apr]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [RESEND PATCH v4] clk/axs10x: Add I2S PLL clock driver
Date
Hi Jose, Stephen,

On Wed, 2016-04-20 at 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

\
 
 \ /
  Last update: 2016-04-20 18:41    [W:0.076 / U:0.700 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site