[PATCH 1/3] usb: phy: Add RCAR Gen2 USB phy

[PATCH 1/3] usb: phy: Add RCAR Gen2 USB phy

[Valentine Barshak]

This adds RCAR Gen2 USB phy support. The driver configures
USB channels 0/2, which are shared between PCI USB hosts and
USBHS/USBSS devices. It also controls internal USBHS phy.

Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
---
 drivers/usb/phy/Kconfig                         |  13 ++
 drivers/usb/phy/Makefile                        |   1 +
 drivers/usb/phy/phy-rcar-gen2-usb.c             | 271 ++++++++++++++++++++++++
 include/linux/platform_data/usb-rcar-gen2-phy.h |  22 ++
 4 files changed, 307 insertions(+)
 create mode 100644 drivers/usb/phy/phy-rcar-gen2-usb.c
 create mode 100644 include/linux/platform_data/usb-rcar-gen2-phy.h

diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index d5589f9..297062c 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -214,6 +214,19 @@ config USB_RCAR_PHY
 	  To compile this driver as a module, choose M here: the
 	  module will be called phy-rcar-usb.
 
+config USB_RCAR_GEN2_PHY
+	tristate "Renesas R-Car Gen2 USB PHY support"
+	depends on ARCH_R8A7790 || ARCH_R8A7791
+	select USB_PHY
+	help
+	  Say Y here to add support for the Renesas R-Car Gen2 USB PHY driver.
+	  It is typically used to control internal USB PHY for USBHS,
+	  and to configure shared USB channels 0 and 2.
+	  This driver supports R8A7790 and R8A7791.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called phy-rcar-gen2-usb.
+
 config USB_ULPI
 	bool "Generic ULPI Transceiver Driver"
 	depends on ARM
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index 2135e85..8c5b147 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -29,5 +29,6 @@ obj-$(CONFIG_USB_MSM_OTG)		+= phy-msm-usb.o
 obj-$(CONFIG_USB_MV_OTG)		+= phy-mv-usb.o
 obj-$(CONFIG_USB_MXS_PHY)		+= phy-mxs-usb.o
 obj-$(CONFIG_USB_RCAR_PHY)		+= phy-rcar-usb.o
+obj-$(CONFIG_USB_RCAR_GEN2_PHY)		+= phy-rcar-gen2-usb.o
 obj-$(CONFIG_USB_ULPI)			+= phy-ulpi.o
 obj-$(CONFIG_USB_ULPI_VIEWPORT)		+= phy-ulpi-viewport.o
diff --git a/drivers/usb/phy/phy-rcar-gen2-usb.c b/drivers/usb/phy/phy-rcar-gen2-usb.c
new file mode 100644
index 0000000..b7b2102
--- /dev/null
+++ b/drivers/usb/phy/phy-rcar-gen2-usb.c
@@ -0,0 +1,271 @@
+/*
+ * Renesas R-Car Gen2 USB phy driver
+ *
+ * Copyright (C) 2013 Renesas Solutions Corp.
+ * Copyright (C) 2013 Cogent Embedded, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/usb/otg.h>
+#include <linux/platform_data/usb-rcar-gen2-phy.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/module.h>
+
+struct rcar_gen2_usb_phy_priv {
+	struct usb_phy phy;
+	void __iomem *base;
+	struct clk *clk;
+	spinlock_t lock;
+	int usecount;
+	u32 ugctrl2;
+};
+
+#define usb_phy_to_priv(p) container_of(p, struct rcar_gen2_usb_phy_priv, phy)
+
+/* Low Power Status register */
+#define USBHS_LPSTS_REG			0x02
+#define USBHS_LPSTS_SUSPM		(1 << 14)
+
+/* USB General control register */
+#define USBHS_UGCTRL_REG		0x80
+#define USBHS_UGCTRL_CONNECT		(1 << 2)
+#define USBHS_UGCTRL_PLLRESET		(1 << 0)
+
+/* USB General control register 2 */
+#define USBHS_UGCTRL2_REG		0x84
+#define USBHS_UGCTRL2_USB0_PCI		(1 << 4)
+#define USBHS_UGCTRL2_USB0_HS		(3 << 4)
+#define USBHS_UGCTRL2_USB2_PCI		(0 << 31)
+#define USBHS_UGCTRL2_USB2_SS		(1 << 31)
+
+/* USB General status register */
+#define USBHS_UGSTS_REG			0x88
+#define USBHS_UGSTS_LOCK		(3 << 8)
+
+/* Setup USB channels */
+static void __rcar_gen2_usb_phy_setup(struct rcar_gen2_usb_phy_priv *priv)
+{
+	u32 val;
+
+	clk_prepare_enable(priv->clk);
+
+	/* Set USB channels in the USBHS UGCTRL2 register */
+	val = ioread32(priv->base);
+	val &= ~(USBHS_UGCTRL2_USB0_HS | USBHS_UGCTRL2_USB2_SS);
+	val |= priv->ugctrl2;
+	iowrite32(val, priv->base);
+}
+
+/* Disable USBHS internal phy */
+static int __rcar_gen2_usbhs_phy_enable(void __iomem *base)
+{
+	u32 val;
+	int i;
+
+	/* USBHS PHY power on */
+	val = ioread32(base + USBHS_UGCTRL_REG);
+	val &= ~USBHS_UGCTRL_PLLRESET;
+	iowrite32(val, base + USBHS_UGCTRL_REG);
+
+	val = ioread16(base + USBHS_LPSTS_REG);
+	val |= USBHS_LPSTS_SUSPM;
+	iowrite16(val, base + USBHS_LPSTS_REG);
+
+	for (i = 0; i < 20; i++) {
+		val = ioread32(base + USBHS_UGSTS_REG);
+		if ((val & USBHS_UGSTS_LOCK) = USBHS_UGSTS_LOCK) {
+			val = ioread32(base + USBHS_UGCTRL_REG);
+			val |= USBHS_UGCTRL_CONNECT;
+			iowrite32(val, base + USBHS_UGCTRL_REG);
+			return 0;
+		}
+		udelay(1);
+	}
+
+	/* Timed out waiting for the PLL lock */
+	return -ETIMEDOUT;
+}
+
+/* Ensable USBHS internal phy */
+static int __rcar_gen2_usbhs_phy_disable(void __iomem *base)
+{
+	u32 val;
+
+	/* USBHS PHY power off */
+	val = ioread32(base + USBHS_UGCTRL_REG);
+	val &= ~USBHS_UGCTRL_CONNECT;
+	iowrite32(val, base + USBHS_UGCTRL_REG);
+
+	val = ioread16(base + USBHS_LPSTS_REG);
+	val &= ~USBHS_LPSTS_SUSPM;
+	iowrite16(val, base + USBHS_LPSTS_REG);
+
+	val = ioread32(base + USBHS_UGCTRL_REG);
+	val |= USBHS_UGCTRL_PLLRESET;
+	iowrite32(val, base + USBHS_UGCTRL_REG);
+	return 0;
+}
+
+static int rcar_gen2_usb_phy_set_suspend(struct usb_phy *phy, int suspend)
+{
+	struct rcar_gen2_usb_phy_priv *priv = usb_phy_to_priv(phy);
+	unsigned long flags;
+	int retval;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	if (suspend) {
+		/* Suspend USBHS internal phy */
+		retval = __rcar_gen2_usbhs_phy_disable(priv->base);
+		/*
+		 * If nothing else is using USB channel 0/2
+		 * disable the clocks as well
+		 */
+		if (priv->usecount = 1) {
+			clk_disable_unprepare(priv->clk);
+			priv->usecount--;
+		}
+	} else {
+		/*
+		 * Enable the clock and setup USB channels
+		 * if needed.
+		 */
+		if (!priv->usecount) {
+			priv->usecount++;
+			__rcar_gen2_usb_phy_setup(priv);
+		}
+		/* Resume USBHS internal phy */
+		retval = __rcar_gen2_usbhs_phy_enable(priv->base);
+	}
+	spin_unlock_irqrestore(&priv->lock, flags);
+	return retval;
+}
+
+static int rcar_gen2_usb_phy_init(struct usb_phy *phy)
+{
+	struct rcar_gen2_usb_phy_priv *priv = usb_phy_to_priv(phy);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	/*
+	 * Enable the clock and setup USB channels
+	 * if it's the first user
+	 */
+	if (!priv->usecount++)
+		__rcar_gen2_usb_phy_setup(priv);
+	spin_unlock_irqrestore(&priv->lock, flags);
+	return 0;
+}
+
+static void rcar_gen2_usb_phy_shutdown(struct usb_phy *phy)
+{
+	struct rcar_gen2_usb_phy_priv *priv = usb_phy_to_priv(phy);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	if (!priv->usecount) {
+		dev_warn(phy->dev, "Trying to disable phy with 0 usecount\n");
+		goto out;
+	}
+
+	/* Disable everything if it's the last user */
+	if (!--priv->usecount) {
+		__rcar_gen2_usbhs_phy_disable(priv->base);
+		clk_disable_unprepare(priv->clk);
+	}
+out:
+	spin_unlock_irqrestore(&priv->lock, flags);
+}
+
+static int rcar_gen2_usb_phy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rcar_gen2_phy_platform_data *pdata;
+	struct rcar_gen2_usb_phy_priv *priv;
+	struct resource *res;
+	void __iomem *base;
+	struct clk *clk;
+	int retval;
+
+	pdata = dev_get_platdata(&pdev->dev);
+	if (!pdata) {
+		dev_err(dev, "No platform data\n");
+		return -EINVAL;
+	}
+
+	clk = devm_clk_get(&pdev->dev, "usbhs");
+	if (IS_ERR(clk)) {
+		dev_err(&pdev->dev, "Can't get the clock\n");
+		return PTR_ERR(clk);
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(dev, "No memory resource\n");
+		return -ENODEV;
+	}
+
+	base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(base)) {
+		dev_err(dev, "Failed to ioremap resource\n");
+		return PTR_ERR(base);
+	}
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		dev_err(dev, "Memory allocation failed\n");
+		return -ENOMEM;
+	}
+
+	spin_lock_init(&priv->lock);
+	priv->clk = clk;
+	priv->base = base;
+	priv->ugctrl2 = pdata->chan0_pci ?
+			USBHS_UGCTRL2_USB0_PCI : USBHS_UGCTRL2_USB0_HS;
+	priv->ugctrl2 |= pdata->chan2_pci ?
+			USBHS_UGCTRL2_USB2_PCI : USBHS_UGCTRL2_USB2_SS;
+	priv->phy.dev = dev;
+	priv->phy.label = dev_name(dev);
+	priv->phy.init = rcar_gen2_usb_phy_init;
+	priv->phy.shutdown = rcar_gen2_usb_phy_shutdown;
+	priv->phy.set_suspend = rcar_gen2_usb_phy_set_suspend;
+
+	retval = usb_add_phy(&priv->phy, USB_PHY_TYPE_USB2);
+	if (retval < 0) {
+		dev_err(dev, "Failed to add USB phy\n");
+		return retval;
+	}
+
+	platform_set_drvdata(pdev, priv);
+
+	return retval;
+}
+
+static int rcar_gen2_usb_phy_remove(struct platform_device *pdev)
+{
+	struct rcar_gen2_usb_phy_priv *priv = platform_get_drvdata(pdev);
+
+	usb_remove_phy(&priv->phy);
+
+	return 0;
+}
+
+static struct platform_driver rcar_gen2_usb_phy_driver = {
+	.driver = {
+		.name = "usb_phy_rcar_gen2",
+	},
+	.probe = rcar_gen2_usb_phy_probe,
+	.remove = rcar_gen2_usb_phy_remove,
+};
+
+module_platform_driver(rcar_gen2_usb_phy_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Renesas R-Car Gen2 USB phy");
+MODULE_AUTHOR("Valentine Barshak <valentine.barshak@cogentembedded.com>");
diff --git a/include/linux/platform_data/usb-rcar-gen2-phy.h b/include/linux/platform_data/usb-rcar-gen2-phy.h
new file mode 100644
index 0000000..dd3ba46
--- /dev/null
+++ b/include/linux/platform_data/usb-rcar-gen2-phy.h
@@ -0,0 +1,22 @@
+/*
+ * Copyright (C) 2013 Renesas Solutions Corp.
+ * Copyright (C) 2013 Cogent Embedded, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __USB_RCAR_GEN2_PHY_H
+#define __USB_RCAR_GEN2_PHY_H
+
+#include <linux/types.h>
+
+struct rcar_gen2_phy_platform_data {
+	/* USB channel 0 configuration */
+	bool chan0_pci:1;	/* true: PCI USB host 0, false: USBHS */
+	/* USB channel 2 configuration */
+	bool chan2_pci:1;	/* true: PCI USB host 2, false: USBSS */
+};
+
+#endif
-- 
1.8.3.1

Re: [PATCH 1/3] usb: phy: Add RCAR Gen2 USB phy

[Valentine]

On 10/08/2013 03:27 AM, Valentine Barshak wrote:
> This adds RCAR Gen2 USB phy support. The driver configures
> USB channels 0/2, which are shared between PCI USB hosts and
> USBHS/USBSS devices. It also controls internal USBHS phy.

[snip]

> diff --git a/drivers/usb/phy/phy-rcar-gen2-usb.c b/drivers/usb/phy/phy-rcar-gen2-usb.c
> new file mode 100644
> index 0000000..b7b2102
> --- /dev/null
> +++ b/drivers/usb/phy/phy-rcar-gen2-usb.c
> @@ -0,0 +1,271 @@

[snip]

> +
> +/* Low Power Status register */
> +#define USBHS_LPSTS_REG			0x02
> +#define USBHS_LPSTS_SUSPM		(1 << 14)
> +
> +/* USB General control register */
> +#define USBHS_UGCTRL_REG		0x80
> +#define USBHS_UGCTRL_CONNECT		(1 << 2)
> +#define USBHS_UGCTRL_PLLRESET		(1 << 0)
> +
> +/* USB General control register 2 */
> +#define USBHS_UGCTRL2_REG		0x84
> +#define USBHS_UGCTRL2_USB0_PCI		(1 << 4)
> +#define USBHS_UGCTRL2_USB0_HS		(3 << 4)
> +#define USBHS_UGCTRL2_USB2_PCI		(0 << 31)
> +#define USBHS_UGCTRL2_USB2_SS		(1 << 31)
> +
> +/* USB General status register */
> +#define USBHS_UGSTS_REG			0x88
> +#define USBHS_UGSTS_LOCK		(3 << 8)
> +

Please, note that the latest H2/M2 hardware manuals have the USB General 
status register (UGSTS) at offset 0x190.

(The PHY driver uses offsets relative to USBHS base + 0x100)

The PLL LOCK is supposed to be in bits 0-1 of the UGSTS register.
However, I've found that this is not correct, and the value read
from 0x190 offset is always 0.

The PLL LOCK status is actually read from bits 8-9 at the offset
of 0x188.

So when UTMI is suspended, the value read from 0x188 is 0x0.
When PLL is taken out of reset and normal UTMI mode is selected in
the LPSTS register, the register at 0x188 reads 0x300.

Thanks,
Val.

Re: [PATCH 1/3] usb: phy: Add RCAR Gen2 USB phy

[Kuninori Morimoto]

Hi Valentine

Thank you for your patch

> +/* Setup USB channels */
> +static void __rcar_gen2_usb_phy_setup(struct rcar_gen2_usb_phy_priv *priv)
> +{
> +	u32 val;
> +
> +	clk_prepare_enable(priv->clk);
> +
> +	/* Set USB channels in the USBHS UGCTRL2 register */
> +	val = ioread32(priv->base);
> +	val &= ~(USBHS_UGCTRL2_USB0_HS | USBHS_UGCTRL2_USB2_SS);
> +	val |= priv->ugctrl2;
> +	iowrite32(val, priv->base);
> +}

From my point of view, if you use clk_enable() on setup(),
then, it is easy to read if it has exit() or similar name function
which calls clk_disable()

> +static int rcar_gen2_usb_phy_set_suspend(struct usb_phy *phy, int suspend)
> +{
> +	struct rcar_gen2_usb_phy_priv *priv = usb_phy_to_priv(phy);
> +	unsigned long flags;
> +	int retval;
> +
> +	spin_lock_irqsave(&priv->lock, flags);
> +	if (suspend) {
> +		/* Suspend USBHS internal phy */
> +		retval = __rcar_gen2_usbhs_phy_disable(priv->base);
> +		/*
> +		 * If nothing else is using USB channel 0/2
> +		 * disable the clocks as well
> +		 */
> +		if (priv->usecount = 1) {
> +			clk_disable_unprepare(priv->clk);
> +			priv->usecount--;
> +		}
> +	} else {
> +		/*
> +		 * Enable the clock and setup USB channels
> +		 * if needed.
> +		 */
> +		if (!priv->usecount) {
> +			priv->usecount++;
> +			__rcar_gen2_usb_phy_setup(priv);
> +		}
> +		/* Resume USBHS internal phy */
> +		retval = __rcar_gen2_usbhs_phy_enable(priv->base);
> +	}

Are these usecount++/usecount-- position correct ?

> +static int rcar_gen2_usb_phy_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct rcar_gen2_phy_platform_data *pdata;
> +	struct rcar_gen2_usb_phy_priv *priv;
> +	struct resource *res;
> +	void __iomem *base;
> +	struct clk *clk;
> +	int retval;
> +
> +	pdata = dev_get_platdata(&pdev->dev);
> +	if (!pdata) {
> +		dev_err(dev, "No platform data\n");
> +		return -EINVAL;
> +	}
> +
> +	clk = devm_clk_get(&pdev->dev, "usbhs");
> +	if (IS_ERR(clk)) {
> +		dev_err(&pdev->dev, "Can't get the clock\n");
> +		return PTR_ERR(clk);
> +	}

This case (if you use usb_phy_rcar_gen2 driver),
you can use pm_runtime_xxx instead of clk_get/enable/disable()

Re: [PATCH 1/3] usb: phy: Add RCAR Gen2 USB phy

[Valentine]

On 10/08/2013 07:27 AM, Kuninori Morimoto wrote:
>
> Hi Valentine

Hi Morimoto-san,

>
> Thank you for your patch

Thanks for looking at it.

>
>> +/* Setup USB channels */
>> +static void __rcar_gen2_usb_phy_setup(struct rcar_gen2_usb_phy_priv *priv)
>> +{
>> +	u32 val;
>> +
>> +	clk_prepare_enable(priv->clk);
>> +
>> +	/* Set USB channels in the USBHS UGCTRL2 register */
>> +	val = ioread32(priv->base);
>> +	val &= ~(USBHS_UGCTRL2_USB0_HS | USBHS_UGCTRL2_USB2_SS);
>> +	val |= priv->ugctrl2;
>> +	iowrite32(val, priv->base);
>> +}
>
>  From my point of view, if you use clk_enable() on setup(),
> then, it is easy to read if it has exit() or similar name function
> which calls clk_disable()

Since in this case all is needed is to disable the clocks, I've decided 
not to put it in a separate exit function. I'll add one for better 
readability.

>
>> +static int rcar_gen2_usb_phy_set_suspend(struct usb_phy *phy, int suspend)
>> +{
>> +	struct rcar_gen2_usb_phy_priv *priv = usb_phy_to_priv(phy);
>> +	unsigned long flags;
>> +	int retval;
>> +
>> +	spin_lock_irqsave(&priv->lock, flags);
>> +	if (suspend) {
>> +		/* Suspend USBHS internal phy */
>> +		retval = __rcar_gen2_usbhs_phy_disable(priv->base);
>> +		/*
>> +		 * If nothing else is using USB channel 0/2
>> +		 * disable the clocks as well
>> +		 */
>> +		if (priv->usecount = 1) {
>> +			clk_disable_unprepare(priv->clk);
>> +			priv->usecount--;
>> +		}
>> +	} else {
>> +		/*
>> +		 * Enable the clock and setup USB channels
>> +		 * if needed.
>> +		 */
>> +		if (!priv->usecount) {
>> +			priv->usecount++;
>> +			__rcar_gen2_usb_phy_setup(priv);
>> +		}
>> +		/* Resume USBHS internal phy */
>> +		retval = __rcar_gen2_usbhs_phy_enable(priv->base);
>> +	}
>
> Are these usecount++/usecount-- position correct ?

The idea was to disable the clocks here if the phy is not used by other 
drivers (PCI USB host or USBSS), so that suspending the gadget would 
disable USBHS clocks. However, this needs phy enabled before the 
shutdown is called. I guess I'll drop the clock handling here and leave 
it solely to init/shutdown callbacks.

>
>> +static int rcar_gen2_usb_phy_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct rcar_gen2_phy_platform_data *pdata;
>> +	struct rcar_gen2_usb_phy_priv *priv;
>> +	struct resource *res;
>> +	void __iomem *base;
>> +	struct clk *clk;
>> +	int retval;
>> +
>> +	pdata = dev_get_platdata(&pdev->dev);
>> +	if (!pdata) {
>> +		dev_err(dev, "No platform data\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	clk = devm_clk_get(&pdev->dev, "usbhs");
>> +	if (IS_ERR(clk)) {
>> +		dev_err(&pdev->dev, "Can't get the clock\n");
>> +		return PTR_ERR(clk);
>> +	}
>
> This case (if you use usb_phy_rcar_gen2 driver),
> you can use pm_runtime_xxx instead of clk_get/enable/disable()
>

Yes, I could. The reason I did not is that I'm not sure that a phy 
driver should use runtime PM, since it is actually mastered by other 
drivers which are supposed to control its power via init/shutdown and 
set_suspend callbacks. Thus, looks like the phy driver can't really 
auto-suspend and doesn't really support runtime PM.

I think that handling clocks in the init/shutdown is a bit cleaner.
It gives us more control over the phy power, where pm_runtime_xxx will 
do nothing if CONFIG_PM_RUNTIME is disabled.

Thanks,
Val.

Re: [PATCH 1/3] usb: phy: Add RCAR Gen2 USB phy

[Kuninori Morimoto]

Hi Valentine

> >  From my point of view, if you use clk_enable() on setup(),
> > then, it is easy to read if it has exit() or similar name function
> > which calls clk_disable()
> 
> Since in this case all is needed is to disable the clocks, I've decided 
> not to put it in a separate exit function. I'll add one for better 
> readability.

Thank you

> > Are these usecount++/usecount-- position correct ?
> 
> The idea was to disable the clocks here if the phy is not used by other 
> drivers (PCI USB host or USBSS), so that suspending the gadget would 
> disable USBHS clocks. However, this needs phy enabled before the 
> shutdown is called. I guess I'll drop the clock handling here and leave 
> it solely to init/shutdown callbacks.

Thank you

> >> +	clk = devm_clk_get(&pdev->dev, "usbhs");
> >> +	if (IS_ERR(clk)) {
> >> +		dev_err(&pdev->dev, "Can't get the clock\n");
> >> +		return PTR_ERR(clk);
> >> +	}
> >
> > This case (if you use usb_phy_rcar_gen2 driver),
> > you can use pm_runtime_xxx instead of clk_get/enable/disable()
> >
> 
> Yes, I could. The reason I did not is that I'm not sure that a phy 
> driver should use runtime PM, since it is actually mastered by other 
> drivers which are supposed to control its power via init/shutdown and 
> set_suspend callbacks. Thus, looks like the phy driver can't really 
> auto-suspend and doesn't really support runtime PM.
> 
> I think that handling clocks in the init/shutdown is a bit cleaner.
> It gives us more control over the phy power, where pm_runtime_xxx will 
> do nothing if CONFIG_PM_RUNTIME is disabled.

OK, it is reasonable for me

Best regards
---
Kuninori Morimoto

[PATCH 1/3] usb: phy: Add RCAR Gen2 USB phy

[Valentine Barshak]

This adds RCAR Gen2 USB phy support. The driver configures
USB channels 0/2 which are shared between PCI USB hosts and
USBHS/USBSS devices. It also controls internal USBHS phy.

Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
---
 drivers/usb/phy/Kconfig                         |  13 ++
 drivers/usb/phy/Makefile                        |   1 +
 drivers/usb/phy/phy-rcar-gen2-usb.c             | 255 ++++++++++++++++++++++++
 include/linux/platform_data/usb-rcar-gen2-phy.h |  22 ++
 4 files changed, 291 insertions(+)
 create mode 100644 drivers/usb/phy/phy-rcar-gen2-usb.c
 create mode 100644 include/linux/platform_data/usb-rcar-gen2-phy.h

diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index d5589f9..297062c 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -214,6 +214,19 @@ config USB_RCAR_PHY
 	  To compile this driver as a module, choose M here: the
 	  module will be called phy-rcar-usb.
 
+config USB_RCAR_GEN2_PHY
+	tristate "Renesas R-Car Gen2 USB PHY support"
+	depends on ARCH_R8A7790 || ARCH_R8A7791
+	select USB_PHY
+	help
+	  Say Y here to add support for the Renesas R-Car Gen2 USB PHY driver.
+	  It is typically used to control internal USB PHY for USBHS,
+	  and to configure shared USB channels 0 and 2.
+	  This driver supports R8A7790 and R8A7791.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called phy-rcar-gen2-usb.
+
 config USB_ULPI
 	bool "Generic ULPI Transceiver Driver"
 	depends on ARM
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index 2135e85..8c5b147 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -29,5 +29,6 @@ obj-$(CONFIG_USB_MSM_OTG)		+= phy-msm-usb.o
 obj-$(CONFIG_USB_MV_OTG)		+= phy-mv-usb.o
 obj-$(CONFIG_USB_MXS_PHY)		+= phy-mxs-usb.o
 obj-$(CONFIG_USB_RCAR_PHY)		+= phy-rcar-usb.o
+obj-$(CONFIG_USB_RCAR_GEN2_PHY)		+= phy-rcar-gen2-usb.o
 obj-$(CONFIG_USB_ULPI)			+= phy-ulpi.o
 obj-$(CONFIG_USB_ULPI_VIEWPORT)		+= phy-ulpi-viewport.o
diff --git a/drivers/usb/phy/phy-rcar-gen2-usb.c b/drivers/usb/phy/phy-rcar-gen2-usb.c
new file mode 100644
index 0000000..a2e6f9f
--- /dev/null
+++ b/drivers/usb/phy/phy-rcar-gen2-usb.c
@@ -0,0 +1,255 @@
+/*
+ * Renesas R-Car Gen2 USB phy driver
+ *
+ * Copyright (C) 2013 Renesas Solutions Corp.
+ * Copyright (C) 2013 Cogent Embedded, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/usb/otg.h>
+#include <linux/platform_data/usb-rcar-gen2-phy.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/module.h>
+
+struct rcar_gen2_usb_phy_priv {
+	struct usb_phy phy;
+	void __iomem *base;
+	struct clk *clk;
+	spinlock_t lock;
+	int usecount;
+	u32 ugctrl2;
+};
+
+#define usb_phy_to_priv(p) container_of(p, struct rcar_gen2_usb_phy_priv, phy)
+
+/* Low Power Status register */
+#define USBHS_LPSTS_REG			0x02
+#define USBHS_LPSTS_SUSPM		(1 << 14)
+
+/* USB General control register */
+#define USBHS_UGCTRL_REG		0x80
+#define USBHS_UGCTRL_CONNECT		(1 << 2)
+#define USBHS_UGCTRL_PLLRESET		(1 << 0)
+
+/* USB General control register 2 */
+#define USBHS_UGCTRL2_REG		0x84
+#define USBHS_UGCTRL2_USB0_PCI		(1 << 4)
+#define USBHS_UGCTRL2_USB0_HS		(3 << 4)
+#define USBHS_UGCTRL2_USB2_PCI		(0 << 31)
+#define USBHS_UGCTRL2_USB2_SS		(1 << 31)
+
+/* USB General status register */
+#define USBHS_UGSTS_REG			0x88
+#define USBHS_UGSTS_LOCK		(3 << 8)
+
+/* Enable USBHS internal phy */
+static int __rcar_gen2_usbhs_phy_enable(void __iomem *base)
+{
+	u32 val;
+	int i;
+
+	/* USBHS PHY power on */
+	val = ioread32(base + USBHS_UGCTRL_REG);
+	val &= ~USBHS_UGCTRL_PLLRESET;
+	iowrite32(val, base + USBHS_UGCTRL_REG);
+
+	val = ioread16(base + USBHS_LPSTS_REG);
+	val |= USBHS_LPSTS_SUSPM;
+	iowrite16(val, base + USBHS_LPSTS_REG);
+
+	for (i = 0; i < 20; i++) {
+		val = ioread32(base + USBHS_UGSTS_REG);
+		if ((val & USBHS_UGSTS_LOCK) = USBHS_UGSTS_LOCK) {
+			val = ioread32(base + USBHS_UGCTRL_REG);
+			val |= USBHS_UGCTRL_CONNECT;
+			iowrite32(val, base + USBHS_UGCTRL_REG);
+			return 0;
+		}
+		udelay(1);
+	}
+
+	/* Timed out waiting for the PLL lock */
+	return -ETIMEDOUT;
+}
+
+/* Disable USBHS internal phy */
+static int __rcar_gen2_usbhs_phy_disable(void __iomem *base)
+{
+	u32 val;
+
+	/* USBHS PHY power off */
+	val = ioread32(base + USBHS_UGCTRL_REG);
+	val &= ~USBHS_UGCTRL_CONNECT;
+	iowrite32(val, base + USBHS_UGCTRL_REG);
+
+	val = ioread16(base + USBHS_LPSTS_REG);
+	val &= ~USBHS_LPSTS_SUSPM;
+	iowrite16(val, base + USBHS_LPSTS_REG);
+
+	val = ioread32(base + USBHS_UGCTRL_REG);
+	val |= USBHS_UGCTRL_PLLRESET;
+	iowrite32(val, base + USBHS_UGCTRL_REG);
+	return 0;
+}
+
+/* Setup USB channels */
+static void __rcar_gen2_usb_phy_init(struct rcar_gen2_usb_phy_priv *priv)
+{
+	u32 val;
+
+	clk_prepare_enable(priv->clk);
+
+	/* Set USB channels in the USBHS UGCTRL2 register */
+	val = ioread32(priv->base);
+	val &= ~(USBHS_UGCTRL2_USB0_HS | USBHS_UGCTRL2_USB2_SS);
+	val |= priv->ugctrl2;
+	iowrite32(val, priv->base);
+}
+
+/* Shutdown USB channels */
+static void __rcar_gen2_usb_phy_shutdown(struct rcar_gen2_usb_phy_priv *priv)
+{
+	__rcar_gen2_usbhs_phy_disable(priv->base);
+	clk_disable_unprepare(priv->clk);
+}
+
+static int rcar_gen2_usb_phy_set_suspend(struct usb_phy *phy, int suspend)
+{
+	struct rcar_gen2_usb_phy_priv *priv = usb_phy_to_priv(phy);
+	unsigned long flags;
+	int retval;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	retval = suspend ? __rcar_gen2_usbhs_phy_disable(priv->base) :
+			   __rcar_gen2_usbhs_phy_enable(priv->base);
+	spin_unlock_irqrestore(&priv->lock, flags);
+	return retval;
+}
+
+static int rcar_gen2_usb_phy_init(struct usb_phy *phy)
+{
+	struct rcar_gen2_usb_phy_priv *priv = usb_phy_to_priv(phy);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	/*
+	 * Enable the clock and setup USB channels
+	 * if it's the first user
+	 */
+	if (!priv->usecount++)
+		__rcar_gen2_usb_phy_init(priv);
+	spin_unlock_irqrestore(&priv->lock, flags);
+	return 0;
+}
+
+static void rcar_gen2_usb_phy_shutdown(struct usb_phy *phy)
+{
+	struct rcar_gen2_usb_phy_priv *priv = usb_phy_to_priv(phy);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	if (!priv->usecount) {
+		dev_warn(phy->dev, "Trying to disable phy with 0 usecount\n");
+		goto out;
+	}
+
+	/* Disable everything if it's the last user */
+	if (!--priv->usecount)
+		__rcar_gen2_usb_phy_shutdown(priv);
+out:
+	spin_unlock_irqrestore(&priv->lock, flags);
+}
+
+static int rcar_gen2_usb_phy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rcar_gen2_phy_platform_data *pdata;
+	struct rcar_gen2_usb_phy_priv *priv;
+	struct resource *res;
+	void __iomem *base;
+	struct clk *clk;
+	int retval;
+
+	pdata = dev_get_platdata(&pdev->dev);
+	if (!pdata) {
+		dev_err(dev, "No platform data\n");
+		return -EINVAL;
+	}
+
+	clk = devm_clk_get(&pdev->dev, "usbhs");
+	if (IS_ERR(clk)) {
+		dev_err(&pdev->dev, "Can't get the clock\n");
+		return PTR_ERR(clk);
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(dev, "No memory resource\n");
+		return -ENODEV;
+	}
+
+	base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(base)) {
+		dev_err(dev, "Failed to ioremap resource\n");
+		return PTR_ERR(base);
+	}
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		dev_err(dev, "Memory allocation failed\n");
+		return -ENOMEM;
+	}
+
+	spin_lock_init(&priv->lock);
+	priv->clk = clk;
+	priv->base = base;
+	priv->ugctrl2 = pdata->chan0_pci ?
+			USBHS_UGCTRL2_USB0_PCI : USBHS_UGCTRL2_USB0_HS;
+	priv->ugctrl2 |= pdata->chan2_pci ?
+			USBHS_UGCTRL2_USB2_PCI : USBHS_UGCTRL2_USB2_SS;
+	priv->phy.dev = dev;
+	priv->phy.label = dev_name(dev);
+	priv->phy.init = rcar_gen2_usb_phy_init;
+	priv->phy.shutdown = rcar_gen2_usb_phy_shutdown;
+	priv->phy.set_suspend = rcar_gen2_usb_phy_set_suspend;
+
+	retval = usb_add_phy(&priv->phy, USB_PHY_TYPE_USB2);
+	if (retval < 0) {
+		dev_err(dev, "Failed to add USB phy\n");
+		return retval;
+	}
+
+	platform_set_drvdata(pdev, priv);
+
+	return retval;
+}
+
+static int rcar_gen2_usb_phy_remove(struct platform_device *pdev)
+{
+	struct rcar_gen2_usb_phy_priv *priv = platform_get_drvdata(pdev);
+
+	usb_remove_phy(&priv->phy);
+
+	return 0;
+}
+
+static struct platform_driver rcar_gen2_usb_phy_driver = {
+	.driver = {
+		.name = "usb_phy_rcar_gen2",
+	},
+	.probe = rcar_gen2_usb_phy_probe,
+	.remove = rcar_gen2_usb_phy_remove,
+};
+
+module_platform_driver(rcar_gen2_usb_phy_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Renesas R-Car Gen2 USB phy");
+MODULE_AUTHOR("Valentine Barshak <valentine.barshak@cogentembedded.com>");
diff --git a/include/linux/platform_data/usb-rcar-gen2-phy.h b/include/linux/platform_data/usb-rcar-gen2-phy.h
new file mode 100644
index 0000000..dd3ba46
--- /dev/null
+++ b/include/linux/platform_data/usb-rcar-gen2-phy.h
@@ -0,0 +1,22 @@
+/*
+ * Copyright (C) 2013 Renesas Solutions Corp.
+ * Copyright (C) 2013 Cogent Embedded, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __USB_RCAR_GEN2_PHY_H
+#define __USB_RCAR_GEN2_PHY_H
+
+#include <linux/types.h>
+
+struct rcar_gen2_phy_platform_data {
+	/* USB channel 0 configuration */
+	bool chan0_pci:1;	/* true: PCI USB host 0, false: USBHS */
+	/* USB channel 2 configuration */
+	bool chan2_pci:1;	/* true: PCI USB host 2, false: USBSS */
+};
+
+#endif
-- 
1.8.3.1

Re: [PATCH 1/3] usb: phy: Add RCAR Gen2 USB phy

[Laurent Pinchart]

Hi Valentine,

Thank you for the patch.

On Tuesday 08 October 2013 23:43:25 Valentine Barshak wrote:
> This adds RCAR Gen2 USB phy support. The driver configures
> USB channels 0/2 which are shared between PCI USB hosts and
> USBHS/USBSS devices. It also controls internal USBHS phy.
> 
> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
> ---
>  drivers/usb/phy/Kconfig                         |  13 ++
>  drivers/usb/phy/Makefile                        |   1 +
>  drivers/usb/phy/phy-rcar-gen2-usb.c             | 255 +++++++++++++++++++++
>  include/linux/platform_data/usb-rcar-gen2-phy.h |  22 ++
>  4 files changed, 291 insertions(+)
>  create mode 100644 drivers/usb/phy/phy-rcar-gen2-usb.c
>  create mode 100644 include/linux/platform_data/usb-rcar-gen2-phy.h
> 
> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index d5589f9..297062c 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -214,6 +214,19 @@ config USB_RCAR_PHY
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called phy-rcar-usb.
> 
> +config USB_RCAR_GEN2_PHY
> +	tristate "Renesas R-Car Gen2 USB PHY support"
> +	depends on ARCH_R8A7790 || ARCH_R8A7791

From a development point of view it's always nice to be able to compile the 
driver for a wider range of devices, even if the device is only found in the 
R8A779[01]. This allows catching compilation errors, for instance caused by 
API changes that affect all drivers using the API being modified.

I would use either

	depends on ARM

or

	depends on ARCH_R8A7790 || ARCH_R8A7791 || COMPILE_TEST

(assuming the driver can compile on non-ARM platforms, otherwise the above 
line could be changed to ARCH_R8A7790 || ARCH_R8A7791 || (ARM && 
COMPILE_TEST)).

> +	select USB_PHY
> +	help
> +	  Say Y here to add support for the Renesas R-Car Gen2 USB PHY driver.
> +	  It is typically used to control internal USB PHY for USBHS,
> +	  and to configure shared USB channels 0 and 2.
> +	  This driver supports R8A7790 and R8A7791.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called phy-rcar-gen2-usb.
> +
>  config USB_ULPI
>  	bool "Generic ULPI Transceiver Driver"
>  	depends on ARM
> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
> index 2135e85..8c5b147 100644
> --- a/drivers/usb/phy/Makefile
> +++ b/drivers/usb/phy/Makefile
> @@ -29,5 +29,6 @@ obj-$(CONFIG_USB_MSM_OTG)		+= phy-msm-usb.o
>  obj-$(CONFIG_USB_MV_OTG)		+= phy-mv-usb.o
>  obj-$(CONFIG_USB_MXS_PHY)		+= phy-mxs-usb.o
>  obj-$(CONFIG_USB_RCAR_PHY)		+= phy-rcar-usb.o
> +obj-$(CONFIG_USB_RCAR_GEN2_PHY)		+= phy-rcar-gen2-usb.o
>  obj-$(CONFIG_USB_ULPI)			+= phy-ulpi.o
>  obj-$(CONFIG_USB_ULPI_VIEWPORT)		+= phy-ulpi-viewport.o
> diff --git a/drivers/usb/phy/phy-rcar-gen2-usb.c
> b/drivers/usb/phy/phy-rcar-gen2-usb.c new file mode 100644
> index 0000000..a2e6f9f
> --- /dev/null
> +++ b/drivers/usb/phy/phy-rcar-gen2-usb.c
> @@ -0,0 +1,255 @@
> +/*
> + * Renesas R-Car Gen2 USB phy driver
> + *
> + * Copyright (C) 2013 Renesas Solutions Corp.
> + * Copyright (C) 2013 Cogent Embedded, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/usb/otg.h>
> +#include <linux/platform_data/usb-rcar-gen2-phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +#include <linux/module.h>

Would you mind ordering the headers alphabetically ? This helps avoiding 
duplicate includes, an existing include might not be noticed at first glance 
when adding a new one if the list is long and not sorted.

> +struct rcar_gen2_usb_phy_priv {
> +	struct usb_phy phy;
> +	void __iomem *base;
> +	struct clk *clk;
> +	spinlock_t lock;
> +	int usecount;
> +	u32 ugctrl2;
> +};
> +
> +#define usb_phy_to_priv(p) container_of(p, struct rcar_gen2_usb_phy_priv,
> phy) +
> +/* Low Power Status register */
> +#define USBHS_LPSTS_REG			0x02
> +#define USBHS_LPSTS_SUSPM		(1 << 14)
> +
> +/* USB General control register */
> +#define USBHS_UGCTRL_REG		0x80
> +#define USBHS_UGCTRL_CONNECT		(1 << 2)
> +#define USBHS_UGCTRL_PLLRESET		(1 << 0)
> +
> +/* USB General control register 2 */
> +#define USBHS_UGCTRL2_REG		0x84
> +#define USBHS_UGCTRL2_USB0_PCI		(1 << 4)
> +#define USBHS_UGCTRL2_USB0_HS		(3 << 4)
> +#define USBHS_UGCTRL2_USB2_PCI		(0 << 31)
> +#define USBHS_UGCTRL2_USB2_SS		(1 << 31)
> +
> +/* USB General status register */
> +#define USBHS_UGSTS_REG			0x88
> +#define USBHS_UGSTS_LOCK		(3 << 8)
> +
> +/* Enable USBHS internal phy */
> +static int __rcar_gen2_usbhs_phy_enable(void __iomem *base)

Any reason for the __ as a function prefix ?

I would have passed a struct rcar_gen2_usb_phy_priv pointer to this function, 
which would allow you to print a debugging message using dev_dbg() in case of 
a timeout. That's up to you.

> +{
> +	u32 val;
> +	int i;

I tend to use unsigned int for loop counters that can't take negative values.

> +
> +	/* USBHS PHY power on */
> +	val = ioread32(base + USBHS_UGCTRL_REG);
> +	val &= ~USBHS_UGCTRL_PLLRESET;
> +	iowrite32(val, base + USBHS_UGCTRL_REG);
> +
> +	val = ioread16(base + USBHS_LPSTS_REG);
> +	val |= USBHS_LPSTS_SUSPM;
> +	iowrite16(val, base + USBHS_LPSTS_REG);
> +
> +	for (i = 0; i < 20; i++) {
> +		val = ioread32(base + USBHS_UGSTS_REG);
> +		if ((val & USBHS_UGSTS_LOCK) = USBHS_UGSTS_LOCK) {
> +			val = ioread32(base + USBHS_UGCTRL_REG);
> +			val |= USBHS_UGCTRL_CONNECT;
> +			iowrite32(val, base + USBHS_UGCTRL_REG);
> +			return 0;
> +		}
> +		udelay(1);

What's the maximum number of iterations you've observed in practice ? Does the 
20 limit come from somewhere in particular ?

> +	}
> +
> +	/* Timed out waiting for the PLL lock */
> +	return -ETIMEDOUT;
> +}
> +
> +/* Disable USBHS internal phy */
> +static int __rcar_gen2_usbhs_phy_disable(void __iomem *base)
> +{
> +	u32 val;
> +
> +	/* USBHS PHY power off */
> +	val = ioread32(base + USBHS_UGCTRL_REG);
> +	val &= ~USBHS_UGCTRL_CONNECT;
> +	iowrite32(val, base + USBHS_UGCTRL_REG);
> +
> +	val = ioread16(base + USBHS_LPSTS_REG);
> +	val &= ~USBHS_LPSTS_SUSPM;
> +	iowrite16(val, base + USBHS_LPSTS_REG);
> +
> +	val = ioread32(base + USBHS_UGCTRL_REG);
> +	val |= USBHS_UGCTRL_PLLRESET;
> +	iowrite32(val, base + USBHS_UGCTRL_REG);
> +	return 0;
> +}
> +
> +/* Setup USB channels */
> +static void __rcar_gen2_usb_phy_init(struct rcar_gen2_usb_phy_priv *priv)
> +{
> +	u32 val;
> +
> +	clk_prepare_enable(priv->clk);
> +
> +	/* Set USB channels in the USBHS UGCTRL2 register */
> +	val = ioread32(priv->base);
> +	val &= ~(USBHS_UGCTRL2_USB0_HS | USBHS_UGCTRL2_USB2_SS);
> +	val |= priv->ugctrl2;
> +	iowrite32(val, priv->base);
> +}
> +
> +/* Shutdown USB channels */
> +static void __rcar_gen2_usb_phy_shutdown(struct rcar_gen2_usb_phy_priv
> *priv)
> +{
> +	__rcar_gen2_usbhs_phy_disable(priv->base);
> +	clk_disable_unprepare(priv->clk);
> +}
> +
> +static int rcar_gen2_usb_phy_set_suspend(struct usb_phy *phy, int suspend)
> +{
> +	struct rcar_gen2_usb_phy_priv *priv = usb_phy_to_priv(phy);
> +	unsigned long flags;
> +	int retval;
> +
> +	spin_lock_irqsave(&priv->lock, flags);
> +	retval = suspend ? __rcar_gen2_usbhs_phy_disable(priv->base) :
> +			   __rcar_gen2_usbhs_phy_enable(priv->base);
> +	spin_unlock_irqrestore(&priv->lock, flags);
> +	return retval;
> +}
> +
> +static int rcar_gen2_usb_phy_init(struct usb_phy *phy)
> +{
> +	struct rcar_gen2_usb_phy_priv *priv = usb_phy_to_priv(phy);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&priv->lock, flags);
> +	/*
> +	 * Enable the clock and setup USB channels
> +	 * if it's the first user
> +	 */
> +	if (!priv->usecount++)
> +		__rcar_gen2_usb_phy_init(priv);
> +	spin_unlock_irqrestore(&priv->lock, flags);
> +	return 0;
> +}
> +
> +static void rcar_gen2_usb_phy_shutdown(struct usb_phy *phy)
> +{
> +	struct rcar_gen2_usb_phy_priv *priv = usb_phy_to_priv(phy);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&priv->lock, flags);
> +	if (!priv->usecount) {
> +		dev_warn(phy->dev, "Trying to disable phy with 0 usecount\n");
> +		goto out;
> +	}
> +
> +	/* Disable everything if it's the last user */
> +	if (!--priv->usecount)
> +		__rcar_gen2_usb_phy_shutdown(priv);
> +out:
> +	spin_unlock_irqrestore(&priv->lock, flags);
> +}
> +
> +static int rcar_gen2_usb_phy_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct rcar_gen2_phy_platform_data *pdata;
> +	struct rcar_gen2_usb_phy_priv *priv;
> +	struct resource *res;
> +	void __iomem *base;
> +	struct clk *clk;
> +	int retval;
> +
> +	pdata = dev_get_platdata(&pdev->dev);

DT support would be nice too :-)

> +	if (!pdata) {
> +		dev_err(dev, "No platform data\n");
> +		return -EINVAL;
> +	}
> +
> +	clk = devm_clk_get(&pdev->dev, "usbhs");
> +	if (IS_ERR(clk)) {
> +		dev_err(&pdev->dev, "Can't get the clock\n");
> +		return PTR_ERR(clk);
> +	}
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(dev, "No memory resource\n");

No need to check the return value or print an error message, 
devm_ioremap_resource() already takes care of that.

> +		return -ENODEV;
> +	}
> +
> +	base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(base)) {
> +		dev_err(dev, "Failed to ioremap resource\n");

No need to print an error message here either.

> +		return PTR_ERR(base);
> +	}
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv) {
> +		dev_err(dev, "Memory allocation failed\n");
> +		return -ENOMEM;
> +	}
> +
> +	spin_lock_init(&priv->lock);
> +	priv->clk = clk;
> +	priv->base = base;
> +	priv->ugctrl2 = pdata->chan0_pci ?
> +			USBHS_UGCTRL2_USB0_PCI : USBHS_UGCTRL2_USB0_HS;
> +	priv->ugctrl2 |= pdata->chan2_pci ?
> +			USBHS_UGCTRL2_USB2_PCI : USBHS_UGCTRL2_USB2_SS;
> +	priv->phy.dev = dev;
> +	priv->phy.label = dev_name(dev);
> +	priv->phy.init = rcar_gen2_usb_phy_init;
> +	priv->phy.shutdown = rcar_gen2_usb_phy_shutdown;
> +	priv->phy.set_suspend = rcar_gen2_usb_phy_set_suspend;
> +
> +	retval = usb_add_phy(&priv->phy, USB_PHY_TYPE_USB2);
> +	if (retval < 0) {
> +		dev_err(dev, "Failed to add USB phy\n");
> +		return retval;
> +	}
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	return retval;
> +}
> +
> +static int rcar_gen2_usb_phy_remove(struct platform_device *pdev)
> +{
> +	struct rcar_gen2_usb_phy_priv *priv = platform_get_drvdata(pdev);
> +
> +	usb_remove_phy(&priv->phy);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver rcar_gen2_usb_phy_driver = {
> +	.driver = {
> +		.name = "usb_phy_rcar_gen2",
> +	},
> +	.probe = rcar_gen2_usb_phy_probe,
> +	.remove = rcar_gen2_usb_phy_remove,
> +};
> +
> +module_platform_driver(rcar_gen2_usb_phy_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Renesas R-Car Gen2 USB phy");
> +MODULE_AUTHOR("Valentine Barshak <valentine.barshak@cogentembedded.com>");
> diff --git a/include/linux/platform_data/usb-rcar-gen2-phy.h
> b/include/linux/platform_data/usb-rcar-gen2-phy.h new file mode 100644
> index 0000000..dd3ba46
> --- /dev/null
> +++ b/include/linux/platform_data/usb-rcar-gen2-phy.h
> @@ -0,0 +1,22 @@
> +/*
> + * Copyright (C) 2013 Renesas Solutions Corp.
> + * Copyright (C) 2013 Cogent Embedded, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef __USB_RCAR_GEN2_PHY_H
> +#define __USB_RCAR_GEN2_PHY_H
> +
> +#include <linux/types.h>
> +
> +struct rcar_gen2_phy_platform_data {
> +	/* USB channel 0 configuration */
> +	bool chan0_pci:1;	/* true: PCI USB host 0, false: USBHS */
> +	/* USB channel 2 configuration */
> +	bool chan2_pci:1;	/* true: PCI USB host 2, false: USBSS */
> +};
> +
> +#endif
-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/3] usb: phy: Add RCAR Gen2 USB phy

[Valentine]

On 10/10/2013 12:32 AM, Laurent Pinchart wrote:
> Hi Valentine,
>

Hi Laurent,

> Thank you for the patch.
>
> On Tuesday 08 October 2013 23:43:25 Valentine Barshak wrote:
>> This adds RCAR Gen2 USB phy support. The driver configures
>> USB channels 0/2 which are shared between PCI USB hosts and
>> USBHS/USBSS devices. It also controls internal USBHS phy.
>>
>> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
>> ---
>>   drivers/usb/phy/Kconfig                         |  13 ++
>>   drivers/usb/phy/Makefile                        |   1 +
>>   drivers/usb/phy/phy-rcar-gen2-usb.c             | 255 +++++++++++++++++++++
>>   include/linux/platform_data/usb-rcar-gen2-phy.h |  22 ++
>>   4 files changed, 291 insertions(+)
>>   create mode 100644 drivers/usb/phy/phy-rcar-gen2-usb.c
>>   create mode 100644 include/linux/platform_data/usb-rcar-gen2-phy.h
>>
>> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
>> index d5589f9..297062c 100644
>> --- a/drivers/usb/phy/Kconfig
>> +++ b/drivers/usb/phy/Kconfig
>> @@ -214,6 +214,19 @@ config USB_RCAR_PHY
>>   	  To compile this driver as a module, choose M here: the
>>   	  module will be called phy-rcar-usb.
>>
>> +config USB_RCAR_GEN2_PHY
>> +	tristate "Renesas R-Car Gen2 USB PHY support"
>> +	depends on ARCH_R8A7790 || ARCH_R8A7791
>
>  From a development point of view it's always nice to be able to compile the
> driver for a wider range of devices, even if the device is only found in the
> R8A779[01]. This allows catching compilation errors, for instance caused by
> API changes that affect all drivers using the API being modified.

Compiling a dirver for an unsupported architecture also seems to be more 
error-prone.

>
> I would use either
>
> 	depends on ARM
>
> or
>
> 	depends on ARCH_R8A7790 || ARCH_R8A7791 || COMPILE_TEST
>
> (assuming the driver can compile on non-ARM platforms, otherwise the above
> line could be changed to ARCH_R8A7790 || ARCH_R8A7791 || (ARM &&
> COMPILE_TEST)).

OK, I'll take a look.
Do all the drivers have to support COMPILE_TEST?

>
>> +	select USB_PHY
>> +	help
>> +	  Say Y here to add support for the Renesas R-Car Gen2 USB PHY driver.
>> +	  It is typically used to control internal USB PHY for USBHS,
>> +	  and to configure shared USB channels 0 and 2.
>> +	  This driver supports R8A7790 and R8A7791.
>> +
>> +	  To compile this driver as a module, choose M here: the
>> +	  module will be called phy-rcar-gen2-usb.
>> +
>>   config USB_ULPI
>>   	bool "Generic ULPI Transceiver Driver"
>>   	depends on ARM
>> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
>> index 2135e85..8c5b147 100644
>> --- a/drivers/usb/phy/Makefile
>> +++ b/drivers/usb/phy/Makefile
>> @@ -29,5 +29,6 @@ obj-$(CONFIG_USB_MSM_OTG)		+= phy-msm-usb.o
>>   obj-$(CONFIG_USB_MV_OTG)		+= phy-mv-usb.o
>>   obj-$(CONFIG_USB_MXS_PHY)		+= phy-mxs-usb.o
>>   obj-$(CONFIG_USB_RCAR_PHY)		+= phy-rcar-usb.o
>> +obj-$(CONFIG_USB_RCAR_GEN2_PHY)		+= phy-rcar-gen2-usb.o
>>   obj-$(CONFIG_USB_ULPI)			+= phy-ulpi.o
>>   obj-$(CONFIG_USB_ULPI_VIEWPORT)		+= phy-ulpi-viewport.o
>> diff --git a/drivers/usb/phy/phy-rcar-gen2-usb.c
>> b/drivers/usb/phy/phy-rcar-gen2-usb.c new file mode 100644
>> index 0000000..a2e6f9f
>> --- /dev/null
>> +++ b/drivers/usb/phy/phy-rcar-gen2-usb.c
>> @@ -0,0 +1,255 @@
>> +/*
>> + * Renesas R-Car Gen2 USB phy driver
>> + *
>> + * Copyright (C) 2013 Renesas Solutions Corp.
>> + * Copyright (C) 2013 Cogent Embedded, Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/io.h>
>> +#include <linux/usb/otg.h>
>> +#include <linux/platform_data/usb-rcar-gen2-phy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/spinlock.h>
>> +#include <linux/module.h>
>
> Would you mind ordering the headers alphabetically ? This helps avoiding
> duplicate includes, an existing include might not be noticed at first glance
> when adding a new one if the list is long and not sorted.

OK.

>
>> +struct rcar_gen2_usb_phy_priv {
>> +	struct usb_phy phy;
>> +	void __iomem *base;
>> +	struct clk *clk;
>> +	spinlock_t lock;
>> +	int usecount;
>> +	u32 ugctrl2;
>> +};
>> +
>> +#define usb_phy_to_priv(p) container_of(p, struct rcar_gen2_usb_phy_priv,
>> phy) +
>> +/* Low Power Status register */
>> +#define USBHS_LPSTS_REG			0x02
>> +#define USBHS_LPSTS_SUSPM		(1 << 14)
>> +
>> +/* USB General control register */
>> +#define USBHS_UGCTRL_REG		0x80
>> +#define USBHS_UGCTRL_CONNECT		(1 << 2)
>> +#define USBHS_UGCTRL_PLLRESET		(1 << 0)
>> +
>> +/* USB General control register 2 */
>> +#define USBHS_UGCTRL2_REG		0x84
>> +#define USBHS_UGCTRL2_USB0_PCI		(1 << 4)
>> +#define USBHS_UGCTRL2_USB0_HS		(3 << 4)
>> +#define USBHS_UGCTRL2_USB2_PCI		(0 << 31)
>> +#define USBHS_UGCTRL2_USB2_SS		(1 << 31)
>> +
>> +/* USB General status register */
>> +#define USBHS_UGSTS_REG			0x88
>> +#define USBHS_UGSTS_LOCK		(3 << 8)
>> +
>> +/* Enable USBHS internal phy */
>> +static int __rcar_gen2_usbhs_phy_enable(void __iomem *base)
>
> Any reason for the __ as a function prefix ?

This indicates that the function is called with the spinlock held.

>
> I would have passed a struct rcar_gen2_usb_phy_priv pointer to this function,
> which would allow you to print a debugging message using dev_dbg() in case of
> a timeout. That's up to you.
>
>> +{
>> +	u32 val;
>> +	int i;
>
> I tend to use unsigned int for loop counters that can't take negative values.

Do not see much of a difference really.

>
>> +
>> +	/* USBHS PHY power on */
>> +	val = ioread32(base + USBHS_UGCTRL_REG);
>> +	val &= ~USBHS_UGCTRL_PLLRESET;
>> +	iowrite32(val, base + USBHS_UGCTRL_REG);
>> +
>> +	val = ioread16(base + USBHS_LPSTS_REG);
>> +	val |= USBHS_LPSTS_SUSPM;
>> +	iowrite16(val, base + USBHS_LPSTS_REG);
>> +
>> +	for (i = 0; i < 20; i++) {
>> +		val = ioread32(base + USBHS_UGSTS_REG);
>> +		if ((val & USBHS_UGSTS_LOCK) = USBHS_UGSTS_LOCK) {
>> +			val = ioread32(base + USBHS_UGCTRL_REG);
>> +			val |= USBHS_UGCTRL_CONNECT;
>> +			iowrite32(val, base + USBHS_UGCTRL_REG);
>> +			return 0;
>> +		}
>> +		udelay(1);
>
> What's the maximum number of iterations you've observed in practice ? Does the
> 20 limit come from somewhere in particular ?

The maximum I've seen is 4. The docs say nothing about the limit. Thus, 
20 seemed more than enough to me.

[snip]

>> +static int rcar_gen2_usb_phy_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct rcar_gen2_phy_platform_data *pdata;
>> +	struct rcar_gen2_usb_phy_priv *priv;
>> +	struct resource *res;
>> +	void __iomem *base;
>> +	struct clk *clk;
>> +	int retval;
>> +
>> +	pdata = dev_get_platdata(&pdev->dev);
>
> DT support would be nice too :-)

I Plan to add it with a separate patch later since we still need to add 
DT support to renesas_usbhs.

>
>> +	if (!pdata) {
>> +		dev_err(dev, "No platform data\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	clk = devm_clk_get(&pdev->dev, "usbhs");
>> +	if (IS_ERR(clk)) {
>> +		dev_err(&pdev->dev, "Can't get the clock\n");
>> +		return PTR_ERR(clk);
>> +	}
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	if (!res) {
>> +		dev_err(dev, "No memory resource\n");
>
> No need to check the return value or print an error message,
> devm_ioremap_resource() already takes care of that.

OK, thanks.

>
>> +		return -ENODEV;
>> +	}
>> +
>> +	base = devm_ioremap_resource(dev, res);
>> +	if (IS_ERR(base)) {
>> +		dev_err(dev, "Failed to ioremap resource\n");
>
> No need to print an error message here either.

OK, thanks.

[snip]

Thanks,
Val.

Re: [PATCH 1/3] usb: phy: Add RCAR Gen2 USB phy

[Laurent Pinchart]

Hi Valentine,

On Thursday 10 October 2013 01:21:27 Valentine wrote:
> On 10/10/2013 12:32 AM, Laurent Pinchart wrote:
> > Hi Valentine,
> 
> Hi Laurent,
> 
> > Thank you for the patch.
> > 
> > On Tuesday 08 October 2013 23:43:25 Valentine Barshak wrote:
> >> This adds RCAR Gen2 USB phy support. The driver configures
> >> USB channels 0/2 which are shared between PCI USB hosts and
> >> USBHS/USBSS devices. It also controls internal USBHS phy.
> >> 
> >> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
> >> ---
> >> 
> >>   drivers/usb/phy/Kconfig                         |  13 ++
> >>   drivers/usb/phy/Makefile                        |   1 +
> >>   drivers/usb/phy/phy-rcar-gen2-usb.c             | 255 +++++++++++++++++
> >>   include/linux/platform_data/usb-rcar-gen2-phy.h |  22 ++
> >>   4 files changed, 291 insertions(+)
> >>   create mode 100644 drivers/usb/phy/phy-rcar-gen2-usb.c
> >>   create mode 100644 include/linux/platform_data/usb-rcar-gen2-phy.h
> >> 
> >> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> >> index d5589f9..297062c 100644
> >> --- a/drivers/usb/phy/Kconfig
> >> +++ b/drivers/usb/phy/Kconfig
> >> @@ -214,6 +214,19 @@ config USB_RCAR_PHY
> >>   	  To compile this driver as a module, choose M here: the
> >>   	  module will be called phy-rcar-usb.
> >> 
> >> +config USB_RCAR_GEN2_PHY
> >> +	tristate "Renesas R-Car Gen2 USB PHY support"
> >> +	depends on ARCH_R8A7790 || ARCH_R8A7791
> >> 
> > From a development point of view it's always nice to be able to compile
> > the driver for a wider range of devices, even if the device is only found
> > in the R8A779[01]. This allows catching compilation errors, for instance
> > caused by API changes that affect all drivers using the API being
> > modified.
> 
> Compiling a dirver for an unsupported architecture also seems to be more
> error-prone.

It happened to me previously that a subsystem refactoring touching lots of 
drivers forgot to update one of the drivers I was maintaining. This went 
undetected as the driver could only be compiled for a very restricted set of 
platforms, breaking compilation in mainline. It's easier to avoid this kind of 
situation if the driver can be compiled for a larger number of platforms.

> > I would use either
> > 
> > 	depends on ARM
> > 
> > or
> > 
> > 	depends on ARCH_R8A7790 || ARCH_R8A7791 || COMPILE_TEST
> > 
> > (assuming the driver can compile on non-ARM platforms, otherwise the above
> > line could be changed to ARCH_R8A7790 || ARCH_R8A7791 || (ARM &&
> > COMPILE_TEST)).
> 
> OK, I'll take a look.
> Do all the drivers have to support COMPILE_TEST?

There's currently no rule, but if the driver can only be compiled for a 
restricted set of platforms, I would say that supporting COMPILE_TEST would be 
a good practice. It of course needs to be restricted to the platforms on which 
the driver will actually compile :-)

> >> +	select USB_PHY
> >> +	help
> >> +	  Say Y here to add support for the Renesas R-Car Gen2 USB PHY
> >> driver.
> >> +	  It is typically used to control internal USB PHY for USBHS,
> >> +	  and to configure shared USB channels 0 and 2.
> >> +	  This driver supports R8A7790 and R8A7791.
> >> +
> >> +	  To compile this driver as a module, choose M here: the
> >> +	  module will be called phy-rcar-gen2-usb.
> >> +
> >>   config USB_ULPI
> >>   	bool "Generic ULPI Transceiver Driver"
> >>   	depends on ARM

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/3] usb: phy: Add RCAR Gen2 USB phy

[Valentine]

On 10/10/2013 01:28 AM, Laurent Pinchart wrote:
> Hi Valentine,
>
> On Thursday 10 October 2013 01:21:27 Valentine wrote:
>> On 10/10/2013 12:32 AM, Laurent Pinchart wrote:
>>> Hi Valentine,
>>
>> Hi Laurent,
>>
>>> Thank you for the patch.
>>>
>>> On Tuesday 08 October 2013 23:43:25 Valentine Barshak wrote:
>>>> This adds RCAR Gen2 USB phy support. The driver configures
>>>> USB channels 0/2 which are shared between PCI USB hosts and
>>>> USBHS/USBSS devices. It also controls internal USBHS phy.
>>>>
>>>> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
>>>> ---
>>>>
>>>>    drivers/usb/phy/Kconfig                         |  13 ++
>>>>    drivers/usb/phy/Makefile                        |   1 +
>>>>    drivers/usb/phy/phy-rcar-gen2-usb.c             | 255 +++++++++++++++++
>>>>    include/linux/platform_data/usb-rcar-gen2-phy.h |  22 ++
>>>>    4 files changed, 291 insertions(+)
>>>>    create mode 100644 drivers/usb/phy/phy-rcar-gen2-usb.c
>>>>    create mode 100644 include/linux/platform_data/usb-rcar-gen2-phy.h
>>>>
>>>> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
>>>> index d5589f9..297062c 100644
>>>> --- a/drivers/usb/phy/Kconfig
>>>> +++ b/drivers/usb/phy/Kconfig
>>>> @@ -214,6 +214,19 @@ config USB_RCAR_PHY
>>>>    	  To compile this driver as a module, choose M here: the
>>>>    	  module will be called phy-rcar-usb.
>>>>
>>>> +config USB_RCAR_GEN2_PHY
>>>> +	tristate "Renesas R-Car Gen2 USB PHY support"
>>>> +	depends on ARCH_R8A7790 || ARCH_R8A7791
>>>>
>>>  From a development point of view it's always nice to be able to compile
>>> the driver for a wider range of devices, even if the device is only found
>>> in the R8A779[01]. This allows catching compilation errors, for instance
>>> caused by API changes that affect all drivers using the API being
>>> modified.
>>
>> Compiling a dirver for an unsupported architecture also seems to be more
>> error-prone.
>
> It happened to me previously that a subsystem refactoring touching lots of
> drivers forgot to update one of the drivers I was maintaining. This went
> undetected as the driver could only be compiled for a very restricted set of
> platforms, breaking compilation in mainline. It's easier to avoid this kind of
> situation if the driver can be compiled for a larger number of platforms.
>
>>> I would use either
>>>
>>> 	depends on ARM
>>>
>>> or
>>>
>>> 	depends on ARCH_R8A7790 || ARCH_R8A7791 || COMPILE_TEST
>>>
>>> (assuming the driver can compile on non-ARM platforms, otherwise the above
>>> line could be changed to ARCH_R8A7790 || ARCH_R8A7791 || (ARM &&
>>> COMPILE_TEST)).
>>
>> OK, I'll take a look.
>> Do all the drivers have to support COMPILE_TEST?
>
> There's currently no rule, but if the driver can only be compiled for a
> restricted set of platforms, I would say that supporting COMPILE_TEST would be
> a good practice. It of course needs to be restricted to the platforms on which
> the driver will actually compile :-)

OK thanks.
I'll probably go with the safest option:
ARCH_R8A7790 || ARCH_R8A7791 || (ARM && COMPILE_TEST)

It does compile on x86 though, and may compile on other platforms as well.

Thanks,
Val.

[PATCH 1/3] usb: phy: Add RCAR Gen2 USB phy

[Valentine Barshak]

This adds RCAR Gen2 USB phy support. The driver configures
USB channels 0/2 which are shared between PCI USB hosts and
USBHS/USBSS devices. It also controls internal USBHS phy.

Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
---
 drivers/usb/phy/Kconfig                         |  13 ++
 drivers/usb/phy/Makefile                        |   1 +
 drivers/usb/phy/phy-rcar-gen2-usb.c             | 248 ++++++++++++++++++++++++
 include/linux/platform_data/usb-rcar-gen2-phy.h |  22 +++
 4 files changed, 284 insertions(+)
 create mode 100644 drivers/usb/phy/phy-rcar-gen2-usb.c
 create mode 100644 include/linux/platform_data/usb-rcar-gen2-phy.h

diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index d5589f9..bba7d29 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -214,6 +214,19 @@ config USB_RCAR_PHY
 	  To compile this driver as a module, choose M here: the
 	  module will be called phy-rcar-usb.
 
+config USB_RCAR_GEN2_PHY
+	tristate "Renesas R-Car Gen2 USB PHY support"
+	depends on ARCH_R8A7790 || ARCH_R8A7791 || (ARM && COMPILE_TEST)
+	select USB_PHY
+	help
+	  Say Y here to add support for the Renesas R-Car Gen2 USB PHY driver.
+	  It is typically used to control internal USB PHY for USBHS,
+	  and to configure shared USB channels 0 and 2.
+	  This driver supports R8A7790 and R8A7791.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called phy-rcar-gen2-usb.
+
 config USB_ULPI
 	bool "Generic ULPI Transceiver Driver"
 	depends on ARM
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index 2135e85..8c5b147 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -29,5 +29,6 @@ obj-$(CONFIG_USB_MSM_OTG)		+= phy-msm-usb.o
 obj-$(CONFIG_USB_MV_OTG)		+= phy-mv-usb.o
 obj-$(CONFIG_USB_MXS_PHY)		+= phy-mxs-usb.o
 obj-$(CONFIG_USB_RCAR_PHY)		+= phy-rcar-usb.o
+obj-$(CONFIG_USB_RCAR_GEN2_PHY)		+= phy-rcar-gen2-usb.o
 obj-$(CONFIG_USB_ULPI)			+= phy-ulpi.o
 obj-$(CONFIG_USB_ULPI_VIEWPORT)		+= phy-ulpi-viewport.o
diff --git a/drivers/usb/phy/phy-rcar-gen2-usb.c b/drivers/usb/phy/phy-rcar-gen2-usb.c
new file mode 100644
index 0000000..a99a695
--- /dev/null
+++ b/drivers/usb/phy/phy-rcar-gen2-usb.c
@@ -0,0 +1,248 @@
+/*
+ * Renesas R-Car Gen2 USB phy driver
+ *
+ * Copyright (C) 2013 Renesas Solutions Corp.
+ * Copyright (C) 2013 Cogent Embedded, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_data/usb-rcar-gen2-phy.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/usb/otg.h>
+
+struct rcar_gen2_usb_phy_priv {
+	struct usb_phy phy;
+	void __iomem *base;
+	struct clk *clk;
+	spinlock_t lock;
+	int usecount;
+	u32 ugctrl2;
+};
+
+#define usb_phy_to_priv(p) container_of(p, struct rcar_gen2_usb_phy_priv, phy)
+
+/* Low Power Status register */
+#define USBHS_LPSTS_REG			0x02
+#define USBHS_LPSTS_SUSPM		(1 << 14)
+
+/* USB General control register */
+#define USBHS_UGCTRL_REG		0x80
+#define USBHS_UGCTRL_CONNECT		(1 << 2)
+#define USBHS_UGCTRL_PLLRESET		(1 << 0)
+
+/* USB General control register 2 */
+#define USBHS_UGCTRL2_REG		0x84
+#define USBHS_UGCTRL2_USB0_PCI		(1 << 4)
+#define USBHS_UGCTRL2_USB0_HS		(3 << 4)
+#define USBHS_UGCTRL2_USB2_PCI		(0 << 31)
+#define USBHS_UGCTRL2_USB2_SS		(1 << 31)
+
+/* USB General status register */
+#define USBHS_UGSTS_REG			0x88
+#define USBHS_UGSTS_LOCK		(3 << 8)
+
+/* Enable USBHS internal phy */
+static int __rcar_gen2_usbhs_phy_enable(void __iomem *base)
+{
+	u32 val;
+	int i;
+
+	/* USBHS PHY power on */
+	val = ioread32(base + USBHS_UGCTRL_REG);
+	val &= ~USBHS_UGCTRL_PLLRESET;
+	iowrite32(val, base + USBHS_UGCTRL_REG);
+
+	val = ioread16(base + USBHS_LPSTS_REG);
+	val |= USBHS_LPSTS_SUSPM;
+	iowrite16(val, base + USBHS_LPSTS_REG);
+
+	for (i = 0; i < 20; i++) {
+		val = ioread32(base + USBHS_UGSTS_REG);
+		if ((val & USBHS_UGSTS_LOCK) = USBHS_UGSTS_LOCK) {
+			val = ioread32(base + USBHS_UGCTRL_REG);
+			val |= USBHS_UGCTRL_CONNECT;
+			iowrite32(val, base + USBHS_UGCTRL_REG);
+			return 0;
+		}
+		udelay(1);
+	}
+
+	/* Timed out waiting for the PLL lock */
+	return -ETIMEDOUT;
+}
+
+/* Disable USBHS internal phy */
+static int __rcar_gen2_usbhs_phy_disable(void __iomem *base)
+{
+	u32 val;
+
+	/* USBHS PHY power off */
+	val = ioread32(base + USBHS_UGCTRL_REG);
+	val &= ~USBHS_UGCTRL_CONNECT;
+	iowrite32(val, base + USBHS_UGCTRL_REG);
+
+	val = ioread16(base + USBHS_LPSTS_REG);
+	val &= ~USBHS_LPSTS_SUSPM;
+	iowrite16(val, base + USBHS_LPSTS_REG);
+
+	val = ioread32(base + USBHS_UGCTRL_REG);
+	val |= USBHS_UGCTRL_PLLRESET;
+	iowrite32(val, base + USBHS_UGCTRL_REG);
+	return 0;
+}
+
+/* Setup USB channels */
+static void __rcar_gen2_usb_phy_init(struct rcar_gen2_usb_phy_priv *priv)
+{
+	u32 val;
+
+	clk_prepare_enable(priv->clk);
+
+	/* Set USB channels in the USBHS UGCTRL2 register */
+	val = ioread32(priv->base);
+	val &= ~(USBHS_UGCTRL2_USB0_HS | USBHS_UGCTRL2_USB2_SS);
+	val |= priv->ugctrl2;
+	iowrite32(val, priv->base);
+}
+
+/* Shutdown USB channels */
+static void __rcar_gen2_usb_phy_shutdown(struct rcar_gen2_usb_phy_priv *priv)
+{
+	__rcar_gen2_usbhs_phy_disable(priv->base);
+	clk_disable_unprepare(priv->clk);
+}
+
+static int rcar_gen2_usb_phy_set_suspend(struct usb_phy *phy, int suspend)
+{
+	struct rcar_gen2_usb_phy_priv *priv = usb_phy_to_priv(phy);
+	unsigned long flags;
+	int retval;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	retval = suspend ? __rcar_gen2_usbhs_phy_disable(priv->base) :
+			   __rcar_gen2_usbhs_phy_enable(priv->base);
+	spin_unlock_irqrestore(&priv->lock, flags);
+	return retval;
+}
+
+static int rcar_gen2_usb_phy_init(struct usb_phy *phy)
+{
+	struct rcar_gen2_usb_phy_priv *priv = usb_phy_to_priv(phy);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	/*
+	 * Enable the clock and setup USB channels
+	 * if it's the first user
+	 */
+	if (!priv->usecount++)
+		__rcar_gen2_usb_phy_init(priv);
+	spin_unlock_irqrestore(&priv->lock, flags);
+	return 0;
+}
+
+static void rcar_gen2_usb_phy_shutdown(struct usb_phy *phy)
+{
+	struct rcar_gen2_usb_phy_priv *priv = usb_phy_to_priv(phy);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	if (!priv->usecount) {
+		dev_warn(phy->dev, "Trying to disable phy with 0 usecount\n");
+		goto out;
+	}
+
+	/* Disable everything if it's the last user */
+	if (!--priv->usecount)
+		__rcar_gen2_usb_phy_shutdown(priv);
+out:
+	spin_unlock_irqrestore(&priv->lock, flags);
+}
+
+static int rcar_gen2_usb_phy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rcar_gen2_phy_platform_data *pdata;
+	struct rcar_gen2_usb_phy_priv *priv;
+	struct resource *res;
+	void __iomem *base;
+	struct clk *clk;
+	int retval;
+
+	pdata = dev_get_platdata(&pdev->dev);
+	if (!pdata) {
+		dev_err(dev, "No platform data\n");
+		return -EINVAL;
+	}
+
+	clk = devm_clk_get(&pdev->dev, "usbhs");
+	if (IS_ERR(clk)) {
+		dev_err(&pdev->dev, "Can't get the clock\n");
+		return PTR_ERR(clk);
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		dev_err(dev, "Memory allocation failed\n");
+		return -ENOMEM;
+	}
+
+	spin_lock_init(&priv->lock);
+	priv->clk = clk;
+	priv->base = base;
+	priv->ugctrl2 = pdata->chan0_pci ?
+			USBHS_UGCTRL2_USB0_PCI : USBHS_UGCTRL2_USB0_HS;
+	priv->ugctrl2 |= pdata->chan2_pci ?
+			USBHS_UGCTRL2_USB2_PCI : USBHS_UGCTRL2_USB2_SS;
+	priv->phy.dev = dev;
+	priv->phy.label = dev_name(dev);
+	priv->phy.init = rcar_gen2_usb_phy_init;
+	priv->phy.shutdown = rcar_gen2_usb_phy_shutdown;
+	priv->phy.set_suspend = rcar_gen2_usb_phy_set_suspend;
+
+	retval = usb_add_phy(&priv->phy, USB_PHY_TYPE_USB2);
+	if (retval < 0) {
+		dev_err(dev, "Failed to add USB phy\n");
+		return retval;
+	}
+
+	platform_set_drvdata(pdev, priv);
+
+	return retval;
+}
+
+static int rcar_gen2_usb_phy_remove(struct platform_device *pdev)
+{
+	struct rcar_gen2_usb_phy_priv *priv = platform_get_drvdata(pdev);
+
+	usb_remove_phy(&priv->phy);
+
+	return 0;
+}
+
+static struct platform_driver rcar_gen2_usb_phy_driver = {
+	.driver = {
+		.name = "usb_phy_rcar_gen2",
+	},
+	.probe = rcar_gen2_usb_phy_probe,
+	.remove = rcar_gen2_usb_phy_remove,
+};
+
+module_platform_driver(rcar_gen2_usb_phy_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Renesas R-Car Gen2 USB phy");
+MODULE_AUTHOR("Valentine Barshak <valentine.barshak@cogentembedded.com>");
diff --git a/include/linux/platform_data/usb-rcar-gen2-phy.h b/include/linux/platform_data/usb-rcar-gen2-phy.h
new file mode 100644
index 0000000..dd3ba46
--- /dev/null
+++ b/include/linux/platform_data/usb-rcar-gen2-phy.h
@@ -0,0 +1,22 @@
+/*
+ * Copyright (C) 2013 Renesas Solutions Corp.
+ * Copyright (C) 2013 Cogent Embedded, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __USB_RCAR_GEN2_PHY_H
+#define __USB_RCAR_GEN2_PHY_H
+
+#include <linux/types.h>
+
+struct rcar_gen2_phy_platform_data {
+	/* USB channel 0 configuration */
+	bool chan0_pci:1;	/* true: PCI USB host 0, false: USBHS */
+	/* USB channel 2 configuration */
+	bool chan2_pci:1;	/* true: PCI USB host 2, false: USBSS */
+};
+
+#endif
-- 
1.8.3.1

Re: [PATCH 1/3] usb: phy: Add RCAR Gen2 USB phy

[Felipe Balbi]

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

On Wed, Oct 09, 2013 at 10:32:12PM +0200, Laurent Pinchart wrote:
> Hi Valentine,
> 
> Thank you for the patch.
> 
> On Tuesday 08 October 2013 23:43:25 Valentine Barshak wrote:
> > This adds RCAR Gen2 USB phy support. The driver configures
> > USB channels 0/2 which are shared between PCI USB hosts and
> > USBHS/USBSS devices. It also controls internal USBHS phy.
> > 
> > Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
> > ---
> >  drivers/usb/phy/Kconfig                         |  13 ++
> >  drivers/usb/phy/Makefile                        |   1 +
> >  drivers/usb/phy/phy-rcar-gen2-usb.c             | 255 +++++++++++++++++++++
> >  include/linux/platform_data/usb-rcar-gen2-phy.h |  22 ++
> >  4 files changed, 291 insertions(+)
> >  create mode 100644 drivers/usb/phy/phy-rcar-gen2-usb.c
> >  create mode 100644 include/linux/platform_data/usb-rcar-gen2-phy.h
> > 
> > diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> > index d5589f9..297062c 100644
> > --- a/drivers/usb/phy/Kconfig
> > +++ b/drivers/usb/phy/Kconfig
> > @@ -214,6 +214,19 @@ config USB_RCAR_PHY
> >  	  To compile this driver as a module, choose M here: the
> >  	  module will be called phy-rcar-usb.
> > 
> > +config USB_RCAR_GEN2_PHY
> > +	tristate "Renesas R-Car Gen2 USB PHY support"
> > +	depends on ARCH_R8A7790 || ARCH_R8A7791
> 
> From a development point of view it's always nice to be able to compile the 
> driver for a wider range of devices, even if the device is only found in the 
> R8A779[01]. This allows catching compilation errors, for instance caused by 
> API changes that affect all drivers using the API being modified.
> 
> I would use either
> 
> 	depends on ARM
> 
> or
> 
> 	depends on ARCH_R8A7790 || ARCH_R8A7791 || COMPILE_TEST
> 
> (assuming the driver can compile on non-ARM platforms, otherwise the above 
> line could be changed to ARCH_R8A7790 || ARCH_R8A7791 || (ARM && 
> COMPILE_TEST)).

every driver should compile on all arches. I will be enforcing
COMPILE_TEST before applying any new drivers.

-- 
balbi

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

Re: [PATCH 1/3] usb: phy: Add RCAR Gen2 USB phy

[Felipe Balbi]

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

Hi,

On Thu, Oct 10, 2013 at 01:21:27AM +0400, Valentine wrote:
> On 10/10/2013 12:32 AM, Laurent Pinchart wrote:
> >Hi Valentine,
> >
> 
> Hi Laurent,
> 
> >Thank you for the patch.
> >
> >On Tuesday 08 October 2013 23:43:25 Valentine Barshak wrote:
> >>This adds RCAR Gen2 USB phy support. The driver configures
> >>USB channels 0/2 which are shared between PCI USB hosts and
> >>USBHS/USBSS devices. It also controls internal USBHS phy.
> >>
> >>Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
> >>---
> >>  drivers/usb/phy/Kconfig                         |  13 ++
> >>  drivers/usb/phy/Makefile                        |   1 +
> >>  drivers/usb/phy/phy-rcar-gen2-usb.c             | 255 +++++++++++++++++++++
> >>  include/linux/platform_data/usb-rcar-gen2-phy.h |  22 ++
> >>  4 files changed, 291 insertions(+)
> >>  create mode 100644 drivers/usb/phy/phy-rcar-gen2-usb.c
> >>  create mode 100644 include/linux/platform_data/usb-rcar-gen2-phy.h
> >>
> >>diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> >>index d5589f9..297062c 100644
> >>--- a/drivers/usb/phy/Kconfig
> >>+++ b/drivers/usb/phy/Kconfig
> >>@@ -214,6 +214,19 @@ config USB_RCAR_PHY
> >>  	  To compile this driver as a module, choose M here: the
> >>  	  module will be called phy-rcar-usb.
> >>
> >>+config USB_RCAR_GEN2_PHY
> >>+	tristate "Renesas R-Car Gen2 USB PHY support"
> >>+	depends on ARCH_R8A7790 || ARCH_R8A7791
> >
> > From a development point of view it's always nice to be able to compile the
> >driver for a wider range of devices, even if the device is only found in the
> >R8A779[01]. This allows catching compilation errors, for instance caused by
> >API changes that affect all drivers using the API being modified.
> 
> Compiling a dirver for an unsupported architecture also seems to be
> more error-prone.

quite the opposite, it helps finding wrong includes and indirect
inclusion mistakes.

> >I would use either
> >
> >	depends on ARM
> >
> >or
> >
> >	depends on ARCH_R8A7790 || ARCH_R8A7791 || COMPILE_TEST
> >
> >(assuming the driver can compile on non-ARM platforms, otherwise the above
> >line could be changed to ARCH_R8A7790 || ARCH_R8A7791 || (ARM &&
> >COMPILE_TEST)).
> 
> OK, I'll take a look.
> Do all the drivers have to support COMPILE_TEST?

new ones, yes. Older ones, just a few. It depends on the subsystem too,
I like to enforce COMPILE_TEST because it helps me build-testing drivers
for which I don't have the HW.

-- 
balbi

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

Re: [PATCH 1/3] usb: phy: Add RCAR Gen2 USB phy

[Felipe Balbi]

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

Hi,

On Thu, Oct 10, 2013 at 01:47:26AM +0400, Valentine wrote:
> >>>I would use either
> >>>
> >>>	depends on ARM
> >>>
> >>>or
> >>>
> >>>	depends on ARCH_R8A7790 || ARCH_R8A7791 || COMPILE_TEST
> >>>
> >>>(assuming the driver can compile on non-ARM platforms, otherwise the above
> >>>line could be changed to ARCH_R8A7790 || ARCH_R8A7791 || (ARM &&
> >>>COMPILE_TEST)).
> >>
> >>OK, I'll take a look.
> >>Do all the drivers have to support COMPILE_TEST?
> >
> >There's currently no rule, but if the driver can only be compiled for a
> >restricted set of platforms, I would say that supporting COMPILE_TEST would be
> >a good practice. It of course needs to be restricted to the platforms on which
> >the driver will actually compile :-)
> 
> OK thanks.
> I'll probably go with the safest option:
> ARCH_R8A7790 || ARCH_R8A7791 || (ARM && COMPILE_TEST)
> 
> It does compile on x86 though, and may compile on other platforms as well.

why not ? What's the reason for not building under x86 ? Drivers are
supposed to be just plain C, completely portable.

-- 
balbi

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

Re: [PATCH 1/3] usb: phy: Add RCAR Gen2 USB phy

[Felipe Balbi]

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

Hi,

On Thu, Oct 10, 2013 at 02:14:45AM +0400, Valentine Barshak wrote:
> This adds RCAR Gen2 USB phy support. The driver configures
> USB channels 0/2 which are shared between PCI USB hosts and
> USBHS/USBSS devices. It also controls internal USBHS phy.
> 
> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
> ---
>  drivers/usb/phy/Kconfig                         |  13 ++
>  drivers/usb/phy/Makefile                        |   1 +
>  drivers/usb/phy/phy-rcar-gen2-usb.c             | 248 ++++++++++++++++++++++++
>  include/linux/platform_data/usb-rcar-gen2-phy.h |  22 +++
>  4 files changed, 284 insertions(+)
>  create mode 100644 drivers/usb/phy/phy-rcar-gen2-usb.c
>  create mode 100644 include/linux/platform_data/usb-rcar-gen2-phy.h
> 
> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index d5589f9..bba7d29 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -214,6 +214,19 @@ config USB_RCAR_PHY
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called phy-rcar-usb.
>  
> +config USB_RCAR_GEN2_PHY
> +	tristate "Renesas R-Car Gen2 USB PHY support"
> +	depends on ARCH_R8A7790 || ARCH_R8A7791 || (ARM && COMPILE_TEST)

see my question on other thread, why the forced ARM dependency ?

-- 
balbi

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

Re: [PATCH 1/3] usb: phy: Add RCAR Gen2 USB phy

[Valentine]

On 10/10/2013 07:23 PM, Felipe Balbi wrote:
> Hi,
>
> On Thu, Oct 10, 2013 at 02:14:45AM +0400, Valentine Barshak wrote:
>> This adds RCAR Gen2 USB phy support. The driver configures
>> USB channels 0/2 which are shared between PCI USB hosts and
>> USBHS/USBSS devices. It also controls internal USBHS phy.
>>
>> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
>> ---
>>   drivers/usb/phy/Kconfig                         |  13 ++
>>   drivers/usb/phy/Makefile                        |   1 +
>>   drivers/usb/phy/phy-rcar-gen2-usb.c             | 248 ++++++++++++++++++++++++
>>   include/linux/platform_data/usb-rcar-gen2-phy.h |  22 +++
>>   4 files changed, 284 insertions(+)
>>   create mode 100644 drivers/usb/phy/phy-rcar-gen2-usb.c
>>   create mode 100644 include/linux/platform_data/usb-rcar-gen2-phy.h
>>
>> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
>> index d5589f9..bba7d29 100644
>> --- a/drivers/usb/phy/Kconfig
>> +++ b/drivers/usb/phy/Kconfig
>> @@ -214,6 +214,19 @@ config USB_RCAR_PHY
>>   	  To compile this driver as a module, choose M here: the
>>   	  module will be called phy-rcar-usb.
>>
>> +config USB_RCAR_GEN2_PHY
>> +	tristate "Renesas R-Car Gen2 USB PHY support"
>> +	depends on ARCH_R8A7790 || ARCH_R8A7791 || (ARM && COMPILE_TEST)
>
> see my question on other thread, why the forced ARM dependency ?
>

I just can't test building it architectures other than ARM and X86, 
though I guess it should probably build fine.
Just thought restricting to ARM would be safe enough.
I'll remove ARM dependency and resubmit just this patch in a bit.

Thanks,
Val.

Re: [PATCH 1/3] usb: phy: Add RCAR Gen2 USB phy

[Felipe Balbi]

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

Hi,

On Thu, Oct 10, 2013 at 08:29:43PM +0400, Valentine wrote:
> On 10/10/2013 07:23 PM, Felipe Balbi wrote:
> >Hi,
> >
> >On Thu, Oct 10, 2013 at 02:14:45AM +0400, Valentine Barshak wrote:
> >>This adds RCAR Gen2 USB phy support. The driver configures
> >>USB channels 0/2 which are shared between PCI USB hosts and
> >>USBHS/USBSS devices. It also controls internal USBHS phy.
> >>
> >>Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
> >>---
> >>  drivers/usb/phy/Kconfig                         |  13 ++
> >>  drivers/usb/phy/Makefile                        |   1 +
> >>  drivers/usb/phy/phy-rcar-gen2-usb.c             | 248 ++++++++++++++++++++++++
> >>  include/linux/platform_data/usb-rcar-gen2-phy.h |  22 +++
> >>  4 files changed, 284 insertions(+)
> >>  create mode 100644 drivers/usb/phy/phy-rcar-gen2-usb.c
> >>  create mode 100644 include/linux/platform_data/usb-rcar-gen2-phy.h
> >>
> >>diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> >>index d5589f9..bba7d29 100644
> >>--- a/drivers/usb/phy/Kconfig
> >>+++ b/drivers/usb/phy/Kconfig
> >>@@ -214,6 +214,19 @@ config USB_RCAR_PHY
> >>  	  To compile this driver as a module, choose M here: the
> >>  	  module will be called phy-rcar-usb.
> >>
> >>+config USB_RCAR_GEN2_PHY
> >>+	tristate "Renesas R-Car Gen2 USB PHY support"
> >>+	depends on ARCH_R8A7790 || ARCH_R8A7791 || (ARM && COMPILE_TEST)
> >
> >see my question on other thread, why the forced ARM dependency ?
> >
> 
> I just can't test building it architectures other than ARM and X86,
> though I guess it should probably build fine.

right, the idea of COMPILE_TEST is exactly that. You gotta be willing to
build test on that arch. Let me rephrase that a bit: if someone with PPC
cross-compilers installed wants to build-test your driver, they can set
COMPILE_TEST on their builds, if something breaks you never "supported"
that arch, fixes are welcome though ;-)

> Just thought restricting to ARM would be safe enough.
> I'll remove ARM dependency and resubmit just this patch in a bit.

please do.

-- 
balbi

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

[PATCH 1/3] usb: phy: Add RCAR Gen2 USB phy

[Valentine Barshak]

This adds RCAR Gen2 USB phy support. The driver configures
USB channels 0/2 which are shared between PCI USB hosts and
USBHS/USBSS devices. It also controls internal USBHS phy.

Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
---
 drivers/usb/phy/Kconfig                         |  13 ++
 drivers/usb/phy/Makefile                        |   1 +
 drivers/usb/phy/phy-rcar-gen2-usb.c             | 248 ++++++++++++++++++++++++
 include/linux/platform_data/usb-rcar-gen2-phy.h |  22 +++
 4 files changed, 284 insertions(+)
 create mode 100644 drivers/usb/phy/phy-rcar-gen2-usb.c
 create mode 100644 include/linux/platform_data/usb-rcar-gen2-phy.h

diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index d5589f9..c0c8cd3 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -214,6 +214,19 @@ config USB_RCAR_PHY
 	  To compile this driver as a module, choose M here: the
 	  module will be called phy-rcar-usb.
 
+config USB_RCAR_GEN2_PHY
+	tristate "Renesas R-Car Gen2 USB PHY support"
+	depends on ARCH_R8A7790 || ARCH_R8A7791 || COMPILE_TEST
+	select USB_PHY
+	help
+	  Say Y here to add support for the Renesas R-Car Gen2 USB PHY driver.
+	  It is typically used to control internal USB PHY for USBHS,
+	  and to configure shared USB channels 0 and 2.
+	  This driver supports R8A7790 and R8A7791.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called phy-rcar-gen2-usb.
+
 config USB_ULPI
 	bool "Generic ULPI Transceiver Driver"
 	depends on ARM
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index 2135e85..8c5b147 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -29,5 +29,6 @@ obj-$(CONFIG_USB_MSM_OTG)		+= phy-msm-usb.o
 obj-$(CONFIG_USB_MV_OTG)		+= phy-mv-usb.o
 obj-$(CONFIG_USB_MXS_PHY)		+= phy-mxs-usb.o
 obj-$(CONFIG_USB_RCAR_PHY)		+= phy-rcar-usb.o
+obj-$(CONFIG_USB_RCAR_GEN2_PHY)		+= phy-rcar-gen2-usb.o
 obj-$(CONFIG_USB_ULPI)			+= phy-ulpi.o
 obj-$(CONFIG_USB_ULPI_VIEWPORT)		+= phy-ulpi-viewport.o
diff --git a/drivers/usb/phy/phy-rcar-gen2-usb.c b/drivers/usb/phy/phy-rcar-gen2-usb.c
new file mode 100644
index 0000000..a99a695
--- /dev/null
+++ b/drivers/usb/phy/phy-rcar-gen2-usb.c
@@ -0,0 +1,248 @@
+/*
+ * Renesas R-Car Gen2 USB phy driver
+ *
+ * Copyright (C) 2013 Renesas Solutions Corp.
+ * Copyright (C) 2013 Cogent Embedded, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_data/usb-rcar-gen2-phy.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/usb/otg.h>
+
+struct rcar_gen2_usb_phy_priv {
+	struct usb_phy phy;
+	void __iomem *base;
+	struct clk *clk;
+	spinlock_t lock;
+	int usecount;
+	u32 ugctrl2;
+};
+
+#define usb_phy_to_priv(p) container_of(p, struct rcar_gen2_usb_phy_priv, phy)
+
+/* Low Power Status register */
+#define USBHS_LPSTS_REG			0x02
+#define USBHS_LPSTS_SUSPM		(1 << 14)
+
+/* USB General control register */
+#define USBHS_UGCTRL_REG		0x80
+#define USBHS_UGCTRL_CONNECT		(1 << 2)
+#define USBHS_UGCTRL_PLLRESET		(1 << 0)
+
+/* USB General control register 2 */
+#define USBHS_UGCTRL2_REG		0x84
+#define USBHS_UGCTRL2_USB0_PCI		(1 << 4)
+#define USBHS_UGCTRL2_USB0_HS		(3 << 4)
+#define USBHS_UGCTRL2_USB2_PCI		(0 << 31)
+#define USBHS_UGCTRL2_USB2_SS		(1 << 31)
+
+/* USB General status register */
+#define USBHS_UGSTS_REG			0x88
+#define USBHS_UGSTS_LOCK		(3 << 8)
+
+/* Enable USBHS internal phy */
+static int __rcar_gen2_usbhs_phy_enable(void __iomem *base)
+{
+	u32 val;
+	int i;
+
+	/* USBHS PHY power on */
+	val = ioread32(base + USBHS_UGCTRL_REG);
+	val &= ~USBHS_UGCTRL_PLLRESET;
+	iowrite32(val, base + USBHS_UGCTRL_REG);
+
+	val = ioread16(base + USBHS_LPSTS_REG);
+	val |= USBHS_LPSTS_SUSPM;
+	iowrite16(val, base + USBHS_LPSTS_REG);
+
+	for (i = 0; i < 20; i++) {
+		val = ioread32(base + USBHS_UGSTS_REG);
+		if ((val & USBHS_UGSTS_LOCK) = USBHS_UGSTS_LOCK) {
+			val = ioread32(base + USBHS_UGCTRL_REG);
+			val |= USBHS_UGCTRL_CONNECT;
+			iowrite32(val, base + USBHS_UGCTRL_REG);
+			return 0;
+		}
+		udelay(1);
+	}
+
+	/* Timed out waiting for the PLL lock */
+	return -ETIMEDOUT;
+}
+
+/* Disable USBHS internal phy */
+static int __rcar_gen2_usbhs_phy_disable(void __iomem *base)
+{
+	u32 val;
+
+	/* USBHS PHY power off */
+	val = ioread32(base + USBHS_UGCTRL_REG);
+	val &= ~USBHS_UGCTRL_CONNECT;
+	iowrite32(val, base + USBHS_UGCTRL_REG);
+
+	val = ioread16(base + USBHS_LPSTS_REG);
+	val &= ~USBHS_LPSTS_SUSPM;
+	iowrite16(val, base + USBHS_LPSTS_REG);
+
+	val = ioread32(base + USBHS_UGCTRL_REG);
+	val |= USBHS_UGCTRL_PLLRESET;
+	iowrite32(val, base + USBHS_UGCTRL_REG);
+	return 0;
+}
+
+/* Setup USB channels */
+static void __rcar_gen2_usb_phy_init(struct rcar_gen2_usb_phy_priv *priv)
+{
+	u32 val;
+
+	clk_prepare_enable(priv->clk);
+
+	/* Set USB channels in the USBHS UGCTRL2 register */
+	val = ioread32(priv->base);
+	val &= ~(USBHS_UGCTRL2_USB0_HS | USBHS_UGCTRL2_USB2_SS);
+	val |= priv->ugctrl2;
+	iowrite32(val, priv->base);
+}
+
+/* Shutdown USB channels */
+static void __rcar_gen2_usb_phy_shutdown(struct rcar_gen2_usb_phy_priv *priv)
+{
+	__rcar_gen2_usbhs_phy_disable(priv->base);
+	clk_disable_unprepare(priv->clk);
+}
+
+static int rcar_gen2_usb_phy_set_suspend(struct usb_phy *phy, int suspend)
+{
+	struct rcar_gen2_usb_phy_priv *priv = usb_phy_to_priv(phy);
+	unsigned long flags;
+	int retval;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	retval = suspend ? __rcar_gen2_usbhs_phy_disable(priv->base) :
+			   __rcar_gen2_usbhs_phy_enable(priv->base);
+	spin_unlock_irqrestore(&priv->lock, flags);
+	return retval;
+}
+
+static int rcar_gen2_usb_phy_init(struct usb_phy *phy)
+{
+	struct rcar_gen2_usb_phy_priv *priv = usb_phy_to_priv(phy);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	/*
+	 * Enable the clock and setup USB channels
+	 * if it's the first user
+	 */
+	if (!priv->usecount++)
+		__rcar_gen2_usb_phy_init(priv);
+	spin_unlock_irqrestore(&priv->lock, flags);
+	return 0;
+}
+
+static void rcar_gen2_usb_phy_shutdown(struct usb_phy *phy)
+{
+	struct rcar_gen2_usb_phy_priv *priv = usb_phy_to_priv(phy);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	if (!priv->usecount) {
+		dev_warn(phy->dev, "Trying to disable phy with 0 usecount\n");
+		goto out;
+	}
+
+	/* Disable everything if it's the last user */
+	if (!--priv->usecount)
+		__rcar_gen2_usb_phy_shutdown(priv);
+out:
+	spin_unlock_irqrestore(&priv->lock, flags);
+}
+
+static int rcar_gen2_usb_phy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rcar_gen2_phy_platform_data *pdata;
+	struct rcar_gen2_usb_phy_priv *priv;
+	struct resource *res;
+	void __iomem *base;
+	struct clk *clk;
+	int retval;
+
+	pdata = dev_get_platdata(&pdev->dev);
+	if (!pdata) {
+		dev_err(dev, "No platform data\n");
+		return -EINVAL;
+	}
+
+	clk = devm_clk_get(&pdev->dev, "usbhs");
+	if (IS_ERR(clk)) {
+		dev_err(&pdev->dev, "Can't get the clock\n");
+		return PTR_ERR(clk);
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		dev_err(dev, "Memory allocation failed\n");
+		return -ENOMEM;
+	}
+
+	spin_lock_init(&priv->lock);
+	priv->clk = clk;
+	priv->base = base;
+	priv->ugctrl2 = pdata->chan0_pci ?
+			USBHS_UGCTRL2_USB0_PCI : USBHS_UGCTRL2_USB0_HS;
+	priv->ugctrl2 |= pdata->chan2_pci ?
+			USBHS_UGCTRL2_USB2_PCI : USBHS_UGCTRL2_USB2_SS;
+	priv->phy.dev = dev;
+	priv->phy.label = dev_name(dev);
+	priv->phy.init = rcar_gen2_usb_phy_init;
+	priv->phy.shutdown = rcar_gen2_usb_phy_shutdown;
+	priv->phy.set_suspend = rcar_gen2_usb_phy_set_suspend;
+
+	retval = usb_add_phy(&priv->phy, USB_PHY_TYPE_USB2);
+	if (retval < 0) {
+		dev_err(dev, "Failed to add USB phy\n");
+		return retval;
+	}
+
+	platform_set_drvdata(pdev, priv);
+
+	return retval;
+}
+
+static int rcar_gen2_usb_phy_remove(struct platform_device *pdev)
+{
+	struct rcar_gen2_usb_phy_priv *priv = platform_get_drvdata(pdev);
+
+	usb_remove_phy(&priv->phy);
+
+	return 0;
+}
+
+static struct platform_driver rcar_gen2_usb_phy_driver = {
+	.driver = {
+		.name = "usb_phy_rcar_gen2",
+	},
+	.probe = rcar_gen2_usb_phy_probe,
+	.remove = rcar_gen2_usb_phy_remove,
+};
+
+module_platform_driver(rcar_gen2_usb_phy_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Renesas R-Car Gen2 USB phy");
+MODULE_AUTHOR("Valentine Barshak <valentine.barshak@cogentembedded.com>");
diff --git a/include/linux/platform_data/usb-rcar-gen2-phy.h b/include/linux/platform_data/usb-rcar-gen2-phy.h
new file mode 100644
index 0000000..dd3ba46
--- /dev/null
+++ b/include/linux/platform_data/usb-rcar-gen2-phy.h
@@ -0,0 +1,22 @@
+/*
+ * Copyright (C) 2013 Renesas Solutions Corp.
+ * Copyright (C) 2013 Cogent Embedded, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __USB_RCAR_GEN2_PHY_H
+#define __USB_RCAR_GEN2_PHY_H
+
+#include <linux/types.h>
+
+struct rcar_gen2_phy_platform_data {
+	/* USB channel 0 configuration */
+	bool chan0_pci:1;	/* true: PCI USB host 0, false: USBHS */
+	/* USB channel 2 configuration */
+	bool chan2_pci:1;	/* true: PCI USB host 2, false: USBSS */
+};
+
+#endif
-- 
1.8.3.1