Re: [PATCH v5 1/6] usb: separate out sysdev pointer from usb_bus

[Grygorii Strashko]

On 12/02/2016 06:19 PM, Brian Norris wrote:
> Hi all,
> 
> On Thu, Nov 17, 2016 at 05:13:43PM +0530, Sriram Dash wrote:
>> From: Arnd Bergmann 
>>
>> 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 
>> Signed-off-by: Sriram Dash 
>> Tested-by: Baolin Wang 
>> Cc: Felipe Balbi 
>> Cc: Grygorii Strashko 
>> Cc: Sinjan Kumar 
>> Cc: David Fisher 
>> Cc: Catalin Marinas 
>> Cc: "Thang Q. Nguyen" 
>> Cc: Yoshihiro Shimoda 
>> Cc: Stephen Boyd 
>> Cc: Bjorn Andersson 
>> Cc: Ming Lei 
>> Cc: Jon Masters 
>> Cc: Dann Frazier 
>> Cc: Peter Chen 
>> Cc: Leo Li 
>> ---
>> 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
> 
> I didn't notice this series had been reposted a few times. For some
> reason, this wasn't that easy to find in search engines... Anyway, when
> the whole series is applied, this fixes my XHCI probe issues for DWC3
> host mode. Thanks!
> 
> Tested-by: Brian Norris 
> 
> But I noticed that Felipe has applied patches 5 and 6 in -next, while
> the rest are still outstanding. That means I hit the dma_mask WARN_ON()
> in xhci-plat.c, and it eventually fails to probe with -EIO still:
> 
> [2.060272] [ cut here ]
> [2.064908] WARNING: CPU: 5 PID: 1 at drivers/usb/host/xhci-plat.c:159 
> xhci_plat_probe+0x5c/0x444
> ...
> [2.25] [] xhci_plat_probe+0x5c/0x444
> [2.294456] [] platform_drv_probe+0x60/0xac
> [2.300200] [] driver_probe_device+0x12c/0x2a0
> [2.306204] [] __driver_attach+0x84/0xb0
> [2.311687] [] bus_for_each_dev+0x9c/0xcc
> [2.317256] [] driver_attach+0x2c/0x34
> [2.322566] [] bus_add_driver+0xf0/0x1f4
> [2.328049] [] driver_register+0x9c/0xe8
> [2.333530] [] __platform_driver_register+0x60/0x6c
> [2.339968] [] xhci_plat_init+0x2c/0x34
> [2.345366] [] do_one_initcall+0xa4/0x13c
> [2.350936] [] kernel_init_freeable+0x1bc/0x274
> [2.357026] [] kernel_init+0x18/0x104
> [2.362247] [] ret_from_fork+0x10/0x50
> [2.374615] xhci-hcd: probe of xhci-hcd.1.auto failed with error -5
> [2.380962] [ cut here ]
> [2.385588] WARNING: CPU: 4 PID: 1 at drivers/usb/host/xhci-plat.c:159 
> xhci_plat_probe+0x5c/0x444
> ...
> [2.637372] [] xhci_plat_probe+0x5c/0x444
> [2.642941] [] platform_drv_probe+0x60/0xac
> [2.648685] [] driver_probe_device+0x12c/0x2a0
> [2.654688] [] __driver_attach+0x84/0xb0
> [2.660170] [] bus_for_each_dev+0x9c/0xcc
> [2.665739] [] driver_attach+0x2c/0x34
> [2.671048] [] bus_add_driver+0xf0/0x1f4
> [2.676532] [] driver_register+0x9c/0xe8
> [2.682012] [] __platform_driver_register+0x60/0x6c
> [2.688450] [] xhci_plat_init+0x2c/0x34
> [2.693845] [] do_one_initcall+0xa4/0x13c
> [2.699415] [] kernel_init_freeable+0x1bc/0x274
> [2.705505] [] kernel_init+0x18/0x104
> [2.710726] [] ret_from_fork+0x10/0x50
> [2.716075] xhci-hcd: probe of xhci-hcd.2.auto failed with error -5
> 
> What's happening with patches 1-4?
> 

I can observe this warning also with 4.10-rc2

  11.709204] [ cut here ]
[   11.709218] WARNING: CPU: 0 PID: 163 at drivers/usb/host/xhci-plat.c:168 
xhci_plat_probe+0x180/0x450 [xhci_plat_hcd]
[   11.709220] Modules linked in: xhci_plat_hcd(+) xhci_hcd usbcore dwc3 
udc_core usb_common evdev snd_soc_simple_card snd_soc_simple_card_utils omapfb 
cfbfillrect cfbimgblt encoder_tpd12s015 connector_hdmi cfbcopyarea leds_gpi4
[   11.709308] CPU: 0 PID: 163 Comm: systemd-udevd Not tainted 
4.10.0-rc2-00328-g0eeded4-dirty #124
[   11.709310] Hardware name: Generic DRA74X (Flattened Device Tree)
[   11.709326] [] (unwind_backtrace) from [] 
(show_stack+0x10/0x14)
[   11.709334] [] (show_stack) 

Re: [PATCH v5 1/6] usb: separate out sysdev pointer from usb_bus

[Brian Norris]

Hi all,

On Thu, Nov 17, 2016 at 05:13:43PM +0530, Sriram Dash wrote:
> From: Arnd Bergmann 
> 
> 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 
> Signed-off-by: Sriram Dash 
> Tested-by: Baolin Wang 
> Cc: Felipe Balbi 
> Cc: Grygorii Strashko 
> Cc: Sinjan Kumar 
> Cc: David Fisher 
> Cc: Catalin Marinas 
> Cc: "Thang Q. Nguyen" 
> Cc: Yoshihiro Shimoda 
> Cc: Stephen Boyd 
> Cc: Bjorn Andersson 
> Cc: Ming Lei 
> Cc: Jon Masters 
> Cc: Dann Frazier 
> Cc: Peter Chen 
> Cc: Leo Li 
> ---
> 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

I didn't notice this series had been reposted a few times. For some
reason, this wasn't that easy to find in search engines... Anyway, when
the whole series is applied, this fixes my XHCI probe issues for DWC3
host mode. Thanks!

Tested-by: Brian Norris 

But I noticed that Felipe has applied patches 5 and 6 in -next, while
the rest are still outstanding. That means I hit the dma_mask WARN_ON()
in xhci-plat.c, and it eventually fails to probe with -EIO still:

[2.060272] [ cut here ]
[2.064908] WARNING: CPU: 5 PID: 1 at drivers/usb/host/xhci-plat.c:159 
xhci_plat_probe+0x5c/0x444
...
[2.25] [] xhci_plat_probe+0x5c/0x444
[2.294456] [] platform_drv_probe+0x60/0xac
[2.300200] [] driver_probe_device+0x12c/0x2a0
[2.306204] [] __driver_attach+0x84/0xb0
[2.311687] [] bus_for_each_dev+0x9c/0xcc
[2.317256] [] driver_attach+0x2c/0x34
[2.322566] [] bus_add_driver+0xf0/0x1f4
[2.328049] [] driver_register+0x9c/0xe8
[2.333530] [] __platform_driver_register+0x60/0x6c
[2.339968] [] xhci_plat_init+0x2c/0x34
[2.345366] [] do_one_initcall+0xa4/0x13c
[2.350936] [] kernel_init_freeable+0x1bc/0x274
[2.357026] [] kernel_init+0x18/0x104
[2.362247] [] ret_from_fork+0x10/0x50
[2.374615] xhci-hcd: probe of xhci-hcd.1.auto failed with error -5
[2.380962] [ cut here ]
[2.385588] WARNING: CPU: 4 PID: 1 at drivers/usb/host/xhci-plat.c:159 
xhci_plat_probe+0x5c/0x444
...
[2.637372] [] xhci_plat_probe+0x5c/0x444
[2.642941] [] platform_drv_probe+0x60/0xac
[2.648685] [] driver_probe_device+0x12c/0x2a0
[2.654688] [] __driver_attach+0x84/0xb0
[2.660170] [] bus_for_each_dev+0x9c/0xcc
[2.665739] [] driver_attach+0x2c/0x34
[2.671048] [] bus_add_driver+0xf0/0x1f4
[2.676532] [] driver_register+0x9c/0xe8
[2.682012] [] __platform_driver_register+0x60/0x6c
[2.688450] [] xhci_plat_init+0x2c/0x34
[2.693845] [] do_one_initcall+0xa4/0x13c
[2.699415] [] kernel_init_freeable+0x1bc/0x274
[2.705505] [] kernel_init+0x18/0x104
[2.710726] [] ret_from_fork+0x10/0x50
[2.716075] xhci-hcd: probe of xhci-hcd.2.auto failed with error -5

What's happening with patches 1-4?

Regards,
Brian
--
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

[PATCH v5 1/6] usb: separate out sysdev pointer from usb_bus

[Sriram Dash]

From: Arnd Bergmann 

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 
Signed-off-by: Sriram Dash 
Tested-by: Baolin Wang 
Cc: Felipe Balbi 
Cc: Grygorii Strashko 
Cc: Sinjan Kumar 
Cc: David Fisher 
Cc: Catalin Marinas 
Cc: "Thang Q. Nguyen" 
Cc: Yoshihiro Shimoda 
Cc: Stephen Boyd 
Cc: Bjorn Andersson 
Cc: Ming Lei 
Cc: Jon Masters 
Cc: Dann Frazier 
Cc: Peter Chen 
Cc: Leo Li 
---
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;
+