linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/2] phy: Add a driver for simple phy
@ 2016-01-28 19:52 Alban Bedel
  2016-01-28 19:52 ` [PATCH v3 2/2] phy: Add a driver for the ATH79 USB phy Alban Bedel
  2016-02-01 11:42 ` [PATCH v3 1/2] phy: Add a driver for simple phy Kishon Vijay Abraham I
  0 siblings, 2 replies; 9+ messages in thread
From: Alban Bedel @ 2016-01-28 19:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Kishon Vijay Abraham I, Andrew Morton, Greg Kroah-Hartman,
	David S. Miller, Mauro Carvalho Chehab, Joe Perches, Jiri Slaby,
	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 e7e117d..efbaee4 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -125,6 +125,18 @@ config PHY_RCAR_GEN3_USB2
 	help
 	  Support for USB 2.0 PHY found on Renesas R-Car generation 3 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 c80f09d..1cc7268 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_PHY_EXYNOS5250_SATA)	+= phy-exynos5250-sata.o
 obj-$(CONFIG_PHY_HIX5HD2_SATA)		+= phy-hix5hd2-sata.o
 obj-$(CONFIG_PHY_HI6220_USB)		+= phy-hi6220-usb.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

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

* [PATCH v3 2/2] phy: Add a driver for the ATH79 USB phy
  2016-01-28 19:52 [PATCH v3 1/2] phy: Add a driver for simple phy Alban Bedel
@ 2016-01-28 19:52 ` Alban Bedel
  2016-02-02  6:11   ` Kishon Vijay Abraham I
  2016-02-01 11:42 ` [PATCH v3 1/2] phy: Add a driver for simple phy Kishon Vijay Abraham I
  1 sibling, 1 reply; 9+ messages in thread
From: Alban Bedel @ 2016-01-28 19:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Kishon Vijay Abraham I, Andrew Morton, Greg Kroah-Hartman,
	David S. Miller, Mauro Carvalho Chehab, Joe Perches, Jiri Slaby,
	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 30aca4a..f536d52 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1866,6 +1866,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 efbaee4..6090c63 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 1cc7268..5c9ca5f 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

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

* Re: [PATCH v3 1/2] phy: Add a driver for simple phy
  2016-01-28 19:52 [PATCH v3 1/2] phy: Add a driver for simple phy Alban Bedel
  2016-01-28 19:52 ` [PATCH v3 2/2] phy: Add a driver for the ATH79 USB phy Alban Bedel
@ 2016-02-01 11:42 ` Kishon Vijay Abraham I
  2016-02-17 14:21   ` Alban
  1 sibling, 1 reply; 9+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-01 11:42 UTC (permalink / raw)
  To: Alban Bedel, linux-kernel
  Cc: Andrew Morton, Greg Kroah-Hartman, David S. Miller,
	Mauro Carvalho Chehab, Joe Perches, Jiri Slaby

Hi,

On Friday 29 January 2016 01:22 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>
> ---

please also include what changed from the previous versions here or in the
cover letter.
>  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 e7e117d..efbaee4 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -125,6 +125,18 @@ config PHY_RCAR_GEN3_USB2
>  	help
>  	  Support for USB 2.0 PHY found on Renesas R-Car generation 3 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 c80f09d..1cc7268 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_PHY_EXYNOS5250_SATA)	+= phy-exynos5250-sata.o
>  obj-$(CONFIG_PHY_HIX5HD2_SATA)		+= phy-hix5hd2-sata.o
>  obj-$(CONFIG_PHY_HI6220_USB)		+= phy-hi6220-usb.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>

2016?
> + *
> + * 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;
> +	}

phy_power_on already enables the regulator, so this might be redundant. A
simple PHY can have multiple regulators?

Also this doesn't have reference count. I think we should try to reuse the
existing phy_* APIs for simple PHY.
> +
> +	if (sphy->clk) {
> +		err = clk_prepare_enable(sphy->clk);
> +		if (err)
> +			goto regulator_disable;
> +	}

how do we enable multiple clocks?
> +
> +	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 },

the compatible string should be documented.

Thanks
Kishon

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

* Re: [PATCH v3 2/2] phy: Add a driver for the ATH79 USB phy
  2016-01-28 19:52 ` [PATCH v3 2/2] phy: Add a driver for the ATH79 USB phy Alban Bedel
@ 2016-02-02  6:11   ` Kishon Vijay Abraham I
  2016-02-17 14:34     ` Alban
  0 siblings, 1 reply; 9+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-02  6:11 UTC (permalink / raw)
  To: Alban Bedel, linux-kernel
  Cc: Andrew Morton, Greg Kroah-Hartman, David S. Miller,
	Mauro Carvalho Chehab, Joe Perches, Jiri Slaby

Hi,

On Friday 29 January 2016 01:22 AM, Alban Bedel wrote:
> 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 30aca4a..f536d52 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1866,6 +1866,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 efbaee4..6090c63 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 1cc7268..5c9ca5f 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>

2016?
> + *
> + * 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);

Why do you need a separate driver for ath79_usb? We should be able to directly
use simple phy driver right?

Thanks
Kishon

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

* Re: [PATCH v3 1/2] phy: Add a driver for simple phy
  2016-02-01 11:42 ` [PATCH v3 1/2] phy: Add a driver for simple phy Kishon Vijay Abraham I
@ 2016-02-17 14:21   ` Alban
  2016-10-07  8:27     ` Vivek Gautam
  0 siblings, 1 reply; 9+ messages in thread
From: Alban @ 2016-02-17 14:21 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Aban Bedel, linux-kernel, Andrew Morton, Greg Kroah-Hartman,
	David S. Miller, Mauro Carvalho Chehab, Joe Perches, Jiri Slaby

On Mon, 1 Feb 2016 17:12:42 +0530
Kishon Vijay Abraham I <kishon@ti.com> wrote:

> Hi,
> 
> On Friday 29 January 2016 01:22 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>
> > ---
> 
> please also include what changed from the previous versions here or in the
> cover letter.
> >  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 e7e117d..efbaee4 100644
> > --- a/drivers/phy/Kconfig
> > +++ b/drivers/phy/Kconfig
> > @@ -125,6 +125,18 @@ config PHY_RCAR_GEN3_USB2
> >  	help
> >  	  Support for USB 2.0 PHY found on Renesas R-Car generation 3 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 c80f09d..1cc7268 100644
> > --- a/drivers/phy/Makefile
> > +++ b/drivers/phy/Makefile
> > @@ -26,6 +26,7 @@ obj-$(CONFIG_PHY_EXYNOS5250_SATA)	+= phy-exynos5250-sata.o
> >  obj-$(CONFIG_PHY_HIX5HD2_SATA)		+= phy-hix5hd2-sata.o
> >  obj-$(CONFIG_PHY_HI6220_USB)		+= phy-hi6220-usb.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>
> 
> 2016?

I'll add 2016.

> > + *
> > + * 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;
> > +	}
> 
> phy_power_on already enables the regulator, so this might be redundant. A
> simple PHY can have multiple regulators?

You are right, this can be dropped.
 
> Also this doesn't have reference count. I think we should try to reuse the
> existing phy_* APIs for simple PHY.

I don't really get what you mean here. The phy core already count the
enable/disable and only call the callbacks implemented here for the
first enable and last disable. What should be done differently?

> > +
> > +	if (sphy->clk) {
> > +		err = clk_prepare_enable(sphy->clk);
> > +		if (err)
> > +			goto regulator_disable;
> > +	}
> 
> how do we enable multiple clocks?

Currently you can't. However I don't think it would make much sense
because if there is multiple clocks it start to be non-trivial. In such
case you will often get dependencies between the clocks and/or power
supplies. At this point you might as well write a dedicated driver.

> > +
> > +	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 },
> 
> the compatible string should be documented.

I'll add a patch with the binding to the next re-roll.

Alban

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

* Re: [PATCH v3 2/2] phy: Add a driver for the ATH79 USB phy
  2016-02-02  6:11   ` Kishon Vijay Abraham I
@ 2016-02-17 14:34     ` Alban
  0 siblings, 0 replies; 9+ messages in thread
From: Alban @ 2016-02-17 14:34 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Aban Bedel, linux-kernel, Andrew Morton, Greg Kroah-Hartman,
	David S. Miller, Mauro Carvalho Chehab, Joe Perches, Jiri Slaby

On Tue, 2 Feb 2016 11:41:15 +0530
Kishon Vijay Abraham I <kishon@ti.com> wrote:

> Hi,
> 
> On Friday 29 January 2016 01:22 AM, Alban Bedel wrote:
> > 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 30aca4a..f536d52 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1866,6 +1866,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 efbaee4..6090c63 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 1cc7268..5c9ca5f 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>
> 
> 2016?

I'll fix this one too.

> > + *
> > + * 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);
> 
> Why do you need a separate driver for ath79_usb? We should be able to directly
> use simple phy driver right?

The ath79 PHY controller misuse the reset controller to control a
suspend override. When this reset line is deasserted the PHY is forced
in suspend mode, for normal operations it must be asserted. In short
this extra reset is inverted compared to the normal use of a reset line.

To support that that in the simple driver we would first need to find a
way to support multiple resets. Not impossible, but not trivial either
as we would need to at least get a name list from somewhere. Then we
would need another scheme to indicate that this extra reset need to be
inverted, again possible but it would start be a bit hacky. All in all
that didn't sound like a good idea for something that is supposed to be
a "simple" driver, so a dedicated driver sounded like the best approach
for this case.

Alban

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

* Re: [PATCH v3 1/2] phy: Add a driver for simple phy
  2016-02-17 14:21   ` Alban
@ 2016-10-07  8:27     ` Vivek Gautam
  2016-10-12 22:17       ` Alban
  0 siblings, 1 reply; 9+ messages in thread
From: Vivek Gautam @ 2016-10-07  8:27 UTC (permalink / raw)
  To: Alban
  Cc: Kishon Vijay Abraham I, linux-kernel@vger.kernel.org,
	Andrew Morton, Greg Kroah-Hartman, David S. Miller,
	Mauro Carvalho Chehab, Joe Perches, Jiri Slaby

Hi Alban,


On Wed, Feb 17, 2016 at 7:51 PM, Alban <albeu@free.fr> wrote:
> On Mon, 1 Feb 2016 17:12:42 +0530
> Kishon Vijay Abraham I <kishon@ti.com> wrote:
>
>> Hi,
>>
>> On Friday 29 January 2016 01:22 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>

Do you have any plans to re-spin this patch ?


Regards
Vivek

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* Re: [PATCH v3 1/2] phy: Add a driver for simple phy
  2016-10-07  8:27     ` Vivek Gautam
@ 2016-10-12 22:17       ` Alban
  2016-10-13  8:26         ` Vivek Gautam
  0 siblings, 1 reply; 9+ messages in thread
From: Alban @ 2016-10-12 22:17 UTC (permalink / raw)
  To: Vivek Gautam
  Cc: Aban Bedel, Kishon Vijay Abraham I, linux-kernel@vger.kernel.org,
	Andrew Morton, Greg Kroah-Hartman, David S. Miller,
	Mauro Carvalho Chehab, Joe Perches, Jiri Slaby

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

On Fri, 7 Oct 2016 13:57:08 +0530
Vivek Gautam <vivek.gautam@codeaurora.org> wrote:

> Hi Alban,
> 
> 
> On Wed, Feb 17, 2016 at 7:51 PM, Alban <albeu@free.fr> wrote:
> > On Mon, 1 Feb 2016 17:12:42 +0530
> > Kishon Vijay Abraham I <kishon@ti.com> wrote:
> >
> >> Hi,
> >>
> >> On Friday 29 January 2016 01:22 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>
> 
> Do you have any plans to re-spin this patch ?

Not right now as I have too much other things going on.

Alban

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

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

* Re: [PATCH v3 1/2] phy: Add a driver for simple phy
  2016-10-12 22:17       ` Alban
@ 2016-10-13  8:26         ` Vivek Gautam
  0 siblings, 0 replies; 9+ messages in thread
From: Vivek Gautam @ 2016-10-13  8:26 UTC (permalink / raw)
  To: Alban
  Cc: Kishon Vijay Abraham I, linux-kernel@vger.kernel.org,
	Andrew Morton, Greg Kroah-Hartman, David S. Miller,
	Mauro Carvalho Chehab, Joe Perches, Jiri Slaby



On 10/13/2016 03:47 AM, Alban wrote:
> On Fri, 7 Oct 2016 13:57:08 +0530
> Vivek Gautam <vivek.gautam@codeaurora.org> wrote:
>
>> Hi Alban,
>>
>>
>> On Wed, Feb 17, 2016 at 7:51 PM, Alban <albeu@free.fr> wrote:
>>> On Mon, 1 Feb 2016 17:12:42 +0530
>>> Kishon Vijay Abraham I <kishon@ti.com> wrote:
>>>
>>>> Hi,
>>>>
>>>> On Friday 29 January 2016 01:22 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>
>> Do you have any plans to re-spin this patch ?
> Not right now as I have too much other things going on.

Alright, no problem. Thanks.

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

end of thread, other threads:[~2016-10-13  8:51 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-28 19:52 [PATCH v3 1/2] phy: Add a driver for simple phy Alban Bedel
2016-01-28 19:52 ` [PATCH v3 2/2] phy: Add a driver for the ATH79 USB phy Alban Bedel
2016-02-02  6:11   ` Kishon Vijay Abraham I
2016-02-17 14:34     ` Alban
2016-02-01 11:42 ` [PATCH v3 1/2] phy: Add a driver for simple phy Kishon Vijay Abraham I
2016-02-17 14:21   ` Alban
2016-10-07  8:27     ` Vivek Gautam
2016-10-12 22:17       ` Alban
2016-10-13  8:26         ` Vivek Gautam

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).