[PATCH 1/2] usb: dwc2: Option to disable dma descriptor mode

[PATCH 1/2] usb: dwc2: Option to disable dma descriptor mode

[dinguyen-EIB2kfCEclfQT0dZR+AlfA]

From: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>

Even though the IP supports Descriptor DMA mode, it does not support SPLIT
transactions in this mode. Since the driver can get the Descriptor DMA mode
support from hardware, the driver in its currently form cannot fully support
LS/FS devices connected to a HS Hub when Descriptor DMA mode is enabled in the
driver. Thus, we give the option to disable Descriptor DMA from device tree.

Signed-off-by: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
---
 drivers/usb/dwc2/platform.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index eaba547..cb2133c 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -123,6 +123,7 @@ static int dwc2_driver_probe(struct platform_device *dev)
 	struct resource *res;
 	int retval;
 	int irq;
+	u32 prop;
 
 	if (usb_disabled())
 		return -ENODEV;
@@ -135,6 +136,8 @@ static int dwc2_driver_probe(struct platform_device *dev)
 		dwc2_set_all_params(&defparams, -1);
 		params = &defparams;
 	}
+	if (!of_property_read_u32(dev->dev.of_node, "dma-desc-enable", &prop))
+		defparams.dma_desc_enable = prop;
 
 	hsotg = devm_kzalloc(&dev->dev, sizeof(*hsotg), GFP_KERNEL);
 	if (!hsotg)
-- 
1.7.9.5

--
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 2/2] Documentation: dt: add an optional dma-desc-enable property for dwc2 usb

[dinguyen-EIB2kfCEclfQT0dZR+AlfA]

From: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>

The "dma-desc-enable" property can be used to disable descriptor DMA mode
for the DWC2 USB IP.

Signed-off-by: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
---
 Documentation/devicetree/bindings/usb/dwc2.txt |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index b8b6871..37af3bc 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -14,6 +14,11 @@ Refer to clk/clock-bindings.txt for generic clock consumer properties
 Optional properties:
 - phys: phy provider specifier
 - phy-names: shall be "device"
+- dma-desc-enable: This property is used to disable descriptor DMA support.
+	The reason why this property is needed is that even if the HW supports
+	description DMA support, it does not support it for SPLIT transactions.
+	So this property can be used to optionally disable descriptor DMA
+	support.
 Refer to phy/phy-bindings.txt for generic phy consumer properties
 
 Example:
-- 
1.7.9.5

--
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/2] usb: dwc2: Option to disable dma descriptor mode

[Sergei Shtylyov]

Hello.

On 05/06/2014 02:23 AM, dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org wrote:

> From: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>

> Even though the IP supports Descriptor DMA mode, it does not support SPLIT
> transactions in this mode. Since the driver can get the Descriptor DMA mode
> support from hardware, the driver in its currently form cannot fully support
> LS/FS devices connected to a HS Hub when Descriptor DMA mode is enabled in the
> driver. Thus, we give the option to disable Descriptor DMA from device tree.

> Signed-off-by: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
> ---
>   drivers/usb/dwc2/platform.c |    3 +++
>   1 file changed, 3 insertions(+)

> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> index eaba547..cb2133c 100644
> --- a/drivers/usb/dwc2/platform.c
> +++ b/drivers/usb/dwc2/platform.c
> @@ -123,6 +123,7 @@ static int dwc2_driver_probe(struct platform_device *dev)
>   	struct resource *res;
>   	int retval;
>   	int irq;
> +	u32 prop;
>
>   	if (usb_disabled())
>   		return -ENODEV;
> @@ -135,6 +136,8 @@ static int dwc2_driver_probe(struct platform_device *dev)
>   		dwc2_set_all_params(&defparams, -1);
>   		params = &defparams;
>   	}
> +	if (!of_property_read_u32(dev->dev.of_node, "dma-desc-enable", &prop))
> +		defparams.dma_desc_enable = prop;

    You don't really need a helper local variable here, the variable receiving 
the property value won't be changed if there was an error retrieving it.

WBR, 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 1/2] usb: dwc2: Option to disable dma descriptor mode

[Dinh Nguyen]

On Tue, 2014-05-06 at 19:39 +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 05/06/2014 02:23 AM, dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org wrote:
> 
> > From: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
> 
> > Even though the IP supports Descriptor DMA mode, it does not support SPLIT
> > transactions in this mode. Since the driver can get the Descriptor DMA mode
> > support from hardware, the driver in its currently form cannot fully support
> > LS/FS devices connected to a HS Hub when Descriptor DMA mode is enabled in the
> > driver. Thus, we give the option to disable Descriptor DMA from device tree.
> 
> > Signed-off-by: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
> > ---
> >   drivers/usb/dwc2/platform.c |    3 +++
> >   1 file changed, 3 insertions(+)
> 
> > diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> > index eaba547..cb2133c 100644
> > --- a/drivers/usb/dwc2/platform.c
> > +++ b/drivers/usb/dwc2/platform.c
> > @@ -123,6 +123,7 @@ static int dwc2_driver_probe(struct platform_device *dev)
> >   	struct resource *res;
> >   	int retval;
> >   	int irq;
> > +	u32 prop;
> >
> >   	if (usb_disabled())
> >   		return -ENODEV;
> > @@ -135,6 +136,8 @@ static int dwc2_driver_probe(struct platform_device *dev)
> >   		dwc2_set_all_params(&defparams, -1);
> >   		params = &defparams;
> >   	}
> > +	if (!of_property_read_u32(dev->dev.of_node, "dma-desc-enable", &prop))
> > +		defparams.dma_desc_enable = prop;
> 
>     You don't really need a helper local variable here, the variable receiving 
> the property value won't be changed if there was an error retrieving it.
> 

Ah yes, you're right. Thanks for the review.

Dinh
> WBR, 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/2] usb: dwc2: Option to disable dma descriptor mode

[Paul Zimmerman]

> From: dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org [mailto:dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org]
> Sent: Monday, May 05, 2014 3:23 PM
> 
> Even though the IP supports Descriptor DMA mode, it does not support SPLIT
> transactions in this mode. Since the driver can get the Descriptor DMA mode
> support from hardware, the driver in its currently form cannot fully support
> LS/FS devices connected to a HS Hub when Descriptor DMA mode is enabled in the
> driver. Thus, we give the option to disable Descriptor DMA from device tree.

Hi Dinh,

Instead of this patch, how about if we make the driver default to buffer
DMA mode instead, and if anyone wants to use descriptor DMA mode, they
can add a DT property for that? I don't think anyone will want that,
since as you say it doesn't fully support LS/FS devices in that mode.

The driver originally worked like that. I guess the default got changed by
one of Matthijs' patches, and I didn't notice it.

-- 
Paul

--
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/2] usb: dwc2: Option to disable dma descriptor mode

[Felipe Balbi]

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

On Tue, May 06, 2014 at 06:21:02PM +0000, Paul Zimmerman wrote:
> > From: dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org [mailto:dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org]
> > Sent: Monday, May 05, 2014 3:23 PM
> > 
> > Even though the IP supports Descriptor DMA mode, it does not support SPLIT
> > transactions in this mode. Since the driver can get the Descriptor DMA mode
> > support from hardware, the driver in its currently form cannot fully support
> > LS/FS devices connected to a HS Hub when Descriptor DMA mode is enabled in the
> > driver. Thus, we give the option to disable Descriptor DMA from device tree.
> 
> Hi Dinh,
> 
> Instead of this patch, how about if we make the driver default to buffer
> DMA mode instead, and if anyone wants to use descriptor DMA mode, they
> can add a DT property for that? I don't think anyone will want that,
> since as you say it doesn't fully support LS/FS devices in that mode.
> 
> The driver originally worked like that. I guess the default got changed by
> one of Matthijs' patches, and I didn't notice it.

if this is the case, I guess you could call it a regression - well, it
_did_ break LS/FS use cases - and still merge it during the -rc cycle.

cheers

-- 
balbi

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

RE: [PATCH 1/2] usb: dwc2: Option to disable dma descriptor mode

[Dinh Nguyen]

On Tue, 2014-05-06 at 18:21 +0000, Paul Zimmerman wrote:
> > From: dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org [mailto:dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org]
> > Sent: Monday, May 05, 2014 3:23 PM
> > 
> > Even though the IP supports Descriptor DMA mode, it does not support SPLIT
> > transactions in this mode. Since the driver can get the Descriptor DMA mode
> > support from hardware, the driver in its currently form cannot fully support
> > LS/FS devices connected to a HS Hub when Descriptor DMA mode is enabled in the
> > driver. Thus, we give the option to disable Descriptor DMA from device tree.
> 
> Hi Dinh,
> 
> Instead of this patch, how about if we make the driver default to buffer
> DMA mode instead, and if anyone wants to use descriptor DMA mode, they
> can add a DT property for that? I don't think anyone will want that,
> since as you say it doesn't fully support LS/FS devices in that mode.

Yes, I think that's a good idea. Let me spin up a patchset to do that.

Dinh
> 
> The driver originally worked like that. I guess the default got changed by
> one of Matthijs' patches, and I didn't notice it.
> 


--
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/2] usb: dwc2: Option to disable dma descriptor mode

[Paul Zimmerman]

> From: Felipe Balbi [mailto:balbi-l0cyMroinI0@public.gmane.org]
> Sent: Tuesday, May 06, 2014 11:37 AM
> 
> On Tue, May 06, 2014 at 06:21:02PM +0000, Paul Zimmerman wrote:
> > > From: dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org [mailto:dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org]
> > > Sent: Monday, May 05, 2014 3:23 PM
> > >
> > > Even though the IP supports Descriptor DMA mode, it does not support SPLIT
> > > transactions in this mode. Since the driver can get the Descriptor DMA mode
> > > support from hardware, the driver in its currently form cannot fully support
> > > LS/FS devices connected to a HS Hub when Descriptor DMA mode is enabled in the
> > > driver. Thus, we give the option to disable Descriptor DMA from device tree.
> >
> > Hi Dinh,
> >
> > Instead of this patch, how about if we make the driver default to buffer
> > DMA mode instead, and if anyone wants to use descriptor DMA mode, they
> > can add a DT property for that? I don't think anyone will want that,
> > since as you say it doesn't fully support LS/FS devices in that mode.
> >
> > The driver originally worked like that. I guess the default got changed by
> > one of Matthijs' patches, and I didn't notice it.
> 
> if this is the case, I guess you could call it a regression - well, it
> _did_ break LS/FS use cases - and still merge it during the -rc cycle.

Well, the change from Matthijs went in on August 30th last year. So it's
not exactly a recent regression ;)

-- 
Paul

--
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/2] usb: dwc2: Option to disable dma descriptor mode

[Felipe Balbi]

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

On Tue, May 06, 2014 at 08:11:34PM +0000, Paul Zimmerman wrote:
> > From: Felipe Balbi [mailto:balbi-l0cyMroinI0@public.gmane.org]
> > Sent: Tuesday, May 06, 2014 11:37 AM
> > 
> > On Tue, May 06, 2014 at 06:21:02PM +0000, Paul Zimmerman wrote:
> > > > From: dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org [mailto:dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org]
> > > > Sent: Monday, May 05, 2014 3:23 PM
> > > >
> > > > Even though the IP supports Descriptor DMA mode, it does not support SPLIT
> > > > transactions in this mode. Since the driver can get the Descriptor DMA mode
> > > > support from hardware, the driver in its currently form cannot fully support
> > > > LS/FS devices connected to a HS Hub when Descriptor DMA mode is enabled in the
> > > > driver. Thus, we give the option to disable Descriptor DMA from device tree.
> > >
> > > Hi Dinh,
> > >
> > > Instead of this patch, how about if we make the driver default to buffer
> > > DMA mode instead, and if anyone wants to use descriptor DMA mode, they
> > > can add a DT property for that? I don't think anyone will want that,
> > > since as you say it doesn't fully support LS/FS devices in that mode.
> > >
> > > The driver originally worked like that. I guess the default got changed by
> > > one of Matthijs' patches, and I didn't notice it.
> > 
> > if this is the case, I guess you could call it a regression - well, it
> > _did_ break LS/FS use cases - and still merge it during the -rc cycle.
> 
> Well, the change from Matthijs went in on August 30th last year. So it's
> not exactly a recent regression ;)

damn :-) alright, then merge window possibly with a stable tag, so it
gets backported ?

cheers

-- 
balbi

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