Vernon Sauder <vernoninhand@xxxxxxxxx> writes:
> From: Vernon Sauder <vsauder@xxxxxxxxxx>
>
> If a configuration change results in an endpoint being de-commissioned,
> clean it up before deconfiguring it so the gadgets don't hang when
> removed.
> diff --git a/drivers/usb/gadget/pxa27x_udc.c b/drivers/usb/gadget/pxa27x_udc.c
> index 4b5ac47..a896431 100644
> --- a/drivers/usb/gadget/pxa27x_udc.c
> +++ b/drivers/usb/gadget/pxa27x_udc.c
> @@ -19,6 +19,9 @@
> * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> *
> */
> +
> +/* #define VERBOSE_DEBUG */
> +
A leftover maybe ?
<snip>
> @@ -405,8 +409,16 @@ static void update_pxa_ep_matches(struct pxa_udc *udc)
>
> for (i = 1; i < NR_USB_ENDPOINTS; i++) {
> udc_usb_ep = &udc->udc_usb_ep[i];
> - if (udc_usb_ep->pxa_ep)
> - udc_usb_ep->pxa_ep = find_pxa_ep(udc, udc_usb_ep);
> + if (udc_usb_ep->pxa_ep) {
> + struct pxa_ep *ep = find_pxa_ep(udc, udc_usb_ep);
Please, no declaration in code flow. Take that "struct pxa_ep *ep" to the
beginning of the function.
> + if (ep) {
> + udc_usb_ep->pxa_ep = ep;
> + }
> + else {
> + ep_warn(udc_usb_ep->pxa_ep, "new config drops ep; disabling now\n");
> + pxa_ep_disable(&udc_usb_ep->usb_ep);
> + }
> + }
> }
> }
I think coding style requires something like :
if (ep) {
udc_usb_ep->pxa_ep = ep;
} else {
ep_warn(udc_usb_ep->pxa_ep, "new config drops ep; disabling now\n");
pxa_ep_disable(&udc_usb_ep->usb_ep);
}
And then, you'll have to explain to me why this is necessary.
The normal path, as I understand it, is :
- reconf irq arrives
- irq_udc_reconfig() is called
=> pxa27x_change_interface() is called
=> gadget.setup() fonction is called
=> and then, update_pxa_ep_matches() is called after
My understanding of the gadget API is that it is the "setup()" duty to disable
unecessary endpoints by calling pxa_ep_disable() when needed.
Now, if you've thought of a case where the gadget API does not behave that way,
perhaps due to lack of knowledge of pxa27x_udc internal "mess" of pxa_ep and
usb_ep matching, please explain it to me and we'll talk about that chunk again.
Cheers.
--
Robert
--
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
