Message-Id: <20220421162158.31364-1-jmaselbas@kalray.eu>
Date:   Thu, 21 Apr 2022 18:21:58 +0200
From:   Jules Maselbas <jmaselbas@...ray.eu>
To:     linux-usb@...r.kernel.org
Cc:     linux-phy@...ts.infradead.org, Vinod Koul <vkoul@...nel.org>,
        Mark Brown <broonie@...nel.org>, linux-kernel@...r.kernel.org,
        Felipe Balbi <balbi@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Arnd Bergmann <arnd@...db.de>,
        Jules Maselbas <jmaselbas@...ray.eu>
Subject: [RFC PATCH] phy: Convert usb-nop-xceiv to the generic phy subsystem

I recently encountered some limitations when using the usb-nop-xceiv
driver using the usb-phy subsystem. The situation is: I have a board
with two external usb phys (ULPI) that only needs to be reset by the
USB controller. However both phys shares the same reset line, which
will get toggled twice by the USB driver, once by controller.

This is what I observed with the current Linux usb-nop-xceiv driver:
          _______                            ___      _________
   RESET:        \__________________________/   \____/
                1^                         2^  3^   4^
 1. and 2. first controller toggle the reset
 3. and 4. second controller toggle the reset

After the second toggle the first driver didn't expected the reset and
causing issues, but I don't recall what exactly, it is maybe a driver
that simply fail to get probed.

The following is what I observed when using the "usb-nop-xceiv"
driver in Barebox, which is a bit different than the Linux one.

           _______                            _________________
    RESET:        \__________________________/
                 1^                         2^

In this case the reset is only toggled once, this is because the phy
driver count the number of time it has called init and power_on, which
is exactly what is done by the generic phy subsystem in Linux, but not
by the "legacy" usb-nop-xceiv usb-phy driver.

This difference in behavior is what motivated to "port" the existing
usb-nop-xceiv driver to the generic phy subsystem. However this is far
from being complete... I've only tested the reset function for this nop
phy, and it lack a lot of features from the original driver to be a
valid replacement for it.

I would like to know if there is an interest in converting the current
usb-nop-xceiv driver to the generic phy subsystem?

I would like to keep the usb-nop-xceiv compatible as it used by Barebox
and it works OK. If a new compatible is added then device-tree nodes
shouldn't have both, the new and usb-nop-xceiv, compatible because of
the specific check in phy-core.c which will always return ENODEV.

Signed-off-by: Jules Maselbas <jmaselbas@...ray.eu>
---
 drivers/phy/Makefile          |   1 +
 drivers/phy/phy-core.c        |   4 -
 drivers/phy/phy-usb-generic.c | 241 ++++++++++++++++++++++++++++++++++
 3 files changed, 242 insertions(+), 4 deletions(-)
 create mode 100644 drivers/phy/phy-usb-generic.c

diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 6eb2916773c5..8eeecf6d6f74 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -4,6 +4,7 @@
 #
 
 obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
+obj-$(CONFIG_GENERIC_PHY)		+= phy-usb-generic.o
 obj-$(CONFIG_GENERIC_PHY_MIPI_DPHY)	+= phy-core-mipi-dphy.o
 obj-$(CONFIG_PHY_LPC18XX_USB_OTG)	+= phy-lpc18xx-usb-otg.o
 obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 71cb10826326..e09fd1cb4455 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -506,10 +506,6 @@ static struct phy *_of_phy_get(struct device_node *np, int index)
 	if (ret)
 		return ERR_PTR(-ENODEV);
 
-	/* This phy type handled by the usb-phy subsystem for now */
-	if (of_device_is_compatible(args.np, "usb-nop-xceiv"))
-		return ERR_PTR(-ENODEV);
-
 	mutex_lock(&phy_provider_mutex);
 	phy_provider = of_phy_provider_lookup(args.np);
 	if (IS_ERR(phy_provider) || !try_module_get(phy_provider->owner)) {
diff --git a/drivers/phy/phy-usb-generic.c b/drivers/phy/phy-usb-generic.c
new file mode 100644
index 000000000000..fdd05bbd98f2
--- /dev/null
+++ b/drivers/phy/phy-usb-generic.c
@@ -0,0 +1,241 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * derived from driver/usb/phy/phy-generic.c
+ *
+ * Generic USB PHY driver for all USB "nop" transceiver which are mostly
+ * autonomous. This driver is derived from the usb-nop-xceiv driver, but
+ * this one use the new generic phy api.
+ *
+ * Copyright (c) 2022 Kalray Inc.
+ * Author(s): Jules Maselbas
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/gpio/consumer.h>
+#include <linux/delay.h>
+#include <linux/usb/otg.h>
+
+struct phy_usb_generic {
+	struct device *dev;
+	struct clk *clk;
+	struct regulator *vcc;
+	struct gpio_desc *gpiod_reset;
+	unsigned long mA;
+	unsigned int vbus;
+	enum usb_dr_mode dr_mode;
+};
+
+static int phy_usb_generic_init(struct phy *phy)
+{
+	struct phy_usb_generic *priv = phy_get_drvdata(phy);
+
+	if (priv->clk)
+		clk_prepare(priv->clk);
+
+	return 0;
+}
+
+static int phy_usb_generic_exit(struct phy *phy)
+{
+	struct phy_usb_generic *priv = phy_get_drvdata(phy);
+
+	if (priv->clk)
+		clk_unprepare(priv->clk);
+
+	return 0;
+}
+
+static int phy_usb_generic_power_on(struct phy *phy)
+{
+	struct phy_usb_generic *priv = phy_get_drvdata(phy);
+	int ret;
+
+	if (priv->vcc) {
+		ret = regulator_enable(priv->vcc);
+		if (ret) {
+			dev_err(priv->dev, "Failed to enable power\n");
+			return ret;
+		}
+	}
+
+	if (priv->clk) {
+		ret = clk_enable(priv->clk);
+		if (ret) {
+			dev_err(priv->dev, "Failed to enable clock\n");
+			return ret;
+		}
+	}
+
+	if (priv->gpiod_reset) {
+		dev_dbg(priv->dev, "Reset toggle\n");
+		gpiod_set_value_cansleep(priv->gpiod_reset, 1);
+		usleep_range(10000, 20000);
+		gpiod_set_value_cansleep(priv->gpiod_reset, 0);
+	}
+
+	return 0;
+}
+
+static int phy_usb_generic_power_off(struct phy *phy)
+{
+	struct phy_usb_generic *priv = phy_get_drvdata(phy);
+	int ret;
+
+	if (priv->gpiod_reset)
+		gpiod_set_value_cansleep(priv->gpiod_reset, 1);
+
+	if (priv->clk)
+		clk_disable_unprepare(priv->clk);
+
+	if (priv->vcc) {
+		ret = regulator_disable(priv->vcc);
+		if (ret) {
+			dev_err(priv->dev, "Failed to disable power\n");
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int phy_usb_generic_set_mode(struct phy *phy,
+				    enum phy_mode mode, int submode)
+{
+	struct phy_usb_generic *priv = phy_get_drvdata(phy);
+	enum usb_dr_mode new_mode;
+	const char *s = "";
+
+	switch (mode) {
+	case PHY_MODE_USB_HOST:
+	case PHY_MODE_USB_HOST_LS:
+	case PHY_MODE_USB_HOST_HS:
+	case PHY_MODE_USB_HOST_FS:
+		new_mode = USB_DR_MODE_HOST;
+		s = "host";
+		break;
+	case PHY_MODE_USB_DEVICE:
+	case PHY_MODE_USB_DEVICE_LS:
+	case PHY_MODE_USB_DEVICE_HS:
+	case PHY_MODE_USB_DEVICE_FS:
+		new_mode = USB_DR_MODE_PERIPHERAL;
+		s = "peripheral";
+		break;
+	case PHY_MODE_USB_OTG:
+		new_mode = USB_DR_MODE_OTG;
+		s = "otg";
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (new_mode != priv->dr_mode) {
+		dev_info(priv->dev, "Changing dr_mode to %s\n", s);
+		priv->dr_mode = new_mode;
+	}
+
+	return 0;
+}
+
+static const struct phy_ops phy_usb_generic_ops = {
+	.init = phy_usb_generic_init,
+	.exit = phy_usb_generic_exit,
+	.power_on = phy_usb_generic_power_on,
+	.power_off = phy_usb_generic_power_off,
+	.set_mode  = phy_usb_generic_set_mode,
+	.owner = THIS_MODULE,
+};
+
+static int phy_usb_generic_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct phy_usb_generic *priv;
+	struct phy_provider *provider;
+	struct phy *phy;
+	int err;
+	u32 clk_rate = 0;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	priv->dev = &pdev->dev;
+	platform_set_drvdata(pdev, priv);
+
+	priv->gpiod_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_ASIS);
+	if (IS_ERR(priv->gpiod_reset)) {
+		err = PTR_ERR(priv->gpiod_reset);
+		dev_err(dev, "Failed to get reset gpio: %d\n", err);
+		return err;
+	}
+
+	if (np && of_property_read_u32(np, "clock-frequency", &clk_rate))
+		clk_rate = 0;
+
+	priv->clk = devm_clk_get_optional(dev, "main_clk");
+	if (IS_ERR(priv->clk)) {
+		err = PTR_ERR(priv->clk);
+		dev_err(dev, "Can't get phy clock: %d\n", err);
+		return err;
+	}
+	if (priv->clk && clk_rate) {
+		err = clk_set_rate(priv->clk, clk_rate);
+		if (err) {
+			dev_err(dev, "Error setting clock rate\n");
+			return err;
+		}
+	}
+
+	priv->vcc = devm_regulator_get_optional(dev, "vcc");
+	if (IS_ERR(priv->vcc)) {
+		err = PTR_ERR(priv->vcc);
+		if (err == -ENODEV || err == -ENOENT) {
+			priv->vcc = NULL;
+		} else {
+			dev_err(dev, "Error getting vcc regulator: %d\n", err);
+			return err;
+		}
+	}
+
+	phy = devm_phy_create(dev, NULL, &phy_usb_generic_ops);
+	if (IS_ERR(phy)) {
+		err = PTR_ERR(phy);
+		dev_err(dev, "Failed to create PHY: %d\n", err);
+		return err;
+	}
+	phy_set_drvdata(phy, priv);
+
+	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	if (IS_ERR(provider)) {
+		err = PTR_ERR(provider);
+		dev_err(dev, "Failed to register PHY provider: %d\n", err);
+		return err;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id phy_usb_nop_dt_ids[] = {
+	{ .compatible = "phy-usb-generic" },
+	{ .compatible = "usb-nop-xceiv" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, phy_usb_nop_dt_ids);
+
+static struct platform_driver phy_usb_generic_driver = {
+	.probe		= phy_usb_generic_probe,
+	.driver		= {
+		.name	= "phy_usb_generic",
+		.of_match_table = phy_usb_nop_dt_ids,
+	},
+};
+module_platform_driver(phy_usb_generic_driver);
+
+MODULE_AUTHOR("Kalray Inc");
+MODULE_DESCRIPTION("Generic USB PHY driver");
+MODULE_LICENSE("GPL");
-- 
2.17.1

Powered by blists - more mailing lists