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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 11/8/2016 1:12 AM, Felipe Balbi wrote:
> 
> Hi,
> 
> John Youn <johnyoun@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
>> ---
>>  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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux