| Message ID | 1392377036-12816-1-git-send-email-lee.jones@linaro.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
Hi, On Friday 14 February 2014 04:53 PM, Lee Jones wrote: > The MiPHY365x is a Generic PHY which can serve various SATA or PCIe > devices. It has 2 ports which it can use for either; both SATA, both > PCIe or one of each in any configuration. > > Cc: devicetree@vger.kernel.org > Cc: Srinivas Kandagatla <srinivas.kandagatla@st.com> > Signed-off-by: Lee Jones <lee.jones@linaro.org> since this uses 'dt-bindings/phy/phy-miphy365x.h' which is used in phy driver as well, I need ACK from dt maintainers so that I can queue both the driver and dt patches myself. Thanks Kishon > --- > arch/arm/boot/dts/stih416-b2020-revE.dts | 6 +++++- > arch/arm/boot/dts/stih416-b2020.dts | 6 ++++++ > arch/arm/boot/dts/stih416.dtsi | 13 +++++++++++++ > 3 files changed, 24 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/stih416-b2020-revE.dts b/arch/arm/boot/dts/stih416-b2020-revE.dts > index a874570..dbe67fa 100644 > --- a/arch/arm/boot/dts/stih416-b2020-revE.dts > +++ b/arch/arm/boot/dts/stih416-b2020-revE.dts > @@ -32,6 +32,10 @@ > ethernet1: ethernet@fef08000 { > snps,reset-gpio = <&PIO0 7>; > }; > - }; > > + miphy365x_phy: miphy365x@0 { > + st,pcie_tx_pol_inv = <1>; > + st,sata_gen = "gen3"; > + }; > + }; > }; > diff --git a/arch/arm/boot/dts/stih416-b2020.dts b/arch/arm/boot/dts/stih416-b2020.dts > index 276f28d..fd9cbad 100644 > --- a/arch/arm/boot/dts/stih416-b2020.dts > +++ b/arch/arm/boot/dts/stih416-b2020.dts > @@ -13,4 +13,10 @@ > model = "STiH416 B2020"; > compatible = "st,stih416", "st,stih416-b2020"; > > + soc { > + miphy365x_phy: miphy365x@0 { > + st,pcie_tx_pol_inv = <1>; > + st,sata_gen = "gen3"; > + }; > + }; > }; > diff --git a/arch/arm/boot/dts/stih416.dtsi b/arch/arm/boot/dts/stih416.dtsi > index 85b8063..9fd8efb 100644 > --- a/arch/arm/boot/dts/stih416.dtsi > +++ b/arch/arm/boot/dts/stih416.dtsi > @@ -9,6 +9,8 @@ > #include "stih41x.dtsi" > #include "stih416-clock.dtsi" > #include "stih416-pinctrl.dtsi" > + > +#include <dt-bindings/phy/phy-miphy365x.h> > #include <dt-bindings/interrupt-controller/arm-gic.h> > #include <dt-bindings/reset-controller/stih416-resets.h> > / { > @@ -140,5 +142,16 @@ > clocks = <&CLK_S_ICN_REG_0>; > }; > > + miphy365x_phy: miphy365x@0 { > + compatible = "st,miphy365x-phy"; > + reg = <0xfe382000 0x100>, > + <0xfe38a000 0x100>, > + <0xfe394000 0x100>, > + <0xfe804000 0x100>; > + reg-names = "sata0", "sata1", "pcie0", "pcie1"; > + > + #phy-cells = <2>; > + st,syscfg = <&syscfg_rear>; > + }; > }; > }; > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 14, 2014 at 11:23:53AM +0000, Lee Jones wrote: > The MiPHY365x is a Generic PHY which can serve various SATA or PCIe > devices. It has 2 ports which it can use for either; both SATA, both > PCIe or one of each in any configuration. > > Cc: devicetree@vger.kernel.org > Cc: Srinivas Kandagatla <srinivas.kandagatla@st.com> > Signed-off-by: Lee Jones <lee.jones@linaro.org> > --- > .../devicetree/bindings/phy/phy-miphy365x.txt | 54 ++++++++++++++++++++++ > 1 file changed, 54 insertions(+) > create mode 100644 Documentation/devicetree/bindings/phy/phy-miphy365x.txt > > diff --git a/Documentation/devicetree/bindings/phy/phy-miphy365x.txt b/Documentation/devicetree/bindings/phy/phy-miphy365x.txt > new file mode 100644 > index 0000000..96f269f > --- /dev/null > +++ b/Documentation/devicetree/bindings/phy/phy-miphy365x.txt > @@ -0,0 +1,54 @@ > +STMicroelectronics STi MIPHY365x PHY binding > +============================================ > + > +This binding describes a miphy device that is used to control PHY hardware > +for SATA and PCIe. > + > +Required properties: > +- compatible: Should be "st,miphy365x-phy" > +- #phy-cells: Should be 2 (See second example) > + First cell is the port number; MIPHY_PORT_{0,1} > + Second cell is device type; MIPHY_TYPE_{SATA,PCI} Either this should refer to the header file, or specific values should be given in the binding document. > +- reg: Address and length of the register set for the device > +- reg-names: The names of the register addresses corresponding to the > + registers filled in "reg" > + Options are; sata{0,1} and pcie{0,1} (See first example) How about something like: - reg: a list of offset + length pairs, one for each entry in reg-names - reg-names: should contain some of: * "sata0" for ... * "sata1" for ... * "pcie0" for ... * "pcie1" for ... Where ... might just be "the sata port 0 registers" > +- st,syscfg : Should be a phandle of the system configuration register group > + which contain the SATA, PCIe mode setting bits I'll assume this is well-defined by some other binding. > + > +Optional properties: > +- st,sata-gen : Generation of locally attached SATA IP. Expected values > + are {1,2,3). If not supplied generation 1 hardware will > + be expected > +- st,pcie-tx-pol-inv : Bool property to invert the polarity PCIe Tx (Txn/Txp) > +- st,sata-tx-pol-inv : Bool property to invert the polarity SATA Tx (Txn/Txp) It might just be me, but the phrase "invert the polarity {SATA,PCIe} Tx" sounds odd. What exactly is being inverted? > + > +Example: > + > + miphy365x_phy: miphy365x@0 { > + compatible = "st,miphy365x-phy"; > + #phy-cells = <2>; > + reg = <0xfe382000 0x100>, > + <0xfe38a000 0x100>, > + <0xfe394000 0x100>, > + <0xfe804000 0x100>; > + reg-names = "sata0", "sata1", "pcie0", "pcie1"; > + st,syscfg= <&syscfg_rear>; Nit: missing space before '='. > + }; > + > +Specifying phy control of devices > +================================= > + > +Device nodes should specify the configuration required in their "phys" > +property, containing a phandle to the miphy device node, a port number > +and a device type. > + > +Example: > + > +#include <dt-bindings/phy/phy-miphy365x.h> > + > + sata0: sata@fe380000 { > + ... > + phys = <&miphy365x_phy MIPHY_PORT_0 MIPHY_TYPE_SATA>; > + ... > + }; Is there not a generic phy binding we can point to? It seems a bit redundant to do this in each phy binding. Cheers, Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 14, 2014 at 11:23:54AM +0000, Lee Jones wrote: > This provides the shared header file which will be reference from both > the MiPHY365x driver and its associated Device Tree node(s). > > Cc: devicetree@vger.kernel.org > Cc: Srinivas Kandagatla <srinivas.kandagatla@st.com> > Signed-off-by: Lee Jones <lee.jones@linaro.org> > --- > include/dt-bindings/phy/phy-miphy365x.h | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > create mode 100644 include/dt-bindings/phy/phy-miphy365x.h Given this defines part of the binding, it should probably go in the same patch. Cheers, Mark. > > diff --git a/include/dt-bindings/phy/phy-miphy365x.h b/include/dt-bindings/phy/phy-miphy365x.h > new file mode 100644 > index 0000000..8757c02 > --- /dev/null > +++ b/include/dt-bindings/phy/phy-miphy365x.h > @@ -0,0 +1,25 @@ > +/* > + * This header provides constants for the phy framework > + * based on the STMicroelectronics miphy365x. > + */ > +#ifndef _DT_BINDINGS_PHY_MIPHY > +#define _DT_BINDINGS_PHY_MIPHY > + > +/* Supports 16 ports without a datatype change (u8 & 0xF0). */ > +#define MIPHY_PORT_0 0 > +#define MIPHY_PORT_1 1 > +#define MIPHY_PORT_2 2 > +#define MIPHY_PORT_3 3 > + > +/* Supports 16 types without a datatype change (u8 & 0x0F). */ > +#define MIPHY_TYPE_SHIFT 4 > +#define MIPHY_TYPE_SATA (0 << MIPHY_TYPE_SHIFT) > +#define MIPHY_TYPE_PCIE (1 << MIPHY_TYPE_SHIFT) > +#define MIPHY_TYPE_USB (2 << MIPHY_TYPE_SHIFT) > + > +#define MIPHY_SATA_PORT0 (MIPHY_TYPE_SATA) | (MIPHY_PORT_0) > +#define MIPHY_SATA_PORT1 (MIPHY_TYPE_SATA) | (MIPHY_PORT_1) > +#define MIPHY_PCIE_PORT0 (MIPHY_TYPE_PCIE) | (MIPHY_PORT_0) > +#define MIPHY_PCIE_PORT1 (MIPHY_TYPE_PCIE) | (MIPHY_PORT_1) > + > +#endif /* _DT_BINDINGS_PHY_MIPHY */ > -- > 1.8.3.2 > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 14, 2014 at 11:23:55AM +0000, Lee Jones wrote: > The MiPHY365x is a Generic PHY which can serve various SATA or PCIe > devices. It has 2 ports which it can use for either; both SATA, both > PCIe or one of each in any configuration. > > Cc: devicetree@vger.kernel.org > Cc: Srinivas Kandagatla <srinivas.kandagatla@st.com> > Signed-off-by: Lee Jones <lee.jones@linaro.org> > --- > arch/arm/boot/dts/stih416-b2020-revE.dts | 6 +++++- > arch/arm/boot/dts/stih416-b2020.dts | 6 ++++++ > arch/arm/boot/dts/stih416.dtsi | 13 +++++++++++++ > 3 files changed, 24 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/stih416-b2020-revE.dts b/arch/arm/boot/dts/stih416-b2020-revE.dts > index a874570..dbe67fa 100644 > --- a/arch/arm/boot/dts/stih416-b2020-revE.dts > +++ b/arch/arm/boot/dts/stih416-b2020-revE.dts > @@ -32,6 +32,10 @@ > ethernet1: ethernet@fef08000 { > snps,reset-gpio = <&PIO0 7>; > }; > - }; > > + miphy365x_phy: miphy365x@0 { This has registers at 0x0? Or is the unit-address wrong? > + st,pcie_tx_pol_inv = <1>; This is a boolean. The '= <1>' is not required and is confusing. > + st,sata_gen = "gen3"; s/"gen3"/<3>/ Both these properties need s/_/-/ applied. All these apply to the other dts too. > + }; > + }; > }; > diff --git a/arch/arm/boot/dts/stih416-b2020.dts b/arch/arm/boot/dts/stih416-b2020.dts > index 276f28d..fd9cbad 100644 > --- a/arch/arm/boot/dts/stih416-b2020.dts > +++ b/arch/arm/boot/dts/stih416-b2020.dts > @@ -13,4 +13,10 @@ > model = "STiH416 B2020"; > compatible = "st,stih416", "st,stih416-b2020"; This compatible list is the wrong way around. Left to right should go from most specific to most general / oldest variant. > > + soc { > + miphy365x_phy: miphy365x@0 { > + st,pcie_tx_pol_inv = <1>; > + st,sata_gen = "gen3"; > + }; > + }; > }; > diff --git a/arch/arm/boot/dts/stih416.dtsi b/arch/arm/boot/dts/stih416.dtsi > index 85b8063..9fd8efb 100644 > --- a/arch/arm/boot/dts/stih416.dtsi > +++ b/arch/arm/boot/dts/stih416.dtsi > @@ -9,6 +9,8 @@ > #include "stih41x.dtsi" > #include "stih416-clock.dtsi" > #include "stih416-pinctrl.dtsi" > + > +#include <dt-bindings/phy/phy-miphy365x.h> > #include <dt-bindings/interrupt-controller/arm-gic.h> > #include <dt-bindings/reset-controller/stih416-resets.h> > / { > @@ -140,5 +142,16 @@ > clocks = <&CLK_S_ICN_REG_0>; > }; > > + miphy365x_phy: miphy365x@0 { The unit-address should be fe382000 rather than 0 to match the first reg entry. Cheers, Mark. > + compatible = "st,miphy365x-phy"; > + reg = <0xfe382000 0x100>, > + <0xfe38a000 0x100>, > + <0xfe394000 0x100>, > + <0xfe804000 0x100>; > + reg-names = "sata0", "sata1", "pcie0", "pcie1"; > + > + #phy-cells = <2>; > + st,syscfg = <&syscfg_rear>; > + }; > }; > }; > -- > 1.8.3.2 > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 05 Mar 2014, Mark Rutland wrote: > On Fri, Feb 14, 2014 at 11:23:53AM +0000, Lee Jones wrote: > > The MiPHY365x is a Generic PHY which can serve various SATA or PCIe > > devices. It has 2 ports which it can use for either; both SATA, both > > PCIe or one of each in any configuration. > > > > Cc: devicetree@vger.kernel.org > > Cc: Srinivas Kandagatla <srinivas.kandagatla@st.com> > > Signed-off-by: Lee Jones <lee.jones@linaro.org> > > --- > > .../devicetree/bindings/phy/phy-miphy365x.txt | 54 ++++++++++++++++++++++ > > 1 file changed, 54 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/phy/phy-miphy365x.txt > > > > diff --git a/Documentation/devicetree/bindings/phy/phy-miphy365x.txt b/Documentation/devicetree/bindings/phy/phy-miphy365x.txt > > new file mode 100644 > > index 0000000..96f269f > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/phy/phy-miphy365x.txt > > @@ -0,0 +1,54 @@ > > +STMicroelectronics STi MIPHY365x PHY binding > > +============================================ > > + > > +This binding describes a miphy device that is used to control PHY hardware > > +for SATA and PCIe. > > + > > +Required properties: > > +- compatible: Should be "st,miphy365x-phy" > > +- #phy-cells: Should be 2 (See second example) > > + First cell is the port number; MIPHY_PORT_{0,1} > > + Second cell is device type; MIPHY_TYPE_{SATA,PCI} > > Either this should refer to the header file, or specific values should > be given in the binding document. When you say specific values you mean break out the {,}s? I thought most people would know what they mean. > > +- reg: Address and length of the register set for the device > > +- reg-names: The names of the register addresses corresponding to the > > + registers filled in "reg" > > + Options are; sata{0,1} and pcie{0,1} (See first example) > > How about something like: Same here. > - reg: a list of offset + length pairs, one for each entry in reg-names > - reg-names: should contain some of: > * "sata0" for ... > * "sata1" for ... > * "pcie0" for ... > * "pcie1" for ... > > Where ... might just be "the sata port 0 registers" Seems awfully redundant. > > +- st,syscfg : Should be a phandle of the system configuration register group > > + which contain the SATA, PCIe mode setting bits > > I'll assume this is well-defined by some other binding. Right. > > + > > +Optional properties: > > +- st,sata-gen : Generation of locally attached SATA IP. Expected values > > + are {1,2,3). If not supplied generation 1 hardware will > > + be expected > > +- st,pcie-tx-pol-inv : Bool property to invert the polarity PCIe Tx (Txn/Txp) > > +- st,sata-tx-pol-inv : Bool property to invert the polarity SATA Tx (Txn/Txp) > > It might just be me, but the phrase "invert the polarity {SATA,PCIe} Tx" > sounds odd. What exactly is being inverted? >From the doc (and the only reference of this functionallity): rx_polarity: 1: switch rxp/rxn tx_polarity: 1: switch txp/txn If we don't set these registers up to reflect the h/w configuration of the board, the MiPHY will not work. > > + > > +Example: > > + > > + miphy365x_phy: miphy365x@0 { > > + compatible = "st,miphy365x-phy"; > > + #phy-cells = <2>; > > + reg = <0xfe382000 0x100>, > > + <0xfe38a000 0x100>, > > + <0xfe394000 0x100>, > > + <0xfe804000 0x100>; > > + reg-names = "sata0", "sata1", "pcie0", "pcie1"; > > + st,syscfg= <&syscfg_rear>; > > Nit: missing space before '='. Will fix. > > + }; > > + > > +Specifying phy control of devices > > +================================= > > + > > +Device nodes should specify the configuration required in their "phys" > > +property, containing a phandle to the miphy device node, a port number > > +and a device type. > > + > > +Example: > > + > > +#include <dt-bindings/phy/phy-miphy365x.h> > > + > > + sata0: sata@fe380000 { > > + ... > > + phys = <&miphy365x_phy MIPHY_PORT_0 MIPHY_TYPE_SATA>; > > + ... > > + }; > > Is there not a generic phy binding we can point to? It seems a bit > redundant to do this in each phy binding. Sure, but that wouldn't make much of an example. Documentation/devicetree/bindings/phy/phy-bindings.txt
> > > +Specifying phy control of devices > > > +================================= > > > + > > > +Device nodes should specify the configuration required in their "phys" > > > +property, containing a phandle to the miphy device node, a port number > > > +and a device type. > > > + > > > +Example: > > > + > > > +#include <dt-bindings/phy/phy-miphy365x.h> > > > + > > > + sata0: sata@fe380000 { > > > + ... > > > + phys = <&miphy365x_phy MIPHY_PORT_0 MIPHY_TYPE_SATA>; > > > + ... > > > + }; > > > > Is there not a generic phy binding we can point to? It seems a bit > > redundant to do this in each phy binding. > > Sure, but that wouldn't make much of an example. > > Documentation/devicetree/bindings/phy/phy-bindings.txt Ah sorry, I see what you mean now. No, not yet. That's the next thing to go up into Mainline. I will replace this second example with a link when the other documentation document has been accepted.
diff --git a/Documentation/devicetree/bindings/phy/phy-miphy365x.txt b/Documentation/devicetree/bindings/phy/phy-miphy365x.txt new file mode 100644 index 0000000..96f269f --- /dev/null +++ b/Documentation/devicetree/bindings/phy/phy-miphy365x.txt @@ -0,0 +1,54 @@ +STMicroelectronics STi MIPHY365x PHY binding +============================================ + +This binding describes a miphy device that is used to control PHY hardware +for SATA and PCIe. + +Required properties: +- compatible: Should be "st,miphy365x-phy" +- #phy-cells: Should be 2 (See second example) + First cell is the port number; MIPHY_PORT_{0,1} + Second cell is device type; MIPHY_TYPE_{SATA,PCI} +- reg: Address and length of the register set for the device +- reg-names: The names of the register addresses corresponding to the + registers filled in "reg" + Options are; sata{0,1} and pcie{0,1} (See first example) +- st,syscfg : Should be a phandle of the system configuration register group + which contain the SATA, PCIe mode setting bits + +Optional properties: +- st,sata-gen : Generation of locally attached SATA IP. Expected values + are {1,2,3). If not supplied generation 1 hardware will + be expected +- st,pcie-tx-pol-inv : Bool property to invert the polarity PCIe Tx (Txn/Txp) +- st,sata-tx-pol-inv : Bool property to invert the polarity SATA Tx (Txn/Txp) + +Example: + + miphy365x_phy: miphy365x@0 { + compatible = "st,miphy365x-phy"; + #phy-cells = <2>; + reg = <0xfe382000 0x100>, + <0xfe38a000 0x100>, + <0xfe394000 0x100>, + <0xfe804000 0x100>; + reg-names = "sata0", "sata1", "pcie0", "pcie1"; + st,syscfg= <&syscfg_rear>; + }; + +Specifying phy control of devices +================================= + +Device nodes should specify the configuration required in their "phys" +property, containing a phandle to the miphy device node, a port number +and a device type. + +Example: + +#include <dt-bindings/phy/phy-miphy365x.h> + + sata0: sata@fe380000 { + ... + phys = <&miphy365x_phy MIPHY_PORT_0 MIPHY_TYPE_SATA>; + ... + };
The MiPHY365x is a Generic PHY which can serve various SATA or PCIe devices. It has 2 ports which it can use for either; both SATA, both PCIe or one of each in any configuration. Cc: devicetree@vger.kernel.org Cc: Srinivas Kandagatla <srinivas.kandagatla@st.com> Signed-off-by: Lee Jones <lee.jones@linaro.org> --- .../devicetree/bindings/phy/phy-miphy365x.txt | 54 ++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/phy-miphy365x.txt