[PATCH] phy: Add a driver for dm816x USB PHY

[PATCH] phy: Add a driver for dm816x USB PHY

[Tony Lindgren]

Add a minimal driver for dm816x USB. Otherwise we can just use
the existing musb_am335x and musb_dsps on dm816x.

Cc: Brian Hutchinson <b.hutchman@gmail.com>
Cc: Felipe Balbi <balbi@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>

--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/dm816x-phy.txt
@@ -0,0 +1,24 @@
+Device tree binding documentation for am816x USB PHY
+=========================
+
+Required properties:
+- compatible : should be "ti,dm816x-usb-phy"
+- reg : offset and length of the PHY register set.
+- reg-names : name for the phy registers
+- clocks : phandle to the clock
+- clock-names : name of the clock
+- syscon: phandle for the syscon node to access misc registers
+- #phy-cells : from the generic PHY bindings, must be 1
+- syscon: phandle for the syscon node to access misc registers
+
+Example:
+
+usb_phy0: usb-phy@20 {
+	compatible = "ti,dm8168-usb-phy";
+	reg = <0x20 0x8>;
+	reg-names = "phy";
+	clocks = <&main_fapll 6>;
+	clock-names = "refclk";
+	#phy-cells = <0>;
+	syscon = <&scm_conf>;
+};
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -35,6 +35,13 @@ config ARMADA375_USBCLUSTER_PHY
 	depends on OF
 	select GENERIC_PHY
 
+config PHY_DM816X_USB
+	tristate "TI dm816x USB PHY driver"
+	depends on ARCH_OMAP2PLUS
+	select GENERIC_PHY
+	help
+	  Enable this for dm81xx USB to work."
+
 config PHY_EXYNOS_MIPI_VIDEO
 	tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
 	depends on HAS_IOMEM
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -5,6 +5,7 @@
 obj-$(CONFIG_GENERIC_PHY)		+= phy-core.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
 obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
 obj-$(CONFIG_BCM_KONA_USB2_PHY)		+= phy-bcm-kona-usb2.o
 obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
--- /dev/null
+++ b/drivers/phy/phy-dm816x-usb.c
@@ -0,0 +1,295 @@
+/*
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/io.h>
+#include <linux/usb/phy_companion.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/pm_runtime.h>
+#include <linux/delay.h>
+#include <linux/phy/phy.h>
+#include <linux/of_platform.h>
+
+#include <linux/mfd/syscon.h>
+
+/*
+ * TRM has two sets of USB_CTRL registers.. The correct register bits
+ * are in TRM section 24.9.8.2 USB_CTRL Register.
+ */
+#define DM816X_USB_CTRL_PHYCLKSRC	BIT(8)	/* 1 = PLL ref clock */
+#define DM816X_USB_CTRL_PHYSLEEP1	BIT(1)
+#define DM816X_USB_CTRL_PHYSLEEP0	BIT(0)
+
+#define DM816X_USBPHY_CTRL_TXRISETUNE	1
+#define DM816X_USBPHY_CTRL_TXVREFTUNE	0xc
+#define DM816X_USBPHY_CTRL_TXPREEMTUNE	0x2
+
+struct dm816x_usb_phy {
+	struct regmap *syscon;
+	struct device *dev;
+	unsigned int instance;
+	struct clk *refclk;
+	struct usb_phy phy;
+	unsigned int usb_ctrl;		/* Shared between phy0 and phy1 */
+	unsigned int usbphy_ctrl;
+};
+
+static int dm816x_usb_phy_set_host(struct usb_otg *otg, struct usb_bus *host)
+{
+	otg->host = host;
+	if (!host)
+		otg->state = OTG_STATE_UNDEFINED;
+
+	return 0;
+}
+
+static int dm816x_usb_phy_set_peripheral(struct usb_otg *otg,
+					 struct usb_gadget *gadget)
+{
+	otg->gadget = gadget;
+	if (!gadget)
+		otg->state = OTG_STATE_UNDEFINED;
+
+	return 0;
+}
+
+static int dm816x_usb_phy_power_off(struct phy *x)
+{
+	struct dm816x_usb_phy *phy = phy_get_drvdata(x);
+
+	pm_runtime_put(phy->dev);
+
+	return 0;
+}
+
+static int dm816x_usb_phy_power_on(struct phy *x)
+{
+	struct dm816x_usb_phy *phy = phy_get_drvdata(x);
+
+	return pm_runtime_get_sync(phy->dev);
+}
+
+static int dm816x_usb_phy_init(struct phy *x)
+{
+	struct dm816x_usb_phy *phy = phy_get_drvdata(x);
+	unsigned int val;
+	int error;
+
+	error = pm_runtime_get_sync(phy->dev);
+	if (error)
+		return error;
+
+	if (clk_get_rate(phy->refclk) != 24000000)
+		dev_warn(phy->dev, "nonstandard phy refclk\n");
+
+	/* Set PLL ref clock and put phys to sleep */
+	error = regmap_update_bits(phy->syscon, phy->usb_ctrl,
+				   DM816X_USB_CTRL_PHYCLKSRC |
+				   DM816X_USB_CTRL_PHYSLEEP1 |
+				   DM816X_USB_CTRL_PHYSLEEP0,
+				   0);
+	regmap_read(phy->syscon, phy->usb_ctrl, &val);
+
+	/*
+	 * TI kernel sets these values for "symmetrical eye diagram and
+	 * better signal quality" so let's assume somebody checked the
+	 * values with a scope and set them here too.
+	 */
+	regmap_read(phy->syscon, phy->usbphy_ctrl, &val);
+	val |= DM816X_USBPHY_CTRL_TXRISETUNE |
+		DM816X_USBPHY_CTRL_TXVREFTUNE |
+		DM816X_USBPHY_CTRL_TXPREEMTUNE;
+	regmap_write(phy->syscon, phy->usbphy_ctrl, val);
+
+	pm_runtime_put(phy->dev);
+
+	return 0;
+}
+
+static struct phy_ops ops = {
+	.init		= dm816x_usb_phy_init,
+	.power_on	= dm816x_usb_phy_power_on,
+	.power_off	= dm816x_usb_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static int dm816x_usb_phy_runtime_suspend(struct device *dev)
+{
+	struct dm816x_usb_phy *phy = dev_get_drvdata(dev);
+	unsigned int mask, val;
+	int error = 0;
+
+	mask = BIT(phy->instance);
+	val = ~BIT(phy->instance);
+	error = regmap_update_bits(phy->syscon, phy->usb_ctrl,
+				   mask, val);
+	if (error)
+		dev_err(phy->dev, "phy%i failed to power off\n",
+			phy->instance);
+	clk_disable(phy->refclk);
+
+	return 0;
+}
+
+static int dm816x_usb_phy_runtime_resume(struct device *dev)
+{
+	struct dm816x_usb_phy *phy = dev_get_drvdata(dev);
+	unsigned int mask, val;
+	int error;
+
+	error = clk_enable(phy->refclk);
+	if (error)
+		return error;
+
+	mask = BIT(phy->instance);
+	val = BIT(phy->instance);
+	error = regmap_update_bits(phy->syscon, phy->usb_ctrl,
+				   mask, val);
+	if (error) {
+		dev_err(phy->dev, "phy%i failed to power on\n",
+			phy->instance);
+		clk_disable(phy->refclk);
+		return error;
+	}
+
+	return 0;
+}
+
+static UNIVERSAL_DEV_PM_OPS(dm816x_usb_phy_pm_ops,
+			    dm816x_usb_phy_runtime_suspend,
+			    dm816x_usb_phy_runtime_resume,
+			    NULL);
+
+#ifdef CONFIG_OF
+static const struct of_device_id dm816x_usb_phy_id_table[] = {
+	{
+		.compatible = "ti,dm8168-usb-phy",
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, dm816x_usb_phy_id_table);
+#endif
+
+static int dm816x_usb_phy_probe(struct platform_device *pdev)
+{
+	struct dm816x_usb_phy *phy;
+	struct resource *res;
+	struct phy *generic_phy;
+	struct phy_provider *phy_provider;
+	struct usb_otg *otg;
+	const struct of_device_id *of_id;
+	const struct usb_phy_data *phy_data;
+	int error;
+
+	of_id = of_match_device(of_match_ptr(dm816x_usb_phy_id_table),
+				&pdev->dev);
+	if (!of_id)
+		return -EINVAL;
+
+	phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
+	if (!phy)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENOENT;
+
+	phy->syscon = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+						      "syscon");
+	if (IS_ERR(phy->syscon))
+		return PTR_ERR(phy->syscon);
+
+	/*
+	 * According to sprs614e.pdf, the first usb_ctrl is shared and
+	 * the second instance for usb_ctrl is reserved.. Also the
+	 * register bits are different from earlier TRMs.
+	 */
+	phy->usb_ctrl = 0x20;
+	phy->usbphy_ctrl = (res->start & 0xff) + 4;
+	if (phy->usbphy_ctrl == 0x2c)
+		phy->instance = 1;
+
+	phy_data = of_id->data;
+
+	otg = devm_kzalloc(&pdev->dev, sizeof(*otg), GFP_KERNEL);
+	if (!otg)
+		return -ENOMEM;
+
+	phy->dev = &pdev->dev;
+	phy->phy.dev = phy->dev;
+	phy->phy.label = "dm8168_usb_phy";
+	phy->phy.otg = otg;
+	phy->phy.type = USB_PHY_TYPE_USB2;
+	otg->set_host = dm816x_usb_phy_set_host;
+	otg->set_peripheral = dm816x_usb_phy_set_peripheral;
+	otg->usb_phy = &phy->phy;
+
+	platform_set_drvdata(pdev, phy);
+
+	phy->refclk = devm_clk_get(phy->dev, "refclk");
+	if (IS_ERR(phy->refclk))
+		return PTR_ERR(phy->refclk);
+
+	generic_phy = devm_phy_create(phy->dev, NULL, &ops);
+	if (IS_ERR(generic_phy))
+		return PTR_ERR(generic_phy);
+
+	phy_set_drvdata(generic_phy, phy);
+
+	phy_provider = devm_of_phy_provider_register(phy->dev,
+						     of_phy_simple_xlate);
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	error = clk_prepare(phy->refclk);
+	if (error)
+		return error;
+
+	pm_runtime_enable(phy->dev);
+	usb_add_phy_dev(&phy->phy);
+
+	return 0;
+}
+
+static int dm816x_usb_phy_remove(struct platform_device *pdev)
+{
+	struct dm816x_usb_phy *phy = platform_get_drvdata(pdev);
+
+	usb_remove_phy(&phy->phy);
+	pm_runtime_disable(phy->dev);
+	clk_unprepare(phy->refclk);
+
+	return 0;
+}
+
+static struct platform_driver dm816x_usb_phy_driver = {
+	.probe		= dm816x_usb_phy_probe,
+	.remove		= dm816x_usb_phy_remove,
+	.driver		= {
+		.name	= "dm816x-usb-phy",
+		.pm	= &dm816x_usb_phy_pm_ops,
+		.of_match_table = of_match_ptr(dm816x_usb_phy_id_table),
+	},
+};
+
+module_platform_driver(dm816x_usb_phy_driver);
+
+MODULE_ALIAS("platform: dm816x_usb");
+MODULE_AUTHOR("Tony Lindgren <tony@atomide.com>");
+MODULE_DESCRIPTION("dm816x usb phy driver");
+MODULE_LICENSE("GPL v2");

Re: [PATCH] phy: Add a driver for dm816x USB PHY

[Bin Liu]

Hi,

On Mon, Mar 9, 2015 at 3:51 PM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
> Add a minimal driver for dm816x USB. Otherwise we can just use
> the existing musb_am335x and musb_dsps on dm816x.

dm816x has the almost identical usbss as that in am335x, we should be
able to adopt musb_am335x and musb_dsps for dm816x, and dm814x too?

Regards,
-Bin.

>
> Cc: Brian Hutchinson <b.hutchman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
>
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/dm816x-phy.txt
> @@ -0,0 +1,24 @@
> +Device tree binding documentation for am816x USB PHY
> +=========================
> +
> +Required properties:
> +- compatible : should be "ti,dm816x-usb-phy"
> +- reg : offset and length of the PHY register set.
> +- reg-names : name for the phy registers
> +- clocks : phandle to the clock
> +- clock-names : name of the clock
> +- syscon: phandle for the syscon node to access misc registers
> +- #phy-cells : from the generic PHY bindings, must be 1
> +- syscon: phandle for the syscon node to access misc registers
> +
> +Example:
> +
> +usb_phy0: usb-phy@20 {
> +       compatible = "ti,dm8168-usb-phy";
> +       reg = <0x20 0x8>;
> +       reg-names = "phy";
> +       clocks = <&main_fapll 6>;
> +       clock-names = "refclk";
> +       #phy-cells = <0>;
> +       syscon = <&scm_conf>;
> +};
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -35,6 +35,13 @@ config ARMADA375_USBCLUSTER_PHY
>         depends on OF
>         select GENERIC_PHY
>
> +config PHY_DM816X_USB
> +       tristate "TI dm816x USB PHY driver"
> +       depends on ARCH_OMAP2PLUS
> +       select GENERIC_PHY
> +       help
> +         Enable this for dm81xx USB to work."
> +
>  config PHY_EXYNOS_MIPI_VIDEO
>         tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
>         depends on HAS_IOMEM
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -5,6 +5,7 @@
>  obj-$(CONFIG_GENERIC_PHY)              += phy-core.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
>  obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY) += phy-armada375-usb2.o
>  obj-$(CONFIG_BCM_KONA_USB2_PHY)                += phy-bcm-kona-usb2.o
>  obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)      += phy-exynos-dp-video.o
> --- /dev/null
> +++ b/drivers/phy/phy-dm816x-usb.c
> @@ -0,0 +1,295 @@
> +/*
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/io.h>
> +#include <linux/usb/phy_companion.h>
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/delay.h>
> +#include <linux/phy/phy.h>
> +#include <linux/of_platform.h>
> +
> +#include <linux/mfd/syscon.h>
> +
> +/*
> + * TRM has two sets of USB_CTRL registers.. The correct register bits
> + * are in TRM section 24.9.8.2 USB_CTRL Register.
> + */
> +#define DM816X_USB_CTRL_PHYCLKSRC      BIT(8)  /* 1 = PLL ref clock */
> +#define DM816X_USB_CTRL_PHYSLEEP1      BIT(1)
> +#define DM816X_USB_CTRL_PHYSLEEP0      BIT(0)
> +
> +#define DM816X_USBPHY_CTRL_TXRISETUNE  1
> +#define DM816X_USBPHY_CTRL_TXVREFTUNE  0xc
> +#define DM816X_USBPHY_CTRL_TXPREEMTUNE 0x2
> +
> +struct dm816x_usb_phy {
> +       struct regmap *syscon;
> +       struct device *dev;
> +       unsigned int instance;
> +       struct clk *refclk;
> +       struct usb_phy phy;
> +       unsigned int usb_ctrl;          /* Shared between phy0 and phy1 */
> +       unsigned int usbphy_ctrl;
> +};
> +
> +static int dm816x_usb_phy_set_host(struct usb_otg *otg, struct usb_bus *host)
> +{
> +       otg->host = host;
> +       if (!host)
> +               otg->state = OTG_STATE_UNDEFINED;
> +
> +       return 0;
> +}
> +
> +static int dm816x_usb_phy_set_peripheral(struct usb_otg *otg,
> +                                        struct usb_gadget *gadget)
> +{
> +       otg->gadget = gadget;
> +       if (!gadget)
> +               otg->state = OTG_STATE_UNDEFINED;
> +
> +       return 0;
> +}
> +
> +static int dm816x_usb_phy_power_off(struct phy *x)
> +{
> +       struct dm816x_usb_phy *phy = phy_get_drvdata(x);
> +
> +       pm_runtime_put(phy->dev);
> +
> +       return 0;
> +}
> +
> +static int dm816x_usb_phy_power_on(struct phy *x)
> +{
> +       struct dm816x_usb_phy *phy = phy_get_drvdata(x);
> +
> +       return pm_runtime_get_sync(phy->dev);
> +}
> +
> +static int dm816x_usb_phy_init(struct phy *x)
> +{
> +       struct dm816x_usb_phy *phy = phy_get_drvdata(x);
> +       unsigned int val;
> +       int error;
> +
> +       error = pm_runtime_get_sync(phy->dev);
> +       if (error)
> +               return error;
> +
> +       if (clk_get_rate(phy->refclk) != 24000000)
> +               dev_warn(phy->dev, "nonstandard phy refclk\n");
> +
> +       /* Set PLL ref clock and put phys to sleep */
> +       error = regmap_update_bits(phy->syscon, phy->usb_ctrl,
> +                                  DM816X_USB_CTRL_PHYCLKSRC |
> +                                  DM816X_USB_CTRL_PHYSLEEP1 |
> +                                  DM816X_USB_CTRL_PHYSLEEP0,
> +                                  0);
> +       regmap_read(phy->syscon, phy->usb_ctrl, &val);
> +
> +       /*
> +        * TI kernel sets these values for "symmetrical eye diagram and
> +        * better signal quality" so let's assume somebody checked the
> +        * values with a scope and set them here too.
> +        */
> +       regmap_read(phy->syscon, phy->usbphy_ctrl, &val);
> +       val |= DM816X_USBPHY_CTRL_TXRISETUNE |
> +               DM816X_USBPHY_CTRL_TXVREFTUNE |
> +               DM816X_USBPHY_CTRL_TXPREEMTUNE;
> +       regmap_write(phy->syscon, phy->usbphy_ctrl, val);
> +
> +       pm_runtime_put(phy->dev);
> +
> +       return 0;
> +}
> +
> +static struct phy_ops ops = {
> +       .init           = dm816x_usb_phy_init,
> +       .power_on       = dm816x_usb_phy_power_on,
> +       .power_off      = dm816x_usb_phy_power_off,
> +       .owner          = THIS_MODULE,
> +};
> +
> +static int dm816x_usb_phy_runtime_suspend(struct device *dev)
> +{
> +       struct dm816x_usb_phy *phy = dev_get_drvdata(dev);
> +       unsigned int mask, val;
> +       int error = 0;
> +
> +       mask = BIT(phy->instance);
> +       val = ~BIT(phy->instance);
> +       error = regmap_update_bits(phy->syscon, phy->usb_ctrl,
> +                                  mask, val);
> +       if (error)
> +               dev_err(phy->dev, "phy%i failed to power off\n",
> +                       phy->instance);
> +       clk_disable(phy->refclk);
> +
> +       return 0;
> +}
> +
> +static int dm816x_usb_phy_runtime_resume(struct device *dev)
> +{
> +       struct dm816x_usb_phy *phy = dev_get_drvdata(dev);
> +       unsigned int mask, val;
> +       int error;
> +
> +       error = clk_enable(phy->refclk);
> +       if (error)
> +               return error;
> +
> +       mask = BIT(phy->instance);
> +       val = BIT(phy->instance);
> +       error = regmap_update_bits(phy->syscon, phy->usb_ctrl,
> +                                  mask, val);
> +       if (error) {
> +               dev_err(phy->dev, "phy%i failed to power on\n",
> +                       phy->instance);
> +               clk_disable(phy->refclk);
> +               return error;
> +       }
> +
> +       return 0;
> +}
> +
> +static UNIVERSAL_DEV_PM_OPS(dm816x_usb_phy_pm_ops,
> +                           dm816x_usb_phy_runtime_suspend,
> +                           dm816x_usb_phy_runtime_resume,
> +                           NULL);
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id dm816x_usb_phy_id_table[] = {
> +       {
> +               .compatible = "ti,dm8168-usb-phy",
> +       },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, dm816x_usb_phy_id_table);
> +#endif
> +
> +static int dm816x_usb_phy_probe(struct platform_device *pdev)
> +{
> +       struct dm816x_usb_phy *phy;
> +       struct resource *res;
> +       struct phy *generic_phy;
> +       struct phy_provider *phy_provider;
> +       struct usb_otg *otg;
> +       const struct of_device_id *of_id;
> +       const struct usb_phy_data *phy_data;
> +       int error;
> +
> +       of_id = of_match_device(of_match_ptr(dm816x_usb_phy_id_table),
> +                               &pdev->dev);
> +       if (!of_id)
> +               return -EINVAL;
> +
> +       phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
> +       if (!phy)
> +               return -ENOMEM;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!res)
> +               return -ENOENT;
> +
> +       phy->syscon = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> +                                                     "syscon");
> +       if (IS_ERR(phy->syscon))
> +               return PTR_ERR(phy->syscon);
> +
> +       /*
> +        * According to sprs614e.pdf, the first usb_ctrl is shared and
> +        * the second instance for usb_ctrl is reserved.. Also the
> +        * register bits are different from earlier TRMs.
> +        */
> +       phy->usb_ctrl = 0x20;
> +       phy->usbphy_ctrl = (res->start & 0xff) + 4;
> +       if (phy->usbphy_ctrl == 0x2c)
> +               phy->instance = 1;
> +
> +       phy_data = of_id->data;
> +
> +       otg = devm_kzalloc(&pdev->dev, sizeof(*otg), GFP_KERNEL);
> +       if (!otg)
> +               return -ENOMEM;
> +
> +       phy->dev = &pdev->dev;
> +       phy->phy.dev = phy->dev;
> +       phy->phy.label = "dm8168_usb_phy";
> +       phy->phy.otg = otg;
> +       phy->phy.type = USB_PHY_TYPE_USB2;
> +       otg->set_host = dm816x_usb_phy_set_host;
> +       otg->set_peripheral = dm816x_usb_phy_set_peripheral;
> +       otg->usb_phy = &phy->phy;
> +
> +       platform_set_drvdata(pdev, phy);
> +
> +       phy->refclk = devm_clk_get(phy->dev, "refclk");
> +       if (IS_ERR(phy->refclk))
> +               return PTR_ERR(phy->refclk);
> +
> +       generic_phy = devm_phy_create(phy->dev, NULL, &ops);
> +       if (IS_ERR(generic_phy))
> +               return PTR_ERR(generic_phy);
> +
> +       phy_set_drvdata(generic_phy, phy);
> +
> +       phy_provider = devm_of_phy_provider_register(phy->dev,
> +                                                    of_phy_simple_xlate);
> +       if (IS_ERR(phy_provider))
> +               return PTR_ERR(phy_provider);
> +
> +       error = clk_prepare(phy->refclk);
> +       if (error)
> +               return error;
> +
> +       pm_runtime_enable(phy->dev);
> +       usb_add_phy_dev(&phy->phy);
> +
> +       return 0;
> +}
> +
> +static int dm816x_usb_phy_remove(struct platform_device *pdev)
> +{
> +       struct dm816x_usb_phy *phy = platform_get_drvdata(pdev);
> +
> +       usb_remove_phy(&phy->phy);
> +       pm_runtime_disable(phy->dev);
> +       clk_unprepare(phy->refclk);
> +
> +       return 0;
> +}
> +
> +static struct platform_driver dm816x_usb_phy_driver = {
> +       .probe          = dm816x_usb_phy_probe,
> +       .remove         = dm816x_usb_phy_remove,
> +       .driver         = {
> +               .name   = "dm816x-usb-phy",
> +               .pm     = &dm816x_usb_phy_pm_ops,
> +               .of_match_table = of_match_ptr(dm816x_usb_phy_id_table),
> +       },
> +};
> +
> +module_platform_driver(dm816x_usb_phy_driver);
> +
> +MODULE_ALIAS("platform: dm816x_usb");
> +MODULE_AUTHOR("Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>");
> +MODULE_DESCRIPTION("dm816x usb phy driver");
> +MODULE_LICENSE("GPL v2");
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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] phy: Add a driver for dm816x USB PHY

[Felipe Balbi]

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

On Mon, Mar 09, 2015 at 04:11:28PM -0500, Bin Liu wrote:
> Hi,
> 
> On Mon, Mar 9, 2015 at 3:51 PM, Tony Lindgren <tony@atomide.com> wrote:
> > Add a minimal driver for dm816x USB. Otherwise we can just use
> > the existing musb_am335x and musb_dsps on dm816x.
> 
> dm816x has the almost identical usbss as that in am335x, we should be
> able to adopt musb_am335x and musb_dsps for dm816x, and dm814x too?

Tony's using the same musb glue layers, this is just a phy driver,
right ?

-- 
balbi

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

Re: [PATCH] phy: Add a driver for dm816x USB PHY

[Bin Liu]

On Mon, Mar 9, 2015 at 4:13 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Mon, Mar 09, 2015 at 04:11:28PM -0500, Bin Liu wrote:
>> Hi,
>>
>> On Mon, Mar 9, 2015 at 3:51 PM, Tony Lindgren <tony@atomide.com> wrote:
>> > Add a minimal driver for dm816x USB. Otherwise we can just use
>> > the existing musb_am335x and musb_dsps on dm816x.
>>
>> dm816x has the almost identical usbss as that in am335x, we should be
>> able to adopt musb_am335x and musb_dsps for dm816x, and dm814x too?
>
> Tony's using the same musb glue layers, this is just a phy driver,
> right ?

Can the current am335x phy driver be adopted too? I remember it is
under drivers/usb/phy/.

Regards,
-Bin.

>
> --
> balbi

Re: [PATCH] phy: Add a driver for dm816x USB PHY

[Felipe Balbi]

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

On Mon, Mar 09, 2015 at 04:17:29PM -0500, Bin Liu wrote:
> On Mon, Mar 9, 2015 at 4:13 PM, Felipe Balbi <balbi@ti.com> wrote:
> > On Mon, Mar 09, 2015 at 04:11:28PM -0500, Bin Liu wrote:
> >> Hi,
> >>
> >> On Mon, Mar 9, 2015 at 3:51 PM, Tony Lindgren <tony@atomide.com> wrote:
> >> > Add a minimal driver for dm816x USB. Otherwise we can just use
> >> > the existing musb_am335x and musb_dsps on dm816x.
> >>
> >> dm816x has the almost identical usbss as that in am335x, we should be
> >> able to adopt musb_am335x and musb_dsps for dm816x, and dm814x too?
> >
> > Tony's using the same musb glue layers, this is just a phy driver,
> > right ?
> 
> Can the current am335x phy driver be adopted too? I remember it is
> under drivers/usb/phy/.

Tony will be best to answer, but according to our IRC discussions, it's
too different. Tony ?

-- 
balbi

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

Re: [PATCH] phy: Add a driver for dm816x USB PHY

[Tony Lindgren]

* Felipe Balbi <balbi@ti.com> [150309 14:21]:
> On Mon, Mar 09, 2015 at 04:17:29PM -0500, Bin Liu wrote:
> > On Mon, Mar 9, 2015 at 4:13 PM, Felipe Balbi <balbi@ti.com> wrote:
> > > On Mon, Mar 09, 2015 at 04:11:28PM -0500, Bin Liu wrote:
> > >> Hi,
> > >>
> > >> On Mon, Mar 9, 2015 at 3:51 PM, Tony Lindgren <tony@atomide.com> wrote:
> > >> > Add a minimal driver for dm816x USB. Otherwise we can just use
> > >> > the existing musb_am335x and musb_dsps on dm816x.
> > >>
> > >> dm816x has the almost identical usbss as that in am335x, we should be
> > >> able to adopt musb_am335x and musb_dsps for dm816x, and dm814x too?
> > >
> > > Tony's using the same musb glue layers, this is just a phy driver,
> > > right ?
> > 
> > Can the current am335x phy driver be adopted too? I remember it is
> > under drivers/usb/phy/.
> 
> Tony will be best to answer, but according to our IRC discussions, it's
> too different. Tony ?

Well we should check again between dm814x and am335x documentation
against the $subject driver as the dm816x docs were buggy and I may
have gotten a wrong idea initially. 

Regards,

Tony

Re: [PATCH] phy: Add a driver for dm816x USB PHY

[Bin Liu]

On Mon, Mar 9, 2015 at 4:26 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Felipe Balbi <balbi@ti.com> [150309 14:21]:
>> On Mon, Mar 09, 2015 at 04:17:29PM -0500, Bin Liu wrote:
>> > On Mon, Mar 9, 2015 at 4:13 PM, Felipe Balbi <balbi@ti.com> wrote:
>> > > On Mon, Mar 09, 2015 at 04:11:28PM -0500, Bin Liu wrote:
>> > >> Hi,
>> > >>
>> > >> On Mon, Mar 9, 2015 at 3:51 PM, Tony Lindgren <tony@atomide.com> wrote:
>> > >> > Add a minimal driver for dm816x USB. Otherwise we can just use
>> > >> > the existing musb_am335x and musb_dsps on dm816x.
>> > >>
>> > >> dm816x has the almost identical usbss as that in am335x, we should be
>> > >> able to adopt musb_am335x and musb_dsps for dm816x, and dm814x too?
>> > >
>> > > Tony's using the same musb glue layers, this is just a phy driver,
>> > > right ?
>> >
>> > Can the current am335x phy driver be adopted too? I remember it is
>> > under drivers/usb/phy/.
>>
>> Tony will be best to answer, but according to our IRC discussions, it's
>> too different. Tony ?
>
> Well we should check again between dm814x and am335x documentation
> against the $subject driver as the dm816x docs were buggy and I may
> have gotten a wrong idea initially.

Will it make our life a little easier by comparing the usb drivers for
dm81xx and am335x in previous TI kernel releases? I cam dig out the
source code if that helps.

Regards,
-Bin,

>
> Regards,
>
> Tony

Re: [PATCH] phy: Add a driver for dm816x USB PHY

[Tony Lindgren]

* Bin Liu <binmlist-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> [150309 14:35]:
> On Mon, Mar 9, 2015 at 4:26 PM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
> > * Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> [150309 14:21]:
> >> On Mon, Mar 09, 2015 at 04:17:29PM -0500, Bin Liu wrote:
> >> > On Mon, Mar 9, 2015 at 4:13 PM, Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> wrote:
> >> > > On Mon, Mar 09, 2015 at 04:11:28PM -0500, Bin Liu wrote:
> >> > >> Hi,
> >> > >>
> >> > >> On Mon, Mar 9, 2015 at 3:51 PM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
> >> > >> > Add a minimal driver for dm816x USB. Otherwise we can just use
> >> > >> > the existing musb_am335x and musb_dsps on dm816x.
> >> > >>
> >> > >> dm816x has the almost identical usbss as that in am335x, we should be
> >> > >> able to adopt musb_am335x and musb_dsps for dm816x, and dm814x too?
> >> > >
> >> > > Tony's using the same musb glue layers, this is just a phy driver,
> >> > > right ?
> >> >
> >> > Can the current am335x phy driver be adopted too? I remember it is
> >> > under drivers/usb/phy/.
> >>
> >> Tony will be best to answer, but according to our IRC discussions, it's
> >> too different. Tony ?
> >
> > Well we should check again between dm814x and am335x documentation
> > against the $subject driver as the dm816x docs were buggy and I may
> > have gotten a wrong idea initially.
> 
> Will it make our life a little easier by comparing the usb drivers for
> dm81xx and am335x in previous TI kernel releases? I cam dig out the
> source code if that helps.

Yeah that probably helps if you've dealt with dm814x earlier :)
Just don't look at the dm816x trm, only the errata pdf works for
dm816x phy.

Note that we still are missing basic support for dm814x in mainline,
I'm planning to tackle that at some point but I don't know when I'm
going to get to it..

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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] phy: Add a driver for dm816x USB PHY

[Bin Liu]

On Mon, Mar 9, 2015 at 4:41 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Bin Liu <binmlist@gmail.com> [150309 14:35]:
>> On Mon, Mar 9, 2015 at 4:26 PM, Tony Lindgren <tony@atomide.com> wrote:
>> > * Felipe Balbi <balbi@ti.com> [150309 14:21]:
>> >> On Mon, Mar 09, 2015 at 04:17:29PM -0500, Bin Liu wrote:
>> >> > On Mon, Mar 9, 2015 at 4:13 PM, Felipe Balbi <balbi@ti.com> wrote:
>> >> > > On Mon, Mar 09, 2015 at 04:11:28PM -0500, Bin Liu wrote:
>> >> > >> Hi,
>> >> > >>
>> >> > >> On Mon, Mar 9, 2015 at 3:51 PM, Tony Lindgren <tony@atomide.com> wrote:
>> >> > >> > Add a minimal driver for dm816x USB. Otherwise we can just use
>> >> > >> > the existing musb_am335x and musb_dsps on dm816x.
>> >> > >>
>> >> > >> dm816x has the almost identical usbss as that in am335x, we should be
>> >> > >> able to adopt musb_am335x and musb_dsps for dm816x, and dm814x too?
>> >> > >
>> >> > > Tony's using the same musb glue layers, this is just a phy driver,
>> >> > > right ?
>> >> >
>> >> > Can the current am335x phy driver be adopted too? I remember it is
>> >> > under drivers/usb/phy/.
>> >>
>> >> Tony will be best to answer, but according to our IRC discussions, it's
>> >> too different. Tony ?
>> >
>> > Well we should check again between dm814x and am335x documentation
>> > against the $subject driver as the dm816x docs were buggy and I may
>> > have gotten a wrong idea initially.
>>
>> Will it make our life a little easier by comparing the usb drivers for
>> dm81xx and am335x in previous TI kernel releases? I cam dig out the
>> source code if that helps.
>
> Yeah that probably helps if you've dealt with dm814x earlier :)

Well, don't expect much from me about dm814x ;), I did not read its
trm nor errata, but only booted the board a few times trying to
compare the MUSB behaviours while debugging issues on am335x.

Ok, the previous TI released 3.2 kernel for am335x is at [1], and
2.6.37 kernel dm81xx is at [2].

Regards,
-Bin.

[1] http://arago-project.org/git/projects/?p=linux-am33x.git;a=shortlog;h=refs/heads/v3.2-staging

[2] http://arago-project.org/git/projects/?p=linux-omap3.git;a=shortlog;h=refs/heads/TI81XXPSP_04.04.00.01


> Just don't look at the dm816x trm, only the errata pdf works for
> dm816x phy.
>
> Note that we still are missing basic support for dm814x in mainline,
> I'm planning to tackle that at some point but I don't know when I'm
> going to get to it..
>
> Regards,
>
> Tony

Re: [PATCH] phy: Add a driver for dm816x USB PHY

[Matthijs van Duin]

Given that the documentation mentions the actual phy used, it may be
worth mentioning this also in the driver?  i.e. that it's not a
"dm816x phy" but a "SR70LX Synopsys USB 2.0 OTG nanoPHY" (in contrast
to the dm814x and am335x which use a phy TI made themselves)

USB is one of the few subsystems of the DM814x I've never really
examined in any detail, but it appears nearly identical to the AM335x.
(e.g. the phy registers in the control module are nearly identical,
differing only in some bits related to GPIO-mode)


On 9 March 2015 at 22:41, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
> Note that we still are missing basic support for dm814x in mainline,
> I'm planning to tackle that at some point but I don't know when I'm
> going to get to it..

Do ping me if you have questions. While I still don't have time to
really throw myself on the task myself, and I'm still hindered by
insufficient knowledge of the kernel, I do have quite a bit of
experience with the processor itself.


BTW, w.r.t. the old "official" TI kernel for dm81xx: there are
actually two relevant forks of the unmaintained
arago-omap3/ti81xx-master branch, namely:

http://arago-project.org/git/projects/?p=linux-ipnc-rdk-dm81xx.git
http://arago-project.org/git/projects/?p=linux-dvr-rdk-dm81xx.git

which contain various bugfixes, including for usb (though also patches
specific to those SDKs).
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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] phy: Add a driver for dm816x USB PHY

[Tony Lindgren]

* Matthijs van Duin <matthijsvanduin@gmail.com> [150313 11:39]:
> Given that the documentation mentions the actual phy used, it may be
> worth mentioning this also in the driver?  i.e. that it's not a
> "dm816x phy" but a "SR70LX Synopsys USB 2.0 OTG nanoPHY" (in contrast
> to the dm814x and am335x which use a phy TI made themselves)

Hmm OK have to check that. It could also be that dm816x documentation
is copy-paste from da850 or am3517 and the PHY got changed in the
hardware as the registers don't match the documentation. Only the
dm816x errata has right documentation for the USB PHY.
 
> USB is one of the few subsystems of the DM814x I've never really
> examined in any detail, but it appears nearly identical to the AM335x.
> (e.g. the phy registers in the control module are nearly identical,
> differing only in some bits related to GPIO-mode)

OK thanks for confirming that. It's different from this one then.
 
> On 9 March 2015 at 22:41, Tony Lindgren <tony@atomide.com> wrote:
> > Note that we still are missing basic support for dm814x in mainline,
> > I'm planning to tackle that at some point but I don't know when I'm
> > going to get to it..
> 
> Do ping me if you have questions. While I still don't have time to
> really throw myself on the task myself, and I'm still hindered by
> insufficient knowledge of the kernel, I do have quite a bit of
> experience with the processor itself.
> 
> 
> BTW, w.r.t. the old "official" TI kernel for dm81xx: there are
> actually two relevant forks of the unmaintained
> arago-omap3/ti81xx-master branch, namely:
> 
> http://arago-project.org/git/projects/?p=linux-ipnc-rdk-dm81xx.git
> http://arago-project.org/git/projects/?p=linux-dvr-rdk-dm81xx.git
> 
> which contain various bugfixes, including for usb (though also patches
> specific to those SDKs).

Yeah it will be several days of work anyways, not currently my
priority for sure. Would be nice to get it done though.

Regards,

Tony

Re: [PATCH] phy: Add a driver for dm816x USB PHY

[Matthijs van Duin]

On 13 March 2015 at 20:30, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
> Hmm OK have to check that. It could also be that dm816x documentation
> is copy-paste from da850 or am3517 and the PHY got changed in the
> hardware as the registers don't match the documentation. Only the
> dm816x errata has right documentation for the USB PHY.

Hmm? While I see plenty of usb errata (mostly DMA bugs), I don't see
anything about registers being different.

I do see something curious: advisory 70, the only PHY-related erratum
I see, is also present in the DM814x errata and even in AM335x r1.0.
This strongly suggests the PHYs must at least be closely related...

The dm816x TRM makes three separate mentions of the synopsys usb phy
though, while I found no other TRMs that mention it, so if it's a
copy-paste error (which certainly would not be exceptional) I don't
know where from.

I suppose it's still possible TI acquired a sufficiently permissive
license for the synopsys phy to fork it and call it a "TI PHY" as they
do in the AM335x docs. (No mention of its origin is made in the DM814x
docs.)

BTW, da850? Is that yet another instance of Primus? (i.e.
omap-L1xx/c674x/am1xxx with odd final digit, also da830/da828)

Matthijs
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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] phy: Add a driver for dm816x USB PHY

[Tony Lindgren]

* Matthijs van Duin <matthijsvanduin@gmail.com> [150314 14:04]:
> On 13 March 2015 at 20:30, Tony Lindgren <tony@atomide.com> wrote:
> > Hmm OK have to check that. It could also be that dm816x documentation
> > is copy-paste from da850 or am3517 and the PHY got changed in the
> > hardware as the registers don't match the documentation. Only the
> > dm816x errata has right documentation for the USB PHY.
> 
> Hmm? While I see plenty of usb errata (mostly DMA bugs), I don't see
> anything about registers being different.

Sorry it seems to be the partially updated TRM instead, it's the
sprs614e.pdf instead. That has the USBPHY_CTRL registers right.
No mention of Synopsys in sprs614e.pdf though so who knows. It
seems something got swapped compared to the TRM as the USB_CTRL
registers totally changed.

I'll just add a comments you mentioned earlier about it probably
being Synopsys phy.
 
> I do see something curious: advisory 70, the only PHY-related erratum
> I see, is also present in the DM814x errata and even in AM335x r1.0.
> This strongly suggests the PHYs must at least be closely related...

Hmm interesting. But dm814x has again different USB_CTRL registers,
seems to be wired up like am335x. Also there's no USBPHY_CTRL
registers on dm814x or am335x. Chances are that dm814x is wired up
the same way as am335x.

> The dm816x TRM makes three separate mentions of the synopsys usb phy
> though, while I found no other TRMs that mention it, so if it's a
> copy-paste error (which certainly would not be exceptional) I don't
> know where from.

Yes I checked am3517 trm, and that too mentions Synopsys once, but
has different registers. And also checked the l-138/da850 TRM, and
that does not have USB_CTRL and USBPHY_CTRL registers either.. Looks
like the copy paste errors come from tms320dm6446.pdf that has
the USB_CTRL and USBPHY_CTRL registers, but then no mention of
Synopsys.

> I suppose it's still possible TI acquired a sufficiently permissive
> license for the synopsys phy to fork it and call it a "TI PHY" as they
> do in the AM335x docs. (No mention of its origin is made in the DM814x
> docs.)
> 
> BTW, da850? Is that yet another instance of Primus? (i.e.
> omap-L1xx/c674x/am1xxx with odd final digit, also da830/da828)

Yes it's the arm926 based series, l-138 is da850 I believe.

Regards,

Tony

Re: [PATCH] phy: Add a driver for dm816x USB PHY

[Matthijs van Duin]

*gets increasingly confused*

The datasheet (sprs614e) only contains register addresses, and they
seem to match the TRM's USB chapter.  The only disagreement I can spot
is related to USB_CTRL register(s) in the control module (offsets
0x620 and 0x628) where
* the TRM claims both exist in the control module chapter (1.16.1.3),
one for each both
* the TRM's USB chapter claims only one exists, and its layout
(24.9.8.2) doesn't resemble the former one
* the datasheet agrees only one exists (but gives no layout)
* reality seems to disagree with both: I booted up our evm816x,
plugged in a mouse to confirm USB is functional, and attached JTAG:
both registers read as zero (contradicting both layouts) and appear to
ignore writes.

There doesn't seem to be any disagreement in the docs about the
USBPHY_CTRL regs (offsets 0x624 and 0x62c in control module) as far as
I can tell, and the documented reset values also match what I see via
JTAG.

> But dm814x [..] seems to be wired up like am335x.

This I mentioned: the only difference is related to GPIO mode (which
on the dm814x yields exactly that: GPIO, while the am335x uses it to
hook up UARTs).

> Yes I checked am3517 trm, and that too mentions Synopsys once

Only in its ECHI/OCHI host controller, not the OTG one...


Anyhow, I didn't expect this small observation to end up becoming a
device archeology expedition, sorry about that ;-)


>> BTW, da850? Is that yet another instance of Primus? (i.e.
>> omap-L1xx/c674x/am1xxx with odd final digit, also da830/da828)
>
> Yes it's the arm926 based series, l-138 is da850 I believe.

Ah, but there are two such series: Freon and Primus. Just to be
different, their part numbers are both allocated from the omap-L1xx
(arm+dsp) / tms320c674x (dsp only) / am1xxx (arm only) ranges, but
distinguished by Freon having even final digit while Primus has odd
final digit. Of course this doesn't hold for the da8xx parts, that
would be too consistent.

But you're right, da850 indeed seems to be a Freon rather than a
Primus based on some more googling (apparently da850 has SATA --
Primus doesn't).

Matthijs

On 16 March 2015 at 17:49, Tony Lindgren <tony@atomide.com> wrote:
> * Matthijs van Duin <matthijsvanduin@gmail.com> [150314 14:04]:
>> On 13 March 2015 at 20:30, Tony Lindgren <tony@atomide.com> wrote:
>> > Hmm OK have to check that. It could also be that dm816x documentation
>> > is copy-paste from da850 or am3517 and the PHY got changed in the
>> > hardware as the registers don't match the documentation. Only the
>> > dm816x errata has right documentation for the USB PHY.
>>
>> Hmm? While I see plenty of usb errata (mostly DMA bugs), I don't see
>> anything about registers being different.
>
> Sorry it seems to be the partially updated TRM instead, it's the
> sprs614e.pdf instead. That has the USBPHY_CTRL registers right.
> No mention of Synopsys in sprs614e.pdf though so who knows. It
> seems something got swapped compared to the TRM as the USB_CTRL
> registers totally changed.
>
> I'll just add a comments you mentioned earlier about it probably
> being Synopsys phy.
>
>> I do see something curious: advisory 70, the only PHY-related erratum
>> I see, is also present in the DM814x errata and even in AM335x r1.0.
>> This strongly suggests the PHYs must at least be closely related...
>
> Hmm interesting. But dm814x has again different USB_CTRL registers,
> seems to be wired up like am335x. Also there's no USBPHY_CTRL
> registers on dm814x or am335x. Chances are that dm814x is wired up
> the same way as am335x.
>
>> The dm816x TRM makes three separate mentions of the synopsys usb phy
>> though, while I found no other TRMs that mention it, so if it's a
>> copy-paste error (which certainly would not be exceptional) I don't
>> know where from.
>
> Yes I checked am3517 trm, and that too mentions Synopsys once, but
> has different registers. And also checked the l-138/da850 TRM, and
> that does not have USB_CTRL and USBPHY_CTRL registers either.. Looks
> like the copy paste errors come from tms320dm6446.pdf that has
> the USB_CTRL and USBPHY_CTRL registers, but then no mention of
> Synopsys.
>
>> I suppose it's still possible TI acquired a sufficiently permissive
>> license for the synopsys phy to fork it and call it a "TI PHY" as they
>> do in the AM335x docs. (No mention of its origin is made in the DM814x
>> docs.)
>>
>> BTW, da850? Is that yet another instance of Primus? (i.e.
>> omap-L1xx/c674x/am1xxx with odd final digit, also da830/da828)
>
> Yes it's the arm926 based series, l-138 is da850 I believe.
>
> Regards,
>
> Tony

Re: [PATCH] phy: Add a driver for dm816x USB PHY

[Tony Lindgren]

* Matthijs van Duin <matthijsvanduin@gmail.com> [150316 14:17]:
> *gets increasingly confused*

:)
 
> The datasheet (sprs614e) only contains register addresses, and they
> seem to match the TRM's USB chapter.  The only disagreement I can spot
> is related to USB_CTRL register(s) in the control module (offsets
> 0x620 and 0x628) where
> * the TRM claims both exist in the control module chapter (1.16.1.3),
> one for each both
> * the TRM's USB chapter claims only one exists, and its layout
> (24.9.8.2) doesn't resemble the former one

Yes the second entry is the closest thing to a documentation
here it seems.

> * the datasheet agrees only one exists (but gives no layout)
> * reality seems to disagree with both: I booted up our evm816x,
> plugged in a mouse to confirm USB is functional, and attached JTAG:
> both registers read as zero (contradicting both layouts) and appear to
> ignore writes.

Yes so it seem here too, this is dm816x rev c, what do you have?

Anyways, I'll add a note that at least rev c does not seems to do
anything with USB_CTRL but that we follow what the TI tree is doing
in case some other revisions of the hardware use it.
 
> There doesn't seem to be any disagreement in the docs about the
> USBPHY_CTRL regs (offsets 0x624 and 0x62c in control module) as far as
> I can tell, and the documented reset values also match what I see via
> JTAG.

Yes agreed.

> > But dm814x [..] seems to be wired up like am335x.
> 
> This I mentioned: the only difference is related to GPIO mode (which
> on the dm814x yields exactly that: GPIO, while the am335x uses it to
> hook up UARTs).
> 
> > Yes I checked am3517 trm, and that too mentions Synopsys once
> 
> Only in its ECHI/OCHI host controller, not the OTG one...

Oh OK I missed that part.
 
> Anyhow, I didn't expect this small observation to end up becoming a
> device archeology expedition, sorry about that ;-)

Well thanks for spotting the weirdness :) 
 
> >> BTW, da850? Is that yet another instance of Primus? (i.e.
> >> omap-L1xx/c674x/am1xxx with odd final digit, also da830/da828)
> >
> > Yes it's the arm926 based series, l-138 is da850 I believe.
> 
> Ah, but there are two such series: Freon and Primus. Just to be
> different, their part numbers are both allocated from the omap-L1xx
> (arm+dsp) / tms320c674x (dsp only) / am1xxx (arm only) ranges, but
> distinguished by Freon having even final digit while Primus has odd
> final digit. Of course this doesn't hold for the da8xx parts, that
> would be too consistent.

I can't keep up with these part numbers..
 
> But you're right, da850 indeed seems to be a Freon rather than a
> Primus based on some more googling (apparently da850 has SATA --
> Primus doesn't).

OK

Regards,

Tony

Re: [PATCH] phy: Add a driver for dm816x USB PHY

[Matthijs van Duin]

On 17 March 2015 at 03:19, Tony Lindgren <tony@atomide.com> wrote:
> Yes so it seem here too, this is dm816x rev c, what do you have?

jtag ID reads 0x1b81e02f, so that would be rev 1.1 / rev A.

> Anyways, I'll add a note that at least rev c does not seems to do
> anything with USB_CTRL but that we follow what the TI tree is doing
> in case some other revisions of the hardware use it.

Another idea would be to check if it's nonzero and if so, yell loudly
in the kernel log that they should mail the linux-omap list ;-)

Matthijs

Re: [PATCH] phy: Add a driver for dm816x USB PHY

[Tony Lindgren]

* Matthijs van Duin <matthijsvanduin@gmail.com> [150319 04:20]:
> On 17 March 2015 at 03:19, Tony Lindgren <tony@atomide.com> wrote:
> > Yes so it seem here too, this is dm816x rev c, what do you have?
> 
> jtag ID reads 0x1b81e02f, so that would be rev 1.1 / rev A.
> 
> > Anyways, I'll add a note that at least rev c does not seems to do
> > anything with USB_CTRL but that we follow what the TI tree is doing
> > in case some other revisions of the hardware use it.
> 
> Another idea would be to check if it's nonzero and if so, yell loudly
> in the kernel log that they should mail the linux-omap list ;-)

Sure I'll add that to the phy start up code :)

Regards,

Tony

Re: [PATCH] phy: Add a driver for dm816x USB PHY

[Tony Lindgren]

* Bin Liu <binmlist@gmail.com> [150309 14:17]:
> On Mon, Mar 9, 2015 at 4:13 PM, Felipe Balbi <balbi@ti.com> wrote:
> > On Mon, Mar 09, 2015 at 04:11:28PM -0500, Bin Liu wrote:
> >> Hi,
> >>
> >> On Mon, Mar 9, 2015 at 3:51 PM, Tony Lindgren <tony@atomide.com> wrote:
> >> > Add a minimal driver for dm816x USB. Otherwise we can just use
> >> > the existing musb_am335x and musb_dsps on dm816x.
> >>
> >> dm816x has the almost identical usbss as that in am335x, we should be
> >> able to adopt musb_am335x and musb_dsps for dm816x, and dm814x too?
> >
> > Tony's using the same musb glue layers, this is just a phy driver,
> > right ?
> 
> Can the current am335x phy driver be adopted too? I remember it is
> under drivers/usb/phy/.

Yes this is just the phy, other than that things work the same.

I believe this is different from the am335x phy, and more similar to
what we have in old drivers/usb/musb/davinci.c. Chances are dm814x
is same as am335x phy.

But yeah, we really should do a proper phy driver for am335x too
along the same lines. Might even be be able to combine those drivers
with just different compatible values to set the quirks and the
enable/disable functions.

Regards,

Tony

Re: [PATCH] phy: Add a driver for dm816x USB PHY

[Bin Liu]

On Mon, Mar 9, 2015 at 4:20 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Bin Liu <binmlist@gmail.com> [150309 14:17]:
>> On Mon, Mar 9, 2015 at 4:13 PM, Felipe Balbi <balbi@ti.com> wrote:
>> > On Mon, Mar 09, 2015 at 04:11:28PM -0500, Bin Liu wrote:
>> >> Hi,
>> >>
>> >> On Mon, Mar 9, 2015 at 3:51 PM, Tony Lindgren <tony@atomide.com> wrote:
>> >> > Add a minimal driver for dm816x USB. Otherwise we can just use
>> >> > the existing musb_am335x and musb_dsps on dm816x.
>> >>
>> >> dm816x has the almost identical usbss as that in am335x, we should be
>> >> able to adopt musb_am335x and musb_dsps for dm816x, and dm814x too?
>> >
>> > Tony's using the same musb glue layers, this is just a phy driver,
>> > right ?
>>
>> Can the current am335x phy driver be adopted too? I remember it is
>> under drivers/usb/phy/.
>
> Yes this is just the phy, other than that things work the same.
>
> I believe this is different from the am335x phy, and more similar to
> what we have in old drivers/usb/musb/davinci.c. Chances are dm814x
> is same as am335x phy.

Alright, I only played the dm814x board a few times, but never with
dm816x, and thought dm816x and dm814x usbss are the same and very
close to am335x - am335x came out later than dm81xx and has some
improvement in usbss.

>
> But yeah, we really should do a proper phy driver for am335x too
> along the same lines. Might even be be able to combine those drivers
> with just different compatible values to set the quirks and the
> enable/disable functions.

Ok, I agreed we can make dm816x working first, then gradually move
am335x aligned to the framework.

Regards,
-Bin.

>
> Regards,
>
> Tony

Re: [PATCH] phy: Add a driver for dm816x USB PHY

[Kishon Vijay Abraham I]

Hi Tony,

On Tuesday 10 March 2015 02:21 AM, Tony Lindgren wrote:
> Add a minimal driver for dm816x USB. Otherwise we can just use
> the existing musb_am335x and musb_dsps on dm816x.

If we can use an existing driver, I'd prefer that.
>
> Cc: Brian Hutchinson <b.hutchman@gmail.com>
> Cc: Felipe Balbi <balbi@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
>
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/dm816x-phy.txt
> @@ -0,0 +1,24 @@
> +Device tree binding documentation for am816x USB PHY
> +=========================
> +
> +Required properties:
> +- compatible : should be "ti,dm816x-usb-phy"
> +- reg : offset and length of the PHY register set.
> +- reg-names : name for the phy registers
> +- clocks : phandle to the clock
> +- clock-names : name of the clock
> +- syscon: phandle for the syscon node to access misc registers
> +- #phy-cells : from the generic PHY bindings, must be 1
> +- syscon: phandle for the syscon node to access misc registers
> +
> +Example:
> +
> +usb_phy0: usb-phy@20 {
> +	compatible = "ti,dm8168-usb-phy";
> +	reg = <0x20 0x8>;
> +	reg-names = "phy";
> +	clocks = <&main_fapll 6>;
> +	clock-names = "refclk";
> +	#phy-cells = <0>;
> +	syscon = <&scm_conf>;
> +};
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -35,6 +35,13 @@ config ARMADA375_USBCLUSTER_PHY
>   	depends on OF
>   	select GENERIC_PHY
>
> +config PHY_DM816X_USB
> +	tristate "TI dm816x USB PHY driver"
> +	depends on ARCH_OMAP2PLUS
> +	select GENERIC_PHY
> +	help
> +	  Enable this for dm81xx USB to work."
> +
>   config PHY_EXYNOS_MIPI_VIDEO
>   	tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
>   	depends on HAS_IOMEM
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -5,6 +5,7 @@
>   obj-$(CONFIG_GENERIC_PHY)		+= phy-core.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
>   obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
>   obj-$(CONFIG_BCM_KONA_USB2_PHY)		+= phy-bcm-kona-usb2.o
>   obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
> --- /dev/null
> +++ b/drivers/phy/phy-dm816x-usb.c
> @@ -0,0 +1,295 @@
> +/*
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/io.h>
> +#include <linux/usb/phy_companion.h>
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/delay.h>
> +#include <linux/phy/phy.h>
> +#include <linux/of_platform.h>
> +
> +#include <linux/mfd/syscon.h>
> +
> +/*
> + * TRM has two sets of USB_CTRL registers.. The correct register bits
> + * are in TRM section 24.9.8.2 USB_CTRL Register.
> + */
> +#define DM816X_USB_CTRL_PHYCLKSRC	BIT(8)	/* 1 = PLL ref clock */
> +#define DM816X_USB_CTRL_PHYSLEEP1	BIT(1)
> +#define DM816X_USB_CTRL_PHYSLEEP0	BIT(0)
> +
> +#define DM816X_USBPHY_CTRL_TXRISETUNE	1
> +#define DM816X_USBPHY_CTRL_TXVREFTUNE	0xc
> +#define DM816X_USBPHY_CTRL_TXPREEMTUNE	0x2
> +
> +struct dm816x_usb_phy {
> +	struct regmap *syscon;
> +	struct device *dev;
> +	unsigned int instance;
> +	struct clk *refclk;
> +	struct usb_phy phy;
> +	unsigned int usb_ctrl;		/* Shared between phy0 and phy1 */
> +	unsigned int usbphy_ctrl;
> +};
> +
> +static int dm816x_usb_phy_set_host(struct usb_otg *otg, struct usb_bus *host)
> +{
> +	otg->host = host;
> +	if (!host)
> +		otg->state = OTG_STATE_UNDEFINED;
> +
> +	return 0;
> +}
> +
> +static int dm816x_usb_phy_set_peripheral(struct usb_otg *otg,
> +					 struct usb_gadget *gadget)
> +{
> +	otg->gadget = gadget;
> +	if (!gadget)
> +		otg->state = OTG_STATE_UNDEFINED;
> +
> +	return 0;
> +}
> +
> +static int dm816x_usb_phy_power_off(struct phy *x)
> +{
> +	struct dm816x_usb_phy *phy = phy_get_drvdata(x);
> +
> +	pm_runtime_put(phy->dev);

phy core takes care of invoking pm_runtime_put on power_off.
So this function shouldn't be needed at all.
> +
> +	return 0;
> +}
> +
> +static int dm816x_usb_phy_power_on(struct phy *x)
> +{
> +	struct dm816x_usb_phy *phy = phy_get_drvdata(x);
> +
> +	return pm_runtime_get_sync(phy->dev);

similarly on power_on, phy_core invokes pm_runtime_get_sync.
> +}
> +
> +static int dm816x_usb_phy_init(struct phy *x)
> +{
> +	struct dm816x_usb_phy *phy = phy_get_drvdata(x);
> +	unsigned int val;
> +	int error;
> +
> +	error = pm_runtime_get_sync(phy->dev);
> +	if (error)
> +		return error;

not required.
> +
> +	if (clk_get_rate(phy->refclk) != 24000000)
> +		dev_warn(phy->dev, "nonstandard phy refclk\n");
> +
> +	/* Set PLL ref clock and put phys to sleep */
> +	error = regmap_update_bits(phy->syscon, phy->usb_ctrl,
> +				   DM816X_USB_CTRL_PHYCLKSRC |
> +				   DM816X_USB_CTRL_PHYSLEEP1 |
> +				   DM816X_USB_CTRL_PHYSLEEP0,
> +				   0);
> +	regmap_read(phy->syscon, phy->usb_ctrl, &val);
> +
> +	/*
> +	 * TI kernel sets these values for "symmetrical eye diagram and
> +	 * better signal quality" so let's assume somebody checked the
> +	 * values with a scope and set them here too.
> +	 */
> +	regmap_read(phy->syscon, phy->usbphy_ctrl, &val);
> +	val |= DM816X_USBPHY_CTRL_TXRISETUNE |
> +		DM816X_USBPHY_CTRL_TXVREFTUNE |
> +		DM816X_USBPHY_CTRL_TXPREEMTUNE;
> +	regmap_write(phy->syscon, phy->usbphy_ctrl, val);
> +
> +	pm_runtime_put(phy->dev);

same here.
> +
> +	return 0;
> +}
> +
> +static struct phy_ops ops = {
> +	.init		= dm816x_usb_phy_init,
> +	.power_on	= dm816x_usb_phy_power_on,
> +	.power_off	= dm816x_usb_phy_power_off,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static int dm816x_usb_phy_runtime_suspend(struct device *dev)
> +{
> +	struct dm816x_usb_phy *phy = dev_get_drvdata(dev);
> +	unsigned int mask, val;
> +	int error = 0;
> +
> +	mask = BIT(phy->instance);
> +	val = ~BIT(phy->instance);
> +	error = regmap_update_bits(phy->syscon, phy->usb_ctrl,
> +				   mask, val);
> +	if (error)
> +		dev_err(phy->dev, "phy%i failed to power off\n",
> +			phy->instance);
> +	clk_disable(phy->refclk);
> +
> +	return 0;
> +}
> +
> +static int dm816x_usb_phy_runtime_resume(struct device *dev)
> +{
> +	struct dm816x_usb_phy *phy = dev_get_drvdata(dev);
> +	unsigned int mask, val;
> +	int error;
> +
> +	error = clk_enable(phy->refclk);
> +	if (error)
> +		return error;
> +
> +	mask = BIT(phy->instance);
> +	val = BIT(phy->instance);
> +	error = regmap_update_bits(phy->syscon, phy->usb_ctrl,
> +				   mask, val);
> +	if (error) {
> +		dev_err(phy->dev, "phy%i failed to power on\n",
> +			phy->instance);
> +		clk_disable(phy->refclk);
> +		return error;
> +	}
> +
> +	return 0;
> +}
> +
> +static UNIVERSAL_DEV_PM_OPS(dm816x_usb_phy_pm_ops,
> +			    dm816x_usb_phy_runtime_suspend,
> +			    dm816x_usb_phy_runtime_resume,
> +			    NULL);
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id dm816x_usb_phy_id_table[] = {
> +	{
> +		.compatible = "ti,dm8168-usb-phy",
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, dm816x_usb_phy_id_table);
> +#endif
> +
> +static int dm816x_usb_phy_probe(struct platform_device *pdev)
> +{
> +	struct dm816x_usb_phy *phy;
> +	struct resource *res;
> +	struct phy *generic_phy;
> +	struct phy_provider *phy_provider;
> +	struct usb_otg *otg;
> +	const struct of_device_id *of_id;
> +	const struct usb_phy_data *phy_data;
> +	int error;
> +
> +	of_id = of_match_device(of_match_ptr(dm816x_usb_phy_id_table),
> +				&pdev->dev);
> +	if (!of_id)
> +		return -EINVAL;
> +
> +	phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
> +	if (!phy)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return -ENOENT;
> +
> +	phy->syscon = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> +						      "syscon");
> +	if (IS_ERR(phy->syscon))
> +		return PTR_ERR(phy->syscon);
> +
> +	/*
> +	 * According to sprs614e.pdf, the first usb_ctrl is shared and
> +	 * the second instance for usb_ctrl is reserved.. Also the
> +	 * register bits are different from earlier TRMs.
> +	 */
> +	phy->usb_ctrl = 0x20;
> +	phy->usbphy_ctrl = (res->start & 0xff) + 4;
> +	if (phy->usbphy_ctrl == 0x2c)
> +		phy->instance = 1;
> +
> +	phy_data = of_id->data;
> +
> +	otg = devm_kzalloc(&pdev->dev, sizeof(*otg), GFP_KERNEL);
> +	if (!otg)
> +		return -ENOMEM;
> +
> +	phy->dev = &pdev->dev;
> +	phy->phy.dev = phy->dev;
> +	phy->phy.label = "dm8168_usb_phy";
> +	phy->phy.otg = otg;
> +	phy->phy.type = USB_PHY_TYPE_USB2;
> +	otg->set_host = dm816x_usb_phy_set_host;
> +	otg->set_peripheral = dm816x_usb_phy_set_peripheral;
> +	otg->usb_phy = &phy->phy;
> +
> +	platform_set_drvdata(pdev, phy);
> +
> +	phy->refclk = devm_clk_get(phy->dev, "refclk");
> +	if (IS_ERR(phy->refclk))
> +		return PTR_ERR(phy->refclk);
> +
> +	generic_phy = devm_phy_create(phy->dev, NULL, &ops);
> +	if (IS_ERR(generic_phy))
> +		return PTR_ERR(generic_phy);

Just invoke pm_runtime_enable before phy_create and phy core will take care of
invoking all pm_runtime functions at appropriate time.

Cheers
Kishon

Re: [PATCH] phy: Add a driver for dm816x USB PHY

[Tony Lindgren]

* Kishon Vijay Abraham I <kishon@ti.com> [150311 02:58]:
> Hi Tony,
> 
> On Tuesday 10 March 2015 02:21 AM, Tony Lindgren wrote:
> >Add a minimal driver for dm816x USB. Otherwise we can just use
> >the existing musb_am335x and musb_dsps on dm816x.
> 
> If we can use an existing driver, I'd prefer that.

Hmm that needs rewording.. Should say that with this phy
driver musb works with existing musb_dsps usb driver on
dm816x. There is no existing driver, the closest similar
thing is the legacy drivers/usb/musb/davinci.c that has
no separate phy driver.

> >+static int dm816x_usb_phy_power_off(struct phy *x)
> >+{
> >+	struct dm816x_usb_phy *phy = phy_get_drvdata(x);
> >+
> >+	pm_runtime_put(phy->dev);
> 
> phy core takes care of invoking pm_runtime_put on power_off.
> So this function shouldn't be needed at all.

OK will remove the pm_runtime calls for all of them.

...

> >+	phy->refclk = devm_clk_get(phy->dev, "refclk");
> >+	if (IS_ERR(phy->refclk))
> >+		return PTR_ERR(phy->refclk);
> >+
> >+	generic_phy = devm_phy_create(phy->dev, NULL, &ops);
> >+	if (IS_ERR(generic_phy))
> >+		return PTR_ERR(generic_phy);
> 
> Just invoke pm_runtime_enable before phy_create and phy core will take care of
> invoking all pm_runtime functions at appropriate time.

OK will.

Thanks,

Tony

Re: [PATCH] phy: Add a driver for dm816x USB PHY

[Paul Bolle]

On Mon, 2015-03-09 at 13:51 -0700, Tony Lindgren wrote:
> --- /dev/null
> +++ b/drivers/phy/phy-dm816x-usb.c
> @@ -0,0 +1,295 @@
> +/*
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */

This states the license is GPL v2 or later.

> +MODULE_LICENSE("GPL v2");

So you probably want
    MODULE_LICENSE("GPL");

here.


Paul Bolle

Re: [PATCH] phy: Add a driver for dm816x USB PHY

[Tony Lindgren]

* Paul Bolle <pebolle@tiscali.nl> [150311 04:16]:
> On Mon, 2015-03-09 at 13:51 -0700, Tony Lindgren wrote:
> > --- /dev/null
> > +++ b/drivers/phy/phy-dm816x-usb.c
> > @@ -0,0 +1,295 @@
> > +/*
> > + * 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.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> 
> This states the license is GPL v2 or later.
> 
> > +MODULE_LICENSE("GPL v2");
> 
> So you probably want
>     MODULE_LICENSE("GPL");
> 
> here.

Oh, it should be just GPL v2 like most of the kernel. Probably copied
the header from some other phy driver, will update that instead.

Thanks,

Tony

Re: [PATCH] phy: Add a driver for dm816x USB PHY

[Rusty Russell]

Tony Lindgren <tony@atomide.com> writes:
> * Paul Bolle <pebolle@tiscali.nl> [150311 04:16]:
> Oh, it should be just GPL v2 like most of the kernel. Probably copied
> the header from some other phy driver, will update that instead.

Well, all my code is explicitly v2 or later.

I'll leave it someone else to try to figure out the stats.  I vote for
Jon!

Cheers,
Rusty.

Re: [PATCH] phy: Add a driver for dm816x USB PHY

[Tony Lindgren]

* Rusty Russell <rusty@rustcorp.com.au> [150311 18:15]:
> Tony Lindgren <tony@atomide.com> writes:
> > * Paul Bolle <pebolle@tiscali.nl> [150311 04:16]:
> > Oh, it should be just GPL v2 like most of the kernel. Probably copied
> > the header from some other phy driver, will update that instead.
> 
> Well, all my code is explicitly v2 or later.
> 
> I'll leave it someone else to try to figure out the stats.  I vote for
> Jon!

It seems I nowadays prefer a mix of GPLv2 with just a sprinkle of the
later! Keeps people guessing you know :)

Tony