From: Arnd Bergmann <arnd@arndb.de>
To: Sriram Dash <sriram.dash@nxp.com>
Cc: linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
        mathias.nyman@intel.com, gregkh@linuxfoundation.org,
        suresh.gupta@nxp.com, felipe.balbi@linux.intel.com,
        stern@rowland.harvard.edu, pku.leo@gmail.com
Subject: Re: [PATCH 2/3] usb: dwc3: host: Do not use dma_set_coherent_mask
Date: Tue, 25 Oct 2016 22:42:24 +0200
Message-ID: <3451589.MhzIjWDHep@wuerfel>
User-Agent: KMail/5.1.3 (Linux/4.4.0-34-generic; KDE/5.18.0; x86_64; ; )
In-Reply-To: <1477392988-7253-3-git-send-email-sriram.dash@nxp.com>
References: <1477392988-7253-1-git-send-email-sriram.dash@nxp.com> <1477392988-7253-3-git-send-email-sriram.dash@nxp.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2185
Lines: 66

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