[PATCH v2 0/5] Kesytone II USB support

[PATCH v2 0/5] Kesytone II USB support

[WingMan Kwok]

Here is the updated version of the series which addresses comments from
earlier version [1].  The PHY register programming is moved to a separate
PHY driver.

Series adds USB host support for Keystone SOCs. Keystone SOCs uses dwc3
hardware IP implementation.  On Keystone II platforms, we use no-op phy driver.

All three patches are posted together just for completeness. Only first patch
is expected to go via usb tree and other two via linux-keystone tree.

Patchset are tested on Keystone II EVM with USB2.0 and USB3.0 flash drives.

Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>

WingMan Kwok (5):
  usb: dwc3: Add Keystone specific glue layer
  usb: phy: Add keystone usb phy driver
  ARM: dts: keystone: Add usb phy devicetree bindings
  ARM: dts: keystone: Add usb devicetree bindings
  ARM: keystone: defconfig: enable USB support

 .../devicetree/bindings/usb/keystone-phy.txt       |   19 ++
 .../devicetree/bindings/usb/keystone-usb.txt       |   41 ++++
 arch/arm/boot/dts/keystone.dtsi                    |   25 +++
 arch/arm/configs/keystone_defconfig                |   20 +-
 drivers/usb/dwc3/Kconfig                           |    7 +
 drivers/usb/dwc3/Makefile                          |    1 +
 drivers/usb/dwc3/dwc3-keystone.c                   |  210 ++++++++++++++++++++
 drivers/usb/phy/Kconfig                            |   10 +
 drivers/usb/phy/Makefile                           |    1 +
 drivers/usb/phy/phy-keystone.c                     |  142 +++++++++++++
 10 files changed, 475 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/usb/keystone-phy.txt
 create mode 100644 Documentation/devicetree/bindings/usb/keystone-usb.txt
 create mode 100644 drivers/usb/dwc3/dwc3-keystone.c
 create mode 100644 drivers/usb/phy/phy-keystone.c

[1] http://www.spinics.net/lists/linux-usb/msg98166.html
-- 
1.7.9.5

[PATCH v2 1/5] usb: dwc3: Add Keystone specific glue layer

[WingMan Kwok]

Add Keystone platform specific glue layer to support
USB3 Host mode.

Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Felipe Balbi <balbi@ti.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
---
 drivers/usb/dwc3/Kconfig         |    7 ++
 drivers/usb/dwc3/Makefile        |    1 +
 drivers/usb/dwc3/dwc3-keystone.c |  210 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 218 insertions(+)
 create mode 100644 drivers/usb/dwc3/dwc3-keystone.c

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 70fc430..e2c730f 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -70,6 +70,13 @@ config USB_DWC3_PCI
 	  One such PCIe-based platform is Synopsys' PCIe HAPS model of
 	  this IP.
 
+config USB_DWC3_KEYSTONE
+	tristate "Texas Instruments Keystone2 Platforms"
+	default USB_DWC3
+	help
+	  Support of USB2/3 functionality in TI Keystone2 platforms.
+	  Say 'Y' or 'M' here if you have one such device
+
 comment "Debugging features"
 
 config USB_DWC3_DEBUG
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index dd17601..10ac3e7 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -32,3 +32,4 @@ endif
 obj-$(CONFIG_USB_DWC3_OMAP)		+= dwc3-omap.o
 obj-$(CONFIG_USB_DWC3_EXYNOS)		+= dwc3-exynos.o
 obj-$(CONFIG_USB_DWC3_PCI)		+= dwc3-pci.o
+obj-$(CONFIG_USB_DWC3_KEYSTONE)		+= dwc3-keystone.o
diff --git a/drivers/usb/dwc3/dwc3-keystone.c b/drivers/usb/dwc3/dwc3-keystone.c
new file mode 100644
index 0000000..d3c0dc5
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-keystone.c
@@ -0,0 +1,210 @@
+/**
+ * dwc3-keystone.c - Keystone Specific Glue layer
+ *
+ * Copyright (C) 2010-2013 Texas Instruments Incorporated - http://www.ti.com
+ *
+ * Author: WingMan Kwok <w-kwok2@ti.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License as published by the Free Software Foundation.
+ *
+ * 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/clk.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/interrupt.h>
+#include <linux/spinlock.h>
+#include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
+#include <linux/io.h>
+#include <linux/of_platform.h>
+
+/* USBSS register offsets */
+#define USBSS_REVISION		0x0000
+#define USBSS_SYSCONFIG		0x0010
+#define USBSS_IRQ_EOI		0x0018
+#define USBSS_IRQSTATUS_RAW_0	0x0020
+#define USBSS_IRQSTATUS_0	0x0024
+#define USBSS_IRQENABLE_SET_0	0x0028
+#define USBSS_IRQENABLE_CLR_0	0x002c
+
+/* IRQ register bits */
+#define USBSS_IRQ_EOI_LINE(n)	BIT(n)
+#define USBSS_IRQ_EVENT_ST	BIT(0)
+#define USBSS_IRQ_COREIRQ_EN	BIT(0)
+#define USBSS_IRQ_COREIRQ_CLR	BIT(0)
+
+static u64 kdwc3_dma_mask;
+
+struct dwc3_keystone {
+	spinlock_t			lock;
+	struct device			*dev;
+	struct clk			*clk;
+	void __iomem			*usbss;
+};
+
+static inline u32 kdwc3_readl(void __iomem *base, u32 offset)
+{
+	return readl(base + offset);
+}
+
+static inline void kdwc3_writel(void __iomem *base, u32 offset, u32 value)
+{
+	writel(value, base + offset);
+}
+
+static void kdwc3_enable_irqs(struct dwc3_keystone *kdwc)
+{
+	u32 val;
+
+	val = kdwc3_readl(kdwc->usbss, USBSS_IRQENABLE_SET_0);
+	val = USBSS_IRQ_COREIRQ_EN;
+	kdwc3_writel(kdwc->usbss, USBSS_IRQENABLE_SET_0, val);
+}
+
+static void kdwc3_disable_irqs(struct dwc3_keystone *kdwc)
+{
+	u32 val;
+
+	val = kdwc3_readl(kdwc->usbss, USBSS_IRQENABLE_SET_0);
+	val &= ~USBSS_IRQ_COREIRQ_EN;
+	kdwc3_writel(kdwc->usbss, USBSS_IRQENABLE_SET_0, val);
+}
+
+static irqreturn_t dwc3_keystone_interrupt(int irq, void *_kdwc)
+{
+	struct dwc3_keystone	*kdwc = _kdwc;
+
+	spin_lock(&kdwc->lock);
+	kdwc3_writel(kdwc->usbss, USBSS_IRQENABLE_CLR_0, USBSS_IRQ_COREIRQ_CLR);
+	kdwc3_writel(kdwc->usbss, USBSS_IRQSTATUS_0, USBSS_IRQ_EVENT_ST);
+	kdwc3_writel(kdwc->usbss, USBSS_IRQENABLE_SET_0, USBSS_IRQ_COREIRQ_EN);
+	kdwc3_writel(kdwc->usbss, USBSS_IRQ_EOI, USBSS_IRQ_EOI_LINE(0));
+	spin_unlock(&kdwc->lock);
+
+	return IRQ_HANDLED;
+}
+
+static int kdwc3_probe(struct platform_device *pdev)
+{
+	struct device_node	*node = pdev->dev.of_node;
+	struct device		*dev = &pdev->dev;
+	struct dwc3_keystone	*kdwc;
+	struct resource		*res;
+	int			error, irq;
+
+	kdwc = devm_kzalloc(dev, sizeof(*kdwc), GFP_KERNEL);
+	if (!kdwc)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, kdwc);
+
+	spin_lock_init(&kdwc->lock);
+	kdwc->dev = dev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(dev, "missing usbss resource\n");
+		return -EINVAL;
+	}
+
+	kdwc->usbss = devm_ioremap_resource(dev, res);
+	if (IS_ERR(kdwc->usbss))
+		return PTR_ERR(kdwc->usbss);
+
+	kdwc3_dma_mask = dma_get_mask(dev);
+	dev->dma_mask = &kdwc3_dma_mask;
+
+	kdwc->clk = devm_clk_get(kdwc->dev, "usb");
+	if (IS_ERR_OR_NULL(kdwc->clk)) {
+		dev_err(kdwc->dev, "unable to get kdwc usb clock\n");
+		return -ENODEV;
+	}
+
+	error = clk_prepare_enable(kdwc->clk);
+	if (error < 0) {
+		dev_dbg(kdwc->dev, "unable to enable usb clock, err %d\n",
+			error);
+		return error;
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "missing irq\n");
+		goto err_irq;
+	}
+
+	error = devm_request_irq(dev, irq, dwc3_keystone_interrupt, IRQF_SHARED,
+			dev_name(dev), kdwc);
+	if (error) {
+		dev_err(dev, "failed to request IRQ #%d --> %d\n",
+				irq, error);
+		goto err_irq;
+	}
+
+	kdwc3_enable_irqs(kdwc);
+
+	error = of_platform_populate(node, NULL, NULL, dev);
+	if (error) {
+		dev_err(&pdev->dev, "failed to create dwc3 core\n");
+		goto err_core;
+	}
+
+	return 0;
+
+err_core:
+	kdwc3_disable_irqs(kdwc);
+err_irq:
+	clk_disable_unprepare(kdwc->clk);
+
+	return error;
+}
+
+static int kdwc3_remove_core(struct device *dev, void *c)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+
+	platform_device_unregister(pdev);
+
+	return 0;
+}
+
+static int kdwc3_remove(struct platform_device *pdev)
+{
+	struct dwc3_keystone *kdwc = platform_get_drvdata(pdev);
+
+	kdwc3_disable_irqs(kdwc);
+	clk_disable_unprepare(kdwc->clk);
+	device_for_each_child(&pdev->dev, NULL, kdwc3_remove_core);
+	platform_set_drvdata(pdev, NULL);
+	return 0;
+}
+
+static const struct of_device_id kdwc3_of_match[] = {
+	{ .compatible = "ti,keystone-dwc3", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, kdwc3_of_match);
+
+static struct platform_driver kdwc3_driver = {
+	.probe		= kdwc3_probe,
+	.remove		= kdwc3_remove,
+	.driver		= {
+		.name	= "keystone-dwc3",
+		.owner	        = THIS_MODULE,
+		.of_match_table	= kdwc3_of_match,
+	},
+};
+
+module_platform_driver(kdwc3_driver);
+
+MODULE_ALIAS("platform:keystone-dwc3");
+MODULE_AUTHOR("WingMan Kwok <w-kwok2@ti.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("DesignWare USB3 KEYSTONE Glue Layer");
-- 
1.7.9.5

[PATCH v2 2/5] usb: phy: Add keystone usb phy driver

[WingMan Kwok]

Add Keystone platform USB PHY driver support. Current main purpose
of this driver is to enable the PHY reference clock gate on the
Keystone SoC. Otherwise it is a nop PHY.

Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Felipe Balbi <balbi@ti.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
---
 drivers/usb/phy/Kconfig        |   10 +++
 drivers/usb/phy/Makefile       |    1 +
 drivers/usb/phy/phy-keystone.c |  142 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 153 insertions(+)
 create mode 100644 drivers/usb/phy/phy-keystone.c

diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 08e2f39..c6792f43 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -40,6 +40,16 @@ config ISP1301_OMAP
 	  This driver can also be built as a module.  If so, the module
 	  will be called isp1301_omap.
 
+config KEYSTONE_USB_PHY
+	tristate "Keystone USB PHY Driver"
+	depends on ARCH_KEYSTONE
+	select USB_PHY
+	select NOP_USB_XCEIV
+	help
+	  Enable this to support Keystone USB phy. This driver provides
+	  interface to interact with USB 2.0 and USB 3.0 PHY that is part
+	  of the Keystone SOC.
+
 config MV_U3D_PHY
 	bool "Marvell USB 3.0 PHY controller Driver"
 	depends on CPU_MMP3
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index 022c1da..311b47b 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -30,3 +30,4 @@ 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
+obj-$(CONFIG_KEYSTONE_USB_PHY)		+= phy-keystone.o
diff --git a/drivers/usb/phy/phy-keystone.c b/drivers/usb/phy/phy-keystone.c
new file mode 100644
index 0000000..e025eb2
--- /dev/null
+++ b/drivers/usb/phy/phy-keystone.c
@@ -0,0 +1,142 @@
+/*
+ * phy-keystone - USB PHY, talking to dwc3 controller in Keystone.
+ *
+ * Copyright (C) 2013 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: WingMan Kwok <w-kwok2@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/usb/usb_phy_gen_xceiv.h>
+#include <linux/io.h>
+#include <linux/of.h>
+
+#include "phy-generic.h"
+
+/* USB PHY control register offsets */
+#define USB_PHY_CTL_UTMI		0x0000
+#define USB_PHY_CTL_PIPE		0x0004
+#define USB_PHY_CTL_PARAM_1		0x0008
+#define USB_PHY_CTL_PARAM_2		0x000c
+#define USB_PHY_CTL_CLOCK		0x0010
+#define USB_PHY_CTL_PLL			0x0014
+
+#define PHY_REF_SSP_EN			BIT(29)
+
+struct keystone_usbphy {
+	struct usb_phy_gen_xceiv	usb_phy_gen;
+	void __iomem			*phy_ctrl;
+};
+
+static inline u32 keystone_usbphy_readl(void __iomem *base, u32 offset)
+{
+	return readl(base + offset);
+}
+
+static inline void keystone_usbphy_writel(void __iomem *base,
+					  u32 offset, u32 value)
+{
+	writel(value, base + offset);
+}
+
+static int keystone_usbphy_init(struct usb_phy *phy)
+{
+	struct keystone_usbphy *k_phy = dev_get_drvdata(phy->dev);
+	u32 val;
+
+	val  = keystone_usbphy_readl(k_phy->phy_ctrl, USB_PHY_CTL_CLOCK);
+	keystone_usbphy_writel(k_phy->phy_ctrl, USB_PHY_CTL_CLOCK,
+				val | PHY_REF_SSP_EN);
+	udelay(20);
+	return 0;
+}
+
+static void keystone_usbphy_shutdown(struct usb_phy *phy)
+{
+	struct keystone_usbphy *k_phy = dev_get_drvdata(phy->dev);
+	u32 val;
+
+	val  = keystone_usbphy_readl(k_phy->phy_ctrl, USB_PHY_CTL_CLOCK);
+	keystone_usbphy_writel(k_phy->phy_ctrl, USB_PHY_CTL_CLOCK,
+				val &= ~PHY_REF_SSP_EN);
+}
+
+static int keystone_usbphy_probe(struct platform_device *pdev)
+{
+	struct device		*dev = &pdev->dev;
+	struct keystone_usbphy	*k_phy;
+	struct resource		*res;
+	int ret;
+
+	k_phy = devm_kzalloc(dev, sizeof(*k_phy), GFP_KERNEL);
+	if (!k_phy)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(dev, "missing usb phy resource\n");
+		return -EINVAL;
+	}
+
+	k_phy->phy_ctrl = devm_ioremap_resource(dev, res);
+	if (IS_ERR(k_phy->phy_ctrl))
+		return PTR_ERR(k_phy->phy_ctrl);
+
+	ret = usb_phy_gen_create_phy(dev, &k_phy->usb_phy_gen,
+			USB_PHY_TYPE_USB2, 0, false);
+	if (ret)
+		return ret;
+
+	ret = usb_add_phy_dev(&k_phy->usb_phy_gen.phy);
+	if (ret)
+		return ret;
+	k_phy->usb_phy_gen.phy.init = keystone_usbphy_init;
+	k_phy->usb_phy_gen.phy.shutdown = keystone_usbphy_shutdown;
+
+	platform_set_drvdata(pdev, k_phy);
+
+	return 0;
+}
+
+static int keystone_usbphy_remove(struct platform_device *pdev)
+{
+	struct keystone_usbphy *k_phy = platform_get_drvdata(pdev);
+
+	usb_remove_phy(&k_phy->usb_phy_gen.phy);
+
+	return 0;
+}
+
+static const struct of_device_id keystone_usbphy_ids[] = {
+	{ .compatible = "ti,keystone-usbphy" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, keystone_usbphy_ids);
+
+static struct platform_driver keystone_usbphy_driver = {
+	.probe          = keystone_usbphy_probe,
+	.remove         = keystone_usbphy_remove,
+	.driver         = {
+		.name   = "keystone-usbphy",
+		.owner  = THIS_MODULE,
+		.of_match_table = of_match_ptr(keystone_usbphy_ids),
+	},
+};
+
+module_platform_driver(keystone_usbphy_driver);
+
+MODULE_ALIAS("platform: keystone-usbphy");
+MODULE_AUTHOR("Texas Instruments Inc.");
+MODULE_DESCRIPTION("Keystone USB phy driver");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5

[PATCH v2 3/5] ARM: dts: keystone: Add usb phy devicetree bindings

[WingMan Kwok]

Added device tree support for TI's Keystone USB PHY driver and updated the
Documentation with device tree binding information.

Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
---
 .../devicetree/bindings/usb/keystone-phy.txt       |   19 +++++++++++++++++++
 arch/arm/boot/dts/keystone.dtsi                    |    7 +++++++
 2 files changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/keystone-phy.txt

diff --git a/Documentation/devicetree/bindings/usb/keystone-phy.txt b/Documentation/devicetree/bindings/usb/keystone-phy.txt
new file mode 100644
index 0000000..300830d
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/keystone-phy.txt
@@ -0,0 +1,19 @@
+TI Keystone USB PHY
+
+Required properties:
+ - compatible: should be "ti,keystone-usbphy".
+ - #address-cells, #size-cells : should be '1' if the device has sub-nodes
+   with 'reg' property.
+ - reg : Address and length of the usb phy control register set.
+
+The main purpose of this PHY driver is to enable the USB PHY reference clock
+gate on the Keystone SOC for both the USB2 and USB3 PHY. Otherwise it is just
+an NOP PHY driver.  Hence this node is referenced as both the usb2 and usb3
+phy node in the USB Glue layer driver node.
+
+usb_phy: usb_phy at 2620738 {
+	compatible = "ti,keystone-usbphy";
+	#address-cells = <1>;
+	#size-cells = <1>;
+	reg = <0x2620738 32>;
+};
diff --git a/arch/arm/boot/dts/keystone.dtsi b/arch/arm/boot/dts/keystone.dtsi
index f6d6d9e..d497d9e 100644
--- a/arch/arm/boot/dts/keystone.dtsi
+++ b/arch/arm/boot/dts/keystone.dtsi
@@ -181,5 +181,12 @@
 			interrupts = <GIC_SPI 300 IRQ_TYPE_EDGE_RISING>;
 			clocks = <&clkspi>;
 		};
+
+		usb_phy: usb_phy at 2620738 {
+			compatible = "ti,keystone-usbphy";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			reg = <0x2620738 32>;
+		};
 	};
 };
-- 
1.7.9.5

[PATCH v2 4/5] ARM: dts: keystone: Add usb devicetree bindings

[WingMan Kwok]

Added device tree support for TI's Keystone USB driver and updated the
Documentation with device tree binding information.

Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
---
 .../devicetree/bindings/usb/keystone-usb.txt       |   41 ++++++++++++++++++++
 arch/arm/boot/dts/keystone.dtsi                    |   18 +++++++++
 2 files changed, 59 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/keystone-usb.txt

diff --git a/Documentation/devicetree/bindings/usb/keystone-usb.txt b/Documentation/devicetree/bindings/usb/keystone-usb.txt
new file mode 100644
index 0000000..ac2c7cc
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/keystone-usb.txt
@@ -0,0 +1,41 @@
+TI Keystone Soc USB Controller
+
+DWC3 GLUE
+
+Required properties:
+ - compatible: should be "ti,keystone-dwc3".
+ - #address-cells, #size-cells : should be '1' if the device has sub-nodes
+   with 'reg' property.
+ - reg : Address and length of the register set for the USB subsystem on
+   the SOC.
+ - interrupts : The irq number of this device that is used to interrupt the
+   MPU.
+ - ranges: allows valid 1:1 translation between child's address space and
+   parent's address space.
+ - clocks: Clock IDs array as required by the controller.
+ - clock-names: names of clocks correseponding to IDs in the clock property.
+
+Sub-nodes:
+The dwc3 core should be added as subnode to Keystone DWC3 glue.
+- dwc3 :
+   The binding details of dwc3 can be found in:
+   Documentation/devicetree/bindings/usb/dwc3.txt
+
+Example:
+	usb: usb at 2680000 {
+		compatible = "ti,keystone-dwc3";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0x2680000 0x10000>;
+		clocks = <&clkusb>;
+		clock-names = "usb";
+		interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
+		ranges;
+
+		dwc3 at 2690000 {
+			compatible = "synopsys,dwc3";
+			reg = <0x2690000 0x70000>;
+			interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
+			usb-phy = <&usb_phy>, <&usb_phy>;
+		};
+	};
diff --git a/arch/arm/boot/dts/keystone.dtsi b/arch/arm/boot/dts/keystone.dtsi
index d497d9e..0911bb7 100644
--- a/arch/arm/boot/dts/keystone.dtsi
+++ b/arch/arm/boot/dts/keystone.dtsi
@@ -188,5 +188,23 @@
 			#size-cells = <1>;
 			reg = <0x2620738 32>;
 		};
+
+		usb: usb at 2680000 {
+			compatible = "ti,keystone-dwc3";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			reg = <0x2680000 0x10000>;
+			clocks = <&clkusb>;
+			clock-names = "usb";
+			interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
+			ranges;
+
+			dwc3 at 2690000 {
+				compatible = "synopsys,dwc3";
+				reg = <0x2690000 0x70000>;
+				interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
+				usb-phy = <&usb_phy>, <&usb_phy>;
+			};
+		};
 	};
 };
-- 
1.7.9.5

[PATCH v2 5/5] ARM: keystone: defconfig: enable USB support

[WingMan Kwok]

Enable the USB support (Host mode only) on TI's Keystone platform.
It also enables the support of usb mass storage, FAT and Ext4
filesystems to test rootfs mount over an USB disk.

Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
---
 arch/arm/configs/keystone_defconfig |   20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/arch/arm/configs/keystone_defconfig b/arch/arm/configs/keystone_defconfig
index 9943e5d..a018244 100644
--- a/arch/arm/configs/keystone_defconfig
+++ b/arch/arm/configs/keystone_defconfig
@@ -115,6 +115,8 @@ CONFIG_MTD_UBI=y
 CONFIG_PROC_DEVICETREE=y
 CONFIG_BLK_DEV_LOOP=y
 CONFIG_EEPROM_AT24=y
+CONFIG_SCSI=y
+CONFIG_BLK_DEV_SD=y
 CONFIG_NETDEVICES=y
 CONFIG_SERIAL_8250=y
 CONFIG_SERIAL_8250_CONSOLE=y
@@ -129,10 +131,24 @@ CONFIG_SPI_DAVINCI=y
 CONFIG_SPI_SPIDEV=y
 # CONFIG_HWMON is not set
 CONFIG_WATCHDOG=y
-# CONFIG_USB_SUPPORT is not set
+CONFIG_USB=y
+CONFIG_USB_DEBUG=y
+CONFIG_USB_ANNOUNCE_NEW_DEVICES=y
+CONFIG_USB_MON=y
+CONFIG_USB_XHCI_HCD=y
+CONFIG_USB_STORAGE=y
+CONFIG_USB_DWC3=y
+CONFIG_USB_DWC3_DEBUG=y
+CONFIG_USB_DWC3_VERBOSE=y
+CONFIG_KEYSTONE_USB_PHY=y
 CONFIG_DMADEVICES=y
 CONFIG_COMMON_CLK_DEBUG=y
 CONFIG_MEMORY=y
+CONFIG_EXT4_FS=y
+CONFIG_EXT4_FS_POSIX_ACL=y
+CONFIG_MSDOS_FS=y
+CONFIG_VFAT_FS=y
+CONFIG_NTFS_FS=y
 CONFIG_TMPFS=y
 CONFIG_JFFS2_FS=y
 CONFIG_JFFS2_FS_WBUF_VERIFY=y
@@ -144,6 +160,8 @@ CONFIG_ROOT_NFS=y
 CONFIG_NFSD=y
 CONFIG_NFSD_V3=y
 CONFIG_NFSD_V3_ACL=y
+CONFIG_NLS_CODEPAGE_437=y
+CONFIG_NLS_ISO8859_1=y
 CONFIG_PRINTK_TIME=y
 CONFIG_DEBUG_SHIRQ=y
 CONFIG_DEBUG_INFO=y
-- 
1.7.9.5

[PATCH v2 1/5] usb: dwc3: Add Keystone specific glue layer

[Felipe Balbi]

Hi,

On Wed, Dec 04, 2013 at 03:10:07PM -0500, WingMan Kwok wrote:
> Add Keystone platform specific glue layer to support
> USB3 Host mode.
> 
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Felipe Balbi <balbi@ti.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
> ---
>  drivers/usb/dwc3/Kconfig         |    7 ++
>  drivers/usb/dwc3/Makefile        |    1 +
>  drivers/usb/dwc3/dwc3-keystone.c |  210 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 218 insertions(+)
>  create mode 100644 drivers/usb/dwc3/dwc3-keystone.c
> 
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index 70fc430..e2c730f 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -70,6 +70,13 @@ config USB_DWC3_PCI
>  	  One such PCIe-based platform is Synopsys' PCIe HAPS model of
>  	  this IP.
>  
> +config USB_DWC3_KEYSTONE
> +	tristate "Texas Instruments Keystone2 Platforms"
> +	default USB_DWC3
> +	help
> +	  Support of USB2/3 functionality in TI Keystone2 platforms.
> +	  Say 'Y' or 'M' here if you have one such device
> +
>  comment "Debugging features"
>  
>  config USB_DWC3_DEBUG
> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> index dd17601..10ac3e7 100644
> --- a/drivers/usb/dwc3/Makefile
> +++ b/drivers/usb/dwc3/Makefile
> @@ -32,3 +32,4 @@ endif
>  obj-$(CONFIG_USB_DWC3_OMAP)		+= dwc3-omap.o
>  obj-$(CONFIG_USB_DWC3_EXYNOS)		+= dwc3-exynos.o
>  obj-$(CONFIG_USB_DWC3_PCI)		+= dwc3-pci.o
> +obj-$(CONFIG_USB_DWC3_KEYSTONE)		+= dwc3-keystone.o
> diff --git a/drivers/usb/dwc3/dwc3-keystone.c b/drivers/usb/dwc3/dwc3-keystone.c
> new file mode 100644
> index 0000000..d3c0dc5
> --- /dev/null
> +++ b/drivers/usb/dwc3/dwc3-keystone.c
> @@ -0,0 +1,210 @@
> +/**
> + * dwc3-keystone.c - Keystone Specific Glue layer
> + *
> + * Copyright (C) 2010-2013 Texas Instruments Incorporated - http://www.ti.com
> + *
> + * Author: WingMan Kwok <w-kwok2@ti.com>
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2  of
> + * the License as published by the Free Software Foundation.
> + *
> + * 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/clk.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/interrupt.h>
> +#include <linux/spinlock.h>
> +#include <linux/platform_device.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/io.h>
> +#include <linux/of_platform.h>
> +
> +/* USBSS register offsets */
> +#define USBSS_REVISION		0x0000
> +#define USBSS_SYSCONFIG		0x0010
> +#define USBSS_IRQ_EOI		0x0018
> +#define USBSS_IRQSTATUS_RAW_0	0x0020
> +#define USBSS_IRQSTATUS_0	0x0024
> +#define USBSS_IRQENABLE_SET_0	0x0028
> +#define USBSS_IRQENABLE_CLR_0	0x002c
> +
> +/* IRQ register bits */
> +#define USBSS_IRQ_EOI_LINE(n)	BIT(n)
> +#define USBSS_IRQ_EVENT_ST	BIT(0)
> +#define USBSS_IRQ_COREIRQ_EN	BIT(0)
> +#define USBSS_IRQ_COREIRQ_CLR	BIT(0)
> +
> +static u64 kdwc3_dma_mask;
> +
> +struct dwc3_keystone {
> +	spinlock_t			lock;
> +	struct device			*dev;
> +	struct clk			*clk;
> +	void __iomem			*usbss;
> +};
> +
> +static inline u32 kdwc3_readl(void __iomem *base, u32 offset)
> +{
> +	return readl(base + offset);
> +}
> +
> +static inline void kdwc3_writel(void __iomem *base, u32 offset, u32 value)
> +{
> +	writel(value, base + offset);
> +}
> +
> +static void kdwc3_enable_irqs(struct dwc3_keystone *kdwc)
> +{
> +	u32 val;
> +
> +	val = kdwc3_readl(kdwc->usbss, USBSS_IRQENABLE_SET_0);
> +	val = USBSS_IRQ_COREIRQ_EN;
> +	kdwc3_writel(kdwc->usbss, USBSS_IRQENABLE_SET_0, val);
> +}
> +
> +static void kdwc3_disable_irqs(struct dwc3_keystone *kdwc)
> +{
> +	u32 val;
> +
> +	val = kdwc3_readl(kdwc->usbss, USBSS_IRQENABLE_SET_0);
> +	val &= ~USBSS_IRQ_COREIRQ_EN;
> +	kdwc3_writel(kdwc->usbss, USBSS_IRQENABLE_SET_0, val);
> +}
> +
> +static irqreturn_t dwc3_keystone_interrupt(int irq, void *_kdwc)
> +{
> +	struct dwc3_keystone	*kdwc = _kdwc;
> +
> +	spin_lock(&kdwc->lock);

Isn't this lock unnecessary ? This handler will run with IRQs disabled
anyway.

> +	kdwc3_writel(kdwc->usbss, USBSS_IRQENABLE_CLR_0, USBSS_IRQ_COREIRQ_CLR);
> +	kdwc3_writel(kdwc->usbss, USBSS_IRQSTATUS_0, USBSS_IRQ_EVENT_ST);
> +	kdwc3_writel(kdwc->usbss, USBSS_IRQENABLE_SET_0, USBSS_IRQ_COREIRQ_EN);
> +	kdwc3_writel(kdwc->usbss, USBSS_IRQ_EOI, USBSS_IRQ_EOI_LINE(0));
> +	spin_unlock(&kdwc->lock);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int kdwc3_probe(struct platform_device *pdev)
> +{
> +	struct device_node	*node = pdev->dev.of_node;
> +	struct device		*dev = &pdev->dev;

if you invert these lines, it'll look a little nicer:

	struct device		*dev = &pdev->dev;
	struct device_node	*node = dev->of_node;

everything else looks pretty good, thanks

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131206/69c07116/attachment.sig>

[PATCH v2 2/5] usb: phy: Add keystone usb phy driver

[Felipe Balbi]

Hi,

On Wed, Dec 04, 2013 at 03:10:08PM -0500, WingMan Kwok wrote:
> Add Keystone platform USB PHY driver support. Current main purpose
> of this driver is to enable the PHY reference clock gate on the
> Keystone SoC. Otherwise it is a nop PHY.
> 
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Felipe Balbi <balbi@ti.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: WingMan Kwok <w-kwok2@ti.com>

looks good.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131206/3ea2de7e/attachment.sig>

[PATCH v2 3/5] ARM: dts: keystone: Add usb phy devicetree bindings

[Felipe Balbi]

On Wed, Dec 04, 2013 at 03:10:09PM -0500, WingMan Kwok wrote:
> Added device tree support for TI's Keystone USB PHY driver and updated the
> Documentation with device tree binding information.
> 
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
> ---
>  .../devicetree/bindings/usb/keystone-phy.txt       |   19 +++++++++++++++++++
>  arch/arm/boot/dts/keystone.dtsi                    |    7 +++++++
>  2 files changed, 26 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/keystone-phy.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/keystone-phy.txt b/Documentation/devicetree/bindings/usb/keystone-phy.txt
> new file mode 100644
> index 0000000..300830d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/keystone-phy.txt
> @@ -0,0 +1,19 @@
> +TI Keystone USB PHY
> +
> +Required properties:
> + - compatible: should be "ti,keystone-usbphy".
> + - #address-cells, #size-cells : should be '1' if the device has sub-nodes
> +   with 'reg' property.
> + - reg : Address and length of the usb phy control register set.
> +
> +The main purpose of this PHY driver is to enable the USB PHY reference clock
> +gate on the Keystone SOC for both the USB2 and USB3 PHY. Otherwise it is just
> +an NOP PHY driver.  Hence this node is referenced as both the usb2 and usb3
> +phy node in the USB Glue layer driver node.
> +
> +usb_phy: usb_phy at 2620738 {
> +	compatible = "ti,keystone-usbphy";
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	reg = <0x2620738 32>;
> +};
> diff --git a/arch/arm/boot/dts/keystone.dtsi b/arch/arm/boot/dts/keystone.dtsi
> index f6d6d9e..d497d9e 100644
> --- a/arch/arm/boot/dts/keystone.dtsi
> +++ b/arch/arm/boot/dts/keystone.dtsi
> @@ -181,5 +181,12 @@
>  			interrupts = <GIC_SPI 300 IRQ_TYPE_EDGE_RISING>;
>  			clocks = <&clkspi>;
>  		};
> +
> +		usb_phy: usb_phy at 2620738 {
> +			compatible = "ti,keystone-usbphy";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			reg = <0x2620738 32>;

should this one have status = "disabled"; and let board dts enable the
PHY ?

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131206/0e279e40/attachment.sig>

[PATCH v2 4/5] ARM: dts: keystone: Add usb devicetree bindings

[Felipe Balbi]

Hi,

On Wed, Dec 04, 2013 at 03:10:10PM -0500, WingMan Kwok wrote:
> diff --git a/Documentation/devicetree/bindings/usb/keystone-usb.txt b/Documentation/devicetree/bindings/usb/keystone-usb.txt
> new file mode 100644
> index 0000000..ac2c7cc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/keystone-usb.txt
> @@ -0,0 +1,41 @@
> +TI Keystone Soc USB Controller
> +
> +DWC3 GLUE
> +
> +Required properties:
> + - compatible: should be "ti,keystone-dwc3".
> + - #address-cells, #size-cells : should be '1' if the device has sub-nodes
> +   with 'reg' property.
> + - reg : Address and length of the register set for the USB subsystem on
> +   the SOC.
> + - interrupts : The irq number of this device that is used to interrupt the
> +   MPU.
> + - ranges: allows valid 1:1 translation between child's address space and
> +   parent's address space.
> + - clocks: Clock IDs array as required by the controller.
> + - clock-names: names of clocks correseponding to IDs in the clock property.
> +
> +Sub-nodes:
> +The dwc3 core should be added as subnode to Keystone DWC3 glue.
> +- dwc3 :
> +   The binding details of dwc3 can be found in:
> +   Documentation/devicetree/bindings/usb/dwc3.txt
> +
> +Example:
> +	usb: usb at 2680000 {
> +		compatible = "ti,keystone-dwc3";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		reg = <0x2680000 0x10000>;
> +		clocks = <&clkusb>;
> +		clock-names = "usb";
> +		interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
> +		ranges;
> +
> +		dwc3 at 2690000 {
> +			compatible = "synopsys,dwc3";
> +			reg = <0x2690000 0x70000>;
> +			interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
> +			usb-phy = <&usb_phy>, <&usb_phy>;
> +		};
> +	};
> diff --git a/arch/arm/boot/dts/keystone.dtsi b/arch/arm/boot/dts/keystone.dtsi
> index d497d9e..0911bb7 100644
> --- a/arch/arm/boot/dts/keystone.dtsi
> +++ b/arch/arm/boot/dts/keystone.dtsi
> @@ -188,5 +188,23 @@
>  			#size-cells = <1>;
>  			reg = <0x2620738 32>;
>  		};
> +
> +		usb: usb at 2680000 {

likewise, status = disabled ?

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131206/1d736762/attachment-0001.sig>

[PATCH v2 1/5] usb: dwc3: Add Keystone specific glue layer

[Santosh Shilimkar]

On Friday 06 December 2013 03:28 PM, Felipe Balbi wrote:
> Hi,
> 
> On Wed, Dec 04, 2013 at 03:10:07PM -0500, WingMan Kwok wrote:
>> Add Keystone platform specific glue layer to support
>> USB3 Host mode.
>>
>> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
>> Cc: Felipe Balbi <balbi@ti.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
>> ---
>>  drivers/usb/dwc3/Kconfig         |    7 ++
>>  drivers/usb/dwc3/Makefile        |    1 +
>>  drivers/usb/dwc3/dwc3-keystone.c |  210 ++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 218 insertions(+)
>>  create mode 100644 drivers/usb/dwc3/dwc3-keystone.c

[..]

>> +static irqreturn_t dwc3_keystone_interrupt(int irq, void *_kdwc)
>> +{
>> +	struct dwc3_keystone	*kdwc = _kdwc;
>> +
>> +	spin_lock(&kdwc->lock);
> 
> Isn't this lock unnecessary ? This handler will run with IRQs disabled
> anyway.
> 
Indeed.

>> +	kdwc3_writel(kdwc->usbss, USBSS_IRQENABLE_CLR_0, USBSS_IRQ_COREIRQ_CLR);
>> +	kdwc3_writel(kdwc->usbss, USBSS_IRQSTATUS_0, USBSS_IRQ_EVENT_ST);
>> +	kdwc3_writel(kdwc->usbss, USBSS_IRQENABLE_SET_0, USBSS_IRQ_COREIRQ_EN);
>> +	kdwc3_writel(kdwc->usbss, USBSS_IRQ_EOI, USBSS_IRQ_EOI_LINE(0));
>> +	spin_unlock(&kdwc->lock);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int kdwc3_probe(struct platform_device *pdev)
>> +{
>> +	struct device_node	*node = pdev->dev.of_node;
>> +	struct device		*dev = &pdev->dev;
> 
> if you invert these lines, it'll look a little nicer:
> 
> 	struct device		*dev = &pdev->dev;
> 	struct device_node	*node = dev->of_node;
> 
> everything else looks pretty good, thanks
> 
Looks good to me as well. With above update, 
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

[PATCH v2 2/5] usb: phy: Add keystone usb phy driver

[Santosh Shilimkar]

On Wednesday 04 December 2013 03:10 PM, WingMan Kwok wrote:
> Add Keystone platform USB PHY driver support. Current main purpose
> of this driver is to enable the PHY reference clock gate on the
> Keystone SoC. Otherwise it is a nop PHY.
> 
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Felipe Balbi <balbi@ti.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
> ---
>  drivers/usb/phy/Kconfig        |   10 +++
>  drivers/usb/phy/Makefile       |    1 +
>  drivers/usb/phy/phy-keystone.c |  142 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 153 insertions(+)
>  create mode 100644 drivers/usb/phy/phy-keystone.c
> 
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

[PATCH v2 3/5] ARM: dts: keystone: Add usb phy devicetree bindings

[Santosh Shilimkar]

On Friday 06 December 2013 03:30 PM, Felipe Balbi wrote:
> On Wed, Dec 04, 2013 at 03:10:09PM -0500, WingMan Kwok wrote:
>> Added device tree support for TI's Keystone USB PHY driver and updated the
>> Documentation with device tree binding information.
>>
>> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
>> Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
>> ---
>>  .../devicetree/bindings/usb/keystone-phy.txt       |   19 +++++++++++++++++++
>>  arch/arm/boot/dts/keystone.dtsi                    |    7 +++++++
>>  2 files changed, 26 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/usb/keystone-phy.txt
>>
>> diff --git a/Documentation/devicetree/bindings/usb/keystone-phy.txt b/Documentation/devicetree/bindings/usb/keystone-phy.txt
>> new file mode 100644
>> index 0000000..300830d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/keystone-phy.txt
>> @@ -0,0 +1,19 @@
>> +TI Keystone USB PHY
>> +
>> +Required properties:
>> + - compatible: should be "ti,keystone-usbphy".
>> + - #address-cells, #size-cells : should be '1' if the device has sub-nodes
>> +   with 'reg' property.
>> + - reg : Address and length of the usb phy control register set.
>> +
>> +The main purpose of this PHY driver is to enable the USB PHY reference clock
>> +gate on the Keystone SOC for both the USB2 and USB3 PHY. Otherwise it is just
>> +an NOP PHY driver.  Hence this node is referenced as both the usb2 and usb3
>> +phy node in the USB Glue layer driver node.
>> +
>> +usb_phy: usb_phy at 2620738 {
>> +	compatible = "ti,keystone-usbphy";
>> +	#address-cells = <1>;
>> +	#size-cells = <1>;
>> +	reg = <0x2620738 32>;
>> +};
>> diff --git a/arch/arm/boot/dts/keystone.dtsi b/arch/arm/boot/dts/keystone.dtsi
>> index f6d6d9e..d497d9e 100644
>> --- a/arch/arm/boot/dts/keystone.dtsi
>> +++ b/arch/arm/boot/dts/keystone.dtsi
>> @@ -181,5 +181,12 @@
>>  			interrupts = <GIC_SPI 300 IRQ_TYPE_EDGE_RISING>;
>>  			clocks = <&clkspi>;
>>  		};
>> +
>> +		usb_phy: usb_phy at 2620738 {
>> +			compatible = "ti,keystone-usbphy";
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +			reg = <0x2620738 32>;
> 
> should this one have status = "disabled"; and let board dts enable the
> PHY ?
> 
Currently there is only one board but probably not a bad idea to enable
it from board dts. Lets do that

Regards,
Santosh 

[PATCH v2 0/5] Kesytone II USB support

[Santosh Shilimkar]

Wingman,

On Wednesday 04 December 2013 03:10 PM, WingMan Kwok wrote:
> Here is the updated version of the series which addresses comments from
> earlier version [1].  The PHY register programming is moved to a separate
> PHY driver.
> 
> Series adds USB host support for Keystone SOCs. Keystone SOCs uses dwc3
> hardware IP implementation.  On Keystone II platforms, we use no-op phy driver.
> 
> All three patches are posted together just for completeness. Only first patch
> is expected to go via usb tree and other two via linux-keystone tree.
>
The series looks fine now and probably can be split now.
 
> Patchset are tested on Keystone II EVM with USB2.0 and USB3.0 flash drives.
> 
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> 
> WingMan Kwok (5):
>   usb: dwc3: Add Keystone specific glue layer
>   usb: phy: Add keystone usb phy driver
Please update the minor comments on patch 1 and resubmit above
two patches so that Felipe can pick these up. 

>   ARM: dts: keystone: Add usb phy devicetree bindings
>   ARM: dts: keystone: Add usb devicetree bindings
>   ARM: keystone: defconfig: enable USB support
> 
On the dts part just enable the phy,usb3 nodes from
board dts file and have status disabled in common dts file.
With those update, please repost them and I will take
them in my 3.14 queue.

Regards,
Santosh

[PATCH v2 1/5] usb: dwc3: Add Keystone specific glue layer

[Paul Zimmerman]

> From: linux-usb-owner at vger.kernel.org [mailto:linux-usb-owner at vger.kernel.org] On Behalf Of Santosh Shilimkar
> Sent: Friday, December 06, 2013 1:40 PM
> 
> On Friday 06 December 2013 03:28 PM, Felipe Balbi wrote:
> > Hi,
> >
> > On Wed, Dec 04, 2013 at 03:10:07PM -0500, WingMan Kwok wrote:
> >> Add Keystone platform specific glue layer to support
> >> USB3 Host mode.
> >>
> >> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> >> Cc: Felipe Balbi <balbi@ti.com>
> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >> Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
> >> ---
> >>  drivers/usb/dwc3/Kconfig         |    7 ++
> >>  drivers/usb/dwc3/Makefile        |    1 +
> >>  drivers/usb/dwc3/dwc3-keystone.c |  210 ++++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 218 insertions(+)
> >>  create mode 100644 drivers/usb/dwc3/dwc3-keystone.c
> 
> [..]
> 
> >> +static irqreturn_t dwc3_keystone_interrupt(int irq, void *_kdwc)
> >> +{
> >> +	struct dwc3_keystone	*kdwc = _kdwc;
> >> +
> >> +	spin_lock(&kdwc->lock);
> >
> > Isn't this lock unnecessary ? This handler will run with IRQs disabled
> > anyway.
> >
> Indeed.

Say what? AFAIK it's necessary to take the driver lock inside the interrupt
handler, because of SMP. Here is the equivalent routine from dwc3-omap.c:

static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap)
{
        struct dwc3_omap        *omap = _omap;
        u32                     reg;

        spin_lock(&omap->lock);

        ...
}

Has this now become unnecessary?

-- 
Paul

[PATCH v2 1/5] usb: dwc3: Add Keystone specific glue layer

[Shilimkar, Santosh]

(sorry for html reply)
________________________________________
From: Paul Zimmerman [Paul.Zimmerman at synopsys.com]
Sent: Friday, December 06, 2013 5:23 PM
To: Shilimkar, Santosh; Balbi, Felipe; Kwok, WingMan
Cc: linux-arm-kernel at lists.infradead.org; linux-usb at vger.kernel.org; Greg Kroah-Hartman
Subject: RE: [PATCH v2 1/5] usb: dwc3: Add Keystone specific glue layer

> From: linux-usb-owner at vger.kernel.org [mailto:linux-usb-owner at vger.kernel.org] On Behalf Of Santosh Shilimkar
> Sent: Friday, December 06, 2013 1:40 PM
>
> On Friday 06 December 2013 03:28 PM, Felipe Balbi wrote:
> > Hi,
> >
> > On Wed, Dec 04, 2013 at 03:10:07PM -0500, WingMan Kwok wrote:
> >> Add Keystone platform specific glue layer to support
> >> USB3 Host mode.
> >>
> >> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> >> Cc: Felipe Balbi <balbi@ti.com>
> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >> Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
> >> ---
> >>  drivers/usb/dwc3/Kconfig         |    7 ++
> >>  drivers/usb/dwc3/Makefile        |    1 +
> >>  drivers/usb/dwc3/dwc3-keystone.c |  210 ++++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 218 insertions(+)
> >>  create mode 100644 drivers/usb/dwc3/dwc3-keystone.c
>
> [..]
>
> >> +static irqreturn_t dwc3_keystone_interrupt(int irq, void *_kdwc)
> >> +{
> >> +  struct dwc3_keystone    *kdwc = _kdwc;
> >> +
> >> +  spin_lock(&kdwc->lock);
> >
> > Isn't this lock unnecessary ? This handler will run with IRQs disabled
> > anyway.
> >
> Indeed.
-------------------------------
Say what? AFAIK it's necessary to take the driver lock inside the interrupt
handler, because of SMP. Here is the equivalent routine from dwc3-omap.c:

static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap)
{
        struct dwc3_omap        *omap = _omap;
        u32                     reg;

        spin_lock(&omap->lock);

        ...--
Paul


}

Has this now become unnecessary?
----------------

[Santosh] Not sure if i am missing your point, but this lock is not
needed IMHO. The register update which is protected by the lock
happens only in ISR where the source of the irq is disabled, so
even on SMP machines there is no race possible which needs to
be protected.

I would have agreed to you if there was some additional code outside
isr  and the code in ISR were both needed lock and that would have been
issue on SMP machine without spin lock.

On  OMAP code as well the lock is not needed but I let Felipe comment
about it. 

Regards,
Santosh

[PATCH v2 1/5] usb: dwc3: Add Keystone specific glue layer

[Paul Zimmerman]

> From: Shilimkar, Santosh [mailto:santosh.shilimkar at ti.com]
> Sent: Friday, December 06, 2013 9:25 PM
> 
>> From: Paul Zimmerman [Paul.Zimmerman at synopsys.com]
>> Sent: Friday, December 06, 2013 5:23 PM
>> 
>> > From: linux-usb-owner at vger.kernel.org [mailto:linux-usb-owner at vger.kernel.org] On Behalf Of Santosh Shilimkar
>> > Sent: Friday, December 06, 2013 1:40 PM
>> >
>> > On Friday 06 December 2013 03:28 PM, Felipe Balbi wrote:
>> > > Hi,
>> > >
>> > > On Wed, Dec 04, 2013 at 03:10:07PM -0500, WingMan Kwok wrote:
>> > >> Add Keystone platform specific glue layer to support
>> > >> USB3 Host mode.
>> > >>
>> > >> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
>> > >> Cc: Felipe Balbi <balbi@ti.com>
>> > >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> > >> Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
>> > >> ---
>> > >>  drivers/usb/dwc3/Kconfig         |    7 ++
>> > >>  drivers/usb/dwc3/Makefile        |    1 +
>> > >>  drivers/usb/dwc3/dwc3-keystone.c |  210 ++++++++++++++++++++++++++++++++++++++
>> > >>  3 files changed, 218 insertions(+)
>> > >>  create mode 100644 drivers/usb/dwc3/dwc3-keystone.c
>> >
>> > [..]
>> >
>> > >> +static irqreturn_t dwc3_keystone_interrupt(int irq, void *_kdwc)
>> > >> +{
>> > >> +  struct dwc3_keystone    *kdwc = _kdwc;
>> > >> +
>> > >> +  spin_lock(&kdwc->lock);
>> > >
>> > > Isn't this lock unnecessary ? This handler will run with IRQs disabled
>> > > anyway.
>> > >
>> > Indeed.
>>
>> Say what? AFAIK it's necessary to take the driver lock inside the interrupt
>> handler, because of SMP. Here is the equivalent routine from dwc3-omap.c:
>> 
>> static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap)
>> {
>>         struct dwc3_omap        *omap = _omap;
>>         u32                     reg;
>> 
>>         spin_lock(&omap->lock);
>> 
>>         ...
>> }
>> 
>> Has this now become unnecessary?
> 
> [Santosh] Not sure if i am missing your point, but this lock is not
> needed IMHO. The register update which is protected by the lock
> happens only in ISR where the source of the irq is disabled, so
> even on SMP machines there is no race possible which needs to
> be protected.
> 
> I would have agreed to you if there was some additional code outside
> isr  and the code in ISR were both needed lock and that would have been
> issue on SMP machine without spin lock.
> 
> On  OMAP code as well the lock is not needed but I let Felipe comment
> about it.

Yes sorry, I see now the lock is only taken in the ISR.

So you should probably get rid of the lock altogether, no?

-- 
Paul

[PATCH v2 1/5] usb: dwc3: Add Keystone specific glue layer

[Felipe Balbi]

On Fri, Dec 06, 2013 at 11:25:24PM -0600, Shilimkar, Santosh wrote:
> (sorry for html reply)
> ________________________________________
> From: Paul Zimmerman [Paul.Zimmerman at synopsys.com]
> Sent: Friday, December 06, 2013 5:23 PM
> To: Shilimkar, Santosh; Balbi, Felipe; Kwok, WingMan
> Cc: linux-arm-kernel at lists.infradead.org; linux-usb at vger.kernel.org; Greg Kroah-Hartman
> Subject: RE: [PATCH v2 1/5] usb: dwc3: Add Keystone specific glue layer
> 
> > From: linux-usb-owner at vger.kernel.org [mailto:linux-usb-owner at vger.kernel.org] On Behalf Of Santosh Shilimkar
> > Sent: Friday, December 06, 2013 1:40 PM
> >
> > On Friday 06 December 2013 03:28 PM, Felipe Balbi wrote:
> > > Hi,
> > >
> > > On Wed, Dec 04, 2013 at 03:10:07PM -0500, WingMan Kwok wrote:
> > >> Add Keystone platform specific glue layer to support
> > >> USB3 Host mode.
> > >>
> > >> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> > >> Cc: Felipe Balbi <balbi@ti.com>
> > >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > >> Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
> > >> ---
> > >>  drivers/usb/dwc3/Kconfig         |    7 ++
> > >>  drivers/usb/dwc3/Makefile        |    1 +
> > >>  drivers/usb/dwc3/dwc3-keystone.c |  210 ++++++++++++++++++++++++++++++++++++++
> > >>  3 files changed, 218 insertions(+)
> > >>  create mode 100644 drivers/usb/dwc3/dwc3-keystone.c
> >
> > [..]
> >
> > >> +static irqreturn_t dwc3_keystone_interrupt(int irq, void *_kdwc)
> > >> +{
> > >> +  struct dwc3_keystone    *kdwc = _kdwc;
> > >> +
> > >> +  spin_lock(&kdwc->lock);
> > >
> > > Isn't this lock unnecessary ? This handler will run with IRQs disabled
> > > anyway.
> > >
> > Indeed.
> -------------------------------
> Say what? AFAIK it's necessary to take the driver lock inside the interrupt
> handler, because of SMP. Here is the equivalent routine from dwc3-omap.c:
> 
> static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap)
> {
>         struct dwc3_omap        *omap = _omap;
>         u32                     reg;
> 
>         spin_lock(&omap->lock);
> 
>         ...--
> Paul
> 
> 
> }
> 
> Has this now become unnecessary?
> ----------------
> 
> [Santosh] Not sure if i am missing your point, but this lock is not
> needed IMHO. The register update which is protected by the lock
> happens only in ISR where the source of the irq is disabled, so
> even on SMP machines there is no race possible which needs to
> be protected.
> 
> I would have agreed to you if there was some additional code outside
> isr  and the code in ISR were both needed lock and that would have been
> issue on SMP machine without spin lock.
> 
> On  OMAP code as well the lock is not needed but I let Felipe comment
> about it.

correct, I'll patch that up for v3.14 :-)

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131207/6584a472/attachment.sig>