[PATCH v4 00/13] ata: ahci_platform: support allwinner R40 AHCI

[PATCH v4 00/13] ata: ahci_platform: support allwinner R40 AHCI

[Corentin Labbe]

Hello

This patchset add support for allwinner R40 AHCI controller.

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.

The last patch(ata: ahci_sunxi: remove PHY code) should not be merged,
but will be resent for inclustion when all patchs will have hit linus
tree.

Changes since v3:
- Moved PHY code to a new sun4i-a10-phy-sata driver
- Removed reset code since ahci_platform support now reset controller.

Changes since V2
- Moved all ressources management to ahci_platform

Corentin Labbe (13):
  dt-bindings: ata: ahci-platform: fix indentation of target-supply
  ata: ahci_platform: add support for AHCI controller regulator
  dt-bindings: ata: ahci-platform: document ahci-supply
  phy: Add sun4i-a10-phy-sata driver
  dt-bindings: phy: document sun4i-a10-sata-phy
  dt-bindings: ata: update ahci_sunxi bindings
  ata: ahci_sunxi: Bypass PHY init when using the new binding
  ata: ahci_sunxi: add support for r40
  ARM: dts: sun8i: r40: add sata node
  ARM: dts: sun8i: sun8i-r40-bananapi-m2-ultra: enable AHCI
  ARM: dts: sun7i: a20: add sata-port/sata-phy nodes
  ARM: dts: sun4i: a10: add sata-port/sata-phy nodes
  ata: ahci_sunxi: remove PHY code

 .../devicetree/bindings/ata/ahci-platform.txt      |  11 +-
 .../devicetree/bindings/phy/sun4i-sata-phy.txt     |  20 ++
 arch/arm/boot/dts/sun4i-a10.dtsi                   |  13 ++
 arch/arm/boot/dts/sun7i-a20.dtsi                   |  13 ++
 arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts  |  21 +++
 arch/arm/boot/dts/sun8i-r40.dtsi                   |  23 +++
 drivers/ata/ahci.h                                 |   1 +
 drivers/ata/ahci_sunxi.c                           |  87 +--------
 drivers/ata/libahci_platform.c                     |  26 ++-
 drivers/phy/allwinner/Kconfig                      |   7 +
 drivers/phy/allwinner/Makefile                     |   1 +
 drivers/phy/allwinner/phy-sun4i-sata.c             | 208 +++++++++++++++++++++
 12 files changed, 343 insertions(+), 88 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/sun4i-sata-phy.txt
 create mode 100644 drivers/phy/allwinner/phy-sun4i-sata.c

-- 
2.16.4

[PATCH v4 01/13] 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.montjoie@gmail.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 5d5bd456d9d9..b88820b4c01e 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
@@ -47,7 +47,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 v4 02/13] 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.montjoie@gmail.com>
---
 drivers/ata/ahci.h             |  1 +
 drivers/ata/libahci_platform.c | 26 ++++++++++++++++++++++++--
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 6a1515f0da40..1415f1012de5 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -352,6 +352,7 @@ struct ahci_host_priv {
 	struct clk		*clks[AHCI_MAX_CLKS]; /* Optional */
 	struct reset_control	*rsts;		/* 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 c92c10d55374..a886b61476a3 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -139,7 +139,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.
@@ -151,6 +151,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;
@@ -167,6 +173,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);
@@ -175,7 +183,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)
 {
@@ -186,6 +195,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);
 /**
@@ -351,6 +363,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) resets, if flags has AHCI_PLATFORM_GET_RESETS (optional)
@@ -408,6 +421,15 @@ 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;
+	}
+
 	if (flags & AHCI_PLATFORM_GET_RESETS) {
 		hpriv->rsts = devm_reset_control_array_get_optional_shared(dev);
 		if (IS_ERR(hpriv->rsts)) {
-- 
2.16.4

[PATCH v4 03/13] dt-bindings: ata: ahci-platform: document ahci-supply

[Corentin Labbe]

This patch document the new optional ahci-supply.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 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 b88820b4c01e..f495774c8af9 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 v4 04/13] phy: Add sun4i-a10-phy-sata driver

[Corentin Labbe]

This patch create a PHY SATA driver for allwinner SoC A10/A20/R40.
The code is taken from drivers/ata/ahci_sunxi.c but cannot be removed
from it yet, since we need to finish the transition to new binding with
the PHY usage.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/phy/allwinner/Kconfig          |   7 ++
 drivers/phy/allwinner/Makefile         |   1 +
 drivers/phy/allwinner/phy-sun4i-sata.c | 208 +++++++++++++++++++++++++++++++++
 3 files changed, 216 insertions(+)
 create mode 100644 drivers/phy/allwinner/phy-sun4i-sata.c

diff --git a/drivers/phy/allwinner/Kconfig b/drivers/phy/allwinner/Kconfig
index cdc1e745ba47..ef6fa389389a 100644
--- a/drivers/phy/allwinner/Kconfig
+++ b/drivers/phy/allwinner/Kconfig
@@ -1,6 +1,13 @@
 #
 # Phy drivers for Allwinner platforms
 #
+config PHY_SUN4I_SATA
+	tristate "Allwinner sunxi SoC SATA PHY driver"
+	depends on ARCH_SUNXI && HAS_IOMEM && OF
+	select GENERIC_PHY
+	help
+	  Enable this to support the SATA PHY present on A10/A20/R40.
+
 config PHY_SUN4I_USB
 	tristate "Allwinner sunxi SoC USB PHY driver"
 	depends on ARCH_SUNXI && HAS_IOMEM && OF
diff --git a/drivers/phy/allwinner/Makefile b/drivers/phy/allwinner/Makefile
index 8605529c01a1..73ef882edb53 100644
--- a/drivers/phy/allwinner/Makefile
+++ b/drivers/phy/allwinner/Makefile
@@ -1,2 +1,3 @@
+obj-$(CONFIG_PHY_SUN4I_SATA)		+= phy-sun4i-sata.o
 obj-$(CONFIG_PHY_SUN4I_USB)		+= phy-sun4i-usb.o
 obj-$(CONFIG_PHY_SUN9I_USB)		+= phy-sun9i-usb.o
diff --git a/drivers/phy/allwinner/phy-sun4i-sata.c b/drivers/phy/allwinner/phy-sun4i-sata.c
new file mode 100644
index 000000000000..93e29f99d26c
--- /dev/null
+++ b/drivers/phy/allwinner/phy-sun4i-sata.c
@@ -0,0 +1,208 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/*
+ * Allwinner sun4i SATA phy driver
+ * Copyright (C) 2018 Corentin Labbe <clabbe.montjoie@gmail.com>
+ *
+ * PHY init code taken from drivers/ahci/ahci_sunxi.c
+ *
+ */
+
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+
+#define AHCI_PHYCS0R	0x0000
+#define AHCI_PHYCS1R	0x0004
+#define AHCI_PHYCS2R	0x0008
+#define AHCI_TIMER1MS	0x0020
+#define AHCI_GPARAM1R	0x0028
+#define AHCI_GPARAM2R	0x002c
+#define AHCI_PPARAMR	0x0030
+#define AHCI_TESTR	0x0034
+#define AHCI_VERSIONR	0x0038
+#define AHCI_IDR	0x003c
+#define AHCI_RWCR	0x003c
+#define AHCI_P0DMACR	0x00b0
+#define AHCI_P0PHYCR	0x00b8
+#define AHCI_P0PHYSR	0x00bc
+
+struct sun4i_sata_phy_data {
+	struct phy *phy;
+	struct device *dev;
+	void __iomem            *base;
+};
+
+static void sunxi_clrbits(void __iomem *reg, u32 clr_val)
+{
+	u32 reg_val;
+
+	reg_val = readl(reg);
+	reg_val &= ~(clr_val);
+	writel(reg_val, reg);
+}
+
+static void sunxi_setbits(void __iomem *reg, u32 set_val)
+{
+	u32 reg_val;
+
+	reg_val = readl(reg);
+	reg_val |= set_val;
+	writel(reg_val, reg);
+}
+
+static void sunxi_clrsetbits(void __iomem *reg, u32 clr_val, u32 set_val)
+{
+	u32 reg_val;
+
+	reg_val = readl(reg);
+	reg_val &= ~(clr_val);
+	reg_val |= set_val;
+	writel(reg_val, reg);
+}
+
+static u32 sunxi_getbits(void __iomem *reg, u8 mask, u8 shift)
+{
+	return (readl(reg) >> shift) & mask;
+}
+
+static int ahci_sunxi_phy_init(struct device *dev, void __iomem *reg_base)
+{
+	u32 reg_val;
+	int timeout;
+
+	/* This magic is from the original code */
+	writel(0, reg_base + AHCI_RWCR);
+	msleep(5);
+
+	sunxi_setbits(reg_base + AHCI_PHYCS1R, BIT(19));
+	sunxi_clrsetbits(reg_base + AHCI_PHYCS0R, (0x7 << 24),
+			 (0x5 << 24) | BIT(23) | BIT(18));
+	sunxi_clrsetbits(reg_base + AHCI_PHYCS1R,
+			 (0x3 << 16) | (0x1f << 8) | (0x3 << 6),
+			 (0x2 << 16) | (0x6 << 8) | (0x2 << 6));
+	sunxi_setbits(reg_base + AHCI_PHYCS1R, BIT(28) | BIT(15));
+	sunxi_clrbits(reg_base + AHCI_PHYCS1R, BIT(19));
+	sunxi_clrsetbits(reg_base + AHCI_PHYCS0R, (0x7 << 20), (0x3 << 20));
+	sunxi_clrsetbits(reg_base + AHCI_PHYCS2R, (0x1f << 5), (0x19 << 5));
+	msleep(5);
+
+	sunxi_setbits(reg_base + AHCI_PHYCS0R, (0x1 << 19));
+
+	timeout = 250; /* Power up takes aprox 50 us */
+	do {
+		reg_val = sunxi_getbits(reg_base + AHCI_PHYCS0R, 0x7, 28);
+		if (reg_val == 0x02)
+			break;
+
+		if (--timeout == 0) {
+			dev_err(dev, "PHY power up failed.\n");
+			return -EIO;
+		}
+		udelay(1);
+	} while (1);
+
+	sunxi_setbits(reg_base + AHCI_PHYCS2R, (0x1 << 24));
+
+	timeout = 100; /* Calibration takes aprox 10 us */
+	do {
+		reg_val = sunxi_getbits(reg_base + AHCI_PHYCS2R, 0x1, 24);
+		if (reg_val == 0x00)
+			break;
+
+		if (--timeout == 0) {
+			dev_err(dev, "PHY calibration failed.\n");
+			return -EIO;
+		}
+		udelay(1);
+	} while (1);
+
+	msleep(15);
+
+	writel(0x7, reg_base + AHCI_RWCR);
+
+	return 0;
+}
+
+static int sun4i_sata_phy_power_on(struct phy *_phy)
+{
+	struct sun4i_sata_phy_data *data = phy_get_drvdata(_phy);
+
+	dev_info(data->dev, "%s\n", __func__);
+	return ahci_sunxi_phy_init(data->dev, data->base);
+}
+
+static const struct phy_ops sun4i_sata_phy_ops = {
+	.power_on	= sun4i_sata_phy_power_on,
+	.owner		= THIS_MODULE,
+};
+
+static int sun4i_sata_phy_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static int sun4i_sata_phy_probe(struct platform_device *pdev)
+{
+	struct sun4i_sata_phy_data *data;
+	struct device *dev = &pdev->dev;
+	struct phy_provider *phy_provider;
+	struct resource *res;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -EINVAL;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	dev_set_drvdata(dev, data);
+	data->dev = dev;
+
+	data->base = devm_ioremap(dev, res->start, resource_size(res));
+	if (!data->base)
+		return -ENOMEM;
+
+	data->phy = devm_phy_create(dev, NULL, &sun4i_sata_phy_ops);
+	if (IS_ERR(data->phy)) {
+		dev_err(dev, "failed to create PHY\n");
+		return PTR_ERR(data->phy);
+	}
+		phy_set_drvdata(data->phy, data);
+
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	dev_info(dev, "successfully loaded\n");
+
+	return 0;
+}
+
+static const struct of_device_id sun4i_sata_phy_of_match[] = {
+	{ .compatible = "allwinner,sun4i-a10-sata-phy", },
+	{ .compatible = "allwinner,sun8i-r40-sata-phy", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, sun4i_sata_phy_of_match);
+
+static struct platform_driver sun4i_sata_phy_driver = {
+	.probe	= sun4i_sata_phy_probe,
+	.remove	= sun4i_sata_phy_remove,
+	.driver = {
+		.of_match_table	= sun4i_sata_phy_of_match,
+		.name  = "sun4i-a10-sata-phy",
+	}
+};
+module_platform_driver(sun4i_sata_phy_driver);
+
+MODULE_DESCRIPTION("sun4i SATA PHY driver");
+MODULE_AUTHOR("Corentin LABBE <clabbe.montjoie@gmail.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.16.4

[PATCH v4 05/13] dt-bindings: phy: document sun4i-a10-sata-phy

[Corentin Labbe]

This patch document the sun4i-a10-sata-phy bindings.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 .../devicetree/bindings/phy/sun4i-sata-phy.txt       | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/sun4i-sata-phy.txt

diff --git a/Documentation/devicetree/bindings/phy/sun4i-sata-phy.txt b/Documentation/devicetree/bindings/phy/sun4i-sata-phy.txt
new file mode 100644
index 000000000000..f031542717c1
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/sun4i-sata-phy.txt
@@ -0,0 +1,20 @@
+Allwinner sun4i SATA PHY
+-----------------------
+
+Required properties:
+- compatible : should be one of
+  * allwinner,sun4i-a10-sata-phy
+  * allwinner,sun8i-r40-sata-phy
+- reg : a list of offset + length pairs
+- #phy-cells : from the generic phy bindings, must be 0
+
+Optional properties:
+- phy-supply : from the generic phy bindings, a phandle to a regulator that
+	       provides power to the PHY.
+
+Example:
+	sata_phy0: sata-phy@a01800 {
+		compatible = "allwinner,sun8i-r40-sata-phy";
+		reg = <0x00a01800 0x4>;
+		#phy-cells = <0>;
+	};
-- 
2.16.4

[PATCH v4 06/13] dt-bindings: ata: update ahci_sunxi bindings

[Corentin Labbe]

Since phy code is moved from ahci_sunxi to a dedicated driver, the
binding need to be updated with the new phy node requirement.

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

diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
index f495774c8af9..aa6f4c745097 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
@@ -50,6 +50,14 @@ And at least one of the following properties:
 - phys		    : reference to the SATA PHY node
 - target-supply     : regulator for SATA target power
 
+Required properties for:
+* allwinner,sun4i-a10-ahci
+* allwinner,sun8i-r40-ahci
+A port sub-node must be present and linked to a sata_phy.
+
+Required properties for allwinner,sun8i-r40-ahci
+The reset and ahci-supply properties must be present.
+
 Examples:
         sata@ffe08000 {
 		compatible = "snps,spear-ahci";
-- 
2.16.4

[PATCH v4 07/13] ata: ahci_sunxi: Bypass PHY init when using the new binding

[Corentin Labbe]

The new binding split sata in two (ahci + PHY).
ahci_sunxi must not mess with PHY when the new binding is in use.
So when we detect sub-nodes, bypass the PHY init code.
This is a temporarly workaround for the period where DT and ata code
will be merged from separate tree.
When both new binding and PHY driver will be merged, a new patch which
remove all PHY code from ahci_sunxi.c will be sent.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/ata/ahci_sunxi.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/ata/ahci_sunxi.c b/drivers/ata/ahci_sunxi.c
index 631610b72aa5..a09d189c6dda 100644
--- a/drivers/ata/ahci_sunxi.c
+++ b/drivers/ata/ahci_sunxi.c
@@ -96,6 +96,15 @@ static int ahci_sunxi_phy_init(struct device *dev, void __iomem *reg_base)
 	u32 reg_val;
 	int timeout;
 
+	/*
+	 * When using the new binding, the presence of a sata port node
+	 * means that PHY is handled by the PHY driver.
+	 * */
+	if (of_get_child_count(dev->of_node)) {
+		dev_info(dev, "Bypassing PHY init\n");
+		return 0;
+	}
+
 	/* This magic is from the original code */
 	writel(0, reg_base + AHCI_RWCR);
 	msleep(5);
-- 
2.16.4

[PATCH v4 08/13] ata: ahci_sunxi: add support for r40

[Corentin Labbe]

This patch add the r40 compatible to the ahci_sunxi's supported list of
compatible.

Since R40 need ahci_platform to handle the reset controller, we also add
the new AHCI_PLATFORM_GET_RESETS flag for ahci_platform_get_resources().
This has no consequence for older platform (a10, a20) since the reset is
optional.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/ata/ahci_sunxi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/ahci_sunxi.c b/drivers/ata/ahci_sunxi.c
index a09d189c6dda..b8cf3a1be80b 100644
--- a/drivers/ata/ahci_sunxi.c
+++ b/drivers/ata/ahci_sunxi.c
@@ -190,7 +190,7 @@ static int ahci_sunxi_probe(struct platform_device *pdev)
 	struct ahci_host_priv *hpriv;
 	int rc;
 
-	hpriv = ahci_platform_get_resources(pdev, 0);
+	hpriv = ahci_platform_get_resources(pdev, AHCI_PLATFORM_GET_RESETS);
 	if (IS_ERR(hpriv))
 		return PTR_ERR(hpriv);
 
@@ -259,6 +259,7 @@ static SIMPLE_DEV_PM_OPS(ahci_sunxi_pm_ops, ahci_platform_suspend,
 
 static const struct of_device_id ahci_sunxi_of_match[] = {
 	{ .compatible = "allwinner,sun4i-a10-ahci", },
+	{ .compatible = "allwinner,sun8i-r40-ahci", },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, ahci_sunxi_of_match);
-- 
2.16.4

[PATCH v4 09/13] 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.montjoie@gmail.com>
---
 arch/arm/boot/dts/sun8i-r40.dtsi | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
index 852c2ccc3268..d6b5820da850 100644
--- a/arch/arm/boot/dts/sun8i-r40.dtsi
+++ b/arch/arm/boot/dts/sun8i-r40.dtsi
@@ -550,6 +550,29 @@
 			#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>;
+			resets-name = "ahci";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+
+			sata_port: sata-port@0 {
+				reg = <0>;
+				phys = <&sata_phy>;
+			};
+		};
+
+		sata_phy: sata-phy@1c180c0  {
+			compatible = "allwinner,sun8i-r40-sata-phy";
+			reg = <0x1c180c0 0x200>;
+			#phy-cells = <0>;
+		};
+
 		gmac: ethernet@1c50000 {
 			compatible = "allwinner,sun8i-r40-gmac";
 			syscon = <&ccu>;
-- 
2.16.4

[PATCH v4 10/13] 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.montjoie@gmail.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 a891a387e8f1..b991b635c07d 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";
 };
@@ -250,6 +255,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_phy {
+	phy-supply = <&reg_eldo3>;
+};
+
 &tcon_tv0 {
 	status = "okay";
 };
-- 
2.16.4

[PATCH v4 11/13] ARM: dts: sun7i: a20: add sata-port/sata-phy nodes

[Corentin Labbe]

This patch convert sun7i-a20 sata to the new binding which use a sata
phy node.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 arch/arm/boot/dts/sun7i-a20.dtsi | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 8f7e1e29841f..8ff28f7afa85 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -667,6 +667,19 @@
 			interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&ccu CLK_AHB_SATA>, <&ccu CLK_SATA>;
 			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			sata_port: sata-port@0 {
+				reg = <0>;
+				phys = <&sata_phy>;
+			};
+		};
+
+		sata_phy: sata-phy@1c180c0  {
+			compatible = "allwinner,sun4i-a10-sata-phy";
+			reg = <0x1c180c0 0x200>;
+			#phy-cells = <0>;
 		};
 
 		ehci1: usb@1c1c000 {
-- 
2.16.4

[PATCH v4 12/13] ARM: dts: sun4i: a10: add sata-port/sata-phy nodes

[Corentin Labbe]

This patch convert sun4i-a10 sata to the new binding which use a sata
phy node.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 arch/arm/boot/dts/sun4i-a10.dtsi | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 3d62a8950720..52d5c2e79499 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -556,7 +556,20 @@
 			reg = <0x01c18000 0x1000>;
 			interrupts = <56>;
 			clocks = <&ccu CLK_AHB_SATA>, <&ccu CLK_SATA>;
+			#address-cells = <1>;
+			#size-cells = <0>;
 			status = "disabled";
+
+			sata_port: sata-port@0 {
+				reg = <0>;
+				phys = <&sata_phy>;
+			};
+		};
+
+		sata_phy: sata_phy@1c180c0 {
+			compatible = "allwinner,sun4i-a10-sata-phy";
+			reg = <0x01c180c0 0x200>;
+			#phy-cells = <0>;
 		};
 
 		ehci1: usb@1c1c000 {
-- 
2.16.4

[PATCH v4 13/13 DONOTMERGE] ata: ahci_sunxi: remove PHY code

[Corentin Labbe]

Since PHY code is now handled by sun4i-a10-sata-phy, the code in
ahci_sunxi is useless, remove it.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/ata/ahci_sunxi.c | 93 ------------------------------------------------
 1 file changed, 93 deletions(-)

diff --git a/drivers/ata/ahci_sunxi.c b/drivers/ata/ahci_sunxi.c
index b8cf3a1be80b..af17f8ce65b2 100644
--- a/drivers/ata/ahci_sunxi.c
+++ b/drivers/ata/ahci_sunxi.c
@@ -58,15 +58,6 @@ MODULE_PARM_DESC(enable_pmp,
 #define AHCI_P0PHYCR	0x0178
 #define AHCI_P0PHYSR	0x017c
 
-static void sunxi_clrbits(void __iomem *reg, u32 clr_val)
-{
-	u32 reg_val;
-
-	reg_val = readl(reg);
-	reg_val &= ~(clr_val);
-	writel(reg_val, reg);
-}
-
 static void sunxi_setbits(void __iomem *reg, u32 set_val)
 {
 	u32 reg_val;
@@ -86,81 +77,6 @@ static void sunxi_clrsetbits(void __iomem *reg, u32 clr_val, u32 set_val)
 	writel(reg_val, reg);
 }
 
-static u32 sunxi_getbits(void __iomem *reg, u8 mask, u8 shift)
-{
-	return (readl(reg) >> shift) & mask;
-}
-
-static int ahci_sunxi_phy_init(struct device *dev, void __iomem *reg_base)
-{
-	u32 reg_val;
-	int timeout;
-
-	/*
-	 * When using the new binding, the presence of a sata port node
-	 * means that PHY is handled by the PHY driver.
-	 * */
-	if (of_get_child_count(dev->of_node)) {
-		dev_info(dev, "Bypassing PHY init\n");
-		return 0;
-	}
-
-	/* This magic is from the original code */
-	writel(0, reg_base + AHCI_RWCR);
-	msleep(5);
-
-	sunxi_setbits(reg_base + AHCI_PHYCS1R, BIT(19));
-	sunxi_clrsetbits(reg_base + AHCI_PHYCS0R,
-			 (0x7 << 24),
-			 (0x5 << 24) | BIT(23) | BIT(18));
-	sunxi_clrsetbits(reg_base + AHCI_PHYCS1R,
-			 (0x3 << 16) | (0x1f << 8) | (0x3 << 6),
-			 (0x2 << 16) | (0x6 << 8) | (0x2 << 6));
-	sunxi_setbits(reg_base + AHCI_PHYCS1R, BIT(28) | BIT(15));
-	sunxi_clrbits(reg_base + AHCI_PHYCS1R, BIT(19));
-	sunxi_clrsetbits(reg_base + AHCI_PHYCS0R,
-			 (0x7 << 20), (0x3 << 20));
-	sunxi_clrsetbits(reg_base + AHCI_PHYCS2R,
-			 (0x1f << 5), (0x19 << 5));
-	msleep(5);
-
-	sunxi_setbits(reg_base + AHCI_PHYCS0R, (0x1 << 19));
-
-	timeout = 250; /* Power up takes aprox 50 us */
-	do {
-		reg_val = sunxi_getbits(reg_base + AHCI_PHYCS0R, 0x7, 28);
-		if (reg_val == 0x02)
-			break;
-
-		if (--timeout == 0) {
-			dev_err(dev, "PHY power up failed.\n");
-			return -EIO;
-		}
-		udelay(1);
-	} while (1);
-
-	sunxi_setbits(reg_base + AHCI_PHYCS2R, (0x1 << 24));
-
-	timeout = 100; /* Calibration takes aprox 10 us */
-	do {
-		reg_val = sunxi_getbits(reg_base + AHCI_PHYCS2R, 0x1, 24);
-		if (reg_val == 0x00)
-			break;
-
-		if (--timeout == 0) {
-			dev_err(dev, "PHY calibration failed.\n");
-			return -EIO;
-		}
-		udelay(1);
-	} while (1);
-
-	msleep(15);
-
-	writel(0x7, reg_base + AHCI_RWCR);
-
-	return 0;
-}
-
 static void ahci_sunxi_start_engine(struct ata_port *ap)
 {
 	void __iomem *port_mmio = ahci_port_base(ap);
@@ -186,7 +102,6 @@ static struct scsi_host_template ahci_platform_sht = {
 
 static int ahci_sunxi_probe(struct platform_device *pdev)
 {
-	struct device *dev = &pdev->dev;
 	struct ahci_host_priv *hpriv;
 	int rc;
 
@@ -200,10 +115,6 @@ static int ahci_sunxi_probe(struct platform_device *pdev)
 	if (rc)
 		return rc;
 
-	rc = ahci_sunxi_phy_init(dev, hpriv->mmio);
-	if (rc)
-		goto disable_resources;
-
 	hpriv->flags = AHCI_HFLAG_32BIT_ONLY | AHCI_HFLAG_NO_MSI |
 		       AHCI_HFLAG_YES_NCQ;
 
@@ -238,10 +149,6 @@ static int ahci_sunxi_resume(struct device *dev)
 	if (rc)
 		return rc;
 
-	rc = ahci_sunxi_phy_init(dev, hpriv->mmio);
-	if (rc)
-		goto disable_resources;
-
 	rc = ahci_platform_resume_host(dev);
 	if (rc)
 		goto disable_resources;
-- 
2.16.4

Re: [PATCH v4 00/13] ata: ahci_platform: support allwinner R40 AHCI

[Hans de Goede]

Hi,

On 30-08-18 21:01, Corentin Labbe wrote:
> Hello
> 
> This patchset add support for allwinner R40 AHCI controller.
> 
> 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.
> 
> The last patch(ata: ahci_sunxi: remove PHY code) should not be merged,
> but will be resent for inclustion when all patchs will have hit linus
> tree.

Thank you for your work on this, the entire series looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Note I've one remark for the final do-not-merge patch I will reply
to that patch separately.

Regards,

Hans



> 
> Changes since v3:
> - Moved PHY code to a new sun4i-a10-phy-sata driver
> - Removed reset code since ahci_platform support now reset controller.
> 
> Changes since V2
> - Moved all ressources management to ahci_platform
> 
> Corentin Labbe (13):
>    dt-bindings: ata: ahci-platform: fix indentation of target-supply
>    ata: ahci_platform: add support for AHCI controller regulator
>    dt-bindings: ata: ahci-platform: document ahci-supply
>    phy: Add sun4i-a10-phy-sata driver
>    dt-bindings: phy: document sun4i-a10-sata-phy
>    dt-bindings: ata: update ahci_sunxi bindings
>    ata: ahci_sunxi: Bypass PHY init when using the new binding
>    ata: ahci_sunxi: add support for r40
>    ARM: dts: sun8i: r40: add sata node
>    ARM: dts: sun8i: sun8i-r40-bananapi-m2-ultra: enable AHCI
>    ARM: dts: sun7i: a20: add sata-port/sata-phy nodes
>    ARM: dts: sun4i: a10: add sata-port/sata-phy nodes
>    ata: ahci_sunxi: remove PHY code
> 
>   .../devicetree/bindings/ata/ahci-platform.txt      |  11 +-
>   .../devicetree/bindings/phy/sun4i-sata-phy.txt     |  20 ++
>   arch/arm/boot/dts/sun4i-a10.dtsi                   |  13 ++
>   arch/arm/boot/dts/sun7i-a20.dtsi                   |  13 ++
>   arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts  |  21 +++
>   arch/arm/boot/dts/sun8i-r40.dtsi                   |  23 +++
>   drivers/ata/ahci.h                                 |   1 +
>   drivers/ata/ahci_sunxi.c                           |  87 +--------
>   drivers/ata/libahci_platform.c                     |  26 ++-
>   drivers/phy/allwinner/Kconfig                      |   7 +
>   drivers/phy/allwinner/Makefile                     |   1 +
>   drivers/phy/allwinner/phy-sun4i-sata.c             | 208 +++++++++++++++++++++
>   12 files changed, 343 insertions(+), 88 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/phy/sun4i-sata-phy.txt
>   create mode 100644 drivers/phy/allwinner/phy-sun4i-sata.c
> 

Re: [PATCH v4 13/13 DONOTMERGE] ata: ahci_sunxi: remove PHY code

[Hans de Goede]

HI,

On 30-08-18 21:01, Corentin Labbe wrote:
> Since PHY code is now handled by sun4i-a10-sata-phy, the code in
> ahci_sunxi is useless, remove it.
> 
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
>   drivers/ata/ahci_sunxi.c | 93 ------------------------------------------------
>   1 file changed, 93 deletions(-)
> 
> diff --git a/drivers/ata/ahci_sunxi.c b/drivers/ata/ahci_sunxi.c
> index b8cf3a1be80b..af17f8ce65b2 100644
> --- a/drivers/ata/ahci_sunxi.c
> +++ b/drivers/ata/ahci_sunxi.c
> @@ -58,15 +58,6 @@ MODULE_PARM_DESC(enable_pmp,
>   #define AHCI_P0PHYCR	0x0178
>   #define AHCI_P0PHYSR	0x017c
>   
> -static void sunxi_clrbits(void __iomem *reg, u32 clr_val)
> -{
> -	u32 reg_val;
> -
> -	reg_val = readl(reg);
> -	reg_val &= ~(clr_val);
> -	writel(reg_val, reg);
> -}
> -
>   static void sunxi_setbits(void __iomem *reg, u32 set_val)
>   {
>   	u32 reg_val;
> @@ -86,81 +77,6 @@ static void sunxi_clrsetbits(void __iomem *reg, u32 clr_val, u32 set_val)
>   	writel(reg_val, reg);
>   }
>   
> -static u32 sunxi_getbits(void __iomem *reg, u8 mask, u8 shift)
> -{
> -	return (readl(reg) >> shift) & mask;
> -}
> -
> -static int ahci_sunxi_phy_init(struct device *dev, void __iomem *reg_base)
> -{
> -	u32 reg_val;
> -	int timeout;
> -
> -	/*
> -	 * When using the new binding, the presence of a sata port node
> -	 * means that PHY is handled by the PHY driver.
> -	 * */
> -	if (of_get_child_count(dev->of_node)) {
> -		dev_info(dev, "Bypassing PHY init\n");
> -		return 0;
> -	}
> -
> -	/* This magic is from the original code */
> -	writel(0, reg_base + AHCI_RWCR);
> -	msleep(5);
> -
> -	sunxi_setbits(reg_base + AHCI_PHYCS1R, BIT(19));
> -	sunxi_clrsetbits(reg_base + AHCI_PHYCS0R,
> -			 (0x7 << 24),
> -			 (0x5 << 24) | BIT(23) | BIT(18));
> -	sunxi_clrsetbits(reg_base + AHCI_PHYCS1R,
> -			 (0x3 << 16) | (0x1f << 8) | (0x3 << 6),
> -			 (0x2 << 16) | (0x6 << 8) | (0x2 << 6));
> -	sunxi_setbits(reg_base + AHCI_PHYCS1R, BIT(28) | BIT(15));
> -	sunxi_clrbits(reg_base + AHCI_PHYCS1R, BIT(19));
> -	sunxi_clrsetbits(reg_base + AHCI_PHYCS0R,
> -			 (0x7 << 20), (0x3 << 20));
> -	sunxi_clrsetbits(reg_base + AHCI_PHYCS2R,
> -			 (0x1f << 5), (0x19 << 5));
> -	msleep(5);
> -
> -	sunxi_setbits(reg_base + AHCI_PHYCS0R, (0x1 << 19));
> -
> -	timeout = 250; /* Power up takes aprox 50 us */
> -	do {
> -		reg_val = sunxi_getbits(reg_base + AHCI_PHYCS0R, 0x7, 28);
> -		if (reg_val == 0x02)
> -			break;
> -
> -		if (--timeout == 0) {
> -			dev_err(dev, "PHY power up failed.\n");
> -			return -EIO;
> -		}
> -		udelay(1);
> -	} while (1);
> -
> -	sunxi_setbits(reg_base + AHCI_PHYCS2R, (0x1 << 24));
> -
> -	timeout = 100; /* Calibration takes aprox 10 us */
> -	do {
> -		reg_val = sunxi_getbits(reg_base + AHCI_PHYCS2R, 0x1, 24);
> -		if (reg_val == 0x00)
> -			break;
> -
> -		if (--timeout == 0) {
> -			dev_err(dev, "PHY calibration failed.\n");
> -			return -EIO;
> -		}
> -		udelay(1);
> -	} while (1);
> -
> -	msleep(15);
> -
> -	writel(0x7, reg_base + AHCI_RWCR);
> -
> -	return 0;
> -}
> -
>   static void ahci_sunxi_start_engine(struct ata_port *ap)
>   {
>   	void __iomem *port_mmio = ahci_port_base(ap);
> @@ -186,7 +102,6 @@ static struct scsi_host_template ahci_platform_sht = {
>   
>   static int ahci_sunxi_probe(struct platform_device *pdev)
>   {
> -	struct device *dev = &pdev->dev;
>   	struct ahci_host_priv *hpriv;
>   	int rc;
>   
> @@ -200,10 +115,6 @@ static int ahci_sunxi_probe(struct platform_device *pdev)
>   	if (rc)
>   		return rc;
>   
> -	rc = ahci_sunxi_phy_init(dev, hpriv->mmio);
> -	if (rc)
> -		goto disable_resources;
> -
>   	hpriv->flags = AHCI_HFLAG_32BIT_ONLY | AHCI_HFLAG_NO_MSI |
>   		       AHCI_HFLAG_YES_NCQ;
>   
> @@ -238,10 +149,6 @@ static int ahci_sunxi_resume(struct device *dev)
>   	if (rc)
>   		return rc;
>   
> -	rc = ahci_sunxi_phy_init(dev, hpriv->mmio);
> -	if (rc)
> -		goto disable_resources;
> -
>   	rc = ahci_platform_resume_host(dev);
>   	if (rc)
>   		goto disable_resources;
> 

After this change ahci_sunxi_resume() is the same as ahci_platform_resume,
so you can drop the entire function and directly refer to
ahci_platform_resume in ahci_sunxi_pm_ops.

Regards,

Hans

Re: [PATCH v4 00/13] ata: ahci_platform: support allwinner R40 AHCI

[Jens Axboe]

On 8/30/18 1:01 PM, Corentin Labbe wrote:
> Hello
> 
> This patchset add support for allwinner R40 AHCI controller.
> 
> 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.
> 
> The last patch(ata: ahci_sunxi: remove PHY code) should not be merged,
> but will be resent for inclustion when all patchs will have hit linus
> tree.

Applied 1-12 with Hans's blessing, thanks Corentin.

-- 
Jens Axboe

Re: [PATCH v4 00/13] ata: ahci_platform: support allwinner R40 AHCI

[Chen-Yu Tsai]

Hi,

On Fri, Aug 31, 2018 at 4:31 AM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 8/30/18 1:01 PM, Corentin Labbe wrote:
> > Hello
> >
> > This patchset add support for allwinner R40 AHCI controller.
> >
> > 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.
> >
> > The last patch(ata: ahci_sunxi: remove PHY code) should not be merged,
> > but will be resent for inclustion when all patchs will have hit linus
> > tree.
>
> Applied 1-12 with Hans's blessing, thanks Corentin.

Please don't merge device tree ("dts") patches, i.e. patches 9-12. We will
merge them through the sunxi / armsoc tree. Having them in separate trees
introduces conflicts when we have other stuff going through our tree.

Corentin, it's best to lay out the plan to get patches merges in the cover
letter, specifically which maintainer should take which patches, or if an
immutable tag/branch is preferred, when things can't be separated cleanly.
This helps other subsystem maintainers that don't routinely deal with armsoc.

Also, we probably can't merge the last patch that removes the PHY code,
since we have to support old device trees.

Thanks
ChenYu

Re: [PATCH v4 00/13] ata: ahci_platform: support allwinner R40 AHCI

[Jens Axboe]

On 8/30/18 8:32 PM, Chen-Yu Tsai wrote:
> Hi,
> 
> On Fri, Aug 31, 2018 at 4:31 AM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> On 8/30/18 1:01 PM, Corentin Labbe wrote:
>>> Hello
>>>
>>> This patchset add support for allwinner R40 AHCI controller.
>>>
>>> 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.
>>>
>>> The last patch(ata: ahci_sunxi: remove PHY code) should not be merged,
>>> but will be resent for inclustion when all patchs will have hit linus
>>> tree.
>>
>> Applied 1-12 with Hans's blessing, thanks Corentin.
> 
> Please don't merge device tree ("dts") patches, i.e. patches 9-12. We will
> merge them through the sunxi / armsoc tree. Having them in separate trees
> introduces conflicts when we have other stuff going through our tree.

OK, fair enough, I can drop those.

-- 
Jens Axboe

Re: [PATCH v4 09/13] ARM: dts: sun8i: r40: add sata node

[Maxime Ripard]

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

On Thu, Aug 30, 2018 at 09:01:16PM +0200, Corentin Labbe wrote:
> 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.montjoie@gmail.com>
> ---
>  arch/arm/boot/dts/sun8i-r40.dtsi | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
> index 852c2ccc3268..d6b5820da850 100644
> --- a/arch/arm/boot/dts/sun8i-r40.dtsi
> +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
> @@ -550,6 +550,29 @@
>  			#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>;
> +			resets-name = "ahci";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			status = "disabled";
> +
> +			sata_port: sata-port@0 {
> +				reg = <0>;
> +				phys = <&sata_phy>;
> +			};
> +		};
> +
> +		sata_phy: sata-phy@1c180c0  {
> +			compatible = "allwinner,sun8i-r40-sata-phy";
> +			reg = <0x1c180c0 0x200>;

Overlapping devices in the DTS is not ok.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

Re: [PATCH v4 12/13] ARM: dts: sun4i: a10: add sata-port/sata-phy nodes

[Maxime Ripard]

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

On Thu, Aug 30, 2018 at 09:01:19PM +0200, Corentin Labbe wrote:
> This patch convert sun4i-a10 sata to the new binding which use a sata
> phy node.
> 
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
>  arch/arm/boot/dts/sun4i-a10.dtsi | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
> index 3d62a8950720..52d5c2e79499 100644
> --- a/arch/arm/boot/dts/sun4i-a10.dtsi
> +++ b/arch/arm/boot/dts/sun4i-a10.dtsi
> @@ -556,7 +556,20 @@
>  			reg = <0x01c18000 0x1000>;
>  			interrupts = <56>;
>  			clocks = <&ccu CLK_AHB_SATA>, <&ccu CLK_SATA>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
>  			status = "disabled";
> +
> +			sata_port: sata-port@0 {
> +				reg = <0>;
> +				phys = <&sata_phy>;
> +			};
> +		};
> +
> +		sata_phy: sata_phy@1c180c0 {
> +			compatible = "allwinner,sun4i-a10-sata-phy";
> +			reg = <0x01c180c0 0x200>;
> +			#phy-cells = <0>;
>  		};

This patch, together with the following one, breaks the DT ABI, this
isn't something we can do anymore.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

Re: [PATCH v4 00/13] ata: ahci_platform: support allwinner R40 AHCI

[Maxime Ripard]

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

Hi Jens,

On Thu, Aug 30, 2018 at 08:52:52PM -0600, Jens Axboe wrote:
> On 8/30/18 8:32 PM, Chen-Yu Tsai wrote:
> > Hi,
> > 
> > On Fri, Aug 31, 2018 at 4:31 AM Jens Axboe <axboe@kernel.dk> wrote:
> >>
> >> On 8/30/18 1:01 PM, Corentin Labbe wrote:
> >>> Hello
> >>>
> >>> This patchset add support for allwinner R40 AHCI controller.
> >>>
> >>> 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.
> >>>
> >>> The last patch(ata: ahci_sunxi: remove PHY code) should not be merged,
> >>> but will be resent for inclustion when all patchs will have hit linus
> >>> tree.
> >>
> >> Applied 1-12 with Hans's blessing, thanks Corentin.
> > 
> > Please don't merge device tree ("dts") patches, i.e. patches 9-12. We will
> > merge them through the sunxi / armsoc tree. Having them in separate trees
> > introduces conflicts when we have other stuff going through our tree.
> 
> OK, fair enough, I can drop those.

And the DT bits actually have some issues that would need to change
the code significantly.

Can you drop all of them please?

Thanks!
Maxmie

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

Re: [PATCH v4 12/13] ARM: dts: sun4i: a10: add sata-port/sata-phy nodes

[Corentin Labbe]

On Fri, Aug 31, 2018 at 09:37:24AM +0200, Maxime Ripard wrote:
> On Thu, Aug 30, 2018 at 09:01:19PM +0200, Corentin Labbe wrote:
> > This patch convert sun4i-a10 sata to the new binding which use a sata
> > phy node.
> > 
> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> > ---
> >  arch/arm/boot/dts/sun4i-a10.dtsi | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
> > index 3d62a8950720..52d5c2e79499 100644
> > --- a/arch/arm/boot/dts/sun4i-a10.dtsi
> > +++ b/arch/arm/boot/dts/sun4i-a10.dtsi
> > @@ -556,7 +556,20 @@
> >  			reg = <0x01c18000 0x1000>;
> >  			interrupts = <56>;
> >  			clocks = <&ccu CLK_AHB_SATA>, <&ccu CLK_SATA>;
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> >  			status = "disabled";
> > +
> > +			sata_port: sata-port@0 {
> > +				reg = <0>;
> > +				phys = <&sata_phy>;
> > +			};
> > +		};
> > +
> > +		sata_phy: sata_phy@1c180c0 {
> > +			compatible = "allwinner,sun4i-a10-sata-phy";
> > +			reg = <0x01c180c0 0x200>;
> > +			#phy-cells = <0>;
> >  		};
> 
> This patch, together with the following one, breaks the DT ABI, this
> isn't something we can do anymore.
> 

So what can I do ?
Keep the DT in place and drop the patch 13 like wens suggested ?

Regards

Re: [PATCH v4 09/13] ARM: dts: sun8i: r40: add sata node

[Corentin Labbe]

On Fri, Aug 31, 2018 at 09:35:00AM +0200, Maxime Ripard wrote:
> On Thu, Aug 30, 2018 at 09:01:16PM +0200, Corentin Labbe wrote:
> > 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.montjoie@gmail.com>
> > ---
> >  arch/arm/boot/dts/sun8i-r40.dtsi | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
> > index 852c2ccc3268..d6b5820da850 100644
> > --- a/arch/arm/boot/dts/sun8i-r40.dtsi
> > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
> > @@ -550,6 +550,29 @@
> >  			#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>;
> > +			resets-name = "ahci";
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +			status = "disabled";
> > +
> > +			sata_port: sata-port@0 {
> > +				reg = <0>;
> > +				phys = <&sata_phy>;
> > +			};
> > +		};
> > +
> > +		sata_phy: sata-phy@1c180c0  {
> > +			compatible = "allwinner,sun8i-r40-sata-phy";
> > +			reg = <0x1c180c0 0x200>;
> 
> Overlapping devices in the DTS is not ok.
> 

I do the same than arch/arm/boot/dts/berlin2.dtsi (sata@e90000 phy@e900a0)
But since it is not a good justification, it seems that regmap is my only solution ?

Regards

Re: [PATCH v4 09/13] ARM: dts: sun8i: r40: add sata node

[Chen-Yu Tsai]

On Fri, Aug 31, 2018 at 3:56 PM Corentin Labbe
<clabbe.montjoie@gmail.com> wrote:
>
> On Fri, Aug 31, 2018 at 09:35:00AM +0200, Maxime Ripard wrote:
> > On Thu, Aug 30, 2018 at 09:01:16PM +0200, Corentin Labbe wrote:
> > > 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.montjoie@gmail.com>
> > > ---
> > >  arch/arm/boot/dts/sun8i-r40.dtsi | 23 +++++++++++++++++++++++
> > >  1 file changed, 23 insertions(+)
> > >
> > > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
> > > index 852c2ccc3268..d6b5820da850 100644
> > > --- a/arch/arm/boot/dts/sun8i-r40.dtsi
> > > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
> > > @@ -550,6 +550,29 @@
> > >                     #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>;
> > > +                   resets-name = "ahci";
> > > +                   #address-cells = <1>;
> > > +                   #size-cells = <0>;
> > > +                   status = "disabled";
> > > +
> > > +                   sata_port: sata-port@0 {
> > > +                           reg = <0>;
> > > +                           phys = <&sata_phy>;
> > > +                   };
> > > +           };
> > > +
> > > +           sata_phy: sata-phy@1c180c0  {
> > > +                   compatible = "allwinner,sun8i-r40-sata-phy";
> > > +                   reg = <0x1c180c0 0x200>;
> >
> > Overlapping devices in the DTS is not ok.
> >
>
> I do the same than arch/arm/boot/dts/berlin2.dtsi (sata@e90000 phy@e900a0)
> But since it is not a good justification, it seems that regmap is my only solution ?

Since you are effectively splitting one device node into two, you should
adjust the original (ahci) device nodes register range.

ChenYu

Re: [PATCH v4 09/13] ARM: dts: sun8i: r40: add sata node

[Corentin Labbe]

On Fri, Aug 31, 2018 at 03:58:34PM +0800, Chen-Yu Tsai wrote:
> On Fri, Aug 31, 2018 at 3:56 PM Corentin Labbe
> <clabbe.montjoie@gmail.com> wrote:
> >
> > On Fri, Aug 31, 2018 at 09:35:00AM +0200, Maxime Ripard wrote:
> > > On Thu, Aug 30, 2018 at 09:01:16PM +0200, Corentin Labbe wrote:
> > > > 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.montjoie@gmail.com>
> > > > ---
> > > >  arch/arm/boot/dts/sun8i-r40.dtsi | 23 +++++++++++++++++++++++
> > > >  1 file changed, 23 insertions(+)
> > > >
> > > > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > index 852c2ccc3268..d6b5820da850 100644
> > > > --- a/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > @@ -550,6 +550,29 @@
> > > >                     #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>;
> > > > +                   resets-name = "ahci";
> > > > +                   #address-cells = <1>;
> > > > +                   #size-cells = <0>;
> > > > +                   status = "disabled";
> > > > +
> > > > +                   sata_port: sata-port@0 {
> > > > +                           reg = <0>;
> > > > +                           phys = <&sata_phy>;
> > > > +                   };
> > > > +           };
> > > > +
> > > > +           sata_phy: sata-phy@1c180c0  {
> > > > +                   compatible = "allwinner,sun8i-r40-sata-phy";
> > > > +                   reg = <0x1c180c0 0x200>;
> > >
> > > Overlapping devices in the DTS is not ok.
> > >
> >
> > I do the same than arch/arm/boot/dts/berlin2.dtsi (sata@e90000 phy@e900a0)
> > But since it is not a good justification, it seems that regmap is my only solution ?
> 
> Since you are effectively splitting one device node into two, you should
> adjust the original (ahci) device nodes register range.
> 

I cannot do that if I remove patch 13, iow If I keep phy init code in both place as you requested.

Re: [PATCH v4 09/13] ARM: dts: sun8i: r40: add sata node

[maxime.ripard]

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

On Fri, Aug 31, 2018 at 09:56:31AM +0200, Corentin Labbe wrote:
> On Fri, Aug 31, 2018 at 09:35:00AM +0200, Maxime Ripard wrote:
> > On Thu, Aug 30, 2018 at 09:01:16PM +0200, Corentin Labbe wrote:
> > > 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.montjoie@gmail.com>
> > > ---
> > >  arch/arm/boot/dts/sun8i-r40.dtsi | 23 +++++++++++++++++++++++
> > >  1 file changed, 23 insertions(+)
> > > 
> > > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
> > > index 852c2ccc3268..d6b5820da850 100644
> > > --- a/arch/arm/boot/dts/sun8i-r40.dtsi
> > > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
> > > @@ -550,6 +550,29 @@
> > >  			#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>;
> > > +			resets-name = "ahci";
> > > +			#address-cells = <1>;
> > > +			#size-cells = <0>;
> > > +			status = "disabled";
> > > +
> > > +			sata_port: sata-port@0 {
> > > +				reg = <0>;
> > > +				phys = <&sata_phy>;
> > > +			};
> > > +		};
> > > +
> > > +		sata_phy: sata-phy@1c180c0  {
> > > +			compatible = "allwinner,sun8i-r40-sata-phy";
> > > +			reg = <0x1c180c0 0x200>;
> > 
> > Overlapping devices in the DTS is not ok.
> > 
> 
> I do the same than arch/arm/boot/dts/berlin2.dtsi (sata@e90000
> phy@e900a0)
>
> But since it is not a good justification, it seems that regmap is my
> only solution ?

I'm not even sure why you are moving the phy out of its original node
(and driver).

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

Re: [PATCH v4 12/13] ARM: dts: sun4i: a10: add sata-port/sata-phy nodes

[Maxime Ripard]

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

On Fri, Aug 31, 2018 at 09:53:50AM +0200, Corentin Labbe wrote:
> On Fri, Aug 31, 2018 at 09:37:24AM +0200, Maxime Ripard wrote:
> > On Thu, Aug 30, 2018 at 09:01:19PM +0200, Corentin Labbe wrote:
> > > This patch convert sun4i-a10 sata to the new binding which use a sata
> > > phy node.
> > > 
> > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> > > ---
> > >  arch/arm/boot/dts/sun4i-a10.dtsi | 13 +++++++++++++
> > >  1 file changed, 13 insertions(+)
> > > 
> > > diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
> > > index 3d62a8950720..52d5c2e79499 100644
> > > --- a/arch/arm/boot/dts/sun4i-a10.dtsi
> > > +++ b/arch/arm/boot/dts/sun4i-a10.dtsi
> > > @@ -556,7 +556,20 @@
> > >  			reg = <0x01c18000 0x1000>;
> > >  			interrupts = <56>;
> > >  			clocks = <&ccu CLK_AHB_SATA>, <&ccu CLK_SATA>;
> > > +			#address-cells = <1>;
> > > +			#size-cells = <0>;
> > >  			status = "disabled";
> > > +
> > > +			sata_port: sata-port@0 {
> > > +				reg = <0>;
> > > +				phys = <&sata_phy>;
> > > +			};
> > > +		};
> > > +
> > > +		sata_phy: sata_phy@1c180c0 {
> > > +			compatible = "allwinner,sun4i-a10-sata-phy";
> > > +			reg = <0x01c180c0 0x200>;
> > > +			#phy-cells = <0>;
> > >  		};
> > 
> > This patch, together with the following one, breaks the DT ABI, this
> > isn't something we can do anymore.
> 
> So what can I do ?

Well, you always have the option of not doing anything.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

Re: [PATCH v4 09/13] ARM: dts: sun8i: r40: add sata node

[Corentin Labbe]

On Fri, Aug 31, 2018 at 12:20:21PM +0200, maxime.ripard@bootlin.com wrote:
> On Fri, Aug 31, 2018 at 09:56:31AM +0200, Corentin Labbe wrote:
> > On Fri, Aug 31, 2018 at 09:35:00AM +0200, Maxime Ripard wrote:
> > > On Thu, Aug 30, 2018 at 09:01:16PM +0200, Corentin Labbe wrote:
> > > > 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.montjoie@gmail.com>
> > > > ---
> > > >  arch/arm/boot/dts/sun8i-r40.dtsi | 23 +++++++++++++++++++++++
> > > >  1 file changed, 23 insertions(+)
> > > > 
> > > > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > index 852c2ccc3268..d6b5820da850 100644
> > > > --- a/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > @@ -550,6 +550,29 @@
> > > >  			#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>;
> > > > +			resets-name = "ahci";
> > > > +			#address-cells = <1>;
> > > > +			#size-cells = <0>;
> > > > +			status = "disabled";
> > > > +
> > > > +			sata_port: sata-port@0 {
> > > > +				reg = <0>;
> > > > +				phys = <&sata_phy>;
> > > > +			};
> > > > +		};
> > > > +
> > > > +		sata_phy: sata-phy@1c180c0  {
> > > > +			compatible = "allwinner,sun8i-r40-sata-phy";
> > > > +			reg = <0x1c180c0 0x200>;
> > > 
> > > Overlapping devices in the DTS is not ok.
> > > 
> > 
> > I do the same than arch/arm/boot/dts/berlin2.dtsi (sata@e90000
> > phy@e900a0)
> >
> > But since it is not a good justification, it seems that regmap is my
> > only solution ?
> 
> I'm not even sure why you are moving the phy out of its original node
> (and driver).
> 

For using the phy-supply already handled by the code.
The other choice is to add another xxx-supply to ahci_platform.
Or to use hackily port_regulator for this regulator.

Re: [PATCH v4 09/13] ARM: dts: sun8i: r40: add sata node

[Chen-Yu Tsai]

On Fri, Aug 31, 2018 at 5:29 PM Corentin Labbe
<clabbe.montjoie@gmail.com> wrote:
>
> On Fri, Aug 31, 2018 at 03:58:34PM +0800, Chen-Yu Tsai wrote:
> > On Fri, Aug 31, 2018 at 3:56 PM Corentin Labbe
> > <clabbe.montjoie@gmail.com> wrote:
> > >
> > > On Fri, Aug 31, 2018 at 09:35:00AM +0200, Maxime Ripard wrote:
> > > > On Thu, Aug 30, 2018 at 09:01:16PM +0200, Corentin Labbe wrote:
> > > > > 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.montjoie@gmail.com>
> > > > > ---
> > > > >  arch/arm/boot/dts/sun8i-r40.dtsi | 23 +++++++++++++++++++++++
> > > > >  1 file changed, 23 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > > index 852c2ccc3268..d6b5820da850 100644
> > > > > --- a/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > > @@ -550,6 +550,29 @@
> > > > >                     #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>;
> > > > > +                   resets-name = "ahci";
> > > > > +                   #address-cells = <1>;
> > > > > +                   #size-cells = <0>;
> > > > > +                   status = "disabled";
> > > > > +
> > > > > +                   sata_port: sata-port@0 {
> > > > > +                           reg = <0>;
> > > > > +                           phys = <&sata_phy>;
> > > > > +                   };
> > > > > +           };
> > > > > +
> > > > > +           sata_phy: sata-phy@1c180c0  {
> > > > > +                   compatible = "allwinner,sun8i-r40-sata-phy";
> > > > > +                   reg = <0x1c180c0 0x200>;
> > > >
> > > > Overlapping devices in the DTS is not ok.
> > > >
> > >
> > > I do the same than arch/arm/boot/dts/berlin2.dtsi (sata@e90000 phy@e900a0)
> > > But since it is not a good justification, it seems that regmap is my only solution ?
> >
> > Since you are effectively splitting one device node into two, you should
> > adjust the original (ahci) device nodes register range.
> >
>
> I cannot do that if I remove patch 13, iow If I keep phy init code in both place as you requested.

Then you need to split the phy handling between a10 and r40. A10 doesn't
need phy-supply at the moment anyway. And migrating A10 to newer binding
is only causing you problems anyway. Just drop that part, and handle the
R40.

ChenYu

Re: [PATCH v4 09/13] ARM: dts: sun8i: r40: add sata node

[Chen-Yu Tsai]

On Fri, Aug 31, 2018 at 6:54 PM Corentin Labbe
<clabbe.montjoie@gmail.com> wrote:
>
> On Fri, Aug 31, 2018 at 12:20:21PM +0200, maxime.ripard@bootlin.com wrote:
> > On Fri, Aug 31, 2018 at 09:56:31AM +0200, Corentin Labbe wrote:
> > > On Fri, Aug 31, 2018 at 09:35:00AM +0200, Maxime Ripard wrote:
> > > > On Thu, Aug 30, 2018 at 09:01:16PM +0200, Corentin Labbe wrote:
> > > > > 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.montjoie@gmail.com>
> > > > > ---
> > > > >  arch/arm/boot/dts/sun8i-r40.dtsi | 23 +++++++++++++++++++++++
> > > > >  1 file changed, 23 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > > index 852c2ccc3268..d6b5820da850 100644
> > > > > --- a/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > > @@ -550,6 +550,29 @@
> > > > >                         #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>;
> > > > > +                       resets-name = "ahci";
> > > > > +                       #address-cells = <1>;
> > > > > +                       #size-cells = <0>;
> > > > > +                       status = "disabled";
> > > > > +
> > > > > +                       sata_port: sata-port@0 {
> > > > > +                               reg = <0>;
> > > > > +                               phys = <&sata_phy>;
> > > > > +                       };
> > > > > +               };
> > > > > +
> > > > > +               sata_phy: sata-phy@1c180c0  {
> > > > > +                       compatible = "allwinner,sun8i-r40-sata-phy";
> > > > > +                       reg = <0x1c180c0 0x200>;
> > > >
> > > > Overlapping devices in the DTS is not ok.
> > > >
> > >
> > > I do the same than arch/arm/boot/dts/berlin2.dtsi (sata@e90000
> > > phy@e900a0)
> > >
> > > But since it is not a good justification, it seems that regmap is my
> > > only solution ?
> >
> > I'm not even sure why you are moving the phy out of its original node
> > (and driver).
> >
>
> For using the phy-supply already handled by the code.
> The other choice is to add another xxx-supply to ahci_platform.
> Or to use hackily port_regulator for this regulator.

The PHY registers are in the AHCI's "vendor specific registers"
region. Following that are the per-port registers, which the ahci
driver will need access to. This doesn't look like it should
deserve a separate device node.

What's wrong with handling the regulator directly in the ahci-sunxi
PHY init code?

ChenYu

Re: [PATCH v4 09/13] ARM: dts: sun8i: r40: add sata node

[Corentin Labbe]

On Fri, Aug 31, 2018 at 07:31:37PM +0800, Chen-Yu Tsai wrote:
> On Fri, Aug 31, 2018 at 6:54 PM Corentin Labbe
> <clabbe.montjoie@gmail.com> wrote:
> >
> > On Fri, Aug 31, 2018 at 12:20:21PM +0200, maxime.ripard@bootlin.com wrote:
> > > On Fri, Aug 31, 2018 at 09:56:31AM +0200, Corentin Labbe wrote:
> > > > On Fri, Aug 31, 2018 at 09:35:00AM +0200, Maxime Ripard wrote:
> > > > > On Thu, Aug 30, 2018 at 09:01:16PM +0200, Corentin Labbe wrote:
> > > > > > 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.montjoie@gmail.com>
> > > > > > ---
> > > > > >  arch/arm/boot/dts/sun8i-r40.dtsi | 23 +++++++++++++++++++++++
> > > > > >  1 file changed, 23 insertions(+)
> > > > > >
> > > > > > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > > > index 852c2ccc3268..d6b5820da850 100644
> > > > > > --- a/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > > > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > > > @@ -550,6 +550,29 @@
> > > > > >                         #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>;
> > > > > > +                       resets-name = "ahci";
> > > > > > +                       #address-cells = <1>;
> > > > > > +                       #size-cells = <0>;
> > > > > > +                       status = "disabled";
> > > > > > +
> > > > > > +                       sata_port: sata-port@0 {
> > > > > > +                               reg = <0>;
> > > > > > +                               phys = <&sata_phy>;
> > > > > > +                       };
> > > > > > +               };
> > > > > > +
> > > > > > +               sata_phy: sata-phy@1c180c0  {
> > > > > > +                       compatible = "allwinner,sun8i-r40-sata-phy";
> > > > > > +                       reg = <0x1c180c0 0x200>;
> > > > >
> > > > > Overlapping devices in the DTS is not ok.
> > > > >
> > > >
> > > > I do the same than arch/arm/boot/dts/berlin2.dtsi (sata@e90000
> > > > phy@e900a0)
> > > >
> > > > But since it is not a good justification, it seems that regmap is my
> > > > only solution ?
> > >
> > > I'm not even sure why you are moving the phy out of its original node
> > > (and driver).
> > >
> >
> > For using the phy-supply already handled by the code.
> > The other choice is to add another xxx-supply to ahci_platform.
> > Or to use hackily port_regulator for this regulator.
> 
> The PHY registers are in the AHCI's "vendor specific registers"
> region. Following that are the per-port registers, which the ahci
> driver will need access to. This doesn't look like it should
> deserve a separate device node.
> 
> What's wrong with handling the regulator directly in the ahci-sunxi
> PHY init code?
> 

The reason are that I didnt wanted to use the port-regulator, and I didnt want to add new code to ahci_platform.
I tried to place a phy-supply in the ahci node, but it doesnt work (with or without a phy subnode).

Moving PHy code in a dedicated driver seemed to be good, but with all your comments and Maxime's one, it seems not.

I will keep ahci_sunxi as-is and will try to found how to add the needed phy-supply.

Regards

Re: [PATCH v4 09/13] ARM: dts: sun8i: r40: add sata node

[Icenowy Zheng]

在 2018-08-31五的 19:10 +0800，Chen-Yu Tsai写道：
> On Fri, Aug 31, 2018 at 5:29 PM Corentin Labbe
> <clabbe.montjoie@gmail.com> wrote:
> > 
> > On Fri, Aug 31, 2018 at 03:58:34PM +0800, Chen-Yu Tsai wrote:
> > > On Fri, Aug 31, 2018 at 3:56 PM Corentin Labbe
> > > <clabbe.montjoie@gmail.com> wrote:
> > > > 
> > > > On Fri, Aug 31, 2018 at 09:35:00AM +0200, Maxime Ripard wrote:
> > > > > On Thu, Aug 30, 2018 at 09:01:16PM +0200, Corentin Labbe
> > > > > wrote:
> > > > > > 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.montjoie@gmail.com>
> > > > > > ---
> > > > > >  arch/arm/boot/dts/sun8i-r40.dtsi | 23
> > > > > > +++++++++++++++++++++++
> > > > > >  1 file changed, 23 insertions(+)
> > > > > > 
> > > > > > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > > > b/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > > > index 852c2ccc3268..d6b5820da850 100644
> > > > > > --- a/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > > > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > > > @@ -550,6 +550,29 @@
> > > > > >                     #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>;
> > > > > > +                   resets-name = "ahci";
> > > > > > +                   #address-cells = <1>;
> > > > > > +                   #size-cells = <0>;
> > > > > > +                   status = "disabled";
> > > > > > +
> > > > > > +                   sata_port: sata-port@0 {
> > > > > > +                           reg = <0>;
> > > > > > +                           phys = <&sata_phy>;
> > > > > > +                   };
> > > > > > +           };
> > > > > > +
> > > > > > +           sata_phy: sata-phy@1c180c0  {
> > > > > > +                   compatible = "allwinner,sun8i-r40-sata-
> > > > > > phy";
> > > > > > +                   reg = <0x1c180c0 0x200>;
> > > > > 
> > > > > Overlapping devices in the DTS is not ok.
> > > > > 
> > > > 
> > > > I do the same than arch/arm/boot/dts/berlin2.dtsi (sata@e90000 
> > > > phy@e900a0)
> > > > But since it is not a good justification, it seems that regmap
> > > > is my only solution ?
> > > 
> > > Since you are effectively splitting one device node into two, you
> > > should
> > > adjust the original (ahci) device nodes register range.
> > > 
> > 
> > I cannot do that if I remove patch 13, iow If I keep phy init code
> > in both place as you requested.
> 
> Then you need to split the phy handling between a10 and r40. A10
> doesn't
> need phy-supply at the moment anyway. And migrating A10 to newer
> binding
> is only causing you problems anyway. Just drop that part, and handle
> the
> R40.

From the hardware perspective, they're the same. The A10/A20 has also
two dedicated VDD pins for SATA, although on boards they're usually
always on.

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

Re: [PATCH v4 09/13] ARM: dts: sun8i: r40: add sata node

[Icenowy Zheng]

在 2018-08-31五的 19:31 +0800，Chen-Yu Tsai写道：
> On Fri, Aug 31, 2018 at 6:54 PM Corentin Labbe
> <clabbe.montjoie@gmail.com> wrote:
> > 
> > On Fri, Aug 31, 2018 at 12:20:21PM +0200, maxime.ripard@bootlin.com
> >  wrote:
> > > On Fri, Aug 31, 2018 at 09:56:31AM +0200, Corentin Labbe wrote:
> > > > On Fri, Aug 31, 2018 at 09:35:00AM +0200, Maxime Ripard wrote:
> > > > > On Thu, Aug 30, 2018 at 09:01:16PM +0200, Corentin Labbe
> > > > > wrote:
> > > > > > 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.montjoie@gmail.com>
> > > > > > ---
> > > > > >  arch/arm/boot/dts/sun8i-r40.dtsi | 23
> > > > > > +++++++++++++++++++++++
> > > > > >  1 file changed, 23 insertions(+)
> > > > > > 
> > > > > > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > > > b/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > > > index 852c2ccc3268..d6b5820da850 100644
> > > > > > --- a/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > > > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > > > @@ -550,6 +550,29 @@
> > > > > >                         #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>;
> > > > > > +                       resets-name = "ahci";
> > > > > > +                       #address-cells = <1>;
> > > > > > +                       #size-cells = <0>;
> > > > > > +                       status = "disabled";
> > > > > > +
> > > > > > +                       sata_port: sata-port@0 {
> > > > > > +                               reg = <0>;
> > > > > > +                               phys = <&sata_phy>;
> > > > > > +                       };
> > > > > > +               };
> > > > > > +
> > > > > > +               sata_phy: sata-phy@1c180c0  {
> > > > > > +                       compatible = "allwinner,sun8i-r40-
> > > > > > sata-phy";
> > > > > > +                       reg = <0x1c180c0 0x200>;
> > > > > 
> > > > > Overlapping devices in the DTS is not ok.
> > > > > 
> > > > 
> > > > I do the same than arch/arm/boot/dts/berlin2.dtsi (sata@e90000
> > > > phy@e900a0)
> > > > 
> > > > But since it is not a good justification, it seems that regmap
> > > > is my
> > > > only solution ?
> > > 
> > > I'm not even sure why you are moving the phy out of its original
> > > node
> > > (and driver).
> > > 
> > 
> > For using the phy-supply already handled by the code.
> > The other choice is to add another xxx-supply to ahci_platform.
> > Or to use hackily port_regulator for this regulator.
> 
> The PHY registers are in the AHCI's "vendor specific registers"
> region. Following that are the per-port registers, which the ahci
> driver will need access to. This doesn't look like it should
> deserve a separate device node.
> 
> What's wrong with handling the regulator directly in the ahci-sunxi
> PHY init code?

I remember I sent a patch that did this some times ago, and gets
rejected.

> 
> ChenYu

Re: [PATCH v4 00/13] ata: ahci_platform: support allwinner R40 AHCI

[Jens Axboe]

On 8/31/18 1:40 AM, Maxime Ripard wrote:
> Hi Jens,
> 
> On Thu, Aug 30, 2018 at 08:52:52PM -0600, Jens Axboe wrote:
>> On 8/30/18 8:32 PM, Chen-Yu Tsai wrote:
>>> Hi,
>>>
>>> On Fri, Aug 31, 2018 at 4:31 AM Jens Axboe <axboe@kernel.dk> wrote:
>>>>
>>>> On 8/30/18 1:01 PM, Corentin Labbe wrote:
>>>>> Hello
>>>>>
>>>>> This patchset add support for allwinner R40 AHCI controller.
>>>>>
>>>>> 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.
>>>>>
>>>>> The last patch(ata: ahci_sunxi: remove PHY code) should not be merged,
>>>>> but will be resent for inclustion when all patchs will have hit linus
>>>>> tree.
>>>>
>>>> Applied 1-12 with Hans's blessing, thanks Corentin.
>>>
>>> Please don't merge device tree ("dts") patches, i.e. patches 9-12. We will
>>> merge them through the sunxi / armsoc tree. Having them in separate trees
>>> introduces conflicts when we have other stuff going through our tree.
>>
>> OK, fair enough, I can drop those.
> 
> And the DT bits actually have some issues that would need to change
> the code significantly.
> 
> Can you drop all of them please?

OK, can do. Corentin, please resend when everybody is happy with it.

-- 
Jens Axboe

Re: [PATCH v4 00/13] ata: ahci_platform: support allwinner R40 AHCI

[Maxime Ripard]

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

On Fri, Aug 31, 2018 at 08:35:23AM -0600, Jens Axboe wrote:
> On 8/31/18 1:40 AM, Maxime Ripard wrote:
> > Hi Jens,
11;rgb:0000/2b2b/3636> > 
> > On Thu, Aug 30, 2018 at 08:52:52PM -0600, Jens Axboe wrote:
> >> On 8/30/18 8:32 PM, Chen-Yu Tsai wrote:
> >>> Hi,
> >>>
> >>> On Fri, Aug 31, 2018 at 4:31 AM Jens Axboe <axboe@kernel.dk> wrote:
> >>>>
> >>>> On 8/30/18 1:01 PM, Corentin Labbe wrote:
> >>>>> Hello
> >>>>>
> >>>>> This patchset add support for allwinner R40 AHCI controller.
> >>>>>
> >>>>> 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.
> >>>>>
> >>>>> The last patch(ata: ahci_sunxi: remove PHY code) should not be merged,
> >>>>> but will be resent for inclustion when all patchs will have hit linus
> >>>>> tree.
> >>>>
> >>>> Applied 1-12 with Hans's blessing, thanks Corentin.
> >>>
> >>> Please don't merge device tree ("dts") patches, i.e. patches 9-12. We will
> >>> merge them through the sunxi / armsoc tree. Having them in separate trees
> >>> introduces conflicts when we have other stuff going through our tree.
> >>
> >> OK, fair enough, I can drop those.
> > 
> > And the DT bits actually have some issues that would need to change
> > the code significantly.
> > 
> > Can you drop all of them please?
> 
> OK, can do.

Thanks!

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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