Re: [PATCH v2] usb: mtu3: Convert to platform remove callback returning void

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 1550 bytes --]

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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]