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
R40 have a sata controller which is the same as A20.
This patch adds a DT node for it.
Signed-off-by: Icenowy Zheng <[email protected]>
Signed-off-by: Corentin Labbe <[email protected]>
---
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
This patch enable the AHCI controller.
Since this controller need two regulator, this patch add them.
Signed-off-by: Corentin Labbe <[email protected]>
---
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 = <®_dldo4>;
+ status = "okay";
+};
+
&de {
status = "okay";
};
@@ -251,6 +256,22 @@
regulator-name = "vcc-wifi";
};
+®_dldo4 {
+ regulator-min-microvolt = <2500000>;
+ regulator-max-microvolt = <2500000>;
+ regulator-name = "vdd2v5-sata";
+};
+
+®_eldo3 {
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ regulator-name = "vdd1v2-sata";
+};
+
+&sata_port0 {
+ port-supply = <®_eldo3>;
+};
+
&tcon_tv0 {
status = "okay";
};
--
2.16.4
This patch document the new optional port-supply
Signed-off-by: Corentin Labbe <[email protected]>
---
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
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 <[email protected]>
---
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
This patch fix the indentation of target-supply's ':'.
Signed-off-by: Corentin Labbe <[email protected]>
---
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
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 <[email protected]>
---
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
This patch document the new optional ahci-supply.
Signed-off-by: Corentin Labbe <[email protected]>
---
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
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 <[email protected]>
---
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
This patch document the new optional resets for ahci node.
Signed-off-by: Corentin Labbe <[email protected]>
---
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
于 2018年7月13日 GMT+08:00 下午7:03:03, Corentin Labbe <[email protected]> 写到:
>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 <[email protected]>
>---
> 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) {
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> ---
> Documentation/devicetree/bindings/ata/ahci-platform.txt | 1 +
> 1 file changed, 1 insertion(+)
Reviewed-by: Rob Herring <[email protected]>
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> ---
> 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 <[email protected]>
>
> 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
>
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 <[email protected]>
> ---
> 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
>
于 2018年7月16日 GMT+08:00 下午11:59:38, Rob Herring <[email protected]> 写到:
>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 <[email protected]>
>> ---
>> 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
>[email protected]
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
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 <[email protected]>
> > ---
> > 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
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.