Subject: Re: [v5,4/6] usb: xhci: use bus->sysdev for DMA configuration
To: Sriram Dash <sriram.dash@nxp.com>, <linux-kernel@vger.kernel.org>,
        <linux-usb@vger.kernel.org>
References: <1479383028-27701-5-git-send-email-sriram.dash@nxp.com>
CC: <mathias.nyman@intel.com>, <gregkh@linuxfoundation.org>,
        <suresh.gupta@nxp.com>, <felipe.balbi@linux.intel.com>,
        <stern@rowland.harvard.edu>, <pku.leo@gmail.com>,
        Arnd Bergmann <arnd@arndb.de>
From: Alexander Sverdlin <alexander.sverdlin@nokia.com>
Message-ID: <319e09b5-8217-fff3-819f-f2482aae07fa@nokia.com>
Date: Tue, 13 Dec 2016 17:09:34 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.5.0
MIME-Version: 1.0
In-Reply-To: <1479383028-27701-5-git-send-email-sriram.dash@nxp.com>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Dec 2016 16:09:38.3029 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR07MB1327
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 8272
Lines: 235

Hi!

On 17/11/16 12:43, Sriram Dash wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> For xhci-hcd platform device, all the DMA parameters are not
> configured properly, notably dma ops for dwc3 devices. So, set
> the dma for xhci from sysdev. sysdev is pointing to device that
> is known to the system firmware or hardware.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
> Tested-by: Baolin Wang <baolin.wang@linaro.org>

Successfully tested on arm64/axxia with DWC3 USB host, XHCIs properly inherit
DMA configuration. Therefore:

Tested-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>

> ---
> Changes in v5:
>   - No update
> 
> Changes in v4:
>   - No update
> 
> Changes in v3:
>   - No update
> 
> Changes in v2:
>   - Separate out xhci driver changes apart
> 
>  drivers/usb/host/xhci-mem.c  | 12 ++++++------
>  drivers/usb/host/xhci-plat.c | 33 ++++++++++++++++++++++++++-------
>  drivers/usb/host/xhci.c      | 15 +++++++++++----
>  3 files changed, 43 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 48a26d378..eb32de9 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -586,7 +586,7 @@ static void xhci_free_stream_ctx(struct xhci_hcd *xhci,
>  		unsigned int num_stream_ctxs,
>  		struct xhci_stream_ctx *stream_ctx, dma_addr_t dma)
>  {
> -	struct device *dev = xhci_to_hcd(xhci)->self.controller;
> +	struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
>  	size_t size = sizeof(struct xhci_stream_ctx) * num_stream_ctxs;
>  
>  	if (size > MEDIUM_STREAM_ARRAY_SIZE)
> @@ -614,7 +614,7 @@ static struct xhci_stream_ctx *xhci_alloc_stream_ctx(struct xhci_hcd *xhci,
>  		unsigned int num_stream_ctxs, dma_addr_t *dma,
>  		gfp_t mem_flags)
>  {
> -	struct device *dev = xhci_to_hcd(xhci)->self.controller;
> +	struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
>  	size_t size = sizeof(struct xhci_stream_ctx) * num_stream_ctxs;
>  
>  	if (size > MEDIUM_STREAM_ARRAY_SIZE)
> @@ -1644,7 +1644,7 @@ void xhci_slot_copy(struct xhci_hcd *xhci,
>  static int scratchpad_alloc(struct xhci_hcd *xhci, gfp_t flags)
>  {
>  	int i;
> -	struct device *dev = xhci_to_hcd(xhci)->self.controller;
> +	struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
>  	int num_sp = HCS_MAX_SCRATCHPAD(xhci->hcs_params2);
>  
>  	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
> @@ -1716,7 +1716,7 @@ static void scratchpad_free(struct xhci_hcd *xhci)
>  {
>  	int num_sp;
>  	int i;
> -	struct device *dev = xhci_to_hcd(xhci)->self.controller;
> +	struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
>  
>  	if (!xhci->scratchpad)
>  		return;
> @@ -1792,7 +1792,7 @@ void xhci_free_command(struct xhci_hcd *xhci,
>  
>  void xhci_mem_cleanup(struct xhci_hcd *xhci)
>  {
> -	struct device	*dev = xhci_to_hcd(xhci)->self.controller;
> +	struct device	*dev = xhci_to_hcd(xhci)->self.sysdev;
>  	int size;
>  	int i, j, num_ports;
>  
> @@ -2334,7 +2334,7 @@ static int xhci_setup_port_arrays(struct xhci_hcd *xhci, gfp_t flags)
>  int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
>  {
>  	dma_addr_t	dma;
> -	struct device	*dev = xhci_to_hcd(xhci)->self.controller;
> +	struct device	*dev = xhci_to_hcd(xhci)->self.sysdev;
>  	unsigned int	val, val2;
>  	u64		val_64;
>  	struct xhci_segment	*seg;
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index ed56bf9..beb95c8 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -14,6 +14,7 @@
>  #include <linux/clk.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/module.h>
> +#include <linux/pci.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/usb/phy.h>
> @@ -139,6 +140,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
>  {
>  	const struct of_device_id *match;
>  	const struct hc_driver	*driver;
> +	struct device		*sysdev;
>  	struct xhci_hcd		*xhci;
>  	struct resource         *res;
>  	struct usb_hcd		*hcd;
> @@ -155,22 +157,39 @@ static int xhci_plat_probe(struct platform_device *pdev)
>  	if (irq < 0)
>  		return -ENODEV;
>  
> +	/*
> +	 * sysdev must point to a device that is known to the system firmware
> +	 * or PCI hardware. We handle these three cases here:
> +	 * 1. xhci_plat comes from firmware
> +	 * 2. xhci_plat is child of a device from firmware (dwc3-plat)
> +	 * 3. xhci_plat is grandchild of a pci device (dwc3-pci)
> +	 */
> +	sysdev = &pdev->dev;
> +	if (sysdev->parent && !sysdev->of_node && sysdev->parent->of_node)
> +		sysdev = sysdev->parent;
> +#ifdef CONFIG_PCI
> +	else if (sysdev->parent && sysdev->parent->parent &&
> +		 sysdev->parent->parent->bus == &pci_bus_type)
> +		sysdev = sysdev->parent->parent;
> +#endif
> +
>  	/* Try to set 64-bit DMA first */
> -	if (WARN_ON(!pdev->dev.dma_mask))
> +	if (WARN_ON(!sysdev->dma_mask))
>  		/* Platform did not initialize dma_mask */
> -		ret = dma_coerce_mask_and_coherent(&pdev->dev,
> +		ret = dma_coerce_mask_and_coherent(sysdev,
>  						   DMA_BIT_MASK(64));
>  	else
> -		ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> +		ret = dma_set_mask_and_coherent(sysdev, 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(&pdev->dev, DMA_BIT_MASK(32));
> +		ret = dma_set_mask_and_coherent(sysdev, DMA_BIT_MASK(32));
>  		if (ret)
>  			return ret;
>  	}
>  
> -	hcd = usb_create_hcd(driver, &pdev->dev, dev_name(&pdev->dev));
> +	hcd = __usb_create_hcd(driver, sysdev, &pdev->dev,
> +			       dev_name(&pdev->dev), NULL);
>  	if (!hcd)
>  		return -ENOMEM;
>  
> @@ -220,13 +239,13 @@ static int xhci_plat_probe(struct platform_device *pdev)
>  		goto disable_clk;
>  	}
>  
> -	if (device_property_read_bool(&pdev->dev, "usb3-lpm-capable"))
> +	if (device_property_read_bool(sysdev, "usb3-lpm-capable"))
>  		xhci->quirks |= XHCI_LPM_SUPPORT;
>  
>  	if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
>  		xhci->shared_hcd->can_do_streams = 1;
>  
> -	hcd->usb_phy = devm_usb_get_phy_by_phandle(&pdev->dev, "usb-phy", 0);
> +	hcd->usb_phy = devm_usb_get_phy_by_phandle(sysdev, "usb-phy", 0);
>  	if (IS_ERR(hcd->usb_phy)) {
>  		ret = PTR_ERR(hcd->usb_phy);
>  		if (ret == -EPROBE_DEFER)
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index cf30cb6..fc76671 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -231,6 +231,9 @@ static int xhci_free_msi(struct xhci_hcd *xhci)
>  static int xhci_setup_msi(struct xhci_hcd *xhci)
>  {
>  	int ret;
> +	/*
> +	 * TODO:Check with MSI Soc for sysdev
> +	 */
>  	struct pci_dev  *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
>  
>  	ret = pci_enable_msi(pdev);
> @@ -257,7 +260,7 @@ static int xhci_setup_msi(struct xhci_hcd *xhci)
>   */
>  static void xhci_free_irq(struct xhci_hcd *xhci)
>  {
> -	struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
> +	struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.sysdev);
>  	int ret;
>  
>  	/* return if using legacy interrupt */
> @@ -743,7 +746,7 @@ void xhci_shutdown(struct usb_hcd *hcd)
>  	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>  
>  	if (xhci->quirks & XHCI_SPURIOUS_REBOOT)
> -		usb_disable_xhci_ports(to_pci_dev(hcd->self.controller));
> +		usb_disable_xhci_ports(to_pci_dev(hcd->self.sysdev));
>  
>  	spin_lock_irq(&xhci->lock);
>  	xhci_halt(xhci);
> @@ -760,7 +763,7 @@ void xhci_shutdown(struct usb_hcd *hcd)
>  
>  	/* Yet another workaround for spurious wakeups at shutdown with HSW */
>  	if (xhci->quirks & XHCI_SPURIOUS_WAKEUP)
> -		pci_set_power_state(to_pci_dev(hcd->self.controller), PCI_D3hot);
> +		pci_set_power_state(to_pci_dev(hcd->self.sysdev), PCI_D3hot);
>  }
>  
>  #ifdef CONFIG_PM
> @@ -4832,7 +4835,11 @@ int xhci_get_frame(struct usb_hcd *hcd)
>  int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
>  {
>  	struct xhci_hcd		*xhci;
> -	struct device		*dev = hcd->self.controller;
> +	/*
> +	 * TODO: Check with DWC3 clients for sysdev according to
> +	 * quirks
> +	 */
> +	struct device		*dev = hcd->self.sysdev;
>  	int			retval;
>  
>  	/* Accept arbitrarily long scatter-gather lists */
> 

-- 
Best regards,
Alexander Sverdlin.