Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1508616 > unrolled thread

Re: [PATCH 2/3] usb: dwc3: host: Do not use dma_set_coherent_mask

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/3] usb: dwc3: host: Do not use dma_set_coherent_mask Arnd Bergmann <arnd@arndb.de> - 2016-10-25 22:50 +0200
    RE: [PATCH 2/3] usb: dwc3: host: Do not use dma_set_coherent_mask Sriram Dash <sriram.dash@nxp.com> - 2016-10-27 07:50 +0200

#1508616 — Re: [PATCH 2/3] usb: dwc3: host: Do not use dma_set_coherent_mask

On Tuesday, October 25, 2016 4:26:27 PM CEST Sriram Dash wrote:
> Do not require dma_set_coherent_mask for hcd
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Aside from the comments I had for patch 3, you are doing two
different things here:

> diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
> index 89a2f71..4d7439c 100644
> --- a/drivers/usb/dwc3/dwc3-st.c
> +++ b/drivers/usb/dwc3/dwc3-st.c
> @@ -218,7 +218,6 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  	if (IS_ERR(regmap))
>  		return PTR_ERR(regmap);
>  
> -	dma_set_coherent_mask(dev, dev->coherent_dma_mask);
>  	dwc3_data->dev = dev;
>  	dwc3_data->regmap = regmap;
>  

This one was setting the mask for the device itself (incorrectly),
so it can be removed along with the dma_coerce_mask_and_coherent()
call in dwc3_exynos_probe, or as a separate patch. It is an
independent cleanup.

> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -833,9 +833,6 @@ struct platform_device *ci_hdrc_add_device(struct device *dev,
>
> -	dma_set_coherent_mask(&pdev->dev, dev->coherent_dma_mask);
>  
>  	ret = platform_device_add_resources(pdev, res, nres);
>  	if (ret)

> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1059,12 +1059,6 @@ static int dwc3_probe(struct platform_device *pdev)
>  
>  	spin_lock_init(&dwc->lock);
>  
> -		dma_set_coherent_mask(dev, dev->parent->coherent_dma_mask);

> --- a/drivers/usb/dwc3/host.c
> +++ b/drivers/usb/dwc3/host.c
> @@ -72,11 +72,7 @@ int dwc3_host_init(struct dwc3 *dwc)
>  		return -ENOMEM;
>  	}
>  
> -	dma_set_coherent_mask(&xhci->dev, dwc->dev->coherent_dma_mask);
> -

These three all set the mask for the *child* devices, as
that is no longer needed after the change in patch 1/3.
I'd suggest leaving those changes together with the rest of
that patch.

However, it's probably better to split up that patch along the
boundaries of the drivers, starting with the USB core:

1/4 usb: separate out sysdev pointer from usb_bus
2/4 usb: chipidea: use bus->sysdev for DMA configuration
3/4 usb: xhci: use bus->sysdev for DMA configuration
4/4 usb: dwc3: use bus->sysdev for DMA configuration

	Arnd

[toc] | [next] | [standalone]

#1510057

>From: Arnd Bergmann [mailto:arnd@arndb.de]
>On Tuesday, October 25, 2016 4:26:27 PM CEST Sriram Dash wrote:
>> Do not require dma_set_coherent_mask for hcd
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
>Aside from the comments I had for patch 3, you are doing two different things
>here:
>
>> diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
>> index 89a2f71..4d7439c 100644
>> --- a/drivers/usb/dwc3/dwc3-st.c
>> +++ b/drivers/usb/dwc3/dwc3-st.c
>> @@ -218,7 +218,6 @@ static int st_dwc3_probe(struct platform_device *pdev)
>>  	if (IS_ERR(regmap))
>>  		return PTR_ERR(regmap);
>>
>> -	dma_set_coherent_mask(dev, dev->coherent_dma_mask);
>>  	dwc3_data->dev = dev;
>>  	dwc3_data->regmap = regmap;
>>
>
>This one was setting the mask for the device itself (incorrectly), so it can be
>removed along with the dma_coerce_mask_and_coherent() call in
>dwc3_exynos_probe, or as a separate patch. It is an independent cleanup.
>
>> --- a/drivers/usb/chipidea/core.c
>> +++ b/drivers/usb/chipidea/core.c
>> @@ -833,9 +833,6 @@ struct platform_device *ci_hdrc_add_device(struct
>> device *dev,
>>
>> -	dma_set_coherent_mask(&pdev->dev, dev->coherent_dma_mask);
>>
>>  	ret = platform_device_add_resources(pdev, res, nres);
>>  	if (ret)
>
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -1059,12 +1059,6 @@ static int dwc3_probe(struct platform_device
>> *pdev)
>>
>>  	spin_lock_init(&dwc->lock);
>>
>> -		dma_set_coherent_mask(dev, dev->parent-
>>coherent_dma_mask);
>
>> --- a/drivers/usb/dwc3/host.c
>> +++ b/drivers/usb/dwc3/host.c
>> @@ -72,11 +72,7 @@ int dwc3_host_init(struct dwc3 *dwc)
>>  		return -ENOMEM;
>>  	}
>>
>> -	dma_set_coherent_mask(&xhci->dev, dwc->dev->coherent_dma_mask);
>> -
>
>These three all set the mask for the *child* devices, as that is no longer needed
>after the change in patch 1/3.
>I'd suggest leaving those changes together with the rest of that patch.
>
>However, it's probably better to split up that patch along the boundaries of the
>drivers, starting with the USB core:
>

Ok. Will do the needful in the next version.

>1/4 usb: separate out sysdev pointer from usb_bus
>2/4 usb: chipidea: use bus->sysdev for DMA configuration
>3/4 usb: xhci: use bus->sysdev for DMA configuration
>4/4 usb: dwc3: use bus->sysdev for DMA configuration
>
>	Arnd

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web