| Message ID | 20240612180448.1.I805556c176c626872c15ce001f0e8198e1f95ae1@changeid (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c456c5763da4042348040e2ad727f10f7ac17982 |
| Headers | show |
| Series | usb: misc: onboard_usb_dev: Add match function | expand |
On Wed, Jun 12, 2024 at 06:04:48PM +0000, Matthias Kaehlcke wrote: > Add a match function for the onboard_usb_dev driver. Primary > matching is still done through the VID:PID pair, as usual for > USB devices. The new match function checks in addition whether > the device has a device tree node, which is a needed for using > the onboard_usb_dev driver. So this fixes a problem with the existing code? Or is for future changes? confused, greg k-h
Hi Greg, this fixes an existing problem. On chromebooks using this driver for an onboard hub, connecting an external hub in this ID table fails to bind to the generic USB driver at the lock screen (devices default to unauthorized). We are still trying to figure out why the hub isn't able to bind to the generic USB driver after the onboard_usb_dev driver when the device is authorized after it enumerates. But, I think it would be preferable for this driver to not match external devices in the ID table. This resolves the issue for me. Tested-by: Jameson Thies <jthies@google.com> Reviewed-by: Jameson Thies <jthies@google.com> Thanks, Jameson
On Tue, Jun 25, 2024 at 07:27:01AM -0700, Jameson Thies wrote: > Hi Greg, > this fixes an existing problem. On chromebooks using this driver for > an onboard hub, connecting an external hub in this ID table fails to > bind to the generic USB driver at the lock screen (devices default to > unauthorized). We are still trying to figure out why the hub isn't > able to bind to the generic USB driver after the onboard_usb_dev > driver when the device is authorized after it enumerates. But, I think > it would be preferable for this driver to not match external devices > in the ID table. This resolves the issue for me. > > Tested-by: Jameson Thies <jthies@google.com> > Reviewed-by: Jameson Thies <jthies@google.com> Thanks Jameson! To Greg's question: from the perspective of the onboard usb dev driver I would call the change an optimization, there is no point in binding the device to the driver if it is known beforehand that probing will fail. With respect to the issue Jameson described the change is a workaround, but not a proper fix.
diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c index f2bcc1a8b95f..56710e6b1653 100644 --- a/drivers/usb/misc/onboard_usb_dev.c +++ b/drivers/usb/misc/onboard_usb_dev.c @@ -454,16 +454,18 @@ static struct onboard_dev *_find_onboard_dev(struct device *dev) return onboard_dev; } +static bool onboard_dev_usbdev_match(struct usb_device *udev) +{ + /* Onboard devices using this driver must have a device tree node */ + return !!udev->dev.of_node; +} + static int onboard_dev_usbdev_probe(struct usb_device *udev) { struct device *dev = &udev->dev; struct onboard_dev *onboard_dev; int err; - /* ignore supported devices without device tree node */ - if (!dev->of_node) - return -ENODEV; - onboard_dev = _find_onboard_dev(dev); if (IS_ERR(onboard_dev)) return PTR_ERR(onboard_dev); @@ -513,6 +515,7 @@ MODULE_DEVICE_TABLE(usb, onboard_dev_id_table); static struct usb_device_driver onboard_dev_usbdev_driver = { .name = "onboard-usb-dev", + .match = onboard_dev_usbdev_match, .probe = onboard_dev_usbdev_probe, .disconnect = onboard_dev_usbdev_disconnect, .generic_subclass = 1,
Add a match function for the onboard_usb_dev driver. Primary matching is still done through the VID:PID pair, as usual for USB devices. The new match function checks in addition whether the device has a device tree node, which is a needed for using the onboard_usb_dev driver. Remove the check for a device tree node from _probe(), the new match functions prevents devices without DT node from probing. Signed-off-by: Matthias Kaehlcke <mka@chromium.org> --- drivers/usb/misc/onboard_usb_dev.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)