LKML Archive on lore.kernel.org
 help / Atom feed
* [PATCH v2 0/4] sun8i: r40: add AHCI
@ 2018-07-09 15:20 Corentin Labbe
  2018-07-09 15:20 ` [PATCH v2 1/4] dt-bindings: add binding for Allwinner R40 SATA AHCI controller Corentin Labbe
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Corentin Labbe @ 2018-07-09 15:20 UTC (permalink / raw)
  To: linux, mark.rutland, maxime.ripard, robh+dt, tj, wens
  Cc: devicetree, linux-arm-kernel, linux-ide, linux-kernel,
	linux-sunxi, icenowy, 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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/4] dt-bindings: add binding for Allwinner R40 SATA AHCI controller
  2018-07-09 15:20 [PATCH v2 0/4] sun8i: r40: add AHCI Corentin Labbe
@ 2018-07-09 15:20 ` Corentin Labbe
  2018-07-11 19:12   ` Rob Herring
  2018-07-09 15:20 ` [PATCH v2 2/4] ata: ahci_sunxi: add support for R40 SATA controller Corentin Labbe
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Corentin Labbe @ 2018-07-09 15:20 UTC (permalink / raw)
  To: linux, mark.rutland, maxime.ripard, robh+dt, tj, wens
  Cc: devicetree, linux-arm-kernel, linux-ide, linux-kernel,
	linux-sunxi, icenowy, 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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 2/4] ata: ahci_sunxi: add support for R40 SATA controller
  2018-07-09 15:20 [PATCH v2 0/4] sun8i: r40: add AHCI Corentin Labbe
  2018-07-09 15:20 ` [PATCH v2 1/4] dt-bindings: add binding for Allwinner R40 SATA AHCI controller Corentin Labbe
@ 2018-07-09 15:20 ` Corentin Labbe
  2018-07-09 15:20 ` [PATCH v2 3/4] ARM: dts: sun8i: r40: add sata node Corentin Labbe
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Corentin Labbe @ 2018-07-09 15:20 UTC (permalink / raw)
  To: linux, mark.rutland, maxime.ripard, robh+dt, tj, wens
  Cc: devicetree, linux-arm-kernel, linux-ide, linux-kernel,
	linux-sunxi, icenowy, 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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 3/4] ARM: dts: sun8i: r40: add sata node
  2018-07-09 15:20 [PATCH v2 0/4] sun8i: r40: add AHCI Corentin Labbe
  2018-07-09 15:20 ` [PATCH v2 1/4] dt-bindings: add binding for Allwinner R40 SATA AHCI controller Corentin Labbe
  2018-07-09 15:20 ` [PATCH v2 2/4] ata: ahci_sunxi: add support for R40 SATA controller Corentin Labbe
@ 2018-07-09 15:20 ` Corentin Labbe
  2018-07-09 15:20 ` [PATCH v2 4/4] ARM: dts: sun8i: sun8i-r40-bananapi-m2-ultra: enable AHCI Corentin Labbe
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Corentin Labbe @ 2018-07-09 15:20 UTC (permalink / raw)
  To: linux, mark.rutland, maxime.ripard, robh+dt, tj, wens
  Cc: devicetree, linux-arm-kernel, linux-ide, linux-kernel,
	linux-sunxi, icenowy, 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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 4/4] ARM: dts: sun8i: sun8i-r40-bananapi-m2-ultra: enable AHCI
  2018-07-09 15:20 [PATCH v2 0/4] sun8i: r40: add AHCI Corentin Labbe
                   ` (2 preceding siblings ...)
  2018-07-09 15:20 ` [PATCH v2 3/4] ARM: dts: sun8i: r40: add sata node Corentin Labbe
@ 2018-07-09 15:20 ` Corentin Labbe
  2018-07-09 15:44 ` [PATCH v2 0/4] sun8i: r40: add AHCI Icenowy Zheng
  2018-07-10 12:29 ` Maxime Ripard
  5 siblings, 0 replies; 12+ messages in thread
From: Corentin Labbe @ 2018-07-09 15:20 UTC (permalink / raw)
  To: linux, mark.rutland, maxime.ripard, robh+dt, tj, wens
  Cc: devicetree, linux-arm-kernel, linux-ide, linux-kernel,
	linux-sunxi, icenowy, 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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/4] sun8i: r40: add AHCI
  2018-07-09 15:20 [PATCH v2 0/4] sun8i: r40: add AHCI Corentin Labbe
                   ` (3 preceding siblings ...)
  2018-07-09 15:20 ` [PATCH v2 4/4] ARM: dts: sun8i: sun8i-r40-bananapi-m2-ultra: enable AHCI Corentin Labbe
@ 2018-07-09 15:44 ` Icenowy Zheng
  2018-07-10 13:05   ` LABBE Corentin
  2018-07-10 12:29 ` Maxime Ripard
  5 siblings, 1 reply; 12+ messages in thread
From: Icenowy Zheng @ 2018-07-09 15:44 UTC (permalink / raw)
  To: Corentin Labbe, linux, mark.rutland, maxime.ripard, robh+dt, tj, wens
  Cc: devicetree, linux-arm-kernel, linux-ide, linux-kernel, linux-sunxi



于 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/4] sun8i: r40: add AHCI
  2018-07-09 15:20 [PATCH v2 0/4] sun8i: r40: add AHCI Corentin Labbe
                   ` (4 preceding siblings ...)
  2018-07-09 15:44 ` [PATCH v2 0/4] sun8i: r40: add AHCI Icenowy Zheng
@ 2018-07-10 12:29 ` Maxime Ripard
  2018-07-10 12:32   ` Icenowy Zheng
  2018-07-11 18:59   ` LABBE Corentin
  5 siblings, 2 replies; 12+ messages in thread
From: Maxime Ripard @ 2018-07-10 12:29 UTC (permalink / raw)
  To: Corentin Labbe
  Cc: linux, mark.rutland, robh+dt, tj, wens, devicetree,
	linux-arm-kernel, linux-ide, linux-kernel, linux-sunxi, icenowy

[-- 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 --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/4] sun8i: r40: add AHCI
  2018-07-10 12:29 ` Maxime Ripard
@ 2018-07-10 12:32   ` Icenowy Zheng
  2018-07-11 18:59   ` LABBE Corentin
  1 sibling, 0 replies; 12+ messages in thread
From: Icenowy Zheng @ 2018-07-10 12:32 UTC (permalink / raw)
  To: Maxime Ripard, Corentin Labbe
  Cc: linux, mark.rutland, robh+dt, tj, wens, devicetree,
	linux-arm-kernel, linux-ide, linux-kernel, linux-sunxi



于 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/4] sun8i: r40: add AHCI
  2018-07-09 15:44 ` [PATCH v2 0/4] sun8i: r40: add AHCI Icenowy Zheng
@ 2018-07-10 13:05   ` LABBE Corentin
  2018-07-10 13:15     ` Icenowy Zheng
  0 siblings, 1 reply; 12+ messages in thread
From: LABBE Corentin @ 2018-07-10 13:05 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: linux, mark.rutland, maxime.ripard, robh+dt, tj, wens,
	devicetree, linux-arm-kernel, linux-ide, linux-kernel,
	linux-sunxi

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.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/4] sun8i: r40: add AHCI
  2018-07-10 13:05   ` LABBE Corentin
@ 2018-07-10 13:15     ` Icenowy Zheng
  0 siblings, 0 replies; 12+ messages in thread
From: Icenowy Zheng @ 2018-07-10 13:15 UTC (permalink / raw)
  To: linux-arm-kernel, LABBE Corentin
  Cc: mark.rutland, devicetree, linux-sunxi, linux, linux-kernel,
	linux-ide, wens, robh+dt, tj, maxime.ripard



于 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/4] sun8i: r40: add AHCI
  2018-07-10 12:29 ` Maxime Ripard
  2018-07-10 12:32   ` Icenowy Zheng
@ 2018-07-11 18:59   ` LABBE Corentin
  1 sibling, 0 replies; 12+ messages in thread
From: LABBE Corentin @ 2018-07-11 18:59 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: linux, mark.rutland, robh+dt, tj, wens, devicetree,
	linux-arm-kernel, linux-ide, linux-kernel, linux-sunxi, icenowy

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/4] dt-bindings: add binding for Allwinner R40 SATA AHCI controller
  2018-07-09 15:20 ` [PATCH v2 1/4] dt-bindings: add binding for Allwinner R40 SATA AHCI controller Corentin Labbe
@ 2018-07-11 19:12   ` Rob Herring
  0 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2018-07-11 19:12 UTC (permalink / raw)
  To: Corentin Labbe
  Cc: linux, mark.rutland, maxime.ripard, tj, wens, devicetree,
	linux-arm-kernel, linux-ide, linux-kernel, linux-sunxi, icenowy

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
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, back to index

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-09 15:20 [PATCH v2 0/4] sun8i: r40: add AHCI Corentin Labbe
2018-07-09 15:20 ` [PATCH v2 1/4] dt-bindings: add binding for Allwinner R40 SATA AHCI controller Corentin Labbe
2018-07-11 19:12   ` Rob Herring
2018-07-09 15:20 ` [PATCH v2 2/4] ata: ahci_sunxi: add support for R40 SATA controller Corentin Labbe
2018-07-09 15:20 ` [PATCH v2 3/4] ARM: dts: sun8i: r40: add sata node Corentin Labbe
2018-07-09 15:20 ` [PATCH v2 4/4] ARM: dts: sun8i: sun8i-r40-bananapi-m2-ultra: enable AHCI Corentin Labbe
2018-07-09 15:44 ` [PATCH v2 0/4] sun8i: r40: add AHCI Icenowy Zheng
2018-07-10 13:05   ` LABBE Corentin
2018-07-10 13:15     ` Icenowy Zheng
2018-07-10 12:29 ` Maxime Ripard
2018-07-10 12:32   ` Icenowy Zheng
2018-07-11 18:59   ` LABBE Corentin

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox