USB: core: Use device_driver directly in struct usb_driver and usb_device_driver
Subsystems (9)
PATCH 1/1 USB: core: Use device_driver directly in struct usb_driver and usb_device_driver
There is usbdrv_wrap in struct usb_driver and usb_device_driver, it contains device_driver and for_devices. for_devices is used to distinguish between device drivers and interface drivers.
We can compare that if 'drv->probe' is equal to usb_probe_device instead of using for_devices in is_usb_device_driver().
Remove struct usbdrv_wrap, use device_driver directly in struct usb_driver and usb_device_driver. This makes the code more concise.
--- drivers/bluetooth/btusb.c | 6 +- drivers/net/can/usb/peak_usb/pcan_usb_core.c | 2 +- .../broadcom/brcm80211/brcmfmac/usb.c | 2 +- drivers/net/wireless/marvell/mwifiex/usb.c | 2 +- drivers/usb/core/driver.c | 56 +++++++++---------- drivers/usb/core/usb.c | 2 +- drivers/usb/core/usb.h | 7 ++- drivers/usb/misc/onboard_usb_hub.c | 2 +- drivers/usb/serial/bus.c | 2 +- drivers/usb/serial/usb-serial.c | 2 +- drivers/usb/storage/uas.c | 2 +- drivers/usb/usbip/stub_main.c | 8 +-- include/linux/usb.h | 24 +++----- 13 files changed, 52 insertions(+), 65 deletions(-)
drivers/bluetooth/btusb.c +2 -4
@@ -4794,10 +4794,8 @@ static struct usb_driver btusb_driver = { .disable_hub_initiated_lpm = 1,#ifdef CONFIG_DEV_COREDUMP - .drvwrap = { - .driver = { - .coredump = btusb_coredump, - }, + .driver = { + .coredump = btusb_coredump, },#endif };
drivers/net/can/usb/peak_usb/pcan_usb_core.c +1 -1
@@ -1143,7 +1143,7 @@ static void __exit peak_usb_exit(void)int err;/* last chance do send any synchronous commands here */ - err =driver_for_each_device (&peak_usb_driver.drvwrap.driver,NULL , + err =driver_for_each_device (&peak_usb_driver.driver,NULL ,NULL , peak_usb_do_device_exit);if (err) pr_err("%s: failed to stop all can devices (err %d)\n",
drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c +1 -1
@@ -1581,7 +1581,7 @@ static int brcmf_usb_reset_device(struct device *dev, void *notused)void brcmf_usb_exit (void ) { -struct device_driver *drv = &brcmf_usbdrvr.drvwrap.driver; +struct device_driver *drv = &brcmf_usbdrvr.driver;int ret;brcmf_dbg (USB ,"Enter\n" );
drivers/net/wireless/marvell/mwifiex/usb.c +1 -1
@@ -687,7 +687,7 @@ static struct usb_driver mwifiex_usb_driver = { .suspend = mwifiex_usb_suspend, .resume = mwifiex_usb_resume, .soft_unbind = 1, - .drvwrap.driver = { + .driver = { .coredump = mwifiex_usb_coredump, }, };
drivers/usb/core/driver.c +27 -29
131 lines not on this page · raw message
drivers/usb/core/usb.c +1 -1
@@ -431,7 +431,7 @@ struct usb_interface *usb_find_interface(struct usb_driver *drv, int minor)struct device *dev; argb.minor = minor; - argb.drv = &drv->drvwrap.driver; + argb.drv = &drv->driver; dev =bus_find_device (&usb_bus_type,NULL , &argb, __find_interface);
drivers/usb/core/usb.h +4 -3
@@ -176,11 +176,12 @@ static inline int is_root_hub(struct usb_device *udev) }/* Do the same for device drivers and interface drivers. */ - +extern int usb_probe_device (struct device *dev);static inline int is_usb_device_driver (struct device_driver *drv) { - return container_of(drv, struct usbdrv_wrap, driver)-> - for_devices; +if (drv->probe == usb_probe_device) +return 1 ; +return 0 ; }/* for labeling diagnostics */
drivers/usb/misc/onboard_usb_hub.c +1 -1
@@ -244,7 +244,7 @@ static void onboard_hub_attach_usb_driver(struct work_struct *work) {int err; - err =driver_attach (&onboard_hub_usbdev_driver.drvwrap.driver); + err =driver_attach (&onboard_hub_usbdev_driver.driver);if (err)pr_err ("Failed to attach USB driver: %pe\n" ,ERR_PTR (err)); }
drivers/usb/serial/bus.c +1 -1
@@ -113,7 +113,7 @@ static ssize_t new_id_store(struct device_driver *driver,if (retval >=0 && usb_drv->usb_driver !=NULL ) retval =usb_store_new_id (&usb_drv->usb_driver->dynids, usb_drv->usb_driver->id_table, - &usb_drv->usb_driver->drvwrap.driver, + &usb_drv->usb_driver->driver, buf, count);return retval; }
drivers/usb/serial/usb-serial.c +1 -1
@@ -1521,7 +1521,7 @@ int usb_serial_register_drivers(struct usb_serial_driver *const serial_drivers[]/* Now set udriver's id_table and look for matches */ udriver->id_table = id_table; - rc =driver_attach (&udriver->drvwrap.driver); + rc =driver_attach (&udriver->driver);return 0 ; err_deregister_drivers:
drivers/usb/storage/uas.c +1 -1
@@ -1246,7 +1246,7 @@ static struct usb_driver uas_driver = { .suspend = uas_suspend, .resume = uas_resume, .reset_resume = uas_reset_resume, - .drvwrap.driver.shutdown = uas_shutdown, + .driver.shutdown = uas_shutdown, .id_table = uas_usb_ids, };
drivers/usb/usbip/stub_main.c +4 -4
@@ -377,14 +377,14 @@ static int __init usbip_host_init(void) goto err_usb_register; } - ret =driver_create_file (&stub_driver.drvwrap.driver, + ret =driver_create_file (&stub_driver.driver, &driver_attr_match_busid);if (ret) {pr_err ("driver_create_file failed\n" ); goto err_create_file; } - ret =driver_create_file (&stub_driver.drvwrap.driver, + ret =driver_create_file (&stub_driver.driver, &driver_attr_rebind);if (ret) {pr_err ("driver_create_file failed\n" );
@@ -402,10 +402,10 @@ static int __init usbip_host_init(void) static void __exit usbip_host_exit(void) { -driver_remove_file (&stub_driver.drvwrap.driver, +driver_remove_file (&stub_driver.driver, &driver_attr_match_busid); -driver_remove_file (&stub_driver.drvwrap.driver, +driver_remove_file (&stub_driver.driver, &driver_attr_rebind);/*
include/linux/usb.h +7 -17
@@ -1144,16 +1144,6 @@ extern ssize_t usb_store_new_id(struct usb_dynids *dynids,extern ssize_t usb_show_dynids (struct usb_dynids *dynids,char *buf); -/** -* struct usbdrv_wrap - wrapper for driver-model structure -* @driver: The driver-model core driver structure. -* @for_devices: Non-zero for device drivers, 0 for interface drivers. -*/ -struct usbdrv_wrap { -struct device_driver driver; -int for_devices; -}; -/** * struct usb_driver - identifies USB interface driver to usbcore * @name: The driver name should be unique among USB drivers,
@@ -1194,7 +1184,7 @@ struct usbdrv_wrap {* is bound to the driver. * @dynids: used internally to hold the list of dynamically added device * ids for this driver. -* @drvwrap: Driver-model core structure wrapper. +* @driver: The driver-model core driver structure. * @no_dynamic_id: if set to 1, the USB core will not allow dynamic ids to be * added to this driver by preventing the sysfs file from being created. * @supports_autosuspend: if set to 0, the USB core will not allow autosuspend
@@ -1242,13 +1232,13 @@ struct usb_driver {const struct attribute_group **dev_groups;struct usb_dynids dynids; -struct usbdrv_wrap drvwrap; +struct device_driver driver; unsigned int no_dynamic_id:1; unsigned int supports_autosuspend:1; unsigned int disable_hub_initiated_lpm:1; unsigned int soft_unbind:1; }; -#define to_usb_driver (d) container_of(d, struct usb_driver, drvwrap.driver) +#define to_usb_driver (d) container_of(d, struct usb_driver, driver)/** * struct usb_device_driver - identifies USB device driver to usbcore
@@ -1269,7 +1259,7 @@ struct usb_driver {* on to call the normal usb_choose_configuration(). * @dev_groups: Attributes attached to the device that will be created once it * is bound to the driver. -* @drvwrap: Driver-model core structure wrapper. +* @driver: The driver-model core driver structure. * @id_table: used with @match() to select better matching driver at * probe() time. * @supports_autosuspend: if set to 0, the USB core will not allow autosuspend
@@ -1278,7 +1268,7 @@ struct usb_driver {* resume and suspend functions will be called in addition to the driver's * own, so this part of the setup does not need to be replicated. * -* USB drivers must provide all the fields listed above except drvwrap, +* USB drivers must provide all the fields listed above except driver, * match, and id_table. */ struct usb_device_driver {
@@ -1294,13 +1284,13 @@ struct usb_device_driver {int (*choose_configuration) (struct usb_device *udev);const struct attribute_group **dev_groups; -struct usbdrv_wrap drvwrap; +struct device_driver driver;const struct usb_device_id *id_table; unsigned int supports_autosuspend:1; unsigned int generic_subclass:1; };#define to_usb_device_driver (d) container_of(d, struct usb_device_driver, \ - drvwrap.driver) + driver)/** * struct usb_class_driver - identifies a USB driver that wants to use the USB major number
Signature · 1 line
2.25.1
Greg KH
maintainerQuoted 3 lines · Yajun Deng
There is
usbdrv_wrapin structusb_driverandusb_device_driver, it containsdevice_driverandfor_devices.for_devicesis used to distinguish between device drivers and interface drivers.
Yes.
Quoted 2 lines
We can compare that if '
drv->probe' is equal tousb_probe_deviceinstead of usingfor_devicesinis_usb_device_driver().
Why?
Quoted 2 lines
Remove struct
usbdrv_wrap, usedevice_driverdirectly in structusb_driverandusb_device_driver. This makes the code more concise.
Really? What does this help out with? Are there future changes that require this?
I'm all for cleanups, but I don't see what this helps with.
Also, you have a coding style issue in this patch, which means I couldn't take it anyway:
Quoted 1 line
+extern int
usb_probe_device(struct device *dev);
We don't do that in .c files :(
thanks,
greg k-h
Quoted 9 lines · Greg KH
Quoted 3 lines · Yajun Deng
There is
usbdrv_wrapin structusb_driverandusb_device_driver, it containsdevice_driverandfor_devices.for_devicesis used to distinguish between device drivers and interface drivers.Yes.
Quoted 2 lines
We can compare that if '
drv->probe' is equal tousb_probe_deviceinstead of usingfor_devicesinis_usb_device_driver().Why?
For remove struct usbdrv_wrap. As we can see, other structs use device_driver directly.
struct sdio_driver {
... ...
struct device_driver drv;
};
struct pcie_port_service_driver {
... ...
struct device_driver driver;
};
and so on ...
So there is no need a wrapper for device_driver if we can implement is_usb_device_driver().
Quoted 5 lines
Quoted 3 lines
Remove struct
usbdrv_wrap, usedevice_driverdirectly in structusb_driverandusb_device_driver. This makes the code more concise.Really? What does this help out with? Are there future changes that require this?
No, just for cleanups and consistency with other structs.
Quoted 7 lines
I'm all for cleanups, but I don't see what this helps with.
Also, you have a coding style issue in this patch, which means I couldn't take it anyway:
Quoted 1 line
+extern int
usb_probe_device(struct device *dev);We don't do that in .c files :(
It's in drivers/usb/core/usb.h, not in .c files.
Quoted 4 lines
thanks,
greg k-h
Quoted 4 lines · Yajun Deng
There is
usbdrv_wrapin structusb_driverandusb_device_driver, it containsdevice_driverandfor_devices.for_devicesis used to distinguish between device drivers and interface drivers.
Quoted 8 lines
We can compare that if '
drv->probe' is equal tousb_probe_deviceinstead of usingfor_devicesinis_usb_device_driver().Remove struct
usbdrv_wrap, usedevice_driverdirectly in structusb_driverandusb_device_driver. This makes the code more concise.Signed-off-by Yajun Deng <[email protected]>---
However you should simplify the new test:
Quoted 6 lines
drivers/usb/core/usb.h +0
-0@@ -176,11 +176,12 @@ static inline int is_root_hub(struct usb_device *udev) }/* Do the same for device drivers and interface drivers. */
drivers/usb/core/usb.h static inline int is_root_hub(struct usb_device *udev) · line 185
Quoted 10 lines
drivers/usb/core/usb.h +4
-3- +extern int usb_probe_device (struct device *dev);static inline int is_usb_device_driver (struct device_driver *drv) { - return container_of(drv, struct usbdrv_wrap, driver)-> - for_devices; +if (drv->probe == usb_probe_device) +return 1 ; +return 0 ; }
This should just be:
return drv->probe == usb_probe_device;
Alan Stern