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

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Fri, Oct 20, 2023 at 11:53:25AM +0200, Uwe Kleine-König wrote:
> Hello Greg,
> 
> On Thu, Oct 05, 2023 at 09:13:18AM +0200, Greg Kroah-Hartman wrote:
> > On Mon, Oct 02, 2023 at 11:41:58PM +0200, Uwe Kleine-König wrote:
> > > On Mon, Oct 02, 2023 at 04:53:05PM +0200, Greg Kroah-Hartman wrote:
> > > > On Mon, Oct 02, 2023 at 04:49:59PM +0200, Uwe Kleine-König wrote:
> > > > > On Mon, Oct 02, 2023 at 04:39:47PM +0200, Greg Kroah-Hartman wrote:
> > > > > > On Thu, Sep 14, 2023 at 10:02:51PM +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);
> > > > > > 
> > > > > > Please don't add new WARN_ON() calls to the kernel, print out a big
> > > > > > error message and return, don't reboot the machine.
> > > > > 
> > > > > Huh, printing out an loud error message was my intention. It's news to
> > > > > me that WARN_ON() reboots the machine?! I thought BUG_ON() was the one
> > > > > with the effects you describe that I shouldn't use.
> > > > 
> > > > panic-on-warn is set for zillions[1] of Linux systems out there, so systems
> > > > will reboot.
> > > 
> > > The people enabling panic-on-warn *ask* for a reboot if something
> > > strange happens, right? If ssusb->dr_mode is USB_DR_MODE_UNKNOWN in
> > > .remove() but wasn't in .probe(), that's strange, right? If I don't
> > > enable panic-on-warn, my system just emits a warning and then the driver
> > > copes with what it has, right? Sounds to me as if WARN_ON does exactly
> > > what is the right thing here.
> > 
> > I really don't want to add more WARN_ON() to the kernel if at all
> > possible.
> > 
> > If this "can not happen" then just don't even add code for it, why have
> > this at all?  The compiler warning can be handled a different way,
> > right?
> 
> Sure, what do you suggest? A dev_warn()?

Sure.  but again, if this can not happen, why do anything?

> ..ooOO(But maybe there are people who reboot on dev_warn(), too?)

No, that's not a thing, thankfully :)

greg k-h