devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] MVEBU SATA PHY driver
@ 2013-12-04 17:15 Andrew Lunn
  2013-12-04 17:16 ` [PATCH 1/5] Phy: DT binding documentation for Marvell MVEBU SATA phy Andrew Lunn
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Andrew Lunn @ 2013-12-04 17:15 UTC (permalink / raw)
  To: Jason Cooper, kishon-l0cyMroinI0
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-ide-u79uwXL29TY76Z2rM5mHXA, Gregory Clement,
	Sebastian Hesselbarth, Andrew Lunn

Marvell Kirkwood and Dove can turn the SATA PHY off in order to save
power. For Dove, this can be in the order of 220mW when its SATA PHY
is turned off.

Andrew Lunn (5):
  Phy: DT binding documentation for Marvell MVEBU SATA phy.
  Phy: Add a PHY driver for Marvell MVEBU SATA PHY.
  SATA: MV: Add support for the optional PHYs
  Phy: Add DT nodes on kirkwood and Dove for the SATA PHY
  ARM: defconfig: Enable generic phy in dove and kirkwood.

 Documentation/devicetree/bindings/ata/marvell.txt  |   6 +
 .../devicetree/bindings/phy/phy-mvebu-sata.txt     |  22 ++++
 arch/arm/boot/dts/dove.dtsi                        |  11 ++
 arch/arm/boot/dts/kirkwood-6281.dtsi               |   2 +
 arch/arm/boot/dts/kirkwood-6282.dtsi               |   2 +
 arch/arm/boot/dts/kirkwood.dtsi                    |  18 +++
 arch/arm/configs/dove_defconfig                    |   1 +
 arch/arm/configs/kirkwood_defconfig                |   1 +
 drivers/ata/sata_mv.c                              |  19 +++
 drivers/phy/Kconfig                                |   5 +
 drivers/phy/Makefile                               |   1 +
 drivers/phy/phy-mvebu-sata.c                       | 130 +++++++++++++++++++++
 12 files changed, 218 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-mvebu-sata.txt
 create mode 100644 drivers/phy/phy-mvebu-sata.c

-- 
1.8.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH 1/5] Phy: DT binding documentation for Marvell MVEBU SATA phy.
  2013-12-04 17:15 [PATCH 0/5] MVEBU SATA PHY driver Andrew Lunn
@ 2013-12-04 17:16 ` Andrew Lunn
  2013-12-05  6:03   ` Kishon Vijay Abraham I
       [not found] ` <1386177364-10164-1-git-send-email-andrew-g2DYL2Zd6BY@public.gmane.org>
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Andrew Lunn @ 2013-12-04 17:16 UTC (permalink / raw)
  To: Jason Cooper, kishon
  Cc: devicetree, linux-ide, Gregory Clement, Sebastian Hesselbarth,
	Andrew Lunn

Describe the binding for the Marvell MVEBU SATA phy. This driver
can be used at least with Kirkwood, Dove and maybe others.
Additionally, update the SATA binding with the properties to link
to the phy nodes.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 Documentation/devicetree/bindings/ata/marvell.txt  |  6 ++++++
 .../devicetree/bindings/phy/phy-mvebu-sata.txt     | 22 ++++++++++++++++++++++
 2 files changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-mvebu-sata.txt

diff --git a/Documentation/devicetree/bindings/ata/marvell.txt b/Documentation/devicetree/bindings/ata/marvell.txt
index b5cdd20cde9c..e072fa105b49 100644
--- a/Documentation/devicetree/bindings/ata/marvell.txt
+++ b/Documentation/devicetree/bindings/ata/marvell.txt
@@ -6,11 +6,17 @@ Required Properties:
 - interrupts    : Interrupt controller is using
 - nr-ports      : Number of SATA ports in use.
 
+Optional Properties:
+- phys		: List of phandles to sata phys
+- phy-names	: Should be "0", "1", etc, one number per phandle
+
 Example:
 
 	sata@80000 {
 		compatible = "marvell,orion-sata";
 		reg = <0x80000 0x5000>;
 		interrupts = <21>;
+		phys = <&sata_phy0>, <&sata_phy1>;
+		phy-names = "0", "1";
 		nr-ports = <2>;
 	}
diff --git a/Documentation/devicetree/bindings/phy/phy-mvebu-sata.txt b/Documentation/devicetree/bindings/phy/phy-mvebu-sata.txt
new file mode 100644
index 000000000000..1cf9cef50b4b
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-mvebu-sata.txt
@@ -0,0 +1,22 @@
+* Marvell MVEBU SATA PHY
+
+Power control for the SATA phy found on Marvell MVEBU SoCs.
+
+This document extends the binding described in phy-bindings.txt
+
+Required properties :
+
+ - reg		   : Offset and length of the register set for the SATA device
+ - compatible	   : Should be "marvell,mvebu-sata-phy"
+ - clocks	   : phandle of clock that supplies the SATA device
+ - clock-names	   : Should be "sata"
+
+Example:
+		sata-phy@1 {
+			compatible = "marvell,mvebu-sata-phy";
+			reg = <0x84000 0x0334>;
+			clocks = <&gate_clk 15>;
+			clock-names = "sata";
+			#phy-cells = <1>;
+			status = "ok";
+		};
-- 
1.8.5


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

* [PATCH 2/5] Phy: Add a PHY driver for Marvell MVEBU SATA PHY.
       [not found] ` <1386177364-10164-1-git-send-email-andrew-g2DYL2Zd6BY@public.gmane.org>
@ 2013-12-04 17:16   ` Andrew Lunn
  2013-12-05  6:10     ` Kishon Vijay Abraham I
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Lunn @ 2013-12-04 17:16 UTC (permalink / raw)
  To: Jason Cooper, kishon-l0cyMroinI0
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-ide-u79uwXL29TY76Z2rM5mHXA, Gregory Clement,
	Sebastian Hesselbarth, Andrew Lunn

Kirkwood and Dove can turn the SATA phy on and off. Add a PHY driver
to control this.

Signed-off-by: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
---
 drivers/phy/Kconfig          |   5 ++
 drivers/phy/Makefile         |   1 +
 drivers/phy/phy-mvebu-sata.c | 130 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 136 insertions(+)
 create mode 100644 drivers/phy/phy-mvebu-sata.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index a344f3d52361..2dd97c3bdab7 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -21,6 +21,11 @@ config PHY_EXYNOS_MIPI_VIDEO
 	  Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung S5P
 	  and EXYNOS SoCs.
 
+config PHY_MVEBU_SATA
+       def_bool y
+       depends on ARCH_KIRKWOOD || ARCH_DOVE
+       depends on OF
+
 config OMAP_USB2
 	tristate "OMAP USB2 PHY Driver"
 	depends on ARCH_OMAP2PLUS
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index d0caae9cfb83..4e4adc96f753 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -5,5 +5,6 @@
 obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
 obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
 obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
+obj-$(CONFIG_PHY_MVEBU_SATA)		+= phy-mvebu-sata.o
 obj-$(CONFIG_OMAP_USB2)			+= phy-omap-usb2.o
 obj-$(CONFIG_TWL4030_USB)		+= phy-twl4030-usb.o
diff --git a/drivers/phy/phy-mvebu-sata.c b/drivers/phy/phy-mvebu-sata.c
new file mode 100644
index 000000000000..7d40c3afb090
--- /dev/null
+++ b/drivers/phy/phy-mvebu-sata.c
@@ -0,0 +1,130 @@
+/*
+ *	phy-mvebu-sata.c: SATA Phy driver for the Marvell mvebu SoCs.
+ *
+ *	Copyright (C) 2013 Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
+ *
+ *	This program is free software; you can redistribute it and/or
+ *	modify it under the terms of the GNU General Public License
+ *	as published by the Free Software Foundation; either version
+ *	2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/clk.h>
+#include <linux/phy/phy.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+
+struct priv {
+	struct clk	*clk;
+	void __iomem	*base;
+};
+
+#define SATA_PHY_MODE_2	0x0330
+#define SATA_IF_CTRL	0x0050
+
+static int phy_mvebu_sata_power_on(struct phy *phy)
+{
+	struct priv *priv = phy_get_drvdata(phy);
+	u32 reg;
+
+	clk_prepare_enable(priv->clk);
+
+	/* Enable PLL and IVREF */
+	reg = readl(priv->base + SATA_PHY_MODE_2);
+	reg |= 0xf;
+	writel(reg , priv->base + SATA_PHY_MODE_2);
+
+	/* Enable PHY */
+	reg = readl(priv->base + SATA_IF_CTRL);
+	reg &= ~0x200;
+	writel(reg, priv->base + SATA_IF_CTRL);
+
+	clk_disable_unprepare(priv->clk);
+
+	return 0;
+}
+
+static int phy_mvebu_sata_power_off(struct phy *phy)
+{
+	struct priv *priv = phy_get_drvdata(phy);
+	u32 reg;
+
+	clk_prepare_enable(priv->clk);
+
+	/* Disable PLL and IVREF */
+	reg = readl(priv->base + SATA_PHY_MODE_2);
+	reg &= ~0xf;
+	writel(reg, priv->base + SATA_PHY_MODE_2);
+
+	/* Disable PHY */
+	reg = readl(priv->base + SATA_IF_CTRL);
+	reg |= 0x200;
+	writel(reg, priv->base + SATA_IF_CTRL);
+
+	clk_disable_unprepare(priv->clk);
+
+	return 0;
+}
+
+static struct phy_ops phy_mvebu_sata_ops = {
+	.power_on	= phy_mvebu_sata_power_on,
+	.power_off	= phy_mvebu_sata_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static int phy_mvebu_sata_probe(struct platform_device *pdev)
+{
+	struct phy_provider *phy_provider;
+	struct resource *res;
+	struct priv *priv;
+	struct phy *phy;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	priv->clk = devm_clk_get(&pdev->dev, "sata");
+	if (IS_ERR(priv->clk))
+		return PTR_ERR(priv->clk);
+
+	phy_provider = devm_of_phy_provider_register(&pdev->dev,
+						     of_phy_simple_xlate);
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	phy = devm_phy_create(&pdev->dev, &phy_mvebu_sata_ops, NULL);
+	if (IS_ERR(phy))
+		return PTR_ERR(phy);
+
+	phy_set_drvdata(phy, priv);
+
+	/* The boot loader may of left it on. Turn it off. */
+	phy_mvebu_sata_power_off(phy);
+
+	return 0;
+}
+
+static const struct of_device_id phy_mvebu_sata_of_match[] = {
+	{ .compatible = "marvell,mvebu-sata-phy" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, phy_mvebu_sata_of_match);
+
+static struct platform_driver phy_mvebu_sata_driver = {
+	.probe	= phy_mvebu_sata_probe,
+	.driver = {
+		.name	= "phy-mvebu-sata",
+		.owner	= THIS_MODULE,
+		.of_match_table	= phy_mvebu_sata_of_match,
+	}
+};
+module_platform_driver(phy_mvebu_sata_driver);
+
+MODULE_AUTHOR("Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>");
+MODULE_DESCRIPTION("Marvell MVEBU SATA PHY driver");
+MODULE_LICENSE("GPL v2");
-- 
1.8.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH 3/5] SATA: MV: Add support for the optional PHYs
  2013-12-04 17:15 [PATCH 0/5] MVEBU SATA PHY driver Andrew Lunn
  2013-12-04 17:16 ` [PATCH 1/5] Phy: DT binding documentation for Marvell MVEBU SATA phy Andrew Lunn
       [not found] ` <1386177364-10164-1-git-send-email-andrew-g2DYL2Zd6BY@public.gmane.org>
@ 2013-12-04 17:16 ` Andrew Lunn
  2013-12-05  6:15   ` Kishon Vijay Abraham I
  2013-12-04 17:16 ` [PATCH 4/5] Phy: Add DT nodes on kirkwood and Dove for the SATA PHY Andrew Lunn
  2013-12-04 17:16 ` [PATCH 5/5] ARM: defconfig: Enable generic phy in dove and kirkwood Andrew Lunn
  4 siblings, 1 reply; 18+ messages in thread
From: Andrew Lunn @ 2013-12-04 17:16 UTC (permalink / raw)
  To: Jason Cooper, kishon
  Cc: devicetree, linux-ide, Gregory Clement, Sebastian Hesselbarth,
	Andrew Lunn

Some Marvell SoCs have a SATA PHY which can be powered off, in order
to save power. Make use of the generic phy framework to control these
phys.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/ata/sata_mv.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 56be31819897..9a728bcfbb9a 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -60,6 +60,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/device.h>
 #include <linux/clk.h>
+#include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 #include <linux/ata_platform.h>
 #include <linux/mbus.h>
@@ -563,6 +564,12 @@ struct mv_host_priv {
 	struct clk		*clk;
 	struct clk              **port_clks;
 	/*
+	 * Some devices have a SATA PHY which can be enabled/disabled
+	 * in order to save power. These are optional: if the platform
+	 * devices does not have any phy, they won't be used.
+	 */
+	struct phy		**port_phys;
+	/*
 	 * These consistent DMA memory pools give us guaranteed
 	 * alignment for hardware-accessed data structures,
 	 * and less memory waste in accomplishing the alignment.
@@ -4076,6 +4083,11 @@ static int mv_platform_probe(struct platform_device *pdev)
 					GFP_KERNEL);
 	if (!hpriv->port_clks)
 		return -ENOMEM;
+	hpriv->port_phys = devm_kzalloc(&pdev->dev,
+					sizeof(struct phy *) * n_ports,
+					GFP_KERNEL);
+	if (!hpriv->port_phys)
+		return -ENOMEM;
 	host->private_data = hpriv;
 	hpriv->n_ports = n_ports;
 	hpriv->board_idx = chip_soc;
@@ -4097,6 +4109,9 @@ static int mv_platform_probe(struct platform_device *pdev)
 		hpriv->port_clks[port] = clk_get(&pdev->dev, port_number);
 		if (!IS_ERR(hpriv->port_clks[port]))
 			clk_prepare_enable(hpriv->port_clks[port]);
+		hpriv->port_phys[port] = devm_phy_get(&pdev->dev, port_number);
+		if (!IS_ERR(hpriv->port_phys[port]))
+			phy_power_on(hpriv->port_phys[port]);
 	}
 
 	/*
@@ -4132,6 +4147,8 @@ err:
 			clk_disable_unprepare(hpriv->port_clks[port]);
 			clk_put(hpriv->port_clks[port]);
 		}
+		if (!IS_ERR(hpriv->port_phys[port]))
+			phy_power_off(hpriv->port_phys[port]);
 	}
 
 	return rc;
@@ -4161,6 +4178,8 @@ static int mv_platform_remove(struct platform_device *pdev)
 			clk_disable_unprepare(hpriv->port_clks[port]);
 			clk_put(hpriv->port_clks[port]);
 		}
+		if (!IS_ERR(hpriv->port_phys[port]))
+			phy_power_off(hpriv->port_phys[port]);
 	}
 	return 0;
 }
-- 
1.8.5


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

* [PATCH 4/5] Phy: Add DT nodes on kirkwood and Dove for the SATA PHY
  2013-12-04 17:15 [PATCH 0/5] MVEBU SATA PHY driver Andrew Lunn
                   ` (2 preceding siblings ...)
  2013-12-04 17:16 ` [PATCH 3/5] SATA: MV: Add support for the optional PHYs Andrew Lunn
@ 2013-12-04 17:16 ` Andrew Lunn
  2013-12-04 20:01   ` Sergei Shtylyov
  2013-12-05  6:17   ` Kishon Vijay Abraham I
  2013-12-04 17:16 ` [PATCH 5/5] ARM: defconfig: Enable generic phy in dove and kirkwood Andrew Lunn
  4 siblings, 2 replies; 18+ messages in thread
From: Andrew Lunn @ 2013-12-04 17:16 UTC (permalink / raw)
  To: Jason Cooper, kishon
  Cc: devicetree, linux-ide, Gregory Clement, Sebastian Hesselbarth,
	Andrew Lunn

Add nodes for the two SATA PHYs on kirkewood.
Add node for the one SATA PHY on Dove.
Add pHandles to the PHYs in the sata nodes.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 arch/arm/boot/dts/dove.dtsi          | 11 +++++++++++
 arch/arm/boot/dts/kirkwood-6281.dtsi |  2 ++
 arch/arm/boot/dts/kirkwood-6282.dtsi |  2 ++
 arch/arm/boot/dts/kirkwood.dtsi      | 18 ++++++++++++++++++
 4 files changed, 33 insertions(+)

diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi
index 113a8bc7bee7..d56b3c675249 100644
--- a/arch/arm/boot/dts/dove.dtsi
+++ b/arch/arm/boot/dts/dove.dtsi
@@ -490,10 +490,21 @@
 				reg = <0xa0000 0x2400>;
 				interrupts = <62>;
 				clocks = <&gate_clk 3>;
+				phys = <&sata_phy0>;
+				phy-names = "0";
 				nr-ports = <1>;
 				status = "disabled";
 			};
 
+			sata_phy0: sata_phy@0 {
+				compatible = "marvell,mvebu-sata-phy";
+				reg = <0xa2000 0x0334>;
+				clocks = <&gate_clk 3>;
+				clock-names = "sata";
+				#phy-cells = <0>;
+				status = "ok";
+			};
+
 			rtc: real-time-clock@d8500 {
 				compatible = "marvell,orion-rtc";
 				reg = <0xd8500 0x20>;
diff --git a/arch/arm/boot/dts/kirkwood-6281.dtsi b/arch/arm/boot/dts/kirkwood-6281.dtsi
index 650ef30e1856..92a1c747456a 100644
--- a/arch/arm/boot/dts/kirkwood-6281.dtsi
+++ b/arch/arm/boot/dts/kirkwood-6281.dtsi
@@ -89,6 +89,8 @@
 			interrupts = <21>;
 			clocks = <&gate_clk 14>, <&gate_clk 15>;
 			clock-names = "0", "1";
+			phys = <&sata_phy0>, <&sata_phy1>;
+			phy-names = "0", "1";
 			status = "disabled";
 		};
 
diff --git a/arch/arm/boot/dts/kirkwood-6282.dtsi b/arch/arm/boot/dts/kirkwood-6282.dtsi
index 3933a331ddc2..dfa6b073cf00 100644
--- a/arch/arm/boot/dts/kirkwood-6282.dtsi
+++ b/arch/arm/boot/dts/kirkwood-6282.dtsi
@@ -117,6 +117,8 @@
 			interrupts = <21>;
 			clocks = <&gate_clk 14>, <&gate_clk 15>;
 			clock-names = "0", "1";
+			phys = <&sata_phy0>, <&sata_phy1>;
+			phy-names = "0", "1";
 			status = "disabled";
 		};
 
diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi
index 8b73c80f1dad..5558b89d48ce 100644
--- a/arch/arm/boot/dts/kirkwood.dtsi
+++ b/arch/arm/boot/dts/kirkwood.dtsi
@@ -282,5 +282,23 @@
 				/* set phy-handle property in board file */
 			};
 		};
+
+		sata_phy0: sata_phy@0 {
+			compatible = "marvell,mvebu-sata-phy";
+			reg = <0x82000 0x0334>;
+			clocks = <&gate_clk 14>;
+			clock-names = "sata";
+			#phy-cells = <0>;
+			status = "ok";
+		};
+
+		sata_phy1: sata_phy@1 {
+			compatible = "marvell,mvebu-sata-phy";
+			reg = <0x84000 0x0334>;
+			clocks = <&gate_clk 15>;
+			clock-names = "sata";
+			#phy-cells = <0>;
+			status = "ok";
+		};
 	};
 };
-- 
1.8.5


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

* [PATCH 5/5] ARM: defconfig: Enable generic phy in dove and kirkwood.
  2013-12-04 17:15 [PATCH 0/5] MVEBU SATA PHY driver Andrew Lunn
                   ` (3 preceding siblings ...)
  2013-12-04 17:16 ` [PATCH 4/5] Phy: Add DT nodes on kirkwood and Dove for the SATA PHY Andrew Lunn
@ 2013-12-04 17:16 ` Andrew Lunn
  2013-12-04 20:03   ` Sergei Shtylyov
  4 siblings, 1 reply; 18+ messages in thread
From: Andrew Lunn @ 2013-12-04 17:16 UTC (permalink / raw)
  To: Jason Cooper, kishon
  Cc: devicetree, linux-ide, Gregory Clement, Sebastian Hesselbarth,
	Andrew Lunn

Now that there is a phy driver for sata, enable generic phy in the
kirkwood and dove defconfig.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
_
---
 arch/arm/configs/dove_defconfig     | 1 +
 arch/arm/configs/kirkwood_defconfig | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/arm/configs/dove_defconfig b/arch/arm/configs/dove_defconfig
index 110105476848..fa03d7356168 100644
--- a/arch/arm/configs/dove_defconfig
+++ b/arch/arm/configs/dove_defconfig
@@ -100,6 +100,7 @@ CONFIG_RTC_CLASS=y
 CONFIG_RTC_DRV_MV=y
 CONFIG_DMADEVICES=y
 CONFIG_MV_XOR=y
+CONFIG_GENERIC_PHY=y
 CONFIG_EXT2_FS=y
 CONFIG_EXT3_FS=y
 # CONFIG_EXT3_FS_XATTR is not set
diff --git a/arch/arm/configs/kirkwood_defconfig b/arch/arm/configs/kirkwood_defconfig
index 0ae0eaebf6b2..fe4136ecfa4c 100644
--- a/arch/arm/configs/kirkwood_defconfig
+++ b/arch/arm/configs/kirkwood_defconfig
@@ -142,6 +142,7 @@ CONFIG_RTC_DRV_S35390A=y
 CONFIG_RTC_DRV_MV=y
 CONFIG_DMADEVICES=y
 CONFIG_MV_XOR=y
+CONFIG_GENERIC_PHY=y
 CONFIG_EXT2_FS=y
 CONFIG_EXT3_FS=y
 # CONFIG_EXT3_FS_XATTR is not set
-- 
1.8.5


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

* Re: [PATCH 4/5] Phy: Add DT nodes on kirkwood and Dove for the SATA PHY
  2013-12-04 20:01   ` Sergei Shtylyov
@ 2013-12-04 19:18     ` Andrew Lunn
  0 siblings, 0 replies; 18+ messages in thread
From: Andrew Lunn @ 2013-12-04 19:18 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Andrew Lunn, Jason Cooper, kishon, devicetree, linux-ide,
	Gregory Clement, Sebastian Hesselbarth

On Wed, Dec 04, 2013 at 11:01:20PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 12/04/2013 08:16 PM, Andrew Lunn wrote:
> 
> >Add nodes for the two SATA PHYs on kirkewood.
> >Add node for the one SATA PHY on Dove.
> >Add pHandles to the PHYs in the sata nodes.
> 
>    I don't think this patch should be pushed thru the libata tree.
> However, you didn't Cc linux-arm-kernel...

I've actually no idea how this will go upstream, since it is three
different subsystems, generic phy, libata, and arm/mvebu. I will leave
it up to the maintainers to decide that. I can add some more CC: for
v2.

> >+			sata_phy0: sata_phy@0 {
> 
>    The node should rather be named "sata-phy@0" to be in line with
> "ethernet-phy" found in the ePAPR spec. [1], hyphen is generally
> preferred to underscore in the device trees.

O.K, will do. kirkwood.dtsi and dove.dtsi already follow this pattern
of - in the @ part.

Thanks
	Andrew

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

* Re: [PATCH 4/5] Phy: Add DT nodes on kirkwood and Dove for the SATA PHY
  2013-12-04 17:16 ` [PATCH 4/5] Phy: Add DT nodes on kirkwood and Dove for the SATA PHY Andrew Lunn
@ 2013-12-04 20:01   ` Sergei Shtylyov
  2013-12-04 19:18     ` Andrew Lunn
  2013-12-05  6:17   ` Kishon Vijay Abraham I
  1 sibling, 1 reply; 18+ messages in thread
From: Sergei Shtylyov @ 2013-12-04 20:01 UTC (permalink / raw)
  To: Andrew Lunn, Jason Cooper, kishon
  Cc: devicetree, linux-ide, Gregory Clement, Sebastian Hesselbarth

Hello.

On 12/04/2013 08:16 PM, Andrew Lunn wrote:

> Add nodes for the two SATA PHYs on kirkewood.
> Add node for the one SATA PHY on Dove.
> Add pHandles to the PHYs in the sata nodes.

    I don't think this patch should be pushed thru the libata tree. However, 
you didn't Cc linux-arm-kernel...

> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
[...]

> diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi
> index 113a8bc7bee7..d56b3c675249 100644
> --- a/arch/arm/boot/dts/dove.dtsi
> +++ b/arch/arm/boot/dts/dove.dtsi
> @@ -490,10 +490,21 @@
[...]
> +			sata_phy0: sata_phy@0 {

    The node should rather be named "sata-phy@0" to be in line with 
"ethernet-phy" found in the ePAPR spec. [1], hyphen is generally preferred to 
underscore in the device trees.

> +				compatible = "marvell,mvebu-sata-phy";
> +				reg = <0xa2000 0x0334>;
> +				clocks = <&gate_clk 3>;
> +				clock-names = "sata";
> +				#phy-cells = <0>;
> +				status = "ok";
> +			};
> +
>   			rtc: real-time-clock@d8500 {
>   				compatible = "marvell,orion-rtc";
>   				reg = <0xd8500 0x20>;
[...]
> diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi
> index 8b73c80f1dad..5558b89d48ce 100644
> --- a/arch/arm/boot/dts/kirkwood.dtsi
> +++ b/arch/arm/boot/dts/kirkwood.dtsi
> @@ -282,5 +282,23 @@
>   				/* set phy-handle property in board file */
>   			};
>   		};
> +
> +		sata_phy0: sata_phy@0 {

    Same comment.

> +			compatible = "marvell,mvebu-sata-phy";
> +			reg = <0x82000 0x0334>;
> +			clocks = <&gate_clk 14>;
> +			clock-names = "sata";
> +			#phy-cells = <0>;
> +			status = "ok";
> +		};
> +
> +		sata_phy1: sata_phy@1 {

    ... and here.

> +			compatible = "marvell,mvebu-sata-phy";
> +			reg = <0x84000 0x0334>;
> +			clocks = <&gate_clk 15>;
> +			clock-names = "sata";
> +			#phy-cells = <0>;
> +			status = "ok";
> +		};
>   	};
>   };

[1] http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf

WBR, Sergei



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

* Re: [PATCH 5/5] ARM: defconfig: Enable generic phy in dove and kirkwood.
  2013-12-04 17:16 ` [PATCH 5/5] ARM: defconfig: Enable generic phy in dove and kirkwood Andrew Lunn
@ 2013-12-04 20:03   ` Sergei Shtylyov
  0 siblings, 0 replies; 18+ messages in thread
From: Sergei Shtylyov @ 2013-12-04 20:03 UTC (permalink / raw)
  To: Andrew Lunn, Jason Cooper, kishon
  Cc: devicetree, linux-ide, Gregory Clement, Sebastian Hesselbarth

Hello.

On 12/04/2013 08:16 PM, Andrew Lunn wrote:

> Now that there is a phy driver for sata, enable generic phy in the
> kirkwood and dove defconfig.

    Same comment about missing linux-arm-kernel in the CC -- this is not a 
libata patch but ARM one.

> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

WBR, Sergei


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

* Re: [PATCH 1/5] Phy: DT binding documentation for Marvell MVEBU SATA phy.
  2013-12-04 17:16 ` [PATCH 1/5] Phy: DT binding documentation for Marvell MVEBU SATA phy Andrew Lunn
@ 2013-12-05  6:03   ` Kishon Vijay Abraham I
  2013-12-05  9:43     ` Andrew Lunn
  0 siblings, 1 reply; 18+ messages in thread
From: Kishon Vijay Abraham I @ 2013-12-05  6:03 UTC (permalink / raw)
  To: Andrew Lunn, Jason Cooper
  Cc: devicetree, linux-ide, Gregory Clement, Sebastian Hesselbarth

Hi,

On Wednesday 04 December 2013 10:46 PM, Andrew Lunn wrote:
> Describe the binding for the Marvell MVEBU SATA phy. This driver
> can be used at least with Kirkwood, Dove and maybe others.
> Additionally, update the SATA binding with the properties to link
> to the phy nodes.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  Documentation/devicetree/bindings/ata/marvell.txt  |  6 ++++++
>  .../devicetree/bindings/phy/phy-mvebu-sata.txt     | 22 ++++++++++++++++++++++
>  2 files changed, 28 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/phy-mvebu-sata.txt
> 
> diff --git a/Documentation/devicetree/bindings/ata/marvell.txt b/Documentation/devicetree/bindings/ata/marvell.txt
> index b5cdd20cde9c..e072fa105b49 100644
> --- a/Documentation/devicetree/bindings/ata/marvell.txt
> +++ b/Documentation/devicetree/bindings/ata/marvell.txt
> @@ -6,11 +6,17 @@ Required Properties:
>  - interrupts    : Interrupt controller is using
>  - nr-ports      : Number of SATA ports in use.
>  
> +Optional Properties:
> +- phys		: List of phandles to sata phys
> +- phy-names	: Should be "0", "1", etc, one number per phandle

over aligned..
> +
>  Example:
>  
>  	sata@80000 {
>  		compatible = "marvell,orion-sata";
>  		reg = <0x80000 0x5000>;
>  		interrupts = <21>;
> +		phys = <&sata_phy0>, <&sata_phy1>;
> +		phy-names = "0", "1";

more descriptive phy-names? sata-phy0?
>  		nr-ports = <2>;
>  	}
> diff --git a/Documentation/devicetree/bindings/phy/phy-mvebu-sata.txt b/Documentation/devicetree/bindings/phy/phy-mvebu-sata.txt
> new file mode 100644
> index 000000000000..1cf9cef50b4b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-mvebu-sata.txt

Just name this mvebu-phy.txt so that we can add bindings of other mvebu PHYs
here when it's added.
> @@ -0,0 +1,22 @@
> +* Marvell MVEBU SATA PHY
> +
> +Power control for the SATA phy found on Marvell MVEBU SoCs.
> +
> +This document extends the binding described in phy-bindings.txt
> +
> +Required properties :
> +
> + - reg		   : Offset and length of the register set for the SATA device
> + - compatible	   : Should be "marvell,mvebu-sata-phy"
> + - clocks	   : phandle of clock that supplies the SATA device

some alignment mismatch here?
> + - clock-names	   : Should be "sata"
> +
> +Example:
> +		sata-phy@1 {

The value after '@' must match the first address specified in the reg property
of the node according to the ePAPR spec.
> +			compatible = "marvell,mvebu-sata-phy";
> +			reg = <0x84000 0x0334>;
> +			clocks = <&gate_clk 15>;
> +			clock-names = "sata";
> +			#phy-cells = <1>;

Is it on purpose that your are having phy-cells value to 1?

Thanks
Kishon

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

* Re: [PATCH 2/5] Phy: Add a PHY driver for Marvell MVEBU SATA PHY.
  2013-12-04 17:16   ` [PATCH 2/5] Phy: Add a PHY driver for Marvell MVEBU SATA PHY Andrew Lunn
@ 2013-12-05  6:10     ` Kishon Vijay Abraham I
  0 siblings, 0 replies; 18+ messages in thread
From: Kishon Vijay Abraham I @ 2013-12-05  6:10 UTC (permalink / raw)
  To: Andrew Lunn, Jason Cooper
  Cc: devicetree, linux-ide, Gregory Clement, Sebastian Hesselbarth

Hi,

On Wednesday 04 December 2013 10:46 PM, Andrew Lunn wrote:
> Kirkwood and Dove can turn the SATA phy on and off. Add a PHY driver
> to control this.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/phy/Kconfig          |   5 ++
>  drivers/phy/Makefile         |   1 +
>  drivers/phy/phy-mvebu-sata.c | 130 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 136 insertions(+)
>  create mode 100644 drivers/phy/phy-mvebu-sata.c
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index a344f3d52361..2dd97c3bdab7 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -21,6 +21,11 @@ config PHY_EXYNOS_MIPI_VIDEO
>  	  Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung S5P
>  	  and EXYNOS SoCs.
>  
> +config PHY_MVEBU_SATA
> +       def_bool y
> +       depends on ARCH_KIRKWOOD || ARCH_DOVE
> +       depends on OF

select GENERIC_PHY?
> +
>  config OMAP_USB2
>  	tristate "OMAP USB2 PHY Driver"
>  	depends on ARCH_OMAP2PLUS
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index d0caae9cfb83..4e4adc96f753 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -5,5 +5,6 @@
>  obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
>  obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
>  obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
> +obj-$(CONFIG_PHY_MVEBU_SATA)		+= phy-mvebu-sata.o
>  obj-$(CONFIG_OMAP_USB2)			+= phy-omap-usb2.o
>  obj-$(CONFIG_TWL4030_USB)		+= phy-twl4030-usb.o
> diff --git a/drivers/phy/phy-mvebu-sata.c b/drivers/phy/phy-mvebu-sata.c
> new file mode 100644
> index 000000000000..7d40c3afb090
> --- /dev/null
> +++ b/drivers/phy/phy-mvebu-sata.c
> @@ -0,0 +1,130 @@
> +/*
> + *	phy-mvebu-sata.c: SATA Phy driver for the Marvell mvebu SoCs.
> + *
> + *	Copyright (C) 2013 Andrew Lunn <andrew@lunn.ch>
> + *
> + *	This program is free software; you can redistribute it and/or
> + *	modify it under the terms of the GNU General Public License
> + *	as published by the Free Software Foundation; either version
> + *	2 of the License, or (at your option) any later version.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/clk.h>
> +#include <linux/phy/phy.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +
> +struct priv {
> +	struct clk	*clk;
> +	void __iomem	*base;
> +};
> +
> +#define SATA_PHY_MODE_2	0x0330
> +#define SATA_IF_CTRL	0x0050
> +
> +static int phy_mvebu_sata_power_on(struct phy *phy)
> +{
> +	struct priv *priv = phy_get_drvdata(phy);
> +	u32 reg;
> +
> +	clk_prepare_enable(priv->clk);
> +
> +	/* Enable PLL and IVREF */
> +	reg = readl(priv->base + SATA_PHY_MODE_2);
> +	reg |= 0xf;

No magic values here. Please add macros for these.
> +	writel(reg , priv->base + SATA_PHY_MODE_2);
> +
> +	/* Enable PHY */
> +	reg = readl(priv->base + SATA_IF_CTRL);
> +	reg &= ~0x200;

same here.
> +	writel(reg, priv->base + SATA_IF_CTRL);

It would be nice to add mvebu_readl and mvebu_writel apis. No strong feelings
though.
> +
> +	clk_disable_unprepare(priv->clk);
> +
> +	return 0;
> +}
> +
> +static int phy_mvebu_sata_power_off(struct phy *phy)
> +{
> +	struct priv *priv = phy_get_drvdata(phy);
> +	u32 reg;
> +
> +	clk_prepare_enable(priv->clk);
> +
> +	/* Disable PLL and IVREF */
> +	reg = readl(priv->base + SATA_PHY_MODE_2);
> +	reg &= ~0xf;

no magic values.
> +	writel(reg, priv->base + SATA_PHY_MODE_2);
> +
> +	/* Disable PHY */
> +	reg = readl(priv->base + SATA_IF_CTRL);
> +	reg |= 0x200;

same here.
> +	writel(reg, priv->base + SATA_IF_CTRL);
> +
> +	clk_disable_unprepare(priv->clk);
> +
> +	return 0;
> +}
> +
> +static struct phy_ops phy_mvebu_sata_ops = {
> +	.power_on	= phy_mvebu_sata_power_on,
> +	.power_off	= phy_mvebu_sata_power_off,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static int phy_mvebu_sata_probe(struct platform_device *pdev)
> +{
> +	struct phy_provider *phy_provider;
> +	struct resource *res;
> +	struct priv *priv;
> +	struct phy *phy;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	priv->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(priv->base))
> +		return PTR_ERR(priv->base);
> +
> +	priv->clk = devm_clk_get(&pdev->dev, "sata");
> +	if (IS_ERR(priv->clk))
> +		return PTR_ERR(priv->clk);
> +
> +	phy_provider = devm_of_phy_provider_register(&pdev->dev,
> +						     of_phy_simple_xlate);

heh.. so your PHY implements a sinlge PHY device onle. You need to have '0' as
phy-cells values in both Documentation and dt data.

Thanks
Kishon

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

* Re: [PATCH 3/5] SATA: MV: Add support for the optional PHYs
  2013-12-04 17:16 ` [PATCH 3/5] SATA: MV: Add support for the optional PHYs Andrew Lunn
@ 2013-12-05  6:15   ` Kishon Vijay Abraham I
  0 siblings, 0 replies; 18+ messages in thread
From: Kishon Vijay Abraham I @ 2013-12-05  6:15 UTC (permalink / raw)
  To: Andrew Lunn, Jason Cooper
  Cc: devicetree, linux-ide, Gregory Clement, Sebastian Hesselbarth

Hi,

On Wednesday 04 December 2013 10:46 PM, Andrew Lunn wrote:
> Some Marvell SoCs have a SATA PHY which can be powered off, in order
> to save power. Make use of the generic phy framework to control these
> phys.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/ata/sata_mv.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
> index 56be31819897..9a728bcfbb9a 100644
> --- a/drivers/ata/sata_mv.c
> +++ b/drivers/ata/sata_mv.c
> @@ -60,6 +60,7 @@
>  #include <linux/dma-mapping.h>
>  #include <linux/device.h>
>  #include <linux/clk.h>
> +#include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
>  #include <linux/ata_platform.h>
>  #include <linux/mbus.h>
> @@ -563,6 +564,12 @@ struct mv_host_priv {
>  	struct clk		*clk;
>  	struct clk              **port_clks;
>  	/*
> +	 * Some devices have a SATA PHY which can be enabled/disabled
> +	 * in order to save power. These are optional: if the platform
> +	 * devices does not have any phy, they won't be used.
> +	 */
> +	struct phy		**port_phys;
> +	/*
>  	 * These consistent DMA memory pools give us guaranteed
>  	 * alignment for hardware-accessed data structures,
>  	 * and less memory waste in accomplishing the alignment.
> @@ -4076,6 +4083,11 @@ static int mv_platform_probe(struct platform_device *pdev)
>  					GFP_KERNEL);
>  	if (!hpriv->port_clks)
>  		return -ENOMEM;
> +	hpriv->port_phys = devm_kzalloc(&pdev->dev,
> +					sizeof(struct phy *) * n_ports,
> +					GFP_KERNEL);
> +	if (!hpriv->port_phys)
> +		return -ENOMEM;
>  	host->private_data = hpriv;
>  	hpriv->n_ports = n_ports;
>  	hpriv->board_idx = chip_soc;
> @@ -4097,6 +4109,9 @@ static int mv_platform_probe(struct platform_device *pdev)
>  		hpriv->port_clks[port] = clk_get(&pdev->dev, port_number);
>  		if (!IS_ERR(hpriv->port_clks[port]))
>  			clk_prepare_enable(hpriv->port_clks[port]);
> +		hpriv->port_phys[port] = devm_phy_get(&pdev->dev, port_number);

once you change the phy-names in dt data, corresponding change should be done
here too.
> +		if (!IS_ERR(hpriv->port_phys[port]))
> +			phy_power_on(hpriv->port_phys[port]);
>  	}
>  
>  	/*
> @@ -4132,6 +4147,8 @@ err:
>  			clk_disable_unprepare(hpriv->port_clks[port]);
>  			clk_put(hpriv->port_clks[port]);
>  		}
> +		if (!IS_ERR(hpriv->port_phys[port]))
> +			phy_power_off(hpriv->port_phys[port]);
>  	}
>  
>  	return rc;
> @@ -4161,6 +4178,8 @@ static int mv_platform_remove(struct platform_device *pdev)
>  			clk_disable_unprepare(hpriv->port_clks[port]);
>  			clk_put(hpriv->port_clks[port]);
>  		}
> +		if (!IS_ERR(hpriv->port_phys[port]))
> +			phy_power_off(hpriv->port_phys[port]);
>  	}
>  	return 0;
>  }
> 
Apart from that comment this patch looks good.

Acked-by: Kishon Vijay Abraham I <kishon@ti.com>

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

* Re: [PATCH 4/5] Phy: Add DT nodes on kirkwood and Dove for the SATA PHY
  2013-12-04 17:16 ` [PATCH 4/5] Phy: Add DT nodes on kirkwood and Dove for the SATA PHY Andrew Lunn
  2013-12-04 20:01   ` Sergei Shtylyov
@ 2013-12-05  6:17   ` Kishon Vijay Abraham I
  1 sibling, 0 replies; 18+ messages in thread
From: Kishon Vijay Abraham I @ 2013-12-05  6:17 UTC (permalink / raw)
  To: Andrew Lunn, Jason Cooper
  Cc: devicetree, linux-ide, Gregory Clement, Sebastian Hesselbarth

Hi,
On Wednesday 04 December 2013 10:46 PM, Andrew Lunn wrote:
> Add nodes for the two SATA PHYs on kirkewood.
> Add node for the one SATA PHY on Dove.
> Add pHandles to the PHYs in the sata nodes.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  arch/arm/boot/dts/dove.dtsi          | 11 +++++++++++
>  arch/arm/boot/dts/kirkwood-6281.dtsi |  2 ++
>  arch/arm/boot/dts/kirkwood-6282.dtsi |  2 ++
>  arch/arm/boot/dts/kirkwood.dtsi      | 18 ++++++++++++++++++
>  4 files changed, 33 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi
> index 113a8bc7bee7..d56b3c675249 100644
> --- a/arch/arm/boot/dts/dove.dtsi
> +++ b/arch/arm/boot/dts/dove.dtsi
> @@ -490,10 +490,21 @@
>  				reg = <0xa0000 0x2400>;
>  				interrupts = <62>;
>  				clocks = <&gate_clk 3>;
> +				phys = <&sata_phy0>;
> +				phy-names = "0";

A more descriptive phy-names would be good here and below.
>  				nr-ports = <1>;
>  				status = "disabled";
>  			};
>  
> +			sata_phy0: sata_phy@0 {
> +				compatible = "marvell,mvebu-sata-phy";
> +				reg = <0xa2000 0x0334>;
> +				clocks = <&gate_clk 3>;
> +				clock-names = "sata";
> +				#phy-cells = <0>;

Ah.. only you documentation was having this value as '1'. Only that needs to be
fixed then.

Thanks
Kishon

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

* Re: [PATCH 1/5] Phy: DT binding documentation for Marvell MVEBU SATA phy.
  2013-12-05  6:03   ` Kishon Vijay Abraham I
@ 2013-12-05  9:43     ` Andrew Lunn
  2013-12-05  9:47       ` Andrew Lunn
                         ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Andrew Lunn @ 2013-12-05  9:43 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Andrew Lunn, Jason Cooper, devicetree, linux-ide, Gregory Clement,
	Sebastian Hesselbarth

On Thu, Dec 05, 2013 at 11:33:46AM +0530, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Wednesday 04 December 2013 10:46 PM, Andrew Lunn wrote:
> > Describe the binding for the Marvell MVEBU SATA phy. This driver
> > can be used at least with Kirkwood, Dove and maybe others.
> > Additionally, update the SATA binding with the properties to link
> > to the phy nodes.
> > 
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > ---
> >  Documentation/devicetree/bindings/ata/marvell.txt  |  6 ++++++
> >  .../devicetree/bindings/phy/phy-mvebu-sata.txt     | 22 ++++++++++++++++++++++
> >  2 files changed, 28 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/phy/phy-mvebu-sata.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/ata/marvell.txt b/Documentation/devicetree/bindings/ata/marvell.txt
> > index b5cdd20cde9c..e072fa105b49 100644
> > --- a/Documentation/devicetree/bindings/ata/marvell.txt
> > +++ b/Documentation/devicetree/bindings/ata/marvell.txt
> > @@ -6,11 +6,17 @@ Required Properties:
> >  - interrupts    : Interrupt controller is using
> >  - nr-ports      : Number of SATA ports in use.
> >  
> > +Optional Properties:
> > +- phys		: List of phandles to sata phys
> > +- phy-names	: Should be "0", "1", etc, one number per phandle
> 
> over aligned..

Could you explain please? Here is what it looks like without the patch
formatting, which could be messing up the display of tabs, due to the
+ at the beginning:

Required Properties:
- compatibility : "marvell,orion-sata"
- reg		: Address range of controller
- interrupts	: Interrupt controller is using
- nr-ports	: Number of SATA ports in use.

Optional Properties:
- phys		: List of phandles to sata phys
- phy-names	: Should be "0", "1", etc, one number per phandle



> > +
> >  Example:
> >  
> >  	sata@80000 {
> >  		compatible = "marvell,orion-sata";
> >  		reg = <0x80000 0x5000>;
> >  		interrupts = <21>;
> > +		phys = <&sata_phy0>, <&sata_phy1>;
> > +		phy-names = "0", "1";
> 
> more descriptive phy-names? sata-phy0?
> >  		nr-ports = <2>;

I could do, but i was following how the clocks work. Unfortunately,
the binding documentation is out of date and does not contain
clocks. A real example is:

		sata@80000 {
			compatible = "marvell,orion-sata";
			reg = <0x80000 0x5000>;
			interrupts = <21>;
			clocks = <&gate_clk 14>, <&gate_clk 15>;
			clock-names = "0", "1";
			phys = <&sata_phy0>, <&sata_phy1>;
			phy-names = "0", "1";
			status = "disabled";
		};

So clocks and the phy are described nearly identically. I can however
handle phys differently if you wish.

I will also submit a separate patch to the binding documentation to
add the clocks.


> >  	}
> > diff --git a/Documentation/devicetree/bindings/phy/phy-mvebu-sata.txt b/Documentation/devicetree/bindings/phy/phy-mvebu-sata.txt
> > new file mode 100644
> > index 000000000000..1cf9cef50b4b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/phy-mvebu-sata.txt
> 
> Just name this mvebu-phy.txt so that we can add bindings of other mvebu PHYs
> here when it's added.

O.K. I also have a pcie phy driver, but it does not work
yet. devm_phy_get() is too restrictive for a complex device like a
PCIe controller.

> > +		sata-phy@1 {
> 
> The value after '@' must match the first address specified in the reg property
> of the node according to the ePAPR spec.

O.K, will fix.

> > +			compatible = "marvell,mvebu-sata-phy";
> > +			reg = <0x84000 0x0334>;
> > +			clocks = <&gate_clk 15>;
> > +			clock-names = "sata";
> > +			#phy-cells = <1>;
> 
> Is it on purpose that your are having phy-cells value to 1?

Yes. Each instance only controls one phy.

     Thanks
	Andrew

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

* Re: [PATCH 1/5] Phy: DT binding documentation for Marvell MVEBU SATA phy.
  2013-12-05  9:43     ` Andrew Lunn
@ 2013-12-05  9:47       ` Andrew Lunn
  2013-12-05 10:35       ` Kishon Vijay Abraham I
  2013-12-10 22:59       ` Andrew Lunn
  2 siblings, 0 replies; 18+ messages in thread
From: Andrew Lunn @ 2013-12-05  9:47 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Kishon Vijay Abraham I, Jason Cooper, devicetree, linux-ide,
	Gregory Clement, Sebastian Hesselbarth

> > > +			compatible = "marvell,mvebu-sata-phy";
> > > +			reg = <0x84000 0x0334>;
> > > +			clocks = <&gate_clk 15>;
> > > +			clock-names = "sata";
> > > +			#phy-cells = <1>;
> > 
> > Is it on purpose that your are having phy-cells value to 1?
> 
> Yes. Each instance only controls one phy.

Ah, sorry, got that wrong. As you say in a later comment, this should
be 0. Will be fixed in v2.

   Thanks
	Andrew

   

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

* Re: [PATCH 1/5] Phy: DT binding documentation for Marvell MVEBU SATA phy.
  2013-12-05  9:43     ` Andrew Lunn
  2013-12-05  9:47       ` Andrew Lunn
@ 2013-12-05 10:35       ` Kishon Vijay Abraham I
  2013-12-10 22:59       ` Andrew Lunn
  2 siblings, 0 replies; 18+ messages in thread
From: Kishon Vijay Abraham I @ 2013-12-05 10:35 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Jason Cooper, devicetree, linux-ide, Gregory Clement,
	Sebastian Hesselbarth

Hi,

On Thursday 05 December 2013 03:13 PM, Andrew Lunn wrote:
> On Thu, Dec 05, 2013 at 11:33:46AM +0530, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Wednesday 04 December 2013 10:46 PM, Andrew Lunn wrote:
>>> Describe the binding for the Marvell MVEBU SATA phy. This driver
>>> can be used at least with Kirkwood, Dove and maybe others.
>>> Additionally, update the SATA binding with the properties to link
>>> to the phy nodes.
>>>
>>> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
>>> ---
>>>  Documentation/devicetree/bindings/ata/marvell.txt  |  6 ++++++
>>>  .../devicetree/bindings/phy/phy-mvebu-sata.txt     | 22 ++++++++++++++++++++++
>>>  2 files changed, 28 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/phy/phy-mvebu-sata.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/ata/marvell.txt b/Documentation/devicetree/bindings/ata/marvell.txt
>>> index b5cdd20cde9c..e072fa105b49 100644
>>> --- a/Documentation/devicetree/bindings/ata/marvell.txt
>>> +++ b/Documentation/devicetree/bindings/ata/marvell.txt
>>> @@ -6,11 +6,17 @@ Required Properties:
>>>  - interrupts    : Interrupt controller is using
>>>  - nr-ports      : Number of SATA ports in use.
>>>  
>>> +Optional Properties:
>>> +- phys		: List of phandles to sata phys
>>> +- phy-names	: Should be "0", "1", etc, one number per phandle
>>
>> over aligned..
> 
> Could you explain please? Here is what it looks like without the patch
> formatting, which could be messing up the display of tabs, due to the
> + at the beginning:

ah.. yeah. Sorry for the noise.

Thanks
Kishon
> 
> Required Properties:
> - compatibility : "marvell,orion-sata"
> - reg		: Address range of controller
> - interrupts	: Interrupt controller is using
> - nr-ports	: Number of SATA ports in use.
> 
> Optional Properties:
> - phys		: List of phandles to sata phys
> - phy-names	: Should be "0", "1", etc, one number per phandle
> 
> 
> 
>>> +
>>>  Example:
>>>  
>>>  	sata@80000 {
>>>  		compatible = "marvell,orion-sata";
>>>  		reg = <0x80000 0x5000>;
>>>  		interrupts = <21>;
>>> +		phys = <&sata_phy0>, <&sata_phy1>;
>>> +		phy-names = "0", "1";
>>
>> more descriptive phy-names? sata-phy0?
>>>  		nr-ports = <2>;
> 
> I could do, but i was following how the clocks work. Unfortunately,
> the binding documentation is out of date and does not contain
> clocks. A real example is:
> 
> 		sata@80000 {
> 			compatible = "marvell,orion-sata";
> 			reg = <0x80000 0x5000>;
> 			interrupts = <21>;
> 			clocks = <&gate_clk 14>, <&gate_clk 15>;
> 			clock-names = "0", "1";
> 			phys = <&sata_phy0>, <&sata_phy1>;
> 			phy-names = "0", "1";
> 			status = "disabled";
> 		};
> 
> So clocks and the phy are described nearly identically. I can however
> handle phys differently if you wish.
> 
> I will also submit a separate patch to the binding documentation to
> add the clocks.
> 
> 
>>>  	}
>>> diff --git a/Documentation/devicetree/bindings/phy/phy-mvebu-sata.txt b/Documentation/devicetree/bindings/phy/phy-mvebu-sata.txt
>>> new file mode 100644
>>> index 000000000000..1cf9cef50b4b
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/phy/phy-mvebu-sata.txt
>>
>> Just name this mvebu-phy.txt so that we can add bindings of other mvebu PHYs
>> here when it's added.
> 
> O.K. I also have a pcie phy driver, but it does not work
> yet. devm_phy_get() is too restrictive for a complex device like a
> PCIe controller.
> 
>>> +		sata-phy@1 {
>>
>> The value after '@' must match the first address specified in the reg property
>> of the node according to the ePAPR spec.
> 
> O.K, will fix.
> 
>>> +			compatible = "marvell,mvebu-sata-phy";
>>> +			reg = <0x84000 0x0334>;
>>> +			clocks = <&gate_clk 15>;
>>> +			clock-names = "sata";
>>> +			#phy-cells = <1>;
>>
>> Is it on purpose that your are having phy-cells value to 1?
> 
> Yes. Each instance only controls one phy.
> 
>      Thanks
> 	Andrew
> 


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

* Re: [PATCH 1/5] Phy: DT binding documentation for Marvell MVEBU SATA phy.
  2013-12-05  9:43     ` Andrew Lunn
  2013-12-05  9:47       ` Andrew Lunn
  2013-12-05 10:35       ` Kishon Vijay Abraham I
@ 2013-12-10 22:59       ` Andrew Lunn
  2013-12-11  5:41         ` Kishon Vijay Abraham I
  2 siblings, 1 reply; 18+ messages in thread
From: Andrew Lunn @ 2013-12-10 22:59 UTC (permalink / raw)
  To: Andrew Lunn, kishon
  Cc: Jason Cooper, devicetree, linux-ide, Gregory Clement,
	Sebastian Hesselbarth

On Thu, Dec 05, 2013 at 10:43:19AM +0100, Andrew Lunn wrote:
> > >  Example:
> > >  
> > >  	sata@80000 {
> > >  		compatible = "marvell,orion-sata";
> > >  		reg = <0x80000 0x5000>;
> > >  		interrupts = <21>;
> > > +		phys = <&sata_phy0>, <&sata_phy1>;
> > > +		phy-names = "0", "1";
> > 
> > more descriptive phy-names? sata-phy0?
> > >  		nr-ports = <2>;
> 
> I could do, but i was following how the clocks work. Unfortunately,
> the binding documentation is out of date and does not contain
> clocks. A real example is:
> 
>               sata@80000 {
>                       compatible = "marvell,orion-sata";
>                       reg = <0x80000 0x5000>;
>                       interrupts = <21>;
>                       clocks = <&gate_clk 14>, <&gate_clk 15>;
>                       clock-names = "0", "1";
>                       phys = <&sata_phy0>, <&sata_phy1>;
>                       phy-names = "0", "1";
>                       status = "disabled";
>               };
> 
> So clocks and the phy are described nearly identically. I can however
> handle phys differently if you wish.

Hi Kishon

Please could you comment on this. Are you O.K. if i use the same
naming scheme for phys as clocks?

Thanks
	Andrew

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

* Re: [PATCH 1/5] Phy: DT binding documentation for Marvell MVEBU SATA phy.
  2013-12-10 22:59       ` Andrew Lunn
@ 2013-12-11  5:41         ` Kishon Vijay Abraham I
  0 siblings, 0 replies; 18+ messages in thread
From: Kishon Vijay Abraham I @ 2013-12-11  5:41 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Jason Cooper, devicetree, linux-ide, Gregory Clement,
	Sebastian Hesselbarth

Hi,

On Wednesday 11 December 2013 04:29 AM, Andrew Lunn wrote:
> On Thu, Dec 05, 2013 at 10:43:19AM +0100, Andrew Lunn wrote:
>>>>  Example:
>>>>  
>>>>  	sata@80000 {
>>>>  		compatible = "marvell,orion-sata";
>>>>  		reg = <0x80000 0x5000>;
>>>>  		interrupts = <21>;
>>>> +		phys = <&sata_phy0>, <&sata_phy1>;
>>>> +		phy-names = "0", "1";
>>>
>>> more descriptive phy-names? sata-phy0?
>>>>  		nr-ports = <2>;
>>
>> I could do, but i was following how the clocks work. Unfortunately,
>> the binding documentation is out of date and does not contain
>> clocks. A real example is:
>>
>>               sata@80000 {
>>                       compatible = "marvell,orion-sata";
>>                       reg = <0x80000 0x5000>;
>>                       interrupts = <21>;
>>                       clocks = <&gate_clk 14>, <&gate_clk 15>;
>>                       clock-names = "0", "1";
>>                       phys = <&sata_phy0>, <&sata_phy1>;
>>                       phy-names = "0", "1";
>>                       status = "disabled";
>>               };
>>
>> So clocks and the phy are described nearly identically. I can however
>> handle phys differently if you wish.
> 
> Hi Kishon
> 
> Please could you comment on this. Are you O.K. if i use the same
> naming scheme for phys as clocks?

I'd prefer descriptive names for phys.

Thanks
Kishon

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

end of thread, other threads:[~2013-12-11  5:41 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-04 17:15 [PATCH 0/5] MVEBU SATA PHY driver Andrew Lunn
2013-12-04 17:16 ` [PATCH 1/5] Phy: DT binding documentation for Marvell MVEBU SATA phy Andrew Lunn
2013-12-05  6:03   ` Kishon Vijay Abraham I
2013-12-05  9:43     ` Andrew Lunn
2013-12-05  9:47       ` Andrew Lunn
2013-12-05 10:35       ` Kishon Vijay Abraham I
2013-12-10 22:59       ` Andrew Lunn
2013-12-11  5:41         ` Kishon Vijay Abraham I
     [not found] ` <1386177364-10164-1-git-send-email-andrew-g2DYL2Zd6BY@public.gmane.org>
2013-12-04 17:16   ` [PATCH 2/5] Phy: Add a PHY driver for Marvell MVEBU SATA PHY Andrew Lunn
2013-12-05  6:10     ` Kishon Vijay Abraham I
2013-12-04 17:16 ` [PATCH 3/5] SATA: MV: Add support for the optional PHYs Andrew Lunn
2013-12-05  6:15   ` Kishon Vijay Abraham I
2013-12-04 17:16 ` [PATCH 4/5] Phy: Add DT nodes on kirkwood and Dove for the SATA PHY Andrew Lunn
2013-12-04 20:01   ` Sergei Shtylyov
2013-12-04 19:18     ` Andrew Lunn
2013-12-05  6:17   ` Kishon Vijay Abraham I
2013-12-04 17:16 ` [PATCH 5/5] ARM: defconfig: Enable generic phy in dove and kirkwood Andrew Lunn
2013-12-04 20:03   ` Sergei Shtylyov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).