Hello, On Tue, Sep 19, 2023 at 07:52:04AM +0000, Chunfeng Yun (云春峰) wrote: > On Thu, 2023-09-14 at 22:02 +0200, Uwe Kleine-König wrote: > > @@ -469,8 +469,17 @@ static int mtu3_remove(struct platform_device > > *pdev) > > ssusb_gadget_exit(ssusb); > > ssusb_host_exit(ssusb); > > break; > > -default: > > -return -EINVAL; > > +case USB_DR_MODE_UNKNOWN: > > +/* > > + * This cannot happen because with dr_mode == > > + * USB_DR_MODE_UNKNOWN, .probe() doesn't succeed and so > > + * .remove() wouldn't be called at all. However (little > > + * surprising) the compiler isn't smart enough to see that, so > > + * we explicitly have this case item to not make the compiler > > + * wail about an unhandled enumeration value. > > + */ > > +WARN_ON(1); > > +break; > How about changing as below: > defualt: > break; > > } I think a warning is a good idea as today that case cannot happen (unless I missed something) and if it still happened, you'd want to know as the handling is insufficient then. And I also think that if the enum usb_dr_mode should ever be expanded, this code location should be revisited, so the explicit "case USB_DR_MODE_UNKNOWN" is better in my opinion. As you suggest this variant you seem to have some upside in mind, didn't put it into your message though. Would you share your thoughts? Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
