[PATCH v2 0/4] sun8i: r40: add AHCI

[PATCH v2 0/4] sun8i: r40: add AHCI

[Corentin Labbe]

Hello

With Moeicenowy's agreement, I have take leadership ot this patchset.

There are no really changes appart renaming struct quirck to variant.

Since the last serie is really old, I will answer comment here.
The two regulator (1.2 and 2.5V) are not for the PHY since:
- nothing in the schematic said that they are for the PHY, they seems
  only for controller
- all other AHCI driver use 5V for the target/PHY (vs 1.2/2.5 which
  cannot be used for target)

Furthermore, the AHCI binding support only one regulator per PHY, so
using the "target" regulator is out of question for registring this two
non-phy regulator.

I hope this answer all comments done on last version.

Regards

Corentin Labbe (3):
  ata: ahci_sunxi: add support for R40 SATA controller
  ARM: dts: sun8i: r40: add sata node
  ARM: dts: sun8i: sun8i-r40-bananapi-m2-ultra: enable AHCI

Icenowy Zheng (1):
  dt-bindings: add binding for Allwinner R40 SATA AHCI controller

 .../devicetree/bindings/ata/ahci-platform.txt      |   1 -
 .../bindings/ata/allwinner,sun4i-a10-ahci.txt      |  40 +++++++
 arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts  |  18 +++
 arch/arm/boot/dts/sun8i-r40.dtsi                   |   9 ++
 drivers/ata/ahci_sunxi.c                           | 124 ++++++++++++++++++++-
 5 files changed, 188 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/ata/allwinner,sun4i-a10-ahci.txt

-- 
2.16.4

[PATCH v2 1/4] dt-bindings: add binding for Allwinner R40 SATA AHCI controller

[Corentin Labbe]

From: Icenowy Zheng <icenowy@aosc.io>

The Allwinner R40 SoC contains a SATA AHCI controller like the one in
A10/A20 SoCs, however a reset control and two power supplies are added
to it.

Add a binding document for it.

As a dedicated binding document is needed now for the A10/A20/R40 AHCI
controller, drop the A10 compatible line from generic platform AHCI
controller binding document.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 .../devicetree/bindings/ata/ahci-platform.txt      |  1 -
 .../bindings/ata/allwinner,sun4i-a10-ahci.txt      | 40 ++++++++++++++++++++++
 2 files changed, 40 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/ata/allwinner,sun4i-a10-ahci.txt

diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
index c760ecb81381..1bea4b5ef9fd 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
@@ -9,7 +9,6 @@ PHYs.
 
 Required properties:
 - compatible        : compatible string, one of:
-  - "allwinner,sun4i-a10-ahci"
   - "brcm,iproc-ahci"
   - "hisilicon,hisi-ahci"
   - "cavium,octeon-7130-ahci"
diff --git a/Documentation/devicetree/bindings/ata/allwinner,sun4i-a10-ahci.txt b/Documentation/devicetree/bindings/ata/allwinner,sun4i-a10-ahci.txt
new file mode 100644
index 000000000000..0eea78c14ad3
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/allwinner,sun4i-a10-ahci.txt
@@ -0,0 +1,40 @@
+Allwinner A10/A20/R40 SoC SATA AHCI Controller
+
+Required properties:
+- compatible        : compatible string, one of:
+  - "allwinner,sun4i-a10-ahci"
+  - "allwinner,sun8i-r40-ahci"
+- interrupts        : the SATA IRQ
+- reg               : the register mapping
+- clocks            : the clocks needed by SATA controller, usually contains
+		      an AHB clock and a mod clock
+
+Optional properties:
+- target-supply     : regulator for SATA target power
+
+Required properties for the following compatibles:
+  - "allwinner,sun8i-r40-ahci"
+- resets            : the reset control needed by SATA controller
+- vdd1v2-supply     : regulator for SATA controller's 1.2V VDD
+- vdd2v5-supply     : regulator for SATA controller's 2.5V VDD
+
+
+Examples for A10:
+	ahci: sata@1c18000 {
+		compatible = "allwinner,sun4i-a10-ahci";
+		reg = <0x01c18000 0x1000>;
+		interrupts = <56>;
+		clocks = <&pll6 0>, <&ahb_gates 25>;
+		target-supply = <&reg_ahci_5v>;
+	};
+
+Examples for R40:
+	ahci: sata@1c18000 {
+		compatible = "allwinner,sun8i-r40-ahci";
+		reg = <0x01c18000 0x1000>;
+		interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&ccu CLK_SATA>, <&ccu CLK_BUS_SATA>;
+		resets = <&ccu RST_BUS_SATA>;
+		vdd1v2-supply = <&reg_eldo3>;
+		vdd2v5-supply = <&reg_dldo4>;
+	};
-- 
2.16.4

Re: [PATCH v2 1/4] dt-bindings: add binding for Allwinner R40 SATA AHCI controller

[Rob Herring]

On Mon, Jul 09, 2018 at 03:20:55PM +0000, Corentin Labbe wrote:
> From: Icenowy Zheng <icenowy@aosc.io>
> 
> The Allwinner R40 SoC contains a SATA AHCI controller like the one in
> A10/A20 SoCs, however a reset control and two power supplies are added
> to it.
> 
> Add a binding document for it.
> 
> As a dedicated binding document is needed now for the A10/A20/R40 AHCI
> controller, drop the A10 compatible line from generic platform AHCI
> controller binding document.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> ---
>  .../devicetree/bindings/ata/ahci-platform.txt      |  1 -
>  .../bindings/ata/allwinner,sun4i-a10-ahci.txt      | 40 ++++++++++++++++++++++
>  2 files changed, 40 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/ata/allwinner,sun4i-a10-ahci.txt
> 
> diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
> index c760ecb81381..1bea4b5ef9fd 100644
> --- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
> +++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
> @@ -9,7 +9,6 @@ PHYs.
>  
>  Required properties:
>  - compatible        : compatible string, one of:
> -  - "allwinner,sun4i-a10-ahci"
>    - "brcm,iproc-ahci"
>    - "hisilicon,hisi-ahci"
>    - "cavium,octeon-7130-ahci"
> diff --git a/Documentation/devicetree/bindings/ata/allwinner,sun4i-a10-ahci.txt b/Documentation/devicetree/bindings/ata/allwinner,sun4i-a10-ahci.txt
> new file mode 100644
> index 000000000000..0eea78c14ad3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/allwinner,sun4i-a10-ahci.txt
> @@ -0,0 +1,40 @@
> +Allwinner A10/A20/R40 SoC SATA AHCI Controller
> +
> +Required properties:
> +- compatible        : compatible string, one of:
> +  - "allwinner,sun4i-a10-ahci"
> +  - "allwinner,sun8i-r40-ahci"
> +- interrupts        : the SATA IRQ
> +- reg               : the register mapping
> +- clocks            : the clocks needed by SATA controller, usually contains
> +		      an AHB clock and a mod clock

usually?

Need to specify the order. The examples look reversed of what you have 
here.

> +
> +Optional properties:
> +- target-supply     : regulator for SATA target power
> +
> +Required properties for the following compatibles:
> +  - "allwinner,sun8i-r40-ahci"
> +- resets            : the reset control needed by SATA controller
> +- vdd1v2-supply     : regulator for SATA controller's 1.2V VDD
> +- vdd2v5-supply     : regulator for SATA controller's 2.5V VDD
> +
> +
> +Examples for A10:
> +	ahci: sata@1c18000 {
> +		compatible = "allwinner,sun4i-a10-ahci";
> +		reg = <0x01c18000 0x1000>;
> +		interrupts = <56>;
> +		clocks = <&pll6 0>, <&ahb_gates 25>;
> +		target-supply = <&reg_ahci_5v>;
> +	};
> +
> +Examples for R40:
> +	ahci: sata@1c18000 {
> +		compatible = "allwinner,sun8i-r40-ahci";
> +		reg = <0x01c18000 0x1000>;
> +		interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&ccu CLK_SATA>, <&ccu CLK_BUS_SATA>;
> +		resets = <&ccu RST_BUS_SATA>;
> +		vdd1v2-supply = <&reg_eldo3>;
> +		vdd2v5-supply = <&reg_dldo4>;
> +	};
> -- 
> 2.16.4
> 

[PATCH v2 2/4] ata: ahci_sunxi: add support for R40 SATA controller

[Corentin Labbe]

Allwinner R40 SoC has an AHCI SATA controller like the one in A10/A20,
but with a reset control and two dedicated VDD pins for this controller
(one 1.2v and one 2.5v).

Add support for it.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 drivers/ata/ahci_sunxi.c | 124 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 121 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/ahci_sunxi.c b/drivers/ata/ahci_sunxi.c
index b26437430163..8b1bc04c0435 100644
--- a/drivers/ata/ahci_sunxi.c
+++ b/drivers/ata/ahci_sunxi.c
@@ -25,6 +25,7 @@
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
+#include <linux/reset.h>
 #include "ahci.h"
 
 #define DRV_NAME "ahci-sunxi"
@@ -58,6 +59,19 @@ MODULE_PARM_DESC(enable_pmp,
 #define AHCI_P0PHYCR	0x0178
 #define AHCI_P0PHYSR	0x017c
 
+struct ahci_sunxi_variant {
+	bool has_reset;
+	bool has_vdd1v2;
+	bool has_vdd2v5;
+};
+
+struct ahci_sunxi_data {
+	const struct ahci_sunxi_variant *variant;
+	struct reset_control *reset;
+	struct regulator *vdd1v2;
+	struct regulator *vdd2v5;
+};
+
 static void sunxi_clrbits(void __iomem *reg, u32 clr_val)
 {
 	u32 reg_val;
@@ -179,17 +193,75 @@ static int ahci_sunxi_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct ahci_host_priv *hpriv;
+	struct ahci_sunxi_data *data;
 	int rc;
 
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->variant = of_device_get_match_data(dev);
+	if (!data->variant)
+		return -EINVAL;
+
+	if (data->variant->has_reset) {
+		data->reset = devm_reset_control_get(dev, NULL);
+		if (IS_ERR(data->reset)) {
+			dev_err(dev, "Failed to get reset\n");
+			return PTR_ERR(data->reset);
+		}
+	}
+
+	if (data->variant->has_vdd1v2) {
+		data->vdd1v2 = devm_regulator_get(dev, "vdd1v2");
+		if (IS_ERR(data->vdd1v2)) {
+			rc = PTR_ERR(data->vdd1v2);
+			if (rc == -EPROBE_DEFER)
+				return -EPROBE_DEFER;
+			dev_err(dev, "Failed to get 1.2v VDD regulator\n");
+			return rc;
+		}
+	}
+
+	if (data->variant->has_vdd2v5) {
+		data->vdd2v5 = devm_regulator_get(dev, "vdd2v5");
+		if (IS_ERR(data->vdd2v5)) {
+			rc = PTR_ERR(data->vdd2v5);
+			if (rc == -EPROBE_DEFER)
+				return -EPROBE_DEFER;
+			dev_err(dev, "Failed to get 2.5v VDD regulator\n");
+			return rc;
+		}
+	}
+
 	hpriv = ahci_platform_get_resources(pdev);
 	if (IS_ERR(hpriv))
 		return PTR_ERR(hpriv);
 
+	hpriv->plat_data = data;
 	hpriv->start_engine = ahci_sunxi_start_engine;
 
+	if (data->variant->has_vdd1v2) {
+		rc = regulator_enable(data->vdd1v2);
+		if (rc)
+			return rc;
+	}
+
+	if (data->variant->has_vdd2v5) {
+		rc = regulator_enable(data->vdd2v5);
+		if (rc)
+			goto disable_vdd1v2;
+	}
+
+	if (data->variant->has_reset) {
+		rc = reset_control_deassert(data->reset);
+		if (rc)
+			goto disable_vdd2v5;
+	}
+
 	rc = ahci_platform_enable_resources(hpriv);
 	if (rc)
-		return rc;
+		goto assert_reset;
 
 	rc = ahci_sunxi_phy_init(dev, hpriv->mmio);
 	if (rc)
@@ -215,6 +287,35 @@ static int ahci_sunxi_probe(struct platform_device *pdev)
 
 disable_resources:
 	ahci_platform_disable_resources(hpriv);
+assert_reset:
+	if (data->variant->has_reset)
+		reset_control_assert(data->reset);
+disable_vdd2v5:
+	if (data->variant->has_vdd2v5)
+		regulator_disable(data->vdd2v5);
+disable_vdd1v2:
+	if (data->variant->has_vdd1v2)
+		regulator_disable(data->vdd1v2);
+	return rc;
+}
+
+static int ahci_sunxi_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct ata_host *host = dev_get_drvdata(dev);
+	struct ahci_host_priv *hpriv = host->private_data;
+	struct ahci_sunxi_data *data = hpriv->plat_data;
+	int rc;
+
+	rc = ata_platform_remove_one(pdev);
+
+	if (data->variant->has_reset)
+		reset_control_assert(data->reset);
+	if (data->variant->has_vdd2v5)
+		regulator_disable(data->vdd2v5);
+	if (data->variant->has_vdd1v2)
+		regulator_disable(data->vdd1v2);
+
 	return rc;
 }
 
@@ -248,15 +349,32 @@ static int ahci_sunxi_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(ahci_sunxi_pm_ops, ahci_platform_suspend,
 			 ahci_sunxi_resume);
 
+static const struct ahci_sunxi_variant sun4i_a10_ahci_variant = {
+	/* Nothing special */
+};
+
+static const struct ahci_sunxi_variant sun8i_r40_ahci_variant = {
+	.has_reset = true,
+	.has_vdd1v2 = true,
+	.has_vdd2v5 = true,
+};
+
 static const struct of_device_id ahci_sunxi_of_match[] = {
-	{ .compatible = "allwinner,sun4i-a10-ahci", },
+	{
+		.compatible = "allwinner,sun4i-a10-ahci",
+		.data = &sun4i_a10_ahci_variant,
+	},
+	{
+		.compatible = "allwinner,sun8i-r40-ahci",
+		.data = &sun8i_r40_ahci_variant,
+	},
 	{ },
 };
 MODULE_DEVICE_TABLE(of, ahci_sunxi_of_match);
 
 static struct platform_driver ahci_sunxi_driver = {
 	.probe = ahci_sunxi_probe,
-	.remove = ata_platform_remove_one,
+	.remove = ahci_sunxi_remove,
 	.driver = {
 		.name = DRV_NAME,
 		.of_match_table = ahci_sunxi_of_match,
-- 
2.16.4

[PATCH v2 3/4] 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 | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
index e366b76b47ce..df5b90faca59 100644
--- a/arch/arm/boot/dts/sun8i-r40.dtsi
+++ b/arch/arm/boot/dts/sun8i-r40.dtsi
@@ -551,6 +551,15 @@
 			#size-cells = <0>;
 		};
 
+		ahci: sata@1c18000 {
+			compatible = "allwinner,sun8i-r40-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>;
+			status = "disabled";
+		};
+
 		gmac: ethernet@1c50000 {
 			compatible = "allwinner,sun8i-r40-gmac";
 			syscon = <&ccu>;
-- 
2.16.4

[PATCH v2 4/4] 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: Icenowy Zheng <icenowy@aosc.io>
Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts | 18 ++++++++++++++++++
 1 file changed, 18 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 4f3d583183dc..ceb38739f1d8 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,12 @@
 	};
 };
 
+&ahci {
+	vdd1v2-supply = <&reg_dldo4>;
+	vdd2v5-supply = <&reg_eldo3>;
+	status = "okay";
+};
+
 &de {
 	status = "okay";
 };
@@ -251,6 +257,18 @@
 	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";
+};
+
 &tcon_top_hdmi_in_tcon_tv0 {
 	remote-endpoint = <&tcon_tv0_out_tcon_top>;
 };
-- 
2.16.4

Re: [PATCH v2 0/4] sun8i: r40: add AHCI

[Icenowy Zheng]

于 2018年7月9日 GMT+08:00 下午11:20:54, Corentin Labbe <clabbe@baylibre.com> 写到:
>Hello
>
>With Moeicenowy's agreement, I have take leadership ot this patchset.
>
>There are no really changes appart renaming struct quirck to variant.
>
>Since the last serie is really old, I will answer comment here.
>The two regulator (1.2 and 2.5V) are not for the PHY since:
>- nothing in the schematic said that they are for the PHY, they seems
>  only for controller
>- all other AHCI driver use 5V for the target/PHY (vs 1.2/2.5 which
>  cannot be used for target)

Target is not equal to PHY. Target means the supply
of the disk, which can be usually 5v (for 2.5" HDD) or
12v (3.5" HDD).

By reading Wikipedia articles  about SATA and LVDS, I assume
2.5V is for PHY and 1.2V is for internal digital logic (VDD-SYS
is commonly 1.2V on 40nm Allwinner SoCs; 2.5V VDD can be
used to efficiently deliver ~1.2V LVDS.)

P.S. VDD-SATA and VDD25-SATA also exist on A20, and by checking
Banana Pi M1 (the original Banana Pi) schematics, VDD-SATA
is connected to common VDD-SYS (called INTVDD on the
schematics) and VDD25-SATA is connected to an always-on
fixed LDO, maybe due to the lack of power outputs on AXP209.

>
>Furthermore, the AHCI binding support only one regulator per PHY, so
>using the "target" regulator is out of question for registring this two
>non-phy regulator.
>
>I hope this answer all comments done on last version.
>
>Regards
>
>Corentin Labbe (3):
>  ata: ahci_sunxi: add support for R40 SATA controller
>  ARM: dts: sun8i: r40: add sata node
>  ARM: dts: sun8i: sun8i-r40-bananapi-m2-ultra: enable AHCI
>
>Icenowy Zheng (1):
>  dt-bindings: add binding for Allwinner R40 SATA AHCI controller
>
> .../devicetree/bindings/ata/ahci-platform.txt      |   1 -
> .../bindings/ata/allwinner,sun4i-a10-ahci.txt      |  40 +++++++
> arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts  |  18 +++
> arch/arm/boot/dts/sun8i-r40.dtsi                   |   9 ++
>drivers/ata/ahci_sunxi.c                           | 124
>++++++++++++++++++++-
> 5 files changed, 188 insertions(+), 4 deletions(-)
>create mode 100644
>Documentation/devicetree/bindings/ata/allwinner,sun4i-a10-ahci.txt

Re: [PATCH v2 0/4] sun8i: r40: add AHCI

[LABBE Corentin]

On Mon, Jul 09, 2018 at 11:44:20PM +0800, Icenowy Zheng wrote:
> 
> 
> 于 2018年7月9日 GMT+08:00 下午11:20:54, Corentin Labbe <clabbe@baylibre.com> 写到:
> >Hello
> >
> >With Moeicenowy's agreement, I have take leadership ot this patchset.
> >
> >There are no really changes appart renaming struct quirck to variant.
> >
> >Since the last serie is really old, I will answer comment here.
> >The two regulator (1.2 and 2.5V) are not for the PHY since:
> >- nothing in the schematic said that they are for the PHY, they seems
> >  only for controller
> >- all other AHCI driver use 5V for the target/PHY (vs 1.2/2.5 which
> >  cannot be used for target)
> 
> Target is not equal to PHY. Target means the supply
> of the disk, which can be usually 5v (for 2.5" HDD) or
> 12v (3.5" HDD).
> 
> By reading Wikipedia articles  about SATA and LVDS, I assume
> 2.5V is for PHY and 1.2V is for internal digital logic (VDD-SYS
> is commonly 1.2V on 40nm Allwinner SoCs; 2.5V VDD can be
> used to efficiently deliver ~1.2V LVDS.)
> 
> P.S. VDD-SATA and VDD25-SATA also exist on A20, and by checking
> Banana Pi M1 (the original Banana Pi) schematics, VDD-SATA
> is connected to common VDD-SYS (called INTVDD on the
> schematics) and VDD25-SATA is connected to an always-on
> fixed LDO, maybe due to the lack of power outputs on AXP209.
> 

So we still need to add a regulator on the controller and add an optionnal "phy" regulator for AHCI port.

Re: [PATCH v2 0/4] sun8i: r40: add AHCI

[Icenowy Zheng]

于 2018年7月10日 GMT+08:00 下午9:05:17, LABBE Corentin <clabbe@baylibre.com> 写到:
>On Mon, Jul 09, 2018 at 11:44:20PM +0800, Icenowy Zheng wrote:
>> 
>> 
>> 于 2018年7月9日 GMT+08:00 下午11:20:54, Corentin Labbe
><clabbe@baylibre.com> 写到:
>> >Hello
>> >
>> >With Moeicenowy's agreement, I have take leadership ot this
>patchset.
>> >
>> >There are no really changes appart renaming struct quirck to
>variant.
>> >
>> >Since the last serie is really old, I will answer comment here.
>> >The two regulator (1.2 and 2.5V) are not for the PHY since:
>> >- nothing in the schematic said that they are for the PHY, they
>seems
>> >  only for controller
>> >- all other AHCI driver use 5V for the target/PHY (vs 1.2/2.5 which
>> >  cannot be used for target)
>> 
>> Target is not equal to PHY. Target means the supply
>> of the disk, which can be usually 5v (for 2.5" HDD) or
>> 12v (3.5" HDD).
>> 
>> By reading Wikipedia articles  about SATA and LVDS, I assume
>> 2.5V is for PHY and 1.2V is for internal digital logic (VDD-SYS
>> is commonly 1.2V on 40nm Allwinner SoCs; 2.5V VDD can be
>> used to efficiently deliver ~1.2V LVDS.)
>> 
>> P.S. VDD-SATA and VDD25-SATA also exist on A20, and by checking
>> Banana Pi M1 (the original Banana Pi) schematics, VDD-SATA
>> is connected to common VDD-SYS (called INTVDD on the
>> schematics) and VDD25-SATA is connected to an always-on
>> fixed LDO, maybe due to the lack of power outputs on AXP209.
>> 
>
>So we still need to add a regulator on the controller and add an
>optionnal "phy" regulator for AHCI port.

No, phy is not a part of the port. From the perspective of
the users, phy is part of the controller.

I still suggest generic multiple regulator support, to
reduce sunxi-specified code.

>
>
>_______________________________________________
>linux-arm-kernel mailing list
>linux-arm-kernel@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 0/4] sun8i: r40: add AHCI

[Maxime Ripard]

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

On Mon, Jul 09, 2018 at 03:20:54PM +0000, Corentin Labbe wrote:
> Hello
> 
> With Moeicenowy's agreement, I have take leadership ot this patchset.
> 
> There are no really changes appart renaming struct quirck to variant.
> 
> Since the last serie is really old, I will answer comment here.
> The two regulator (1.2 and 2.5V) are not for the PHY since:
> - nothing in the schematic said that they are for the PHY, they seems
>   only for controller
> - all other AHCI driver use 5V for the target/PHY (vs 1.2/2.5 which
>   cannot be used for target)

That's a pretty bad thing to do, especially for old series. You just
dropped all the context that you reply to, and there's no way for any
reviewer to tell if your answers make any kind of sense, or addresses
any question one might have had.

> Furthermore, the AHCI binding support only one regulator per PHY, so
> using the "target" regulator is out of question for registring this two
> non-phy regulator.

Nothing is "out of question", we have the source code and can change
it if needed.

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 v2 0/4] sun8i: r40: add AHCI

[Icenowy Zheng]

于 2018年7月10日 GMT+08:00 下午8:29:22, Maxime Ripard <maxime.ripard@bootlin.com> 写到:
>On Mon, Jul 09, 2018 at 03:20:54PM +0000, Corentin Labbe wrote:
>> Hello
>> 
>> With Moeicenowy's agreement, I have take leadership ot this patchset.
>> 
>> There are no really changes appart renaming struct quirck to variant.
>> 
>> Since the last serie is really old, I will answer comment here.
>> The two regulator (1.2 and 2.5V) are not for the PHY since:
>> - nothing in the schematic said that they are for the PHY, they seems
>>   only for controller
>> - all other AHCI driver use 5V for the target/PHY (vs 1.2/2.5 which
>>   cannot be used for target)
>
>That's a pretty bad thing to do, especially for old series. You just
>dropped all the context that you reply to, and there's no way for any
>reviewer to tell if your answers make any kind of sense, or addresses
>any question one might have had.
>
>> Furthermore, the AHCI binding support only one regulator per PHY, so
>> using the "target" regulator is out of question for registring this
>two
>> non-phy regulator.
>
>Nothing is "out of question", we have the source code and can change
>it if needed.

The main problem is that he misunderstood the meaning
of target regulator -- it's not for PHY but for disk. And if a
device is designed to accept both 2.5" and 3.5" HDD at
the same port multiple target regulator might really
be useful -- one 5v and one 12v.

>
>Maxime

Re: [PATCH v2 0/4] sun8i: r40: add AHCI

[LABBE Corentin]

On Tue, Jul 10, 2018 at 02:29:22PM +0200, Maxime Ripard wrote:
> On Mon, Jul 09, 2018 at 03:20:54PM +0000, Corentin Labbe wrote:
> > Hello
> > 
> > With Moeicenowy's agreement, I have take leadership ot this patchset.
> > 
> > There are no really changes appart renaming struct quirck to variant.
> > 
> > Since the last serie is really old, I will answer comment here.
> > The two regulator (1.2 and 2.5V) are not for the PHY since:
> > - nothing in the schematic said that they are for the PHY, they seems
> >   only for controller
> > - all other AHCI driver use 5V for the target/PHY (vs 1.2/2.5 which
> >   cannot be used for target)
> 
> That's a pretty bad thing to do, especially for old series. You just
> dropped all the context that you reply to, and there's no way for any
> reviewer to tell if your answers make any kind of sense, or addresses
> any question one might have had.
> 

I apologize, i feared to hack a mail with the messageid.
Next time I will

> > Furthermore, the AHCI binding support only one regulator per PHY, so
> > using the "target" regulator is out of question for registring this two
> > non-phy regulator.
> 
> Nothing is "out of question", we have the source code and can change
> it if needed.
> 

Ok, I will try with a generic AHCI platform solution.

Regards