[PATCH v2 0/5] MIPS: ath79: Add USB support on the TL-WR1043ND

[PATCH v2 0/5] MIPS: ath79: Add USB support on the TL-WR1043ND

[Alban Bedel]

Hi,

First re-roll of this patch series to add a driver for the USB phy
on ATH79 SoCs. As suggested in the previous series this new version
include a generic driver for simple phys which is re-used for the
ATH79 driver. This generic driver should be useful for all kind of
phys that don't need any configuration.

Alban

Alban Bedel (5):
  phy: Add a driver for simple phy
  devicetree: Add bindings for the ATH79 USB phy
  phy: Add a driver for the ATH79 USB phy
  MIPS: ath79: Add the EHCI controller and USB phy to the AR9132 dtsi
  MIPS: ath79: Enable the USB port on the TL-WR1043ND

 .../devicetree/bindings/phy/phy-ath79-usb.txt      |  18 ++
 MAINTAINERS                                        |   8 +
 arch/mips/boot/dts/qca/ar9132.dtsi                 |  26 +++
 arch/mips/boot/dts/qca/ar9132_tl_wr1043nd_v1.dts   |   8 +
 drivers/phy/Kconfig                                |  20 ++
 drivers/phy/Makefile                               |   2 +
 drivers/phy/phy-ath79-usb.c                        | 116 ++++++++++++
 drivers/phy/phy-simple.c                           | 204 +++++++++++++++++++++
 include/linux/phy/simple.h                         |  39 ++++
 9 files changed, 441 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-ath79-usb.txt
 create mode 100644 drivers/phy/phy-ath79-usb.c
 create mode 100644 drivers/phy/phy-simple.c
 create mode 100644 include/linux/phy/simple.h

-- 
2.0.0

[PATCH v2 1/5] phy: Add a driver for simple phy

[Alban Bedel]

This driver is meant to take care of all trivial phys that don't need
any special configuration, it just enable a regulator, a clock and
deassert a reset. A public API is also included to allow re-using the
code in other drivers.

Signed-off-by: Alban Bedel <albeu@free.fr>
---
 drivers/phy/Kconfig        |  12 +++
 drivers/phy/Makefile       |   1 +
 drivers/phy/phy-simple.c   | 204 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/phy/simple.h |  39 +++++++++
 4 files changed, 256 insertions(+)
 create mode 100644 drivers/phy/phy-simple.c
 create mode 100644 include/linux/phy/simple.h

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 7eb5859d..028fb16 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -118,6 +118,18 @@ config PHY_RCAR_GEN2
 	help
 	  Support for USB PHY found on Renesas R-Car generation 2 SoCs.
 
+config PHY_SIMPLE
+	tristate
+	select GENERIC_PHY
+	help
+
+config PHY_SIMPLE_PDEV
+	tristate "Simple PHY driver"
+	select PHY_SIMPLE
+	help
+	  A PHY driver for simple devices that only need a regulator, clock
+	  and reset for power up and shutdown.
+
 config OMAP_CONTROL_PHY
 	tristate "OMAP CONTROL PHY Driver"
 	depends on ARCH_OMAP2PLUS || COMPILE_TEST
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 075db1a..1a44362 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_TWL4030_USB)		+= phy-twl4030-usb.o
 obj-$(CONFIG_PHY_EXYNOS5250_SATA)	+= phy-exynos5250-sata.o
 obj-$(CONFIG_PHY_HIX5HD2_SATA)		+= phy-hix5hd2-sata.o
 obj-$(CONFIG_PHY_MT65XX_USB3)		+= phy-mt65xx-usb3.o
+obj-$(CONFIG_PHY_SIMPLE)		+= phy-simple.o
 obj-$(CONFIG_PHY_SUN4I_USB)		+= phy-sun4i-usb.o
 obj-$(CONFIG_PHY_SUN9I_USB)		+= phy-sun9i-usb.o
 obj-$(CONFIG_PHY_SAMSUNG_USB2)		+= phy-exynos-usb2.o
diff --git a/drivers/phy/phy-simple.c b/drivers/phy/phy-simple.c
new file mode 100644
index 0000000..013f846
--- /dev/null
+++ b/drivers/phy/phy-simple.c
@@ -0,0 +1,204 @@
+/*
+ * Copyright (C) 2015 Alban Bedel <albeu@free.fr>
+ *
+ * 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/module.h>
+#include <linux/of_device.h>
+#include <linux/phy/simple.h>
+#include <linux/clk.h>
+#include <linux/reset.h>
+
+int simple_phy_power_on(struct phy *phy)
+{
+	struct simple_phy *sphy = phy_get_drvdata(phy);
+	int err;
+
+	if (sphy->regulator) {
+		err = regulator_enable(sphy->regulator);
+		if (err)
+			return err;
+	}
+
+	if (sphy->clk) {
+		err = clk_prepare_enable(sphy->clk);
+		if (err)
+			goto regulator_disable;
+	}
+
+	if (sphy->reset) {
+		err = reset_control_deassert(sphy->reset);
+		if (err)
+			goto clock_disable;
+	}
+
+	return 0;
+
+clock_disable:
+	if (sphy->clk)
+		clk_disable_unprepare(sphy->clk);
+regulator_disable:
+	if (sphy->regulator)
+		WARN_ON(regulator_disable(sphy->regulator));
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(simple_phy_power_on);
+
+int simple_phy_power_off(struct phy *phy)
+{
+	struct simple_phy *sphy = phy_get_drvdata(phy);
+	int err;
+
+	if (sphy->reset) {
+		err = reset_control_assert(sphy->reset);
+		if (err)
+			return err;
+	}
+
+	if (sphy->clk)
+		clk_disable_unprepare(sphy->clk);
+
+	if (sphy->regulator) {
+		err = regulator_disable(sphy->regulator);
+		if (err)
+			goto clock_enable;
+	}
+
+	return 0;
+
+clock_enable:
+	if (sphy->clk)
+		WARN_ON(clk_prepare_enable(sphy->clk));
+	if (sphy->reset)
+		WARN_ON(reset_control_deassert(sphy->reset));
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(simple_phy_power_off);
+
+static const struct phy_ops simple_phy_ops = {
+	.power_on	= simple_phy_power_on,
+	.power_off	= simple_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
+struct phy *devm_simple_phy_create(struct device *dev,
+				const struct simple_phy_desc *desc,
+				struct simple_phy *sphy)
+{
+	struct phy *phy;
+
+	if (!dev || !desc)
+		return ERR_PTR(-EINVAL);
+
+	if (!sphy) {
+		sphy = devm_kzalloc(dev, sizeof(*sphy), GFP_KERNEL);
+		if (!sphy)
+			return ERR_PTR(-ENOMEM);
+	}
+
+	if (!IS_ERR_OR_NULL(desc->regulator)) {
+		sphy->regulator = devm_regulator_get(dev, desc->regulator);
+		if (IS_ERR(sphy->regulator)) {
+			if (PTR_ERR(sphy->regulator) == -ENOENT)
+				sphy->regulator = NULL;
+			else
+				return ERR_PTR(PTR_ERR(sphy->regulator));
+		}
+	}
+
+	if (!IS_ERR(desc->clk)) {
+		sphy->clk = devm_clk_get(dev, desc->clk);
+		if (IS_ERR(sphy->clk)) {
+			if (PTR_ERR(sphy->clk) == -ENOENT)
+				sphy->clk = NULL;
+			else
+				return ERR_PTR(PTR_ERR(sphy->clk));
+		}
+	}
+
+	if (!IS_ERR(desc->reset)) {
+		sphy->reset = devm_reset_control_get(dev, desc->reset);
+		if (IS_ERR(sphy->reset)) {
+			int err = PTR_ERR(sphy->reset);
+
+			if (err == -ENOENT || err == -ENOTSUPP)
+				sphy->reset = NULL;
+			else
+				return ERR_PTR(err);
+		}
+	}
+
+	phy = devm_phy_create(dev, NULL, desc->ops ?: &simple_phy_ops);
+	if (IS_ERR(phy))
+		return ERR_PTR(PTR_ERR(phy));
+
+	phy_set_drvdata(phy, sphy);
+
+	return phy;
+}
+EXPORT_SYMBOL_GPL(devm_simple_phy_create);
+
+#ifdef CONFIG_PHY_SIMPLE_PDEV
+#ifdef CONFIG_OF
+/* Default config, no regulator, default clock and reset if any */
+static const struct simple_phy_desc simple_phy_default_desc = {};
+
+static const struct of_device_id simple_phy_of_match[] = {
+	{ .compatible = "simple-phy", .data = &simple_phy_default_desc },
+	{}
+};
+MODULE_DEVICE_TABLE(of, simple_phy_of_match);
+
+const struct simple_phy_desc *simple_phy_get_of_desc(struct device *dev)
+{
+	const struct of_device_id *match;
+
+	match = of_match_device(simple_phy_of_match, dev);
+
+	return match ? match->data : NULL;
+}
+#else
+const struct simple_phy_desc *simple_phy_get_of_desc(struct device *dev)
+{
+	return NULL;
+}
+#endif
+
+static int simple_phy_probe(struct platform_device *pdev)
+{
+	const struct simple_phy_desc *desc = pdev->dev.platform_data;
+	struct phy *phy;
+
+	if (!desc)
+		desc = simple_phy_get_of_desc(&pdev->dev);
+	if (!desc)
+		return -EINVAL;
+
+	phy = devm_simple_phy_create(&pdev->dev, desc, NULL);
+	if (IS_ERR(phy))
+		return PTR_ERR(phy);
+
+	return PTR_ERR_OR_ZERO(devm_of_phy_provider_register(
+				&pdev->dev, of_phy_simple_xlate));
+}
+
+static struct platform_driver simple_phy_driver = {
+	.probe	= simple_phy_probe,
+	.driver = {
+		.of_match_table	= of_match_ptr(simple_phy_of_match),
+		.name		= "phy-simple",
+	}
+};
+module_platform_driver(simple_phy_driver);
+
+#endif /* CONFIG_PHY_SIMPLE_PDEV */
+
+MODULE_DESCRIPTION("Simple PHY driver");
+MODULE_AUTHOR("Alban Bedel <albeu@free.fr>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/phy/simple.h b/include/linux/phy/simple.h
new file mode 100644
index 0000000..f368b57
--- /dev/null
+++ b/include/linux/phy/simple.h
@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2015 Alban Bedel <albeu@free.fr>
+ *
+ * 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.
+ */
+
+#ifndef __LINUX_PHY_SIMPLE__
+#define __LINUX_PHY_SIMPLE__
+
+#include <linux/phy/phy.h>
+
+struct reset_control;
+struct clk;
+
+struct simple_phy {
+	struct regulator *regulator;
+	struct reset_control *reset;
+	struct clk *clk;
+};
+
+struct simple_phy_desc {
+	const struct phy_ops *ops;
+	const char *regulator;
+	const char *reset;
+	const char *clk;
+};
+
+struct phy *devm_simple_phy_create(struct device *dev,
+				const struct simple_phy_desc *desc,
+				struct simple_phy *sphy);
+
+int simple_phy_power_on(struct phy *phy);
+
+int simple_phy_power_off(struct phy *phy);
+
+#endif /* __LINUX_PHY_SIMPLE__ */
-- 
2.0.0

[PATCH v2 2/5] devicetree: Add bindings for the ATH79 USB phy

[Alban Bedel]

Signed-off-by: Alban Bedel <albeu@free.fr>
---
 .../devicetree/bindings/phy/phy-ath79-usb.txt          | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-ath79-usb.txt

diff --git a/Documentation/devicetree/bindings/phy/phy-ath79-usb.txt b/Documentation/devicetree/bindings/phy/phy-ath79-usb.txt
new file mode 100644
index 0000000..cafe219
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-ath79-usb.txt
@@ -0,0 +1,18 @@
+* Atheros AR71XX/9XXX USB PHY
+
+Required properties:
+- compatible: "qca,ar7100-usb-phy"
+- #phys-cells: should be 0
+- reset-names: "usb-phy"[, "usb-suspend-override"]
+- resets: references to the reset controllers
+
+Example:
+
+	usb-phy {
+		compatible = "qca,ar7100-usb-phy";
+
+		reset-names = "usb-phy", "usb-suspend-override";
+		resets = <&rst 4>, <&rst 3>;
+
+		#phy-cells = <0>;
+	};
-- 
2.0.0

[PATCH v2 3/5] phy: Add a driver for the ATH79 USB phy

[Alban Bedel]

The ATH79 USB phy is very simple, it only have a reset. On some SoC a
second reset is used to force the phy in suspend mode regardless of the
USB controller status.

Signed-off-by: Alban Bedel <albeu@free.fr>
---
Changelog:
v2: * Rebased on the simple PHY driver
    * Added myself as maintainer of the driver
---
 MAINTAINERS                 |   8 +++
 drivers/phy/Kconfig         |   8 +++
 drivers/phy/Makefile        |   1 +
 drivers/phy/phy-ath79-usb.c | 116 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 133 insertions(+)
 create mode 100644 drivers/phy/phy-ath79-usb.c

diff --git a/MAINTAINERS b/MAINTAINERS
index e9caa4b..310ff8c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1823,6 +1823,14 @@ S:	Maintained
 F:	drivers/gpio/gpio-ath79.c
 F:	Documentation/devicetree/bindings/gpio/gpio-ath79.txt
 
+ATHEROS 71XX/9XXX USB PHY DRIVER
+M:	Alban Bedel <albeu@free.fr>
+W:	https://github.com/AlbanBedel/linux
+T:	git git://github.com/AlbanBedel/linux
+S:	Maintained
+F:	drivers/phy/phy-ath79-usb.c
+F:	Documentation/devicetree/bindings/phy/phy-ath79-usb.txt
+
 ATHEROS ATH GENERIC UTILITIES
 M:	"Luis R. Rodriguez" <mcgrof@do-not-panic.com>
 L:	linux-wireless@vger.kernel.org
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 028fb16..c8f58ae 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -15,6 +15,14 @@ config GENERIC_PHY
 	  phy users can obtain reference to the PHY. All the users of this
 	  framework should select this config.
 
+config PHY_ATH79_USB
+	tristate "Atheros AR71XX/9XXX USB PHY driver"
+	depends on ATH79 || COMPILE_TEST
+	default y if USB_EHCI_HCD_PLATFORM
+	select PHY_SIMPLE
+	help
+	  Enable this to support the USB PHY on Atheros AR71XX/9XXX SoCs.
+
 config PHY_BERLIN_USB
 	tristate "Marvell Berlin USB PHY Driver"
 	depends on ARCH_BERLIN && RESET_CONTROLLER && HAS_IOMEM && OF
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 1a44362..9ff8550 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -3,6 +3,7 @@
 #
 
 obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
+obj-$(CONFIG_PHY_ATH79_USB)		+= phy-ath79-usb.o
 obj-$(CONFIG_PHY_BERLIN_USB)		+= phy-berlin-usb.o
 obj-$(CONFIG_PHY_BERLIN_SATA)		+= phy-berlin-sata.o
 obj-$(CONFIG_PHY_DM816X_USB)		+= phy-dm816x-usb.o
diff --git a/drivers/phy/phy-ath79-usb.c b/drivers/phy/phy-ath79-usb.c
new file mode 100644
index 0000000..ff49356
--- /dev/null
+++ b/drivers/phy/phy-ath79-usb.c
@@ -0,0 +1,116 @@
+/*
+ * Copyright (C) 2015 Alban Bedel <albeu@free.fr>
+ *
+ * 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/module.h>
+#include <linux/platform_device.h>
+#include <linux/phy/simple.h>
+#include <linux/reset.h>
+
+struct ath79_usb_phy {
+	struct simple_phy sphy;
+	struct reset_control *suspend_override;
+};
+
+static int ath79_usb_phy_power_on(struct phy *phy)
+{
+	struct ath79_usb_phy *priv = container_of(
+		phy_get_drvdata(phy), struct ath79_usb_phy, sphy);
+	int err;
+
+	err = simple_phy_power_on(phy);
+	if (err)
+		return err;
+
+	if (priv->suspend_override) {
+		err = reset_control_assert(priv->suspend_override);
+		if (err) {
+			simple_phy_power_off(phy);
+			return err;
+		}
+	}
+
+	return 0;
+}
+
+static int ath79_usb_phy_power_off(struct phy *phy)
+{
+	struct ath79_usb_phy *priv = container_of(
+		phy_get_drvdata(phy), struct ath79_usb_phy, sphy);
+	int err;
+
+	if (priv->suspend_override) {
+		err = reset_control_deassert(priv->suspend_override);
+		if (err)
+			return err;
+	}
+
+	err = simple_phy_power_off(phy);
+	if (err && priv->suspend_override)
+		reset_control_assert(priv->suspend_override);
+
+	return err;
+}
+
+static const struct phy_ops ath79_usb_phy_ops = {
+	.power_on	= ath79_usb_phy_power_on,
+	.power_off	= ath79_usb_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static const struct simple_phy_desc ath79_usb_phy_desc = {
+	.ops = &ath79_usb_phy_ops,
+	.reset = "usb-phy",
+	.clk = (void *)-ENOENT,
+};
+
+static int ath79_usb_phy_probe(struct platform_device *pdev)
+{
+	struct ath79_usb_phy *priv;
+	struct phy *phy;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->suspend_override = devm_reset_control_get_optional(
+		&pdev->dev, "usb-suspend-override");
+	if (IS_ERR(priv->suspend_override)) {
+		if (PTR_ERR(priv->suspend_override) == -ENOENT)
+			priv->suspend_override = NULL;
+		else
+			return PTR_ERR(priv->suspend_override);
+	}
+
+	phy = devm_simple_phy_create(&pdev->dev,
+				&ath79_usb_phy_desc, &priv->sphy);
+	if (IS_ERR(phy))
+		return PTR_ERR(phy);
+
+	return PTR_ERR_OR_ZERO(devm_of_phy_provider_register(
+				&pdev->dev, of_phy_simple_xlate));
+}
+
+static const struct of_device_id ath79_usb_phy_of_match[] = {
+	{ .compatible = "qca,ar7100-usb-phy" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, ath79_usb_phy_of_match);
+
+static struct platform_driver ath79_usb_phy_driver = {
+	.probe	= ath79_usb_phy_probe,
+	.driver = {
+		.of_match_table	= ath79_usb_phy_of_match,
+		.name		= "ath79-usb-phy",
+	}
+};
+module_platform_driver(ath79_usb_phy_driver);
+
+MODULE_DESCRIPTION("ATH79 USB PHY driver");
+MODULE_AUTHOR("Alban Bedel <albeu@free.fr>");
+MODULE_LICENSE("GPL");
-- 
2.0.0

[PATCH v2 4/5] MIPS: ath79: Add the EHCI controller and USB phy to the AR9132 dtsi

[Alban Bedel]

Signed-off-by: Alban Bedel <albeu@free.fr>
---
Changelog:
v2: * Fix the USB controller node name to follow ePAPR
    * Set the USB phy as disabled like the USB controller
---
 arch/mips/boot/dts/qca/ar9132.dtsi | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/mips/boot/dts/qca/ar9132.dtsi b/arch/mips/boot/dts/qca/ar9132.dtsi
index fb7734e..857ca48 100644
--- a/arch/mips/boot/dts/qca/ar9132.dtsi
+++ b/arch/mips/boot/dts/qca/ar9132.dtsi
@@ -125,6 +125,21 @@
 			};
 		};
 
+		usb@1b000100 {
+			compatible = "qca,ar7100-ehci", "generic-ehci";
+			reg = <0x1b000100 0x100>;
+
+			interrupts = <3>;
+			resets = <&rst 5>;
+
+			has-transaction-translator;
+
+			phy-names = "usb";
+			phys = <&usb_phy>;
+
+			status = "disabled";
+		};
+
 		spi@1f000000 {
 			compatible = "qca,ar9132-spi", "qca,ar7100-spi";
 			reg = <0x1f000000 0x10>;
@@ -138,4 +153,15 @@
 			#size-cells = <0>;
 		};
 	};
+
+	usb_phy: usb-phy {
+		compatible = "qca,ar7100-usb-phy";
+
+		reset-names = "usb-phy", "usb-suspend-override";
+		resets = <&rst 4>, <&rst 3>;
+
+		#phy-cells = <0>;
+
+		status = "disabled";
+	};
 };
-- 
2.0.0

[PATCH v2 5/5] MIPS: ath79: Enable the USB port on the TL-WR1043ND

[Alban Bedel]

Signed-off-by: Alban Bedel <albeu@free.fr>
---
Changelog:
v2: * Fix the USB controller node name to follow ePAPR
    * Enable the USB phy as it is now disabled per default
---
 arch/mips/boot/dts/qca/ar9132_tl_wr1043nd_v1.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/mips/boot/dts/qca/ar9132_tl_wr1043nd_v1.dts b/arch/mips/boot/dts/qca/ar9132_tl_wr1043nd_v1.dts
index 003015a..e535ee3 100644
--- a/arch/mips/boot/dts/qca/ar9132_tl_wr1043nd_v1.dts
+++ b/arch/mips/boot/dts/qca/ar9132_tl_wr1043nd_v1.dts
@@ -35,6 +35,10 @@
 			};
 		};
 
+		usb@1b000100 {
+			status = "okay";
+		};
+
 		spi@1f000000 {
 			status = "okay";
 			num-cs = <1>;
@@ -65,6 +69,10 @@
 		};
 	};
 
+	usb-phy {
+		status = "okay";
+	};
+
 	gpio-keys {
 		compatible = "gpio-keys-polled";
 		#address-cells = <1>;
-- 
2.0.0

Re: [PATCH v2 2/5] devicetree: Add bindings for the ATH79 USB phy

[Rob Herring]

On Mon, Nov 16, 2015 at 10:22:01PM +0100, Alban Bedel wrote:
> Signed-off-by: Alban Bedel <albeu@free.fr>

Acked-by: Rob Herring <robh@kernel.org>

> ---
>  .../devicetree/bindings/phy/phy-ath79-usb.txt          | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/phy-ath79-usb.txt
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy-ath79-usb.txt b/Documentation/devicetree/bindings/phy/phy-ath79-usb.txt
> new file mode 100644
> index 0000000..cafe219
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-ath79-usb.txt
> @@ -0,0 +1,18 @@
> +* Atheros AR71XX/9XXX USB PHY
> +
> +Required properties:
> +- compatible: "qca,ar7100-usb-phy"
> +- #phys-cells: should be 0
> +- reset-names: "usb-phy"[, "usb-suspend-override"]
> +- resets: references to the reset controllers
> +
> +Example:
> +
> +	usb-phy {
> +		compatible = "qca,ar7100-usb-phy";
> +
> +		reset-names = "usb-phy", "usb-suspend-override";
> +		resets = <&rst 4>, <&rst 3>;
> +
> +		#phy-cells = <0>;
> +	};
> -- 
> 2.0.0
> 

Re: [PATCH v2 2/5] devicetree: Add bindings for the ATH79 USB phy

[Rob Herring]

On Mon, Nov 16, 2015 at 10:22:01PM +0100, Alban Bedel wrote:
> Signed-off-by: Alban Bedel <albeu-GANU6spQydw@public.gmane.org>

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

> ---
>  .../devicetree/bindings/phy/phy-ath79-usb.txt          | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/phy-ath79-usb.txt
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy-ath79-usb.txt b/Documentation/devicetree/bindings/phy/phy-ath79-usb.txt
> new file mode 100644
> index 0000000..cafe219
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-ath79-usb.txt
> @@ -0,0 +1,18 @@
> +* Atheros AR71XX/9XXX USB PHY
> +
> +Required properties:
> +- compatible: "qca,ar7100-usb-phy"
> +- #phys-cells: should be 0
> +- reset-names: "usb-phy"[, "usb-suspend-override"]
> +- resets: references to the reset controllers
> +
> +Example:
> +
> +	usb-phy {
> +		compatible = "qca,ar7100-usb-phy";
> +
> +		reset-names = "usb-phy", "usb-suspend-override";
> +		resets = <&rst 4>, <&rst 3>;
> +
> +		#phy-cells = <0>;
> +	};
> -- 
> 2.0.0
> 
--
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

Re: [PATCH v2 1/5] phy: Add a driver for simple phy

[Kishon Vijay Abraham I]

Hi,

On Tuesday 17 November 2015 02:52 AM, Alban Bedel wrote:
> This driver is meant to take care of all trivial phys that don't need
> any special configuration, it just enable a regulator, a clock and
> deassert a reset. A public API is also included to allow re-using the
> code in other drivers.
> 
> Signed-off-by: Alban Bedel <albeu@free.fr>
> ---
>  drivers/phy/Kconfig        |  12 +++
>  drivers/phy/Makefile       |   1 +
>  drivers/phy/phy-simple.c   | 204 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/phy/simple.h |  39 +++++++++
>  4 files changed, 256 insertions(+)
>  create mode 100644 drivers/phy/phy-simple.c
>  create mode 100644 include/linux/phy/simple.h
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 7eb5859d..028fb16 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -118,6 +118,18 @@ config PHY_RCAR_GEN2
>  	help
>  	  Support for USB PHY found on Renesas R-Car generation 2 SoCs.
>  
> +config PHY_SIMPLE
> +	tristate
> +	select GENERIC_PHY
> +	help
> +
> +config PHY_SIMPLE_PDEV

Where will this config be used? Why do we need two configs for a single driver?
> +	tristate "Simple PHY driver"
> +	select PHY_SIMPLE
> +	help
> +	  A PHY driver for simple devices that only need a regulator, clock
> +	  and reset for power up and shutdown.
> +
>  config OMAP_CONTROL_PHY
>  	tristate "OMAP CONTROL PHY Driver"
>  	depends on ARCH_OMAP2PLUS || COMPILE_TEST
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 075db1a..1a44362 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -24,6 +24,7 @@ obj-$(CONFIG_TWL4030_USB)		+= phy-twl4030-usb.o
>  obj-$(CONFIG_PHY_EXYNOS5250_SATA)	+= phy-exynos5250-sata.o
>  obj-$(CONFIG_PHY_HIX5HD2_SATA)		+= phy-hix5hd2-sata.o
>  obj-$(CONFIG_PHY_MT65XX_USB3)		+= phy-mt65xx-usb3.o
> +obj-$(CONFIG_PHY_SIMPLE)		+= phy-simple.o
>  obj-$(CONFIG_PHY_SUN4I_USB)		+= phy-sun4i-usb.o
>  obj-$(CONFIG_PHY_SUN9I_USB)		+= phy-sun9i-usb.o
>  obj-$(CONFIG_PHY_SAMSUNG_USB2)		+= phy-exynos-usb2.o
> diff --git a/drivers/phy/phy-simple.c b/drivers/phy/phy-simple.c
> new file mode 100644
> index 0000000..013f846
> --- /dev/null
> +++ b/drivers/phy/phy-simple.c
> @@ -0,0 +1,204 @@
> +/*
> + * Copyright (C) 2015 Alban Bedel <albeu@free.fr>
> + *
> + * 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/module.h>
> +#include <linux/of_device.h>
> +#include <linux/phy/simple.h>
> +#include <linux/clk.h>
> +#include <linux/reset.h>
> +
> +int simple_phy_power_on(struct phy *phy)
> +{
> +	struct simple_phy *sphy = phy_get_drvdata(phy);
> +	int err;
> +
> +	if (sphy->regulator) {
> +		err = regulator_enable(sphy->regulator);
> +		if (err)
> +			return err;
> +	}
> +
> +	if (sphy->clk) {
> +		err = clk_prepare_enable(sphy->clk);
> +		if (err)
> +			goto regulator_disable;
> +	}
> +
> +	if (sphy->reset) {
> +		err = reset_control_deassert(sphy->reset);
> +		if (err)
> +			goto clock_disable;
> +	}
> +
> +	return 0;
> +
> +clock_disable:
> +	if (sphy->clk)
> +		clk_disable_unprepare(sphy->clk);
> +regulator_disable:
> +	if (sphy->regulator)
> +		WARN_ON(regulator_disable(sphy->regulator));
> +
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(simple_phy_power_on);

IMO simple-phy driver should be an independent driver and shouldn't export
symbols. The dt binding for the simple phy device should be something like
below where all the properties of the simple phy device should be in the
binding documentation.
usbphy {
	compatible = "simple-phy";
	phy-supply = <&supply>;
	clocks = <&clock>;
	reset = <&reset>;
};

Anything that needs more than this shouldn't be a simple phy.

Thanks
Kishon

Re: [PATCH v2 1/5] phy: Add a driver for simple phy

[Kishon Vijay Abraham I]

Hi,

On Tuesday 17 November 2015 02:52 AM, Alban Bedel wrote:
> This driver is meant to take care of all trivial phys that don't need
> any special configuration, it just enable a regulator, a clock and
> deassert a reset. A public API is also included to allow re-using the
> code in other drivers.
> 
> Signed-off-by: Alban Bedel <albeu-GANU6spQydw@public.gmane.org>
> ---
>  drivers/phy/Kconfig        |  12 +++
>  drivers/phy/Makefile       |   1 +
>  drivers/phy/phy-simple.c   | 204 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/phy/simple.h |  39 +++++++++
>  4 files changed, 256 insertions(+)
>  create mode 100644 drivers/phy/phy-simple.c
>  create mode 100644 include/linux/phy/simple.h
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 7eb5859d..028fb16 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -118,6 +118,18 @@ config PHY_RCAR_GEN2
>  	help
>  	  Support for USB PHY found on Renesas R-Car generation 2 SoCs.
>  
> +config PHY_SIMPLE
> +	tristate
> +	select GENERIC_PHY
> +	help
> +
> +config PHY_SIMPLE_PDEV

Where will this config be used? Why do we need two configs for a single driver?
> +	tristate "Simple PHY driver"
> +	select PHY_SIMPLE
> +	help
> +	  A PHY driver for simple devices that only need a regulator, clock
> +	  and reset for power up and shutdown.
> +
>  config OMAP_CONTROL_PHY
>  	tristate "OMAP CONTROL PHY Driver"
>  	depends on ARCH_OMAP2PLUS || COMPILE_TEST
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 075db1a..1a44362 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -24,6 +24,7 @@ obj-$(CONFIG_TWL4030_USB)		+= phy-twl4030-usb.o
>  obj-$(CONFIG_PHY_EXYNOS5250_SATA)	+= phy-exynos5250-sata.o
>  obj-$(CONFIG_PHY_HIX5HD2_SATA)		+= phy-hix5hd2-sata.o
>  obj-$(CONFIG_PHY_MT65XX_USB3)		+= phy-mt65xx-usb3.o
> +obj-$(CONFIG_PHY_SIMPLE)		+= phy-simple.o
>  obj-$(CONFIG_PHY_SUN4I_USB)		+= phy-sun4i-usb.o
>  obj-$(CONFIG_PHY_SUN9I_USB)		+= phy-sun9i-usb.o
>  obj-$(CONFIG_PHY_SAMSUNG_USB2)		+= phy-exynos-usb2.o
> diff --git a/drivers/phy/phy-simple.c b/drivers/phy/phy-simple.c
> new file mode 100644
> index 0000000..013f846
> --- /dev/null
> +++ b/drivers/phy/phy-simple.c
> @@ -0,0 +1,204 @@
> +/*
> + * Copyright (C) 2015 Alban Bedel <albeu-GANU6spQydw@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/module.h>
> +#include <linux/of_device.h>
> +#include <linux/phy/simple.h>
> +#include <linux/clk.h>
> +#include <linux/reset.h>
> +
> +int simple_phy_power_on(struct phy *phy)
> +{
> +	struct simple_phy *sphy = phy_get_drvdata(phy);
> +	int err;
> +
> +	if (sphy->regulator) {
> +		err = regulator_enable(sphy->regulator);
> +		if (err)
> +			return err;
> +	}
> +
> +	if (sphy->clk) {
> +		err = clk_prepare_enable(sphy->clk);
> +		if (err)
> +			goto regulator_disable;
> +	}
> +
> +	if (sphy->reset) {
> +		err = reset_control_deassert(sphy->reset);
> +		if (err)
> +			goto clock_disable;
> +	}
> +
> +	return 0;
> +
> +clock_disable:
> +	if (sphy->clk)
> +		clk_disable_unprepare(sphy->clk);
> +regulator_disable:
> +	if (sphy->regulator)
> +		WARN_ON(regulator_disable(sphy->regulator));
> +
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(simple_phy_power_on);

IMO simple-phy driver should be an independent driver and shouldn't export
symbols. The dt binding for the simple phy device should be something like
below where all the properties of the simple phy device should be in the
binding documentation.
usbphy {
	compatible = "simple-phy";
	phy-supply = <&supply>;
	clocks = <&clock>;
	reset = <&reset>;
};

Anything that needs more than this shouldn't be a simple phy.

Thanks
Kishon
--
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

Re: [PATCH v2 1/5] phy: Add a driver for simple phy

[Kishon Vijay Abraham I]

Hi,

On Tuesday 17 November 2015 02:52 AM, Alban Bedel wrote:
> This driver is meant to take care of all trivial phys that don't need
> any special configuration, it just enable a regulator, a clock and
> deassert a reset. A public API is also included to allow re-using the
> code in other drivers.
> 
> Signed-off-by: Alban Bedel <albeu@free.fr>
> ---
>  drivers/phy/Kconfig        |  12 +++
>  drivers/phy/Makefile       |   1 +
>  drivers/phy/phy-simple.c   | 204 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/phy/simple.h |  39 +++++++++
>  4 files changed, 256 insertions(+)
>  create mode 100644 drivers/phy/phy-simple.c
>  create mode 100644 include/linux/phy/simple.h
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 7eb5859d..028fb16 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -118,6 +118,18 @@ config PHY_RCAR_GEN2
>  	help
>  	  Support for USB PHY found on Renesas R-Car generation 2 SoCs.
>  
> +config PHY_SIMPLE
> +	tristate
> +	select GENERIC_PHY
> +	help
> +
> +config PHY_SIMPLE_PDEV

Where will this config be used? Why do we need two configs for a single driver?
> +	tristate "Simple PHY driver"
> +	select PHY_SIMPLE
> +	help
> +	  A PHY driver for simple devices that only need a regulator, clock
> +	  and reset for power up and shutdown.
> +
>  config OMAP_CONTROL_PHY
>  	tristate "OMAP CONTROL PHY Driver"
>  	depends on ARCH_OMAP2PLUS || COMPILE_TEST
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 075db1a..1a44362 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -24,6 +24,7 @@ obj-$(CONFIG_TWL4030_USB)		+= phy-twl4030-usb.o
>  obj-$(CONFIG_PHY_EXYNOS5250_SATA)	+= phy-exynos5250-sata.o
>  obj-$(CONFIG_PHY_HIX5HD2_SATA)		+= phy-hix5hd2-sata.o
>  obj-$(CONFIG_PHY_MT65XX_USB3)		+= phy-mt65xx-usb3.o
> +obj-$(CONFIG_PHY_SIMPLE)		+= phy-simple.o
>  obj-$(CONFIG_PHY_SUN4I_USB)		+= phy-sun4i-usb.o
>  obj-$(CONFIG_PHY_SUN9I_USB)		+= phy-sun9i-usb.o
>  obj-$(CONFIG_PHY_SAMSUNG_USB2)		+= phy-exynos-usb2.o
> diff --git a/drivers/phy/phy-simple.c b/drivers/phy/phy-simple.c
> new file mode 100644
> index 0000000..013f846
> --- /dev/null
> +++ b/drivers/phy/phy-simple.c
> @@ -0,0 +1,204 @@
> +/*
> + * Copyright (C) 2015 Alban Bedel <albeu@free.fr>
> + *
> + * 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/module.h>
> +#include <linux/of_device.h>
> +#include <linux/phy/simple.h>
> +#include <linux/clk.h>
> +#include <linux/reset.h>
> +
> +int simple_phy_power_on(struct phy *phy)
> +{
> +	struct simple_phy *sphy = phy_get_drvdata(phy);
> +	int err;
> +
> +	if (sphy->regulator) {
> +		err = regulator_enable(sphy->regulator);
> +		if (err)
> +			return err;
> +	}
> +
> +	if (sphy->clk) {
> +		err = clk_prepare_enable(sphy->clk);
> +		if (err)
> +			goto regulator_disable;
> +	}
> +
> +	if (sphy->reset) {
> +		err = reset_control_deassert(sphy->reset);
> +		if (err)
> +			goto clock_disable;
> +	}
> +
> +	return 0;
> +
> +clock_disable:
> +	if (sphy->clk)
> +		clk_disable_unprepare(sphy->clk);
> +regulator_disable:
> +	if (sphy->regulator)
> +		WARN_ON(regulator_disable(sphy->regulator));
> +
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(simple_phy_power_on);

IMO simple-phy driver should be an independent driver and shouldn't export
symbols. The dt binding for the simple phy device should be something like
below where all the properties of the simple phy device should be in the
binding documentation.
usbphy {
	compatible = "simple-phy";
	phy-supply = <&supply>;
	clocks = <&clock>;
	reset = <&reset>;
};

Anything that needs more than this shouldn't be a simple phy.

Thanks
Kishon

Re: [PATCH v2 1/5] phy: Add a driver for simple phy

[Arnd Bergmann]

On Thursday 14 April 2016 11:22:58 Kishon Vijay Abraham I wrote:
> 
> IMO simple-phy driver should be an independent driver and shouldn't export
> symbols. The dt binding for the simple phy device should be something like
> below where all the properties of the simple phy device should be in the
> binding documentation.
> usbphy {
>         compatible = "simple-phy";
>         phy-supply = <&supply>;
>         clocks = <&clock>;
>         reset = <&reset>;
> };
> 
> Anything that needs more than this shouldn't be a simple phy.

I think there are two aspects here:

a) I agree that a driver that matches "simple-phy" should only call
   the generic functions and not use any other properties.

b) Independent of that, I think that it makes a lot of sense to export
   those functions from the generic PHY subsystems so they can be
   called from drivers that are a little less generic, or that already
   have an established binding but need no other code.

	Arnd

Re: [PATCH v2 1/5] phy: Add a driver for simple phy

[Arnd Bergmann]

On Thursday 14 April 2016 11:22:58 Kishon Vijay Abraham I wrote:
> 
> IMO simple-phy driver should be an independent driver and shouldn't export
> symbols. The dt binding for the simple phy device should be something like
> below where all the properties of the simple phy device should be in the
> binding documentation.
> usbphy {
>         compatible = "simple-phy";
>         phy-supply = <&supply>;
>         clocks = <&clock>;
>         reset = <&reset>;
> };
> 
> Anything that needs more than this shouldn't be a simple phy.

I think there are two aspects here:

a) I agree that a driver that matches "simple-phy" should only call
   the generic functions and not use any other properties.

b) Independent of that, I think that it makes a lot of sense to export
   those functions from the generic PHY subsystems so they can be
   called from drivers that are a little less generic, or that already
   have an established binding but need no other code.

	Arnd
--
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

Re: [PATCH v2 1/5] phy: Add a driver for simple phy

[Kishon Vijay Abraham I]

Hi Arnd,

On Sunday 17 April 2016 01:20 AM, Arnd Bergmann wrote:
> On Thursday 14 April 2016 11:22:58 Kishon Vijay Abraham I wrote:
>>
>> IMO simple-phy driver should be an independent driver and shouldn't export
>> symbols. The dt binding for the simple phy device should be something like
>> below where all the properties of the simple phy device should be in the
>> binding documentation.
>> usbphy {
>>         compatible = "simple-phy";
>>         phy-supply = <&supply>;
>>         clocks = <&clock>;
>>         reset = <&reset>;
>> };
>>
>> Anything that needs more than this shouldn't be a simple phy.
> 
> I think there are two aspects here:
> 
> a) I agree that a driver that matches "simple-phy" should only call
>    the generic functions and not use any other properties.
> 
> b) Independent of that, I think that it makes a lot of sense to export
>    those functions from the generic PHY subsystems so they can be
>    called from drivers that are a little less generic, or that already
>    have an established binding but need no other code.

These export functions can be abused and called directly from the controller
driver bypassing the phy core.

Actually lot of generic PHY programming are done in the phy-core itself. (For
example, the generic PHY regulator binding "phy-supply" can be used for the phy
core to enable the regulator during power on and disable during power off, phy
core also invokes pm_runtime API's during power_on and power_off which can be
used to enable/disable clocks). So drivers which are less generic can just
populate their specific handling part in their phy ops and leave the rest to be
done in phy core.
"simple-phy" should be used to avoid adding new PHY drivers that does simple
PHY ops.

Thanks
Kishon

Re: [PATCH v2 1/5] phy: Add a driver for simple phy

[Kishon Vijay Abraham I]

Hi Arnd,

On Sunday 17 April 2016 01:20 AM, Arnd Bergmann wrote:
> On Thursday 14 April 2016 11:22:58 Kishon Vijay Abraham I wrote:
>>
>> IMO simple-phy driver should be an independent driver and shouldn't export
>> symbols. The dt binding for the simple phy device should be something like
>> below where all the properties of the simple phy device should be in the
>> binding documentation.
>> usbphy {
>>         compatible = "simple-phy";
>>         phy-supply = <&supply>;
>>         clocks = <&clock>;
>>         reset = <&reset>;
>> };
>>
>> Anything that needs more than this shouldn't be a simple phy.
> 
> I think there are two aspects here:
> 
> a) I agree that a driver that matches "simple-phy" should only call
>    the generic functions and not use any other properties.
> 
> b) Independent of that, I think that it makes a lot of sense to export
>    those functions from the generic PHY subsystems so they can be
>    called from drivers that are a little less generic, or that already
>    have an established binding but need no other code.

These export functions can be abused and called directly from the controller
driver bypassing the phy core.

Actually lot of generic PHY programming are done in the phy-core itself. (For
example, the generic PHY regulator binding "phy-supply" can be used for the phy
core to enable the regulator during power on and disable during power off, phy
core also invokes pm_runtime API's during power_on and power_off which can be
used to enable/disable clocks). So drivers which are less generic can just
populate their specific handling part in their phy ops and leave the rest to be
done in phy core.
"simple-phy" should be used to avoid adding new PHY drivers that does simple
PHY ops.

Thanks
Kishon

Re: [PATCH v2 1/5] phy: Add a driver for simple phy

[Arnd Bergmann]

On Monday 18 April 2016 18:00:19 Kishon Vijay Abraham I wrote:
> Hi Arnd,
> 
> On Sunday 17 April 2016 01:20 AM, Arnd Bergmann wrote:
> > On Thursday 14 April 2016 11:22:58 Kishon Vijay Abraham I wrote:
> >>
> >> IMO simple-phy driver should be an independent driver and shouldn't export
> >> symbols. The dt binding for the simple phy device should be something like
> >> below where all the properties of the simple phy device should be in the
> >> binding documentation.
> >> usbphy {
> >>         compatible = "simple-phy";
> >>         phy-supply = <&supply>;
> >>         clocks = <&clock>;
> >>         reset = <&reset>;
> >> };
> >>
> >> Anything that needs more than this shouldn't be a simple phy.
> > 
> > I think there are two aspects here:
> > 
> > a) I agree that a driver that matches "simple-phy" should only call
> >    the generic functions and not use any other properties.
> > 
> > b) Independent of that, I think that it makes a lot of sense to export
> >    those functions from the generic PHY subsystems so they can be
> >    called from drivers that are a little less generic, or that already
> >    have an established binding but need no other code.
> 
> These export functions can be abused and called directly from the controller
> driver bypassing the phy core.
> 
> Actually lot of generic PHY programming are done in the phy-core itself. (For
> example, the generic PHY regulator binding "phy-supply" can be used for the phy
> core to enable the regulator during power on and disable during power off, phy
> core also invokes pm_runtime API's during power_on and power_off which can be
> used to enable/disable clocks). So drivers which are less generic can just
> populate their specific handling part in their phy ops and leave the rest to be
> done in phy core.
> "simple-phy" should be used to avoid adding new PHY drivers that does simple
> PHY ops.

Having the phy core do everything automatically indeed sounds superior here,
the simple-phy driver can then become a trivial stub without any calls
into the regulator/clk/reset subsystems. If that works (or can be made to
work), that's great.

	Arnd

Re: [PATCH v2 1/5] phy: Add a driver for simple phy

[Arnd Bergmann]

On Monday 18 April 2016 18:00:19 Kishon Vijay Abraham I wrote:
> Hi Arnd,
> 
> On Sunday 17 April 2016 01:20 AM, Arnd Bergmann wrote:
> > On Thursday 14 April 2016 11:22:58 Kishon Vijay Abraham I wrote:
> >>
> >> IMO simple-phy driver should be an independent driver and shouldn't export
> >> symbols. The dt binding for the simple phy device should be something like
> >> below where all the properties of the simple phy device should be in the
> >> binding documentation.
> >> usbphy {
> >>         compatible = "simple-phy";
> >>         phy-supply = <&supply>;
> >>         clocks = <&clock>;
> >>         reset = <&reset>;
> >> };
> >>
> >> Anything that needs more than this shouldn't be a simple phy.
> > 
> > I think there are two aspects here:
> > 
> > a) I agree that a driver that matches "simple-phy" should only call
> >    the generic functions and not use any other properties.
> > 
> > b) Independent of that, I think that it makes a lot of sense to export
> >    those functions from the generic PHY subsystems so they can be
> >    called from drivers that are a little less generic, or that already
> >    have an established binding but need no other code.
> 
> These export functions can be abused and called directly from the controller
> driver bypassing the phy core.
> 
> Actually lot of generic PHY programming are done in the phy-core itself. (For
> example, the generic PHY regulator binding "phy-supply" can be used for the phy
> core to enable the regulator during power on and disable during power off, phy
> core also invokes pm_runtime API's during power_on and power_off which can be
> used to enable/disable clocks). So drivers which are less generic can just
> populate their specific handling part in their phy ops and leave the rest to be
> done in phy core.
> "simple-phy" should be used to avoid adding new PHY drivers that does simple
> PHY ops.

Having the phy core do everything automatically indeed sounds superior here,
the simple-phy driver can then become a trivial stub without any calls
into the regulator/clk/reset subsystems. If that works (or can be made to
work), that's great.

	Arnd
--
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