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-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 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.

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-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-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

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

[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 
Cc: Felipe Balbi 
Cc: Greg Kroah-Hartman 
Acked-by: Santosh Shilimkar 
Signed-off-by: WingMan Kwok 
---
 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 
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* 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 = 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);
+