[PATCH v3 0/9] ata: ahci_platform: support allwinner R40 AHCI

[PATCH v3 0/9] ata: ahci_platform: support allwinner R40 AHCI

[Corentin Labbe]

Hello

This patchset add support for allwinner R40 AHCI controller.

This controller need two regulator and one reset which is unsupported
for the moment on ahci_platform.

So this patchset add support for reset and regulator for AHCI
controller.
It add also support for non-target regulator per SATA port.

The whole patchset is tested on sun8i-r40-bananapi-m2-ultra and
on sun7i-a20-cubieboard2 which doesnt have any of the ressources added
by this serie, so no regression should come with it.

Since just before sending this patchset, I discovered the revert of
f0f56716fc3e ("ata: ahci-platform: add reset control support")
I have also tested this serie on a tegra124-jetson-tk1.

Changes since V2
- Moved all ressources management to ahci_platform

Corentin Labbe (9):
  ata: ahci_platform: add support for AHCI controller regulator
  dt-bindings: ata: ahci-platform: document ahci-supply
  ata: ahci_platform: add support for AHCI controller reset
  dt-bindings: ata: ahci-platform: document AHCI reset
  dt-bindings: ata: ahci-platform: fix indentation of target-supply
  ata: ahci_platform: add support for port regulator
  dt-bindings: ata: ahci-platform: document port-supply
  ARM: dts: sun8i: r40: add sata node
  ARM: dts: sun8i: sun8i-r40-bananapi-m2-ultra: enable AHCI

 .../devicetree/bindings/ata/ahci-platform.txt      |   7 +-
 arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts  |  21 +++++
 arch/arm/boot/dts/sun8i-r40.dtsi                   |  16 ++++
 drivers/ata/ahci.h                                 |   4 +
 drivers/ata/libahci_platform.c                     | 102 ++++++++++++++++++---
 5 files changed, 138 insertions(+), 12 deletions(-)

-- 
2.16.4

[PATCH v3 1/9] ata: ahci_platform: add support for AHCI controller regulator

[Corentin Labbe]

The SoC R40 AHCI controller need a regulator to work.
So this patch add a way to add an optional regulator on AHCI controller.

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 drivers/ata/ahci.h             |  1 +
 drivers/ata/libahci_platform.c | 27 +++++++++++++++++++++++++--
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 1609ebab4e23..633d3ec5c1df 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -351,6 +351,7 @@ struct ahci_host_priv {
 	bool			got_runtime_pm; /* Did we do pm_runtime_get? */
 	struct clk		*clks[AHCI_MAX_CLKS]; /* Optional */
 	struct regulator	**target_pwrs;	/* Optional */
+	struct regulator	*ahci_regulator;/* Optional */
 	/*
 	 * If platform uses PHYs. There is a 1:1 relation between the port number and
 	 * the PHY position in this array.
diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
index fe8939e161ea..d997a30ce793 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -138,7 +138,7 @@ EXPORT_SYMBOL_GPL(ahci_platform_disable_clks);
  * ahci_platform_enable_regulators - Enable regulators
  * @hpriv: host private area to store config values
  *
- * This function enables all the regulators found in
+ * This function enables all the regulators found in controller and
  * hpriv->target_pwrs, if any.  If a regulator fails to be enabled, it
  * disables all the regulators already enabled in reverse order and
  * returns an error.
@@ -150,6 +150,12 @@ int ahci_platform_enable_regulators(struct ahci_host_priv *hpriv)
 {
 	int rc, i;
 
+	if (hpriv->ahci_regulator) {
+		rc = regulator_enable(hpriv->ahci_regulator);
+		if (rc)
+			return rc;
+	}
+
 	for (i = 0; i < hpriv->nports; i++) {
 		if (!hpriv->target_pwrs[i])
 			continue;
@@ -166,6 +172,8 @@ int ahci_platform_enable_regulators(struct ahci_host_priv *hpriv)
 		if (hpriv->target_pwrs[i])
 			regulator_disable(hpriv->target_pwrs[i]);
 
+	if (hpriv->ahci_regulator)
+		regulator_disable(hpriv->ahci_regulator);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(ahci_platform_enable_regulators);
@@ -174,7 +182,8 @@ EXPORT_SYMBOL_GPL(ahci_platform_enable_regulators);
  * ahci_platform_disable_regulators - Disable regulators
  * @hpriv: host private area to store config values
  *
- * This function disables all regulators found in hpriv->target_pwrs.
+ * This function disables all regulators found in hpriv->target_pwrs and
+ * AHCI controller.
  */
 void ahci_platform_disable_regulators(struct ahci_host_priv *hpriv)
 {
@@ -185,6 +194,9 @@ void ahci_platform_disable_regulators(struct ahci_host_priv *hpriv)
 			continue;
 		regulator_disable(hpriv->target_pwrs[i]);
 	}
+
+	if (hpriv->ahci_regulator)
+		regulator_disable(hpriv->ahci_regulator);
 }
 EXPORT_SYMBOL_GPL(ahci_platform_disable_regulators);
 /**
@@ -336,6 +348,7 @@ static int ahci_platform_get_regulator(struct ahci_host_priv *hpriv, u32 port,
  *
  * 1) mmio registers (IORESOURCE_MEM 0, mandatory)
  * 2) regulator for controlling the targets power (optional)
+ *    regulator for controlling the AHCI controller (optional)
  * 3) 0 - AHCI_MAX_CLKS clocks, as specified in the devs devicetree node,
  *    or for non devicetree enabled platforms a single clock
  * 4) phys (optional)
@@ -391,6 +404,16 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev)
 		hpriv->clks[i] = clk;
 	}
 
+	hpriv->ahci_regulator = devm_regulator_get_optional(dev, "ahci");
+
+	if (IS_ERR(hpriv->ahci_regulator)) {
+		rc = PTR_ERR(hpriv->ahci_regulator);
+		if (rc == -EPROBE_DEFER)
+			goto err_out;
+		rc = 0;
+		hpriv->ahci_regulator = NULL;
+	}
+
 	hpriv->nports = child_nodes = of_get_child_count(dev->of_node);
 
 	/*
-- 
2.16.4

[PATCH v3 2/9] dt-bindings: ata: ahci-platform: document ahci-supply

[Corentin Labbe]

This patch document the new optional ahci-supply.

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 Documentation/devicetree/bindings/ata/ahci-platform.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
index c760ecb81381..5f362af2724c 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
@@ -33,6 +33,7 @@ Optional properties:
 - target-supply     : regulator for SATA target power
 - phys              : reference to the SATA PHY node
 - phy-names         : must be "sata-phy"
+- ahci-supply       : regulator for AHCI controller
 - ports-implemented : Mask that indicates which ports that the HBA supports
 		      are available for software to use. Useful if PORTS_IMPL
 		      is not programmed by the BIOS, which is true with
-- 
2.16.4

[PATCH v3 3/9] ata: ahci_platform: add support for AHCI controller reset

[Corentin Labbe]

The SoC R40 AHCI controller need a reset to work.
So this patch add a way to add an optional reset on AHCI controller.

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 drivers/ata/ahci.h             |  2 ++
 drivers/ata/libahci_platform.c | 28 ++++++++++++++++++++++++----
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 633d3ec5c1df..274c1885a5ad 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -40,6 +40,7 @@
 #include <linux/libata.h>
 #include <linux/phy/phy.h>
 #include <linux/regulator/consumer.h>
+#include <linux/reset.h>
 
 /* Enclosure Management Control */
 #define EM_CTRL_MSG_TYPE              0x000f0000
@@ -352,6 +353,7 @@ struct ahci_host_priv {
 	struct clk		*clks[AHCI_MAX_CLKS]; /* Optional */
 	struct regulator	**target_pwrs;	/* Optional */
 	struct regulator	*ahci_regulator;/* Optional */
+	struct reset_control	*ahci_reset;	/* Optional */
 	/*
 	 * If platform uses PHYs. There is a 1:1 relation between the port number and
 	 * the PHY position in this array.
diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
index d997a30ce793..1199ba411c15 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -207,7 +207,8 @@ EXPORT_SYMBOL_GPL(ahci_platform_disable_regulators);
  * following order:
  * 1) Regulator
  * 2) Clocks (through ahci_platform_enable_clks)
- * 3) Phys
+ * 3) reset
+ * 4) Phys
  *
  * If resource enabling fails at any point the previous enabled resources
  * are disabled in reverse order.
@@ -227,12 +228,19 @@ int ahci_platform_enable_resources(struct ahci_host_priv *hpriv)
 	if (rc)
 		goto disable_regulator;
 
-	rc = ahci_platform_enable_phys(hpriv);
+	rc = reset_control_assert(hpriv->ahci_reset);
 	if (rc)
 		goto disable_clks;
 
+	rc = ahci_platform_enable_phys(hpriv);
+	if (rc)
+		goto disable_reset;
+
 	return 0;
 
+disable_reset:
+	reset_control_deassert(hpriv->ahci_reset);
+
 disable_clks:
 	ahci_platform_disable_clks(hpriv);
 
@@ -250,13 +258,16 @@ EXPORT_SYMBOL_GPL(ahci_platform_enable_resources);
  * This function disables all ahci_platform managed resources in the
  * following order:
  * 1) Phys
- * 2) Clocks (through ahci_platform_disable_clks)
- * 3) Regulator
+ * 2) reset
+ * 3) Clocks (through ahci_platform_disable_clks)
+ * 4) Regulator
  */
 void ahci_platform_disable_resources(struct ahci_host_priv *hpriv)
 {
 	ahci_platform_disable_phys(hpriv);
 
+	reset_control_deassert(hpriv->ahci_reset);
+
 	ahci_platform_disable_clks(hpriv);
 
 	ahci_platform_disable_regulators(hpriv);
@@ -414,6 +425,15 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev)
 		hpriv->ahci_regulator = NULL;
 	}
 
+	hpriv->ahci_reset = devm_reset_control_get_optional(dev, "ahci");
+	if (IS_ERR(hpriv->ahci_reset)) {
+		rc = PTR_ERR(hpriv->ahci_reset);
+		if (rc == -EPROBE_DEFER)
+			goto err_out;
+		rc = 0;
+		hpriv->ahci_reset = NULL;
+	}
+
 	hpriv->nports = child_nodes = of_get_child_count(dev->of_node);
 
 	/*
-- 
2.16.4

[PATCH v3 4/9] dt-bindings: ata: ahci-platform: document AHCI reset

[Corentin Labbe]

This patch document the new optional resets for ahci node.

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 Documentation/devicetree/bindings/ata/ahci-platform.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
index 5f362af2724c..a5281d7557e3 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
@@ -34,6 +34,8 @@ Optional properties:
 - phys              : reference to the SATA PHY node
 - phy-names         : must be "sata-phy"
 - ahci-supply       : regulator for AHCI controller
+- resets            : phandle to the reset line of AHCI controller
+                      If set, must have a reset-names set as "ahci"
 - ports-implemented : Mask that indicates which ports that the HBA supports
 		      are available for software to use. Useful if PORTS_IMPL
 		      is not programmed by the BIOS, which is true with
-- 
2.16.4

[PATCH v3 5/9] dt-bindings: ata: ahci-platform: fix indentation of target-supply

[Corentin Labbe]

This patch fix the indentation of target-supply's ':'.

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 Documentation/devicetree/bindings/ata/ahci-platform.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
index a5281d7557e3..47652521a6ed 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
@@ -50,7 +50,7 @@ Sub-nodes required properties:
 - reg		    : the port number
 And at least one of the following properties:
 - phys		    : reference to the SATA PHY node
-- target-supply    : regulator for SATA target power
+- target-supply     : regulator for SATA target power
 
 Examples:
         sata@ffe08000 {
-- 
2.16.4

[PATCH v3 6/9] ata: ahci_platform: add support for port regulator

[Corentin Labbe]

The SoC R40 AHCI controller need a port regulator to work.
We cannot use the "target-supply" since it's not a target power regulator.
So this patch add a way to add an optional port regulator.

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 drivers/ata/ahci.h             |  1 +
 drivers/ata/libahci_platform.c | 47 +++++++++++++++++++++++++++++++++++++-----
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 274c1885a5ad..716fd679dd3e 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -352,6 +352,7 @@ struct ahci_host_priv {
 	bool			got_runtime_pm; /* Did we do pm_runtime_get? */
 	struct clk		*clks[AHCI_MAX_CLKS]; /* Optional */
 	struct regulator	**target_pwrs;	/* Optional */
+	struct regulator	**port_regulators;/* Optional */
 	struct regulator	*ahci_regulator;/* Optional */
 	struct reset_control	*ahci_reset;	/* Optional */
 	/*
diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
index 1199ba411c15..d7574b3e1f6a 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -148,7 +148,7 @@ EXPORT_SYMBOL_GPL(ahci_platform_disable_clks);
  */
 int ahci_platform_enable_regulators(struct ahci_host_priv *hpriv)
 {
-	int rc, i;
+	int rc, i, j;
 
 	if (hpriv->ahci_regulator) {
 		rc = regulator_enable(hpriv->ahci_regulator);
@@ -164,9 +164,21 @@ int ahci_platform_enable_regulators(struct ahci_host_priv *hpriv)
 		if (rc)
 			goto disable_target_pwrs;
 	}
+	for (j = 0; j < hpriv->nports; j++) {
+		if (!hpriv->port_regulators[j])
+			continue;
+
+		rc = regulator_enable(hpriv->port_regulators[j]);
+		if (rc)
+			goto disable_port_regulators;
+	}
 
 	return 0;
 
+disable_port_regulators:
+	while (--j >= 0)
+		if (hpriv->port_regulators[j])
+			regulator_disable(hpriv->port_regulators[j]);
 disable_target_pwrs:
 	while (--i >= 0)
 		if (hpriv->target_pwrs[i])
@@ -190,9 +202,10 @@ void ahci_platform_disable_regulators(struct ahci_host_priv *hpriv)
 	int i;
 
 	for (i = 0; i < hpriv->nports; i++) {
-		if (!hpriv->target_pwrs[i])
-			continue;
-		regulator_disable(hpriv->target_pwrs[i]);
+		if (hpriv->target_pwrs[i])
+			regulator_disable(hpriv->target_pwrs[i]);
+		if (hpriv->port_regulators[i])
+			regulator_disable(hpriv->port_regulators[i]);
 	}
 
 	if (hpriv->ahci_regulator)
@@ -291,9 +304,12 @@ static void ahci_platform_put_resources(struct device *dev, void *res)
 	 * SATA device itself. So we can't use devm for automatically
 	 * releasing them. We have to do it manually here.
 	 */
-	for (c = 0; c < hpriv->nports; c++)
+	for (c = 0; c < hpriv->nports; c++) {
 		if (hpriv->target_pwrs && hpriv->target_pwrs[c])
 			regulator_put(hpriv->target_pwrs[c]);
+		if (hpriv->port_regulators && hpriv->port_regulators[c])
+			regulator_put(hpriv->port_regulators[c]);
+	}
 }
 
 static int ahci_platform_get_phy(struct ahci_host_priv *hpriv, u32 port,
@@ -338,6 +354,7 @@ static int ahci_platform_get_regulator(struct ahci_host_priv *hpriv, u32 port,
 				struct device *dev)
 {
 	struct regulator *target_pwr;
+	struct regulator *port_regulator;
 	int rc = 0;
 
 	target_pwr = regulator_get_optional(dev, "target");
@@ -346,6 +363,21 @@ static int ahci_platform_get_regulator(struct ahci_host_priv *hpriv, u32 port,
 		hpriv->target_pwrs[port] = target_pwr;
 	else
 		rc = PTR_ERR(target_pwr);
+	/* Only EPROBE_DEFER is important since it's an optional regulator */
+	if (rc != -EPROBE_DEFER)
+		rc = 0;
+	else
+		return rc;
+
+	port_regulator = regulator_get_optional(dev, "port");
+
+	if (!IS_ERR(port_regulator))
+		hpriv->port_regulators[port] = port_regulator;
+	else
+		rc = PTR_ERR(port_regulator);
+	/* Only EPROBE_DEFER is important since it's an optional regulator */
+	if (rc != -EPROBE_DEFER)
+		rc = 0;
 
 	return rc;
 }
@@ -454,6 +486,11 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev)
 		rc = -ENOMEM;
 		goto err_out;
 	}
+	hpriv->port_regulators = devm_kcalloc(dev, hpriv->nports, sizeof(*hpriv->port_regulators), GFP_KERNEL);
+	if (!hpriv->port_regulators) {
+		rc = -ENOMEM;
+		goto err_out;
+	}
 
 	if (child_nodes) {
 		for_each_child_of_node(dev->of_node, child) {
-- 
2.16.4

[PATCH v3 7/9] dt-bindings: ata: ahci-platform: document port-supply

[Corentin Labbe]

This patch document the new optional port-supply

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 Documentation/devicetree/bindings/ata/ahci-platform.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
index 47652521a6ed..40a636b9851e 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
@@ -31,6 +31,7 @@ Optional properties:
 - dma-coherent      : Present if dma operations are coherent
 - clocks            : a list of phandle + clock specifier pairs
 - target-supply     : regulator for SATA target power
+- port-supply       : regulator for SATA port
 - phys              : reference to the SATA PHY node
 - phy-names         : must be "sata-phy"
 - ahci-supply       : regulator for AHCI controller
@@ -51,6 +52,7 @@ Sub-nodes required properties:
 And at least one of the following properties:
 - phys		    : reference to the SATA PHY node
 - target-supply     : regulator for SATA target power
+- port-supply       : regulator for SATA port
 
 Examples:
         sata@ffe08000 {
-- 
2.16.4

[PATCH v3 8/9] ARM: dts: sun8i: r40: add sata node

[Corentin Labbe]

R40 have a sata controller which is the same as A20.
This patch adds a DT node for it.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 arch/arm/boot/dts/sun8i-r40.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
index d4ad55eb2245..bd369112a400 100644
--- a/arch/arm/boot/dts/sun8i-r40.dtsi
+++ b/arch/arm/boot/dts/sun8i-r40.dtsi
@@ -550,6 +550,22 @@
 			#size-cells = <0>;
 		};
 
+		ahci: sata@1c18000 {
+			compatible = "allwinner,sun4i-a10-ahci";
+			reg = <0x01c18000 0x1000>;
+			interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_SATA>, <&ccu CLK_SATA>;
+			resets = <&ccu RST_BUS_SATA>;
+			resets-name = "ahci";
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			sata_port0: sata-port@0 {
+				reg = <0>;
+			};
+		};
+
 		gmac: ethernet@1c50000 {
 			compatible = "allwinner,sun8i-r40-gmac";
 			syscon = <&ccu>;
-- 
2.16.4

[PATCH v3 9/9] ARM: dts: sun8i: sun8i-r40-bananapi-m2-ultra: enable AHCI

[Corentin Labbe]

This patch enable the AHCI controller.
Since this controller need two regulator, this patch add them.

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts b/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
index c39b9169ea64..07eaf6cd9d19 100644
--- a/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
+++ b/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
@@ -105,6 +105,11 @@
 	};
 };
 
+&ahci {
+	ahci-supply = <&reg_dldo4>;
+	status = "okay";
+};
+
 &de {
 	status = "okay";
 };
@@ -251,6 +256,22 @@
 	regulator-name = "vcc-wifi";
 };
 
+&reg_dldo4 {
+	regulator-min-microvolt = <2500000>;
+	regulator-max-microvolt = <2500000>;
+	regulator-name = "vdd2v5-sata";
+};
+
+&reg_eldo3 {
+	regulator-min-microvolt = <1200000>;
+	regulator-max-microvolt = <1200000>;
+	regulator-name = "vdd1v2-sata";
+};
+
+&sata_port0 {
+	port-supply = <&reg_eldo3>;
+};
+
 &tcon_tv0 {
 	status = "okay";
 };
-- 
2.16.4

Re: [PATCH v3 6/9] ata: ahci_platform: add support for port regulator

[Icenowy Zheng]

于 2018年7月13日 GMT+08:00 下午7:03:03, Corentin Labbe <clabbe@baylibre.com> 写到:
>The SoC R40 AHCI controller need a port regulator to work.

In fact it should be a PHY regulator.

>We cannot use the "target-supply" since it's not a target power
>regulator.
>So this patch add a way to add an optional port regulator.
>
>Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
>---
> drivers/ata/ahci.h             |  1 +
>drivers/ata/libahci_platform.c | 47
>+++++++++++++++++++++++++++++++++++++-----
> 2 files changed, 43 insertions(+), 5 deletions(-)
>
>diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
>index 274c1885a5ad..716fd679dd3e 100644
>--- a/drivers/ata/ahci.h
>+++ b/drivers/ata/ahci.h
>@@ -352,6 +352,7 @@ struct ahci_host_priv {
> 	bool			got_runtime_pm; /* Did we do pm_runtime_get? */
> 	struct clk		*clks[AHCI_MAX_CLKS]; /* Optional */
> 	struct regulator	**target_pwrs;	/* Optional */
>+	struct regulator	**port_regulators;/* Optional */
> 	struct regulator	*ahci_regulator;/* Optional */
> 	struct reset_control	*ahci_reset;	/* Optional */
> 	/*
>diff --git a/drivers/ata/libahci_platform.c
>b/drivers/ata/libahci_platform.c
>index 1199ba411c15..d7574b3e1f6a 100644
>--- a/drivers/ata/libahci_platform.c
>+++ b/drivers/ata/libahci_platform.c
>@@ -148,7 +148,7 @@ EXPORT_SYMBOL_GPL(ahci_platform_disable_clks);
>  */
> int ahci_platform_enable_regulators(struct ahci_host_priv *hpriv)
> {
>-	int rc, i;
>+	int rc, i, j;
> 
> 	if (hpriv->ahci_regulator) {
> 		rc = regulator_enable(hpriv->ahci_regulator);
>@@ -164,9 +164,21 @@ int ahci_platform_enable_regulators(struct
>ahci_host_priv *hpriv)
> 		if (rc)
> 			goto disable_target_pwrs;
> 	}
>+	for (j = 0; j < hpriv->nports; j++) {
>+		if (!hpriv->port_regulators[j])
>+			continue;
>+
>+		rc = regulator_enable(hpriv->port_regulators[j]);
>+		if (rc)
>+			goto disable_port_regulators;
>+	}
> 
> 	return 0;
> 
>+disable_port_regulators:
>+	while (--j >= 0)
>+		if (hpriv->port_regulators[j])
>+			regulator_disable(hpriv->port_regulators[j]);
> disable_target_pwrs:
> 	while (--i >= 0)
> 		if (hpriv->target_pwrs[i])
>@@ -190,9 +202,10 @@ void ahci_platform_disable_regulators(struct
>ahci_host_priv *hpriv)
> 	int i;
> 
> 	for (i = 0; i < hpriv->nports; i++) {
>-		if (!hpriv->target_pwrs[i])
>-			continue;
>-		regulator_disable(hpriv->target_pwrs[i]);
>+		if (hpriv->target_pwrs[i])
>+			regulator_disable(hpriv->target_pwrs[i]);
>+		if (hpriv->port_regulators[i])
>+			regulator_disable(hpriv->port_regulators[i]);
> 	}
> 
> 	if (hpriv->ahci_regulator)
>@@ -291,9 +304,12 @@ static void ahci_platform_put_resources(struct
>device *dev, void *res)
> 	 * SATA device itself. So we can't use devm for automatically
> 	 * releasing them. We have to do it manually here.
> 	 */
>-	for (c = 0; c < hpriv->nports; c++)
>+	for (c = 0; c < hpriv->nports; c++) {
> 		if (hpriv->target_pwrs && hpriv->target_pwrs[c])
> 			regulator_put(hpriv->target_pwrs[c]);
>+		if (hpriv->port_regulators && hpriv->port_regulators[c])
>+			regulator_put(hpriv->port_regulators[c]);
>+	}
> }
> 
>static int ahci_platform_get_phy(struct ahci_host_priv *hpriv, u32
>port,
>@@ -338,6 +354,7 @@ static int ahci_platform_get_regulator(struct
>ahci_host_priv *hpriv, u32 port,
> 				struct device *dev)
> {
> 	struct regulator *target_pwr;
>+	struct regulator *port_regulator;
> 	int rc = 0;
> 
> 	target_pwr = regulator_get_optional(dev, "target");
>@@ -346,6 +363,21 @@ static int ahci_platform_get_regulator(struct
>ahci_host_priv *hpriv, u32 port,
> 		hpriv->target_pwrs[port] = target_pwr;
> 	else
> 		rc = PTR_ERR(target_pwr);
>+	/* Only EPROBE_DEFER is important since it's an optional regulator */
>+	if (rc != -EPROBE_DEFER)
>+		rc = 0;
>+	else
>+		return rc;
>+
>+	port_regulator = regulator_get_optional(dev, "port");
>+
>+	if (!IS_ERR(port_regulator))
>+		hpriv->port_regulators[port] = port_regulator;
>+	else
>+		rc = PTR_ERR(port_regulator);
>+	/* Only EPROBE_DEFER is important since it's an optional regulator */
>+	if (rc != -EPROBE_DEFER)
>+		rc = 0;
> 
> 	return rc;
> }
>@@ -454,6 +486,11 @@ struct ahci_host_priv
>*ahci_platform_get_resources(struct platform_device *pdev)
> 		rc = -ENOMEM;
> 		goto err_out;
> 	}
>+	hpriv->port_regulators = devm_kcalloc(dev, hpriv->nports,
>sizeof(*hpriv->port_regulators), GFP_KERNEL);
>+	if (!hpriv->port_regulators) {
>+		rc = -ENOMEM;
>+		goto err_out;
>+	}
> 
> 	if (child_nodes) {
> 		for_each_child_of_node(dev->of_node, child) {

Re: [PATCH v3 4/9] dt-bindings: ata: ahci-platform: document AHCI reset

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1154 bytes --]

On Fri, Jul 13, 2018 at 11:03:01AM +0000, Corentin Labbe wrote:
> This patch document the new optional resets for ahci node.
> 
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> ---
>  Documentation/devicetree/bindings/ata/ahci-platform.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
> index 5f362af2724c..a5281d7557e3 100644
> --- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
> +++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
> @@ -34,6 +34,8 @@ Optional properties:
>  - phys              : reference to the SATA PHY node
>  - phy-names         : must be "sata-phy"
>  - ahci-supply       : regulator for AHCI controller
> +- resets            : phandle to the reset line of AHCI controller
> +                      If set, must have a reset-names set as "ahci"

This isn't optional, it's required for the R40, and the other don't
need it.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 2/9] dt-bindings: ata: ahci-platform: document ahci-supply

[Rob Herring]

On Fri, Jul 13, 2018 at 11:02:59AM +0000, Corentin Labbe wrote:
> This patch document the new optional ahci-supply.
> 
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> ---
>  Documentation/devicetree/bindings/ata/ahci-platform.txt | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 4/9] dt-bindings: ata: ahci-platform: document AHCI reset

[Rob Herring]

On Fri, Jul 13, 2018 at 11:03:01AM +0000, Corentin Labbe wrote:
> This patch document the new optional resets for ahci node.
> 
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> ---
>  Documentation/devicetree/bindings/ata/ahci-platform.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
> index 5f362af2724c..a5281d7557e3 100644
> --- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
> +++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
> @@ -34,6 +34,8 @@ Optional properties:
>  - phys              : reference to the SATA PHY node
>  - phy-names         : must be "sata-phy"
>  - ahci-supply       : regulator for AHCI controller
> +- resets            : phandle to the reset line of AHCI controller
> +                      If set, must have a reset-names set as "ahci"

A name is pointless if there is only one. Also, the name should be what 
the signal in the block is called which I doubt would be called "ahci". 
A likely name is some form of reset, resetn, etc. which again is 
pointless to name if there is only 1.

If you do have 'reset-names' then that needs to be listed too like 
phy-names.

Rob

Re: [PATCH v3 5/9] dt-bindings: ata: ahci-platform: fix indentation of target-supply

[Rob Herring]

On Fri, Jul 13, 2018 at 11:03:02AM +0000, Corentin Labbe wrote:
> This patch fix the indentation of target-supply's ':'.
> 
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> ---
>  Documentation/devicetree/bindings/ata/ahci-platform.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

This one should come before the others. Fixes/restructuring before 
features.

Acked-by: Rob Herring <robh@kernel.org>

> 
> diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
> index a5281d7557e3..47652521a6ed 100644
> --- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
> +++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
> @@ -50,7 +50,7 @@ Sub-nodes required properties:
>  - reg		    : the port number
>  And at least one of the following properties:
>  - phys		    : reference to the SATA PHY node
> -- target-supply    : regulator for SATA target power
> +- target-supply     : regulator for SATA target power
>  
>  Examples:
>          sata@ffe08000 {
> -- 
> 2.16.4
> 

Re: [PATCH v3 7/9] dt-bindings: ata: ahci-platform: document port-supply

[Rob Herring]

On Fri, Jul 13, 2018 at 11:03:04AM +0000, Corentin Labbe wrote:
> This patch document the new optional port-supply
> 
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> ---
>  Documentation/devicetree/bindings/ata/ahci-platform.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
> index 47652521a6ed..40a636b9851e 100644
> --- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
> +++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
> @@ -31,6 +31,7 @@ Optional properties:
>  - dma-coherent      : Present if dma operations are coherent
>  - clocks            : a list of phandle + clock specifier pairs
>  - target-supply     : regulator for SATA target power
> +- port-supply       : regulator for SATA port

Humm, I'm pretty sure that's what target-supply was supposed to be. If 
anything, need to clearly define what is what.

How does this work with multiple ports?

>  - phys              : reference to the SATA PHY node
>  - phy-names         : must be "sata-phy"
>  - ahci-supply       : regulator for AHCI controller

Also, please group all the supplies together.

> @@ -51,6 +52,7 @@ Sub-nodes required properties:
>  And at least one of the following properties:
>  - phys		    : reference to the SATA PHY node
>  - target-supply     : regulator for SATA target power
> +- port-supply       : regulator for SATA port
>  
>  Examples:
>          sata@ffe08000 {
> -- 
> 2.16.4
> 

Re: [PATCH v3 7/9] dt-bindings: ata: ahci-platform: document port-supply

[Icenowy Zheng]

于 2018年7月16日 GMT+08:00 下午11:59:38, Rob Herring <robh@kernel.org> 写到:
>On Fri, Jul 13, 2018 at 11:03:04AM +0000, Corentin Labbe wrote:
>> This patch document the new optional port-supply
>> 
>> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
>> ---
>>  Documentation/devicetree/bindings/ata/ahci-platform.txt | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt
>b/Documentation/devicetree/bindings/ata/ahci-platform.txt
>> index 47652521a6ed..40a636b9851e 100644
>> --- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
>> +++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
>> @@ -31,6 +31,7 @@ Optional properties:
>>  - dma-coherent      : Present if dma operations are coherent
>>  - clocks            : a list of phandle + clock specifier pairs
>>  - target-supply     : regulator for SATA target power
>> +- port-supply       : regulator for SATA port
>
>Humm, I'm pretty sure that's what target-supply was supposed to be. If 
>anything, need to clearly define what is what.
>
>How does this work with multiple ports?

He wrongly named it. It's for PHY.

>
>>  - phys              : reference to the SATA PHY node
>>  - phy-names         : must be "sata-phy"
>>  - ahci-supply       : regulator for AHCI controller
>
>Also, please group all the supplies together.
>
>> @@ -51,6 +52,7 @@ Sub-nodes required properties:
>>  And at least one of the following properties:
>>  - phys		    : reference to the SATA PHY node
>>  - target-supply     : regulator for SATA target power
>> +- port-supply       : regulator for SATA port
>>  
>>  Examples:
>>          sata@ffe08000 {
>> -- 
>> 2.16.4
>> 
>
>_______________________________________________
>linux-arm-kernel mailing list
>linux-arm-kernel@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 4/9] dt-bindings: ata: ahci-platform: document AHCI reset

[LABBE Corentin]

On Mon, Jul 16, 2018 at 09:54:16AM -0600, Rob Herring wrote:
> On Fri, Jul 13, 2018 at 11:03:01AM +0000, Corentin Labbe wrote:
> > This patch document the new optional resets for ahci node.
> > 
> > Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> > ---
> >  Documentation/devicetree/bindings/ata/ahci-platform.txt | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
> > index 5f362af2724c..a5281d7557e3 100644
> > --- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
> > +++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
> > @@ -34,6 +34,8 @@ Optional properties:
> >  - phys              : reference to the SATA PHY node
> >  - phy-names         : must be "sata-phy"
> >  - ahci-supply       : regulator for AHCI controller
> > +- resets            : phandle to the reset line of AHCI controller
> > +                      If set, must have a reset-names set as "ahci"
> 
> A name is pointless if there is only one. Also, the name should be what 
> the signal in the block is called which I doubt would be called "ahci". 
> A likely name is some form of reset, resetn, etc. which again is 
> pointless to name if there is only 1.
> 
> If you do have 'reset-names' then that needs to be listed too like 
> phy-names.
> 

Since the ahci-platform code is common to lots of different drivers, I must have a name otherwise I could grab a reset which wasnt for me.
See the revert of f0f56716fc3e ("ata: ahci-platform: add reset control support") for example.

So the code need to grab a name, and since this name could be used by other people I took the generic "ahci".

In my case (R40), the reset is called sata, so I will change reset-names to "sata" in my next serie.

Regards

Re: [PATCH v3 0/9] ata: ahci_platform: support allwinner R40 AHCI

[Simon Baatz]

Hi Corentin,

On Fri, Jul 13, 2018 at 11:02:57AM +0000, Corentin Labbe wrote:
> Hello
> 
> This patchset add support for allwinner R40 AHCI controller.
> 
> This controller need two regulator and one reset which is unsupported
> for the moment on ahci_platform.
> 
> So this patchset add support for reset and regulator for AHCI
> controller.
> It add also support for non-target regulator per SATA port.
> 
> The whole patchset is tested on sun8i-r40-bananapi-m2-ultra and
> on sun7i-a20-cubieboard2 which doesnt have any of the ressources added
> by this serie, so no regression should come with it.

I tested on sun8i-v40-bananapi-m2-berry (with an adapted device
tree).  I got it to work, but only if the U-Boot has support for AHCI
already (from the Banana Pi M2 Ultra support).  With an U-Boot that
hasn't, the system hangs at boot (probably when probing ahci-sunxi). 

Did you test this case on the M2 Ultra board?  (I think SATA should
work the same on both boards, but there may be subtle differences)

Perhaps I did not apply all necessary patches. I took this series and
your minor fixes series for ahci_platform.