| Message ID | 0bc58ce0fd39767834f486c4c0cfbbd70044caed.1446799912.git.p.fedin@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Nov 6, 2015 at 4:03 AM, Pavel Fedin <p.fedin@samsung.com> wrote: > Add documentation for new subnode properties, allowing bank configuration. > Based on u-boot implementation, but heavily reworked. > > Also, fix size of SROMc mapping in the example. > > Signed-off-by: Pavel Fedin <p.fedin@samsung.com> > Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > --- > .../bindings/arm/samsung/exynos-srom.txt | 71 +++++++++++++++++++++- > 1 file changed, 69 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt b/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt > index 33886d5..3ff2950 100644 > --- a/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt > +++ b/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt > @@ -5,8 +5,75 @@ Required properties: > > - reg: offset and length of the register set > > -Example: > +Optional properties: > +The SROM controller can be used to attach external peripherals. In this case > +extra properties, describing the bus behind it, should be specified as below: > + > +- #address-cells: Must be set to 2 to allow memory address translation 2 is for CS# and offset. > + > +- #size-cells: Must be set to 1 to allow CS address passing size is the size, not the address. > + > +- ranges: Must be set up to reflect the memory layout with four integer values > + per bank: > + <bank-number> 0 <physical address of bank> <size> s/physical/parent/ You could have another level of translation above for the parent. > + > +Sub-nodes: > +The actual device nodes should be added as subnodes to the SROMc node. These > +subnodes, except regular device specification, should contain the following > +properties, describing configuration of the relevant SROM bank: > + > +Required properties: > +- reg: bank number, base address (relative to start of the bank) and size of > + the memory mapped for the device. Note that base address will be > + typically 0 as this is the start of the bank. > + > +- samsung,srom-timing : array of 6 integers, specifying bank timings in the > + following order: Tacp, Tcah, Tcoh, Tacc, Tcos, Tacs. > + Each value is specified in cycles and has the following > + meaning and valid range: > + Tacp : Page mode access cycle at Page mode (0 - 15) > + Tcah : Address holding time after CSn (0 - 15) > + Tcoh : Chip selection hold on OEn (0 - 15) > + Tacc : Access cycle (0 - 31, the actual time is N + 1) > + Tcos : Chip selection set-up before OEn (0 - 15) > + Tacs : Address set-up before CSn (0 - 15) This is not easily extended. Perhaps a property per value instead. > + > +Optional properties: > +- reg-io-width : data width in bytes (1 or 2). If omitted, default of 1 is used. > + > +- samsung,srom-page-mode : page mode configuration for the bank: > + 0 - normal (one data) > + 1 - four data > + If omitted, default of 0 is used. > + > +Example: basic definition, no banks are configured > + sromc@12570000 { > + compatible = "samsung,exynos-srom"; > + reg = <0x12570000 0x14>; > + }; > + > +Example: SROMc with SMSC911x ethernet chip on bank 3 > sromc@12570000 { > + #address-cells = <2>; > + #size-cells = <1>; > + ranges = <0 0 0x04000000 0x20000 // Bank0 > + 1 0 0x05000000 0x20000 // Bank1 > + 2 0 0x06000000 0x20000 // Bank2 > + 3 0 0x07000000 0x20000>; // Bank3 > + > compatible = "samsung,exynos-srom"; > - reg = <0x12570000 0x10>; > + reg = <0x12570000 0x14>; > + > + ethernet@3 { > + compatible = "smsc,lan9115"; > + reg = <3 0 0x10000>; // Bank 3, offset = 0 > + phy-mode = "mii"; > + interrupt-parent = <&gpx0>; > + interrupts = <5 8>; > + reg-io-width = <2>; > + smsc,irq-push-pull; > + smsc,force-internal-phy; > + > + samsung,srom-config = <1 9 12 1 9 1 1>; This doesn't match the doc. > + }; > }; > -- > 2.4.4 >
Hello! > > +- samsung,srom-timing : array of 6 integers, specifying bank timings in the > > + following order: Tacp, Tcah, Tcoh, Tacc, Tcos, Tacs. > > + Each value is specified in cycles and has the following > > + meaning and valid range: > > + Tacp : Page mode access cycle at Page mode (0 - 15) > > + Tcah : Address holding time after CSn (0 - 15) > > + Tcoh : Chip selection hold on OEn (0 - 15) > > + Tacc : Access cycle (0 - 31, the actual time is N + 1) > > + Tcos : Chip selection set-up before OEn (0 - 15) > > + Tacs : Address set-up before CSn (0 - 15) > > This is not easily extended. Perhaps a property per value instead. We had a discussion with Krzysztof about it, he agreed with this form of the property. My concern was that it's just too much typing, and makes little sense because these settings always go together. If register layout changes, or parameter set changes in incompatible way, then it's another device, not exynos-srom anymore. So would you agree with that, or is your position strong? > > compatible = "samsung,exynos-srom"; > > - reg = <0x12570000 0x10>; > > + reg = <0x12570000 0x14>; > > + > > + ethernet@3 { > > + compatible = "smsc,lan9115"; > > + reg = <3 0 0x10000>; // Bank 3, offset = 0 > > + phy-mode = "mii"; > > + interrupt-parent = <&gpx0>; > > + interrupts = <5 8>; > > + reg-io-width = <2>; > > + smsc,irq-push-pull; > > + smsc,force-internal-phy; > > + > > + samsung,srom-config = <1 9 12 1 9 1 1>; > > This doesn't match the doc. Damn, overlooked. :( Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia
On Wed, Nov 11, 2015 at 12:44 AM, Pavel Fedin <p.fedin@samsung.com> wrote: > Hello! > >> > +- samsung,srom-timing : array of 6 integers, specifying bank timings in the >> > + following order: Tacp, Tcah, Tcoh, Tacc, Tcos, Tacs. >> > + Each value is specified in cycles and has the following >> > + meaning and valid range: >> > + Tacp : Page mode access cycle at Page mode (0 - 15) >> > + Tcah : Address holding time after CSn (0 - 15) >> > + Tcoh : Chip selection hold on OEn (0 - 15) >> > + Tacc : Access cycle (0 - 31, the actual time is N + 1) >> > + Tcos : Chip selection set-up before OEn (0 - 15) >> > + Tacs : Address set-up before CSn (0 - 15) >> >> This is not easily extended. Perhaps a property per value instead. > > We had a discussion with Krzysztof about it, he agreed with this form of the property. > My concern was that it's just too much typing, and makes little sense because these > settings always go together. If register layout changes, or parameter set changes in > incompatible way, then it's another device, not exynos-srom anymore. > So would you agree with that, or is your position strong? I'm thinking for a new version of the controller which could add (or remove) new timing parameters, but then I guess you can interpret the field differently based on the compatible string. Anyway, your problem to deal with. Rob
On 12.11.2015 05:43, Rob Herring wrote: > On Wed, Nov 11, 2015 at 12:44 AM, Pavel Fedin <p.fedin@samsung.com> wrote: >> Hello! >> >>>> +- samsung,srom-timing : array of 6 integers, specifying bank timings in the >>>> + following order: Tacp, Tcah, Tcoh, Tacc, Tcos, Tacs. >>>> + Each value is specified in cycles and has the following >>>> + meaning and valid range: >>>> + Tacp : Page mode access cycle at Page mode (0 - 15) >>>> + Tcah : Address holding time after CSn (0 - 15) >>>> + Tcoh : Chip selection hold on OEn (0 - 15) >>>> + Tacc : Access cycle (0 - 31, the actual time is N + 1) >>>> + Tcos : Chip selection set-up before OEn (0 - 15) >>>> + Tacs : Address set-up before CSn (0 - 15) >>> >>> This is not easily extended. Perhaps a property per value instead. >> >> We had a discussion with Krzysztof about it, he agreed with this form of the property. >> My concern was that it's just too much typing, and makes little sense because these >> settings always go together. If register layout changes, or parameter set changes in >> incompatible way, then it's another device, not exynos-srom anymore. >> So would you agree with that, or is your position strong? > > I'm thinking for a new version of the controller which could add (or > remove) new timing parameters, but then I guess you can interpret the > field differently based on the compatible string. Anyway, your problem > to deal with. Actually I also preferred properties per one timing... but finally agreed on simpler approach. Adding new parameters to the array is still possible (because the order matters) and removal as well (by ignoring some indices). All ARMv7 Exynos SoCs have exactly the same registers for SROM controller (Exynos3250, Exynos4, Exynos5). On newer Exynos ARMv8 (Exynos5433 and Exynos7420) I don't know because it is not documented. Best regards, Krzysztof
Hello! > >> > +- samsung,srom-timing : array of 6 integers, specifying bank timings in the > >> > + following order: Tacp, Tcah, Tcoh, Tacc, Tcos, Tacs. > >> > + Each value is specified in cycles and has the following > >> > + meaning and valid range: > >> > + Tacp : Page mode access cycle at Page mode (0 - 15) > >> > + Tcah : Address holding time after CSn (0 - 15) > >> > + Tcoh : Chip selection hold on OEn (0 - 15) > >> > + Tacc : Access cycle (0 - 31, the actual time is N + 1) > >> > + Tcos : Chip selection set-up before OEn (0 - 15) > >> > + Tacs : Address set-up before CSn (0 - 15) > >> > >> This is not easily extended. Perhaps a property per value instead. > > > > We had a discussion with Krzysztof about it, he agreed with this form of the property. > > My concern was that it's just too much typing, and makes little sense because these > > settings always go together. If register layout changes, or parameter set changes in > > incompatible way, then it's another device, not exynos-srom anymore. > > So would you agree with that, or is your position strong? > > I'm thinking for a new version of the controller which could add (or > remove) new timing parameters, but then I guess you can interpret the > field differently based on the compatible string. Anyway, your problem > to deal with. Of course, my thought is that if compatible string is different, then it's already a different device. And of course it would have different parameters. So, OK, i'll post new version with fixed documentation today. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia
diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt b/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt index 33886d5..3ff2950 100644 --- a/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt +++ b/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt @@ -5,8 +5,75 @@ Required properties: - reg: offset and length of the register set -Example: +Optional properties: +The SROM controller can be used to attach external peripherals. In this case +extra properties, describing the bus behind it, should be specified as below: + +- #address-cells: Must be set to 2 to allow memory address translation + +- #size-cells: Must be set to 1 to allow CS address passing + +- ranges: Must be set up to reflect the memory layout with four integer values + per bank: + <bank-number> 0 <physical address of bank> <size> + +Sub-nodes: +The actual device nodes should be added as subnodes to the SROMc node. These +subnodes, except regular device specification, should contain the following +properties, describing configuration of the relevant SROM bank: + +Required properties: +- reg: bank number, base address (relative to start of the bank) and size of + the memory mapped for the device. Note that base address will be + typically 0 as this is the start of the bank. + +- samsung,srom-timing : array of 6 integers, specifying bank timings in the + following order: Tacp, Tcah, Tcoh, Tacc, Tcos, Tacs. + Each value is specified in cycles and has the following + meaning and valid range: + Tacp : Page mode access cycle at Page mode (0 - 15) + Tcah : Address holding time after CSn (0 - 15) + Tcoh : Chip selection hold on OEn (0 - 15) + Tacc : Access cycle (0 - 31, the actual time is N + 1) + Tcos : Chip selection set-up before OEn (0 - 15) + Tacs : Address set-up before CSn (0 - 15) + +Optional properties: +- reg-io-width : data width in bytes (1 or 2). If omitted, default of 1 is used. + +- samsung,srom-page-mode : page mode configuration for the bank: + 0 - normal (one data) + 1 - four data + If omitted, default of 0 is used. + +Example: basic definition, no banks are configured + sromc@12570000 { + compatible = "samsung,exynos-srom"; + reg = <0x12570000 0x14>; + }; + +Example: SROMc with SMSC911x ethernet chip on bank 3 sromc@12570000 { + #address-cells = <2>; + #size-cells = <1>; + ranges = <0 0 0x04000000 0x20000 // Bank0 + 1 0 0x05000000 0x20000 // Bank1 + 2 0 0x06000000 0x20000 // Bank2 + 3 0 0x07000000 0x20000>; // Bank3 + compatible = "samsung,exynos-srom"; - reg = <0x12570000 0x10>; + reg = <0x12570000 0x14>; + + ethernet@3 { + compatible = "smsc,lan9115"; + reg = <3 0 0x10000>; // Bank 3, offset = 0 + phy-mode = "mii"; + interrupt-parent = <&gpx0>; + interrupts = <5 8>; + reg-io-width = <2>; + smsc,irq-push-pull; + smsc,force-internal-phy; + + samsung,srom-config = <1 9 12 1 9 1 1>; + }; };