lkml.org 
[lkml]   [2016]   [Dec]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [PATCH v5 4/6] usb: xhci: use bus->sysdev for DMA configuration
    From
    Date
    Hi,

    On 17/11/16 13: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>
    > ---
    > 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);

    This is fine but we're forgetting to set sysdev while creating the shared HCD.
    This will lead to invalid dma parameters for the shared HCD.
    So we would need,

    - xhci->shared_hcd = usb_create_shared_hcd(driver, &pdev->dev,
    + xhci->shared_hcd = usb_create_shared_hcd(driver, sysdev,
    dev_name(&pdev->dev), hcd);
    if (!xhci->shared_hcd) {
    ret = -ENOMEM;


    Also, I noticed that Felipe has already picked up the dwc3 patches but
    the core and xhci patches are missing in linux-next leading to the following
    warning

    [ 4.648958] ------------[ cut here ]------------
    [ 4.653602] WARNING: CPU: 0 PID: 1 at drivers/usb/host/xhci-plat.c:159 xhci_plat_probe+0x1a0/0x474
    [ 4.662560] Modules linked in:
    [ 4.665612] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W 4.9.0-rc3 #1028
    [ 4.673258] Hardware name: Keystone
    [ 4.676743] [<c020ed74>] (unwind_backtrace) from [<c020afac>] (show_stack+0x10/0x14)
    [ 4.684479] [<c020afac>] (show_stack) from [<c04c9160>] (dump_stack+0x78/0x8c)
    [ 4.691694] [<c04c9160>] (dump_stack) from [<c021c964>] (__warn+0xec/0x104)
    [ 4.698648] [<c021c964>] (__warn) from [<c021ca2c>] (warn_slowpath_null+0x20/0x28)
    [ 4.706210] [<c021ca2c>] (warn_slowpath_null) from [<c0604c40>] (xhci_plat_probe+0x1a0/0x474)
    [ 4.714729] [<c0604c40>] (xhci_plat_probe) from [<c05651c0>] (platform_drv_probe+0x4c/0xb0)
    [ 4.723073] [<c05651c0>] (platform_drv_probe) from [<c0563cb4>] (driver_probe_device+0x208/0x2b4)
    [ 4.731937] [<c0563cb4>] (driver_probe_device) from [<c0563e18>] (__driver_attach+0xb8/0xbc)
    [ 4.740365] [<c0563e18>] (__driver_attach) from [<c0562088>] (bus_for_each_dev+0x68/0x9c)
    [ 4.748535] [<c0562088>] (bus_for_each_dev) from [<c05631d8>] (bus_add_driver+0x1a4/0x21c)
    [ 4.756791] [<c05631d8>] (bus_add_driver) from [<c05642c8>] (driver_register+0x78/0xf8)
    [ 4.764786] [<c05642c8>] (driver_register) from [<c0201774>] (do_one_initcall+0x44/0x174)
    [ 4.772955] [<c0201774>] (do_one_initcall) from [<c0c00de8>] (kernel_init_freeable+0x154/0x1e4)
    [ 4.781645] [<c0c00de8>] (kernel_init_freeable) from [<c07b0a24>] (kernel_init+0x8/0x10c)
    [ 4.789815] [<c07b0a24>] (kernel_init) from [<c0207588>] (ret_from_fork+0x14/0x2c)
    [ 4.797573] ---[ end trace 0ec4c38d69780113 ]---


    cheers,
    -roger

    > 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 */
    >

    \
     
     \ /
      Last update: 2016-12-09 12:57    [W:3.639 / U:0.084 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site