[PATCH 00/30] usb: dwc2: Gadget descriptor DMA and IOT

[PATCH 00/30] usb: dwc2: Gadget descriptor DMA and IOT

[John Youn]

This series implements gadget-side descriptor DMA for the DWC_hsotg
controller.

It also includes support for DWC USB IOT controllers which use the
descriptor DMA mode of operation exclusively. These are two new
device-only USB controller IPs based on DWC_hsotg.

Tested on HAPS platform with:
* HSOTG IP version 3.30a
* FS/LS IOT IP version 1.00a
* HS IOT IP version 1.00a

This series should be applied on top of:
http://marc.info/?l=linux-usb&m=147822095118860&w=2

Regards,
John


John Youn (2):
  usb: dwc2: Deprecate g-use-dma binding
  usb: dwc2: Enable gadget DDMA by default for HAPS

Vahram Aharonyan (25):
  usb: dwc2: Update DMA descriptor structure
  usb: dwc2: gadget: Add descriptor DMA binding
  usb: dwc2: gadget: Add DMA descriptor status quadlet fields
  usb: dwc2: gadget: Enable BNA interrupt in descriptor DMA mode
  usb: dwc2: gadget: Add DMA descriptor chains for EP 0
  usb: dwc2: host: Rename MAX_DMA_DESC_SIZE to HOST_DMA_NBYTES_LIMIT
  usb: dwc2: gadget: Transfer length limit checking for DDMA
  usb: dwc2: gadget: Add DDMA chain pointers to dwc2_hsotg_ep structure
  usb: dwc2: gadget: Add DDMA chain fill and parse functions
  usb: dwc2: gadget: EP 0 specific DDMA programming
  usb: dwc2: gadget: DDMA transfer start and complete
  usb: dwc2: gadget: Fixes for StsPhseRcvd interrupt
  usb: dwc2: gadget: Start DDMA IN status phase in StsPhseRcvd handler
  usb: dwc2: gadget: Enable descriptor DMA mode
  usb: dwc2: gadget: Add DDMA isoc related fields to dwc2_hsotg_ep
  usb: dwc2: gadget: Fill isoc descriptor and start transfer in DDMA
  usb: dwc2: gadget: Add completions for DDMA isoc transfers
  usb: dwc2: gadget: In DDMA keep incompISOOUT and incompISOIN masked
  usb: dwc2: gadget: Add start and complete calls for DDMA ISOC
  usb: dwc2: gadget: Adjust ISOC OUT request's actual len for DDMA
  usb: dwc2: gadget: Adjustments in debug prints
  usb: dwc2: gadget: For DDMA parse setup only after SetUp interrupt
  usb: dwc2: gadget: Correct dwc2_hsotg_ep_stop_xfr() function
  usb: dwc2: gadget: Disable enabled HW endpoint in
    dwc2_hsotg_ep_disable
  usb: dwc2: Add support of dedicated full-speed PHY interface

Vardan Mikayelyan (3):
  usb: dwc2: gadget: Add IOT device IDs, configure core accordingly
  usb: dwc2: gadget: Program ep0_mps for LS
  usb: dwc2: gadget: Add new core parameter for low speed

 Documentation/devicetree/bindings/usb/dwc2.txt |   6 +-
 arch/arm/boot/dts/rk3036.dtsi                  |   2 +-
 arch/arm/boot/dts/rk3288.dtsi                  |   2 +-
 arch/arm/boot/dts/rk3xxx.dtsi                  |   2 +-
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi      |   2 +-
 arch/arm64/boot/dts/rockchip/rk3368.dtsi       |   2 +-
 drivers/usb/dwc2/core.h                        |  48 ++
 drivers/usb/dwc2/gadget.c                      | 978 ++++++++++++++++++++++---
 drivers/usb/dwc2/hcd.c                         |  12 +-
 drivers/usb/dwc2/hcd.h                         |   2 +-
 drivers/usb/dwc2/hcd_ddma.c                    |  52 +-
 drivers/usb/dwc2/hw.h                          |  48 +-
 drivers/usb/dwc2/params.c                      |  42 +-
 drivers/usb/dwc2/pci.c                         |   3 +-
 14 files changed, 1039 insertions(+), 162 deletions(-)

-- 
2.10.0

[PATCH 01/30] usb: dwc2: Deprecate g-use-dma binding

[John Youn]

Add a vendor prefix and make the name more consistent by renaming it to
"snps,gadget-dma-enable".

Signed-off-by: John Youn <johnyoun@synopsys.com>
---
 Documentation/devicetree/bindings/usb/dwc2.txt | 5 ++++-
 arch/arm/boot/dts/rk3036.dtsi                  | 2 +-
 arch/arm/boot/dts/rk3288.dtsi                  | 2 +-
 arch/arm/boot/dts/rk3xxx.dtsi                  | 2 +-
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi      | 2 +-
 arch/arm64/boot/dts/rockchip/rk3368.dtsi       | 2 +-
 drivers/usb/dwc2/params.c                      | 9 ++++++++-
 drivers/usb/dwc2/pci.c                         | 2 +-
 8 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index 9472111..389a461 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -26,11 +26,14 @@ Refer to phy/phy-bindings.txt for generic phy consumer properties
 - dr_mode: shall be one of "host", "peripheral" and "otg"
   Refer to usb/generic.txt
 - snps,host-dma-disable: disable host DMA mode.
-- g-use-dma: enable dma usage in gadget driver.
+- snps,gadget-dma-enable: enable gadget DMA mode.
 - g-rx-fifo-size: size of rx fifo size in gadget mode.
 - g-np-tx-fifo-size: size of non-periodic tx fifo size in gadget mode.
 - g-tx-fifo-size: size of periodic tx fifo per endpoint (except ep0) in gadget mode.
 
+Deprecated properties:
+- g-use-dma: Use "snps,gadget-dma-enable" instead.
+
 Example:
 
         usb at 101c0000 {
diff --git a/arch/arm/boot/dts/rk3036.dtsi b/arch/arm/boot/dts/rk3036.dtsi
index a935523..2604d2d 100644
--- a/arch/arm/boot/dts/rk3036.dtsi
+++ b/arch/arm/boot/dts/rk3036.dtsi
@@ -204,7 +204,7 @@
 		g-np-tx-fifo-size = <16>;
 		g-rx-fifo-size = <275>;
 		g-tx-fifo-size = <256 128 128 64 64 32>;
-		g-use-dma;
+		snps,gadget-dma-enable;
 		status = "disabled";
 	};
 
diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 17ec2e2..c0db4ae 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -596,7 +596,7 @@
 		g-np-tx-fifo-size = <16>;
 		g-rx-fifo-size = <275>;
 		g-tx-fifo-size = <256 128 128 64 64 32>;
-		g-use-dma;
+		snps,gadget-dma-enable;
 		phys = <&usbphy0>;
 		phy-names = "usb2-phy";
 		status = "disabled";
diff --git a/arch/arm/boot/dts/rk3xxx.dtsi b/arch/arm/boot/dts/rk3xxx.dtsi
index e15beb3..c96d622 100644
--- a/arch/arm/boot/dts/rk3xxx.dtsi
+++ b/arch/arm/boot/dts/rk3xxx.dtsi
@@ -181,7 +181,7 @@
 		g-np-tx-fifo-size = <16>;
 		g-rx-fifo-size = <275>;
 		g-tx-fifo-size = <256 128 128 64 64 32>;
-		g-use-dma;
+		snps,gadget-dma-enable;
 		phys = <&usbphy0>;
 		phy-names = "usb2-phy";
 		status = "disabled";
diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
index 17839db..fe441f7 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -747,7 +747,7 @@
 			clocks = <&sys_ctrl HI6220_USBOTG_HCLK>;
 			clock-names = "otg";
 			dr_mode = "otg";
-			g-use-dma;
+			snps,gadget-dma-enable;
 			g-rx-fifo-size = <512>;
 			g-np-tx-fifo-size = <128>;
 			g-tx-fifo-size = <128 128 128 128 128 128>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3368.dtsi b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
index 0fcb214..6b44544 100644
--- a/arch/arm64/boot/dts/rockchip/rk3368.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
@@ -537,7 +537,7 @@
 		g-np-tx-fifo-size = <16>;
 		g-rx-fifo-size = <275>;
 		g-tx-fifo-size = <256 128 128 64 64 32>;
-		g-use-dma;
+		snps,gadget-dma-enable;
 		status = "disabled";
 	};
 
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 2eb79e8..aeece91 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -1161,10 +1161,17 @@ static void dwc2_set_parameters(struct dwc2_hsotg *hsotg,
 	    (hsotg->dr_mode == USB_DR_MODE_OTG)) {
 		dev_dbg(hsotg->dev, "Setting peripheral device properties\n");
 
-		dwc2_set_param_bool(hsotg, &p->g_dma, true, "g-use-dma",
+		dwc2_set_param_bool(hsotg, &p->g_dma, true,
+				    "snps,gadget-dma-enable",
 				    false, false,
 				    dma_capable);
 
+		/* Check the deprecated property. */
+		if (!p->g_dma)
+			dwc2_set_param_bool(hsotg, &p->g_dma, true, "g-use-dma",
+					    false, false,
+					    dma_capable);
+
 		/*
 		 * The values for g_rx_fifo_size (2048) and
 		 * g_np_tx_fifo_size (1024) come from the legacy s3c
diff --git a/drivers/usb/dwc2/pci.c b/drivers/usb/dwc2/pci.c
index b3f3b58..46a9d2b 100644
--- a/drivers/usb/dwc2/pci.c
+++ b/drivers/usb/dwc2/pci.c
@@ -67,7 +67,7 @@ static int dwc2_pci_quirks(struct pci_dev *pdev, struct platform_device *dwc2)
 	if (pdev->vendor == PCI_VENDOR_ID_SYNOPSYS &&
 	    pdev->device == PCI_PRODUCT_ID_HAPS_HSOTG) {
 		struct property_entry properties[] = {
-			PROPERTY_ENTRY_BOOL("g-use-dma"),
+			PROPERTY_ENTRY_BOOL("snps,gadget-dma-enable"),
 			{ },
 		};
 
-- 
2.10.0

[PATCH 01/30] usb: dwc2: Deprecate g-use-dma binding

[Felipe Balbi]

Hi,

John Youn <johnyoun@synopsys.com> writes:
> Add a vendor prefix and make the name more consistent by renaming it to
> "snps,gadget-dma-enable".
>
> Signed-off-by: John Youn <johnyoun@synopsys.com>
> ---
>  Documentation/devicetree/bindings/usb/dwc2.txt | 5 ++++-
>  arch/arm/boot/dts/rk3036.dtsi                  | 2 +-
>  arch/arm/boot/dts/rk3288.dtsi                  | 2 +-
>  arch/arm/boot/dts/rk3xxx.dtsi                  | 2 +-
>  arch/arm64/boot/dts/hisilicon/hi6220.dtsi      | 2 +-
>  arch/arm64/boot/dts/rockchip/rk3368.dtsi       | 2 +-
>  drivers/usb/dwc2/params.c                      | 9 ++++++++-
>  drivers/usb/dwc2/pci.c                         | 2 +-
>  8 files changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
> index 9472111..389a461 100644
> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
> @@ -26,11 +26,14 @@ Refer to phy/phy-bindings.txt for generic phy consumer properties
>  - dr_mode: shall be one of "host", "peripheral" and "otg"
>    Refer to usb/generic.txt
>  - snps,host-dma-disable: disable host DMA mode.
> -- g-use-dma: enable dma usage in gadget driver.
> +- snps,gadget-dma-enable: enable gadget DMA mode.

I don't see why you even have this binding. Looking through the code,
you have:

#define GHWCFG2_SLAVE_ONLY_ARCH			0
#define GHWCFG2_EXT_DMA_ARCH			1
#define GHWCFG2_INT_DMA_ARCH			2

void dwc2_set_param_dma_enable(struct dwc2_hsotg *hsotg, int val)
{
	int valid = 1;

	if (val > 0 && hsotg->hw_params.arch == GHWCFG2_SLAVE_ONLY_ARCH)
		valid = 0;
	if (val < 0)
		valid = 0;

	if (!valid) {
		if (val >= 0)
			dev_err(hsotg->dev,
				"%d invalid for dma_enable parameter. Check HW configuration.\n",
				val);
		val = hsotg->hw_params.arch != GHWCFG2_SLAVE_ONLY_ARCH;
		dev_dbg(hsotg->dev, "Setting dma_enable to %d\n", val);
	}

	hsotg->core_params->dma_enable = val;
}

which seems to hint that DMA support is discoverable. If there is DMA,
why would disable it?

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 800 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161108/28908c8d/attachment-0001.sig>

[PATCH 01/30] usb: dwc2: Deprecate g-use-dma binding

[John Youn]

On 11/8/2016 1:12 AM, Felipe Balbi wrote:
> 
> Hi,
> 
> John Youn <johnyoun@synopsys.com> writes:
>> Add a vendor prefix and make the name more consistent by renaming it to
>> "snps,gadget-dma-enable".
>>
>> Signed-off-by: John Youn <johnyoun@synopsys.com>
>> ---
>>  Documentation/devicetree/bindings/usb/dwc2.txt | 5 ++++-
>>  arch/arm/boot/dts/rk3036.dtsi                  | 2 +-
>>  arch/arm/boot/dts/rk3288.dtsi                  | 2 +-
>>  arch/arm/boot/dts/rk3xxx.dtsi                  | 2 +-
>>  arch/arm64/boot/dts/hisilicon/hi6220.dtsi      | 2 +-
>>  arch/arm64/boot/dts/rockchip/rk3368.dtsi       | 2 +-
>>  drivers/usb/dwc2/params.c                      | 9 ++++++++-
>>  drivers/usb/dwc2/pci.c                         | 2 +-
>>  8 files changed, 18 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
>> index 9472111..389a461 100644
>> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
>> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
>> @@ -26,11 +26,14 @@ Refer to phy/phy-bindings.txt for generic phy consumer properties
>>  - dr_mode: shall be one of "host", "peripheral" and "otg"
>>    Refer to usb/generic.txt
>>  - snps,host-dma-disable: disable host DMA mode.
>> -- g-use-dma: enable dma usage in gadget driver.
>> +- snps,gadget-dma-enable: enable gadget DMA mode.
> 
> I don't see why you even have this binding. Looking through the code,
> you have:
> 
> #define GHWCFG2_SLAVE_ONLY_ARCH			0
> #define GHWCFG2_EXT_DMA_ARCH			1
> #define GHWCFG2_INT_DMA_ARCH			2
> 
> void dwc2_set_param_dma_enable(struct dwc2_hsotg *hsotg, int val)
> {
> 	int valid = 1;
> 
> 	if (val > 0 && hsotg->hw_params.arch == GHWCFG2_SLAVE_ONLY_ARCH)
> 		valid = 0;
> 	if (val < 0)
> 		valid = 0;
> 
> 	if (!valid) {
> 		if (val >= 0)
> 			dev_err(hsotg->dev,
> 				"%d invalid for dma_enable parameter. Check HW configuration.\n",
> 				val);
> 		val = hsotg->hw_params.arch != GHWCFG2_SLAVE_ONLY_ARCH;
> 		dev_dbg(hsotg->dev, "Setting dma_enable to %d\n", val);
> 	}
> 
> 	hsotg->core_params->dma_enable = val;
> }
> 
> which seems to hint that DMA support is discoverable. If there is DMA,
> why would disable it?
> 

Yes that's the case and I would prefer to make it discoverable and
enabled by default.

But the legacy behavior has always been like this because DMA was
never fully implemented in the gadget driver and it was an opt-in
feature. Periodic support was only added recently.

What do you think about enabling it by default now? I think most
platforms already use DMA.

We would still need a "disable" binding for IP validation purposes at
least.

Regards,
John

[PATCH 01/30] usb: dwc2: Deprecate g-use-dma binding

[Felipe Balbi]

Hi,

John Youn <John.Youn@synopsys.com> writes:
> On 11/8/2016 1:12 AM, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> John Youn <johnyoun@synopsys.com> writes:
>>> Add a vendor prefix and make the name more consistent by renaming it to
>>> "snps,gadget-dma-enable".
>>>
>>> Signed-off-by: John Youn <johnyoun@synopsys.com>
>>> ---
>>>  Documentation/devicetree/bindings/usb/dwc2.txt | 5 ++++-
>>>  arch/arm/boot/dts/rk3036.dtsi                  | 2 +-
>>>  arch/arm/boot/dts/rk3288.dtsi                  | 2 +-
>>>  arch/arm/boot/dts/rk3xxx.dtsi                  | 2 +-
>>>  arch/arm64/boot/dts/hisilicon/hi6220.dtsi      | 2 +-
>>>  arch/arm64/boot/dts/rockchip/rk3368.dtsi       | 2 +-
>>>  drivers/usb/dwc2/params.c                      | 9 ++++++++-
>>>  drivers/usb/dwc2/pci.c                         | 2 +-
>>>  8 files changed, 18 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
>>> index 9472111..389a461 100644
>>> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
>>> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
>>> @@ -26,11 +26,14 @@ Refer to phy/phy-bindings.txt for generic phy consumer properties
>>>  - dr_mode: shall be one of "host", "peripheral" and "otg"
>>>    Refer to usb/generic.txt
>>>  - snps,host-dma-disable: disable host DMA mode.
>>> -- g-use-dma: enable dma usage in gadget driver.
>>> +- snps,gadget-dma-enable: enable gadget DMA mode.
>> 
>> I don't see why you even have this binding. Looking through the code,
>> you have:
>> 
>> #define GHWCFG2_SLAVE_ONLY_ARCH			0
>> #define GHWCFG2_EXT_DMA_ARCH			1
>> #define GHWCFG2_INT_DMA_ARCH			2
>> 
>> void dwc2_set_param_dma_enable(struct dwc2_hsotg *hsotg, int val)
>> {
>> 	int valid = 1;
>> 
>> 	if (val > 0 && hsotg->hw_params.arch == GHWCFG2_SLAVE_ONLY_ARCH)
>> 		valid = 0;
>> 	if (val < 0)
>> 		valid = 0;
>> 
>> 	if (!valid) {
>> 		if (val >= 0)
>> 			dev_err(hsotg->dev,
>> 				"%d invalid for dma_enable parameter. Check HW configuration.\n",
>> 				val);
>> 		val = hsotg->hw_params.arch != GHWCFG2_SLAVE_ONLY_ARCH;
>> 		dev_dbg(hsotg->dev, "Setting dma_enable to %d\n", val);
>> 	}
>> 
>> 	hsotg->core_params->dma_enable = val;
>> }
>> 
>> which seems to hint that DMA support is discoverable. If there is DMA,
>> why would disable it?
>> 
>
> Yes that's the case and I would prefer to make it discoverable and
> enabled by default.
>
> But the legacy behavior has always been like this because DMA was
> never fully implemented in the gadget driver and it was an opt-in
> feature. Periodic support was only added recently.

legacy behavior can be changed if another 'policy' makes more
sense. IMHO, whatever can be discovered in runtime, should be enabled by
default. That way, we force people to use it and find bugs in certain
features.

> What do you think about enabling it by default now? I think most
> platforms already use DMA.

I think it should be discovered. Drop all these "*-use-dma" bindings
because they're not needed if you can probe a register to answer the
same question.

> We would still need a "disable" binding for IP validation purposes at
> least.

Yeah, could be a quirk.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 800 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161109/0691c4a0/attachment.sig>

[PATCH 01/30] usb: dwc2: Deprecate g-use-dma binding

[John Youn]

On 11/8/2016 11:54 PM, Felipe Balbi wrote:
> 
> Hi,
> 
> John Youn <John.Youn@synopsys.com> writes:
>> On 11/8/2016 1:12 AM, Felipe Balbi wrote:
>>>
>>> Hi,
>>>
>>> John Youn <johnyoun@synopsys.com> writes:
>>>> Add a vendor prefix and make the name more consistent by renaming it to
>>>> "snps,gadget-dma-enable".
>>>>
>>>> Signed-off-by: John Youn <johnyoun@synopsys.com>
>>>> ---
>>>>  Documentation/devicetree/bindings/usb/dwc2.txt | 5 ++++-
>>>>  arch/arm/boot/dts/rk3036.dtsi                  | 2 +-
>>>>  arch/arm/boot/dts/rk3288.dtsi                  | 2 +-
>>>>  arch/arm/boot/dts/rk3xxx.dtsi                  | 2 +-
>>>>  arch/arm64/boot/dts/hisilicon/hi6220.dtsi      | 2 +-
>>>>  arch/arm64/boot/dts/rockchip/rk3368.dtsi       | 2 +-
>>>>  drivers/usb/dwc2/params.c                      | 9 ++++++++-
>>>>  drivers/usb/dwc2/pci.c                         | 2 +-
>>>>  8 files changed, 18 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
>>>> index 9472111..389a461 100644
>>>> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
>>>> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
>>>> @@ -26,11 +26,14 @@ Refer to phy/phy-bindings.txt for generic phy consumer properties
>>>>  - dr_mode: shall be one of "host", "peripheral" and "otg"
>>>>    Refer to usb/generic.txt
>>>>  - snps,host-dma-disable: disable host DMA mode.
>>>> -- g-use-dma: enable dma usage in gadget driver.
>>>> +- snps,gadget-dma-enable: enable gadget DMA mode.
>>>
>>> I don't see why you even have this binding. Looking through the code,
>>> you have:
>>>
>>> #define GHWCFG2_SLAVE_ONLY_ARCH			0
>>> #define GHWCFG2_EXT_DMA_ARCH			1
>>> #define GHWCFG2_INT_DMA_ARCH			2
>>>
>>> void dwc2_set_param_dma_enable(struct dwc2_hsotg *hsotg, int val)
>>> {
>>> 	int valid = 1;
>>>
>>> 	if (val > 0 && hsotg->hw_params.arch == GHWCFG2_SLAVE_ONLY_ARCH)
>>> 		valid = 0;
>>> 	if (val < 0)
>>> 		valid = 0;
>>>
>>> 	if (!valid) {
>>> 		if (val >= 0)
>>> 			dev_err(hsotg->dev,
>>> 				"%d invalid for dma_enable parameter. Check HW configuration.\n",
>>> 				val);
>>> 		val = hsotg->hw_params.arch != GHWCFG2_SLAVE_ONLY_ARCH;
>>> 		dev_dbg(hsotg->dev, "Setting dma_enable to %d\n", val);
>>> 	}
>>>
>>> 	hsotg->core_params->dma_enable = val;
>>> }
>>>
>>> which seems to hint that DMA support is discoverable. If there is DMA,
>>> why would disable it?
>>>
>>
>> Yes that's the case and I would prefer to make it discoverable and
>> enabled by default.
>>
>> But the legacy behavior has always been like this because DMA was
>> never fully implemented in the gadget driver and it was an opt-in
>> feature. Periodic support was only added recently.
> 
> legacy behavior can be changed if another 'policy' makes more
> sense. IMHO, whatever can be discovered in runtime, should be enabled by
> default. That way, we force people to use it and find bugs in certain
> features.

Sounds good to me. I'll make the changes.

Regards,
John

[PATCH 01/30] usb: dwc2: Deprecate g-use-dma binding

[Rob Herring]

On Tue, Nov 08, 2016 at 09:48:03AM -0800, John Youn wrote:
> On 11/8/2016 1:12 AM, Felipe Balbi wrote:
> > 
> > Hi,
> > 
> > John Youn <johnyoun@synopsys.com> writes:
> >> Add a vendor prefix and make the name more consistent by renaming it to
> >> "snps,gadget-dma-enable".
> >>
> >> Signed-off-by: John Youn <johnyoun@synopsys.com>
> >> ---
> >>  Documentation/devicetree/bindings/usb/dwc2.txt | 5 ++++-
> >>  arch/arm/boot/dts/rk3036.dtsi                  | 2 +-
> >>  arch/arm/boot/dts/rk3288.dtsi                  | 2 +-
> >>  arch/arm/boot/dts/rk3xxx.dtsi                  | 2 +-
> >>  arch/arm64/boot/dts/hisilicon/hi6220.dtsi      | 2 +-
> >>  arch/arm64/boot/dts/rockchip/rk3368.dtsi       | 2 +-
> >>  drivers/usb/dwc2/params.c                      | 9 ++++++++-
> >>  drivers/usb/dwc2/pci.c                         | 2 +-
> >>  8 files changed, 18 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
> >> index 9472111..389a461 100644
> >> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
> >> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
> >> @@ -26,11 +26,14 @@ Refer to phy/phy-bindings.txt for generic phy consumer properties
> >>  - dr_mode: shall be one of "host", "peripheral" and "otg"
> >>    Refer to usb/generic.txt
> >>  - snps,host-dma-disable: disable host DMA mode.
> >> -- g-use-dma: enable dma usage in gadget driver.
> >> +- snps,gadget-dma-enable: enable gadget DMA mode.
> > 
> > I don't see why you even have this binding. Looking through the code,
> > you have:
> > 
> > #define GHWCFG2_SLAVE_ONLY_ARCH			0
> > #define GHWCFG2_EXT_DMA_ARCH			1
> > #define GHWCFG2_INT_DMA_ARCH			2
> > 
> > void dwc2_set_param_dma_enable(struct dwc2_hsotg *hsotg, int val)
> > {
> > 	int valid = 1;
> > 
> > 	if (val > 0 && hsotg->hw_params.arch == GHWCFG2_SLAVE_ONLY_ARCH)
> > 		valid = 0;
> > 	if (val < 0)
> > 		valid = 0;
> > 
> > 	if (!valid) {
> > 		if (val >= 0)
> > 			dev_err(hsotg->dev,
> > 				"%d invalid for dma_enable parameter. Check HW configuration.\n",
> > 				val);
> > 		val = hsotg->hw_params.arch != GHWCFG2_SLAVE_ONLY_ARCH;
> > 		dev_dbg(hsotg->dev, "Setting dma_enable to %d\n", val);
> > 	}
> > 
> > 	hsotg->core_params->dma_enable = val;
> > }
> > 
> > which seems to hint that DMA support is discoverable. If there is DMA,
> > why would disable it?
> > 
> 
> Yes that's the case and I would prefer to make it discoverable and
> enabled by default.
> 
> But the legacy behavior has always been like this because DMA was
> never fully implemented in the gadget driver and it was an opt-in
> feature. Periodic support was only added recently.
> 
> What do you think about enabling it by default now? I think most
> platforms already use DMA.
> 
> We would still need a "disable" binding for IP validation purposes at
> least.

You can hack up your kernel for that. You may need a disable for broken 
h/w perhaps.

Rob