Re: [PATCH v3 1/2] usb: dwc3: Add Keystone specific glue layer

2013-12-12 Thread Felipe Balbi 
Hi,

On Tue, Dec 10, 2013 at 10:11:36AM -0500, Santosh Shilimkar wrote:
  +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);
  
  I hope the clock isn't shared between core and wrapper, otherwise you
  could run into some troubles here. Can you confirm ?
  
 Yes. the clock isn't shared. Thanks for taking care of other parts.

so clock for core is always running too ?

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v3 1/2] usb: dwc3: Add Keystone specific glue layer

2013-12-12 Thread Felipe Balbi 
On Thu, Dec 12, 2013 at 02:36:01PM -0500, Santosh Shilimkar wrote:
 On Thursday 12 December 2013 12:48 PM, Felipe Balbi wrote:
  Hi,
  
  On Tue, Dec 10, 2013 at 10:11:36AM -0500, Santosh Shilimkar wrote:
  +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);
 
  I hope the clock isn't shared between core and wrapper, otherwise you
  could run into some troubles here. Can you confirm ?
 
  Yes. the clock isn't shared. Thanks for taking care of other parts.
  
  so clock for core is always running too ?
  
 I take that back. The clock is actually common so we should disable
 it after removing the  kdwc3_remove_core() as you suggested.
 
 You won't see issue since the  kdwc3_remove_core() not doing
 any register access but moving the clock disable after
 the core remove is right thing to do.

the problem is not kdwc3_remove_core() accessing registers, but
dwc3_remove() _does_ access registers during remove. If you just
mopdrobe -r dwc3-keystone without removing dwc3.ko first, then
kdwc3_remove_core() will cause dwc3.ko to be removed (because of
platform_driver_unregister()) and, since clocks have already been
disabled, then we'd die :-)

cheers

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v3 1/2] usb: dwc3: Add Keystone specific glue layer

2013-12-12 Thread Santosh Shilimkar 
On Thursday 12 December 2013 02:41 PM, Felipe Balbi wrote:
 On Thu, Dec 12, 2013 at 02:36:01PM -0500, Santosh Shilimkar wrote:
 On Thursday 12 December 2013 12:48 PM, Felipe Balbi wrote:
 Hi,

 On Tue, Dec 10, 2013 at 10:11:36AM -0500, Santosh Shilimkar wrote:
 +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);

 I hope the clock isn't shared between core and wrapper, otherwise you
 could run into some troubles here. Can you confirm ?

 Yes. the clock isn't shared. Thanks for taking care of other parts.

 so clock for core is always running too ?

 I take that back. The clock is actually common so we should disable
 it after removing the  kdwc3_remove_core() as you suggested.

 You won't see issue since the  kdwc3_remove_core() not doing
 any register access but moving the clock disable after
 the core remove is right thing to do.
 
 the problem is not kdwc3_remove_core() accessing registers, but
 dwc3_remove() _does_ access registers during remove. If you just
 mopdrobe -r dwc3-keystone without removing dwc3.ko first, then
 kdwc3_remove_core() will cause dwc3.ko to be removed (because of
 platform_driver_unregister()) and, since clocks have already been
 disabled, then we'd die :-)
 
Oh yes, you are right. I see Wingman already posted the updated patch.

Regards,
Santosh

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/2] usb: dwc3: Add Keystone specific glue layer

2013-12-10 Thread Santosh Shilimkar 
On Monday 09 December 2013 09:51 PM, Balbi, Felipe wrote:
 Hi,
 
 On Mon, Dec 09, 2013 at 05:17:03PM -0500, WingMan Kwok wrote:
 +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;
 
 this misses the | in |=. I can fix it up while applying, this time only.
 
 +static int kdwc3_probe(struct platform_device *pdev)
 +{
 +struct device   *dev = pdev-dev;
 +struct device_node  *node = pdev-dev.of_node;
 +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);
 +
 +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)) {
 
 clk_get() will never return NULL. This time, I'll fix it while applying.
 
 +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);
 
 I hope the clock isn't shared between core and wrapper, otherwise you
 could run into some troubles here. Can you confirm ?
 
Yes. the clock isn't shared. Thanks for taking care of other parts.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 1/2] usb: dwc3: Add Keystone specific glue layer

2013-12-09 Thread WingMan Kwok 
Add Keystone platform specific glue layer to support
USB3 Host mode.

Cc: Santosh Shilimkar santosh.shilim...@ti.com
Cc: Felipe Balbi ba...@ti.com
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
Acked-by: Santosh Shilimkar santosh.shilim...@ti.com
Signed-off-by: WingMan Kwok w-kw...@ti.com
---
 drivers/usb/dwc3/Kconfig |7 ++
 drivers/usb/dwc3/Makefile|1 +
 drivers/usb/dwc3/dwc3-keystone.c |  205 ++
 3 files changed, 213 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 000..33f71330b
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-keystone.c
@@ -0,0 +1,205 @@
+/**
+ * dwc3-keystone.c - Keystone Specific Glue layer
+ *
+ * Copyright (C) 2010-2013 Texas Instruments Incorporated - http://www.ti.com
+ *
+ * Author: WingMan Kwok w-kw...@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/platform_device.h
+#include linux/dma-mapping.h
+#include linux/io.h
+#include linux/of_platform.h
+
+/* USBSS register offsets */
+#define USBSS_REVISION 0x
+#define USBSS_SYSCONFIG0x0010
+#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 {
+   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;
+
+   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));
+
+   return IRQ_HANDLED;
+}
+
+static int kdwc3_probe(struct platform_device *pdev)
+{
+   struct device   *dev = pdev-dev;
+   struct device_node  *node = pdev-dev.of_node;
+   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);
+
+   kdwc-dev = dev;
+
+   res =

Re: [PATCH v3 1/2] usb: dwc3: Add Keystone specific glue layer

2013-12-09 Thread Felipe Balbi 
Hi,

On Mon, Dec 09, 2013 at 05:17:03PM -0500, WingMan Kwok wrote:
 +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;

this misses the | in |=. I can fix it up while applying, this time only.

 +static int kdwc3_probe(struct platform_device *pdev)
 +{
 + struct device   *dev = pdev-dev;
 + struct device_node  *node = pdev-dev.of_node;
 + 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);
 +
 + 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)) {

clk_get() will never return NULL. This time, I'll fix it while applying.

 +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);

I hope the clock isn't shared between core and wrapper, otherwise you
could run into some troubles here. Can you confirm ?

-- 
balbi


signature.asc
Description: Digital signature