On Tue, Mar 26, 2013 at 05:58:43PM +0100, Michael Grzeschik wrote:
> There is no need to call ep_queue unlocked inside the own driver. We
> move its functionionality into an unlocked version.
>
> This patch removes potential unlocked timeslots inside
> isr_setup_status_phase and isr_get_status_response, in which the lock
nice :-)
> got released just before acquired again inside usb_ep_queue.
>
> Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
> Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
> ---
> Changes since v1:
> - changed patch description as mentioned by alexander
>
> drivers/usb/chipidea/udc.c | 121 +++++++++++++++++++++++++--------------------
> 1 file changed, 67 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index cab349b..2257320 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -656,6 +656,70 @@ static void isr_get_status_complete(struct usb_ep *ep, struct usb_request *req)
> }
>
> /**
> + * _ep_queue: queues (submits) an I/O request to an endpoint
> + *
> + * Caller must hold lock
> + */
> +static int _ep_queue(struct usb_ep *ep, struct usb_request *req,
> + gfp_t __maybe_unused gfp_flags)
> +{
> + struct ci13xxx_ep *mEp = container_of(ep, struct ci13xxx_ep, ep);
> + struct ci13xxx_req *mReq = container_of(req, struct ci13xxx_req, req);
> + struct ci13xxx *ci = mEp->ci;
> + int retval = 0;
> +
> + if (ep == NULL || req == NULL || mEp->ep.desc == NULL)
> + return -EINVAL;
don't you want to, perhaps, WARN() about these errors ? If any of these
are true, it means someone's violating the API. Gadget drivers shouldn't
queue to disabled endpoints nor to NULL endpoints or NULL requests :-p
BTW, someone needs to give all the chipidea functions a common prefix,
it makes it a lot easier to setup function tracer (by using wildcards,
e.g. ci_*).
--
balbi
Attachment:
signature.asc
Description: Digital signature
