[PATCH 0/4] usb: phy: add usb3 phy driver

[PATCH 0/4] usb: phy: add usb3 phy driver

[Kishon Vijay Abraham I]

Added a driver for usb3 phy that handles the interaction between usb phy
device and dwc3 controller. Currently writing to control module register
is taken care in this driver which will be removed once the control
module driver is in place.

Also included the patch from Moiz to fix gadget pullup in SS mode.

This patch series has been lying in my local tree for some time and it's
been tested in my tree.
git://gitorious.org/linux-usb/linux-usb.git dwc3

This patch series was rebased on
git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git xceiv
However the dependent modules are not yet upstreamed and hence only compile
tested in this tree

Kishon Vijay Abraham I (3):
  usb: phy: add a new driver for usb3 phy
  usb: phy: omap-usb3: Decrease the number of transitions to recovery
  usb: phy: omap-usb2: enable 960Mhz clock for omap5

Moiz Sonasath (1):
  usb: dwc3: Fix gadget pullup in SS mode

 Documentation/devicetree/bindings/usb/usb-phy.txt |   21 ++
 drivers/usb/dwc3/gadget.c                         |   21 +-
 drivers/usb/phy/Kconfig                           |    9 +
 drivers/usb/phy/Makefile                          |    1 +
 drivers/usb/phy/omap-usb2.c                       |   28 +-
 drivers/usb/phy/omap-usb3.c                       |  416 +++++++++++++++++++++
 include/linux/usb/omap_usb.h                      |   73 ++++
 include/linux/usb/phy.h                           |   10 +-
 8 files changed, 571 insertions(+), 8 deletions(-)
 create mode 100644 drivers/usb/phy/omap-usb3.c

-- 
1.7.9.5

[PATCH 1/4] usb: phy: add a new driver for usb3 phy

[Kishon Vijay Abraham I]

Added a driver for usb3 phy that handles the interaction between usb phy
device and dwc3 controller.

This also includes device tree support for usb3 phy driver and
the documentation with device tree binding information is updated.

Currently writing to control module register is taken care in this
driver which will be removed once the control module driver is in place.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Signed-off-by: Moiz Sonasath <m-sonasath@ti.com>
---
 Documentation/devicetree/bindings/usb/usb-phy.txt |   18 +
 drivers/usb/phy/Kconfig                           |    9 +
 drivers/usb/phy/Makefile                          |    1 +
 drivers/usb/phy/omap-usb3.c                       |  369 +++++++++++++++++++++
 include/linux/usb/omap_usb.h                      |   72 ++++
 5 files changed, 469 insertions(+)
 create mode 100644 drivers/usb/phy/omap-usb3.c

diff --git a/Documentation/devicetree/bindings/usb/usb-phy.txt b/Documentation/devicetree/bindings/usb/usb-phy.txt
index 80d4148..7c5fd89 100644
--- a/Documentation/devicetree/bindings/usb/usb-phy.txt
+++ b/Documentation/devicetree/bindings/usb/usb-phy.txt
@@ -15,3 +15,21 @@ usb2phy@4a0ad080 {
 	reg = <0x4a0ad080 0x58>,
 	      <0x4a002300 0x4>;
 };
+
+OMAP USB3 PHY
+
+Required properties:
+ - compatible: Should be "ti,omap-usb3"
+ - reg : Address and length of the register set for the device. Also
+add the address of control module phy power register until a driver for
+control module is added
+
+This is usually a subnode of ocp2scp to which it is connected.
+
+usb3phy@4a084400 {
+	compatible = "ti,omap-usb3";
+	reg = <0x0x4a084400 0x80>,
+	      <0x4a084800 0x64>,
+	      <0x4a084c00 0x40>,
+	      <0x4a002370 0x4>;
+};
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 63c339b..353dc0c 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -13,6 +13,15 @@ config OMAP_USB2
 	  The USB OTG controller communicates with the comparator using this
 	  driver.
 
+config OMAP_USB3
+	tristate "OMAP USB3 PHY Driver"
+	select USB_OTG_UTILS
+	help
+	  Enable this to support the USB3 PHY that is part of SOC. This
+	  driver takes care of all the PHY functionality apart from comparator.
+	  The USB OTG controller communicates with the comparator using this
+	  driver.
+
 config USB_ISP1301
 	tristate "NXP ISP1301 USB transceiver support"
 	depends on USB || USB_GADGET
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index b069f29..973b1e6 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -5,6 +5,7 @@
 ccflags-$(CONFIG_USB_DEBUG) := -DDEBUG
 
 obj-$(CONFIG_OMAP_USB2)			+= omap-usb2.o
+obj-$(CONFIG_OMAP_USB3)			+= omap-usb3.o
 obj-$(CONFIG_USB_ISP1301)		+= isp1301.o
 obj-$(CONFIG_MV_U3D_PHY)		+= mv_u3d_phy.o
 obj-$(CONFIG_USB_EHCI_TEGRA)	+= tegra_usb_phy.o
diff --git a/drivers/usb/phy/omap-usb3.c b/drivers/usb/phy/omap-usb3.c
new file mode 100644
index 0000000..4dc84ca
--- /dev/null
+++ b/drivers/usb/phy/omap-usb3.c
@@ -0,0 +1,369 @@
+/*
+ * omap-usb3 - USB PHY, talking to dwc3 controller in OMAP.
+ *
+ * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com
+ * 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.
+ *
+ * Author: Kishon Vijay Abraham I <kishon@ti.com>
+ *
+ * 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/slab.h>
+#include <linux/usb/omap_usb.h>
+#include <linux/of.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/pm_runtime.h>
+#include <linux/delay.h>
+
+static struct usb_dpll_params omap_usb3_dpll_params[NUM_SYS_CLKS] = {
+	{1250, 5, 4, 20, 0},		/* 12 MHz */
+	{3125, 20, 4, 20, 0},		/* 16.8 MHz */
+	{1172, 8, 4, 20, 65537},	/* 19.2 MHz */
+	{1250, 12, 4, 20, 0},		/* 26 MHz */
+	{3125, 47, 4, 20, 92843},	/* 38.4 MHz */
+};
+
+/**
+ * omap5_usb_phy_power - power on/off the serializer using control module
+ * @phy: struct omap_usb *
+ * @on: 0 to off and 1 to on based on powering on or off the PHY
+ *
+ * omap_usb3 can call this API to power on or off the PHY.
+ */
+static int omap5_usb_phy_power(struct omap_usb *phy, bool on)
+{
+	u32 val;
+	unsigned long rate;
+	struct clk *sys_clk;
+
+	sys_clk = clk_get(NULL, "sys_clkin");
+	if (IS_ERR(sys_clk)) {
+		pr_err("%s: unable to get sys_clkin\n", __func__);
+		return -EINVAL;
+	}
+
+	rate = clk_get_rate(sys_clk);
+	rate = rate/1000000;
+
+	val = readl(phy->control_dev);
+
+	if (on) {
+		val &= ~(USB_PWRCTL_CLK_CMD_MASK | USB_PWRCTL_CLK_FREQ_MASK);
+		val |= USB3_PHY_TX_RX_POWERON << USB_PWRCTL_CLK_CMD_SHIFT;
+		val |= rate << USB_PWRCTL_CLK_FREQ_SHIFT;
+	} else {
+		val &= ~USB_PWRCTL_CLK_CMD_MASK;
+		val |= USB3_PHY_TX_RX_POWEROFF << USB_PWRCTL_CLK_CMD_SHIFT;
+	}
+
+	writel(val, phy->control_dev);
+
+	return 0;
+}
+
+static int omap_usb3_suspend(struct usb_phy *x, int suspend)
+{
+	struct omap_usb *phy = phy_to_omapusb(x);
+	int	val, ret;
+	int timeout = PLL_IDLE_TIME;
+
+	if (suspend && !phy->is_suspended) {
+		val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION2);
+		val |= PLL_IDLE;
+		omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION2, val);
+
+		do {
+			val = omap_usb_readl(phy->pll_ctrl_base, PLL_STATUS);
+			if (val & PLL_TICOPWDN)
+				break;
+			udelay(1);
+		} while (--timeout);
+
+		omap5_usb_phy_power(phy, 0);
+		pm_runtime_put_sync(phy->dev);
+
+		phy->is_suspended	= 1;
+	} else if (!suspend && phy->is_suspended) {
+		phy->is_suspended	= 0;
+		ret = pm_runtime_get_sync(phy->dev);
+		if (ret < 0) {
+			dev_err(phy->dev, "get_sync failed with err %d\n",
+									ret);
+			return ret;
+		}
+
+		val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION2);
+		val &= ~PLL_IDLE;
+		omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION2, val);
+
+		do {
+			val = omap_usb_readl(phy->pll_ctrl_base, PLL_STATUS);
+			if (!(val & PLL_TICOPWDN))
+				break;
+			udelay(1);
+		} while (--timeout);
+	}
+
+	return 0;
+}
+
+static inline enum sys_clk_rate __get_sys_clk_index(unsigned long rate)
+{
+	switch (rate) {
+	case 12000000:
+		return CLK_RATE_12MHZ;
+	case 16800000:
+		return CLK_RATE_16MHZ;
+	case 19200000:
+		return CLK_RATE_19MHZ;
+	case 26000000:
+		return CLK_RATE_26MHZ;
+	case 38400000:
+		return CLK_RATE_38MHZ;
+	default:
+		return CLK_RATE_UNDEFINED;
+	}
+}
+
+static void omap_usb_dpll_relock(struct omap_usb *phy)
+{
+	u32		val;
+	unsigned long	timeout;
+
+	omap_usb_writel(phy->pll_ctrl_base, PLL_GO, SET_PLL_GO);
+
+	timeout = jiffies + msecs_to_jiffies(20);
+	do {
+		val = omap_usb_readl(phy->pll_ctrl_base, PLL_STATUS);
+		if (val & PLL_LOCK)
+			break;
+	} while (!WARN_ON(time_after(jiffies, timeout)));
+}
+
+static int omap_usb_dpll_lock(struct omap_usb *phy)
+{
+	u32			val;
+	struct clk		*sys_clk;
+	unsigned long		rate;
+	enum sys_clk_rate	clk_index;
+
+	sys_clk	= clk_get(NULL, "sys_clkin");
+	if (IS_ERR(sys_clk)) {
+		pr_err("unable to get sys_clkin\n");
+		return PTR_ERR(sys_clk);
+	}
+
+	rate		= clk_get_rate(sys_clk);
+	clk_index	= __get_sys_clk_index(rate);
+
+	if (clk_index == CLK_RATE_UNDEFINED) {
+		pr_err("dpll cannot be locked for sys clk freq:%luHz\n", rate);
+		return -EINVAL;
+	}
+
+	val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION1);
+	val &= ~PLL_REGN_MASK;
+	val |= omap_usb3_dpll_params[clk_index].n << PLL_REGN_SHIFT;
+	omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION1, val);
+
+	val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION2);
+	val &= ~PLL_SELFREQDCO_MASK;
+	val |= omap_usb3_dpll_params[clk_index].freq << PLL_SELFREQDCO_SHIFT;
+	omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION2, val);
+
+	val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION1);
+	val &= ~PLL_REGM_MASK;
+	val |= omap_usb3_dpll_params[clk_index].m << PLL_REGM_SHIFT;
+	omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION1, val);
+
+	val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION4);
+	val &= ~PLL_REGM_F_MASK;
+	val |= omap_usb3_dpll_params[clk_index].mf << PLL_REGM_F_SHIFT;
+	omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION4, val);
+
+	val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION3);
+	val &= ~PLL_SD_MASK;
+	val |= omap_usb3_dpll_params[clk_index].sd << PLL_SD_SHIFT;
+	omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION3, val);
+
+	omap_usb_dpll_relock(phy);
+
+	return 0;
+}
+
+static int omap_usb3_init(struct usb_phy *x)
+{
+	struct omap_usb	*phy = phy_to_omapusb(x);
+
+	omap_usb_dpll_lock(phy);
+	omap5_usb_phy_power(phy, 1);
+
+	return 0;
+}
+
+static int __devinit omap_usb3_probe(struct platform_device *pdev)
+{
+	struct omap_usb			*phy;
+	struct resource			*res;
+
+	phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
+	if (!phy) {
+		dev_err(&pdev->dev, "unable to alloc mem for OMAP USB3 PHY\n");
+		return -ENOMEM;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+	if (!res) {
+		dev_err(&pdev->dev, "unable to get base address of pll_ctrl\n");
+		return -ENODEV;
+	}
+
+	phy->pll_ctrl_base = devm_request_and_ioremap(&pdev->dev, res);
+	if (!phy->pll_ctrl_base) {
+		dev_err(&pdev->dev, "ioremap of pll_ctrl failed\n");
+		return -ENOMEM;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 3);
+	if (!res) {
+		dev_err(&pdev->dev,
+			"unable to get address of control phy power\n");
+		return -ENODEV;
+	}
+
+	phy->control_dev = devm_request_and_ioremap(&pdev->dev, res);
+	if (!phy->control_dev) {
+		dev_err(&pdev->dev, "ioremap of control_dev failed\n");
+		return -ENOMEM;
+	}
+
+	phy->dev		= &pdev->dev;
+
+	phy->phy.dev		= phy->dev;
+	phy->phy.label		= "omap-usb3";
+	phy->phy.init		= omap_usb3_init;
+	phy->phy.set_suspend	= omap_usb3_suspend;
+
+	phy->is_suspended	= 1;
+	omap5_usb_phy_power(phy, 0);
+
+	phy->wkupclk = devm_clk_get(phy->dev, "usb_phy_cm_clk32k");
+	if (IS_ERR(phy->wkupclk)) {
+		dev_err(&pdev->dev, "unable to get usb_phy_cm_clk32k\n");
+		return PTR_ERR(phy->wkupclk);
+	}
+	clk_prepare(phy->wkupclk);
+
+	phy->optclk = devm_clk_get(phy->dev, "usb_otg_ss_refclk960m");
+	if (IS_ERR(phy->optclk)) {
+		dev_err(&pdev->dev, "unable to get usb_otg_ss_refclk960m\n");
+		return PTR_ERR(phy->optclk);
+	}
+	clk_prepare(phy->optclk);
+
+	usb_add_phy(&phy->phy, USB_PHY_TYPE_USB3);
+
+	platform_set_drvdata(pdev, phy);
+
+	pm_runtime_enable(phy->dev);
+
+	return 0;
+}
+
+static int __devexit omap_usb3_remove(struct platform_device *pdev)
+{
+	struct omap_usb *phy = platform_get_drvdata(pdev);
+
+	clk_unprepare(phy->wkupclk);
+	clk_unprepare(phy->optclk);
+	usb_remove_phy(&phy->phy);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_RUNTIME
+
+static int omap_usb3_runtime_suspend(struct device *dev)
+{
+	struct platform_device	*pdev = to_platform_device(dev);
+	struct omap_usb	*phy = platform_get_drvdata(pdev);
+
+	clk_disable(phy->wkupclk);
+	clk_disable(phy->optclk);
+
+	return 0;
+}
+
+static int omap_usb3_runtime_resume(struct device *dev)
+{
+	u32 ret = 0;
+	struct platform_device	*pdev = to_platform_device(dev);
+	struct omap_usb	*phy = platform_get_drvdata(pdev);
+
+	ret = clk_enable(phy->optclk);
+	if (ret) {
+		dev_err(phy->dev, "Failed to enable optclk %d\n", ret);
+		goto err1;
+	}
+
+	ret = clk_enable(phy->wkupclk);
+	if (ret) {
+		dev_err(phy->dev, "Failed to enable wkupclk %d\n", ret);
+		goto err2;
+	}
+
+	return 0;
+
+err2:
+	clk_disable(phy->optclk);
+
+err1:
+	return ret;
+}
+
+static const struct dev_pm_ops omap_usb3_pm_ops = {
+	SET_RUNTIME_PM_OPS(omap_usb3_runtime_suspend, omap_usb3_runtime_resume,
+		NULL)
+};
+
+#define DEV_PM_OPS     (&omap_usb3_pm_ops)
+#else
+#define DEV_PM_OPS     NULL
+#endif
+
+#ifdef CONFIG_OF
+static const struct of_device_id omap_usb3_id_table[] = {
+	{ .compatible = "ti,omap-usb3" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, omap_usb3_id_table);
+#endif
+
+static struct platform_driver omap_usb3_driver = {
+	.probe		= omap_usb3_probe,
+	.remove		= __devexit_p(omap_usb3_remove),
+	.driver		= {
+		.name	= "omap-usb3",
+		.owner	= THIS_MODULE,
+		.pm	= DEV_PM_OPS,
+		.of_match_table = of_match_ptr(omap_usb3_id_table),
+	},
+};
+
+module_platform_driver(omap_usb3_driver);
+
+MODULE_ALIAS("platform: omap_usb3");
+MODULE_AUTHOR("Texas Instruments Inc.");
+MODULE_DESCRIPTION("OMAP USB3 phy driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/usb/omap_usb.h b/include/linux/usb/omap_usb.h
index 0ea17f8..01ed008 100644
--- a/include/linux/usb/omap_usb.h
+++ b/include/linux/usb/omap_usb.h
@@ -19,19 +19,80 @@
 #ifndef __DRIVERS_OMAP_USB2_H
 #define __DRIVERS_OMAP_USB2_H
 
+#include <linux/io.h>
 #include <linux/usb/otg.h>
 
+struct usb_dpll_params {
+	u16	m;
+	u8	n;
+	u8	freq:3;
+	u8	sd;
+	u32	mf;
+};
+
+enum sys_clk_rate {
+	CLK_RATE_UNDEFINED = -1,
+	CLK_RATE_12MHZ,
+	CLK_RATE_16MHZ,
+	CLK_RATE_19MHZ,
+	CLK_RATE_26MHZ,
+	CLK_RATE_38MHZ
+};
+
+#define	NUM_SYS_CLKS		5
+#define	PLL_STATUS		0x00000004
+#define	PLL_GO			0x00000008
+#define	PLL_CONFIGURATION1	0x0000000C
+#define	PLL_CONFIGURATION2	0x00000010
+#define	PLL_CONFIGURATION3	0x00000014
+#define	PLL_CONFIGURATION4	0x00000020
+
+#define	PLL_REGM_MASK		0x001FFE00
+#define	PLL_REGM_SHIFT		0x9
+#define	PLL_REGM_F_MASK		0x0003FFFF
+#define	PLL_REGM_F_SHIFT	0x0
+#define	PLL_REGN_MASK		0x000001FE
+#define	PLL_REGN_SHIFT		0x1
+#define	PLL_SELFREQDCO_MASK	0x0000000E
+#define	PLL_SELFREQDCO_SHIFT	0x1
+#define	PLL_SD_MASK		0x0003FC00
+#define	PLL_SD_SHIFT		0x9
+#define	SET_PLL_GO		0x1
+#define	PLL_TICOPWDN		0x10000
+#define	PLL_LOCK		0x2
+#define	PLL_IDLE		0x1
+
+/*
+ * This is an Empirical value that works, need to confirm the actual
+ * value required for the USB3PHY_PLL_CONFIGURATION2.PLL_IDLE status
+ * to be correctly reflected in the USB3PHY_PLL_STATUS register.
+ */
+# define PLL_IDLE_TIME  100;
+
 struct omap_usb {
 	struct usb_phy		phy;
 	struct phy_companion	*comparator;
+	void __iomem		*pll_ctrl_base;
 	struct device		*dev;
 	u32 __iomem		*control_dev;
 	struct clk		*wkupclk;
+	struct clk		*optclk;
 	u8			is_suspended:1;
 };
 
 #define	PHY_PD	0x1
 
+#define	CONTROL_PHY_POWER_USB		0x00000370
+
+#define	USB_PWRCTL_CLK_CMD_MASK		0x003FC000
+#define	USB_PWRCTL_CLK_CMD_SHIFT	0xE
+
+#define	USB_PWRCTL_CLK_FREQ_MASK	0xFFC00000
+#define	USB_PWRCTL_CLK_FREQ_SHIFT	0x16
+
+#define	USB3_PHY_TX_RX_POWERON		0x3
+#define	USB3_PHY_TX_RX_POWEROFF		0x0
+
 #define	phy_to_omapusb(x)	container_of((x), struct omap_usb, phy)
 
 #if defined(CONFIG_OMAP_USB2) || defined(CONFIG_OMAP_USB2_MODULE)
@@ -43,4 +104,15 @@ static inline int omap_usb2_set_comparator(struct phy_companion *comparator)
 }
 #endif
 
+static inline u32 omap_usb_readl(const void __iomem *addr, unsigned offset)
+{
+	return __raw_readl(addr + offset);
+}
+
+static inline void omap_usb_writel(const void __iomem *addr, unsigned offset,
+								u32 data)
+{
+	__raw_writel(data, addr + offset);
+}
+
 #endif /* __DRIVERS_OMAP_USB_H */
-- 
1.7.9.5

Re: [PATCH 1/4] usb: phy: add a new driver for usb3 phy

[Marc Kleine-Budde]

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

On 09/19/2012 01:30 PM, Kishon Vijay Abraham I wrote:
> Added a driver for usb3 phy that handles the interaction between usb phy
> device and dwc3 controller.
> 
> This also includes device tree support for usb3 phy driver and
> the documentation with device tree binding information is updated.
> 
> Currently writing to control module register is taken care in this
> driver which will be removed once the control module driver is in place.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> Signed-off-by: Moiz Sonasath <m-sonasath@ti.com>
> ---
>  Documentation/devicetree/bindings/usb/usb-phy.txt |   18 +
>  drivers/usb/phy/Kconfig                           |    9 +
>  drivers/usb/phy/Makefile                          |    1 +
>  drivers/usb/phy/omap-usb3.c                       |  369 +++++++++++++++++++++
>  include/linux/usb/omap_usb.h                      |   72 ++++
>  5 files changed, 469 insertions(+)
>  create mode 100644 drivers/usb/phy/omap-usb3.c
> 
> diff --git a/Documentation/devicetree/bindings/usb/usb-phy.txt b/Documentation/devicetree/bindings/usb/usb-phy.txt
> index 80d4148..7c5fd89 100644
> --- a/Documentation/devicetree/bindings/usb/usb-phy.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-phy.txt
> @@ -15,3 +15,21 @@ usb2phy@4a0ad080 {
>  	reg = <0x4a0ad080 0x58>,
>  	      <0x4a002300 0x4>;
>  };
> +
> +OMAP USB3 PHY
> +
> +Required properties:
> + - compatible: Should be "ti,omap-usb3"
> + - reg : Address and length of the register set for the device. Also
> +add the address of control module phy power register until a driver for
> +control module is added
> +
> +This is usually a subnode of ocp2scp to which it is connected.
> +
> +usb3phy@4a084400 {
> +	compatible = "ti,omap-usb3";
> +	reg = <0x0x4a084400 0x80>,
> +	      <0x4a084800 0x64>,
> +	      <0x4a084c00 0x40>,
> +	      <0x4a002370 0x4>;
> +};
> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index 63c339b..353dc0c 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -13,6 +13,15 @@ config OMAP_USB2
>  	  The USB OTG controller communicates with the comparator using this
>  	  driver.
>  
> +config OMAP_USB3
> +	tristate "OMAP USB3 PHY Driver"
> +	select USB_OTG_UTILS
> +	help
> +	  Enable this to support the USB3 PHY that is part of SOC. This
> +	  driver takes care of all the PHY functionality apart from comparator.
> +	  The USB OTG controller communicates with the comparator using this
> +	  driver.
> +
>  config USB_ISP1301
>  	tristate "NXP ISP1301 USB transceiver support"
>  	depends on USB || USB_GADGET
> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
> index b069f29..973b1e6 100644
> --- a/drivers/usb/phy/Makefile
> +++ b/drivers/usb/phy/Makefile
> @@ -5,6 +5,7 @@
>  ccflags-$(CONFIG_USB_DEBUG) := -DDEBUG
>  
>  obj-$(CONFIG_OMAP_USB2)			+= omap-usb2.o
> +obj-$(CONFIG_OMAP_USB3)			+= omap-usb3.o
>  obj-$(CONFIG_USB_ISP1301)		+= isp1301.o
>  obj-$(CONFIG_MV_U3D_PHY)		+= mv_u3d_phy.o
>  obj-$(CONFIG_USB_EHCI_TEGRA)	+= tegra_usb_phy.o
> diff --git a/drivers/usb/phy/omap-usb3.c b/drivers/usb/phy/omap-usb3.c
> new file mode 100644
> index 0000000..4dc84ca
> --- /dev/null
> +++ b/drivers/usb/phy/omap-usb3.c
> @@ -0,0 +1,369 @@
> +/*
> + * omap-usb3 - USB PHY, talking to dwc3 controller in OMAP.
> + *
> + * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com
> + * 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.
> + *
> + * Author: Kishon Vijay Abraham I <kishon@ti.com>
> + *
> + * 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/slab.h>
> +#include <linux/usb/omap_usb.h>
> +#include <linux/of.h>
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/delay.h>
> +
> +static struct usb_dpll_params omap_usb3_dpll_params[NUM_SYS_CLKS] = {
> +	{1250, 5, 4, 20, 0},		/* 12 MHz */
> +	{3125, 20, 4, 20, 0},		/* 16.8 MHz */
> +	{1172, 8, 4, 20, 65537},	/* 19.2 MHz */
> +	{1250, 12, 4, 20, 0},		/* 26 MHz */
> +	{3125, 47, 4, 20, 92843},	/* 38.4 MHz */
> +};
> +
> +/**
> + * omap5_usb_phy_power - power on/off the serializer using control module
> + * @phy: struct omap_usb *
> + * @on: 0 to off and 1 to on based on powering on or off the PHY
> + *
> + * omap_usb3 can call this API to power on or off the PHY.
> + */
> +static int omap5_usb_phy_power(struct omap_usb *phy, bool on)
> +{
> +	u32 val;
> +	unsigned long rate;
> +	struct clk *sys_clk;
> +
> +	sys_clk = clk_get(NULL, "sys_clkin");

Where's the corresponding clk_put()?

> +	if (IS_ERR(sys_clk)) {
> +		pr_err("%s: unable to get sys_clkin\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	rate = clk_get_rate(sys_clk);
> +	rate = rate/1000000;
> +
> +	val = readl(phy->control_dev);
> +
> +	if (on) {
> +		val &= ~(USB_PWRCTL_CLK_CMD_MASK | USB_PWRCTL_CLK_FREQ_MASK);
> +		val |= USB3_PHY_TX_RX_POWERON << USB_PWRCTL_CLK_CMD_SHIFT;
> +		val |= rate << USB_PWRCTL_CLK_FREQ_SHIFT;
> +	} else {
> +		val &= ~USB_PWRCTL_CLK_CMD_MASK;
> +		val |= USB3_PHY_TX_RX_POWEROFF << USB_PWRCTL_CLK_CMD_SHIFT;
> +	}
> +
> +	writel(val, phy->control_dev);
> +
> +	return 0;
> +}
> +
> +static int omap_usb3_suspend(struct usb_phy *x, int suspend)
> +{
> +	struct omap_usb *phy = phy_to_omapusb(x);
> +	int	val, ret;
> +	int timeout = PLL_IDLE_TIME;
> +
> +	if (suspend && !phy->is_suspended) {
> +		val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION2);
> +		val |= PLL_IDLE;
> +		omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION2, val);
> +
> +		do {
> +			val = omap_usb_readl(phy->pll_ctrl_base, PLL_STATUS);
> +			if (val & PLL_TICOPWDN)
> +				break;
> +			udelay(1);
> +		} while (--timeout);
> +
> +		omap5_usb_phy_power(phy, 0);
> +		pm_runtime_put_sync(phy->dev);
> +
> +		phy->is_suspended	= 1;
> +	} else if (!suspend && phy->is_suspended) {
> +		phy->is_suspended	= 0;
> +		ret = pm_runtime_get_sync(phy->dev);
> +		if (ret < 0) {
> +			dev_err(phy->dev, "get_sync failed with err %d\n",
> +									ret);
> +			return ret;
> +		}
> +
> +		val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION2);
> +		val &= ~PLL_IDLE;
> +		omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION2, val);
> +
> +		do {
> +			val = omap_usb_readl(phy->pll_ctrl_base, PLL_STATUS);
> +			if (!(val & PLL_TICOPWDN))
> +				break;
> +			udelay(1);
> +		} while (--timeout);
> +	}
> +
> +	return 0;
> +}
> +
> +static inline enum sys_clk_rate __get_sys_clk_index(unsigned long rate)
> +{
> +	switch (rate) {
> +	case 12000000:
> +		return CLK_RATE_12MHZ;
> +	case 16800000:
> +		return CLK_RATE_16MHZ;
> +	case 19200000:
> +		return CLK_RATE_19MHZ;
> +	case 26000000:
> +		return CLK_RATE_26MHZ;
> +	case 38400000:
> +		return CLK_RATE_38MHZ;
> +	default:
> +		return CLK_RATE_UNDEFINED;
> +	}
> +}
> +
> +static void omap_usb_dpll_relock(struct omap_usb *phy)
> +{
> +	u32		val;
> +	unsigned long	timeout;
> +
> +	omap_usb_writel(phy->pll_ctrl_base, PLL_GO, SET_PLL_GO);
> +
> +	timeout = jiffies + msecs_to_jiffies(20);
> +	do {
> +		val = omap_usb_readl(phy->pll_ctrl_base, PLL_STATUS);
> +		if (val & PLL_LOCK)
> +			break;
> +	} while (!WARN_ON(time_after(jiffies, timeout)));
> +}
> +
> +static int omap_usb_dpll_lock(struct omap_usb *phy)
> +{
> +	u32			val;
> +	struct clk		*sys_clk;
> +	unsigned long		rate;
> +	enum sys_clk_rate	clk_index;
> +
> +	sys_clk	= clk_get(NULL, "sys_clkin");

Where's the corresponding clk_put()?

> +	if (IS_ERR(sys_clk)) {
> +		pr_err("unable to get sys_clkin\n");
> +		return PTR_ERR(sys_clk);
> +	}
> +
> +	rate		= clk_get_rate(sys_clk);
> +	clk_index	= __get_sys_clk_index(rate);
> +
> +	if (clk_index == CLK_RATE_UNDEFINED) {
> +		pr_err("dpll cannot be locked for sys clk freq:%luHz\n", rate);
> +		return -EINVAL;
> +	}
> +
> +	val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION1);
> +	val &= ~PLL_REGN_MASK;
> +	val |= omap_usb3_dpll_params[clk_index].n << PLL_REGN_SHIFT;
> +	omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION1, val);
> +
> +	val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION2);
> +	val &= ~PLL_SELFREQDCO_MASK;
> +	val |= omap_usb3_dpll_params[clk_index].freq << PLL_SELFREQDCO_SHIFT;
> +	omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION2, val);
> +
> +	val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION1);
> +	val &= ~PLL_REGM_MASK;
> +	val |= omap_usb3_dpll_params[clk_index].m << PLL_REGM_SHIFT;
> +	omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION1, val);
> +
> +	val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION4);
> +	val &= ~PLL_REGM_F_MASK;
> +	val |= omap_usb3_dpll_params[clk_index].mf << PLL_REGM_F_SHIFT;
> +	omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION4, val);
> +
> +	val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION3);
> +	val &= ~PLL_SD_MASK;
> +	val |= omap_usb3_dpll_params[clk_index].sd << PLL_SD_SHIFT;
> +	omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION3, val);
> +
> +	omap_usb_dpll_relock(phy);
> +
> +	return 0;
> +}
> +
> +static int omap_usb3_init(struct usb_phy *x)
> +{
> +	struct omap_usb	*phy = phy_to_omapusb(x);
> +
> +	omap_usb_dpll_lock(phy);
> +	omap5_usb_phy_power(phy, 1);
> +
> +	return 0;
> +}
> +
> +static int __devinit omap_usb3_probe(struct platform_device *pdev)
> +{
> +	struct omap_usb			*phy;
> +	struct resource			*res;
> +
> +	phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
> +	if (!phy) {
> +		dev_err(&pdev->dev, "unable to alloc mem for OMAP USB3 PHY\n");
> +		return -ENOMEM;
> +	}
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> +	if (!res) {
> +		dev_err(&pdev->dev, "unable to get base address of pll_ctrl\n");
> +		return -ENODEV;
> +	}
> +
> +	phy->pll_ctrl_base = devm_request_and_ioremap(&pdev->dev, res);
> +	if (!phy->pll_ctrl_base) {
> +		dev_err(&pdev->dev, "ioremap of pll_ctrl failed\n");
> +		return -ENOMEM;
> +	}
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 3);
> +	if (!res) {
> +		dev_err(&pdev->dev,
> +			"unable to get address of control phy power\n");
> +		return -ENODEV;
> +	}
> +
> +	phy->control_dev = devm_request_and_ioremap(&pdev->dev, res);
> +	if (!phy->control_dev) {
> +		dev_err(&pdev->dev, "ioremap of control_dev failed\n");
> +		return -ENOMEM;
> +	}
> +
> +	phy->dev		= &pdev->dev;
> +
> +	phy->phy.dev		= phy->dev;
> +	phy->phy.label		= "omap-usb3";
> +	phy->phy.init		= omap_usb3_init;
> +	phy->phy.set_suspend	= omap_usb3_suspend;
> +
> +	phy->is_suspended	= 1;
> +	omap5_usb_phy_power(phy, 0);
> +
> +	phy->wkupclk = devm_clk_get(phy->dev, "usb_phy_cm_clk32k");
> +	if (IS_ERR(phy->wkupclk)) {
> +		dev_err(&pdev->dev, "unable to get usb_phy_cm_clk32k\n");
> +		return PTR_ERR(phy->wkupclk);
> +	}
> +	clk_prepare(phy->wkupclk);
> +
> +	phy->optclk = devm_clk_get(phy->dev, "usb_otg_ss_refclk960m");
> +	if (IS_ERR(phy->optclk)) {
> +		dev_err(&pdev->dev, "unable to get usb_otg_ss_refclk960m\n");
> +		return PTR_ERR(phy->optclk);
> +	}
> +	clk_prepare(phy->optclk);

Are these clocks only needed on the PM usecase?

> +
> +	usb_add_phy(&phy->phy, USB_PHY_TYPE_USB3);

Just to be sure and avoid race conditions, I would first set the drvdata.

> +
> +	platform_set_drvdata(pdev, phy);
> +
> +	pm_runtime_enable(phy->dev);
> +
> +	return 0;
> +}
> +
> +static int __devexit omap_usb3_remove(struct platform_device *pdev)
> +{
> +	struct omap_usb *phy = platform_get_drvdata(pdev);
> +
> +	clk_unprepare(phy->wkupclk);
> +	clk_unprepare(phy->optclk);
> +	usb_remove_phy(&phy->phy);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_RUNTIME
> +
> +static int omap_usb3_runtime_suspend(struct device *dev)
> +{
> +	struct platform_device	*pdev = to_platform_device(dev);
> +	struct omap_usb	*phy = platform_get_drvdata(pdev);
> +
> +	clk_disable(phy->wkupclk);
> +	clk_disable(phy->optclk);
> +
> +	return 0;
> +}
> +
> +static int omap_usb3_runtime_resume(struct device *dev)
> +{
> +	u32 ret = 0;
> +	struct platform_device	*pdev = to_platform_device(dev);
> +	struct omap_usb	*phy = platform_get_drvdata(pdev);
> +
> +	ret = clk_enable(phy->optclk);
> +	if (ret) {
> +		dev_err(phy->dev, "Failed to enable optclk %d\n", ret);
> +		goto err1;
> +	}
> +
> +	ret = clk_enable(phy->wkupclk);
> +	if (ret) {
> +		dev_err(phy->dev, "Failed to enable wkupclk %d\n", ret);
> +		goto err2;
> +	}
> +
> +	return 0;
> +
> +err2:
> +	clk_disable(phy->optclk);
> +
> +err1:
> +	return ret;
> +}
> +
> +static const struct dev_pm_ops omap_usb3_pm_ops = {
> +	SET_RUNTIME_PM_OPS(omap_usb3_runtime_suspend, omap_usb3_runtime_resume,
> +		NULL)
> +};
> +
> +#define DEV_PM_OPS     (&omap_usb3_pm_ops)
> +#else
> +#define DEV_PM_OPS     NULL
> +#endif
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id omap_usb3_id_table[] = {
> +	{ .compatible = "ti,omap-usb3" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, omap_usb3_id_table);
> +#endif
> +
> +static struct platform_driver omap_usb3_driver = {
> +	.probe		= omap_usb3_probe,
> +	.remove		= __devexit_p(omap_usb3_remove),
> +	.driver		= {
> +		.name	= "omap-usb3",
> +		.owner	= THIS_MODULE,
> +		.pm	= DEV_PM_OPS,
> +		.of_match_table = of_match_ptr(omap_usb3_id_table),
> +	},
> +};
> +
> +module_platform_driver(omap_usb3_driver);
> +
> +MODULE_ALIAS("platform: omap_usb3");
> +MODULE_AUTHOR("Texas Instruments Inc.");
> +MODULE_DESCRIPTION("OMAP USB3 phy driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/usb/omap_usb.h b/include/linux/usb/omap_usb.h
> index 0ea17f8..01ed008 100644
> --- a/include/linux/usb/omap_usb.h
> +++ b/include/linux/usb/omap_usb.h
> @@ -19,19 +19,80 @@
>  #ifndef __DRIVERS_OMAP_USB2_H
>  #define __DRIVERS_OMAP_USB2_H
>  
> +#include <linux/io.h>
>  #include <linux/usb/otg.h>

Are these structs, enums and defines needed outside of the omap-usb3.c
file? If not it's better to move these into this file.

>  
> +struct usb_dpll_params {
> +	u16	m;
> +	u8	n;
> +	u8	freq:3;
> +	u8	sd;
> +	u32	mf;
> +};
> +
> +enum sys_clk_rate {
> +	CLK_RATE_UNDEFINED = -1,
> +	CLK_RATE_12MHZ,
> +	CLK_RATE_16MHZ,
> +	CLK_RATE_19MHZ,
> +	CLK_RATE_26MHZ,
> +	CLK_RATE_38MHZ
> +};
> +
> +#define	NUM_SYS_CLKS		5
> +#define	PLL_STATUS		0x00000004
> +#define	PLL_GO			0x00000008
> +#define	PLL_CONFIGURATION1	0x0000000C
> +#define	PLL_CONFIGURATION2	0x00000010
> +#define	PLL_CONFIGURATION3	0x00000014
> +#define	PLL_CONFIGURATION4	0x00000020
> +
> +#define	PLL_REGM_MASK		0x001FFE00
> +#define	PLL_REGM_SHIFT		0x9
> +#define	PLL_REGM_F_MASK		0x0003FFFF
> +#define	PLL_REGM_F_SHIFT	0x0
> +#define	PLL_REGN_MASK		0x000001FE
> +#define	PLL_REGN_SHIFT		0x1
> +#define	PLL_SELFREQDCO_MASK	0x0000000E
> +#define	PLL_SELFREQDCO_SHIFT	0x1
> +#define	PLL_SD_MASK		0x0003FC00
> +#define	PLL_SD_SHIFT		0x9
> +#define	SET_PLL_GO		0x1
> +#define	PLL_TICOPWDN		0x10000
> +#define	PLL_LOCK		0x2
> +#define	PLL_IDLE		0x1
> +
> +/*
> + * This is an Empirical value that works, need to confirm the actual
> + * value required for the USB3PHY_PLL_CONFIGURATION2.PLL_IDLE status
> + * to be correctly reflected in the USB3PHY_PLL_STATUS register.
> + */
> +# define PLL_IDLE_TIME  100;
> +
>  struct omap_usb {
>  	struct usb_phy		phy;
>  	struct phy_companion	*comparator;
> +	void __iomem		*pll_ctrl_base;
>  	struct device		*dev;
>  	u32 __iomem		*control_dev;
>  	struct clk		*wkupclk;
> +	struct clk		*optclk;
>  	u8			is_suspended:1;
>  };
>  
>  #define	PHY_PD	0x1
>  
> +#define	CONTROL_PHY_POWER_USB		0x00000370
> +
> +#define	USB_PWRCTL_CLK_CMD_MASK		0x003FC000
> +#define	USB_PWRCTL_CLK_CMD_SHIFT	0xE
> +
> +#define	USB_PWRCTL_CLK_FREQ_MASK	0xFFC00000
> +#define	USB_PWRCTL_CLK_FREQ_SHIFT	0x16
> +
> +#define	USB3_PHY_TX_RX_POWERON		0x3
> +#define	USB3_PHY_TX_RX_POWEROFF		0x0
> +
>  #define	phy_to_omapusb(x)	container_of((x), struct omap_usb, phy)
>  
>  #if defined(CONFIG_OMAP_USB2) || defined(CONFIG_OMAP_USB2_MODULE)
> @@ -43,4 +104,15 @@ static inline int omap_usb2_set_comparator(struct phy_companion *comparator)
>  }
>  #endif
>  
> +static inline u32 omap_usb_readl(const void __iomem *addr, unsigned offset)
> +{
> +	return __raw_readl(addr + offset);
> +}
> +
> +static inline void omap_usb_writel(const void __iomem *addr, unsigned offset,
> +								u32 data)
> +{
> +	__raw_writel(data, addr + offset);
> +}
> +
>  #endif /* __DRIVERS_OMAP_USB_H */
> 
regards,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

Re: [PATCH 1/4] usb: phy: add a new driver for usb3 phy

[ABRAHAM, KISHON VIJAY]

Hi,

On Wed, Sep 19, 2012 at 8:11 PM, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> On 09/19/2012 01:30 PM, Kishon Vijay Abraham I wrote:
>> Added a driver for usb3 phy that handles the interaction between usb phy
>> device and dwc3 controller.
>>
>> This also includes device tree support for usb3 phy driver and
>> the documentation with device tree binding information is updated.
>>
>> Currently writing to control module register is taken care in this
>> driver which will be removed once the control module driver is in place.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> Signed-off-by: Felipe Balbi <balbi@ti.com>
>> Signed-off-by: Moiz Sonasath <m-sonasath@ti.com>
>> ---
>>  Documentation/devicetree/bindings/usb/usb-phy.txt |   18 +
>>  drivers/usb/phy/Kconfig                           |    9 +
>>  drivers/usb/phy/Makefile                          |    1 +
>>  drivers/usb/phy/omap-usb3.c                       |  369 +++++++++++++++++++++
>>  include/linux/usb/omap_usb.h                      |   72 ++++
>>  5 files changed, 469 insertions(+)
>>  create mode 100644 drivers/usb/phy/omap-usb3.c
>>
>> diff --git a/Documentation/devicetree/bindings/usb/usb-phy.txt b/Documentation/devicetree/bindings/usb/usb-phy.txt
>> index 80d4148..7c5fd89 100644
>> --- a/Documentation/devicetree/bindings/usb/usb-phy.txt
>> +++ b/Documentation/devicetree/bindings/usb/usb-phy.txt
>> @@ -15,3 +15,21 @@ usb2phy@4a0ad080 {
>>       reg = <0x4a0ad080 0x58>,
>>             <0x4a002300 0x4>;
>>  };
>> +
>> +OMAP USB3 PHY
>> +
>> +Required properties:
>> + - compatible: Should be "ti,omap-usb3"
>> + - reg : Address and length of the register set for the device. Also
>> +add the address of control module phy power register until a driver for
>> +control module is added
>> +
>> +This is usually a subnode of ocp2scp to which it is connected.
>> +
>> +usb3phy@4a084400 {
>> +     compatible = "ti,omap-usb3";
>> +     reg = <0x0x4a084400 0x80>,
>> +           <0x4a084800 0x64>,
>> +           <0x4a084c00 0x40>,
>> +           <0x4a002370 0x4>;
>> +};
>> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
>> index 63c339b..353dc0c 100644
>> --- a/drivers/usb/phy/Kconfig
>> +++ b/drivers/usb/phy/Kconfig
>> @@ -13,6 +13,15 @@ config OMAP_USB2
>>         The USB OTG controller communicates with the comparator using this
>>         driver.
>>
>> +config OMAP_USB3
>> +     tristate "OMAP USB3 PHY Driver"
>> +     select USB_OTG_UTILS
>> +     help
>> +       Enable this to support the USB3 PHY that is part of SOC. This
>> +       driver takes care of all the PHY functionality apart from comparator.
>> +       The USB OTG controller communicates with the comparator using this
>> +       driver.
>> +
>>  config USB_ISP1301
>>       tristate "NXP ISP1301 USB transceiver support"
>>       depends on USB || USB_GADGET
>> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
>> index b069f29..973b1e6 100644
>> --- a/drivers/usb/phy/Makefile
>> +++ b/drivers/usb/phy/Makefile
>> @@ -5,6 +5,7 @@
>>  ccflags-$(CONFIG_USB_DEBUG) := -DDEBUG
>>
>>  obj-$(CONFIG_OMAP_USB2)                      += omap-usb2.o
>> +obj-$(CONFIG_OMAP_USB3)                      += omap-usb3.o
>>  obj-$(CONFIG_USB_ISP1301)            += isp1301.o
>>  obj-$(CONFIG_MV_U3D_PHY)             += mv_u3d_phy.o
>>  obj-$(CONFIG_USB_EHCI_TEGRA) += tegra_usb_phy.o
>> diff --git a/drivers/usb/phy/omap-usb3.c b/drivers/usb/phy/omap-usb3.c
>> new file mode 100644
>> index 0000000..4dc84ca
>> --- /dev/null
>> +++ b/drivers/usb/phy/omap-usb3.c
>> @@ -0,0 +1,369 @@
>> +/*
>> + * omap-usb3 - USB PHY, talking to dwc3 controller in OMAP.
>> + *
>> + * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com
>> + * 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.
>> + *
>> + * Author: Kishon Vijay Abraham I <kishon@ti.com>
>> + *
>> + * 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/slab.h>
>> +#include <linux/usb/omap_usb.h>
>> +#include <linux/of.h>
>> +#include <linux/clk.h>
>> +#include <linux/err.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/delay.h>
>> +
>> +static struct usb_dpll_params omap_usb3_dpll_params[NUM_SYS_CLKS] = {
>> +     {1250, 5, 4, 20, 0},            /* 12 MHz */
>> +     {3125, 20, 4, 20, 0},           /* 16.8 MHz */
>> +     {1172, 8, 4, 20, 65537},        /* 19.2 MHz */
>> +     {1250, 12, 4, 20, 0},           /* 26 MHz */
>> +     {3125, 47, 4, 20, 92843},       /* 38.4 MHz */
>> +};
>> +
>> +/**
>> + * omap5_usb_phy_power - power on/off the serializer using control module
>> + * @phy: struct omap_usb *
>> + * @on: 0 to off and 1 to on based on powering on or off the PHY
>> + *
>> + * omap_usb3 can call this API to power on or off the PHY.
>> + */
>> +static int omap5_usb_phy_power(struct omap_usb *phy, bool on)
>> +{
>> +     u32 val;
>> +     unsigned long rate;
>> +     struct clk *sys_clk;
>> +
>> +     sys_clk = clk_get(NULL, "sys_clkin");
>
> Where's the corresponding clk_put()?

I have missed it. Will fix it in my next version.
>
>> +     if (IS_ERR(sys_clk)) {
>> +             pr_err("%s: unable to get sys_clkin\n", __func__);
>> +             return -EINVAL;
>> +     }
>> +
>> +     rate = clk_get_rate(sys_clk);
>> +     rate = rate/1000000;
>> +
>> +     val = readl(phy->control_dev);
>> +
>> +     if (on) {
>> +             val &= ~(USB_PWRCTL_CLK_CMD_MASK | USB_PWRCTL_CLK_FREQ_MASK);
>> +             val |= USB3_PHY_TX_RX_POWERON << USB_PWRCTL_CLK_CMD_SHIFT;
>> +             val |= rate << USB_PWRCTL_CLK_FREQ_SHIFT;
>> +     } else {
>> +             val &= ~USB_PWRCTL_CLK_CMD_MASK;
>> +             val |= USB3_PHY_TX_RX_POWEROFF << USB_PWRCTL_CLK_CMD_SHIFT;
>> +     }
>> +
>> +     writel(val, phy->control_dev);
>> +
>> +     return 0;
>> +}
>> +
>> +static int omap_usb3_suspend(struct usb_phy *x, int suspend)
>> +{
>> +     struct omap_usb *phy = phy_to_omapusb(x);
>> +     int     val, ret;
>> +     int timeout = PLL_IDLE_TIME;
>> +
>> +     if (suspend && !phy->is_suspended) {
>> +             val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION2);
>> +             val |= PLL_IDLE;
>> +             omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION2, val);
>> +
>> +             do {
>> +                     val = omap_usb_readl(phy->pll_ctrl_base, PLL_STATUS);
>> +                     if (val & PLL_TICOPWDN)
>> +                             break;
>> +                     udelay(1);
>> +             } while (--timeout);
>> +
>> +             omap5_usb_phy_power(phy, 0);
>> +             pm_runtime_put_sync(phy->dev);
>> +
>> +             phy->is_suspended       = 1;
>> +     } else if (!suspend && phy->is_suspended) {
>> +             phy->is_suspended       = 0;
>> +             ret = pm_runtime_get_sync(phy->dev);
>> +             if (ret < 0) {
>> +                     dev_err(phy->dev, "get_sync failed with err %d\n",
>> +                                                                     ret);
>> +                     return ret;
>> +             }
>> +
>> +             val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION2);
>> +             val &= ~PLL_IDLE;
>> +             omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION2, val);
>> +
>> +             do {
>> +                     val = omap_usb_readl(phy->pll_ctrl_base, PLL_STATUS);
>> +                     if (!(val & PLL_TICOPWDN))
>> +                             break;
>> +                     udelay(1);
>> +             } while (--timeout);
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +static inline enum sys_clk_rate __get_sys_clk_index(unsigned long rate)
>> +{
>> +     switch (rate) {
>> +     case 12000000:
>> +             return CLK_RATE_12MHZ;
>> +     case 16800000:
>> +             return CLK_RATE_16MHZ;
>> +     case 19200000:
>> +             return CLK_RATE_19MHZ;
>> +     case 26000000:
>> +             return CLK_RATE_26MHZ;
>> +     case 38400000:
>> +             return CLK_RATE_38MHZ;
>> +     default:
>> +             return CLK_RATE_UNDEFINED;
>> +     }
>> +}
>> +
>> +static void omap_usb_dpll_relock(struct omap_usb *phy)
>> +{
>> +     u32             val;
>> +     unsigned long   timeout;
>> +
>> +     omap_usb_writel(phy->pll_ctrl_base, PLL_GO, SET_PLL_GO);
>> +
>> +     timeout = jiffies + msecs_to_jiffies(20);
>> +     do {
>> +             val = omap_usb_readl(phy->pll_ctrl_base, PLL_STATUS);
>> +             if (val & PLL_LOCK)
>> +                     break;
>> +     } while (!WARN_ON(time_after(jiffies, timeout)));
>> +}
>> +
>> +static int omap_usb_dpll_lock(struct omap_usb *phy)
>> +{
>> +     u32                     val;
>> +     struct clk              *sys_clk;
>> +     unsigned long           rate;
>> +     enum sys_clk_rate       clk_index;
>> +
>> +     sys_clk = clk_get(NULL, "sys_clkin");
>
> Where's the corresponding clk_put()?
>
>> +     if (IS_ERR(sys_clk)) {
>> +             pr_err("unable to get sys_clkin\n");
>> +             return PTR_ERR(sys_clk);
>> +     }
>> +
>> +     rate            = clk_get_rate(sys_clk);
>> +     clk_index       = __get_sys_clk_index(rate);
>> +
>> +     if (clk_index == CLK_RATE_UNDEFINED) {
>> +             pr_err("dpll cannot be locked for sys clk freq:%luHz\n", rate);
>> +             return -EINVAL;
>> +     }
>> +
>> +     val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION1);
>> +     val &= ~PLL_REGN_MASK;
>> +     val |= omap_usb3_dpll_params[clk_index].n << PLL_REGN_SHIFT;
>> +     omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION1, val);
>> +
>> +     val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION2);
>> +     val &= ~PLL_SELFREQDCO_MASK;
>> +     val |= omap_usb3_dpll_params[clk_index].freq << PLL_SELFREQDCO_SHIFT;
>> +     omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION2, val);
>> +
>> +     val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION1);
>> +     val &= ~PLL_REGM_MASK;
>> +     val |= omap_usb3_dpll_params[clk_index].m << PLL_REGM_SHIFT;
>> +     omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION1, val);
>> +
>> +     val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION4);
>> +     val &= ~PLL_REGM_F_MASK;
>> +     val |= omap_usb3_dpll_params[clk_index].mf << PLL_REGM_F_SHIFT;
>> +     omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION4, val);
>> +
>> +     val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION3);
>> +     val &= ~PLL_SD_MASK;
>> +     val |= omap_usb3_dpll_params[clk_index].sd << PLL_SD_SHIFT;
>> +     omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION3, val);
>> +
>> +     omap_usb_dpll_relock(phy);
>> +
>> +     return 0;
>> +}
>> +
>> +static int omap_usb3_init(struct usb_phy *x)
>> +{
>> +     struct omap_usb *phy = phy_to_omapusb(x);
>> +
>> +     omap_usb_dpll_lock(phy);
>> +     omap5_usb_phy_power(phy, 1);
>> +
>> +     return 0;
>> +}
>> +
>> +static int __devinit omap_usb3_probe(struct platform_device *pdev)
>> +{
>> +     struct omap_usb                 *phy;
>> +     struct resource                 *res;
>> +
>> +     phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
>> +     if (!phy) {
>> +             dev_err(&pdev->dev, "unable to alloc mem for OMAP USB3 PHY\n");
>> +             return -ENOMEM;
>> +     }
>> +
>> +     res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
>> +     if (!res) {
>> +             dev_err(&pdev->dev, "unable to get base address of pll_ctrl\n");
>> +             return -ENODEV;
>> +     }
>> +
>> +     phy->pll_ctrl_base = devm_request_and_ioremap(&pdev->dev, res);
>> +     if (!phy->pll_ctrl_base) {
>> +             dev_err(&pdev->dev, "ioremap of pll_ctrl failed\n");
>> +             return -ENOMEM;
>> +     }
>> +
>> +     res = platform_get_resource(pdev, IORESOURCE_MEM, 3);
>> +     if (!res) {
>> +             dev_err(&pdev->dev,
>> +                     "unable to get address of control phy power\n");
>> +             return -ENODEV;
>> +     }
>> +
>> +     phy->control_dev = devm_request_and_ioremap(&pdev->dev, res);
>> +     if (!phy->control_dev) {
>> +             dev_err(&pdev->dev, "ioremap of control_dev failed\n");
>> +             return -ENOMEM;
>> +     }
>> +
>> +     phy->dev                = &pdev->dev;
>> +
>> +     phy->phy.dev            = phy->dev;
>> +     phy->phy.label          = "omap-usb3";
>> +     phy->phy.init           = omap_usb3_init;
>> +     phy->phy.set_suspend    = omap_usb3_suspend;
>> +
>> +     phy->is_suspended       = 1;
>> +     omap5_usb_phy_power(phy, 0);
>> +
>> +     phy->wkupclk = devm_clk_get(phy->dev, "usb_phy_cm_clk32k");
>> +     if (IS_ERR(phy->wkupclk)) {
>> +             dev_err(&pdev->dev, "unable to get usb_phy_cm_clk32k\n");
>> +             return PTR_ERR(phy->wkupclk);
>> +     }
>> +     clk_prepare(phy->wkupclk);
>> +
>> +     phy->optclk = devm_clk_get(phy->dev, "usb_otg_ss_refclk960m");
>> +     if (IS_ERR(phy->optclk)) {
>> +             dev_err(&pdev->dev, "unable to get usb_otg_ss_refclk960m\n");
>> +             return PTR_ERR(phy->optclk);
>> +     }
>> +     clk_prepare(phy->optclk);
>
> Are these clocks only needed on the PM usecase?
clk32k is a wakeup/debounce clock and 960m is a functional reference
clock to usb2 phy.

Thanks
Kishon

Re: [PATCH 1/4] usb: phy: add a new driver for usb3 phy

[Tony Lindgren]

Hi,

* Kishon Vijay Abraham I <kishon@ti.com> [120919 04:32]:
> Added a driver for usb3 phy that handles the interaction between usb phy
> device and dwc3 controller.
> 
> This also includes device tree support for usb3 phy driver and
> the documentation with device tree binding information is updated.
> 
> Currently writing to control module register is taken care in this
> driver which will be removed once the control module driver is in place.

You may be able to set up the control module register with one
of the following Linux standard frameworks:

1. Fixed regulator defined in mach-omap2/control.c

   In this case the PHY driver can pick up the regulator by name.

2. A mux mapped with pinctrl framework using pinctrl-single,bits
   binding

   And in this case the PHY driver can request the named pinctrl
   states like "enabled" and "disabled".

> --- a/Documentation/devicetree/bindings/usb/usb-phy.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-phy.txt
> @@ -15,3 +15,21 @@ usb2phy@4a0ad080 {
>  	reg = <0x4a0ad080 0x58>,
>  	      <0x4a002300 0x4>;
>  };

The comments also apply to the omap_usb2.c driver for
0x4a002300 above.

> +
> +OMAP USB3 PHY
> +
> +Required properties:
> + - compatible: Should be "ti,omap-usb3"
> + - reg : Address and length of the register set for the device. Also
> +add the address of control module phy power register until a driver for
> +control module is added
> +
> +This is usually a subnode of ocp2scp to which it is connected.
> +
> +usb3phy@4a084400 {
> +	compatible = "ti,omap-usb3";
> +	reg = <0x0x4a084400 0x80>,
> +	      <0x4a084800 0x64>,
> +	      <0x4a084c00 0x40>,
> +	      <0x4a002370 0x4>;
> +};

And register 0x4a002370 here. Care to post some info what the
0x4a002370 register bits do? Is that same as CONTROL_DEV_CONF
on omap4, or does it have other bits there too?

The advantage for using regulator fwk and pinctrl fwk is
that the regulator and mux can be children of the SCM
core driver when we have it. And no direct register tinkering
or omap specific custom exported functions are needed ;)

Regards,

Tony

Re: [PATCH 1/4] usb: phy: add a new driver for usb3 phy

[kishon]

Hi Tony,

On Thursday 11 October 2012 06:29 AM, Tony Lindgren wrote:
> Hi,
>
> * Kishon Vijay Abraham I <kishon@ti.com> [120919 04:32]:
>> Added a driver for usb3 phy that handles the interaction between usb phy
>> device and dwc3 controller.
>>
>> This also includes device tree support for usb3 phy driver and
>> the documentation with device tree binding information is updated.
>>
>> Currently writing to control module register is taken care in this
>> driver which will be removed once the control module driver is in place.
>
> You may be able to set up the control module register with one
> of the following Linux standard frameworks:
>
> 1. Fixed regulator defined in mach-omap2/control.c

Is it control.c?
>
>     In this case the PHY driver can pick up the regulator by name.

Do you mean we have to define something like fixed_voltage_config 
defined in board-4430sdp.c?
 From whatever I could make out from regulator/fixed.c, 
enabling/disabling of regulator is done using only gpio. I'm not sure 
how we can use that to write to control module register.
>
> 2. A mux mapped with pinctrl framework using pinctrl-single,bits
>     binding
>
>     And in this case the PHY driver can request the named pinctrl
>     states like "enabled" and "disabled".
>
>> --- a/Documentation/devicetree/bindings/usb/usb-phy.txt
>> +++ b/Documentation/devicetree/bindings/usb/usb-phy.txt
>> @@ -15,3 +15,21 @@ usb2phy@4a0ad080 {
>>   	reg = <0x4a0ad080 0x58>,
>>   	      <0x4a002300 0x4>;
>>   };
>
> The comments also apply to the omap_usb2.c driver for
> 0x4a002300 above.
>
>> +
>> +OMAP USB3 PHY
>> +
>> +Required properties:
>> + - compatible: Should be "ti,omap-usb3"
>> + - reg : Address and length of the register set for the device. Also
>> +add the address of control module phy power register until a driver for
>> +control module is added
>> +
>> +This is usually a subnode of ocp2scp to which it is connected.
>> +
>> +usb3phy@4a084400 {
>> +	compatible = "ti,omap-usb3";
>> +	reg = <0x0x4a084400 0x80>,
>> +	      <0x4a084800 0x64>,
>> +	      <0x4a084c00 0x40>,
>> +	      <0x4a002370 0x4>;
>> +};
>
> And register 0x4a002370 here. Care to post some info what the
> 0x4a002370 register bits do? Is that same as CONTROL_DEV_CONF
> on omap4, or does it have other bits there too?

It's CONTROL_PHY_POWER_USB register and it's structure looks like this.
31:22 USB_PWRCTL_CLK_FREQ
21:14 USB_PWRCTL_CLK_CMD
13:0 RESERVED

CLK_CMD takes values to power up/down the TX and RX in various combinations.

And CLK_FREQ takes values for clock configuration.

Thanks
Kishon

Re: [PATCH 1/4] usb: phy: add a new driver for usb3 phy

[Tony Lindgren]

* kishon <kishon@ti.com> [121012 02:10]:
> Hi Tony,
> 
> On Thursday 11 October 2012 06:29 AM, Tony Lindgren wrote:
> >Hi,
> >
> >* Kishon Vijay Abraham I <kishon@ti.com> [120919 04:32]:
> >>Added a driver for usb3 phy that handles the interaction between usb phy
> >>device and dwc3 controller.
> >>
> >>This also includes device tree support for usb3 phy driver and
> >>the documentation with device tree binding information is updated.
> >>
> >>Currently writing to control module register is taken care in this
> >>driver which will be removed once the control module driver is in place.
> >
> >You may be able to set up the control module register with one
> >of the following Linux standard frameworks:
> >
> >1. Fixed regulator defined in mach-omap2/control.c
> 
> Is it control.c?

Hmm after looking into it more we're missing one piece of the puzzle
to handle SCM regulators, which is omap-scm-regulator.c :)
I'll do a minimal DT only version of that.

> >    In this case the PHY driver can pick up the regulator by name.
> 
> Do you mean we have to define something like fixed_voltage_config
> defined in board-4430sdp.c?
> From whatever I could make out from regulator/fixed.c,
> enabling/disabling of regulator is done using only gpio. I'm not
> sure how we can use that to write to control module register.

Yes you're right, we're missing omap-scm-regulator.c, then it will
be trivial to select the regulator from DT like we have for the
twl-regulator.c.

> >>+usb3phy@4a084400 {
> >>+	compatible = "ti,omap-usb3";
> >>+	reg = <0x0x4a084400 0x80>,
> >>+	      <0x4a084800 0x64>,
> >>+	      <0x4a084c00 0x40>,
> >>+	      <0x4a002370 0x4>;
> >>+};
> >
> >And register 0x4a002370 here. Care to post some info what the
> >0x4a002370 register bits do? Is that same as CONTROL_DEV_CONF
> >on omap4, or does it have other bits there too?
> 
> It's CONTROL_PHY_POWER_USB register and it's structure looks like this.
> 31:22 USB_PWRCTL_CLK_FREQ
> 21:14 USB_PWRCTL_CLK_CMD
> 13:0 RESERVED
> 
> CLK_CMD takes values to power up/down the TX and RX in various combinations.
> 
> And CLK_FREQ takes values for clock configuration.

Oh, it's a clock. Then it would be best to set it up using the
common clock framework and have the clock registered by the
SCM core driver when those are available.

Maybe please just add a comment for that for later on?

Regards,

Tony

[PATCH 2/4] usb: dwc3: Fix gadget pullup in SS mode

[Kishon Vijay Abraham I]

From: Moiz Sonasath <m-sonasath@ti.com>

For the gadget pullup functionality to work in
SS mode it requires a particular sequence of
toggling the run-stop bit. Here is the required
sequence:

- Set DCTL[31]
- Clear DCTL[31]
- Clear OMAP5430_CONTROL_CORE__PHY_POWER_USB[14]
- Clear DCTL[8:5] = 0x00
- Set DCTL[8:5] = 0x05
- Wait 25 Ms
- Set DCTL[31]
- Set OMAP5430_CONTROL_CORE__PHY_POWER_USB[14]

Tested rigourously the gadget pull-up functionality
in bot HS and SS modes.

Signed-off-by: Moiz Sonasath <m-sonasath@ti.com>
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/usb/dwc3/gadget.c   |   21 +++++++++++++++------
 drivers/usb/phy/omap-usb3.c |   16 ++++++++++++++++
 include/linux/usb/phy.h     |   10 +++++++++-
 3 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 58fdfad..bcc0102 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -49,6 +49,7 @@
 
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
+#include <linux/usb/otg.h>
 
 #include "core.h"
 #include "gadget.h"
@@ -1417,19 +1418,27 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on)
 	reg = dwc3_readl(dwc->regs, DWC3_DCTL);
 	if (is_on) {
 		if (dwc->revision <= DWC3_REVISION_187A) {
-			reg &= ~DWC3_DCTL_TRGTULST_MASK;
-			reg |= DWC3_DCTL_TRGTULST_RX_DET;
+			reg &= ~DWC3_DCTL_ULSTCHNGREQ_MASK;
+			dwc3_writel(dwc->regs, DWC3_DCTL, reg);
+			reg |= DWC3_DCTL_ULSTCHNG_RX_DETECT;
+			dwc3_writel(dwc->regs, DWC3_DCTL, reg);
+			mdelay(25);
+			reg |= DWC3_DCTL_RUN_STOP;
+			dwc3_writel(dwc->regs, DWC3_DCTL, reg);
+			usb_phy_poweron(dwc->usb3_phy);
 		}
 
-		if (dwc->revision >= DWC3_REVISION_194A)
+		if (dwc->revision >= DWC3_REVISION_194A) {
 			reg &= ~DWC3_DCTL_KEEP_CONNECT;
-		reg |= DWC3_DCTL_RUN_STOP;
+			reg |= DWC3_DCTL_RUN_STOP;
+			dwc3_writel(dwc->regs, DWC3_DCTL, reg);
+		}
 	} else {
 		reg &= ~DWC3_DCTL_RUN_STOP;
+		dwc3_writel(dwc->regs, DWC3_DCTL, reg);
+		usb_phy_shutdown(dwc->usb3_phy);
 	}
 
-	dwc3_writel(dwc->regs, DWC3_DCTL, reg);
-
 	do {
 		reg = dwc3_readl(dwc->regs, DWC3_DSTS);
 		if (is_on) {
diff --git a/drivers/usb/phy/omap-usb3.c b/drivers/usb/phy/omap-usb3.c
index 4dc84ca..26402d5 100644
--- a/drivers/usb/phy/omap-usb3.c
+++ b/drivers/usb/phy/omap-usb3.c
@@ -212,6 +212,20 @@ static int omap_usb3_init(struct usb_phy *x)
 	return 0;
 }
 
+static void omap_usb3_poweron(struct usb_phy *x)
+{
+	struct omap_usb *phy = phy_to_omapusb(x);
+
+	omap5_usb_phy_power(phy, 1);
+}
+
+static void omap_usb3_shutdown(struct usb_phy *x)
+{
+	struct omap_usb *phy = phy_to_omapusb(x);
+
+	omap5_usb_phy_power(phy, 0);
+}
+
 static int __devinit omap_usb3_probe(struct platform_device *pdev)
 {
 	struct omap_usb			*phy;
@@ -253,6 +267,8 @@ static int __devinit omap_usb3_probe(struct platform_device *pdev)
 	phy->phy.dev		= phy->dev;
 	phy->phy.label		= "omap-usb3";
 	phy->phy.init		= omap_usb3_init;
+	phy->phy.poweron	= omap_usb3_poweron;
+	phy->phy.shutdown	= omap_usb3_shutdown;
 	phy->phy.set_suspend	= omap_usb3_suspend;
 
 	phy->is_suspended	= 1;
diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
index 06b5bae..aaa8e16 100644
--- a/include/linux/usb/phy.h
+++ b/include/linux/usb/phy.h
@@ -86,8 +86,9 @@ struct usb_phy {
 	/* to support controllers that have multiple transceivers */
 	struct list_head	head;
 
-	/* initialize/shutdown the OTG controller */
+	/* initialize/poweron/shutdown the OTG controller */
 	int	(*init)(struct usb_phy *x);
+	void    (*poweron)(struct usb_phy *x);
 	void	(*shutdown)(struct usb_phy *x);
 
 	/* effective for B devices, ignored for A-peripheral */
@@ -135,6 +136,13 @@ usb_phy_init(struct usb_phy *x)
 }
 
 static inline void
+usb_phy_poweron(struct usb_phy *x)
+{
+	if (x->poweron)
+		x->poweron(x);
+}
+
+static inline void
 usb_phy_shutdown(struct usb_phy *x)
 {
 	if (x->shutdown)
-- 
1.7.9.5

Re: [PATCH 2/4] usb: dwc3: Fix gadget pullup in SS mode

[Felipe Balbi]

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

Hi,

On Wed, Sep 19, 2012 at 05:00:27PM +0530, Kishon Vijay Abraham I wrote:
> From: Moiz Sonasath <m-sonasath@ti.com>
> 
> For the gadget pullup functionality to work in
> SS mode it requires a particular sequence of
> toggling the run-stop bit. Here is the required
> sequence:
> 
> - Set DCTL[31]
> - Clear DCTL[31]
> - Clear OMAP5430_CONTROL_CORE__PHY_POWER_USB[14]
> - Clear DCTL[8:5] = 0x00
> - Set DCTL[8:5] = 0x05
> - Wait 25 Ms
> - Set DCTL[31]
> - Set OMAP5430_CONTROL_CORE__PHY_POWER_USB[14]
> 
> Tested rigourously the gadget pull-up functionality
> in bot HS and SS modes.
> 
> Signed-off-by: Moiz Sonasath <m-sonasath@ti.com>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

this needs to split into three patches:

add new poweron field, implement it on omap-usb3, use it on
dwc3/gadget.c

btw, I don't think the changes to run_stop bit are necessary and if they
are, that'd either be a silicon errata or it would've been mentioned on
the databook. I don't remember seeing that on the databook so I'm
assuming that this is caused by a bad use of the PHY.

Why that mdelay(25) ? why 25 ms ? That's quite a long time, actually.

> ---
>  drivers/usb/dwc3/gadget.c   |   21 +++++++++++++++------
>  drivers/usb/phy/omap-usb3.c |   16 ++++++++++++++++
>  include/linux/usb/phy.h     |   10 +++++++++-
>  3 files changed, 40 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 58fdfad..bcc0102 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -49,6 +49,7 @@
>  
>  #include <linux/usb/ch9.h>
>  #include <linux/usb/gadget.h>
> +#include <linux/usb/otg.h>
>  
>  #include "core.h"
>  #include "gadget.h"
> @@ -1417,19 +1418,27 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on)
>  	reg = dwc3_readl(dwc->regs, DWC3_DCTL);
>  	if (is_on) {
>  		if (dwc->revision <= DWC3_REVISION_187A) {
> -			reg &= ~DWC3_DCTL_TRGTULST_MASK;
> -			reg |= DWC3_DCTL_TRGTULST_RX_DET;
> +			reg &= ~DWC3_DCTL_ULSTCHNGREQ_MASK;
> +			dwc3_writel(dwc->regs, DWC3_DCTL, reg);
> +			reg |= DWC3_DCTL_ULSTCHNG_RX_DETECT;
> +			dwc3_writel(dwc->regs, DWC3_DCTL, reg);
> +			mdelay(25);
> +			reg |= DWC3_DCTL_RUN_STOP;
> +			dwc3_writel(dwc->regs, DWC3_DCTL, reg);
> +			usb_phy_poweron(dwc->usb3_phy);
>  		}
>  
> -		if (dwc->revision >= DWC3_REVISION_194A)
> +		if (dwc->revision >= DWC3_REVISION_194A) {
>  			reg &= ~DWC3_DCTL_KEEP_CONNECT;
> -		reg |= DWC3_DCTL_RUN_STOP;
> +			reg |= DWC3_DCTL_RUN_STOP;
> +			dwc3_writel(dwc->regs, DWC3_DCTL, reg);
> +		}
>  	} else {
>  		reg &= ~DWC3_DCTL_RUN_STOP;
> +		dwc3_writel(dwc->regs, DWC3_DCTL, reg);
> +		usb_phy_shutdown(dwc->usb3_phy);
>  	}
>  
> -	dwc3_writel(dwc->regs, DWC3_DCTL, reg);

I'd prefer to hold all values on the variable and write only one.

-- 
balbi

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

Re: [PATCH 2/4] usb: dwc3: Fix gadget pullup in SS mode

[Felipe Balbi]

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

Hi,

On Wed, Sep 19, 2012 at 10:02:48AM -0500, Sonasath, Moiz wrote:
> Felipe,
> 
> On Wed, Sep 19, 2012 at 6:53 AM, Felipe Balbi <balbi@ti.com> wrote:
> 
> > Hi,
> >
> > On Wed, Sep 19, 2012 at 05:00:27PM +0530, Kishon Vijay Abraham I wrote:
> > > From: Moiz Sonasath <m-sonasath@ti.com>
> > >
> > > For the gadget pullup functionality to work in
> > > SS mode it requires a particular sequence of
> > > toggling the run-stop bit. Here is the required
> > > sequence:
> > >
> > > - Set DCTL[31]
> > > - Clear DCTL[31]
> > > - Clear OMAP5430_CONTROL_CORE__PHY_POWER_USB[14]
> > > - Clear DCTL[8:5] = 0x00
> > > - Set DCTL[8:5] = 0x05
> > > - Wait 25 Ms
> > > - Set DCTL[31]
> > > - Set OMAP5430_CONTROL_CORE__PHY_POWER_USB[14]
> > >
> > > Tested rigourously the gadget pull-up functionality
> > > in bot HS and SS modes.
> > >
> > > Signed-off-by: Moiz Sonasath <m-sonasath@ti.com>
> > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> >
> > this needs to split into three patches:
> >
> > add new poweron field, implement it on omap-usb3, use it on
> > dwc3/gadget.c
> >
> > btw, I don't think the changes to run_stop bit are necessary and if they
> > are, that'd either be a silicon errata or it would've been mentioned on
> > the databook. I don't remember seeing that on the databook so I'm
> > assuming that this is caused by a bad use of the PHY.
> >
> > Why that mdelay(25) ? why 25 ms ? That's quite a long time, actually.
> >
> 
> Felipe, This is infact a HW bug that the Si-Val team did accept and gave us
> this workaround sequence with the precise delay :-)
> 
> Supposedly this will be fixed in ES 2.0.

in that case this doesn't have to go to mainline since we're not
supporting ES1.0 in mainline :-)

at minimum this should've come with a proper revision check anyway.

-- 
balbi

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

Re: [PATCH 2/4] usb: dwc3: Fix gadget pullup in SS mode

[Felipe Balbi]

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

On Wed, Sep 19, 2012 at 11:50:53AM -0500, Sonasath, Moiz wrote:
> Felipe,
> 
> On Wed, Sep 19, 2012 at 11:04 AM, Felipe Balbi <balbi@ti.com> wrote:
> 
> > Hi,
> >
> > On Wed, Sep 19, 2012 at 10:02:48AM -0500, Sonasath, Moiz wrote:
> > > Felipe,
> > >
> > > On Wed, Sep 19, 2012 at 6:53 AM, Felipe Balbi <balbi@ti.com> wrote:
> > >
> > > > Hi,
> > > >
> > > > On Wed, Sep 19, 2012 at 05:00:27PM +0530, Kishon Vijay Abraham I wrote:
> > > > > From: Moiz Sonasath <m-sonasath@ti.com>
> > > > >
> > > > > For the gadget pullup functionality to work in
> > > > > SS mode it requires a particular sequence of
> > > > > toggling the run-stop bit. Here is the required
> > > > > sequence:
> > > > >
> > > > > - Set DCTL[31]
> > > > > - Clear DCTL[31]
> > > > > - Clear OMAP5430_CONTROL_CORE__PHY_POWER_USB[14]
> > > > > - Clear DCTL[8:5] = 0x00
> > > > > - Set DCTL[8:5] = 0x05
> > > > > - Wait 25 Ms
> > > > > - Set DCTL[31]
> > > > > - Set OMAP5430_CONTROL_CORE__PHY_POWER_USB[14]
> > > > >
> > > > > Tested rigourously the gadget pull-up functionality
> > > > > in bot HS and SS modes.
> > > > >
> > > > > Signed-off-by: Moiz Sonasath <m-sonasath@ti.com>
> > > > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> > > >
> > > > this needs to split into three patches:
> > > >
> > > > add new poweron field, implement it on omap-usb3, use it on
> > > > dwc3/gadget.c
> > > >
> > > > btw, I don't think the changes to run_stop bit are necessary and if
> > they
> > > > are, that'd either be a silicon errata or it would've been mentioned on
> > > > the databook. I don't remember seeing that on the databook so I'm
> > > > assuming that this is caused by a bad use of the PHY.
> > > >
> > > > Why that mdelay(25) ? why 25 ms ? That's quite a long time, actually.
> > > >
> > >
> > > Felipe, This is infact a HW bug that the Si-Val team did accept and gave
> > us
> > > this workaround sequence with the precise delay :-)
> > >
> > > Supposedly this will be fixed in ES 2.0.
> >
> > in that case this doesn't have to go to mainline since we're not
> > supporting ES1.0 in mainline :-)
> >
> > at minimum this should've come with a proper revision check anyway.
> >
> 
> Actually most of it is under a rev check :)

fair enough.

> Perhaps the last: usb_phy_shutdown(dwc->usb3_phy);
> in the else part should be in
> if (dwc->revision <= DWC3_REVISION_187A) check

Is this an OMAP errata or Synopsys errata ? If it's a Synopsys errata we
need to make sure to add the comment above the workaround, if it's an
OMAP errata, we can't apply the workaround to all users since they might
not need it, so we need a more clever scheme.

On top of that, if it's an ES1 errata, I rather not have this in
mainline since we won't support ES1 at all in mainline, which means this
workaround will be useless in a mainline kernel tree.

-- 
balbi

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

Re: [PATCH 2/4] usb: dwc3: Fix gadget pullup in SS mode

[Felipe Balbi]

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

Hi again,

On Wed, Sep 19, 2012 at 08:24:44PM +0300, Felipe Balbi wrote:
> > > at minimum this should've come with a proper revision check anyway.
> > >
> > 
> > Actually most of it is under a rev check :)
> 
> fair enough.

one extra comment here. That revision check is not related to the
silicon errata you mention. That revision check is there because the bit
definition of that particular register has changed a little bit, so
we're just handling a slightly different programming model between older
and newer revisions.

-- 
balbi

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

[PATCH 3/4] usb: phy: omap-usb3: Decrease the number of transitions to recovery

[Kishon Vijay Abraham I]

Power-cycling the PHY (partially) decreases the number of transitions to
"Recovery" state. Hence changed the power up sequence to a partial
power-up before a full power-up of the PHY.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Signed-off-by: Moiz Sonasath <m-sonasath@ti.com>
---
 drivers/usb/phy/omap-usb3.c  |   43 ++++++++++++++++++++++++++++++++++++------
 include/linux/usb/omap_usb.h |    1 +
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/phy/omap-usb3.c b/drivers/usb/phy/omap-usb3.c
index 26402d5..fb3c5e6 100644
--- a/drivers/usb/phy/omap-usb3.c
+++ b/drivers/usb/phy/omap-usb3.c
@@ -35,13 +35,15 @@ static struct usb_dpll_params omap_usb3_dpll_params[NUM_SYS_CLKS] = {
 };
 
 /**
- * omap5_usb_phy_power - power on/off the serializer using control module
+ * omap5_usb_phy_partial_powerup - power on the serializer using control module
  * @phy: struct omap_usb *
- * @on: 0 to off and 1 to on based on powering on or off the PHY
  *
- * omap_usb3 can call this API to power on or off the PHY.
+ * After the dwc3 module is disabled and enabled again the synchronization
+ * between dwc3 and phy goes bad and the device does not get enumerated
+ * in superspeed mode. After some trials it was found powering up TX and
+ * part of RX PHY helped to solve the issue.
  */
-static int omap5_usb_phy_power(struct omap_usb *phy, bool on)
+static int omap5_usb_phy_partial_powerup(struct omap_usb *phy)
 {
 	u32 val;
 	unsigned long rate;
@@ -58,10 +60,32 @@ static int omap5_usb_phy_power(struct omap_usb *phy, bool on)
 
 	val = readl(phy->control_dev);
 
+	val &= ~(USB_PWRCTL_CLK_CMD_MASK | USB_PWRCTL_CLK_FREQ_MASK);
+	val |= (USB3_PHY_PARTIAL_RX_POWERON | USB3_PHY_TX_RX_POWERON)
+		<< USB_PWRCTL_CLK_CMD_SHIFT;
+	val |= rate << USB_PWRCTL_CLK_FREQ_SHIFT;
+
+	writel(val, phy->control_dev);
+
+	return 0;
+}
+
+/**
+ * omap5_usb_phy_power - power on/off the serializer using control module
+ * @phy: struct omap_usb *
+ * @on: 0 to off and 1 to on based on powering on or off the PHY
+ *
+ * omap_usb3 can call this API to power on or off the PHY.
+ */
+static int omap5_usb_phy_power(struct omap_usb *phy, bool on)
+{
+	u32 val;
+
+	val = readl(phy->control_dev);
+
 	if (on) {
-		val &= ~(USB_PWRCTL_CLK_CMD_MASK | USB_PWRCTL_CLK_FREQ_MASK);
+		val &= ~USB_PWRCTL_CLK_CMD_MASK;
 		val |= USB3_PHY_TX_RX_POWERON << USB_PWRCTL_CLK_CMD_SHIFT;
-		val |= rate << USB_PWRCTL_CLK_FREQ_SHIFT;
 	} else {
 		val &= ~USB_PWRCTL_CLK_CMD_MASK;
 		val |= USB3_PHY_TX_RX_POWEROFF << USB_PWRCTL_CLK_CMD_SHIFT;
@@ -207,6 +231,13 @@ static int omap_usb3_init(struct usb_phy *x)
 	struct omap_usb	*phy = phy_to_omapusb(x);
 
 	omap_usb_dpll_lock(phy);
+	omap5_usb_phy_partial_powerup(phy);
+	/*
+	 * Give enough time for the PHY to partially power-up before
+	 * powering it up completely. delay value suggested by the HW
+	 * team.
+	 */
+	mdelay(100);
 	omap5_usb_phy_power(phy, 1);
 
 	return 0;
diff --git a/include/linux/usb/omap_usb.h b/include/linux/usb/omap_usb.h
index 01ed008..62b637c 100644
--- a/include/linux/usb/omap_usb.h
+++ b/include/linux/usb/omap_usb.h
@@ -92,6 +92,7 @@ struct omap_usb {
 
 #define	USB3_PHY_TX_RX_POWERON		0x3
 #define	USB3_PHY_TX_RX_POWEROFF		0x0
+#define	USB3_PHY_PARTIAL_RX_POWERON	(0x1 << 6)
 
 #define	phy_to_omapusb(x)	container_of((x), struct omap_usb, phy)
 
-- 
1.7.9.5

Re: [PATCH 3/4] usb: phy: omap-usb3: Decrease the number of transitions to recovery

[Felipe Balbi]

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

On Wed, Sep 19, 2012 at 05:00:28PM +0530, Kishon Vijay Abraham I wrote:
> Power-cycling the PHY (partially) decreases the number of transitions to
> "Recovery" state. Hence changed the power up sequence to a partial
> power-up before a full power-up of the PHY.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> Signed-off-by: Moiz Sonasath <m-sonasath@ti.com>

this can be amended to patch 1. Why would you introduce a driver with a
known problem ? ;-)

> ---
>  drivers/usb/phy/omap-usb3.c  |   43 ++++++++++++++++++++++++++++++++++++------
>  include/linux/usb/omap_usb.h |    1 +
>  2 files changed, 38 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/phy/omap-usb3.c b/drivers/usb/phy/omap-usb3.c
> index 26402d5..fb3c5e6 100644
> --- a/drivers/usb/phy/omap-usb3.c
> +++ b/drivers/usb/phy/omap-usb3.c
> @@ -35,13 +35,15 @@ static struct usb_dpll_params omap_usb3_dpll_params[NUM_SYS_CLKS] = {
>  };
>  
>  /**
> - * omap5_usb_phy_power - power on/off the serializer using control module
> + * omap5_usb_phy_partial_powerup - power on the serializer using control module
>   * @phy: struct omap_usb *
> - * @on: 0 to off and 1 to on based on powering on or off the PHY
>   *
> - * omap_usb3 can call this API to power on or off the PHY.
> + * After the dwc3 module is disabled and enabled again the synchronization
> + * between dwc3 and phy goes bad and the device does not get enumerated
> + * in superspeed mode. After some trials it was found powering up TX and
> + * part of RX PHY helped to solve the issue.
>   */
> -static int omap5_usb_phy_power(struct omap_usb *phy, bool on)
> +static int omap5_usb_phy_partial_powerup(struct omap_usb *phy)
>  {
>  	u32 val;
>  	unsigned long rate;
> @@ -58,10 +60,32 @@ static int omap5_usb_phy_power(struct omap_usb *phy, bool on)
>  
>  	val = readl(phy->control_dev);
>  
> +	val &= ~(USB_PWRCTL_CLK_CMD_MASK | USB_PWRCTL_CLK_FREQ_MASK);
> +	val |= (USB3_PHY_PARTIAL_RX_POWERON | USB3_PHY_TX_RX_POWERON)
> +		<< USB_PWRCTL_CLK_CMD_SHIFT;
> +	val |= rate << USB_PWRCTL_CLK_FREQ_SHIFT;
> +
> +	writel(val, phy->control_dev);
> +
> +	return 0;
> +}
> +
> +/**
> + * omap5_usb_phy_power - power on/off the serializer using control module
> + * @phy: struct omap_usb *
> + * @on: 0 to off and 1 to on based on powering on or off the PHY
> + *
> + * omap_usb3 can call this API to power on or off the PHY.
> + */
> +static int omap5_usb_phy_power(struct omap_usb *phy, bool on)
> +{
> +	u32 val;
> +
> +	val = readl(phy->control_dev);
> +
>  	if (on) {
> -		val &= ~(USB_PWRCTL_CLK_CMD_MASK | USB_PWRCTL_CLK_FREQ_MASK);
> +		val &= ~USB_PWRCTL_CLK_CMD_MASK;
>  		val |= USB3_PHY_TX_RX_POWERON << USB_PWRCTL_CLK_CMD_SHIFT;
> -		val |= rate << USB_PWRCTL_CLK_FREQ_SHIFT;
>  	} else {
>  		val &= ~USB_PWRCTL_CLK_CMD_MASK;
>  		val |= USB3_PHY_TX_RX_POWEROFF << USB_PWRCTL_CLK_CMD_SHIFT;
> @@ -207,6 +231,13 @@ static int omap_usb3_init(struct usb_phy *x)
>  	struct omap_usb	*phy = phy_to_omapusb(x);
>  
>  	omap_usb_dpll_lock(phy);
> +	omap5_usb_phy_partial_powerup(phy);
> +	/*
> +	 * Give enough time for the PHY to partially power-up before
> +	 * powering it up completely. delay value suggested by the HW
> +	 * team.
> +	 */
> +	mdelay(100);

why 100 ms ? Has this been measured ? Again, 100 ms is a loooooong time.
Soon enough user will be waiting for a bit over a second to see that
after connecting usb cable, there's activity. We don't want user to
notice that kind of thing ;-)

-- 
balbi

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

[PATCH 4/4] usb: phy: omap-usb2: enable 960Mhz clock for omap5

[Kishon Vijay Abraham I]

"usb_otg_ss_refclk960m" is needed by usb2 phy present in omap5. For
omap4, the clk_get of this clock will fail since it does not have this
clock.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 Documentation/devicetree/bindings/usb/usb-phy.txt |    3 +++
 drivers/usb/phy/omap-usb2.c                       |   28 ++++++++++++++++++++-
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/usb-phy.txt b/Documentation/devicetree/bindings/usb/usb-phy.txt
index 7c5fd89..d5626de 100644
--- a/Documentation/devicetree/bindings/usb/usb-phy.txt
+++ b/Documentation/devicetree/bindings/usb/usb-phy.txt
@@ -24,6 +24,9 @@ Required properties:
 add the address of control module phy power register until a driver for
 control module is added
 
+Optional properties:
+ - has960mhzclk: should be added if the phy needs 960mhz clock
+
 This is usually a subnode of ocp2scp to which it is connected.
 
 usb3phy@4a084400 {
diff --git a/drivers/usb/phy/omap-usb2.c b/drivers/usb/phy/omap-usb2.c
index d36c282..d6612ba 100644
--- a/drivers/usb/phy/omap-usb2.c
+++ b/drivers/usb/phy/omap-usb2.c
@@ -146,6 +146,7 @@ static int __devinit omap_usb2_probe(struct platform_device *pdev)
 	struct omap_usb			*phy;
 	struct usb_otg			*otg;
 	struct resource			*res;
+	struct device_node		*np = pdev->dev.of_node;
 
 	phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
 	if (!phy) {
@@ -190,6 +191,15 @@ static int __devinit omap_usb2_probe(struct platform_device *pdev)
 	}
 	clk_prepare(phy->wkupclk);
 
+	if (of_property_read_bool(np, "has960mhzclk")) {
+		phy->optclk = devm_clk_get(phy->dev, "usb_otg_ss_refclk960m");
+		if (IS_ERR(phy->optclk)) {
+			dev_err(&pdev->dev, "unable to get refclk960m\n");
+			return PTR_ERR(phy->optclk);
+		}
+		clk_prepare(phy->optclk);
+	}
+
 	usb_add_phy(&phy->phy, USB_PHY_TYPE_USB2);
 
 	platform_set_drvdata(pdev, phy);
@@ -204,6 +214,7 @@ static int __devexit omap_usb2_remove(struct platform_device *pdev)
 	struct omap_usb	*phy = platform_get_drvdata(pdev);
 
 	clk_unprepare(phy->wkupclk);
+	clk_unprepare(phy->optclk);
 	usb_remove_phy(&phy->phy);
 
 	return 0;
@@ -217,6 +228,7 @@ static int omap_usb2_runtime_suspend(struct device *dev)
 	struct omap_usb	*phy = platform_get_drvdata(pdev);
 
 	clk_disable(phy->wkupclk);
+	clk_disable(phy->optclk);
 
 	return 0;
 }
@@ -228,8 +240,22 @@ static int omap_usb2_runtime_resume(struct device *dev)
 	struct omap_usb	*phy = platform_get_drvdata(pdev);
 
 	ret = clk_enable(phy->wkupclk);
-	if (ret < 0)
+	if (ret < 0) {
 		dev_err(phy->dev, "Failed to enable wkupclk %d\n", ret);
+		return ret;
+	}
+
+	if (phy->optclk) {
+		ret = clk_enable(phy->optclk);
+		if (ret) {
+			dev_err(phy->dev, "Failed to enable optclk %d\n", ret);
+			goto err;
+		}
+	}
+
+	return 0;
+err:
+	clk_disable(phy->wkupclk);
 
 	return ret;
 }
-- 
1.7.9.5

Re: [PATCH 4/4] usb: phy: omap-usb2: enable 960Mhz clock for omap5

[Felipe Balbi]

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

On Wed, Sep 19, 2012 at 05:00:29PM +0530, Kishon Vijay Abraham I wrote:
> "usb_otg_ss_refclk960m" is needed by usb2 phy present in omap5. For
> omap4, the clk_get of this clock will fail since it does not have this
> clock.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  Documentation/devicetree/bindings/usb/usb-phy.txt |    3 +++
>  drivers/usb/phy/omap-usb2.c                       |   28 ++++++++++++++++++++-
>  2 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/usb-phy.txt b/Documentation/devicetree/bindings/usb/usb-phy.txt
> index 7c5fd89..d5626de 100644
> --- a/Documentation/devicetree/bindings/usb/usb-phy.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-phy.txt
> @@ -24,6 +24,9 @@ Required properties:
>  add the address of control module phy power register until a driver for
>  control module is added
>  
> +Optional properties:
> + - has960mhzclk: should be added if the phy needs 960mhz clock
> +
>  This is usually a subnode of ocp2scp to which it is connected.
>  
>  usb3phy@4a084400 {
> diff --git a/drivers/usb/phy/omap-usb2.c b/drivers/usb/phy/omap-usb2.c
> index d36c282..d6612ba 100644
> --- a/drivers/usb/phy/omap-usb2.c
> +++ b/drivers/usb/phy/omap-usb2.c
> @@ -146,6 +146,7 @@ static int __devinit omap_usb2_probe(struct platform_device *pdev)
>  	struct omap_usb			*phy;
>  	struct usb_otg			*otg;
>  	struct resource			*res;
> +	struct device_node		*np = pdev->dev.of_node;
>  
>  	phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
>  	if (!phy) {
> @@ -190,6 +191,15 @@ static int __devinit omap_usb2_probe(struct platform_device *pdev)
>  	}
>  	clk_prepare(phy->wkupclk);
>  
> +	if (of_property_read_bool(np, "has960mhzclk")) {
> +		phy->optclk = devm_clk_get(phy->dev, "usb_otg_ss_refclk960m");
> +		if (IS_ERR(phy->optclk)) {
> +			dev_err(&pdev->dev, "unable to get refclk960m\n");
> +			return PTR_ERR(phy->optclk);
> +		}
> +		clk_prepare(phy->optclk);
> +	}

instead, can't you just always try to get the clock but ignore the error
if it fails ?

If it works you can set a flag to let you know that you can use that
optional clock.

-- 
balbi

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

Re: [PATCH 4/4] usb: phy: omap-usb2: enable 960Mhz clock for omap5

[Marc Kleine-Budde]

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

On 09/19/2012 01:56 PM, Felipe Balbi wrote:
> On Wed, Sep 19, 2012 at 05:00:29PM +0530, Kishon Vijay Abraham I wrote:
>> "usb_otg_ss_refclk960m" is needed by usb2 phy present in omap5. For
>> omap4, the clk_get of this clock will fail since it does not have this
>> clock.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>>  Documentation/devicetree/bindings/usb/usb-phy.txt |    3 +++
>>  drivers/usb/phy/omap-usb2.c                       |   28 ++++++++++++++++++++-
>>  2 files changed, 30 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/usb-phy.txt b/Documentation/devicetree/bindings/usb/usb-phy.txt
>> index 7c5fd89..d5626de 100644
>> --- a/Documentation/devicetree/bindings/usb/usb-phy.txt
>> +++ b/Documentation/devicetree/bindings/usb/usb-phy.txt
>> @@ -24,6 +24,9 @@ Required properties:
>>  add the address of control module phy power register until a driver for
>>  control module is added
>>  
>> +Optional properties:
>> + - has960mhzclk: should be added if the phy needs 960mhz clock
>> +
>>  This is usually a subnode of ocp2scp to which it is connected.
>>  
>>  usb3phy@4a084400 {
>> diff --git a/drivers/usb/phy/omap-usb2.c b/drivers/usb/phy/omap-usb2.c
>> index d36c282..d6612ba 100644
>> --- a/drivers/usb/phy/omap-usb2.c
>> +++ b/drivers/usb/phy/omap-usb2.c
>> @@ -146,6 +146,7 @@ static int __devinit omap_usb2_probe(struct platform_device *pdev)
>>  	struct omap_usb			*phy;
>>  	struct usb_otg			*otg;
>>  	struct resource			*res;
>> +	struct device_node		*np = pdev->dev.of_node;
>>  
>>  	phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
>>  	if (!phy) {
>> @@ -190,6 +191,15 @@ static int __devinit omap_usb2_probe(struct platform_device *pdev)
>>  	}
>>  	clk_prepare(phy->wkupclk);
>>  
>> +	if (of_property_read_bool(np, "has960mhzclk")) {
>> +		phy->optclk = devm_clk_get(phy->dev, "usb_otg_ss_refclk960m");
>> +		if (IS_ERR(phy->optclk)) {
>> +			dev_err(&pdev->dev, "unable to get refclk960m\n");
>> +			return PTR_ERR(phy->optclk);
>> +		}
>> +		clk_prepare(phy->optclk);
>> +	}
> 
> instead, can't you just always try to get the clock but ignore the error
> if it fails ?

On imx we add no-op dummy clocks, so that the drivers will always find
all needed clocks. If a clk_get fails it's an error.

> If it works you can set a flag to let you know that you can use that
> optional clock.

Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

Re: [PATCH 4/4] usb: phy: omap-usb2: enable 960Mhz clock for omap5

[Felipe Balbi]

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

On Wed, Sep 19, 2012 at 04:45:01PM +0200, Marc Kleine-Budde wrote:
> On 09/19/2012 01:56 PM, Felipe Balbi wrote:
> > On Wed, Sep 19, 2012 at 05:00:29PM +0530, Kishon Vijay Abraham I wrote:
> >> "usb_otg_ss_refclk960m" is needed by usb2 phy present in omap5. For
> >> omap4, the clk_get of this clock will fail since it does not have this
> >> clock.
> >>
> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> >> ---
> >>  Documentation/devicetree/bindings/usb/usb-phy.txt |    3 +++
> >>  drivers/usb/phy/omap-usb2.c                       |   28 ++++++++++++++++++++-
> >>  2 files changed, 30 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/usb/usb-phy.txt b/Documentation/devicetree/bindings/usb/usb-phy.txt
> >> index 7c5fd89..d5626de 100644
> >> --- a/Documentation/devicetree/bindings/usb/usb-phy.txt
> >> +++ b/Documentation/devicetree/bindings/usb/usb-phy.txt
> >> @@ -24,6 +24,9 @@ Required properties:
> >>  add the address of control module phy power register until a driver for
> >>  control module is added
> >>  
> >> +Optional properties:
> >> + - has960mhzclk: should be added if the phy needs 960mhz clock
> >> +
> >>  This is usually a subnode of ocp2scp to which it is connected.
> >>  
> >>  usb3phy@4a084400 {
> >> diff --git a/drivers/usb/phy/omap-usb2.c b/drivers/usb/phy/omap-usb2.c
> >> index d36c282..d6612ba 100644
> >> --- a/drivers/usb/phy/omap-usb2.c
> >> +++ b/drivers/usb/phy/omap-usb2.c
> >> @@ -146,6 +146,7 @@ static int __devinit omap_usb2_probe(struct platform_device *pdev)
> >>  	struct omap_usb			*phy;
> >>  	struct usb_otg			*otg;
> >>  	struct resource			*res;
> >> +	struct device_node		*np = pdev->dev.of_node;
> >>  
> >>  	phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
> >>  	if (!phy) {
> >> @@ -190,6 +191,15 @@ static int __devinit omap_usb2_probe(struct platform_device *pdev)
> >>  	}
> >>  	clk_prepare(phy->wkupclk);
> >>  
> >> +	if (of_property_read_bool(np, "has960mhzclk")) {
> >> +		phy->optclk = devm_clk_get(phy->dev, "usb_otg_ss_refclk960m");
> >> +		if (IS_ERR(phy->optclk)) {
> >> +			dev_err(&pdev->dev, "unable to get refclk960m\n");
> >> +			return PTR_ERR(phy->optclk);
> >> +		}
> >> +		clk_prepare(phy->optclk);
> >> +	}
> > 
> > instead, can't you just always try to get the clock but ignore the error
> > if it fails ?
> 
> On imx we add no-op dummy clocks, so that the drivers will always find
> all needed clocks. If a clk_get fails it's an error.

It's an error from a clk API point of view, it doesn't mean device can't
work without that optional clock. Drivers are free to treat errors
however they like, even by ignoring it and defaulting to some other
behavior.

Adding dummy clocks just bloats the kernel with useless data IMHO.

my 2 cents.

-- 
balbi

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

Re: [PATCH 4/4] usb: phy: omap-usb2: enable 960Mhz clock for omap5

[Marc Kleine-Budde]

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

On 09/19/2012 04:42 PM, Felipe Balbi wrote:
> On Wed, Sep 19, 2012 at 04:45:01PM +0200, Marc Kleine-Budde wrote:
>> On 09/19/2012 01:56 PM, Felipe Balbi wrote:
>>> On Wed, Sep 19, 2012 at 05:00:29PM +0530, Kishon Vijay Abraham I wrote:
>>>> "usb_otg_ss_refclk960m" is needed by usb2 phy present in omap5. For
>>>> omap4, the clk_get of this clock will fail since it does not have this
>>>> clock.
>>>>
>>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>>>> ---
>>>>  Documentation/devicetree/bindings/usb/usb-phy.txt |    3 +++
>>>>  drivers/usb/phy/omap-usb2.c                       |   28 ++++++++++++++++++++-
>>>>  2 files changed, 30 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/usb-phy.txt b/Documentation/devicetree/bindings/usb/usb-phy.txt
>>>> index 7c5fd89..d5626de 100644
>>>> --- a/Documentation/devicetree/bindings/usb/usb-phy.txt
>>>> +++ b/Documentation/devicetree/bindings/usb/usb-phy.txt
>>>> @@ -24,6 +24,9 @@ Required properties:
>>>>  add the address of control module phy power register until a driver for
>>>>  control module is added
>>>>  
>>>> +Optional properties:
>>>> + - has960mhzclk: should be added if the phy needs 960mhz clock
>>>> +
>>>>  This is usually a subnode of ocp2scp to which it is connected.
>>>>  
>>>>  usb3phy@4a084400 {
>>>> diff --git a/drivers/usb/phy/omap-usb2.c b/drivers/usb/phy/omap-usb2.c
>>>> index d36c282..d6612ba 100644
>>>> --- a/drivers/usb/phy/omap-usb2.c
>>>> +++ b/drivers/usb/phy/omap-usb2.c
>>>> @@ -146,6 +146,7 @@ static int __devinit omap_usb2_probe(struct platform_device *pdev)
>>>>  	struct omap_usb			*phy;
>>>>  	struct usb_otg			*otg;
>>>>  	struct resource			*res;
>>>> +	struct device_node		*np = pdev->dev.of_node;
>>>>  
>>>>  	phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
>>>>  	if (!phy) {
>>>> @@ -190,6 +191,15 @@ static int __devinit omap_usb2_probe(struct platform_device *pdev)
>>>>  	}
>>>>  	clk_prepare(phy->wkupclk);
>>>>  
>>>> +	if (of_property_read_bool(np, "has960mhzclk")) {
>>>> +		phy->optclk = devm_clk_get(phy->dev, "usb_otg_ss_refclk960m");
>>>> +		if (IS_ERR(phy->optclk)) {
>>>> +			dev_err(&pdev->dev, "unable to get refclk960m\n");
>>>> +			return PTR_ERR(phy->optclk);
>>>> +		}
>>>> +		clk_prepare(phy->optclk);
>>>> +	}
>>>
>>> instead, can't you just always try to get the clock but ignore the error
>>> if it fails ?
>>
>> On imx we add no-op dummy clocks, so that the drivers will always find
>> all needed clocks. If a clk_get fails it's an error.
> 
> It's an error from a clk API point of view, it doesn't mean device can't
> work without that optional clock. Drivers are free to treat errors
> however they like, even by ignoring it and defaulting to some other
> behavior.

Sure.

> Adding dummy clocks just bloats the kernel with useless data IMHO.

Just wanted to point out there's more than one way to do it.

Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

Re: [PATCH 4/4] usb: phy: omap-usb2: enable 960Mhz clock for omap5

[ABRAHAM, KISHON VIJAY]

Hi,

On Wed, Sep 19, 2012 at 5:26 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Wed, Sep 19, 2012 at 05:00:29PM +0530, Kishon Vijay Abraham I wrote:
>> "usb_otg_ss_refclk960m" is needed by usb2 phy present in omap5. For
>> omap4, the clk_get of this clock will fail since it does not have this
>> clock.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>>  Documentation/devicetree/bindings/usb/usb-phy.txt |    3 +++
>>  drivers/usb/phy/omap-usb2.c                       |   28 ++++++++++++++++++++-
>>  2 files changed, 30 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/usb-phy.txt b/Documentation/devicetree/bindings/usb/usb-phy.txt
>> index 7c5fd89..d5626de 100644
>> --- a/Documentation/devicetree/bindings/usb/usb-phy.txt
>> +++ b/Documentation/devicetree/bindings/usb/usb-phy.txt
>> @@ -24,6 +24,9 @@ Required properties:
>>  add the address of control module phy power register until a driver for
>>  control module is added
>>
>> +Optional properties:
>> + - has960mhzclk: should be added if the phy needs 960mhz clock
>> +
>>  This is usually a subnode of ocp2scp to which it is connected.
>>
>>  usb3phy@4a084400 {
>> diff --git a/drivers/usb/phy/omap-usb2.c b/drivers/usb/phy/omap-usb2.c
>> index d36c282..d6612ba 100644
>> --- a/drivers/usb/phy/omap-usb2.c
>> +++ b/drivers/usb/phy/omap-usb2.c
>> @@ -146,6 +146,7 @@ static int __devinit omap_usb2_probe(struct platform_device *pdev)
>>       struct omap_usb                 *phy;
>>       struct usb_otg                  *otg;
>>       struct resource                 *res;
>> +     struct device_node              *np = pdev->dev.of_node;
>>
>>       phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
>>       if (!phy) {
>> @@ -190,6 +191,15 @@ static int __devinit omap_usb2_probe(struct platform_device *pdev)
>>       }
>>       clk_prepare(phy->wkupclk);
>>
>> +     if (of_property_read_bool(np, "has960mhzclk")) {
>> +             phy->optclk = devm_clk_get(phy->dev, "usb_otg_ss_refclk960m");
>> +             if (IS_ERR(phy->optclk)) {
>> +                     dev_err(&pdev->dev, "unable to get refclk960m\n");
>> +                     return PTR_ERR(phy->optclk);
>> +             }
>> +             clk_prepare(phy->optclk);
>> +     }
>
> instead, can't you just always try to get the clock but ignore the error
> if it fails ?

This clock is needed for usb2 to work in dwc3 (omap5). So we have to
report the error in case we dont get the clock no?

Thanks
Kishon

Re: [PATCH 4/4] usb: phy: omap-usb2: enable 960Mhz clock for omap5

[Felipe Balbi]

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

Hi,

On Wed, Sep 26, 2012 at 11:10:48AM +0530, ABRAHAM, KISHON VIJAY wrote:
> Hi,
> 
> On Wed, Sep 19, 2012 at 5:26 PM, Felipe Balbi <balbi@ti.com> wrote:
> > On Wed, Sep 19, 2012 at 05:00:29PM +0530, Kishon Vijay Abraham I wrote:
> >> "usb_otg_ss_refclk960m" is needed by usb2 phy present in omap5. For
> >> omap4, the clk_get of this clock will fail since it does not have this
> >> clock.
> >>
> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> >> ---
> >>  Documentation/devicetree/bindings/usb/usb-phy.txt |    3 +++
> >>  drivers/usb/phy/omap-usb2.c                       |   28 ++++++++++++++++++++-
> >>  2 files changed, 30 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/usb/usb-phy.txt b/Documentation/devicetree/bindings/usb/usb-phy.txt
> >> index 7c5fd89..d5626de 100644
> >> --- a/Documentation/devicetree/bindings/usb/usb-phy.txt
> >> +++ b/Documentation/devicetree/bindings/usb/usb-phy.txt
> >> @@ -24,6 +24,9 @@ Required properties:
> >>  add the address of control module phy power register until a driver for
> >>  control module is added
> >>
> >> +Optional properties:
> >> + - has960mhzclk: should be added if the phy needs 960mhz clock
> >> +
> >>  This is usually a subnode of ocp2scp to which it is connected.
> >>
> >>  usb3phy@4a084400 {
> >> diff --git a/drivers/usb/phy/omap-usb2.c b/drivers/usb/phy/omap-usb2.c
> >> index d36c282..d6612ba 100644
> >> --- a/drivers/usb/phy/omap-usb2.c
> >> +++ b/drivers/usb/phy/omap-usb2.c
> >> @@ -146,6 +146,7 @@ static int __devinit omap_usb2_probe(struct platform_device *pdev)
> >>       struct omap_usb                 *phy;
> >>       struct usb_otg                  *otg;
> >>       struct resource                 *res;
> >> +     struct device_node              *np = pdev->dev.of_node;
> >>
> >>       phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
> >>       if (!phy) {
> >> @@ -190,6 +191,15 @@ static int __devinit omap_usb2_probe(struct platform_device *pdev)
> >>       }
> >>       clk_prepare(phy->wkupclk);
> >>
> >> +     if (of_property_read_bool(np, "has960mhzclk")) {
> >> +             phy->optclk = devm_clk_get(phy->dev, "usb_otg_ss_refclk960m");
> >> +             if (IS_ERR(phy->optclk)) {
> >> +                     dev_err(&pdev->dev, "unable to get refclk960m\n");
> >> +                     return PTR_ERR(phy->optclk);
> >> +             }
> >> +             clk_prepare(phy->optclk);
> >> +     }
> >
> > instead, can't you just always try to get the clock but ignore the error
> > if it fails ?
> 
> This clock is needed for usb2 to work in dwc3 (omap5). So we have to
> report the error in case we dont get the clock no?

sure, but you don't need to bail out. Print a warning message such as:

dev_dbg(&pdev->dev, "couldn't get refclk960m, trying without\n");

or something similar. Then you don't need to add this has960mhzclk flag
to dts files.

-- 
balbi

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

Re: [PATCH 4/4] usb: phy: omap-usb2: enable 960Mhz clock for omap5

[ABRAHAM, KISHON VIJAY]

Hi,

On Wed, Sep 26, 2012 at 11:57 PM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Wed, Sep 26, 2012 at 11:10:48AM +0530, ABRAHAM, KISHON VIJAY wrote:
>> Hi,
>>
>> On Wed, Sep 19, 2012 at 5:26 PM, Felipe Balbi <balbi@ti.com> wrote:
>> > On Wed, Sep 19, 2012 at 05:00:29PM +0530, Kishon Vijay Abraham I wrote:
>> >> "usb_otg_ss_refclk960m" is needed by usb2 phy present in omap5. For
>> >> omap4, the clk_get of this clock will fail since it does not have this
>> >> clock.
>> >>
>> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> >> ---
>> >>  Documentation/devicetree/bindings/usb/usb-phy.txt |    3 +++
>> >>  drivers/usb/phy/omap-usb2.c                       |   28 ++++++++++++++++++++-
>> >>  2 files changed, 30 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/usb/usb-phy.txt b/Documentation/devicetree/bindings/usb/usb-phy.txt
>> >> index 7c5fd89..d5626de 100644
>> >> --- a/Documentation/devicetree/bindings/usb/usb-phy.txt
>> >> +++ b/Documentation/devicetree/bindings/usb/usb-phy.txt
>> >> @@ -24,6 +24,9 @@ Required properties:
>> >>  add the address of control module phy power register until a driver for
>> >>  control module is added
>> >>
>> >> +Optional properties:
>> >> + - has960mhzclk: should be added if the phy needs 960mhz clock
>> >> +
>> >>  This is usually a subnode of ocp2scp to which it is connected.
>> >>
>> >>  usb3phy@4a084400 {
>> >> diff --git a/drivers/usb/phy/omap-usb2.c b/drivers/usb/phy/omap-usb2.c
>> >> index d36c282..d6612ba 100644
>> >> --- a/drivers/usb/phy/omap-usb2.c
>> >> +++ b/drivers/usb/phy/omap-usb2.c
>> >> @@ -146,6 +146,7 @@ static int __devinit omap_usb2_probe(struct platform_device *pdev)
>> >>       struct omap_usb                 *phy;
>> >>       struct usb_otg                  *otg;
>> >>       struct resource                 *res;
>> >> +     struct device_node              *np = pdev->dev.of_node;
>> >>
>> >>       phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
>> >>       if (!phy) {
>> >> @@ -190,6 +191,15 @@ static int __devinit omap_usb2_probe(struct platform_device *pdev)
>> >>       }
>> >>       clk_prepare(phy->wkupclk);
>> >>
>> >> +     if (of_property_read_bool(np, "has960mhzclk")) {
>> >> +             phy->optclk = devm_clk_get(phy->dev, "usb_otg_ss_refclk960m");
>> >> +             if (IS_ERR(phy->optclk)) {
>> >> +                     dev_err(&pdev->dev, "unable to get refclk960m\n");
>> >> +                     return PTR_ERR(phy->optclk);
>> >> +             }
>> >> +             clk_prepare(phy->optclk);
>> >> +     }
>> >
>> > instead, can't you just always try to get the clock but ignore the error
>> > if it fails ?
>>
>> This clock is needed for usb2 to work in dwc3 (omap5). So we have to
>> report the error in case we dont get the clock no?
>
> sure, but you don't need to bail out. Print a warning message such as:
>
> dev_dbg(&pdev->dev, "couldn't get refclk960m, trying without\n");

but then we'll get this debug message for omap4 which actually doesn't
need 960m clk.

Thanks
Kishon

Re: [PATCH 4/4] usb: phy: omap-usb2: enable 960Mhz clock for omap5

[Felipe Balbi]

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

On Thu, Sep 27, 2012 at 10:43:06AM +0530, ABRAHAM, KISHON VIJAY wrote:
> Hi,
> 
> On Wed, Sep 26, 2012 at 11:57 PM, Felipe Balbi <balbi@ti.com> wrote:
> > Hi,
> >
> > On Wed, Sep 26, 2012 at 11:10:48AM +0530, ABRAHAM, KISHON VIJAY wrote:
> >> Hi,
> >>
> >> On Wed, Sep 19, 2012 at 5:26 PM, Felipe Balbi <balbi@ti.com> wrote:
> >> > On Wed, Sep 19, 2012 at 05:00:29PM +0530, Kishon Vijay Abraham I wrote:
> >> >> "usb_otg_ss_refclk960m" is needed by usb2 phy present in omap5. For
> >> >> omap4, the clk_get of this clock will fail since it does not have this
> >> >> clock.
> >> >>
> >> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> >> >> ---
> >> >>  Documentation/devicetree/bindings/usb/usb-phy.txt |    3 +++
> >> >>  drivers/usb/phy/omap-usb2.c                       |   28 ++++++++++++++++++++-
> >> >>  2 files changed, 30 insertions(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/Documentation/devicetree/bindings/usb/usb-phy.txt b/Documentation/devicetree/bindings/usb/usb-phy.txt
> >> >> index 7c5fd89..d5626de 100644
> >> >> --- a/Documentation/devicetree/bindings/usb/usb-phy.txt
> >> >> +++ b/Documentation/devicetree/bindings/usb/usb-phy.txt
> >> >> @@ -24,6 +24,9 @@ Required properties:
> >> >>  add the address of control module phy power register until a driver for
> >> >>  control module is added
> >> >>
> >> >> +Optional properties:
> >> >> + - has960mhzclk: should be added if the phy needs 960mhz clock
> >> >> +
> >> >>  This is usually a subnode of ocp2scp to which it is connected.
> >> >>
> >> >>  usb3phy@4a084400 {
> >> >> diff --git a/drivers/usb/phy/omap-usb2.c b/drivers/usb/phy/omap-usb2.c
> >> >> index d36c282..d6612ba 100644
> >> >> --- a/drivers/usb/phy/omap-usb2.c
> >> >> +++ b/drivers/usb/phy/omap-usb2.c
> >> >> @@ -146,6 +146,7 @@ static int __devinit omap_usb2_probe(struct platform_device *pdev)
> >> >>       struct omap_usb                 *phy;
> >> >>       struct usb_otg                  *otg;
> >> >>       struct resource                 *res;
> >> >> +     struct device_node              *np = pdev->dev.of_node;
> >> >>
> >> >>       phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
> >> >>       if (!phy) {
> >> >> @@ -190,6 +191,15 @@ static int __devinit omap_usb2_probe(struct platform_device *pdev)
> >> >>       }
> >> >>       clk_prepare(phy->wkupclk);
> >> >>
> >> >> +     if (of_property_read_bool(np, "has960mhzclk")) {
> >> >> +             phy->optclk = devm_clk_get(phy->dev, "usb_otg_ss_refclk960m");
> >> >> +             if (IS_ERR(phy->optclk)) {
> >> >> +                     dev_err(&pdev->dev, "unable to get refclk960m\n");
> >> >> +                     return PTR_ERR(phy->optclk);
> >> >> +             }
> >> >> +             clk_prepare(phy->optclk);
> >> >> +     }
> >> >
> >> > instead, can't you just always try to get the clock but ignore the error
> >> > if it fails ?
> >>
> >> This clock is needed for usb2 to work in dwc3 (omap5). So we have to
> >> report the error in case we dont get the clock no?
> >
> > sure, but you don't need to bail out. Print a warning message such as:
> >
> > dev_dbg(&pdev->dev, "couldn't get refclk960m, trying without\n");
> 
> but then we'll get this debug message for omap4 which actually doesn't
> need 960m clk.

then make it dev_vdbg(). It's just a debugging message, it's not saying
it will fail, it's just stating that clock isn't present and we're
trying without because it's known that some versions don't need that
clock.

Another way to do it, would be to request or not the extra clock, based
on the Revision Register (if this IP has one).

-- 
balbi

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