Re: [PATCH 3/5] usbdevfs: Don't advertise USBDEVFS_CAP_BULK_CONTINUATION on XHCI attached devs

[Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>]

On Mon, Jul 02, 2012 at 11:09:26AM +0200, Hans de Goede wrote:
> As discussed in length on the mailinglist, USBDEVFS_URB_BULK_CONTINUATION
> does not work as it should when combined with USBDEVFS_URB_SHORT_NOT_OK
> (which is its intended use) on devices attached to an XHCI controller.
> 
> Note that this patch only does not advertise the cap, bulk transfers
> with USBDEVFS_URB_BULK_CONTINUATION will still be accepted. Returning
> -EINVAL for them would break existing apps, and in most cases the
> troublesome scenario wrt USBDEVFS_URB_SHORT_NOT_OK urbs on XHCI controllers
> will never get hit, so this would break working use cases.
> 
> The disadvantage of not returning -EINVAL is that cases were it is causing
> real trouble may go undetected / the cause of the trouble may be unclear,
> but this is the best we can do.
> 
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>

Acked-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>

Let me know if Greg wants this patch to come through my queue or not.


> ---
>  drivers/usb/core/devio.c |    3 ++-
>  drivers/usb/host/xhci.c  |    2 ++
>  include/linux/usb.h      |    5 +++++
>  3 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> index 3807ea6..b3a5e2b 100644
> --- a/drivers/usb/core/devio.c
> +++ b/drivers/usb/core/devio.c
> @@ -1823,7 +1823,8 @@ static int proc_get_capabilities(struct dev_state *ps, void __user *arg)
>  	__u32 caps;
>  
>  	caps = USBDEVFS_CAP_ZERO_PACKET | USBDEVFS_CAP_NO_PACKET_SIZE_LIM;
> -	caps |= USBDEVFS_CAP_BULK_CONTINUATION;
> +	if (!ps->dev->bus->no_stop_on_short)
> +		caps |= USBDEVFS_CAP_BULK_CONTINUATION;
>  
>  	if (put_user(caps, (__u32 __user *)arg))
>  		return -EFAULT;
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 36641a7..bde855f 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -3974,6 +3974,8 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
>  
>  	/* Accept arbitrarily long scatter-gather lists */
>  	hcd->self.sg_tablesize = ~0;
> +	/* XHCI controllers don't stop the ep queue on short packets :| */
> +	hcd->self.no_stop_on_short = 1;
>  
>  	if (usb_hcd_is_primary_hcd(hcd)) {
>  		xhci = kzalloc(sizeof(struct xhci_hcd), GFP_KERNEL);
> diff --git a/include/linux/usb.h b/include/linux/usb.h
> index 73b68d1..748ca86 100644
> --- a/include/linux/usb.h
> +++ b/include/linux/usb.h
> @@ -331,6 +331,11 @@ struct usb_bus {
>  	u8 otg_port;			/* 0, or number of OTG/HNP port */
>  	unsigned is_b_host:1;		/* true during some HNP roleswitches */
>  	unsigned b_hnp_enable:1;	/* OTG: did A-Host enable HNP? */
> +	unsigned no_stop_on_short:1;    /*
> +					 * Quirk: some controllers don't stop
> +					 * the ep queue on a short transfer
> +					 * with the URB_SHORT_NOT_OK flag set.
> +					 */
>  	unsigned sg_tablesize;		/* 0 or largest number of sg list entries */
>  
>  	int devnum_next;		/* Next open device number in
> -- 
> 1.7.10.4
> 
> --
> 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
--
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