Hi, On 07/06/2012 07:52 PM, Greg Kroah-Hartman wrote:
On Wed, Jul 04, 2012 at 09:18:04AM +0200, Hans de Goede wrote:We don't support sg for isoc transfers, enforce this. Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> --- drivers/usb/core/hcd.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 140d3e1..3d2f48f 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -1389,7 +1389,12 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb, && !(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)) { if (hcd->self.uses_dma) { if (urb->num_sgs) { - int n = dma_map_sg( + int n; + + /* We don't support sg for isoc transfers ! */ + BUG_ON(usb_endpoint_xfer_isoc(&urb->ep->desc));No, sorry, I will not take new BUG_ON() calls in any drivers. You just crashed a machine, and there is no way that a user can recover. What are they supposed to do here? Who are they going to tell to fix the issue? I can see a WARN_ON() and then handle the error properly, but don't crash a box, that's just rude.
Well this bug_on is intended for driver authors, so that users never get hit by this because it is catched during development. The problem is that currently trying to sg with isoc transfers fails silently (all the data gets dma-ed to / from physical address 0). So the idea is that if a driver author makes the same mistake I did while working on the scatter-gather support for usbfs (where I tried to use it for isoc initially too), they don't end up scratching their head as long as I did :) With that said / explained I completely agree. I guess a WARN_ON + error return is also better for developers, as with a panic the actual reason may well have scrolled off screen / data may be lost etc. So I'll respin this one. Regards, Hans -- 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
