| Message ID | 046d990092dc85cf50db9082fc894d93a3ece7c0.1446018918.git.p.fedin@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 28.10.2015 16:57, Pavel Fedin wrote: > Add documentation for new properties, allowing bank configuration. > > Signed-off-by: Pavel Fedin <p.fedin@samsung.com> > --- > .../bindings/arm/samsung/exynos-srom.txt | 24 +++++++++++++++++++++- > 1 file changed, 23 insertions(+), 1 deletion(-) You missed here a lot of potential Cc. Please use get_maintainer.pl script. It *must* be sent to devicetree list. Please send it also to DeviceTree maintainers because you are adding quite generic names for bindings so they may have interesting thoughts on this. LKML list is also missing. > > diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt b/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt > index 33886d5..9e4a40b 100644 > --- a/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt > +++ b/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt > @@ -5,8 +5,30 @@ Required properties: > > - reg: offset and length of the register set > > -Example: > +Bank configurations can be defined in optional subnodes. Each subnode > +describes one bank and contains the following: > + > +Required properties: > +- bank : bank number (0 - 3) I wonder whether this should be maybe "reg"? > + > +- srom-timing : array of 7 integers: Tacp, Tcah, Tcoh, Tacc, Tcos, Tacs Missing "pmc"? Where is the seventh? This looks vendor specific, so prefix with "samsung,exynos-". > + > +Optional properties: > +- width : data width in bytes (1 or 2). If omitted, default of 1 is used. This is not bank-width? So data-width? Any vendor prefix here? How generic is this? These are first things which came to my mind. Maybe DT maintainers will come with something more... Best regards, Krzysztof > + > +Example: basic definition, no banks are configured > + sromc@12570000 { > + compatible = "samsung,exynos-srom"; > + reg = <0x12570000 0x10>; > + }; > + > +Example: SROMc with bank3 configuration > sromc@12570000 { > compatible = "samsung,exynos-srom"; > reg = <0x12570000 0x10>; > + bank@3 { > + bank = <3>; > + width = <2>; > + srom-timing = <1 9 12 1 9 1 1>; > + }; > }; >
Hello! > You missed here a lot of potential Cc. Please use get_maintainer.pl > script. It *must* be sent to devicetree list. > > Please send it also to DeviceTree maintainers because you are adding > quite generic names for bindings so they may have interesting thoughts > on this. > > LKML list is also missing. Ok. Next version will go there too. > Any vendor prefix here? How generic is this? I just don't know... Does *everything* really need a vendor prefix? How readable would that be? "compatible" property already says that it's samsung-exynos-specific. And IMHO it's quite obvious that properties of vendor-specific device are automatically vendor-specific. Ok, i am currently fixing up the rest and will post v4 soon, and will Cc: it to devicetree ML. P.S. I intentionally omit the rest of the text in order to avoid overquoting, since we are already focused on some specific things. If someone wants to get the whole picture, he/she could just browse back to the start of the thread. Or is it considered wrong approach in this particular ML? Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia
2015-10-29 16:45 GMT+09:00 Pavel Fedin <p.fedin@samsung.com>: > Hello! > >> You missed here a lot of potential Cc. Please use get_maintainer.pl >> script. It *must* be sent to devicetree list. >> >> Please send it also to DeviceTree maintainers because you are adding >> quite generic names for bindings so they may have interesting thoughts >> on this. >> >> LKML list is also missing. > > Ok. Next version will go there too. > >> Any vendor prefix here? How generic is this? > > I just don't know... Does *everything* really need a vendor prefix? How readable would that be? "compatible" property already says > that it's samsung-exynos-specific. And IMHO it's quite obvious that properties of vendor-specific device are automatically > vendor-specific. > Ok, i am currently fixing up the rest and will post v4 soon, and will Cc: it to devicetree ML. Which my reply you are referring to? You stripped part of some sentence and put it without *any context*. Just random sentence. I asked for vendor prefix in few places... srom-timing? width? And I do not remember where I used exactly these words. This is why *context* is needed. > > P.S. I intentionally omit the rest of the text in order to avoid overquoting, since we are already focused on some specific things. > If someone wants to get the whole picture, he/she could just browse back to the start of the thread. Or is it considered wrong > approach in this particular ML? Yep, selecting some sentences and replying only to them, leaving the context away, is highly confusing. I reply in different places, sometimes asking for similar things. Removing context messes this up. In the same time all of my other questions are being removed without any acknowledgment so I do not know if they were ignored or just silently acked. I show you example: > I just don't know... I don't know neither. > If someone wants to get the whole picture, he/she could just browse back to the start of the thread. Yes, I would just scroll up and see the context. No, wait. There is no context. I have to find mine original email and compare which part was ignored, which was applied and which quoted sentence you are referring to. Best regards, Krzysztof
Hello! > >> Any vendor prefix here? How generic is this? > > > > I just don't know... Does *everything* really need a vendor prefix? How readable would that > be? "compatible" property already says > > that it's samsung-exynos-specific. And IMHO it's quite obvious that properties of vendor- > specific device are automatically > > vendor-specific. > > Ok, i am currently fixing up the rest and will post v4 soon, and will Cc: it to devicetree > ML. > > Which my reply you are referring to? You stripped part of some > sentence and put it without *any context*. Just random sentence. > I asked for vendor prefix in few places... srom-timing? width? And I > do not remember where I used exactly these words. Ok, sorry, i promise to improve. :) Anyway, i have figured out how to add sub-devices, and heavily modified the whole thing. And indeed, vendor prefix is now very useful, so i added it to all three properties. Making v4... Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia
On 29.10.2015 20:43, Pavel Fedin wrote: > Hello! > >>>> Any vendor prefix here? How generic is this? >>> >>> I just don't know... Does *everything* really need a vendor prefix? How readable would that >> be? "compatible" property already says >>> that it's samsung-exynos-specific. And IMHO it's quite obvious that properties of vendor- >> specific device are automatically >>> vendor-specific. >>> Ok, i am currently fixing up the rest and will post v4 soon, and will Cc: it to devicetree >> ML. >> >> Which my reply you are referring to? You stripped part of some >> sentence and put it without *any context*. Just random sentence. >> I asked for vendor prefix in few places... srom-timing? width? And I >> do not remember where I used exactly these words. > > Ok, sorry, i promise to improve. :) > Anyway, i have figured out how to add sub-devices, and heavily modified the whole thing. And indeed, vendor prefix is now very useful, so i added it to all three properties. Making v4... Actually now I found: Documentation/devicetree/bindings/net/gpmc-eth.txt Aren't you duplicating this work? This looks very, very similar. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt b/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt index 33886d5..9e4a40b 100644 --- a/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt +++ b/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt @@ -5,8 +5,30 @@ Required properties: - reg: offset and length of the register set -Example: +Bank configurations can be defined in optional subnodes. Each subnode +describes one bank and contains the following: + +Required properties: +- bank : bank number (0 - 3) + +- srom-timing : array of 7 integers: Tacp, Tcah, Tcoh, Tacc, Tcos, Tacs + +Optional properties: +- width : data width in bytes (1 or 2). If omitted, default of 1 is used. + +Example: basic definition, no banks are configured + sromc@12570000 { + compatible = "samsung,exynos-srom"; + reg = <0x12570000 0x10>; + }; + +Example: SROMc with bank3 configuration sromc@12570000 { compatible = "samsung,exynos-srom"; reg = <0x12570000 0x10>; + bank@3 { + bank = <3>; + width = <2>; + srom-timing = <1 9 12 1 9 1 1>; + }; };
Add documentation for new properties, allowing bank configuration. Signed-off-by: Pavel Fedin <p.fedin@samsung.com> --- .../bindings/arm/samsung/exynos-srom.txt | 24 +++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)