Re: Potential fsg->state problem at file_storage.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 15 Aug 2012, Felipe Balbi wrote:

> > A better solution would be to have separate gadget driver callbacks for
> > Reset and Disconnect.  Then we could do the fsg_lun_fsync_sub() only
> > during disconnect, not during reset.
> > 
> > Alternatively, we could keep the single existing Disconnect callback
> > and give the gadget driver a way to ask whether or not VBus power is
> > present.  Then we could do the fsg_lun_fsync_sub() only when there is
> > no power.
> > 
> > Either one of these would be a major change to the Gadget API, but they
> > seem like reasonable things to add.
> > 
> > Felipe, what do you think?
> 
> I would feel better about splitting disconnect from reset. We already
> have a weird vbus_session callback which is actually the other way
> around (it's used by a gadget driver to tell controller vbus is alive,
> go figure). It should be fairly simple to implement and have less impact
> on the controllers themselves. But as you say, it'll be quite invasive,
> so we should try to make sure that gets properly tested.
> 
> I could keep it in my tree on a separate branch and only send upstream
> if I get enough Acked-bys. How does that sound ?

Okay, good.

> FYI, dwc3 would look like this:
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index e09a7c4..723a530 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1846,6 +1846,15 @@ static void dwc3_disconnect_gadget(struct dwc3 *dwc)
>  	}
>  }
>  
> +static void dwc3_reset_gadget(struct dwc3 *dwc)
> +{
> +	if (dwc->gadget_driver && dwc->gadget_driver->reset) {
> +		spin_unlock(&dwc->lock);
> +		dwc->gadget_driver->reset(&dwc->gadget);
> +		spin_lock(&dwc->lock);
> +	}
> +}

Should we try to maintain backward compatibility by invoking the
disconnect callback during a reset if the gadget driver doesn't have a
reset callback?  To tell the truth, I don't know to what extent most
gadget drivers need to know the difference between reset and 
disconnect.

> @@ -2021,8 +2030,7 @@ static void dwc3_gadget_reset_interrupt(struct dwc3 *dwc)
>  		dwc3_gadget_usb3_phy_suspend(dwc, false);
>  	}
>  
> -	if (dwc->gadget.speed != USB_SPEED_UNKNOWN)
> -		dwc3_disconnect_gadget(dwc);
> +	dwc3_reset_gadget(dwc);
>  
>  	reg = dwc3_readl(dwc->regs, DWC3_DCTL);
>  	reg &= ~DWC3_DCTL_TSTCTRL_MASK;

I'll check to see about making a similar change to net2280.  dummy-hcd 
should be fairly easy.

> When doing that, I thought we could also drop the "don't call disconnect
> if we get a reset when gadget never enumerated" thing. What do you say ?

Is there such a rule?  I wasn't aware of it.  Yes, it seems like a very 
minor sort of optimization; we could easily get rid of it.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux