[PATCH 0/4] usb: musb: da8xx: Add DMA support

[PATCH 0/4] usb: musb: da8xx: Add DMA support

[Alexandre Bailon]

This series update musb driver to add DMA support to da8xx.
To work correctly, the follwings patches are required:
"dmaengine: cppi41: Add dma support to da8xx",
"ARM: davinci: da850: Share the usb20 clock",
"ARM: dts: da850: Enable cppi41 dma",
"dmaengine: cppi41: PM runtime fixes"

I haven't changed yet the davinci_all_defconfig to enable the DMA mode
by default. I'm planning to do it once all the warnings happening during
the teardwon will be fixed.

The series has been tested on beaglebone black and omapl138-lcdk
in both host mode (pendrive, headset, wifi, webcam) and device mode
(MSC, CDC).
Expect few warnings on omapl138-lcdk, I haven't noticed any issues.

Alexandre Bailon (4):
  usb: musb: da8xx: Use the right dma controller
  usb: musb: Add support of cppi41 dma controller for da8xx to Kconfig
  usb: musb: Use shared irq
  usb: musb: musb_cppi41: Workaround dma stall issue during teardown

 drivers/dma/Kconfig            | 2 +-
 drivers/usb/musb/Kconfig       | 4 ++--
 drivers/usb/musb/da8xx.c       | 8 ++++----
 drivers/usb/musb/musb_core.c   | 2 +-
 drivers/usb/musb/musb_core.h   | 1 +
 drivers/usb/musb/musb_cppi41.c | 4 ++++
 6 files changed, 13 insertions(+), 8 deletions(-)

-- 
2.10.2

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

[PATCH 1/4] usb: musb: da8xx: Use the right dma controller

[Alexandre Bailon]

da8xx driver is registering and using the cppi dma controller but actually,
the da8xx has a cppi41 dma controller.
Update the driver to register and use the cppi41 dma controller.

Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 drivers/usb/musb/da8xx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index e89708d..74dcc07 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -458,14 +458,14 @@ static inline u8 get_vbus_power(struct device *dev)
 }
 
 static const struct musb_platform_ops da8xx_ops = {
-	.quirks		= MUSB_DMA_CPPI | MUSB_INDEXED_EP,
+	.quirks		= MUSB_DMA_CPPI41 | MUSB_INDEXED_EP,
 	.init		= da8xx_musb_init,
 	.exit		= da8xx_musb_exit,
 
 	.fifo_mode	= 2,
-#ifdef CONFIG_USB_TI_CPPI_DMA
-	.dma_init	= cppi_dma_controller_create,
-	.dma_exit	= cppi_dma_controller_destroy,
+#ifdef CONFIG_USB_TI_CPPI41_DMA
+	.dma_init	= cppi41_dma_controller_create,
+	.dma_exit	= cppi41_dma_controller_destroy,
 #endif
 	.enable		= da8xx_musb_enable,
 	.disable	= da8xx_musb_disable,
-- 
2.10.2

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

Re: [PATCH 1/4] usb: musb: da8xx: Use the right dma controller

[Sergei Shtylyov]

Hello!

On 01/09/2017 07:38 PM, Alexandre Bailon wrote:

> da8xx driver is registering and using the cppi dma controller but actually,
> the da8xx has a cppi41 dma controller.
> Update the driver to register and use the cppi41 dma controller.
>
> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/usb/musb/da8xx.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
> index e89708d..74dcc07 100644
> --- a/drivers/usb/musb/da8xx.c
> +++ b/drivers/usb/musb/da8xx.c
> @@ -458,14 +458,14 @@ static inline u8 get_vbus_power(struct device *dev)
>  }
>
>  static const struct musb_platform_ops da8xx_ops = {
> -	.quirks		= MUSB_DMA_CPPI | MUSB_INDEXED_EP,

    Grr, MUSB_DMA_CPPI is certainly wrong! There's no CPPI 3.0 support for 
DA8xx. Where have this come from?

> +	.quirks		= MUSB_DMA_CPPI41 | MUSB_INDEXED_EP,
>  	.init		= da8xx_musb_init,
>  	.exit		= da8xx_musb_exit,
>
>  	.fifo_mode	= 2,
> -#ifdef CONFIG_USB_TI_CPPI_DMA
> -	.dma_init	= cppi_dma_controller_create,
> -	.dma_exit	= cppi_dma_controller_destroy,

    Likewise, WTF?
    I suggest that you fix this crap in a separate commit, to be backported to 
-stable if needed.

[...]

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] usb: musb: da8xx: Use the right dma controller

[Tony Lindgren]

* Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org> [170109 10:30]:
> Hello!
> 
> On 01/09/2017 07:38 PM, Alexandre Bailon wrote:
> 
> > da8xx driver is registering and using the cppi dma controller but actually,
> > the da8xx has a cppi41 dma controller.
> > Update the driver to register and use the cppi41 dma controller.
> > 
> > Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> > ---
> >  drivers/usb/musb/da8xx.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
> > index e89708d..74dcc07 100644
> > --- a/drivers/usb/musb/da8xx.c
> > +++ b/drivers/usb/musb/da8xx.c
> > @@ -458,14 +458,14 @@ static inline u8 get_vbus_power(struct device *dev)
> >  }
> > 
> >  static const struct musb_platform_ops da8xx_ops = {
> > -	.quirks		= MUSB_DMA_CPPI | MUSB_INDEXED_EP,
> 
>    Grr, MUSB_DMA_CPPI is certainly wrong! There's no CPPI 3.0 support for
> DA8xx. Where have this come from?
> 
> > +	.quirks		= MUSB_DMA_CPPI41 | MUSB_INDEXED_EP,
> >  	.init		= da8xx_musb_init,
> >  	.exit		= da8xx_musb_exit,
> > 
> >  	.fifo_mode	= 2,
> > -#ifdef CONFIG_USB_TI_CPPI_DMA
> > -	.dma_init	= cppi_dma_controller_create,
> > -	.dma_exit	= cppi_dma_controller_destroy,
> 
>    Likewise, WTF?
>    I suggest that you fix this crap in a separate commit, to be backported
> to -stable if needed.

Probably came from my DMA patch 7f6283ed6fe8 ("usb: musb: Set up function
pointers for DMA"). At that point I thought da8xxx.c and davinci.c are just
separate attempts of the same driver.. Sorry the number of musb glue layers
is just too confusing.

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

Re: [PATCH 1/4] usb: musb: da8xx: Use the right dma controller

[Sergei Shtylyov]

On 01/09/2017 09:43 PM, Tony Lindgren wrote:

>>> da8xx driver is registering and using the cppi dma controller but actually,
>>> the da8xx has a cppi41 dma controller.
>>> Update the driver to register and use the cppi41 dma controller.
>>>
>>> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>>> ---
>>>  drivers/usb/musb/da8xx.c | 8 ++++----
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
>>> index e89708d..74dcc07 100644
>>> --- a/drivers/usb/musb/da8xx.c
>>> +++ b/drivers/usb/musb/da8xx.c
>>> @@ -458,14 +458,14 @@ static inline u8 get_vbus_power(struct device *dev)
>>>  }
>>>
>>>  static const struct musb_platform_ops da8xx_ops = {
>>> -	.quirks		= MUSB_DMA_CPPI | MUSB_INDEXED_EP,
>>
>>    Grr, MUSB_DMA_CPPI is certainly wrong! There's no CPPI 3.0 support for
>> DA8xx. Where have this come from?
>>
>>> +	.quirks		= MUSB_DMA_CPPI41 | MUSB_INDEXED_EP,
>>>  	.init		= da8xx_musb_init,
>>>  	.exit		= da8xx_musb_exit,
>>>
>>>  	.fifo_mode	= 2,
>>> -#ifdef CONFIG_USB_TI_CPPI_DMA
>>> -	.dma_init	= cppi_dma_controller_create,
>>> -	.dma_exit	= cppi_dma_controller_destroy,
>>
>>    Likewise, WTF?
>>    I suggest that you fix this crap in a separate commit, to be backported
>> to -stable if needed.
>
> Probably came from my DMA patch 7f6283ed6fe8 ("usb: musb: Set up function

    Too bad I missed it back then. Then certainly this should be fixed in a 
separate patch. The first hunk was not a part of that commit, tho...

> pointers for DMA"). At that point I thought da8xxx.c and davinci.c are just
> separate attempts of the same driver.. Sorry the number of musb glue layers
> is just too confusing.

    :-)

> Tony

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] usb: musb: da8xx: Use the right dma controller

[Tony Lindgren]

* Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org> [170109 10:55]:
> On 01/09/2017 09:43 PM, Tony Lindgren wrote:
> 
> > > > da8xx driver is registering and using the cppi dma controller but actually,
> > > > the da8xx has a cppi41 dma controller.
> > > > Update the driver to register and use the cppi41 dma controller.
> > > > 
> > > > Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> > > > ---
> > > >  drivers/usb/musb/da8xx.c | 8 ++++----
> > > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
> > > > index e89708d..74dcc07 100644
> > > > --- a/drivers/usb/musb/da8xx.c
> > > > +++ b/drivers/usb/musb/da8xx.c
> > > > @@ -458,14 +458,14 @@ static inline u8 get_vbus_power(struct device *dev)
> > > >  }
> > > > 
> > > >  static const struct musb_platform_ops da8xx_ops = {
> > > > -	.quirks		= MUSB_DMA_CPPI | MUSB_INDEXED_EP,
> > > 
> > >    Grr, MUSB_DMA_CPPI is certainly wrong! There's no CPPI 3.0 support for
> > > DA8xx. Where have this come from?
> > > 
> > > > +	.quirks		= MUSB_DMA_CPPI41 | MUSB_INDEXED_EP,
> > > >  	.init		= da8xx_musb_init,
> > > >  	.exit		= da8xx_musb_exit,
> > > > 
> > > >  	.fifo_mode	= 2,
> > > > -#ifdef CONFIG_USB_TI_CPPI_DMA
> > > > -	.dma_init	= cppi_dma_controller_create,
> > > > -	.dma_exit	= cppi_dma_controller_destroy,
> > > 
> > >    Likewise, WTF?
> > >    I suggest that you fix this crap in a separate commit, to be backported
> > > to -stable if needed.
> > 
> > Probably came from my DMA patch 7f6283ed6fe8 ("usb: musb: Set up function
> 
>    Too bad I missed it back then. Then certainly this should be fixed in a
> separate patch. The first hunk was not a part of that commit, tho...
> 
> > pointers for DMA"). At that point I thought da8xxx.c and davinci.c are just
> > separate attempts of the same driver.. Sorry the number of musb glue layers
> > is just too confusing.

Maybe from f8e9f34f80a2 ("usb: musb: Fix up DMA related macros")?

Sorry obviously I have no idea how the da8xx.c should be configured for DMA.

Regards,

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

[PATCH 2/4] usb: musb: Add support of cppi41 dma controller for da8xx to Kconfig

[Alexandre Bailon]

Currently, cppi41 driver can only be built for omap platform.
As da8xx platform has a cppi41 dma controller, update the Kconfig
to also support the da8xx.

Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 drivers/dma/Kconfig      | 2 +-
 drivers/usb/musb/Kconfig | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 0d6a96e..8fe9de7 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -515,7 +515,7 @@ config TIMB_DMA
 
 config TI_CPPI41
 	tristate "AM33xx CPPI41 DMA support"
-	depends on ARCH_OMAP
+	depends on (ARCH_OMAP || ARCH_DAVINCI_DA8XX)
 	select DMA_ENGINE
 	help
 	  The Communications Port Programming Interface (CPPI) 4.1 DMA engine
diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
index 72a2a50..3018518 100644
--- a/drivers/usb/musb/Kconfig
+++ b/drivers/usb/musb/Kconfig
@@ -160,8 +160,8 @@ config USB_TI_CPPI_DMA
 	  Enable DMA transfers when TI CPPI DMA is available.
 
 config USB_TI_CPPI41_DMA
-	bool 'TI CPPI 4.1 (AM335x)'
-	depends on ARCH_OMAP && DMADEVICES
+	bool 'TI CPPI 4.1 (AM335x or DA8xx)'
+	depends on (ARCH_OMAP || ARCH_DAVINCI_DA8XX) && DMADEVICES
 	select TI_CPPI41
 
 config USB_TUSB_OMAP_DMA
-- 
2.10.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/4] usb: musb: Use shared irq

[Alexandre Bailon]

In the da8xx, usb core and cppi41 are sharing the same interrupt line.
Update the driver to request a shared irq.

Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 drivers/usb/musb/musb_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index fca288bb..cf40adf 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -2329,7 +2329,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
 	setup_timer(&musb->otg_timer, musb_otg_timer_func, (unsigned long) musb);
 
 	/* attach to the IRQ */
-	if (request_irq(nIrq, musb->isr, 0, dev_name(dev), musb)) {
+	if (request_irq(nIrq, musb->isr, IRQF_SHARED, dev_name(dev), musb)) {
 		dev_err(dev, "request_irq %d failed!\n", nIrq);
 		status = -ENODEV;
 		goto fail3;
-- 
2.10.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/4] usb: musb: musb_cppi41: Workaround dma stall issue during teardown

[Alexandre Bailon]

The dma may hung up if a teardown is initiated while an endpoint is still
active (Advisory 2.3.27 of da8xx errata).
To workaround this issue, add a delay before to initiate the teardown.

Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 drivers/usb/musb/da8xx.c       | 2 +-
 drivers/usb/musb/musb_core.h   | 1 +
 drivers/usb/musb/musb_cppi41.c | 4 ++++
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index 74dcc07..3513284 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -458,7 +458,7 @@ static inline u8 get_vbus_power(struct device *dev)
 }
 
 static const struct musb_platform_ops da8xx_ops = {
-	.quirks		= MUSB_DMA_CPPI41 | MUSB_INDEXED_EP,
+	.quirks		= MUSB_DMA_CPPI41 | MUSB_INDEXED_EP | MUSB_DA8XX,
 	.init		= da8xx_musb_init,
 	.exit		= da8xx_musb_exit,
 
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index ade902e..d129278 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -172,6 +172,7 @@ struct musb_io;
  */
 struct musb_platform_ops {
 
+#define MUSB_DA8XX		BIT(7)
 #define MUSB_DMA_UX500		BIT(6)
 #define MUSB_DMA_CPPI41		BIT(5)
 #define MUSB_DMA_CPPI		BIT(4)
diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index 1636385..8fdbc17 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -547,6 +547,10 @@ static int cppi41_dma_channel_abort(struct dma_channel *channel)
 		}
 	}
 
+	/* DA8xx Advisory 2.3.27: wait 250 ms before to start the teardown */
+	if (musb->io.quirks & MUSB_DA8XX)
+		mdelay(250);
+
 	tdbit = 1 << cppi41_channel->port_num;
 	if (is_tx)
 		tdbit <<= 16;
-- 
2.10.2

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

Re: [PATCH 4/4] usb: musb: musb_cppi41: Workaround dma stall issue during teardown

[Sergei Shtylyov]

Hello!

On 01/09/2017 07:39 PM, Alexandre Bailon wrote:

> The dma may hung up if a teardown is initiated while an endpoint is still

    The DMA may hang up...

> active (Advisory 2.3.27 of da8xx errata).
> To workaround this issue, add a delay before to initiate the teardown.
>
> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
[...]
> diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
> index 1636385..8fdbc17 100644
> --- a/drivers/usb/musb/musb_cppi41.c
> +++ b/drivers/usb/musb/musb_cppi41.c
> @@ -547,6 +547,10 @@ static int cppi41_dma_channel_abort(struct dma_channel *channel)
>  		}
>  	}
>
> +	/* DA8xx Advisory 2.3.27: wait 250 ms before to start the teardown */
> +	if (musb->io.quirks & MUSB_DA8XX)
> +		mdelay(250);

    This is a horrible workaround, not even msleep()...

[...]

MBR, Sergei

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

Re: [PATCH 4/4] usb: musb: musb_cppi41: Workaround dma stall issue during teardown

[Alexandre Bailon]

On 01/09/2017 07:34 PM, Sergei Shtylyov wrote:
> Hello!
> 
> On 01/09/2017 07:39 PM, Alexandre Bailon wrote:
> 
>> The dma may hung up if a teardown is initiated while an endpoint is still
> 
>    The DMA may hang up...
> 
>> active (Advisory 2.3.27 of da8xx errata).
>> To workaround this issue, add a delay before to initiate the teardown.
>>
>> Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> [...]
>> diff --git a/drivers/usb/musb/musb_cppi41.c
>> b/drivers/usb/musb/musb_cppi41.c
>> index 1636385..8fdbc17 100644
>> --- a/drivers/usb/musb/musb_cppi41.c
>> +++ b/drivers/usb/musb/musb_cppi41.c
>> @@ -547,6 +547,10 @@ static int cppi41_dma_channel_abort(struct
>> dma_channel *channel)
>>          }
>>      }
>>
>> +    /* DA8xx Advisory 2.3.27: wait 250 ms before to start the
>> teardown */
>> +    if (musb->io.quirks & MUSB_DA8XX)
>> +        mdelay(250);
> 
>    This is a horrible workaround, not even msleep()...
Indeed, that's a bad workaround but have not found a better one.
cppi41_dma_channel_abort() is called from atomic context,
so we can't use msleep().
And update the musb driver to call cppi41_dma_channel_abort()
from process context would require too many changes.

Best Regards,
Alexandre
> 
> [...]
> 
> MBR, Sergei
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html