RE: 答复: [v7] clk: corenet: Adds the clock binding

[Yuantian Tang]

Thanks for you review.
See my response inline.

Thanks,
Yuantian

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: 2014年1月9日 星期四 2:44
> To: Mark Rutland
> Cc: Tang Yuantian-B29983; galak at kernel.crashing.org;
> devicetree at vger.kernel.org; linuxppc-dev at lists.ozlabs.org
> Subject: Re: 答复: [v7] clk: corenet: Adds the clock binding
> 
> On Wed, 2014-01-08 at 09:30 +0000, Mark Rutland wrote:
> > On Wed, Jan 08, 2014 at 08:53:56AM +0000, Yuantian Tang wrote:
> > >
> > > ________________________________________
> > > 发件人: Wood Scott-B07421
> > > 发送时间: 2014年1月8日 8:21
> > > 收件人: Tang Yuantian-B29983
> > > 抄送: galak at kernel.crashing.org; mark.rutland at arm.com;
> > > devicetree at vger.kernel.org; linuxppc-dev at lists.ozlabs.org
> > > 主题: Re: [v7] clk: corenet: Adds the clock binding
> > >
> > > On Wed, Nov 20, 2013 at 05:04:49PM +0800, tang yuantian wrote:
> > > > +Recommended properties:
> > > > +- ranges: Allows valid translation between child's address space
> and
> > > > +     parent's. Must be present if the device has sub-nodes.
> > > > +- #address-cells: Specifies the number of cells used to represent
> > > > +     physical base addresses.  Must be present if the device has
> > > > +     sub-nodes and set to 1 if present
> > > > +- #size-cells: Specifies the number of cells used to represent
> > > > +     the size of an address. Must be present if the device has
> > > > +     sub-nodes and set to 1 if present
> > >
> > > Why are we specifying #address-cells/#size-cells here?
> > >
> > > A: it has sub-nodes which have REG property, don't we need to
> > > specify #address-cells/#size-cells?
> >
> > If a node has a reg entry, its parent should have #size-cells and
> > #address-cells to allow it to be parsed properly.
> 
> Yes, but why do we need to specify in this binding how many cells there
> will be, especially since this binding only addresses the clock provider
> aspect of the clockgen nodes (e.g. it doesn't describe the reg)?  Or
> rather, it's partially describing the non-clock aspect, and doesn't
> address the clock aspect at all AFAICT.
> 
First of all, they are not "Required properties", they are optional.
If present, we should give them a value of 1.
Then, yes, this binding describes clockgen node which is "CLOCK BLOCK".
It should take care of its sub-nodes which are clock nodes to be parsed properly.

> Where does the actual input clock frequency go?  U-Boot puts it in the
> clockgen node itself as clock-frequency, but that isn't described in the
> binding.  How does that relate to the sysclk node?  If "fsl,qoriq-sysclk-
> 1.0" is supposed to indicate that clock-frequency can be found in the
> parent node, that isn't specified by the binding, nor is clock-frequency
> shown in the example.
> 
clock-frequency is a wired property. It is in clockgen node right now.
But it should be placed somewhere in clock nodes.
If I describe here, I would be asked why it is related to clockgen node?
If you think showing it up is OK, I like to do it.
 
> What is the difference between "fsl,qoriq-sysclk-1.0" and "fsl,qoriq-
> sysclk-2.0"?  How does the concept of a fixed input clock change?
>
Technically, there is no difference between *sysclk-1.0 and *-2.0, just like
Clockgen-2.0 and 1.0. Naming like this just to indicate they belong to chassis 2.0 
and 1.0 respectively.

Regards,
Yuantian
 
> -Scott
>