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; galak@xxxxxxxxxxxxxxxxxxx; > > devicetree@xxxxxxxxxxxxxxx; linuxppc-dev@xxxxxxxxxxxxxxxx > > 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@xxxxxxxxxxxxxxxxxxx; mark.rutland@xxxxxxx; > > > > devicetree@xxxxxxxxxxxxxxx; linuxppc-dev@xxxxxxxxxxxxxxxx > > > > 主题: 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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
