On Tue, 4 Jul 2017, Peter Chen wrote:
> Hi Alan,
>
> I am working on an new USB3 DRD IP development, in order to avoid the
> problem that the DRD core device drvdata is overwritten by usb_hcd
> at __usb_create_hcd [1], I create one device as DRD core device's child
> for USB core (as controller device from USB core point), the device hierarchy
> likes below:
>
> DRD core device (IP device, like dwc3/chipidea)
> |
> |
> ---------------
> | |
> ip-xhci-dev ip-gadget-dev
> | |
> roothub gadget
>
>
> But this device (ip-xhci-dev) has no device_driver, in that case, the
> udev->bus->controller->driver is NULL, so at hub_port_init, it
> shows NULL pointer oops during the enumeration.
>
> In order to fix this problem, my temp solution is using sysdev instead
> of udev->bus->controller, see below:
>
> commit 8fb6057bd59ebb6985dc39ebf41ad7f13ea41b70
> Author: Peter Chen <peter.chen@xxxxxxx>
> Date: Thu Jun 8 14:37:25 2017 +0800
>
> usb: core: hub: fix NULL pointer for driver->name
>
> Signed-off-by: Peter Chen <peter.chen@xxxxxxx>
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 96c66d7..0d7b4eb 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -4399,7 +4399,8 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
> dev_info(&udev->dev,
> "%s %s USB device number %d using %s\n",
> (udev->config) ? "reset" : "new", speed,
> - devnum, udev->bus->controller->driver->name);
> + devnum, udev->bus->controller->driver ? udev->bus->controller->driver->name :
> + udev->bus->sysdev->driver->name);
>
> /* Set up TT records, if needed */
> if (hdev->tt) {
> @@ -4532,7 +4533,8 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
> "%s SuperSpeed%s USB device number %d using %s\n",
> (udev->config) ? "reset" : "new",
> (udev->speed == USB_SPEED_SUPER_PLUS) ? "Plus" : "",
> - devnum, udev->bus->controller->driver->name);
> + devnum, udev->bus->controller->driver ? udev->bus->controller->driver->name :
> + udev->bus->sysdev->driver->name);
> }
>
> /* cope with hardware quirkiness:
>
> But I think above changes may not good enough, do you have any
> suggestions?
I think it's okay. But if we use it, I would want to improve the patch
a little. For example, create a local variable at the start of the
function and store a pointer to the driver's name there, instead of
computing it on-the-fly in these dev_info() calls. And add a comment
explaining why it is necessary.
Another possibility is to have the HC driver bind to the ip_xhci_dev
device instead of to the core device, like Felipe does with dwc3. I
don't know if you can make that work, but if you can then it would
remove any conflicts.
> If there is no good solution for fixing it, how about just using core device
> as parent for roothub, this is what chipidea does now?
> Since the xhci has no /sys interfaces like ehci at ehci-sysfs.c implements,
> it is ok for now, but I am not sure if the USB core will use platform device's
> drvdata in the future, like what usb_hcd_platform_shutdown does.
Yes, that approach might cause problems in the future. Not so much
because of the conflict between the HC driver and the platform driver,
but because of the conflict between the HC driver and the UDC driver.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
