| Message ID | 1389769910-15505-3-git-send-email-lho@apm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jan 15, 2014 at 07:11:48AM +0000, Loc Ho wrote: > Signed-off-by: Loc Ho <lho@apm.com> > Signed-off-by: Tuan Phan <tphan@apm.com> > Signed-off-by: Suman Tripathi <stripathi@apm.com> > --- > .../devicetree/bindings/ata/apm-xgene.txt | 68 ++++++++++++++++++++ > 1 files changed, 68 insertions(+), 0 deletions(-) > create mode 100644 Documentation/devicetree/bindings/ata/apm-xgene.txt > > diff --git a/Documentation/devicetree/bindings/ata/apm-xgene.txt b/Documentation/devicetree/bindings/ata/apm-xgene.txt > new file mode 100644 > index 0000000..3d1421a > --- /dev/null > +++ b/Documentation/devicetree/bindings/ata/apm-xgene.txt > @@ -0,0 +1,68 @@ > +* APM X-Gene 6.0 Gb/s SATA host controller nodes > + > +SATA host controller nodes are defined to describe on-chip Serial ATA > +controllers. Each SATA controller (pair of ports) have its own node. > + > +Required properties: > +- compatible : Shall be "apm,xgene-ahci-sgmii" if mux'ed with SGMII > + or "apm,xgene-ahci-pcie" if mux'ed with PCIe. > +- reg : First memory resource shall be the AHCI memory > + resource. > + Second memory resource shall be the host controller > + memory resource. > +- interrupts : Interrupt mapping for SATA host controller IRQ. s/Interrupt-mapping/interrupt-specifier/ > +- clocks : Reference to the clock entry. > +- phys : PHY reference with parameter 0. The specific value of the phy-specifier shouldn't matter to this binding. What should matter is what it logically corresponds to. > +- phy-names : Name of the PHY. Shall be "sata-6g". Could you define phys in terms of phy-names please? It makes the relationship clearer: - phys: a list of phandles + phy-sciefiers, one for each entry in phy-names - phy-names: Should contain: * "sata-6g" for the SATA PHY <notes about requirements go here> Cheers, Mark.
Hi, >> +- clocks : Reference to the clock entry. >> +- phys : PHY reference with parameter 0. > > The specific value of the phy-specifier shouldn't matter to this > binding. What should matter is what it logically corresponds to. I not quite following this. Are you suggest that I drop the value 0. In the binding, one needs to specify the mode of operation - 0 is for SATA. Can you explain more? -Loc
On Wednesday 15 January 2014 12:04:02 Loc Ho wrote: > > >> +- clocks : Reference to the clock entry. > >> +- phys : PHY reference with parameter 0. > > > > The specific value of the phy-specifier shouldn't matter to this > > binding. What should matter is what it logically corresponds to. > > I not quite following this. Are you suggest that I drop the value 0. > In the binding, one needs to specify the mode of operation - 0 is for > SATA. Can you explain more? The SATA device should not care what the argument for the PHY device is. You could connect the same device to another PHY that has a different set of arguments, which is the whole point of abstracting it. Arnd
Hi, >> >> >> +- clocks : Reference to the clock entry. >> >> +- phys : PHY reference with parameter 0. >> > >> > The specific value of the phy-specifier shouldn't matter to this >> > binding. What should matter is what it logically corresponds to. >> >> I not quite following this. Are you suggest that I drop the value 0. >> In the binding, one needs to specify the mode of operation - 0 is for >> SATA. Can you explain more? > > The SATA device should not care what the argument for the PHY > device is. You could connect the same device to another PHY > that has a different set of arguments, which is the whole point > of abstracting it. > I understand what you wrote here. We should not have an argument from the host controller. Then my question is how should the PHY node indicates that it needs to be configured itself as an SATA PHY? -Loc
On Wednesday 15 January 2014 13:08:47 Loc Ho wrote: > >> >> +- clocks : Reference to the clock entry. > >> >> +- phys : PHY reference with parameter 0. > >> > > >> > The specific value of the phy-specifier shouldn't matter to this > >> > binding. What should matter is what it logically corresponds to. > >> > >> I not quite following this. Are you suggest that I drop the value 0. > >> In the binding, one needs to specify the mode of operation - 0 is for > >> SATA. Can you explain more? > > > > The SATA device should not care what the argument for the PHY > > device is. You could connect the same device to another PHY > > that has a different set of arguments, which is the whole point > > of abstracting it. > > > > I understand what you wrote here. We should not have an argument from > the host controller. Then my question is how should the PHY node > indicates that it needs to be configured itself as an SATA PHY? The "phys" property should specify whatever the configuration of the phy device is supposed to be. It's just not the business of the sata driver or the sata binding to know what that configuration is. Arnd
Hi, >> >> >> +- clocks : Reference to the clock entry. >> >> >> +- phys : PHY reference with parameter 0. >> >> > >> >> > The specific value of the phy-specifier shouldn't matter to this >> >> > binding. What should matter is what it logically corresponds to. >> >> >> >> I not quite following this. Are you suggest that I drop the value 0. >> >> In the binding, one needs to specify the mode of operation - 0 is for >> >> SATA. Can you explain more? >> > >> > The SATA device should not care what the argument for the PHY >> > device is. You could connect the same device to another PHY >> > that has a different set of arguments, which is the whole point >> > of abstracting it. >> > >> >> I understand what you wrote here. We should not have an argument from >> the host controller. Then my question is how should the PHY node >> indicates that it needs to be configured itself as an SATA PHY? > > The "phys" property should specify whatever the configuration of > the phy device is supposed to be. It's just not the business of > the sata driver or the sata binding to know what that configuration > is. > Here is what I have and I am trying to piece this together: phy1 { #phy-cells = <0>; /* No parameter as suggest by Mark */ }; sata1 { ::: phys = <&phy1>; }; What I don't get is how does one specify the mode (SATA/USB/SGMII/etc)? As another parameter in the phy1 node? -Loc
Hi, >>> >> >> +- clocks : Reference to the clock entry. >>> >> >> +- phys : PHY reference with parameter 0. >>> >> > >>> >> > The specific value of the phy-specifier shouldn't matter to this >>> >> > binding. What should matter is what it logically corresponds to. >>> >> >>> >> I not quite following this. Are you suggest that I drop the value 0. >>> >> In the binding, one needs to specify the mode of operation - 0 is for >>> >> SATA. Can you explain more? >>> > >>> > The SATA device should not care what the argument for the PHY >>> > device is. You could connect the same device to another PHY >>> > that has a different set of arguments, which is the whole point >>> > of abstracting it. >>> > >>> >>> I understand what you wrote here. We should not have an argument from >>> the host controller. Then my question is how should the PHY node >>> indicates that it needs to be configured itself as an SATA PHY? >> >> The "phys" property should specify whatever the configuration of >> the phy device is supposed to be. It's just not the business of >> the sata driver or the sata binding to know what that configuration >> is. >> > > Here is what I have and I am trying to piece this together: > > phy1 { > #phy-cells = <0>; /* No parameter as suggest by Mark */ > }; > > sata1 { > ::: > phys = <&phy1>; > }; > > What I don't get is how does one specify the mode > (SATA/USB/SGMII/etc)? As another parameter in the phy1 node? > May be I misread what Mark mentioned. The binding documentation should not specify the requirement of 0. And the binding of the dts should be as is: phy1 { #phy-cells = <1>; }; sata1 { ::: phys = <&phy1 0>; where 0 indicates the mode of operation? }; -Loc
On Wednesday 15 January 2014 15:59:27 Loc Ho wrote: > > May be I misread what Mark mentioned. The binding documentation should > not specify the requirement of 0. And the binding of the dts should be > as is: > > phy1 { > #phy-cells = <1>; > }; > > sata1 { > ::: > phys = <&phy1 0>; where 0 indicates the mode of operation? > }; > > Yes, that is right. You can keep the zero in the example, but the binding text must not require a specific phy to be used. Arnd
diff --git a/Documentation/devicetree/bindings/ata/apm-xgene.txt b/Documentation/devicetree/bindings/ata/apm-xgene.txt new file mode 100644 index 0000000..3d1421a --- /dev/null +++ b/Documentation/devicetree/bindings/ata/apm-xgene.txt @@ -0,0 +1,68 @@ +* APM X-Gene 6.0 Gb/s SATA host controller nodes + +SATA host controller nodes are defined to describe on-chip Serial ATA +controllers. Each SATA controller (pair of ports) have its own node. + +Required properties: +- compatible : Shall be "apm,xgene-ahci-sgmii" if mux'ed with SGMII + or "apm,xgene-ahci-pcie" if mux'ed with PCIe. +- reg : First memory resource shall be the AHCI memory + resource. + Second memory resource shall be the host controller + memory resource. +- interrupts : Interrupt mapping for SATA host controller IRQ. +- clocks : Reference to the clock entry. +- phys : PHY reference with parameter 0. +- phy-names : Name of the PHY. Shall be "sata-6g". + +Optional properties: +- status : Shall be "ok" if enabled or "disabled" if disabled. + Default is "ok". +- interrupt-parent : Interrupt controller. + +Example: + sataclk: sataclk { + compatible = "fixed-clock"; + #clock-cells = <1>; + clock-frequency = <100000000>; + clock-output-names = "sataclk"; + }; + + phy2: phy@1f22a000 { + compatible = "apm,xgene-phy"; + reg = <0x0 0x1f22a000 0x0 0x100>, + <0x0 0x1f22c000 0x0 0x100>; + #phy-cells = <1>; + }; + + phy3: phy@1f23a000 { + compatible = "apm,xgene-phy-ext"; + reg = <0x0 0x1f23a000 0x0 0x100>, + <0x0 0x1f23c000 0x0 0x100>, + <0x0 0x1f2d0000 0x0 0x100>; + #phy-cells = <1>; + }; + + sata2: sata@1a400000 { + compatible = "apm,xgene-ahci-sgmii"; + reg = <0x0 0x1a400000 0x0 0x1000>, + <0x0 0x1f220000 0x0 0x10000>; + interrupt-parent = <&gic>; + interrupts = <0x0 0x87 0x4>; + status = "ok"; + clocks = <&sataclk 0>; + phys = <&phy2 0>; + phy-names = "sata-6g"; + }; + + sata3: sata@1a800000 { + compatible = "apm,xgene-ahci-pcie"; + reg = <0x0 0x1a800000 0x0 0x1000>, + <0x0 0x1f230000 0x0 0x10000>; + interrupt-parent = <&gic>; + interrupts = <0x0 0x88 0x4>; + status = "ok"; + clocks = <&sataclk 0>; + phys = <&phy3 0>; + phy-names = "sata-6g"; + };