On 11/8/2016 1:12 AM, Felipe Balbi wrote:
> 
> Hi,
> 
> John Youn <johny...@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 <johny...@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
--
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

Reply via email to