Re: [PATCH v3 1/2] usb:dwc3: Enable support for 64-bit system

[Felipe Balbi]

Hi,

(please don't top-post)

"Thang Q. Nguyen"  writes:
> Hi Balbi,
> If CONFIG_DMA_CMA=y, dma mask is set properly. The issue just happen
> when CONFIG_DMA_CMA is not set. In this case, dma mask is not set and
> we need this code to check if dma mask should be manually set to 32 or
> 64.

Can you point me to the code which has this conditional ? Why would
DMA_CMA=n mean that dma_mask isn't initialized ? According to DMA_CMA's
help text (see below) that's supposed to allow drivers to *allocate*
large contiguous buffers, but that's not the case here.

config DMA_CMA
bool "DMA Contiguous Memory Allocator"
depends on HAVE_DMA_CONTIGUOUS && CMA
help
  This enables the Contiguous Memory Allocator which allows drivers
  to allocate big physically-contiguous blocks of memory for use with
  hardware components that do not support I/O map nor scatter-gather.

  You can disable CMA by specifying "cma=0" on the kernel's command
  line.

  For more information see .
  If unsure, say "n".



>
> 
> Thang
>
> On Wed, Mar 30, 2016 at 8:09 PM, Felipe Balbi
>  wrote:
>>
>> Hi,
>>
>> "Thang Q. Nguyen"  writes:
>>> From: "Thang Q. Nguyen" 
>>>
>>> Add 64-bit DMA operation support to the USB DWC3 driver.
>>> First attempt to set the coherent DMA mask for 64-bit DMA.
>>> If that failed, attempt again with 32-bit DMA.
>>>
>>> Changes from v2:
>>>   - None.
>>>
>>> Changes from v1:
>>>   - Remove WARN_ON if dma_mask is NULL
>>
>> these changes lines should be between the tearline (---) and diffstat
>> below.
>>
>>> Signed-off-by: Thang Q. Nguyen 
>>> ---
>>>  drivers/usb/dwc3/core.c | 15 +++
>>>  1 file changed, 15 insertions(+)
>>>
>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>> index de5e01f..2479c24 100644
>>> --- a/drivers/usb/dwc3/core.c
>>> +++ b/drivers/usb/dwc3/core.c
>>> @@ -831,6 +831,21 @@ static int dwc3_probe(struct platform_device *pdev)
>>>   dwc->mem = mem;
>>>   dwc->dev = dev;
>>>
>>> + /* Try to set 64-bit DMA first */
>>> + if (!pdev->dev.dma_mask)
>>> + /* Platform did not initialize dma_mask */
>>> + ret = dma_coerce_mask_and_coherent(>dev,
>>> +DMA_BIT_MASK(64));
>>> + else
>>> + ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(64));
>>> +
>>> + /* If seting 64-bit DMA mask fails, fall back to 32-bit DMA mask */
>>> + if (ret) {
>>> + ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(32));
>>> + if (ret)
>>> + return ret;
>>> + }
>>
>> Also, why is it so that you need this now ? glue layers are copying dma
>> mask from parent device and that should be set properly. This really
>> shouldn't be necessary in dwc3-core; it would mean that glue layer
>> didn't set this device up properly.
>>
>> --
>> balbi
>
>
>
> -- 
>
> Thang Q. Nguyen| Staff SW Eng.
>
> C: +849.7684.7606 | O: +848.3770.0640
>
> F: +848.3770.0641  | tqngu...@apm.com

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v3 1/2] usb:dwc3: Enable support for 64-bit system

[Felipe Balbi]

Hi,

(please don't top-post)

"Thang Q. Nguyen"  writes:
> Hi Balbi,
> If CONFIG_DMA_CMA=y, dma mask is set properly. The issue just happen
> when CONFIG_DMA_CMA is not set. In this case, dma mask is not set and
> we need this code to check if dma mask should be manually set to 32 or
> 64.

Can you point me to the code which has this conditional ? Why would
DMA_CMA=n mean that dma_mask isn't initialized ? According to DMA_CMA's
help text (see below) that's supposed to allow drivers to *allocate*
large contiguous buffers, but that's not the case here.

config DMA_CMA
bool "DMA Contiguous Memory Allocator"
depends on HAVE_DMA_CONTIGUOUS && CMA
help
  This enables the Contiguous Memory Allocator which allows drivers
  to allocate big physically-contiguous blocks of memory for use with
  hardware components that do not support I/O map nor scatter-gather.

  You can disable CMA by specifying "cma=0" on the kernel's command
  line.

  For more information see .
  If unsure, say "n".



>
> 
> Thang
>
> On Wed, Mar 30, 2016 at 8:09 PM, Felipe Balbi
>  wrote:
>>
>> Hi,
>>
>> "Thang Q. Nguyen"  writes:
>>> From: "Thang Q. Nguyen" 
>>>
>>> Add 64-bit DMA operation support to the USB DWC3 driver.
>>> First attempt to set the coherent DMA mask for 64-bit DMA.
>>> If that failed, attempt again with 32-bit DMA.
>>>
>>> Changes from v2:
>>>   - None.
>>>
>>> Changes from v1:
>>>   - Remove WARN_ON if dma_mask is NULL
>>
>> these changes lines should be between the tearline (---) and diffstat
>> below.
>>
>>> Signed-off-by: Thang Q. Nguyen 
>>> ---
>>>  drivers/usb/dwc3/core.c | 15 +++
>>>  1 file changed, 15 insertions(+)
>>>
>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>> index de5e01f..2479c24 100644
>>> --- a/drivers/usb/dwc3/core.c
>>> +++ b/drivers/usb/dwc3/core.c
>>> @@ -831,6 +831,21 @@ static int dwc3_probe(struct platform_device *pdev)
>>>   dwc->mem = mem;
>>>   dwc->dev = dev;
>>>
>>> + /* Try to set 64-bit DMA first */
>>> + if (!pdev->dev.dma_mask)
>>> + /* Platform did not initialize dma_mask */
>>> + ret = dma_coerce_mask_and_coherent(>dev,
>>> +DMA_BIT_MASK(64));
>>> + else
>>> + ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(64));
>>> +
>>> + /* If seting 64-bit DMA mask fails, fall back to 32-bit DMA mask */
>>> + if (ret) {
>>> + ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(32));
>>> + if (ret)
>>> + return ret;
>>> + }
>>
>> Also, why is it so that you need this now ? glue layers are copying dma
>> mask from parent device and that should be set properly. This really
>> shouldn't be necessary in dwc3-core; it would mean that glue layer
>> didn't set this device up properly.
>>
>> --
>> balbi
>
>
>
> -- 
>
> Thang Q. Nguyen| Staff SW Eng.
>
> C: +849.7684.7606 | O: +848.3770.0640
>
> F: +848.3770.0641  | tqngu...@apm.com

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v3 1/2] usb:dwc3: Enable support for 64-bit system

[Thang Q. Nguyen]

Hi Balbi,
If CONFIG_DMA_CMA=y, dma mask is set properly. The issue just happen
when CONFIG_DMA_CMA is not set. In this case, dma mask is not set and
we need this code to check if dma mask should be manually set to 32 or
64.


Thang

On Wed, Mar 30, 2016 at 8:09 PM, Felipe Balbi
 wrote:
>
> Hi,
>
> "Thang Q. Nguyen"  writes:
>> From: "Thang Q. Nguyen" 
>>
>> Add 64-bit DMA operation support to the USB DWC3 driver.
>> First attempt to set the coherent DMA mask for 64-bit DMA.
>> If that failed, attempt again with 32-bit DMA.
>>
>> Changes from v2:
>>   - None.
>>
>> Changes from v1:
>>   - Remove WARN_ON if dma_mask is NULL
>
> these changes lines should be between the tearline (---) and diffstat
> below.
>
>> Signed-off-by: Thang Q. Nguyen 
>> ---
>>  drivers/usb/dwc3/core.c | 15 +++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index de5e01f..2479c24 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -831,6 +831,21 @@ static int dwc3_probe(struct platform_device *pdev)
>>   dwc->mem = mem;
>>   dwc->dev = dev;
>>
>> + /* Try to set 64-bit DMA first */
>> + if (!pdev->dev.dma_mask)
>> + /* Platform did not initialize dma_mask */
>> + ret = dma_coerce_mask_and_coherent(>dev,
>> +DMA_BIT_MASK(64));
>> + else
>> + ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(64));
>> +
>> + /* If seting 64-bit DMA mask fails, fall back to 32-bit DMA mask */
>> + if (ret) {
>> + ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(32));
>> + if (ret)
>> + return ret;
>> + }
>
> Also, why is it so that you need this now ? glue layers are copying dma
> mask from parent device and that should be set properly. This really
> shouldn't be necessary in dwc3-core; it would mean that glue layer
> didn't set this device up properly.
>
> --
> balbi



-- 

Thang Q. Nguyen| Staff SW Eng.

C: +849.7684.7606 | O: +848.3770.0640

F: +848.3770.0641  | tqngu...@apm.com

Re: [PATCH v3 1/2] usb:dwc3: Enable support for 64-bit system

[Thang Q. Nguyen]

Hi Balbi,
If CONFIG_DMA_CMA=y, dma mask is set properly. The issue just happen
when CONFIG_DMA_CMA is not set. In this case, dma mask is not set and
we need this code to check if dma mask should be manually set to 32 or
64.


Thang

On Wed, Mar 30, 2016 at 8:09 PM, Felipe Balbi
 wrote:
>
> Hi,
>
> "Thang Q. Nguyen"  writes:
>> From: "Thang Q. Nguyen" 
>>
>> Add 64-bit DMA operation support to the USB DWC3 driver.
>> First attempt to set the coherent DMA mask for 64-bit DMA.
>> If that failed, attempt again with 32-bit DMA.
>>
>> Changes from v2:
>>   - None.
>>
>> Changes from v1:
>>   - Remove WARN_ON if dma_mask is NULL
>
> these changes lines should be between the tearline (---) and diffstat
> below.
>
>> Signed-off-by: Thang Q. Nguyen 
>> ---
>>  drivers/usb/dwc3/core.c | 15 +++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index de5e01f..2479c24 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -831,6 +831,21 @@ static int dwc3_probe(struct platform_device *pdev)
>>   dwc->mem = mem;
>>   dwc->dev = dev;
>>
>> + /* Try to set 64-bit DMA first */
>> + if (!pdev->dev.dma_mask)
>> + /* Platform did not initialize dma_mask */
>> + ret = dma_coerce_mask_and_coherent(>dev,
>> +DMA_BIT_MASK(64));
>> + else
>> + ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(64));
>> +
>> + /* If seting 64-bit DMA mask fails, fall back to 32-bit DMA mask */
>> + if (ret) {
>> + ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(32));
>> + if (ret)
>> + return ret;
>> + }
>
> Also, why is it so that you need this now ? glue layers are copying dma
> mask from parent device and that should be set properly. This really
> shouldn't be necessary in dwc3-core; it would mean that glue layer
> didn't set this device up properly.
>
> --
> balbi



-- 

Thang Q. Nguyen| Staff SW Eng.

C: +849.7684.7606 | O: +848.3770.0640

F: +848.3770.0641  | tqngu...@apm.com

Re: [PATCH v3 1/2] usb:dwc3: Enable support for 64-bit system

[Felipe Balbi]

Hi,

"Thang Q. Nguyen"  writes:
> From: "Thang Q. Nguyen" 
>
> Add 64-bit DMA operation support to the USB DWC3 driver.
> First attempt to set the coherent DMA mask for 64-bit DMA.
> If that failed, attempt again with 32-bit DMA.
>
> Changes from v2:
>   - None.
>
> Changes from v1:
>   - Remove WARN_ON if dma_mask is NULL

these changes lines should be between the tearline (---) and diffstat
below.

> Signed-off-by: Thang Q. Nguyen 
> ---
>  drivers/usb/dwc3/core.c | 15 +++
>  1 file changed, 15 insertions(+)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index de5e01f..2479c24 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -831,6 +831,21 @@ static int dwc3_probe(struct platform_device *pdev)
>   dwc->mem = mem;
>   dwc->dev = dev;
>  
> + /* Try to set 64-bit DMA first */
> + if (!pdev->dev.dma_mask)
> + /* Platform did not initialize dma_mask */
> + ret = dma_coerce_mask_and_coherent(>dev,
> +DMA_BIT_MASK(64));
> + else
> + ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(64));
> +
> + /* If seting 64-bit DMA mask fails, fall back to 32-bit DMA mask */
> + if (ret) {
> + ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(32));
> + if (ret)
> + return ret;
> + }

Also, why is it so that you need this now ? glue layers are copying dma
mask from parent device and that should be set properly. This really
shouldn't be necessary in dwc3-core; it would mean that glue layer
didn't set this device up properly.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v3 1/2] usb:dwc3: Enable support for 64-bit system

[Felipe Balbi]

Hi,

"Thang Q. Nguyen"  writes:
> From: "Thang Q. Nguyen" 
>
> Add 64-bit DMA operation support to the USB DWC3 driver.
> First attempt to set the coherent DMA mask for 64-bit DMA.
> If that failed, attempt again with 32-bit DMA.
>
> Changes from v2:
>   - None.
>
> Changes from v1:
>   - Remove WARN_ON if dma_mask is NULL

these changes lines should be between the tearline (---) and diffstat
below.

> Signed-off-by: Thang Q. Nguyen 
> ---
>  drivers/usb/dwc3/core.c | 15 +++
>  1 file changed, 15 insertions(+)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index de5e01f..2479c24 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -831,6 +831,21 @@ static int dwc3_probe(struct platform_device *pdev)
>   dwc->mem = mem;
>   dwc->dev = dev;
>  
> + /* Try to set 64-bit DMA first */
> + if (!pdev->dev.dma_mask)
> + /* Platform did not initialize dma_mask */
> + ret = dma_coerce_mask_and_coherent(>dev,
> +DMA_BIT_MASK(64));
> + else
> + ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(64));
> +
> + /* If seting 64-bit DMA mask fails, fall back to 32-bit DMA mask */
> + if (ret) {
> + ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(32));
> + if (ret)
> + return ret;
> + }

Also, why is it so that you need this now ? glue layers are copying dma
mask from parent device and that should be set properly. This really
shouldn't be necessary in dwc3-core; it would mean that glue layer
didn't set this device up properly.

-- 
balbi


signature.asc
Description: PGP signature

[PATCH v3 1/2] usb:dwc3: Enable support for 64-bit system

[Thang Q. Nguyen]

From: "Thang Q. Nguyen" 

Add 64-bit DMA operation support to the USB DWC3 driver.
First attempt to set the coherent DMA mask for 64-bit DMA.
If that failed, attempt again with 32-bit DMA.

Changes from v2:
- None.

Changes from v1:
- Remove WARN_ON if dma_mask is NULL

Signed-off-by: Thang Q. Nguyen 
---
 drivers/usb/dwc3/core.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index de5e01f..2479c24 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -831,6 +831,21 @@ static int dwc3_probe(struct platform_device *pdev)
dwc->mem = mem;
dwc->dev = dev;
 
+   /* Try to set 64-bit DMA first */
+   if (!pdev->dev.dma_mask)
+   /* Platform did not initialize dma_mask */
+   ret = dma_coerce_mask_and_coherent(>dev,
+  DMA_BIT_MASK(64));
+   else
+   ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(64));
+
+   /* If seting 64-bit DMA mask fails, fall back to 32-bit DMA mask */
+   if (ret) {
+   ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(32));
+   if (ret)
+   return ret;
+   }
+
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
if (!res) {
dev_err(dev, "missing IRQ\n");
-- 
2.2.0

[PATCH v3 1/2] usb:dwc3: Enable support for 64-bit system

[Thang Q. Nguyen]

From: "Thang Q. Nguyen" 

Add 64-bit DMA operation support to the USB DWC3 driver.
First attempt to set the coherent DMA mask for 64-bit DMA.
If that failed, attempt again with 32-bit DMA.

Changes from v2:
- None.

Changes from v1:
- Remove WARN_ON if dma_mask is NULL

Signed-off-by: Thang Q. Nguyen 
---
 drivers/usb/dwc3/core.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index de5e01f..2479c24 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -831,6 +831,21 @@ static int dwc3_probe(struct platform_device *pdev)
dwc->mem = mem;
dwc->dev = dev;
 
+   /* Try to set 64-bit DMA first */
+   if (!pdev->dev.dma_mask)
+   /* Platform did not initialize dma_mask */
+   ret = dma_coerce_mask_and_coherent(>dev,
+  DMA_BIT_MASK(64));
+   else
+   ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(64));
+
+   /* If seting 64-bit DMA mask fails, fall back to 32-bit DMA mask */
+   if (ret) {
+   ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(32));
+   if (ret)
+   return ret;
+   }
+
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
if (!res) {
dev_err(dev, "missing IRQ\n");
-- 
2.2.0