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:
quoted
________________________________________
发件人: Wood Scott-B07421
发送时间: 2014年1月8日 8:21
收件人: Tang Yuantian-B29983
抄送: galak@kernel.crashing.org; mark.rutland@arm.com; devicetree@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:
quoted
+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
VGhhbmtzIGZvciB5b3UgcmV2aWV3Lg0KU2VlIG15IHJlc3BvbnNlIGlubGluZS4NCg0KVGhhbmtz
LA0KWXVhbnRpYW4NCg0KPiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBXb29k
IFNjb3R0LUIwNzQyMQ0KPiBTZW50OiAyMDE05bm0MeaciDnml6Ug5pif5pyf5ZubIDI6NDQNCj4g
VG86IE1hcmsgUnV0bGFuZA0KPiBDYzogVGFuZyBZdWFudGlhbi1CMjk5ODM7IGdhbGFrQGtlcm5l
bC5jcmFzaGluZy5vcmc7DQo+IGRldmljZXRyZWVAdmdlci5rZXJuZWwub3JnOyBsaW51eHBwYy1k
ZXZAbGlzdHMub3psYWJzLm9yZw0KPiBTdWJqZWN0OiBSZTog562U5aSNOiBbdjddIGNsazogY29y
ZW5ldDogQWRkcyB0aGUgY2xvY2sgYmluZGluZw0KPiANCj4gT24gV2VkLCAyMDE0LTAxLTA4IGF0
IDA5OjMwICswMDAwLCBNYXJrIFJ1dGxhbmQgd3JvdGU6DQo+ID4gT24gV2VkLCBKYW4gMDgsIDIw
MTQgYXQgMDg6NTM6NTZBTSArMDAwMCwgWXVhbnRpYW4gVGFuZyB3cm90ZToNCj4gPiA+DQo+ID4g
PiBfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fDQo+ID4gPiDlj5Hku7bk
uro6IFdvb2QgU2NvdHQtQjA3NDIxDQo+ID4gPiDlj5HpgIHml7bpl7Q6IDIwMTTlubQx5pyIOOaX
pSA4OjIxDQo+ID4gPiDmlLbku7bkuro6IFRhbmcgWXVhbnRpYW4tQjI5OTgzDQo+ID4gPiDmioTp
gIE6IGdhbGFrQGtlcm5lbC5jcmFzaGluZy5vcmc7IG1hcmsucnV0bGFuZEBhcm0uY29tOw0KPiA+
ID4gZGV2aWNldHJlZUB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4cHBjLWRldkBsaXN0cy5vemxhYnMu
b3JnDQo+ID4gPiDkuLvpopg6IFJlOiBbdjddIGNsazogY29yZW5ldDogQWRkcyB0aGUgY2xvY2sg
YmluZGluZw0KPiA+ID4NCj4gPiA+IE9uIFdlZCwgTm92IDIwLCAyMDEzIGF0IDA1OjA0OjQ5UE0g
KzA4MDAsIHRhbmcgeXVhbnRpYW4gd3JvdGU6DQo+ID4gPiA+ICtSZWNvbW1lbmRlZCBwcm9wZXJ0
aWVzOg0KPiA+ID4gPiArLSByYW5nZXM6IEFsbG93cyB2YWxpZCB0cmFuc2xhdGlvbiBiZXR3ZWVu
IGNoaWxkJ3MgYWRkcmVzcyBzcGFjZQ0KPiBhbmQNCj4gPiA+ID4gKyAgICAgcGFyZW50J3MuIE11
c3QgYmUgcHJlc2VudCBpZiB0aGUgZGV2aWNlIGhhcyBzdWItbm9kZXMuDQo+ID4gPiA+ICstICNh
ZGRyZXNzLWNlbGxzOiBTcGVjaWZpZXMgdGhlIG51bWJlciBvZiBjZWxscyB1c2VkIHRvIHJlcHJl
c2VudA0KPiA+ID4gPiArICAgICBwaHlzaWNhbCBiYXNlIGFkZHJlc3Nlcy4gIE11c3QgYmUgcHJl
c2VudCBpZiB0aGUgZGV2aWNlIGhhcw0KPiA+ID4gPiArICAgICBzdWItbm9kZXMgYW5kIHNldCB0
byAxIGlmIHByZXNlbnQNCj4gPiA+ID4gKy0gI3NpemUtY2VsbHM6IFNwZWNpZmllcyB0aGUgbnVt
YmVyIG9mIGNlbGxzIHVzZWQgdG8gcmVwcmVzZW50DQo+ID4gPiA+ICsgICAgIHRoZSBzaXplIG9m
IGFuIGFkZHJlc3MuIE11c3QgYmUgcHJlc2VudCBpZiB0aGUgZGV2aWNlIGhhcw0KPiA+ID4gPiAr
ICAgICBzdWItbm9kZXMgYW5kIHNldCB0byAxIGlmIHByZXNlbnQNCj4gPiA+DQo+ID4gPiBXaHkg
YXJlIHdlIHNwZWNpZnlpbmcgI2FkZHJlc3MtY2VsbHMvI3NpemUtY2VsbHMgaGVyZT8NCj4gPiA+
DQo+ID4gPiBBOiBpdCBoYXMgc3ViLW5vZGVzIHdoaWNoIGhhdmUgUkVHIHByb3BlcnR5LCBkb24n
dCB3ZSBuZWVkIHRvDQo+ID4gPiBzcGVjaWZ5ICNhZGRyZXNzLWNlbGxzLyNzaXplLWNlbGxzPw0K
PiA+DQo+ID4gSWYgYSBub2RlIGhhcyBhIHJlZyBlbnRyeSwgaXRzIHBhcmVudCBzaG91bGQgaGF2
ZSAjc2l6ZS1jZWxscyBhbmQNCj4gPiAjYWRkcmVzcy1jZWxscyB0byBhbGxvdyBpdCB0byBiZSBw
YXJzZWQgcHJvcGVybHkuDQo+IA0KPiBZZXMsIGJ1dCB3aHkgZG8gd2UgbmVlZCB0byBzcGVjaWZ5
IGluIHRoaXMgYmluZGluZyBob3cgbWFueSBjZWxscyB0aGVyZQ0KPiB3aWxsIGJlLCBlc3BlY2lh
bGx5IHNpbmNlIHRoaXMgYmluZGluZyBvbmx5IGFkZHJlc3NlcyB0aGUgY2xvY2sgcHJvdmlkZXIN
Cj4gYXNwZWN0IG9mIHRoZSBjbG9ja2dlbiBub2RlcyAoZS5nLiBpdCBkb2Vzbid0IGRlc2NyaWJl
IHRoZSByZWcpPyAgT3INCj4gcmF0aGVyLCBpdCdzIHBhcnRpYWxseSBkZXNjcmliaW5nIHRoZSBu
b24tY2xvY2sgYXNwZWN0LCBhbmQgZG9lc24ndA0KPiBhZGRyZXNzIHRoZSBjbG9jayBhc3BlY3Qg
YXQgYWxsIEFGQUlDVC4NCj4gDQpGaXJzdCBvZiBhbGwsIHRoZXkgYXJlIG5vdCAiUmVxdWlyZWQg
cHJvcGVydGllcyIsIHRoZXkgYXJlIG9wdGlvbmFsLg0KSWYgcHJlc2VudCwgd2Ugc2hvdWxkIGdp
dmUgdGhlbSBhIHZhbHVlIG9mIDEuDQpUaGVuLCB5ZXMsIHRoaXMgYmluZGluZyBkZXNjcmliZXMg
Y2xvY2tnZW4gbm9kZSB3aGljaCBpcyAiQ0xPQ0sgQkxPQ0siLg0KSXQgc2hvdWxkIHRha2UgY2Fy
ZSBvZiBpdHMgc3ViLW5vZGVzIHdoaWNoIGFyZSBjbG9jayBub2RlcyB0byBiZSBwYXJzZWQgcHJv
cGVybHkuDQoNCj4gV2hlcmUgZG9lcyB0aGUgYWN0dWFsIGlucHV0IGNsb2NrIGZyZXF1ZW5jeSBn
bz8gIFUtQm9vdCBwdXRzIGl0IGluIHRoZQ0KPiBjbG9ja2dlbiBub2RlIGl0c2VsZiBhcyBjbG9j
ay1mcmVxdWVuY3ksIGJ1dCB0aGF0IGlzbid0IGRlc2NyaWJlZCBpbiB0aGUNCj4gYmluZGluZy4g
IEhvdyBkb2VzIHRoYXQgcmVsYXRlIHRvIHRoZSBzeXNjbGsgbm9kZT8gIElmICJmc2wscW9yaXEt
c3lzY2xrLQ0KPiAxLjAiIGlzIHN1cHBvc2VkIHRvIGluZGljYXRlIHRoYXQgY2xvY2stZnJlcXVl
bmN5IGNhbiBiZSBmb3VuZCBpbiB0aGUNCj4gcGFyZW50IG5vZGUsIHRoYXQgaXNuJ3Qgc3BlY2lm
aWVkIGJ5IHRoZSBiaW5kaW5nLCBub3IgaXMgY2xvY2stZnJlcXVlbmN5DQo+IHNob3duIGluIHRo
ZSBleGFtcGxlLg0KPiANCmNsb2NrLWZyZXF1ZW5jeSBpcyBhIHdpcmVkIHByb3BlcnR5LiBJdCBp
cyBpbiBjbG9ja2dlbiBub2RlIHJpZ2h0IG5vdy4NCkJ1dCBpdCBzaG91bGQgYmUgcGxhY2VkIHNv
bWV3aGVyZSBpbiBjbG9jayBub2Rlcy4NCklmIEkgZGVzY3JpYmUgaGVyZSwgSSB3b3VsZCBiZSBh
c2tlZCB3aHkgaXQgaXMgcmVsYXRlZCB0byBjbG9ja2dlbiBub2RlPw0KSWYgeW91IHRoaW5rIHNo
b3dpbmcgaXQgdXAgaXMgT0ssIEkgbGlrZSB0byBkbyBpdC4NCiANCj4gV2hhdCBpcyB0aGUgZGlm
ZmVyZW5jZSBiZXR3ZWVuICJmc2wscW9yaXEtc3lzY2xrLTEuMCIgYW5kICJmc2wscW9yaXEtDQo+
IHN5c2Nsay0yLjAiPyAgSG93IGRvZXMgdGhlIGNvbmNlcHQgb2YgYSBmaXhlZCBpbnB1dCBjbG9j
ayBjaGFuZ2U/DQo+DQpUZWNobmljYWxseSwgdGhlcmUgaXMgbm8gZGlmZmVyZW5jZSBiZXR3ZWVu
ICpzeXNjbGstMS4wIGFuZCAqLTIuMCwganVzdCBsaWtlDQpDbG9ja2dlbi0yLjAgYW5kIDEuMC4g
TmFtaW5nIGxpa2UgdGhpcyBqdXN0IHRvIGluZGljYXRlIHRoZXkgYmVsb25nIHRvIGNoYXNzaXMg
Mi4wIA0KYW5kIDEuMCByZXNwZWN0aXZlbHkuDQoNClJlZ2FyZHMsDQpZdWFudGlhbg0KIA0KPiAt
U2NvdHQNCj4gDQoNCg==
On Wed, 2014-01-08 at 20:57 -0600, Tang Yuantian-B29983 wrote:
Thanks for you review.
See my response inline.
Thanks,
Yuantian
quoted
-----Original Message-----
From: Wood Scott-B07421
Sent: 2014年1月9日 星期四 2:44
To: Mark Rutland
Cc: Tang Yuantian-B29983; galak@kernel.crashing.org;
devicetree@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 +0000, Mark Rutland wrote:
quoted
On Wed, Jan 08, 2014 at 08:53:56AM +0000, Yuantian Tang wrote:
quoted
________________________________________
发件人: Wood Scott-B07421
发送时间: 2014年1月8日 8:21
收件人: Tang Yuantian-B29983
抄送: galak@kernel.crashing.org; mark.rutland@arm.com;
devicetree@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:
quoted
+Recommended properties:
+- ranges: Allows valid translation between child's address space
and
quoted
quoted
quoted
+ 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.
quoted
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.
quoted
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