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

2014-01-09 Thread Yuantian Tang
Thanks for your review. I will send next version of patch.

Thanks,
Yuantian


> -Original Message-
> From: Wood Scott-B07421
> Sent: 2014年1月10日 星期五 5:19
> To: Tang Yuantian-B29983
> Cc: Mark Rutland; ga...@kernel.crashing.org; devicet...@vger.kernel.org;
> linuxppc-dev@lists.ozlabs.org
> Subject: Re: 答复: [v7] clk: corenet: Adds the clock binding
> 
> On Wed, 2014-01-08 at 20:57 -0600, Tang Yuantian-B29983 wrote:
> > 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; ga...@kernel.crashing.org;
> > > devicet...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
> > > Subject: Re: 答复: [v7] clk: corenet: Adds the clock binding
> > >
> > > On Wed, 2014-01-08 at 09:30 +, Mark Rutland wrote:
> > > > On Wed, Jan 08, 2014 at 08:53:56AM +, Yuantian Tang wrote:
> > > > >
> > > > > 
> > > > > 发件人: Wood Scott-B07421
> > > > > 发送时间: 2014年1月8日 8:21
> > > > > 收件人: Tang Yuantian-B29983
> > > > > 抄送: ga...@kernel.crashing.org; mark.rutl...@arm.com;
> > > > > devicet...@vger.kernel.org; linuxppc-dev@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.
> 
> Why does it matter, so long as the values translate properly?  It's not
> as if you're defining a special reg format.  It's not that big of a deal,
> but it seems unnecessary.
> 
> > Then, yes, this binding describes clockgen node which is "CLOCK BLOCK".
> 
> Sorry, I missed where "reg" was documented due to the
> required/recommended split.
> 
> > > 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.
> 
> Do you mean "weird"?
> 
> > It is in clockgen node right now.
> > But it should be placed somewhere in clock nodes.
> 
> If we were doing this from scratch, yes, but there's existing U-Boot code
> that we want to be compatible with.
> 
> > If I describe here, I would be asked why it is related to clockgen node?
> 
> That's not a good reason to leave it undocumented.
> 
> > > 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.
> 
> I guess it's OK if it encourages people to consider switching to the
> standard fixed-clock for future chassis.
> 
> So the only thing that really needs to be fixed is the missing clock-
> frequency documentation.
> 
> -Scott
> 

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

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

2014-01-09 Thread Scott Wood
On Wed, 2014-01-08 at 20:57 -0600, Tang Yuantian-B29983 wrote:
> 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; ga...@kernel.crashing.org;
> > devicet...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
> > Subject: Re: 答复: [v7] clk: corenet: Adds the clock binding
> > 
> > On Wed, 2014-01-08 at 09:30 +, Mark Rutland wrote:
> > > On Wed, Jan 08, 2014 at 08:53:56AM +, Yuantian Tang wrote:
> > > >
> > > > 
> > > > 发件人: Wood Scott-B07421
> > > > 发送时间: 2014年1月8日 8:21
> > > > 收件人: Tang Yuantian-B29983
> > > > 抄送: ga...@kernel.crashing.org; mark.rutl...@arm.com;
> > > > devicet...@vger.kernel.org; linuxppc-dev@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.

Why does it matter, so long as the values translate properly?  It's not
as if you're defining a special reg format.  It's not that big of a
deal, but it seems unnecessary.

> Then, yes, this binding describes clockgen node which is "CLOCK BLOCK".

Sorry, I missed where "reg" was documented due to the
required/recommended split.

> > 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.

Do you mean "weird"?

> It is in clockgen node right now.
> But it should be placed somewhere in clock nodes.

If we were doing this from scratch, yes, but there's existing U-Boot
code that we want to be compatible with.

> If I describe here, I would be asked why it is related to clockgen node?

That's not a good reason to leave it undocumented.

> > 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.

I guess it's OK if it encourages people to consider switching to the
standard fixed-clock for future chassis.

So the only thing that really needs to be fixed is the missing
clock-frequency documentation.

-Scott


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

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

2014-01-08 Thread 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; ga...@kernel.crashing.org;
> devicet...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
> Subject: Re: 答复: [v7] clk: corenet: Adds the clock binding
> 
> On Wed, 2014-01-08 at 09:30 +, Mark Rutland wrote:
> > On Wed, Jan 08, 2014 at 08:53:56AM +, Yuantian Tang wrote:
> > >
> > > 
> > > 发件人: Wood Scott-B07421
> > > 发送时间: 2014年1月8日 8:21
> > > 收件人: Tang Yuantian-B29983
> > > 抄送: ga...@kernel.crashing.org; mark.rutl...@arm.com;
> > > devicet...@vger.kernel.org; linuxppc-dev@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
> 

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

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

2014-01-08 Thread Scott Wood
On Wed, 2014-01-08 at 09:30 +, Mark Rutland wrote:
> On Wed, Jan 08, 2014 at 08:53:56AM +, Yuantian Tang wrote:
> > 
> > 
> > 发件人: Wood Scott-B07421
> > 发送时间: 2014年1月8日 8:21
> > 收件人: Tang Yuantian-B29983
> > 抄送: ga...@kernel.crashing.org; mark.rutl...@arm.com; 
> > devicet...@vger.kernel.org; linuxppc-dev@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.

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.

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?

-Scott


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

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

2014-01-08 Thread Mark Rutland
On Wed, Jan 08, 2014 at 08:53:56AM +, Yuantian Tang wrote:
> 
> 
> 发件人: Wood Scott-B07421
> 发送时间: 2014年1月8日 8:21
> 收件人: Tang Yuantian-B29983
> 抄送: ga...@kernel.crashing.org; mark.rutl...@arm.com; 
> devicet...@vger.kernel.org; linuxppc-dev@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.

Mark.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev