Hi!

On 17/11/16 12:43, Sriram Dash wrote:
> From: Arnd Bergmann <a...@arndb.de>
> 
> For xhci-hcd platform device, all the DMA parameters are not
> configured properly, notably dma ops for dwc3 devices.
> 
> The idea here is that you pass in the parent of_node along with
> the child device pointer, so it would behave exactly like the
> parent already does. The difference is that it also handles all
> the other attributes besides the mask.
> 
> sysdev will represent the physical device, as seen from firmware
> or bus.Splitting the usb_bus->controller field into the
> Linux-internal device (used for the sysfs hierarchy, for printks
> and for power management) and a new pointer (used for DMA,
> DT enumeration and phy lookup) probably covers all that we really
> need.
> 
> Signed-off-by: Arnd Bergmann <a...@arndb.de>
> Signed-off-by: Sriram Dash <sriram.d...@nxp.com>
> Tested-by: Baolin Wang <baolin.w...@linaro.org>

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

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

> Cc: Felipe Balbi <felipe.ba...@linux.intel.com>
> Cc: Grygorii Strashko <grygorii.stras...@ti.com>
> Cc: Sinjan Kumar <sinj...@codeaurora.org>
> Cc: David Fisher <david.fish...@synopsys.com>
> Cc: Catalin Marinas <catalin.mari...@arm.com>
> Cc: "Thang Q. Nguyen" <tqngu...@apm.com>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda...@renesas.com>
> Cc: Stephen Boyd <sb...@codeaurora.org>
> Cc: Bjorn Andersson <bjorn.anders...@linaro.org>
> Cc: Ming Lei <tom.leim...@gmail.com>
> Cc: Jon Masters <j...@redhat.com>
> Cc: Dann Frazier <dann.fraz...@canonical.com>
> Cc: Peter Chen <peter.c...@nxp.com>
> Cc: Leo Li <pku....@gmail.com>
> Tested-by: Brian Norris <briannor...@chromium.org>
> ---
> Changes in v5:
>   - No update
> 
> Changes in v4:
>   - No update
> 
> Changes in v3: 
>   - usb is_device_dma_capable instead of directly accessing 
>     dma props. 
>  
> Changes in v2: 
>   - Split the patch wrt driver
> 
>  drivers/usb/core/buffer.c | 12 ++++++------
>  drivers/usb/core/hcd.c    | 48 
> ++++++++++++++++++++++++++++-------------------
>  drivers/usb/core/usb.c    | 18 +++++++++---------
>  include/linux/usb.h       |  1 +
>  include/linux/usb/hcd.h   |  3 +++
>  5 files changed, 48 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
> index 98e39f9..a6cd44a 100644
> --- a/drivers/usb/core/buffer.c
> +++ b/drivers/usb/core/buffer.c
> @@ -63,7 +63,7 @@ int hcd_buffer_create(struct usb_hcd *hcd)
>       int             i, size;
>  
>       if (!IS_ENABLED(CONFIG_HAS_DMA) ||
> -         (!hcd->self.controller->dma_mask &&
> +         (!is_device_dma_capable(hcd->self.sysdev) &&
>            !(hcd->driver->flags & HCD_LOCAL_MEM)))
>               return 0;
>  
> @@ -72,7 +72,7 @@ int hcd_buffer_create(struct usb_hcd *hcd)
>               if (!size)
>                       continue;
>               snprintf(name, sizeof(name), "buffer-%d", size);
> -             hcd->pool[i] = dma_pool_create(name, hcd->self.controller,
> +             hcd->pool[i] = dma_pool_create(name, hcd->self.sysdev,
>                               size, size, 0);
>               if (!hcd->pool[i]) {
>                       hcd_buffer_destroy(hcd);
> @@ -127,7 +127,7 @@ void *hcd_buffer_alloc(
>  
>       /* some USB hosts just use PIO */
>       if (!IS_ENABLED(CONFIG_HAS_DMA) ||
> -         (!bus->controller->dma_mask &&
> +         (!is_device_dma_capable(bus->sysdev) &&
>            !(hcd->driver->flags & HCD_LOCAL_MEM))) {
>               *dma = ~(dma_addr_t) 0;
>               return kmalloc(size, mem_flags);
> @@ -137,7 +137,7 @@ void *hcd_buffer_alloc(
>               if (size <= pool_max[i])
>                       return dma_pool_alloc(hcd->pool[i], mem_flags, dma);
>       }
> -     return dma_alloc_coherent(hcd->self.controller, size, dma, mem_flags);
> +     return dma_alloc_coherent(hcd->self.sysdev, size, dma, mem_flags);
>  }
>  
>  void hcd_buffer_free(
> @@ -154,7 +154,7 @@ void hcd_buffer_free(
>               return;
>  
>       if (!IS_ENABLED(CONFIG_HAS_DMA) ||
> -         (!bus->controller->dma_mask &&
> +         (!is_device_dma_capable(bus->sysdev) &&
>            !(hcd->driver->flags & HCD_LOCAL_MEM))) {
>               kfree(addr);
>               return;
> @@ -166,5 +166,5 @@ void hcd_buffer_free(
>                       return;
>               }
>       }
> -     dma_free_coherent(hcd->self.controller, size, addr, dma);
> +     dma_free_coherent(hcd->self.sysdev, size, addr, dma);
>  }
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 479e223..f8feb08 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -1073,6 +1073,7 @@ static void usb_deregister_bus (struct usb_bus *bus)
>  static int register_root_hub(struct usb_hcd *hcd)
>  {
>       struct device *parent_dev = hcd->self.controller;
> +     struct device *sysdev = hcd->self.sysdev;
>       struct usb_device *usb_dev = hcd->self.root_hub;
>       const int devnum = 1;
>       int retval;
> @@ -1119,7 +1120,7 @@ static int register_root_hub(struct usb_hcd *hcd)
>               /* Did the HC die before the root hub was registered? */
>               if (HCD_DEAD(hcd))
>                       usb_hc_died (hcd);      /* This time clean up */
> -             usb_dev->dev.of_node = parent_dev->of_node;
> +             usb_dev->dev.of_node = sysdev->of_node;
>       }
>       mutex_unlock(&usb_bus_idr_lock);
>  
> @@ -1465,19 +1466,19 @@ void usb_hcd_unmap_urb_for_dma(struct usb_hcd *hcd, 
> struct urb *urb)
>       dir = usb_urb_dir_in(urb) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
>       if (IS_ENABLED(CONFIG_HAS_DMA) &&
>           (urb->transfer_flags & URB_DMA_MAP_SG))
> -             dma_unmap_sg(hcd->self.controller,
> +             dma_unmap_sg(hcd->self.sysdev,
>                               urb->sg,
>                               urb->num_sgs,
>                               dir);
>       else if (IS_ENABLED(CONFIG_HAS_DMA) &&
>                (urb->transfer_flags & URB_DMA_MAP_PAGE))
> -             dma_unmap_page(hcd->self.controller,
> +             dma_unmap_page(hcd->self.sysdev,
>                               urb->transfer_dma,
>                               urb->transfer_buffer_length,
>                               dir);
>       else if (IS_ENABLED(CONFIG_HAS_DMA) &&
>                (urb->transfer_flags & URB_DMA_MAP_SINGLE))
> -             dma_unmap_single(hcd->self.controller,
> +             dma_unmap_single(hcd->self.sysdev,
>                               urb->transfer_dma,
>                               urb->transfer_buffer_length,
>                               dir);
> @@ -1520,11 +1521,11 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, 
> struct urb *urb,
>                       return ret;
>               if (IS_ENABLED(CONFIG_HAS_DMA) && hcd->self.uses_dma) {
>                       urb->setup_dma = dma_map_single(
> -                                     hcd->self.controller,
> +                                     hcd->self.sysdev,
>                                       urb->setup_packet,
>                                       sizeof(struct usb_ctrlrequest),
>                                       DMA_TO_DEVICE);
> -                     if (dma_mapping_error(hcd->self.controller,
> +                     if (dma_mapping_error(hcd->self.sysdev,
>                                               urb->setup_dma))
>                               return -EAGAIN;
>                       urb->transfer_flags |= URB_SETUP_MAP_SINGLE;
> @@ -1555,7 +1556,7 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct 
> urb *urb,
>                               }
>  
>                               n = dma_map_sg(
> -                                             hcd->self.controller,
> +                                             hcd->self.sysdev,
>                                               urb->sg,
>                                               urb->num_sgs,
>                                               dir);
> @@ -1570,12 +1571,12 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, 
> struct urb *urb,
>                       } else if (urb->sg) {
>                               struct scatterlist *sg = urb->sg;
>                               urb->transfer_dma = dma_map_page(
> -                                             hcd->self.controller,
> +                                             hcd->self.sysdev,
>                                               sg_page(sg),
>                                               sg->offset,
>                                               urb->transfer_buffer_length,
>                                               dir);
> -                             if (dma_mapping_error(hcd->self.controller,
> +                             if (dma_mapping_error(hcd->self.sysdev,
>                                               urb->transfer_dma))
>                                       ret = -EAGAIN;
>                               else
> @@ -1585,11 +1586,11 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, 
> struct urb *urb,
>                               ret = -EAGAIN;
>                       } else {
>                               urb->transfer_dma = dma_map_single(
> -                                             hcd->self.controller,
> +                                             hcd->self.sysdev,
>                                               urb->transfer_buffer,
>                                               urb->transfer_buffer_length,
>                                               dir);
> -                             if (dma_mapping_error(hcd->self.controller,
> +                             if (dma_mapping_error(hcd->self.sysdev,
>                                               urb->transfer_dma))
>                                       ret = -EAGAIN;
>                               else
> @@ -2511,8 +2512,8 @@ static void init_giveback_urb_bh(struct giveback_urb_bh 
> *bh)
>   * Return: On success, a pointer to the created and initialized HCD 
> structure.
>   * On failure (e.g. if memory is unavailable), %NULL.
>   */
> -struct usb_hcd *usb_create_shared_hcd(const struct hc_driver *driver,
> -             struct device *dev, const char *bus_name,
> +struct usb_hcd *__usb_create_hcd(const struct hc_driver *driver,
> +             struct device *sysdev, struct device *dev, const char *bus_name,
>               struct usb_hcd *primary_hcd)
>  {
>       struct usb_hcd *hcd;
> @@ -2553,8 +2554,9 @@ struct usb_hcd *usb_create_shared_hcd(const struct 
> hc_driver *driver,
>  
>       usb_bus_init(&hcd->self);
>       hcd->self.controller = dev;
> +     hcd->self.sysdev = sysdev;
>       hcd->self.bus_name = bus_name;
> -     hcd->self.uses_dma = (dev->dma_mask != NULL);
> +     hcd->self.uses_dma = (sysdev->dma_mask != NULL);
>  
>       init_timer(&hcd->rh_timer);
>       hcd->rh_timer.function = rh_timer_func;
> @@ -2569,6 +2571,14 @@ struct usb_hcd *usb_create_shared_hcd(const struct 
> hc_driver *driver,
>                       "USB Host Controller";
>       return hcd;
>  }
> +EXPORT_SYMBOL_GPL(__usb_create_hcd);
> +
> +struct usb_hcd *usb_create_shared_hcd(const struct hc_driver *driver,
> +             struct device *dev, const char *bus_name,
> +             struct usb_hcd *primary_hcd)
> +{
> +     return __usb_create_hcd(driver, dev, dev, bus_name, primary_hcd);
> +}
>  EXPORT_SYMBOL_GPL(usb_create_shared_hcd);
>  
>  /**
> @@ -2588,7 +2598,7 @@ EXPORT_SYMBOL_GPL(usb_create_shared_hcd);
>  struct usb_hcd *usb_create_hcd(const struct hc_driver *driver,
>               struct device *dev, const char *bus_name)
>  {
> -     return usb_create_shared_hcd(driver, dev, bus_name, NULL);
> +     return __usb_create_hcd(driver, dev, dev, bus_name, NULL);
>  }
>  EXPORT_SYMBOL_GPL(usb_create_hcd);
>  
> @@ -2715,7 +2725,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
>       struct usb_device *rhdev;
>  
>       if (IS_ENABLED(CONFIG_USB_PHY) && !hcd->usb_phy) {
> -             struct usb_phy *phy = usb_get_phy_dev(hcd->self.controller, 0);
> +             struct usb_phy *phy = usb_get_phy_dev(hcd->self.sysdev, 0);
>  
>               if (IS_ERR(phy)) {
>                       retval = PTR_ERR(phy);
> @@ -2733,7 +2743,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
>       }
>  
>       if (IS_ENABLED(CONFIG_GENERIC_PHY) && !hcd->phy) {
> -             struct phy *phy = phy_get(hcd->self.controller, "usb");
> +             struct phy *phy = phy_get(hcd->self.sysdev, "usb");
>  
>               if (IS_ERR(phy)) {
>                       retval = PTR_ERR(phy);
> @@ -2781,7 +2791,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
>        */
>       retval = hcd_buffer_create(hcd);
>       if (retval != 0) {
> -             dev_dbg(hcd->self.controller, "pool alloc failed\n");
> +             dev_dbg(hcd->self.sysdev, "pool alloc failed\n");
>               goto err_create_buf;
>       }
>  
> @@ -2791,7 +2801,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
>  
>       rhdev = usb_alloc_dev(NULL, &hcd->self, 0);
>       if (rhdev == NULL) {
> -             dev_err(hcd->self.controller, "unable to allocate root hub\n");
> +             dev_err(hcd->self.sysdev, "unable to allocate root hub\n");
>               retval = -ENOMEM;
>               goto err_allocate_root_hub;
>       }
> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> index 5921514..3abe83a 100644
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -450,9 +450,9 @@ struct usb_device *usb_alloc_dev(struct usb_device 
> *parent,
>        * Note: calling dma_set_mask() on a USB device would set the
>        * mask for the entire HCD, so don't do that.
>        */
> -     dev->dev.dma_mask = bus->controller->dma_mask;
> -     dev->dev.dma_pfn_offset = bus->controller->dma_pfn_offset;
> -     set_dev_node(&dev->dev, dev_to_node(bus->controller));
> +     dev->dev.dma_mask = bus->sysdev->dma_mask;
> +     dev->dev.dma_pfn_offset = bus->sysdev->dma_pfn_offset;
> +     set_dev_node(&dev->dev, dev_to_node(bus->sysdev));
>       dev->state = USB_STATE_ATTACHED;
>       dev->lpm_disable_count = 1;
>       atomic_set(&dev->urbnum, 0);
> @@ -800,7 +800,7 @@ struct urb *usb_buffer_map(struct urb *urb)
>       if (!urb
>                       || !urb->dev
>                       || !(bus = urb->dev->bus)
> -                     || !(controller = bus->controller))
> +                     || !(controller = bus->sysdev))
>               return NULL;
>  
>       if (controller->dma_mask) {
> @@ -838,7 +838,7 @@ void usb_buffer_dmasync(struct urb *urb)
>                       || !(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)
>                       || !urb->dev
>                       || !(bus = urb->dev->bus)
> -                     || !(controller = bus->controller))
> +                     || !(controller = bus->sysdev))
>               return;
>  
>       if (controller->dma_mask) {
> @@ -872,7 +872,7 @@ void usb_buffer_unmap(struct urb *urb)
>                       || !(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)
>                       || !urb->dev
>                       || !(bus = urb->dev->bus)
> -                     || !(controller = bus->controller))
> +                     || !(controller = bus->sysdev))
>               return;
>  
>       if (controller->dma_mask) {
> @@ -922,7 +922,7 @@ int usb_buffer_map_sg(const struct usb_device *dev, int 
> is_in,
>  
>       if (!dev
>                       || !(bus = dev->bus)
> -                     || !(controller = bus->controller)
> +                     || !(controller = bus->sysdev)
>                       || !controller->dma_mask)
>               return -EINVAL;
>  
> @@ -958,7 +958,7 @@ void usb_buffer_dmasync_sg(const struct usb_device *dev, 
> int is_in,
>  
>       if (!dev
>                       || !(bus = dev->bus)
> -                     || !(controller = bus->controller)
> +                     || !(controller = bus->sysdev)
>                       || !controller->dma_mask)
>               return;
>  
> @@ -986,7 +986,7 @@ void usb_buffer_unmap_sg(const struct usb_device *dev, 
> int is_in,
>  
>       if (!dev
>                       || !(bus = dev->bus)
> -                     || !(controller = bus->controller)
> +                     || !(controller = bus->sysdev)
>                       || !controller->dma_mask)
>               return;
>  
> diff --git a/include/linux/usb.h b/include/linux/usb.h
> index eba1f10..f3f5d8a 100644
> --- a/include/linux/usb.h
> +++ b/include/linux/usb.h
> @@ -354,6 +354,7 @@ struct usb_devmap {
>   */
>  struct usb_bus {
>       struct device *controller;      /* host/master side hardware */
> +     struct device *sysdev;          /* as seen from firmware or bus */
>       int busnum;                     /* Bus number (in order of reg) */
>       const char *bus_name;           /* stable id (PCI slot_name etc) */
>       u8 uses_dma;                    /* Does the host controller use DMA? */
> diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
> index 66fc137..3860560 100644
> --- a/include/linux/usb/hcd.h
> +++ b/include/linux/usb/hcd.h
> @@ -437,6 +437,9 @@ extern int usb_hcd_alloc_bandwidth(struct usb_device 
> *udev,
>               struct usb_host_interface *new_alt);
>  extern int usb_hcd_get_frame_number(struct usb_device *udev);
>  
> +struct usb_hcd *__usb_create_hcd(const struct hc_driver *driver,
> +             struct device *sysdev, struct device *dev, const char *bus_name,
> +             struct usb_hcd *primary_hcd);
>  extern struct usb_hcd *usb_create_hcd(const struct hc_driver *driver,
>               struct device *dev, const char *bus_name);
>  extern struct usb_hcd *usb_create_shared_hcd(const struct hc_driver *driver,
> 

-- 
Best regards,
Alexander Sverdlin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to