[PATCH v9 0/7] ARM: berlin: add AHCI support

[PATCH v9 0/7] ARM: berlin: add AHCI support

[Antoine Ténart]

This series adds the support for Berlin SoC AHCI controller. The
controller allows to use the SATA host interface and, for example, the
eSATA port on the BG2Q.

The series adds a PHY driver to control the two SATA ports available,
and adds a generic compatible to use the existing ahci_platform driver.

Also enable the eSATA interface on the BG2Q DMP.

Changes since v8:
	- stopped reset the controller from the PHY driver
	- removed fixed array sizes
	- got rid of the custom to_berlin_sata_phy_priv() macro
	- added dependency to HAS_IOMEM

Changes since v7:
        - got back to the each PHY as a sub-node representation
        - renamed the power bit in the PHY driver

Changes since v6:
        - added the 'clocks' property and support in the PHY driver
        - updated the PHY compatible

Changes since v5:
        - rebased on top of v3.16-rc1
        - added the 'clocks' property in the sata node

Changes since v4:
        - updated PHY driver as tristate
        - handled the case were no SATA port is enabled
        - updated the compatible to a generic one
        - cosmetic fixups

Changes since v3:
        - moved all PHY operations to the PHY driver
        - removed PHY sub-nodes
        - removed the custom Berlin AHCI driver and switched to
          ahci_platform
        - added multiple PHYs support to the libahci_platform

Changes since v2:
        - modeled each PHY as a sub-node
        - cosmetic fixups

Changes since v1:
        - added a PHY driver, allowing to enable each port
          individually and removed the 'force-port-map' property
        - made the drivers a bit less magic :)
        - wrote a function to select and configure registers in the
          AHCI driver
        - removed BG2 / BG2CD nodes

Antoine T?nart (7):
  phy: add a driver for the Berlin SATA PHY
  Documentation: bindings: add the Berlin SATA PHY
  ata: libahci: allow to use multiple PHYs
  ata: ahci_platform: add a generic AHCI compatible
  Documentation: bindings: document the sub-nodes AHCI bindings
  ARM: berlin: add the AHCI node for the BG2Q
  ARM: berlin: enable the eSATA interface on the BG2Q DMP

 .../devicetree/bindings/ata/ahci-platform.txt      |  37 +++
 .../devicetree/bindings/phy/berlin-sata-phy.txt    |  34 +++
 arch/arm/boot/dts/berlin2q-marvell-dmp.dts         |   8 +
 arch/arm/boot/dts/berlin2q.dtsi                    |  39 +++
 drivers/ata/ahci.h                                 |   3 +-
 drivers/ata/ahci_platform.c                        |   2 +
 drivers/ata/libahci.c                              |   7 +
 drivers/ata/libahci_platform.c                     | 170 +++++++++---
 drivers/phy/Kconfig                                |   7 +
 drivers/phy/Makefile                               |   1 +
 drivers/phy/phy-berlin-sata.c                      | 284 +++++++++++++++++++++
 11 files changed, 556 insertions(+), 36 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/berlin-sata-phy.txt
 create mode 100644 drivers/phy/phy-berlin-sata.c

-- 
1.9.1

[PATCH v9 1/7] phy: add a driver for the Berlin SATA PHY

[Antoine Ténart]

The Berlin SoC has a two SATA ports. Add a PHY driver to handle them.

The mode selection can let us think this PHY can be configured to fit
other purposes. But there are reasons to think the SATA mode will be
the only one usable: the PHY registers are only accessible indirectly
through two registers in the SATA range, the PHY seems to be integrated
and no information tells us the contrary. For these reasons, make the
driver a SATA PHY driver.

Signed-off-by: Antoine T?nart <antoine.tenart@free-electrons.com>
---
 drivers/phy/Kconfig           |   7 ++
 drivers/phy/Makefile          |   1 +
 drivers/phy/phy-berlin-sata.c | 284 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 292 insertions(+)
 create mode 100644 drivers/phy/phy-berlin-sata.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 16a2f067c242..8ad5c341b8f9 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -15,6 +15,13 @@ config GENERIC_PHY
 	  phy users can obtain reference to the PHY. All the users of this
 	  framework should select this config.
 
+config PHY_BERLIN_SATA
+	tristate "Marvell Berlin SATA PHY driver"
+	depends on ARCH_BERLIN && HAS_IOMEM && OF
+	select GENERIC_PHY
+	help
+	  Enable this to support the SATA PHY on Marvell Berlin SoCs.
+
 config PHY_EXYNOS_MIPI_VIDEO
 	tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
 	depends on HAS_IOMEM
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index b4f1d5770601..a137a2e23218 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -3,6 +3,7 @@
 #
 
 obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
+obj-$(CONFIG_PHY_BERLIN_SATA)		+= phy-berlin-sata.o
 obj-$(CONFIG_BCM_KONA_USB2_PHY)		+= phy-bcm-kona-usb2.o
 obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
 obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
diff --git a/drivers/phy/phy-berlin-sata.c b/drivers/phy/phy-berlin-sata.c
new file mode 100644
index 000000000000..c5e688b0899f
--- /dev/null
+++ b/drivers/phy/phy-berlin-sata.c
@@ -0,0 +1,284 @@
+/*
+ * Marvell Berlin SATA PHY driver
+ *
+ * Copyright (C) 2014 Marvell Technology Group Ltd.
+ *
+ * Antoine T?nart <antoine.tenart@free-electrons.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/phy/phy.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+
+#define HOST_VSA_ADDR		0x0
+#define HOST_VSA_DATA		0x4
+#define PORT_SCR_CTL		0x2c
+#define PORT_VSR_ADDR		0x78
+#define PORT_VSR_DATA		0x7c
+
+#define CONTROL_REGISTER	0x0
+#define MBUS_SIZE_CONTROL	0x4
+
+#define POWER_DOWN_PHY0			BIT(6)
+#define POWER_DOWN_PHY1			BIT(14)
+#define MBUS_WRITE_REQUEST_SIZE_128	(BIT(2) << 16)
+#define MBUS_READ_REQUEST_SIZE_128	(BIT(2) << 19)
+
+#define PHY_BASE		0x200
+
+/* register 0x01 */
+#define REF_FREF_SEL_25		BIT(0)
+#define PHY_MODE_SATA		(0x0 << 5)
+
+/* register 0x02 */
+#define USE_MAX_PLL_RATE	BIT(12)
+
+/* register 0x23 */
+#define DATA_BIT_WIDTH_10	(0x0 << 10)
+#define DATA_BIT_WIDTH_20	(0x1 << 10)
+#define DATA_BIT_WIDTH_40	(0x2 << 10)
+
+/* register 0x25 */
+#define PHY_GEN_MAX_1_5		(0x0 << 10)
+#define PHY_GEN_MAX_3_0		(0x1 << 10)
+#define PHY_GEN_MAX_6_0		(0x2 << 10)
+
+struct phy_berlin_desc {
+	struct phy	*phy;
+	u32		power_bit;
+	unsigned	index;
+};
+
+struct phy_berlin_priv {
+	void __iomem		*base;
+	spinlock_t		lock;
+	struct clk		*clk;
+	struct phy_berlin_desc	**phys;
+	unsigned		nphys;
+};
+
+static inline void phy_berlin_sata_reg_setbits(void __iomem *ctrl_reg, u32 reg,
+					       u32 mask, u32 val)
+{
+	u32 regval;
+
+	/* select register */
+	writel(PHY_BASE + reg, ctrl_reg + PORT_VSR_ADDR);
+
+	/* set bits */
+	regval = readl(ctrl_reg + PORT_VSR_DATA);
+	regval &= ~mask;
+	regval |= val;
+	writel(regval, ctrl_reg + PORT_VSR_DATA);
+}
+
+static int phy_berlin_sata_power_on(struct phy *phy)
+{
+	struct phy_berlin_desc *desc = phy_get_drvdata(phy);
+	struct phy_berlin_priv *priv = dev_get_drvdata(phy->dev.parent);
+	void __iomem *ctrl_reg = priv->base + 0x60 + (desc->index * 0x80);
+	int ret = 0;
+	u32 regval;
+
+	clk_prepare_enable(priv->clk);
+
+	spin_lock(&priv->lock);
+
+	/* Power on PHY */
+	writel(CONTROL_REGISTER, priv->base + HOST_VSA_ADDR);
+	regval = readl(priv->base + HOST_VSA_DATA);
+	regval &= ~desc->power_bit;
+	writel(regval, priv->base + HOST_VSA_DATA);
+
+	/* Configure MBus */
+	writel(MBUS_SIZE_CONTROL, priv->base + HOST_VSA_ADDR);
+	regval = readl(priv->base + HOST_VSA_DATA);
+	regval |= MBUS_WRITE_REQUEST_SIZE_128 | MBUS_READ_REQUEST_SIZE_128;
+	writel(regval, priv->base + HOST_VSA_DATA);
+
+	/* set PHY mode and ref freq to 25 MHz */
+	phy_berlin_sata_reg_setbits(ctrl_reg, 0x1, 0xff,
+				    REF_FREF_SEL_25 | PHY_MODE_SATA);
+
+	/* set PHY up to 6 Gbps */
+	phy_berlin_sata_reg_setbits(ctrl_reg, 0x25, 0xc00, PHY_GEN_MAX_6_0);
+
+	/* set 40 bits width */
+	phy_berlin_sata_reg_setbits(ctrl_reg, 0x23,  0xc00, DATA_BIT_WIDTH_40);
+
+	/* use max pll rate */
+	phy_berlin_sata_reg_setbits(ctrl_reg, 0x2, 0x0, USE_MAX_PLL_RATE);
+
+	/* set Gen3 controller speed */
+	regval = readl(ctrl_reg + PORT_SCR_CTL);
+	regval &= ~GENMASK(7, 4);
+	regval |= 0x30;
+	writel(regval, ctrl_reg + PORT_SCR_CTL);
+
+	spin_unlock(&priv->lock);
+
+	clk_disable_unprepare(priv->clk);
+
+	return ret;
+}
+
+static int phy_berlin_sata_power_off(struct phy *phy)
+{
+	struct phy_berlin_desc *desc = phy_get_drvdata(phy);
+	struct phy_berlin_priv *priv = dev_get_drvdata(phy->dev.parent);
+	u32 regval;
+
+	clk_prepare_enable(priv->clk);
+
+	spin_lock(&priv->lock);
+
+	/* Power down PHY */
+	writel(CONTROL_REGISTER, priv->base + HOST_VSA_ADDR);
+	regval = readl(priv->base + HOST_VSA_DATA);
+	regval |= desc->power_bit;
+	writel(regval, priv->base + HOST_VSA_DATA);
+
+	spin_unlock(&priv->lock);
+
+	clk_disable_unprepare(priv->clk);
+
+	return 0;
+}
+
+static struct phy *phy_berlin_sata_phy_xlate(struct device *dev,
+					     struct of_phandle_args *args)
+{
+	struct phy_berlin_priv *priv = dev_get_drvdata(dev);
+	int i;
+
+	if (WARN_ON(args->args[0] >= priv->nphys))
+		return ERR_PTR(-ENODEV);
+
+	for (i = 0; i < priv->nphys; i++) {
+		if (priv->phys[i]->index == args->args[0])
+			break;
+	}
+
+	if (i == priv->nphys)
+		return ERR_PTR(-ENODEV);
+
+	return priv->phys[i]->phy;
+}
+
+static struct phy_ops phy_berlin_sata_ops = {
+	.power_on	= phy_berlin_sata_power_on,
+	.power_off	= phy_berlin_sata_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static u32 phy_berlin_power_down_bits[] = {
+	POWER_DOWN_PHY0,
+	POWER_DOWN_PHY1,
+};
+
+static int phy_berlin_sata_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *child;
+	struct phy *phy;
+	struct phy_provider *phy_provider;
+	struct phy_berlin_priv *priv;
+	struct resource *res;
+	int i = 0;
+	u32 phy_id;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -EINVAL;
+
+	priv->base = devm_ioremap(dev, res->start, resource_size(res));
+	if (!priv->base)
+		return -ENOMEM;
+
+	priv->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(priv->clk))
+		return PTR_ERR(priv->clk);
+
+	priv->nphys = of_get_child_count(dev->of_node);
+	if (priv->nphys == 0)
+		return -ENODEV;
+
+	priv->phys = devm_kzalloc(dev, priv->nphys * sizeof(*priv->phys),
+				  GFP_KERNEL);
+	if (!priv->phys)
+		return -ENOMEM;
+
+	dev_set_drvdata(dev, priv);
+	spin_lock_init(&priv->lock);
+
+	for_each_available_child_of_node(dev->of_node, child) {
+		struct phy_berlin_desc *phy_desc;
+
+		if (of_property_read_u32(child, "reg", &phy_id)) {
+			dev_err(dev, "missing reg property in node %s\n",
+				child->name);
+			return -EINVAL;
+		}
+
+		if (phy_id >= ARRAY_SIZE(phy_berlin_power_down_bits)) {
+			dev_err(dev, "invalid reg in node %s\n", child->name);
+			return -EINVAL;
+		}
+
+		phy_desc = devm_kzalloc(dev, sizeof(*phy_desc), GFP_KERNEL);
+		if (!phy_desc)
+			return -ENOMEM;
+
+		phy = devm_phy_create(dev, &phy_berlin_sata_ops, NULL);
+		if (IS_ERR(phy)) {
+			dev_err(dev, "failed to create PHY %d\n", phy_id);
+			return PTR_ERR(phy);
+		}
+
+		phy_desc->phy = phy;
+		phy_desc->power_bit = phy_berlin_power_down_bits[phy_id];
+		phy_desc->index = phy_id;
+		phy_set_drvdata(phy, phy_desc);
+
+		priv->phys[i++] = phy_desc;
+
+		/* Make sure the PHY is off */
+		phy_berlin_sata_power_off(phy);
+	}
+
+	phy_provider =
+		devm_of_phy_provider_register(dev, phy_berlin_sata_phy_xlate);
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	return 0;
+}
+
+static const struct of_device_id phy_berlin_sata_of_match[] = {
+	{ .compatible = "marvell,berlin2q-sata-phy" },
+	{ },
+};
+
+static struct platform_driver phy_berlin_sata_driver = {
+	.probe	= phy_berlin_sata_probe,
+	.driver	= {
+		.name		= "phy-berlin-sata",
+		.owner		= THIS_MODULE,
+		.of_match_table	= phy_berlin_sata_of_match,
+	},
+};
+module_platform_driver(phy_berlin_sata_driver);
+
+MODULE_DESCRIPTION("Marvell Berlin SATA PHY driver");
+MODULE_AUTHOR("Antoine T?nart <antoine.tenart@free-electrons.com>");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

[PATCH v9 2/7] Documentation: bindings: add the Berlin SATA PHY

[Antoine Ténart]

The Berlin SATA PHY drives the PHY related to the SATA interface. Add
the corresponding documentation.

Signed-off-by: Antoine T?nart <antoine.tenart@free-electrons.com>
---
 .../devicetree/bindings/phy/berlin-sata-phy.txt    | 34 ++++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/berlin-sata-phy.txt

diff --git a/Documentation/devicetree/bindings/phy/berlin-sata-phy.txt b/Documentation/devicetree/bindings/phy/berlin-sata-phy.txt
new file mode 100644
index 000000000000..88f8c23384c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/berlin-sata-phy.txt
@@ -0,0 +1,34 @@
+Berlin SATA PHY
+---------------
+
+Required properties:
+- compatible: should be "marvell,berlin2q-sata-phy"
+- address-cells: should be 1
+- size-cells: should be 0
+- phy-cells: from the generic PHY bindings, must be 1
+- reg: address and length of the register
+- clocks: reference to the clock entry
+
+Sub-nodes:
+Each PHY should be represented as a sub-node.
+
+Sub-nodes required properties:
+- reg: the PHY number
+
+Example:
+	sata_phy: phy at f7e900a0 {
+		compatible = "marvell,berlin2q-sata-phy";
+		reg = <0xf7e900a0 0x200>;
+		clocks = <&chip CLKID_SATA>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		#phy-cells = <1>;
+
+		sata-phy at 0 {
+			reg = <0>;
+		};
+
+		sata-phy at 1 {
+			reg = <1>;
+		};
+	};
-- 
1.9.1

[PATCH v9 3/7] ata: libahci: allow to use multiple PHYs

[Antoine Ténart]

The current implementation of the libahci does not allow to use multiple
PHYs. This patch adds the support of multiple PHYs by the libahci while
keeping the old bindings valid for device tree compatibility.

This introduce a new way of defining SATA ports in the device tree, with
one port per sub-node. This as the advantage of allowing a per port
configuration. Because some ports may be accessible but disabled in the
device tree, the default port_map is computed automatically when using
this.

Signed-off-by: Antoine T?nart <antoine.tenart@free-electrons.com>
---
 drivers/ata/ahci.h             |   3 +-
 drivers/ata/libahci.c          |   7 ++
 drivers/ata/libahci_platform.c | 170 ++++++++++++++++++++++++++++++++---------
 3 files changed, 144 insertions(+), 36 deletions(-)

diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 05882e4445a6..e5bdbf827ca8 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -330,7 +330,8 @@ struct ahci_host_priv {
 	bool			got_runtime_pm; /* Did we do pm_runtime_get? */
 	struct clk		*clks[AHCI_MAX_CLKS]; /* Optional */
 	struct regulator	*target_pwr;	/* Optional */
-	struct phy		*phy;		/* If platform uses phy */
+	struct phy		**phys;		/* If platform uses phys */
+	unsigned		nphys;		/* Number of phys */
 	void			*plat_data;	/* Other platform data */
 	/*
 	 * Optional ahci_start_engine override, if not set this gets set to the
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 40ea583d3610..0bdee8330dcf 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -482,6 +482,13 @@ void ahci_save_initial_config(struct device *dev,
 		port_map &= mask_port_map;
 	}
 
+	/*
+	 * If port_map was filled automatically when finding port sub-nodes,
+	 * make sure we get the right set here.
+	 */
+	if (hpriv->port_map)
+		port_map &= hpriv->port_map;
+
 	/* cross check port_map and cap.n_ports */
 	if (port_map) {
 		int map_ports = 0;
diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
index 3a5b4ed25a4f..cd2bc50ba913 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -39,6 +39,61 @@ static struct scsi_host_template ahci_platform_sht = {
 };
 
 /**
+ * ahci_platform_enable_phys - Enable PHYs
+ * @hpriv: host private area to store config values
+ *
+ * This function enables all the PHYs found in hpriv->phys, if any.
+ * If a PHY fails to be enabled, it disables all the PHYs already
+ * enabled in reverse order and returns an error.
+ *
+ * RETURNS:
+ * 0 on success otherwise a negative error code
+ */
+int ahci_platform_enable_phys(struct ahci_host_priv *hpriv)
+{
+	int i, rc = 0;
+
+	for (i = 0; i < hpriv->nphys; i++) {
+		rc = phy_init(hpriv->phys[i]);
+		if (rc)
+			goto disable_phys;
+
+		rc = phy_power_on(hpriv->phys[i]);
+		if (rc) {
+			phy_exit(hpriv->phys[i]);
+			goto disable_phys;
+		}
+	}
+
+	return 0;
+
+disable_phys:
+	while (--i >= 0) {
+		phy_power_off(hpriv->phys[i]);
+		phy_exit(hpriv->phys[i]);
+	}
+	return rc;
+}
+EXPORT_SYMBOL_GPL(ahci_platform_enable_phys);
+
+/**
+ * ahci_platform_disable_phys - Enable PHYs
+ * @hpriv: host private area to store config values
+ *
+ * This function disables all PHYs found in hpriv->phys.
+ */
+void ahci_platform_disable_phys(struct ahci_host_priv *hpriv)
+{
+	int i;
+
+	for (i = 0; i < hpriv->nphys; i++) {
+		phy_power_off(hpriv->phys[i]);
+		phy_exit(hpriv->phys[i]);
+	}
+}
+EXPORT_SYMBOL_GPL(ahci_platform_disable_phys);
+
+/**
  * ahci_platform_enable_clks - Enable platform clocks
  * @hpriv: host private area to store config values
  *
@@ -92,7 +147,7 @@ EXPORT_SYMBOL_GPL(ahci_platform_disable_clks);
  * following order:
  * 1) Regulator
  * 2) Clocks (through ahci_platform_enable_clks)
- * 3) Phy
+ * 3) Phys
  *
  * If resource enabling fails at any point the previous enabled resources
  * are disabled in reverse order.
@@ -114,17 +169,9 @@ int ahci_platform_enable_resources(struct ahci_host_priv *hpriv)
 	if (rc)
 		goto disable_regulator;
 
-	if (hpriv->phy) {
-		rc = phy_init(hpriv->phy);
-		if (rc)
-			goto disable_clks;
-
-		rc = phy_power_on(hpriv->phy);
-		if (rc) {
-			phy_exit(hpriv->phy);
-			goto disable_clks;
-		}
-	}
+	rc = ahci_platform_enable_phys(hpriv);
+	if (rc)
+		goto disable_clks;
 
 	return 0;
 
@@ -144,16 +191,13 @@ EXPORT_SYMBOL_GPL(ahci_platform_enable_resources);
  *
  * This function disables all ahci_platform managed resources in the
  * following order:
- * 1) Phy
+ * 1) Phys
  * 2) Clocks (through ahci_platform_disable_clks)
  * 3) Regulator
  */
 void ahci_platform_disable_resources(struct ahci_host_priv *hpriv)
 {
-	if (hpriv->phy) {
-		phy_power_off(hpriv->phy);
-		phy_exit(hpriv->phy);
-	}
+	ahci_platform_disable_phys(hpriv);
 
 	ahci_platform_disable_clks(hpriv);
 
@@ -187,7 +231,7 @@ static void ahci_platform_put_resources(struct device *dev, void *res)
  * 2) regulator for controlling the targets power (optional)
  * 3) 0 - AHCI_MAX_CLKS clocks, as specified in the devs devicetree node,
  *    or for non devicetree enabled platforms a single clock
- *	4) phy (optional)
+ *	4) phys (optional)
  *
  * RETURNS:
  * The allocated ahci_host_priv on success, otherwise an ERR_PTR value
@@ -197,7 +241,8 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct ahci_host_priv *hpriv;
 	struct clk *clk;
-	int i, rc = -ENOMEM;
+	struct device_node *child;
+	int i, nports, rc = -ENOMEM;
 
 	if (!devres_open_group(dev, NULL, GFP_KERNEL))
 		return ERR_PTR(-ENOMEM);
@@ -246,22 +291,77 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev)
 		hpriv->clks[i] = clk;
 	}
 
-	hpriv->phy = devm_phy_get(dev, "sata-phy");
-	if (IS_ERR(hpriv->phy)) {
-		rc = PTR_ERR(hpriv->phy);
-		switch (rc) {
-		case -ENODEV:
-		case -ENOSYS:
-			/* continue normally */
-			hpriv->phy = NULL;
-			break;
+	nports = of_get_child_count(dev->of_node);
 
-		case -EPROBE_DEFER:
+	if (nports) {
+		hpriv->phys = devm_kzalloc(dev, nports * sizeof(*hpriv->phys),
+					   GFP_KERNEL);
+		if (!hpriv->phys) {
+			rc = -ENOMEM;
 			goto err_out;
+		}
 
-		default:
-			dev_err(dev, "couldn't get sata-phy\n");
-			goto err_out;
+		for_each_child_of_node(dev->of_node, child) {
+			u32 port;
+
+			if (!of_device_is_available(child))
+				continue;
+
+			if (of_property_read_u32(child, "reg", &port)) {
+				rc = -EINVAL;
+				goto err_out;
+			}
+
+			hpriv->port_map |= BIT(port);
+
+			hpriv->phys[hpriv->nphys] = devm_of_phy_get(dev, child,
+								    NULL);
+			if (IS_ERR(hpriv->phys[hpriv->nphys])) {
+				rc = PTR_ERR(hpriv->phys[hpriv->nphys]);
+				dev_err(dev,
+					"couldn't get PHY in node %s: %d\n",
+					child->name, rc);
+				goto err_out;
+			}
+
+			hpriv->nphys++;
+		}
+
+		if (!hpriv->nphys) {
+			dev_warn(dev, "No port enabled\n");
+			return ERR_PTR(-ENODEV);
+		}
+	} else {
+		/*
+		 * If no sub-node was found, keep this for device tree
+		 * compatibility
+		 */
+		struct phy *phy = devm_phy_get(dev, "sata-phy");
+		if (!IS_ERR(phy)) {
+			hpriv->phys = devm_kzalloc(dev, sizeof(*hpriv->phys),
+						   GFP_KERNEL);
+			if (!hpriv->phys) {
+				rc = -ENOMEM;
+				goto err_out;
+			}
+
+			hpriv->phys[0] = phy;
+			hpriv->nphys = 1;
+		} else {
+			rc = PTR_ERR(phy);
+			switch (rc) {
+			case -ENODEV:
+			case -ENOSYS:
+				/* continue normally */
+				break;
+
+			case -EPROBE_DEFER:
+				goto err_out;
+
+			default:
+				dev_err(dev, "couldn't get sata-phy\n");
+				goto err_out;
+			}
 		}
 	}
 
@@ -288,7 +388,7 @@ EXPORT_SYMBOL_GPL(ahci_platform_get_resources);
  * @mask_port_map: param passed to ahci_save_initial_config
  *
  * This function does all the usual steps needed to bring up an
- * ahci-platform host, note any necessary resources (ie clks, phy, etc.)
+ * ahci-platform host, note any necessary resources (ie clks, phys, etc.)
  * must be initialized / enabled before calling this.
  *
  * RETURNS:
@@ -394,7 +494,7 @@ static void ahci_host_stop(struct ata_host *host)
  * @dev: device pointer for the host
  *
  * This function does all the usual steps needed to suspend an
- * ahci-platform host, note any necessary resources (ie clks, phy, etc.)
+ * ahci-platform host, note any necessary resources (ie clks, phys, etc.)
  * must be disabled after calling this.
  *
  * RETURNS:
@@ -431,7 +531,7 @@ EXPORT_SYMBOL_GPL(ahci_platform_suspend_host);
  * @dev: device pointer for the host
  *
  * This function does all the usual steps needed to resume an ahci-platform
- * host, note any necessary resources (ie clks, phy, etc.)  must be
+ * host, note any necessary resources (ie clks, phys, etc.)  must be
  * initialized / enabled before calling this.
  *
  * RETURNS:
-- 
1.9.1

[PATCH v9 4/7] ata: ahci_platform: add a generic AHCI compatible

[Antoine Ténart]

The ahci_platform driver is a generic driver using the libahci_platform
functions. Add a generic compatible to avoid having an endless list of
compatibles with no differences for the same driver.

Signed-off-by: Antoine T?nart <antoine.tenart@free-electrons.com>
---
 drivers/ata/ahci_platform.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index ebe505c17763..a30cecd68609 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -78,6 +78,8 @@ static SIMPLE_DEV_PM_OPS(ahci_pm_ops, ahci_platform_suspend,
 			 ahci_platform_resume);
 
 static const struct of_device_id ahci_of_match[] = {
+	{ .compatible = "generic-ahci", },
+	/* Keep the following compatibles for device tree compatibility */
 	{ .compatible = "snps,spear-ahci", },
 	{ .compatible = "snps,exynos5440-ahci", },
 	{ .compatible = "ibm,476gtr-ahci", },
-- 
1.9.1

[PATCH v9 5/7] Documentation: bindings: document the sub-nodes AHCI bindings

[Antoine Ténart]

The libahci now allows to use multiple PHYs and to represent each port
as a sub-node. Add these bindings to the documentation.

Signed-off-by: Antoine T?nart <antoine.tenart@free-electrons.com>
---
 .../devicetree/bindings/ata/ahci-platform.txt      | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
index c96d8dcf98fd..24d95fccfb4a 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
@@ -3,6 +3,10 @@
 SATA nodes are defined to describe on-chip Serial ATA controllers.
 Each SATA controller should have its own node.
 
+It is possible, but not required, to represent each port as a sub-node.
+It allows to enable each port independently when dealing with multiple
+PHYs.
+
 Required properties:
 - compatible        : compatible string, one of:
   - "allwinner,sun4i-a10-ahci"
@@ -14,6 +18,7 @@ Required properties:
   - "snps,dwc-ahci"
   - "snps,exynos5440-ahci"
   - "snps,spear-ahci"
+  - "generic-ahci"
 - interrupts        : <interrupt mapping for SATA IRQ>
 - reg               : <registers mapping>
 
@@ -21,11 +26,23 @@ Optional properties:
 - dma-coherent      : Present if dma operations are coherent
 - clocks            : a list of phandle + clock specifier pairs
 - target-supply     : regulator for SATA target power
+- phys              : reference to the SATA PHY node
+- phy-names         : must be "sata-phy"
 
 "fsl,imx53-ahci", "fsl,imx6q-ahci" required properties:
 - clocks            : must contain the sata, sata_ref and ahb clocks
 - clock-names       : must contain "ahb" for the ahb clock
 
+Required properties when using sub-nodes:
+- #address-cells    : number of cells to encode an address
+- #size-cells       : number of cells representing the size of an address
+
+
+Sub-nodes required properties:
+- reg               : the port number
+- phys              : reference to the SATA PHY node
+
+
 Examples:
         sata at ffe08000 {
 		compatible = "snps,spear-ahci";
@@ -40,3 +57,23 @@ Examples:
 		clocks = <&pll6 0>, <&ahb_gates 25>;
 		target-supply = <&reg_ahci_5v>;
 	};
+
+With sub-nodes:
+	sata at f7e90000 {
+		compatible = "generic-ahci";
+		reg = <0xe90000 0x1000>;
+		interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&chip CLKID_SATA>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		sata0: sata-port at 0 {
+			reg = <0>;
+			phys = <&sata_phy 0>;
+		};
+
+		sata1: sata-port at 1 {
+			reg = <1>;
+			phys = <&sata_phy 1>;
+		};
+	};
-- 
1.9.1

[PATCH v9 6/7] ARM: berlin: add the AHCI node for the BG2Q

[Antoine Ténart]

The BG2Q has an AHCI SATA controller. Add the corresponding nodes
(AHCI, PHY) into its device tree.

Signed-off-by: Antoine T?nart <antoine.tenart@free-electrons.com>
---
 arch/arm/boot/dts/berlin2q.dtsi | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
index 635a16a64cb4..fa34ae6657ac 100644
--- a/arch/arm/boot/dts/berlin2q.dtsi
+++ b/arch/arm/boot/dts/berlin2q.dtsi
@@ -303,6 +303,45 @@
 			clock-names = "refclk";
 		};
 
+		ahci: sata at e90000 {
+			compatible = "generic-ahci";
+			reg = <0xe90000 0x1000>;
+			interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&chip CLKID_SATA>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			sata0: sata-port at 0 {
+				reg = <0>;
+				phys = <&sata_phy 0>;
+				status = "disabled";
+			};
+
+			sata1: sata-port at 1 {
+				reg = <1>;
+				phys = <&sata_phy 1>;
+				status = "disabled";
+			};
+		};
+
+		sata_phy: phy at e900a0 {
+			compatible = "marvell,berlin2q-sata-phy";
+			reg = <0xe900a0 0x200>;
+			clocks = <&chip CLKID_SATA>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			#phy-cells = <1>;
+			status = "disabled";
+
+			sata-phy at 0 {
+				reg = <0>;
+			};
+
+			sata-phy at 1 {
+				reg = <1>;
+			};
+		};
+
 		apb at fc0000 {
 			compatible = "simple-bus";
 			#address-cells = <1>;
-- 
1.9.1

[PATCH v9 7/7] ARM: berlin: enable the eSATA interface on the BG2Q DMP

[Antoine Ténart]

The BG2Q has an AHCI SATA controller with an eSATA interface. Enable it.
Only enable the first port, the BG2Q DMP does not support the second one.

Signed-off-by: Antoine T?nart <antoine.tenart@free-electrons.com>
---
 arch/arm/boot/dts/berlin2q-marvell-dmp.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2q-marvell-dmp.dts b/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
index 995150f93795..385f6af64d0c 100644
--- a/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
+++ b/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
@@ -37,3 +37,11 @@
 &uart0 {
 	status = "okay";
 };
+
+&sata0 {
+	status = "okay";
+};
+
+&sata_phy {
+	status = "okay";
+};
-- 
1.9.1

[PATCH v9 1/7] phy: add a driver for the Berlin SATA PHY

[Kishon Vijay Abraham I]

Hi Antoine,

On Monday 07 July 2014 03:46 PM, Antoine T?nart wrote:
> The Berlin SoC has a two SATA ports. Add a PHY driver to handle them.
> 
> The mode selection can let us think this PHY can be configured to fit
> other purposes. But there are reasons to think the SATA mode will be
> the only one usable: the PHY registers are only accessible indirectly
> through two registers in the SATA range, the PHY seems to be integrated
> and no information tells us the contrary. For these reasons, make the
> driver a SATA PHY driver.

Thanks for doing multiple revisions of this. Looks good to be merged for me now.

Cheers
Kishon
> 
> Signed-off-by: Antoine T?nart <antoine.tenart@free-electrons.com>
> ---
>  drivers/phy/Kconfig           |   7 ++
>  drivers/phy/Makefile          |   1 +
>  drivers/phy/phy-berlin-sata.c | 284 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 292 insertions(+)
>  create mode 100644 drivers/phy/phy-berlin-sata.c
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 16a2f067c242..8ad5c341b8f9 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -15,6 +15,13 @@ config GENERIC_PHY
>  	  phy users can obtain reference to the PHY. All the users of this
>  	  framework should select this config.
>  
> +config PHY_BERLIN_SATA
> +	tristate "Marvell Berlin SATA PHY driver"
> +	depends on ARCH_BERLIN && HAS_IOMEM && OF
> +	select GENERIC_PHY
> +	help
> +	  Enable this to support the SATA PHY on Marvell Berlin SoCs.
> +
>  config PHY_EXYNOS_MIPI_VIDEO
>  	tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
>  	depends on HAS_IOMEM
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index b4f1d5770601..a137a2e23218 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -3,6 +3,7 @@
>  #
>  
>  obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
> +obj-$(CONFIG_PHY_BERLIN_SATA)		+= phy-berlin-sata.o
>  obj-$(CONFIG_BCM_KONA_USB2_PHY)		+= phy-bcm-kona-usb2.o
>  obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
>  obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
> diff --git a/drivers/phy/phy-berlin-sata.c b/drivers/phy/phy-berlin-sata.c
> new file mode 100644
> index 000000000000..c5e688b0899f
> --- /dev/null
> +++ b/drivers/phy/phy-berlin-sata.c
> @@ -0,0 +1,284 @@
> +/*
> + * Marvell Berlin SATA PHY driver
> + *
> + * Copyright (C) 2014 Marvell Technology Group Ltd.
> + *
> + * Antoine T?nart <antoine.tenart@free-electrons.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/module.h>
> +#include <linux/phy/phy.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +
> +#define HOST_VSA_ADDR		0x0
> +#define HOST_VSA_DATA		0x4
> +#define PORT_SCR_CTL		0x2c
> +#define PORT_VSR_ADDR		0x78
> +#define PORT_VSR_DATA		0x7c
> +
> +#define CONTROL_REGISTER	0x0
> +#define MBUS_SIZE_CONTROL	0x4
> +
> +#define POWER_DOWN_PHY0			BIT(6)
> +#define POWER_DOWN_PHY1			BIT(14)
> +#define MBUS_WRITE_REQUEST_SIZE_128	(BIT(2) << 16)
> +#define MBUS_READ_REQUEST_SIZE_128	(BIT(2) << 19)
> +
> +#define PHY_BASE		0x200
> +
> +/* register 0x01 */
> +#define REF_FREF_SEL_25		BIT(0)
> +#define PHY_MODE_SATA		(0x0 << 5)
> +
> +/* register 0x02 */
> +#define USE_MAX_PLL_RATE	BIT(12)
> +
> +/* register 0x23 */
> +#define DATA_BIT_WIDTH_10	(0x0 << 10)
> +#define DATA_BIT_WIDTH_20	(0x1 << 10)
> +#define DATA_BIT_WIDTH_40	(0x2 << 10)
> +
> +/* register 0x25 */
> +#define PHY_GEN_MAX_1_5		(0x0 << 10)
> +#define PHY_GEN_MAX_3_0		(0x1 << 10)
> +#define PHY_GEN_MAX_6_0		(0x2 << 10)
> +
> +struct phy_berlin_desc {
> +	struct phy	*phy;
> +	u32		power_bit;
> +	unsigned	index;
> +};
> +
> +struct phy_berlin_priv {
> +	void __iomem		*base;
> +	spinlock_t		lock;
> +	struct clk		*clk;
> +	struct phy_berlin_desc	**phys;
> +	unsigned		nphys;
> +};
> +
> +static inline void phy_berlin_sata_reg_setbits(void __iomem *ctrl_reg, u32 reg,
> +					       u32 mask, u32 val)
> +{
> +	u32 regval;
> +
> +	/* select register */
> +	writel(PHY_BASE + reg, ctrl_reg + PORT_VSR_ADDR);
> +
> +	/* set bits */
> +	regval = readl(ctrl_reg + PORT_VSR_DATA);
> +	regval &= ~mask;
> +	regval |= val;
> +	writel(regval, ctrl_reg + PORT_VSR_DATA);
> +}
> +
> +static int phy_berlin_sata_power_on(struct phy *phy)
> +{
> +	struct phy_berlin_desc *desc = phy_get_drvdata(phy);
> +	struct phy_berlin_priv *priv = dev_get_drvdata(phy->dev.parent);
> +	void __iomem *ctrl_reg = priv->base + 0x60 + (desc->index * 0x80);
> +	int ret = 0;
> +	u32 regval;
> +
> +	clk_prepare_enable(priv->clk);
> +
> +	spin_lock(&priv->lock);
> +
> +	/* Power on PHY */
> +	writel(CONTROL_REGISTER, priv->base + HOST_VSA_ADDR);
> +	regval = readl(priv->base + HOST_VSA_DATA);
> +	regval &= ~desc->power_bit;
> +	writel(regval, priv->base + HOST_VSA_DATA);
> +
> +	/* Configure MBus */
> +	writel(MBUS_SIZE_CONTROL, priv->base + HOST_VSA_ADDR);
> +	regval = readl(priv->base + HOST_VSA_DATA);
> +	regval |= MBUS_WRITE_REQUEST_SIZE_128 | MBUS_READ_REQUEST_SIZE_128;
> +	writel(regval, priv->base + HOST_VSA_DATA);
> +
> +	/* set PHY mode and ref freq to 25 MHz */
> +	phy_berlin_sata_reg_setbits(ctrl_reg, 0x1, 0xff,
> +				    REF_FREF_SEL_25 | PHY_MODE_SATA);
> +
> +	/* set PHY up to 6 Gbps */
> +	phy_berlin_sata_reg_setbits(ctrl_reg, 0x25, 0xc00, PHY_GEN_MAX_6_0);
> +
> +	/* set 40 bits width */
> +	phy_berlin_sata_reg_setbits(ctrl_reg, 0x23,  0xc00, DATA_BIT_WIDTH_40);
> +
> +	/* use max pll rate */
> +	phy_berlin_sata_reg_setbits(ctrl_reg, 0x2, 0x0, USE_MAX_PLL_RATE);
> +
> +	/* set Gen3 controller speed */
> +	regval = readl(ctrl_reg + PORT_SCR_CTL);
> +	regval &= ~GENMASK(7, 4);
> +	regval |= 0x30;
> +	writel(regval, ctrl_reg + PORT_SCR_CTL);
> +
> +	spin_unlock(&priv->lock);
> +
> +	clk_disable_unprepare(priv->clk);
> +
> +	return ret;
> +}
> +
> +static int phy_berlin_sata_power_off(struct phy *phy)
> +{
> +	struct phy_berlin_desc *desc = phy_get_drvdata(phy);
> +	struct phy_berlin_priv *priv = dev_get_drvdata(phy->dev.parent);
> +	u32 regval;
> +
> +	clk_prepare_enable(priv->clk);
> +
> +	spin_lock(&priv->lock);
> +
> +	/* Power down PHY */
> +	writel(CONTROL_REGISTER, priv->base + HOST_VSA_ADDR);
> +	regval = readl(priv->base + HOST_VSA_DATA);
> +	regval |= desc->power_bit;
> +	writel(regval, priv->base + HOST_VSA_DATA);
> +
> +	spin_unlock(&priv->lock);
> +
> +	clk_disable_unprepare(priv->clk);
> +
> +	return 0;
> +}
> +
> +static struct phy *phy_berlin_sata_phy_xlate(struct device *dev,
> +					     struct of_phandle_args *args)
> +{
> +	struct phy_berlin_priv *priv = dev_get_drvdata(dev);
> +	int i;
> +
> +	if (WARN_ON(args->args[0] >= priv->nphys))
> +		return ERR_PTR(-ENODEV);
> +
> +	for (i = 0; i < priv->nphys; i++) {
> +		if (priv->phys[i]->index == args->args[0])
> +			break;
> +	}
> +
> +	if (i == priv->nphys)
> +		return ERR_PTR(-ENODEV);
> +
> +	return priv->phys[i]->phy;
> +}
> +
> +static struct phy_ops phy_berlin_sata_ops = {
> +	.power_on	= phy_berlin_sata_power_on,
> +	.power_off	= phy_berlin_sata_power_off,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static u32 phy_berlin_power_down_bits[] = {
> +	POWER_DOWN_PHY0,
> +	POWER_DOWN_PHY1,
> +};
> +
> +static int phy_berlin_sata_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *child;
> +	struct phy *phy;
> +	struct phy_provider *phy_provider;
> +	struct phy_berlin_priv *priv;
> +	struct resource *res;
> +	int i = 0;
> +	u32 phy_id;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return -EINVAL;
> +
> +	priv->base = devm_ioremap(dev, res->start, resource_size(res));
> +	if (!priv->base)
> +		return -ENOMEM;
> +
> +	priv->clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(priv->clk))
> +		return PTR_ERR(priv->clk);
> +
> +	priv->nphys = of_get_child_count(dev->of_node);
> +	if (priv->nphys == 0)
> +		return -ENODEV;
> +
> +	priv->phys = devm_kzalloc(dev, priv->nphys * sizeof(*priv->phys),
> +				  GFP_KERNEL);
> +	if (!priv->phys)
> +		return -ENOMEM;
> +
> +	dev_set_drvdata(dev, priv);
> +	spin_lock_init(&priv->lock);
> +
> +	for_each_available_child_of_node(dev->of_node, child) {
> +		struct phy_berlin_desc *phy_desc;
> +
> +		if (of_property_read_u32(child, "reg", &phy_id)) {
> +			dev_err(dev, "missing reg property in node %s\n",
> +				child->name);
> +			return -EINVAL;
> +		}
> +
> +		if (phy_id >= ARRAY_SIZE(phy_berlin_power_down_bits)) {
> +			dev_err(dev, "invalid reg in node %s\n", child->name);
> +			return -EINVAL;
> +		}
> +
> +		phy_desc = devm_kzalloc(dev, sizeof(*phy_desc), GFP_KERNEL);
> +		if (!phy_desc)
> +			return -ENOMEM;
> +
> +		phy = devm_phy_create(dev, &phy_berlin_sata_ops, NULL);
> +		if (IS_ERR(phy)) {
> +			dev_err(dev, "failed to create PHY %d\n", phy_id);
> +			return PTR_ERR(phy);
> +		}
> +
> +		phy_desc->phy = phy;
> +		phy_desc->power_bit = phy_berlin_power_down_bits[phy_id];
> +		phy_desc->index = phy_id;
> +		phy_set_drvdata(phy, phy_desc);
> +
> +		priv->phys[i++] = phy_desc;
> +
> +		/* Make sure the PHY is off */
> +		phy_berlin_sata_power_off(phy);
> +	}
> +
> +	phy_provider =
> +		devm_of_phy_provider_register(dev, phy_berlin_sata_phy_xlate);
> +	if (IS_ERR(phy_provider))
> +		return PTR_ERR(phy_provider);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id phy_berlin_sata_of_match[] = {
> +	{ .compatible = "marvell,berlin2q-sata-phy" },
> +	{ },
> +};
> +
> +static struct platform_driver phy_berlin_sata_driver = {
> +	.probe	= phy_berlin_sata_probe,
> +	.driver	= {
> +		.name		= "phy-berlin-sata",
> +		.owner		= THIS_MODULE,
> +		.of_match_table	= phy_berlin_sata_of_match,
> +	},
> +};
> +module_platform_driver(phy_berlin_sata_driver);
> +
> +MODULE_DESCRIPTION("Marvell Berlin SATA PHY driver");
> +MODULE_AUTHOR("Antoine T?nart <antoine.tenart@free-electrons.com>");
> +MODULE_LICENSE("GPL v2");
> 

[PATCH v9 1/7] phy: add a driver for the Berlin SATA PHY

[Antoine Ténart]

Hi,

On Tue, Jul 08, 2014 at 05:59:13PM +0530, Kishon Vijay Abraham I wrote:
> Hi Antoine,
> 
> On Monday 07 July 2014 03:46 PM, Antoine T?nart wrote:
> > The Berlin SoC has a two SATA ports. Add a PHY driver to handle them.
> > 
> > The mode selection can let us think this PHY can be configured to fit
> > other purposes. But there are reasons to think the SATA mode will be
> > the only one usable: the PHY registers are only accessible indirectly
> > through two registers in the SATA range, the PHY seems to be integrated
> > and no information tells us the contrary. For these reasons, make the
> > driver a SATA PHY driver.
> 
> Thanks for doing multiple revisions of this. Looks good to be merged for me now.

Great!

Thanks,
Antoine

-- 
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH v9 1/7] phy: add a driver for the Berlin SATA PHY

[Sebastian Hesselbarth]

On 07/08/2014 02:29 PM, Kishon Vijay Abraham I wrote:
> On Monday 07 July 2014 03:46 PM, Antoine T?nart wrote:
>> The Berlin SoC has a two SATA ports. Add a PHY driver to handle them.
>>
>> The mode selection can let us think this PHY can be configured to fit
>> other purposes. But there are reasons to think the SATA mode will be
>> the only one usable: the PHY registers are only accessible indirectly
>> through two registers in the SATA range, the PHY seems to be integrated
>> and no information tells us the contrary. For these reasons, make the
>> driver a SATA PHY driver.
>
> Thanks for doing multiple revisions of this. Looks good to be merged for me now.

I'd like to see some Acked-by from Tejun on the AHCI patches first, but
if he agrees, should I prepare a stable branch for each of us to pull
their patches from?

AFAIKS, that would be 1+2 for you, 3-5 for Tejun, and the DT crap 6+7
for me.

Sebastian

[PATCH v9 1/7] phy: add a driver for the Berlin SATA PHY

[Kishon Vijay Abraham I]

Hi,

On Tuesday 08 July 2014 06:30 PM, Sebastian Hesselbarth wrote:
> On 07/08/2014 02:29 PM, Kishon Vijay Abraham I wrote:
>> On Monday 07 July 2014 03:46 PM, Antoine T?nart wrote:
>>> The Berlin SoC has a two SATA ports. Add a PHY driver to handle them.
>>>
>>> The mode selection can let us think this PHY can be configured to fit
>>> other purposes. But there are reasons to think the SATA mode will be
>>> the only one usable: the PHY registers are only accessible indirectly
>>> through two registers in the SATA range, the PHY seems to be integrated
>>> and no information tells us the contrary. For these reasons, make the
>>> driver a SATA PHY driver.
>>
>> Thanks for doing multiple revisions of this. Looks good to be merged for me now.
> 
> I'd like to see some Acked-by from Tejun on the AHCI patches first, but
> if he agrees, should I prepare a stable branch for each of us to pull
> their patches from?

I don't think that's needed. As long as there are no build breaks, each of us
should be able to take it independently.
> 
> AFAIKS, that would be 1+2 for you, 3-5 for Tejun, and the DT crap 6+7
> for me.

right, I'll take 1 and 2 in my tree.

Cheers
Kishon

[PATCH v9 3/7] ata: libahci: allow to use multiple PHYs

[Tejun Heo]

(Cc'ing Hans.)

On Mon, Jul 07, 2014 at 12:16:09PM +0200, Antoine T?nart wrote:
> @@ -482,6 +482,13 @@ void ahci_save_initial_config(struct device *dev,
>  		port_map &= mask_port_map;
>  	}
>  
> +	/*
> +	 * If port_map was filled automatically when finding port sub-nodes,
> +	 * make sure we get the right set here.
> +	 */
> +	if (hpriv->port_map)
> +		port_map &= hpriv->port_map;
> +

So, hpriv->port is both input and output?  This is messy and can lead
to confusing failures and there now are multiple ways to modify
port_map.  If carrying this information through ahci_host_priv is
necessary, let's remove the direct params and introduce new input
fields to the struct.

>  /**
> + * ahci_platform_enable_phys - Enable PHYs
> + * @hpriv: host private area to store config values
> + *
> + * This function enables all the PHYs found in hpriv->phys, if any.
> + * If a PHY fails to be enabled, it disables all the PHYs already
> + * enabled in reverse order and returns an error.
> + *
> + * RETURNS:
> + * 0 on success otherwise a negative error code
> + */
> +int ahci_platform_enable_phys(struct ahci_host_priv *hpriv)
> +{
> +	int i, rc = 0;
> +
> +	for (i = 0; i < hpriv->nphys; i++) {
> +		rc = phy_init(hpriv->phys[i]);
> +		if (rc)
> +			goto disable_phys;
> +
> +		rc = phy_power_on(hpriv->phys[i]);
> +		if (rc) {
> +			phy_exit(hpriv->phys[i]);
> +			goto disable_phys;
> +		}
> +	}
> +
> +	return 0;
> +
> +disable_phys:
> +	while (--i >= 0) {
> +		phy_power_off(hpriv->phys[i]);
> +		phy_exit(hpriv->phys[i]);
> +	}
> +	return rc;
> +}
> +EXPORT_SYMBOL_GPL(ahci_platform_enable_phys);

Do we need to make hpriv->phys[] dynamically allocated?  We already
have hpriv->clks[AHCI_MAX_CLKS] and it's unlikely that we're gonna
need more than several phys per host.  Let's go with a simpler fixed
array.

Thanks.

-- 
tejun

[PATCH v9 3/7] ata: libahci: allow to use multiple PHYs

[Tejun Heo]

One more thing.

On Mon, Jul 07, 2014 at 12:16:09PM +0200, Antoine T?nart wrote:
> This introduce a new way of defining SATA ports in the device tree, with
> one port per sub-node. This as the advantage of allowing a per port
> configuration. Because some ports may be accessible but disabled in the
> device tree, the default port_map is computed automatically when using
> this.

Any chance the old one phy config can be moved over to this scheme so
that we don't have to carry two separate mechanisms?

Thanks.

-- 
tejun

[PATCH v9 3/7] ata: libahci: allow to use multiple PHYs

[Antoine Ténart]

Hello,

On Tue, Jul 08, 2014 at 09:40:00AM -0400, Tejun Heo wrote:
> (Cc'ing Hans.)
> 
> On Mon, Jul 07, 2014 at 12:16:09PM +0200, Antoine T?nart wrote:
> > @@ -482,6 +482,13 @@ void ahci_save_initial_config(struct device *dev,
> >  		port_map &= mask_port_map;
> >  	}
> >  
> > +	/*
> > +	 * If port_map was filled automatically when finding port sub-nodes,
> > +	 * make sure we get the right set here.
> > +	 */
> > +	if (hpriv->port_map)
> > +		port_map &= hpriv->port_map;
> > +
> 
> So, hpriv->port is both input and output?  This is messy and can lead
> to confusing failures and there now are multiple ways to modify
> port_map.  If carrying this information through ahci_host_priv is
> necessary, let's remove the direct params and introduce new input
> fields to the struct.

We just use hpriv->port_map to check port_map is valid and describes
available ports there.

hpriv->port_map is filed by the generic ahci_platform_get_resources()
function when using the new bindings and not by the drivers. port_map is
the input from the drivers.

> 
> >  /**
> > + * ahci_platform_enable_phys - Enable PHYs
> > + * @hpriv: host private area to store config values
> > + *
> > + * This function enables all the PHYs found in hpriv->phys, if any.
> > + * If a PHY fails to be enabled, it disables all the PHYs already
> > + * enabled in reverse order and returns an error.
> > + *
> > + * RETURNS:
> > + * 0 on success otherwise a negative error code
> > + */
> > +int ahci_platform_enable_phys(struct ahci_host_priv *hpriv)
> > +{
> > +	int i, rc = 0;
> > +
> > +	for (i = 0; i < hpriv->nphys; i++) {
> > +		rc = phy_init(hpriv->phys[i]);
> > +		if (rc)
> > +			goto disable_phys;
> > +
> > +		rc = phy_power_on(hpriv->phys[i]);
> > +		if (rc) {
> > +			phy_exit(hpriv->phys[i]);
> > +			goto disable_phys;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +
> > +disable_phys:
> > +	while (--i >= 0) {
> > +		phy_power_off(hpriv->phys[i]);
> > +		phy_exit(hpriv->phys[i]);
> > +	}
> > +	return rc;
> > +}
> > +EXPORT_SYMBOL_GPL(ahci_platform_enable_phys);
> 
> Do we need to make hpriv->phys[] dynamically allocated?  We already
> have hpriv->clks[AHCI_MAX_CLKS] and it's unlikely that we're gonna
> need more than several phys per host.  Let's go with a simpler fixed
> array.
> 

Well, a had a review a week ago about in the PHY driver saying I should
avoid using fixed sized arrays... And it was in a driver were we know
the maximum number of PHY available.

I think in this case were the number of PHYs depends on the h/w, we should
use a dynamically allocated array.


Antoine

-- 
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH v9 3/7] ata: libahci: allow to use multiple PHYs

[Antoine Ténart]

On Tue, Jul 08, 2014 at 09:42:03AM -0400, Tejun Heo wrote:
> One more thing.
> 
> On Mon, Jul 07, 2014 at 12:16:09PM +0200, Antoine T?nart wrote:
> > This introduce a new way of defining SATA ports in the device tree, with
> > one port per sub-node. This as the advantage of allowing a per port
> > configuration. Because some ports may be accessible but disabled in the
> > device tree, the default port_map is computed automatically when using
> > this.
> 
> Any chance the old one phy config can be moved over to this scheme so
> that we don't have to carry two separate mechanisms?

That would break the DT ABI.


Antoine

-- 
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH v9 3/7] ata: libahci: allow to use multiple PHYs

[Tejun Heo]

Hello,

On Tue, Jul 08, 2014 at 07:03:53PM +0200, Antoine T?nart wrote:
> > So, hpriv->port is both input and output?  This is messy and can lead
> > to confusing failures and there now are multiple ways to modify
> > port_map.  If carrying this information through ahci_host_priv is
> > necessary, let's remove the direct params and introduce new input
> > fields to the struct.
> 
> We just use hpriv->port_map to check port_map is valid and describes
> available ports there.
> 
> hpriv->port_map is filed by the generic ahci_platform_get_resources()
> function when using the new bindings and not by the drivers. port_map is
> the input from the drivers.

So, yeah, it's being used both as input and output and we also have
the arguments which affect port_map, right?  It does seem confusing.

> Well, a had a review a week ago about in the PHY driver saying I should
> avoid using fixed sized arrays... And it was in a driver were we know
> the maximum number of PHY available.
> 
> I think in this case were the number of PHYs depends on the h/w, we should
> use a dynamically allocated array.

Well, so does clk.  Let's say clk is more restricted and phy can be
one or more per port and thus needs to be dynamic.  If so, shouldn't
we at least have some correlation between phys and ports?  It bothers
me that now libahci is carrying random number of resources that it has
no idea how to associate with the ports it manages.  What if later we
want to involve phy driver in power managing unoccupied ports?

Thanks.

-- 
tejun

[PATCH v9 3/7] ata: libahci: allow to use multiple PHYs

[Antoine Ténart]

On Tue, Jul 08, 2014 at 01:18:17PM -0400, Tejun Heo wrote:
> Hello,
> 
> On Tue, Jul 08, 2014 at 07:03:53PM +0200, Antoine T?nart wrote:
> > > So, hpriv->port is both input and output?  This is messy and can lead
> > > to confusing failures and there now are multiple ways to modify
> > > port_map.  If carrying this information through ahci_host_priv is
> > > necessary, let's remove the direct params and introduce new input
> > > fields to the struct.
> > 
> > We just use hpriv->port_map to check port_map is valid and describes
> > available ports there.
> > 
> > hpriv->port_map is filed by the generic ahci_platform_get_resources()
> > function when using the new bindings and not by the drivers. port_map is
> > the input from the drivers.
> 
> So, yeah, it's being used both as input and output and we also have
> the arguments which affect port_map, right?  It does seem confusing.

I do see priv->port_map as being automatically set and then restricted
if needed by the port_map input here. I don't see how that's confusing.
The only modification is we restrict the port_map parameter to the set
of available ports. The port_map argument affected priv->port_map before
this patch.

> 
> > Well, a had a review a week ago about in the PHY driver saying I should
> > avoid using fixed sized arrays... And it was in a driver were we know
> > the maximum number of PHY available.
> > 
> > I think in this case were the number of PHYs depends on the h/w, we should
> > use a dynamically allocated array.
> 
> Well, so does clk.  Let's say clk is more restricted and phy can be
> one or more per port and thus needs to be dynamic.  If so, shouldn't
> we at least have some correlation between phys and ports?  It bothers
> me that now libahci is carrying random number of resources that it has
> no idea how to associate with the ports it manages.  What if later we
> want to involve phy driver in power managing unoccupied ports?

I see. This is a first (working) attempt to have a one node per port. I
agree that would be nice to have a correlation between ports and PHYs.
This can definitively be added when needed without changing the dt
bindings as only the internal representation changes. This would also
require to get all phys from the port nodes, which is again internal
stuff.

Don't you think we can go by steps, and have a following up series for
this when needed (like in a power managing series for unoccupied ports)?


Antoine

-- 
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH v9 3/7] ata: libahci: allow to use multiple PHYs

[Tejun Heo]

Hey,

On Tue, Jul 08, 2014 at 07:49:00PM +0200, Antoine T?nart wrote:
> > So, yeah, it's being used both as input and output and we also have
> > the arguments which affect port_map, right?  It does seem confusing.
> 
> I do see priv->port_map as being automatically set and then restricted
> if needed by the port_map input here. I don't see how that's confusing.
> The only modification is we restrict the port_map parameter to the set
> of available ports. The port_map argument affected priv->port_map before
> this patch.

It is confusing.  If you wanna pass around available ports in hpriv,
please add a separate field and replace the arguments to
save_initial_config().

> > Well, so does clk.  Let's say clk is more restricted and phy can be
> > one or more per port and thus needs to be dynamic.  If so, shouldn't
> > we at least have some correlation between phys and ports?  It bothers
> > me that now libahci is carrying random number of resources that it has
> > no idea how to associate with the ports it manages.  What if later we
> > want to involve phy driver in power managing unoccupied ports?
> 
> I see. This is a first (working) attempt to have a one node per port. I
> agree that would be nice to have a correlation between ports and PHYs.
> This can definitively be added when needed without changing the dt
> bindings as only the internal representation changes. This would also
> require to get all phys from the port nodes, which is again internal
> stuff.
> 
> Don't you think we can go by steps, and have a following up series for
> this when needed (like in a power managing series for unoccupied ports)?

I don't know.  It isn't exactly difficult to make it per-port, is it?
We already have ahci_port_priv and wouldn't the code actually be
simpler that way?

Thanks.

-- 
tejun

[PATCH v9 3/7] ata: libahci: allow to use multiple PHYs

[Hans de Goede]

Hi,

On 07/08/2014 03:40 PM, Tejun Heo wrote:
> (Cc'ing Hans.)

Thanks for adding me to the loop.

I've been reading the entire thread sofar, and here are my 2 cents:

1) I think overall this is a good idea, and I like the suggested dt representation
2) I agree with Tejun that it would be better to tie the phy to the port, and thus to
add the struct phy * pointer to struct ahci_port_priv.

One question 2 brings with it is how to deal with older device-tree files which
have the phy directly in the ahci controller node. This seems to only be the
case for the TI dra7 and omap5 platforms. Probably those have only 1 sata port
(this needs to be verified), if that is the case we can simply hard code
tying the phy to port 0 in this case, and we should probably also update their
dts files in the kernel tree to use the new structure (while keeping the
ahci driver compatible with the old structure for now).

Regards,

Hans

[PATCH v9 3/7] ata: libahci: allow to use multiple PHYs

[Antoine Ténart]

Hi Tejun,

On Tue, Jul 08, 2014 at 05:40:10PM -0400, Tejun Heo wrote:
> Hey,
> 
> On Tue, Jul 08, 2014 at 07:49:00PM +0200, Antoine T?nart wrote:
> > > So, yeah, it's being used both as input and output and we also have
> > > the arguments which affect port_map, right?  It does seem confusing.
> > 
> > I do see priv->port_map as being automatically set and then restricted
> > if needed by the port_map input here. I don't see how that's confusing.
> > The only modification is we restrict the port_map parameter to the set
> > of available ports. The port_map argument affected priv->port_map before
> > this patch.
> 
> It is confusing.  If you wanna pass around available ports in hpriv,
> please add a separate field and replace the arguments to
> save_initial_config().

I don't get it. Which argument should I replace in
save_initial_config()? The change is we compute hpriv->port_map.
I don't see which arguments we can add or replace.

If that's more clear, the modification here is equivalent to (when using
the new bindings):

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 40ea583d3610..f9d3cfd5d1bd 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -513,7 +513,7 @@ void ahci_save_initial_config(struct device *dev,
        /* record values to use during operation */
        hpriv->cap = cap;
        hpriv->cap2 = cap2;
-       hpriv->port_map = port_map;
+       hpriv->port_map &= port_map;
 
        if (!hpriv->start_engine)
                hpriv->start_engine = ahci_start_engine;

It just use the argument port_map on the real set of ports. If that's
still not clear, I can add a separate field. But the field will store
the exact same information as hpriv->port_map. We will have something
like:

if (hpriv->available_port_map)
	hpriv->port_map &= hpriv->available_port_map;

> > > Well, so does clk.  Let's say clk is more restricted and phy can be
> > > one or more per port and thus needs to be dynamic.  If so, shouldn't
> > > we at least have some correlation between phys and ports?  It bothers
> > > me that now libahci is carrying random number of resources that it has
> > > no idea how to associate with the ports it manages.  What if later we
> > > want to involve phy driver in power managing unoccupied ports?
> > 
> > I see. This is a first (working) attempt to have a one node per port. I
> > agree that would be nice to have a correlation between ports and PHYs.
> > This can definitively be added when needed without changing the dt
> > bindings as only the internal representation changes. This would also
> > require to get all phys from the port nodes, which is again internal
> > stuff.
> > 
> > Don't you think we can go by steps, and have a following up series for
> > this when needed (like in a power managing series for unoccupied ports)?
> 
> I don't know.  It isn't exactly difficult to make it per-port, is it?
> We already have ahci_port_priv and wouldn't the code actually be
> simpler that way?

I had a quick look on this, and it does not seems to be that simple. The
ahci_port_priv is stored inside the ata_port struct and not accessible
(as of now) from the ahci_host_priv one. The ahci_port_priv is
initialized at the end of ahci_platform_init_host(), far after we need
it. This requires quite a lot of changes. Or is there another way?

To be honest, we are now at v9 and it's been quite a long time since v1.
I'd really like it to be merged in 3.17. As I see it, this patch keeps
the same logic as what was in place before, only with more PHYs.

Don't take me wrong, I really think this is a good idea to have a
per-port PHY information. But this is a refactoring not clearly related
to this series as the logic is not changed. This definitively can be the
subject of a dedicated series, especially if I got it right and the
required modifications are not that obvious.

What do you think?

Thanks,

Antoine

-- 
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH v9 3/7] ata: libahci: allow to use multiple PHYs

[Tejun Heo]

Hello,

On Wed, Jul 09, 2014 at 10:23:31AM +0200, Antoine T?nart wrote:
> > It is confusing.  If you wanna pass around available ports in hpriv,
> > please add a separate field and replace the arguments to
> > save_initial_config().
> 
> I don't get it. Which argument should I replace in
> save_initial_config()? The change is we compute hpriv->port_map.
> I don't see which arguments we can add or replace.

The @force_port_map and @mask_port_map of ahci_save_initial_config().
We end up with three params from two places modifying port_map and one
of those is in/out parameter, so ummm, no.  If you wanna add port
masking to @hpriv, please do it by moving @force_port_map and
@mask_port_map into @hpriv instead.

Sure, the proposed change is small but the end result is messy.

> I had a quick look on this, and it does not seems to be that simple. The
> ahci_port_priv is stored inside the ata_port struct and not accessible
> (as of now) from the ahci_host_priv one. The ahci_port_priv is
> initialized at the end of ahci_platform_init_host(), far after we need
> it. This requires quite a lot of changes. Or is there another way?

Yeah, it'd probably need separating out port resource handling, so
that the order is get_resources, host_alloc, get_port_resources and
then init and activate.  Hans, what do you think?

> To be honest, we are now at v9 and it's been quite a long time since v1.
> I'd really like it to be merged in 3.17. As I see it, this patch keeps
> the same logic as what was in place before, only with more PHYs.
> 
> Don't take me wrong, I really think this is a good idea to have a
> per-port PHY information. But this is a refactoring not clearly related
> to this series as the logic is not changed. This definitively can be the
> subject of a dedicated series, especially if I got it right and the
> required modifications are not that obvious.

Heh, I get that but, at the same time, this is the point where you're
most motivated to actually work on it.  :)

Let's see how much work it's gonna be.

Thanks.

-- 
tejun

[PATCH v9 3/7] ata: libahci: allow to use multiple PHYs

[Hans de Goede]

Hi,

On 07/09/2014 03:59 PM, Tejun Heo wrote:
> Hello,
> 
> On Wed, Jul 09, 2014 at 10:23:31AM +0200, Antoine T?nart wrote:
>>> It is confusing.  If you wanna pass around available ports in hpriv,
>>> please add a separate field and replace the arguments to
>>> save_initial_config().
>>
>> I don't get it. Which argument should I replace in
>> save_initial_config()? The change is we compute hpriv->port_map.
>> I don't see which arguments we can add or replace.
> 
> The @force_port_map and @mask_port_map of ahci_save_initial_config().
> We end up with three params from two places modifying port_map and one
> of those is in/out parameter, so ummm, no.  If you wanna add port
> masking to @hpriv, please do it by moving @force_port_map and
> @mask_port_map into @hpriv instead.
> 
> Sure, the proposed change is small but the end result is messy.
> 
>> I had a quick look on this, and it does not seems to be that simple. The
>> ahci_port_priv is stored inside the ata_port struct and not accessible
>> (as of now) from the ahci_host_priv one. The ahci_port_priv is
>> initialized at the end of ahci_platform_init_host(), far after we need
>> it. This requires quite a lot of changes. Or is there another way?
> 
> Yeah, it'd probably need separating out port resource handling, so
> that the order is get_resources, host_alloc, get_port_resources and
> then init and activate.  Hans, what do you think?

The order (and function names) you're suggesting here sound good to me.

Regards,

Hans

[PATCH v9 3/7] ata: libahci: allow to use multiple PHYs

[Alexandre Belloni]

Hi,

On 09/07/2014 at 09:59:10 -0400, Tejun Heo wrote :
> Heh, I get that but, at the same time, this is the point where you're
> most motivated to actually work on it.  :)
> 

Or, starting that kind of review after 9 revisions, especially so close
to the end of the development cycle is discouraging enough and will
probably be used as an example of why we still have so many evil vendor
trees.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH v9 3/7] ata: libahci: allow to use multiple PHYs

[Tejun Heo]

On Wed, Jul 09, 2014 at 05:24:46PM +0200, Alexandre Belloni wrote:
> Hi,
> 
> On 09/07/2014 at 09:59:10 -0400, Tejun Heo wrote :
> > Heh, I get that but, at the same time, this is the point where you're
> > most motivated to actually work on it.  :)
> > 
> 
> Or, starting that kind of review after 9 revisions, especially so close
> to the end of the development cycle is discouraging enough and will
> probably be used as an example of why we still have so many evil vendor
> trees.

Yeap, that's my bad.  I should have paid more attention.  Sorry about
that.  We still have quite a few weeks to go tho, so I don't think
it's too unreasonable to at least look into it.  We're gonna have to
do it anyway.

Thanks.

-- 
tejun