[PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[Lee Jones]

Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Srinivas Kandagatla <srinivas.kandagatla-qxv4g6HH51o@public.gmane.org>
Acked-by: Alexandre Torgue <alexandre.torgue-qxv4g6HH51o@public.gmane.org>
Signed-off-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 Documentation/devicetree/bindings/ata/ahci-st.txt | 31 +++++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ata/ahci-st.txt

diff --git a/Documentation/devicetree/bindings/ata/ahci-st.txt b/Documentation/devicetree/bindings/ata/ahci-st.txt
new file mode 100644
index 0000000..1b69fa9
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/ahci-st.txt
@@ -0,0 +1,31 @@
+STMicroelectronics STi SATA controller
+
+This binding describes a SATA device.
+
+Required properties:
+ - compatible	   : Must be "st,ahci"
+ - reg		   : Physical base addresses and length of register sets
+ - interrupts	   : Interrupt associated with the SATA device
+ - interrupt-names :   Associated name must be; "hostc"
+ - resets	   : The power-down and soft-reset lines of SATA IP
+ - reset-names	   :   Associated names must be; "pwr-dwn" and "sw-rst"
+ - clocks	   : The phandle for the clock
+ - clock-names	   :   Associated name must be; "ahci_clk"
+ - phys		   : The phandle for the PHY device
+ - phy-names	   :   Associated name must be; "ahci_phy"
+
+Example:
+
+	sata0: sata@fe380000 {
+		compatible      = "st,ahci";
+		reg             = <0xfe380000 0x1000>;
+		interrupts      = <GIC_SPI 157 IRQ_TYPE_NONE>;
+		interrupt-names = "hostc";
+		phys	        = <&miphy365x_phy MIPHY_PORT_0 MIPHY_TYPE_SATA>;
+		phy-names       = "ahci_phy";
+		resets	        = <&powerdown STIH416_SATA0_POWERDOWN>,
+				  <&softreset STIH416_SATA0_SOFTRESET>;
+		reset-names     = "pwr-dwn", "sw-rst";
+		clocks	        = <&CLK_S_ICN_REG_0>;
+		clock-names     = "ahci_clk";
+	};
-- 
1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/3] ARM: DT: STi: Add DT node for ST's SATA device

[Lee Jones]

Cc: devicetree@vger.kernel.org
Cc: Srinivas Kandagatla <srinivas.kandagatla@st.com>
Acked-by: Alexandre Torgue <alexandre.torgue@st.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/stih416-b2020-revE.dts |  4 ++++
 arch/arm/boot/dts/stih416-b2020.dts      |  4 ++++
 arch/arm/boot/dts/stih416.dtsi           | 16 ++++++++++++++++
 3 files changed, 24 insertions(+)

diff --git a/arch/arm/boot/dts/stih416-b2020-revE.dts b/arch/arm/boot/dts/stih416-b2020-revE.dts
index 693d0ec..7350a86 100644
--- a/arch/arm/boot/dts/stih416-b2020-revE.dts
+++ b/arch/arm/boot/dts/stih416-b2020-revE.dts
@@ -37,5 +37,9 @@
 			st,pcie-tx-pol-inv;
 			st,sata-gen = <3>;
 		};
+
+		sata0: sata@fe380000{
+			status = "okay";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/stih416-b2020.dts b/arch/arm/boot/dts/stih416-b2020.dts
index fd9cbad..ebd784b 100644
--- a/arch/arm/boot/dts/stih416-b2020.dts
+++ b/arch/arm/boot/dts/stih416-b2020.dts
@@ -18,5 +18,9 @@
 			st,pcie_tx_pol_inv = <1>;
 			st,sata_gen = "gen3";
 		};
+
+		sata0: sata@fe380000{
+			status = "okay";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/stih416.dtsi b/arch/arm/boot/dts/stih416.dtsi
index 0d3f59c..313d4fd 100644
--- a/arch/arm/boot/dts/stih416.dtsi
+++ b/arch/arm/boot/dts/stih416.dtsi
@@ -205,5 +205,21 @@
 			#phy-cells 	= <2>;
 			st,syscfg  	= <&syscfg_rear>;
 		};
+
+		sata0: sata@fe380000 {
+			compatible      = "st,ahci";
+			reg             = <0xfe380000 0x1000>;
+			interrupts      = <GIC_SPI 157 IRQ_TYPE_NONE>;
+			interrupt-names = "hostc";
+			phys	        = <&miphy365x_phy MIPHY_PORT_0 MIPHY_TYPE_SATA>;
+			phy-names       = "sata-phy";
+			resets	        = <&powerdown STIH416_SATA0_POWERDOWN>,
+					  <&softreset STIH416_SATA0_SOFTRESET>;
+			reset-names     = "pwr-dwn", "sw-rst";
+			clock-names     = "ahci_clk";
+			clocks	        = <&CLK_S_ICN_REG_0>;
+
+			status	        = "disabled";
+		};
 	};
 };
-- 
1.8.3.2

Re: [PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[Mark Rutland]

On Tue, Feb 25, 2014 at 11:41:19AM +0000, Lee Jones wrote:
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Srinivas Kandagatla <srinivas.kandagatla-qxv4g6HH51o@public.gmane.org>
> Acked-by: Alexandre Torgue <alexandre.torgue-qxv4g6HH51o@public.gmane.org>
> Signed-off-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/ata/ahci-st.txt | 31 +++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/ata/ahci-st.txt
> 
> diff --git a/Documentation/devicetree/bindings/ata/ahci-st.txt b/Documentation/devicetree/bindings/ata/ahci-st.txt
> new file mode 100644
> index 0000000..1b69fa9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/ahci-st.txt
> @@ -0,0 +1,31 @@
> +STMicroelectronics STi SATA controller

Is that a family name or a particular device?

> +
> +This binding describes a SATA device.
> +
> +Required properties:
> + - compatible	   : Must be "st,ahci"

Is there not a more specific name? This is awfully generic.

> + - reg		   : Physical base addresses and length of register sets
> + - interrupts	   : Interrupt associated with the SATA device
> + - interrupt-names :   Associated name must be; "hostc"
> + - resets	   : The power-down and soft-reset lines of SATA IP
> + - reset-names	   :   Associated names must be; "pwr-dwn" and "sw-rst"

Get rid of the semicolon, it makes this harder to read. Same for the
other -names proeprties.

> + - clocks	   : The phandle for the clock
> + - clock-names	   :   Associated name must be; "ahci_clk"
> + - phys		   : The phandle for the PHY device
> + - phy-names	   :   Associated name must be; "ahci_phy"

This looks sane, but I'd like to see a more specific description and
compatible string.

Cheers,
Mark.

> +
> +Example:
> +
> +	sata0: sata@fe380000 {
> +		compatible      = "st,ahci";
> +		reg             = <0xfe380000 0x1000>;
> +		interrupts      = <GIC_SPI 157 IRQ_TYPE_NONE>;
> +		interrupt-names = "hostc";
> +		phys	        = <&miphy365x_phy MIPHY_PORT_0 MIPHY_TYPE_SATA>;
> +		phy-names       = "ahci_phy";
> +		resets	        = <&powerdown STIH416_SATA0_POWERDOWN>,
> +				  <&softreset STIH416_SATA0_SOFTRESET>;
> +		reset-names     = "pwr-dwn", "sw-rst";
> +		clocks	        = <&CLK_S_ICN_REG_0>;
> +		clock-names     = "ahci_clk";
> +	};
> -- 
> 1.8.3.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[Lee Jones]

> > +This binding describes a SATA device.
> > +
> > +Required properties:
> > + - compatible	   : Must be "st,ahci"
> 
> Is there not a more specific name? This is awfully generic.

There isn't one. ST only have one AHCI driver. The driver is named as
generically.

> > + - reg		   : Physical base addresses and length of register sets
> > + - interrupts	   : Interrupt associated with the SATA device
> > + - interrupt-names :   Associated name must be; "hostc"
> > + - resets	   : The power-down and soft-reset lines of SATA IP
> > + - reset-names	   :   Associated names must be; "pwr-dwn" and "sw-rst"
> 
> Get rid of the semicolon, it makes this harder to read. Same for the
> other -names proeprties.

Sure.

> > + - clocks	   : The phandle for the clock
> > + - clock-names	   :   Associated name must be; "ahci_clk"
> > + - phys		   : The phandle for the PHY device
> > + - phy-names	   :   Associated name must be; "ahci_phy"
> 
> This looks sane, but I'd like to see a more specific description and
> compatible string.

Specific description in what sense?

"This binding describes a SATA device."

to

"This binding describes ST's SATA device." ?

> > +Example:
> > +
> > +	sata0: sata@fe380000 {
> > +		compatible      = "st,ahci";
> > +		reg             = <0xfe380000 0x1000>;
> > +		interrupts      = <GIC_SPI 157 IRQ_TYPE_NONE>;
> > +		interrupt-names = "hostc";
> > +		phys	        = <&miphy365x_phy MIPHY_PORT_0 MIPHY_TYPE_SATA>;
> > +		phy-names       = "ahci_phy";
> > +		resets	        = <&powerdown STIH416_SATA0_POWERDOWN>,
> > +				  <&softreset STIH416_SATA0_SOFTRESET>;
> > +		reset-names     = "pwr-dwn", "sw-rst";
> > +		clocks	        = <&CLK_S_ICN_REG_0>;
> > +		clock-names     = "ahci_clk";
> > +	};

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[Arnd Bergmann]

On Wednesday 12 March 2014 12:48:35 Lee Jones wrote:
> > > +This binding describes a SATA device.
> > > +
> > > +Required properties:
> > > + - compatible         : Must be "st,ahci"
> > 
> > Is there not a more specific name? This is awfully generic.
> 
> There isn't one. ST only have one AHCI driver. The driver is named as
> generically.

If it is this generic, can't you use the completely generic
ahci-platform driver and add a "generic-ahci" compatible string
to that?

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[Lee Jones]

> > > > +This binding describes a SATA device.
> > > > +
> > > > +Required properties:
> > > > + - compatible         : Must be "st,ahci"
> > > 
> > > Is there not a more specific name? This is awfully generic.
> > 
> > There isn't one. ST only have one AHCI driver. The driver is named as
> > generically.
> 
> If it is this generic, can't you use the completely generic
> ahci-platform driver and add a "generic-ahci" compatible string
> to that?

It pretty much does. The only 'special' functionality we handle in
the driver are resets and configuring the OOB. However, both are
required.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2 1/3] ata: ahci_st: Provide DT bindings for ST's SATA implementation

[Arnd Bergmann]

On Wednesday 12 March 2014 17:14:52 Lee Jones wrote:
> > > > > +This binding describes a SATA device.
> > > > > +
> > > > > +Required properties:
> > > > > + - compatible         : Must be "st,ahci"
> > > > 
> > > > Is there not a more specific name? This is awfully generic.
> > > 
> > > There isn't one. ST only have one AHCI driver. The driver is named as
> > > generically.
> > 
> > If it is this generic, can't you use the completely generic
> > ahci-platform driver and add a "generic-ahci" compatible string
> > to that?
> 
> It pretty much does. The only 'special' functionality we handle in
> the driver are resets and configuring the OOB. However, both are
> required.

The reset handling should be easy to add to the generic driver
usign as an optional property if I'm not mistaken.

What is OOB?

	Arnd