]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
net: usb: cdc-ether: unify error handling in probe
authorOliver Neukum <oneukum@suse.com>
Thu, 12 Mar 2026 08:45:32 +0000 (09:45 +0100)
committerJakub Kicinski <kuba@kernel.org>
Sat, 14 Mar 2026 19:22:01 +0000 (12:22 -0700)
usbnet_generic_cdc_bind() is duplicating the error handling
multiple times. That is bad. Unify it with jumps.

V2: Update error logging with every cause a unique message

Signed-off-by: Oliver Neukum <oneukum@suse.com>
Link: https://patch.msgid.link/20260312084612.1469853-1-oneukum@suse.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
drivers/net/usb/cdc_ether.c

index a032c1ded40634b896d0df85175a5938fb1686cd..a0a5740590b9455b2c2db52297820899f2719bae 100644 (file)
@@ -115,7 +115,7 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
        int                             len = intf->cur_altsetting->extralen;
        struct usb_interface_descriptor *d;
        struct cdc_state                *info = (void *) &dev->data;
-       int                             status;
+       int                             status = -ENODEV;
        int                             rndis;
        bool                            android_rndis_quirk = false;
        struct usb_driver               *driver = driver_of(intf);
@@ -169,10 +169,13 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
        info->header = header.usb_cdc_header_desc;
        info->ether = header.usb_cdc_ether_desc;
        if (!info->u) {
-               if (rndis)
+               if (rndis) {
                        goto skip;
-               else /* in that case a quirk is mandatory */
+               } else {
+                       /* in that case a quirk is mandatory */
+                       dev_err(&dev->udev->dev, "No union descriptors\n");
                        goto bad_desc;
+               }
        }
        /* we need a master/control interface (what we're
         * probed with) and a slave/data interface; union
@@ -192,18 +195,20 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
                        android_rndis_quirk = true;
                        goto skip;
                }
+               dev_err(&intf->dev, "bad CDC descriptors\n");
                goto bad_desc;
        }
        if (info->control != intf) {
-               dev_dbg(&intf->dev, "bogus CDC Union\n");
                /* Ambit USB Cable Modem (and maybe others)
                 * interchanges master and slave interface.
                 */
                if (info->data == intf) {
                        info->data = info->control;
                        info->control = intf;
-               } else
+               } else {
+                       dev_err(&intf->dev, "bogus CDC Union\n");
                        goto bad_desc;
+               }
        }
 
        /* some devices merge these - skip class check */
@@ -213,7 +218,7 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
        /* a data interface altsetting does the real i/o */
        d = &info->data->cur_altsetting->desc;
        if (d->bInterfaceClass != USB_CLASS_CDC_DATA) {
-               dev_dbg(&intf->dev, "slave class %u\n", d->bInterfaceClass);
+               dev_err(&intf->dev, "slave class %u\n", d->bInterfaceClass);
                goto bad_desc;
        }
 skip:
@@ -227,7 +232,7 @@ skip:
        if (rndis && is_rndis(&intf->cur_altsetting->desc) &&
            header.usb_cdc_acm_descriptor &&
            header.usb_cdc_acm_descriptor->bmCapabilities) {
-               dev_dbg(&intf->dev,
+               dev_err(&intf->dev,
                        "ACM capabilities %02x, not really RNDIS?\n",
                        header.usb_cdc_acm_descriptor->bmCapabilities);
                goto bad_desc;
@@ -242,14 +247,14 @@ skip:
 
        if (header.usb_cdc_mdlm_desc &&
            memcmp(header.usb_cdc_mdlm_desc->bGUID, mbm_guid, 16)) {
-               dev_dbg(&intf->dev, "GUID doesn't match\n");
+               dev_err(&intf->dev, "GUID doesn't match\n");
                goto bad_desc;
        }
 
        if (header.usb_cdc_mdlm_detail_desc &&
                header.usb_cdc_mdlm_detail_desc->bLength <
                        (sizeof(struct usb_cdc_mdlm_detail_desc) + 1)) {
-               dev_dbg(&intf->dev, "Descriptor too short\n");
+               dev_err(&intf->dev, "Descriptor too short\n");
                goto bad_desc;
        }
 
@@ -267,7 +272,7 @@ skip:
                info->control = usb_ifnum_to_if(dev->udev, 0);
                info->data = usb_ifnum_to_if(dev->udev, 1);
                if (!info->control || !info->data || info->control != intf) {
-                       dev_dbg(&intf->dev,
+                       dev_err(&intf->dev,
                                "rndis: master #0/%p slave #1/%p\n",
                                info->control,
                                info->data);
@@ -275,7 +280,7 @@ skip:
                }
 
        } else if (!info->header || (!rndis && !info->ether)) {
-               dev_dbg(&intf->dev, "missing cdc %s%s%sdescriptor\n",
+               dev_err(&intf->dev, "missing cdc %s%s%sdescriptor\n",
                        info->header ? "" : "header ",
                        info->u ? "" : "union ",
                        info->ether ? "" : "ether ");
@@ -287,16 +292,15 @@ skip:
         */
        if (info->data != info->control) {
                status = usb_driver_claim_interface(driver, info->data, dev);
-               if (status < 0)
-                       return status;
+               if (status < 0) {
+                       dev_err(&intf->dev, "Second interface unclaimable\n");
+                       goto bad_desc;
+               }
        }
        status = usbnet_get_endpoints(dev, info->data);
        if (status < 0) {
-               /* ensure immediate exit from usbnet_disconnect */
-               usb_set_intfdata(info->data, NULL);
-               if (info->data != info->control)
-                       usb_driver_release_interface(driver, info->data);
-               return status;
+               dev_dbg(&intf->dev, "Mandatory endpoints missing\n");
+               goto bail_out_and_release;
        }
 
        /* status endpoint: optional for CDC Ethernet, not RNDIS (or ACM) */
@@ -316,10 +320,9 @@ skip:
                }
        }
        if (rndis && !dev->status) {
-               dev_dbg(&intf->dev, "missing RNDIS status endpoint\n");
-               usb_set_intfdata(info->data, NULL);
-               usb_driver_release_interface(driver, info->data);
-               return -ENODEV;
+               dev_err(&intf->dev, "missing RNDIS status endpoint\n");
+               status = -ENODEV;
+               goto bail_out_and_release;
        }
 
        /* override ethtool_ops */
@@ -327,9 +330,12 @@ skip:
 
        return 0;
 
+bail_out_and_release:
+       usb_set_intfdata(info->data, NULL);
+       if (info->data != info->control)
+               usb_driver_release_interface(driver, info->data);
 bad_desc:
-       dev_info(&dev->udev->dev, "bad CDC descriptors\n");
-       return -ENODEV;
+       return status;
 }
 EXPORT_SYMBOL_GPL(usbnet_generic_cdc_bind);