On Fri, 2019-07-05 at 14:28 -0400, Alan Stern wrote:
> On Fri, 5 Jul 2019, Benjamin Herrenschmidt wrote:
>
> > (following our conversation)
> >
> > Here's a completely untested alternative patch (it replaces my previous
> > one) that fixes it a bit differently.
> >
> > This time it should handle the case of a disconnect happening
> > before we have dequeued a config change.
> >
> > This assumes that it's correct to never call
> > usb_composite_setup_continue() if an fsg_disable() happens after a
> > fsg_set_alt() and before we have processed the latter.
>
> That should be handled okay. If it isn't, the composite core needs to
> be fixed.
Ok. I'll have a quick look to make sure.
.../...
> Yes, this looks just right. If I had thought about this a little more
> deeply earlier on, I would have come up with a patch very much like
> this.
Right, so as I grow more familiar with that code and its intent, I
agree, I'm much happier with this. Hopefully it passes my tests. I'll
tidy up as per your comments and repost properly if all goes well along
with some other things I piled up.
Cheers,
Ben.
> My only comments are cosmetic.
>
> > ---
> > drivers/usb/gadget/function/f_mass_storage.c | 26 ++++++++++++--------
> > 1 file changed, 16 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
> > index 043f97ad8f22..2ef029413b01 100644
> > --- a/drivers/usb/gadget/function/f_mass_storage.c
> > +++ b/drivers/usb/gadget/function/f_mass_storage.c
>
> > @@ -2285,16 +2292,14 @@ static int do_set_interface(struct fsg_common *common, struct fsg_dev *new_fsg)
> > static int fsg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
> > {
> > struct fsg_dev *fsg = fsg_from_func(f);
>
> While you're changing this, it would be nice to add the customary blank
> line here.
>
> > - fsg->common->new_fsg = fsg;
> > - raise_exception(fsg->common, FSG_STATE_CONFIG_CHANGE);
> > + __raise_exception(fsg->common, FSG_STATE_CONFIG_CHANGE, fsg);
> > return USB_GADGET_DELAYED_STATUS;
> > }
> >
> > static void fsg_disable(struct usb_function *f)
> > {
> > struct fsg_dev *fsg = fsg_from_func(f);
>
> And here. Otherwise:
>
> Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
>
> Alan Stern
