[PATCH] usb: misc: onboard_usb_dev: Add match function

[PATCH] usb: misc: onboard_usb_dev: Add match function

[Matthias Kaehlcke]

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(-)

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,
-- 
2.45.2.627.g7a2c4fd464-goog

Re: [PATCH] usb: misc: onboard_usb_dev: Add match function

[Greg Kroah-Hartman]

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

Re: [PATCH] usb: misc: onboard_usb_dev: Add match function

[Jameson Thies]

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

Re: [PATCH] usb: misc: onboard_usb_dev: Add match function

[Matthias Kaehlcke]

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.