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.
Signed-off-by: Yajun Deng <[email protected]>
---
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(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 0926e4451802..e02839c1764b 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -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
};
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
index 24ad9f593a77..1efa39e134f4 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
@@ -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",
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
index 2178675ae1a4..0ccf735316c2 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
@@ -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");
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
index d3ab9572e711..515e6db410f2 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.c
+++ b/drivers/net/wireless/marvell/mwifiex/usb.c
@@ -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,
},
};
diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index 1dc0c0413043..fa24fe804c91 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -189,13 +189,13 @@ static int usb_create_newid_files(struct usb_driver *usb_drv)
goto exit;
if (usb_drv->probe != NULL) {
- error = driver_create_file(&usb_drv->drvwrap.driver,
+ error = driver_create_file(&usb_drv->driver,
&driver_attr_new_id);
if (error == 0) {
- error = driver_create_file(&usb_drv->drvwrap.driver,
+ error = driver_create_file(&usb_drv->driver,
&driver_attr_remove_id);
if (error)
- driver_remove_file(&usb_drv->drvwrap.driver,
+ driver_remove_file(&usb_drv->driver,
&driver_attr_new_id);
}
}
@@ -209,9 +209,9 @@ static void usb_remove_newid_files(struct usb_driver *usb_drv)
return;
if (usb_drv->probe != NULL) {
- driver_remove_file(&usb_drv->drvwrap.driver,
+ driver_remove_file(&usb_drv->driver,
&driver_attr_remove_id);
- driver_remove_file(&usb_drv->drvwrap.driver,
+ driver_remove_file(&usb_drv->driver,
&driver_attr_new_id);
}
}
@@ -246,7 +246,7 @@ static const struct usb_device_id *usb_match_dynamic_id(struct usb_interface *in
/* called from driver core with dev locked */
-static int usb_probe_device(struct device *dev)
+int usb_probe_device(struct device *dev)
{
struct usb_device_driver *udriver = to_usb_device_driver(dev->driver);
struct usb_device *udev = to_usb_device(dev);
@@ -552,7 +552,7 @@ int usb_driver_claim_interface(struct usb_driver *driver,
if (!iface->authorized)
return -ENODEV;
- dev->driver = &driver->drvwrap.driver;
+ dev->driver = &driver->driver;
usb_set_intfdata(iface, data);
iface->needs_binding = 0;
@@ -615,7 +615,7 @@ void usb_driver_release_interface(struct usb_driver *driver,
struct device *dev = &iface->dev;
/* this should never happen, don't release something that's not ours */
- if (!dev->driver || dev->driver != &driver->drvwrap.driver)
+ if (!dev->driver || dev->driver != &driver->driver)
return;
/* don't release from within disconnect() */
@@ -950,7 +950,7 @@ static int __usb_bus_reprobe_drivers(struct device *dev, void *data)
int ret;
/* Don't reprobe if current driver isn't usb_generic_driver */
- if (dev->driver != &usb_generic_driver.drvwrap.driver)
+ if (dev->driver != &usb_generic_driver.driver)
return 0;
udev = to_usb_device(dev);
@@ -983,15 +983,14 @@ int usb_register_device_driver(struct usb_device_driver *new_udriver,
if (usb_disabled())
return -ENODEV;
- new_udriver->drvwrap.for_devices = 1;
- new_udriver->drvwrap.driver.name = new_udriver->name;
- new_udriver->drvwrap.driver.bus = &usb_bus_type;
- new_udriver->drvwrap.driver.probe = usb_probe_device;
- new_udriver->drvwrap.driver.remove = usb_unbind_device;
- new_udriver->drvwrap.driver.owner = owner;
- new_udriver->drvwrap.driver.dev_groups = new_udriver->dev_groups;
+ new_udriver->driver.name = new_udriver->name;
+ new_udriver->driver.bus = &usb_bus_type;
+ new_udriver->driver.probe = usb_probe_device;
+ new_udriver->driver.remove = usb_unbind_device;
+ new_udriver->driver.owner = owner;
+ new_udriver->driver.dev_groups = new_udriver->dev_groups;
- retval = driver_register(&new_udriver->drvwrap.driver);
+ retval = driver_register(&new_udriver->driver);
if (!retval) {
pr_info("%s: registered new device driver %s\n",
@@ -1023,7 +1022,7 @@ void usb_deregister_device_driver(struct usb_device_driver *udriver)
pr_info("%s: deregistering device driver %s\n",
usbcore_name, udriver->name);
- driver_unregister(&udriver->drvwrap.driver);
+ driver_unregister(&udriver->driver);
}
EXPORT_SYMBOL_GPL(usb_deregister_device_driver);
@@ -1051,18 +1050,17 @@ int usb_register_driver(struct usb_driver *new_driver, struct module *owner,
if (usb_disabled())
return -ENODEV;
- new_driver->drvwrap.for_devices = 0;
- new_driver->drvwrap.driver.name = new_driver->name;
- new_driver->drvwrap.driver.bus = &usb_bus_type;
- new_driver->drvwrap.driver.probe = usb_probe_interface;
- new_driver->drvwrap.driver.remove = usb_unbind_interface;
- new_driver->drvwrap.driver.owner = owner;
- new_driver->drvwrap.driver.mod_name = mod_name;
- new_driver->drvwrap.driver.dev_groups = new_driver->dev_groups;
+ new_driver->driver.name = new_driver->name;
+ new_driver->driver.bus = &usb_bus_type;
+ new_driver->driver.probe = usb_probe_interface;
+ new_driver->driver.remove = usb_unbind_interface;
+ new_driver->driver.owner = owner;
+ new_driver->driver.mod_name = mod_name;
+ new_driver->driver.dev_groups = new_driver->dev_groups;
spin_lock_init(&new_driver->dynids.lock);
INIT_LIST_HEAD(&new_driver->dynids.list);
- retval = driver_register(&new_driver->drvwrap.driver);
+ retval = driver_register(&new_driver->driver);
if (retval)
goto out;
@@ -1077,7 +1075,7 @@ int usb_register_driver(struct usb_driver *new_driver, struct module *owner,
return retval;
out_newid:
- driver_unregister(&new_driver->drvwrap.driver);
+ driver_unregister(&new_driver->driver);
pr_err("%s: error %d registering interface driver %s\n",
usbcore_name, retval, new_driver->name);
@@ -1102,7 +1100,7 @@ void usb_deregister(struct usb_driver *driver)
usbcore_name, driver->name);
usb_remove_newid_files(driver);
- driver_unregister(&driver->drvwrap.driver);
+ driver_unregister(&driver->driver);
usb_free_dynids(driver);
}
EXPORT_SYMBOL_GPL(usb_deregister);
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 2a938cf47ccd..dc8d9228a5e7 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -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);
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 60363153fc3f..9912a7857ad5 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -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 */
diff --git a/drivers/usb/misc/onboard_usb_hub.c b/drivers/usb/misc/onboard_usb_hub.c
index c09fb6bd7d7d..0dd2b032c90b 100644
--- a/drivers/usb/misc/onboard_usb_hub.c
+++ b/drivers/usb/misc/onboard_usb_hub.c
@@ -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));
}
diff --git a/drivers/usb/serial/bus.c b/drivers/usb/serial/bus.c
index 3eb8dc3a1a8f..6c812d01b37d 100644
--- a/drivers/usb/serial/bus.c
+++ b/drivers/usb/serial/bus.c
@@ -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;
}
diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 17b09f03ef84..f1e91eb7f8a4 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -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:
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 696bb0b23599..9707f53cfda9 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -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,
};
diff --git a/drivers/usb/usbip/stub_main.c b/drivers/usb/usbip/stub_main.c
index 0a6624d37929..79110a69d697 100644
--- a/drivers/usb/usbip/stub_main.c
+++ b/drivers/usb/usbip/stub_main.c
@@ -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);
/*
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 618e5a0b1a22..47f1e8c17674 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -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
--
2.25.1
On Fri, Dec 15, 2023 at 02:31:01PM +0800, Yajun Deng wrote:
> 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.
Yes.
> We can compare that if 'drv->probe' is equal to usb_probe_device instead
> of using for_devices in is_usb_device_driver().
Why?
> Remove struct usbdrv_wrap, use device_driver directly in struct usb_driver
> and usb_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:
> +extern int usb_probe_device(struct device *dev);
We don't do that in .c files :(
thanks,
greg k-h
On 2023/12/15 20:40, Greg KH wrote:
> On Fri, Dec 15, 2023 at 02:31:01PM +0800, Yajun Deng wrote:
>> 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.
> Yes.
>
>> We can compare that if 'drv->probe' is equal to usb_probe_device instead
>> of using for_devices in is_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().
>
>> Remove struct usbdrv_wrap, use device_driver directly in struct usb_driver
>> and usb_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.
> 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:
>
>> +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.
>
> thanks,
>
> greg k-h
On Fri, Dec 15, 2023 at 02:31:01PM +0800, Yajun Deng wrote:
> 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.
>
> Signed-off-by: Yajun Deng <[email protected]>
> ---
Acked-by: Alan Stern <[email protected]>
However you should simplify the new test:
> --- a/drivers/usb/core/usb.h
> +++ b/drivers/usb/core/usb.h
> @@ -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;
> }
This should just be:
return drv->probe == usb_probe_device;
Alan Stern