Re: [PATCH 010/143] musb_host: refactor URB giveback

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

 



On Tue, Jun 30, 2009 at 08:59:18PM +0400, Sergei Shtylyov wrote:
> 
> Greg KH wrote:
> 
> >>>From: Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx>
> 
> >>>As musb_advance_schedule() is now the only remaning
> >>>caller of musb_giveback() (and the only valid context
> >>>of such call), just fold the latter into the former
> >>>and then rename __musb_giveback() into musb_giveback().
> 
> >>>This is a net minor shrink.
> 
> >>>Signed-off-by: Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx>
> >>>Signed-off-by: David Brownell <dbrownell@xxxxxxxxxxxxxxxxxxxxx>
> >>>Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxx>
> 
> >>    Sigh... :-(
> 
> >>>diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
> >>>index e666f60..c1bb192 100644
> >>>--- a/drivers/usb/musb/musb_host.c
> >>>+++ b/drivers/usb/musb/musb_host.c
> 
> >>[...]
> 
> >>>@@ -350,14 +349,22 @@ static inline void musb_save_toggle(struct musb_qh *qh, int is_in,
> >>> 	usb_settoggle(urb->dev, qh->epnum, !is_in, csr ? 1 : 0);
> >>> }
> >>> 
> >>>-/* caller owns controller lock, irqs are blocked */
> >>>-static struct musb_qh *
> >>>-musb_giveback(struct musb_qh *qh, struct urb *urb, int status)
> >>>+/*
> >>>+ * Advance this hardware endpoint's queue, completing the specified URB and
> >>>+ * advancing to either the next URB queued to that qh, or else invalidating
> >>>+ * that qh and advancing to the next qh scheduled after the current one.
> >>>+ *
> >>>+ * Context: caller owns controller lock, IRQs are blocked
> >>>+ */
> >>>+static void musb_advance_schedule(struct musb *musb, struct urb *urb,
> >>>+				  struct musb_hw_ep *hw_ep, int is_in)
> >>> {
> >>>+	struct musb_qh		*qh = musb_ep_get_qh(hw_ep, is_in);
> >>> 	struct musb_hw_ep	*ep = qh->hw_ep;
> >>>-	struct musb		*musb = ep->musb;
> >>>-	int			is_in = usb_pipein(urb->pipe);
> >>> 	int			ready = qh->is_ready;
> >>>+	int			status;
> >>>+
> >>>+	status = (urb->status == -EINPROGRESS) ? 0 : urb->status;
> >>> 
> >>> 	/* save toggle eagerly, for paranoia */
> >>> 	switch (qh->type) {
> >>>@@ -366,13 +373,13 @@ musb_giveback(struct musb_qh *qh, struct urb *urb, int status)
> >>> 		musb_save_toggle(qh, is_in, urb);
> >>> 		break;
> >>> 	case USB_ENDPOINT_XFER_ISOC:
> >>>-		if (status == 0 && urb->error_count)
> >>>+		if (urb->error_count)
> 
> >>    I've posted the take 2 patch (in time for this one to be updated) that 
> >>had this illegetimate change removed but it was (of course) ignored. What I 
> >>am to do now -- send a new patch undoing the change? :-/
> 
> > Care to send the diff of what you want to see the code to look like now?
> 
>     I'm not sure what it'll buy now that this code has been merged but here 
> you are:
> 
> diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
> index e666f60..c1bb192 100644
> --- a/drivers/usb/musb/musb_host.c
> +++ b/drivers/usb/musb/musb_host.c
> @@ -373,13 +373,13 @@ musb_giveback(struct musb_qh *qh, struct urb *urb, int
>   		musb_save_toggle(qh, is_in, urb);
>   		break;
>   	case USB_ENDPOINT_XFER_ISOC:
> -		if (urb->error_count)
> +		if (status == 0 && urb->error_count)
>   			status = -EXDEV;
>   		break;
>   	}

Can you send it in a format that explains the change and it can be
applied, with proper signed-off-by and everything?

Otherwise it's useless :(

thanks,

greg k-h
--
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