svn commit: r219845 - head/sys/dev/usb/controller

[Gavin Atkinson]

Hi,


On Mon, 2011-03-21 at 21:16 +0000, Hans Petter Selasky wrote:
> Date: Mon Mar 21 21:16:25 2011
> New Revision: 219845
> URL: http://svn.freebsd.org/changeset/base/219845
> 
> Log:
>   - Bugfix: Fix a EHCI hardware race, where the hardware computed data toggle
>   value is updated after that we read it in the queue-head. This patch can
>   fix problems with BULK timeouts. The issue was found on a Nvidia chipset.
> 
> Modified:
>   head/sys/dev/usb/controller/ehci.c
> 
> Modified: head/sys/dev/usb/controller/ehci.c
> ==============================================================================
> --- head/sys/dev/usb/controller/ehci.c	Mon Mar 21 21:16:12 2011	(r219844)
> +++ head/sys/dev/usb/controller/ehci.c	Mon Mar 21 21:16:25 2011	(r219845)
> @@ -1180,6 +1180,26 @@ _ehci_remove_qh(ehci_qh_t *sqh, ehci_qh_
>  	return (last);
>  }
>  
> +static void
> +ehci_data_toggle_update(struct usb_xfer *xfer, uint16_t actlen, uint16_t xlen)
> +{
> +	uint8_t full = (actlen == xlen);

style(9) strongly advises against initialising variables in the
declaration.

> +	uint8_t dt;
> +
> +	/* count number of full packets */
> +	dt = (actlen / xfer->max_packet_size) & 1;
> +
> +	/* cumpute remainder */
> +	actlen = actlen % xfer->max_packet_size;
> +
> +	if (actlen > 0)
> +		dt ^= 1;	/* short packet at the end */
> +	else if (!full)

Especially in this case - this would seem to be better written as

	else if (actlen != xlen)

> +		dt ^= 1;	/* zero length packet at the end */
> +
> +	xfer->endpoint->toggle_next ^= dt;
> +}

As an aside, over the years there are several PRs about bulk timeouts on
EHCI on nVidia - is it possible that this fixes any of them?  Some of
the PRs date back to 6.x and 7.x - if I'm understanding the change
properly, it seems that it is possible that this could be the cause?  Do
you have any opinions on whether this could fix any of those PRs?

Thanks,

Gavin

-- 
Gavin Atkinson
FreeBSD committer and bugmeister
GPG: A093262B (313A A79F 697D 3A5C 216A  EDF5 935D EF44 A093 262B)