On Wed, Feb 10, 2010 at 08:08:09PM +0530, Ramya Desai wrote: > > I've just pushed some fixes to the xhci-streams branch. > > I haven't tried your updated kernel. Please find my comments. > > I have checked the allocated number of streams as a return value from > the usb_alloc_streams API. It returned the correct value. However, > the end points are not configured properly for the streams. There are > also some TYPO in the xHCI driver sources which are related to the > streams. Please find the modifications below which I have done in your > xHCI driver sources. Ramya, Thanks for the list of modifications. It would be much more helpful if you could use the diff command to generate a patch. Reading Documentation/SubmittingPatches in the kernel source tree is a good place to start. There are other resources in the Documentation directory that are useful, and http://kernelnewbies.org/ is also useful. John, Thanks for your bug patch. It looks like someone has independently found the same bugs. Can you send patches to me sooner? I know that companies are very competitive to get their USB 3.0 devices and IP to market, but it would be more efficient if we collaborated on bug fixes, changes to the USB core, and new drivers. "Given enough eyeballs, all bugs are shallow." Sarah Sharp > 1) In xhci_check_streams_endpoint function > ret = xhci_check_args(xhci_to_hcd(xhci), udev, ep, 1, __func__); > if (ret >= 0) > return -EINVAL; > > Changed to > > ret = xhci_check_args(xhci_to_hcd(xhci), udev, ep, 1, __func__); > if (ret <= 0) > return -EINVAL; > > The return value from the xhci_check_args is always either 0 or less > than 0 only. > > 2) In xhci_alloc_streams function > for (i = 0; i < num_eps; i++) { > ep_index = xhci_get_endpoint_index(&eps[i]->desc); > vdev->eps[ep_index].ep_state &= ~EP_GETTING_STREAMS; > if (ret < 0) { > xhci_dbg(xhci, "Slot > %u ep ctx %u now has streams.\n", > > udev->slot_id, ep_index); > > vdev->eps[ep_index].ep_state |= EP_HAS_STREAMS; > } > } > > Changed to > > for (i = 0; i < num_eps; i++) { > ep_index = > xhci_get_endpoint_index(&eps[i]->desc); > vdev->eps[ep_index].ep_state &= > ~EP_GETTING_STREAMS; > if (ret == 0) { > xhci_dbg(xhci, "Slot > %u ep ctx %u now has streams.\n", > > udev->slot_id, ep_index); > > vdev->eps[ep_index].ep_state |= EP_HAS_STREAMS; > } > } > > As there is a IF condition before this for loop which checks the ret > value like if (ret < 0). Due to this, the end points are NOT > configured for the STREAMS. > > 3) In xhci_alloc_stream_info function > ret = > radix_tree_insert(&stream_info->trb_address_map, > key, cur_ring); > if (!ret ) { > xhci_ring_free(xhci, cur_ring); > > stream_info->stream_rings[cur_stream] = NULL; > goto cleanup_rings; > } > > Changed to > > ret = > radix_tree_insert(&stream_info->trb_address_map, > key, cur_ring); > if (ret != 0) { > xhci_ring_free(xhci, cur_ring); > > stream_info->stream_rings[cur_stream] = NULL; > goto cleanup_rings; > } > > As radix_tree_insert returns 0 on SUCCESS. For more details, see > ...lib/radix-tree.c file. > > 4) In xhci_alloc_stream_info function > if (!xhci_test_radix_tree(xhci, num_streams, stream_info)) > { > goto cleanup_rings; > } > > Changed to > > if (xhci_test_radix_tree(xhci, num_streams, stream_info)) > { > goto cleanup_rings; > } > > As the function returns 0 for success, I don't wanted to go for > cleanup, This made me to remove !. > > After the above modifications, we are able to work with the streams > with our device. The streams are working fine for Queue Depth one. > However, for the queue depth greater than 1, still I am facing some > issues. These may be related to the Host Controller / driver / device. > > Regards, > RD > -- > 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
