[PATCH 1/1] usb: dwc3: Remove duplicate inclusion of otg.h
otg.h header file was included twice. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/usb/dwc3/core.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 577af1b..474162e 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -33,7 +33,6 @@ #include linux/dma-mapping.h #include linux/of.h -#include linux/usb/otg.h #include linux/usb/ch9.h #include linux/usb/gadget.h #include linux/usb/of.h -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [PATCH-SR9700] Merge USB 1.1 Ethernet Adapter SR9700 DeviceDriver into the Linux Kernel
On Wed, Aug 21, 2013 at 06:02:45PM +0800, liujunliang_ljl wrote: Dear Ben : 1, please give me email address of David Miller, and Thanks a lot. You can find him in the MAINTAINERS file. He should also show up if you use scripts/get_maintainer.pl -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] USB: EHCI: disable IAA_WATCHDOG and START_UNLINK_INTR if they needn't to be handled
On Wed, Aug 21, 2013 at 11:23 PM, Alan Stern st...@rowland.harvard.edu wrote: On Wed, 21 Aug 2013, Ming Lei wrote: By the way, even though it's a little late to ask this... Why did you decide to move only the giveback routine into a tasklet, instead of moving the entire interrupt handler? Looks below reasons in my mind before preparing the tasklet patch: 1, from my observation, on ARM, the most time-consuming part is dma mapping, unmapping and data copy over coherent memory, so moving giveback out of interrupt handler can decrease USB irq handling time a lot. Of course, this would remain true either way. 2, moving giveback out of irq handler can be done in usbcore, so changes can be minimized. This seems to be the most important factor. When you think about it, though, does it really minimize the changes? Consider all the other adjustments we had to make to ehci-hcd: the interrupt QH unlink change and the isochronous stream stuff (which also affects the usb-audio drivers). For other HCDs, this changes on interrupt QH unlink may not need at all if there is no much cost about relink. About isochronous stream stuff, the only change with moving giveback in tasklet is that iso_stream_schedule() may see list_empty(stream-td_list) when underrun, and we can solve it easily, can't we? Also I remembered that you said the isochronous stream stuff needn't to consider on other HCDs. So maybe the change on ehci HCD is a bit much, but for other HCDs, the change is just a little. Another good point with moving giveback only to tasklet is deadlock immune during resubmissions, as you mentioned in below link: http://www.spinics.net/lists/linux-usb/msg88710.html I'm starting to think that moving the entire handler to a tasklet would have been better. Not sure, if so: - one thing is that the HCD private lock can't be held in interrupt handler any more, so new lock need to introduce, not mention each hard irq handler need to split to two parts. - also it should have been better to avoid resubmissions in its giveback handler. 3, driver's complete() may do many driver specific things which may increase irq handling time randomly, so moving complete() to tasklet can help to decrease HCD irq handling time. This is like the first reason. Also moving only the giveback routine into a tasklet can avoid dropping HCD private lock during irq handler, which may simplify HCD code, and I have figured out ehci cleanup patches for this. I don't think we should make these changes. It's okay to keep the private lock during the giveback call, but let's not remove the other things. My feeling is that at some point we may indeed want to move the entire handler to a tasklet or a threaded IRQ. If that happens, all those simplifications would need to be undone. Better not to do them in the first place, since they add very little overhead. I think making HCDs simpler should have been welcome, :-) Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] USB: EHCI: disable IAA_WATCHDOG and START_UNLINK_INTR if they needn't to be handled
On Thu, Aug 22, 2013 at 4:58 PM, Ming Lei ming@canonical.com wrote: On Wed, Aug 21, 2013 at 11:23 PM, Alan Stern st...@rowland.harvard.edu wrote: I'm starting to think that moving the entire handler to a tasklet would have been better. Not sure, if so: - one thing is that the HCD private lock can't be held in interrupt handler any more, so new lock need to introduce, not mention each hard irq handler need to split to two parts. Also one potential big problem of moving entire handler to tasklet is the hardware race, previously the entire hander is run with interrupt disabled and the big HCD private lock held, but after the conversion, both are not true, so each HCD change for this conversion might be big. If only moving giveback into tasklet, there is no such problem. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Trying to develop a Pixelsense kernel driver
Hello everyone, I recently wanted to pick up again on developing a kernel input driver for the Microsoft Pixelsense (formerly Surface 2.0). I already have a working user space driver (see also [1]). However - just like last year [2] - I quickly ran into an issue with a corrupted device firmware after some brief experimentation with the kernel driver, while the user space driver didn't and doesn't show any such issues. This happened on two separate Pixelsense devices, with different kernel versions. So even though userspace and kernel driver look like they perform exactly the same operations on the device, there must be some difference I don't understand - perhaps libusb does some kind of internal error handling which the kernel doesn't do? Userspace version: // for control commands usb_control_msg(handle, 0xC0, cmd, 0x00, index, (char*)buf, len, 1000); // for bulk data transfer uint8_t buffer[512]; result = usb_bulk_read( handle, 0x86, (char*)(buffer), sizeof(buffer), 1000 ); Kernel version: // for control commands #define surface_command(dev, command, index, buffer, size) usb_control_msg (dev-usbdev, usb_rcvctrlpipe (dev-usbdev, 0), command, USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_IN, 0x00, index, buffer, size, 1000) // for bulk data transfer result = usb_bulk_msg (surface-usbdev, usb_rcvbulkpipe (surface-usbdev, 0x86), surface-bulk_in_buffer, 512, bulk_read, 1000); Can anybody venture a guess what kind of functional difference there is between these code snippets? Thanks BR, Florian [1] https://github.com/floe/surface-2.0/ [2] http://floe.butterbrot.org/matrix/hacking/surface/brick.html -- SENT FROM MY DEC VT50 TERMINAL signature.asc Description: OpenPGP digital signature
Re: [PATCH v1 37/49] media: usb: cx231xx: prepare for enabling irq in complete()
On Sat 17 August 2013 18:25:02 Ming Lei wrote: Complete() will be run with interrupt enabled, so change to spin_lock_irqsave(). Cc: Mauro Carvalho Chehab mche...@redhat.com Cc: Hans Verkuil hans.verk...@cisco.com Cc: linux-me...@vger.kernel.org Signed-off-by: Ming Lei ming@canonical.com Acked-by: Hans Verkuil hans.verk...@cisco.com Regards, Hans --- drivers/media/usb/cx231xx/cx231xx-audio.c | 10 ++ drivers/media/usb/cx231xx/cx231xx-core.c | 10 ++ drivers/media/usb/cx231xx/cx231xx-vbi.c |5 +++-- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/drivers/media/usb/cx231xx/cx231xx-audio.c b/drivers/media/usb/cx231xx/cx231xx-audio.c index 81a1d97..f6fa0af 100644 --- a/drivers/media/usb/cx231xx/cx231xx-audio.c +++ b/drivers/media/usb/cx231xx/cx231xx-audio.c @@ -136,6 +136,7 @@ static void cx231xx_audio_isocirq(struct urb *urb) stride = runtime-frame_bits 3; for (i = 0; i urb-number_of_packets; i++) { + unsigned long flags; int length = urb-iso_frame_desc[i].actual_length / stride; cp = (unsigned char *)urb-transfer_buffer + @@ -158,7 +159,7 @@ static void cx231xx_audio_isocirq(struct urb *urb) length * stride); } - snd_pcm_stream_lock(substream); + snd_pcm_stream_lock_irqsave(substream, flags); dev-adev.hwptr_done_capture += length; if (dev-adev.hwptr_done_capture = @@ -173,7 +174,7 @@ static void cx231xx_audio_isocirq(struct urb *urb) runtime-period_size; period_elapsed = 1; } - snd_pcm_stream_unlock(substream); + snd_pcm_stream_unlock_irqrestore(substream, flags); } if (period_elapsed) snd_pcm_period_elapsed(substream); @@ -224,6 +225,7 @@ static void cx231xx_audio_bulkirq(struct urb *urb) stride = runtime-frame_bits 3; if (1) { + unsigned long flags; int length = urb-actual_length / stride; cp = (unsigned char *)urb-transfer_buffer; @@ -242,7 +244,7 @@ static void cx231xx_audio_bulkirq(struct urb *urb) length * stride); } - snd_pcm_stream_lock(substream); + snd_pcm_stream_lock_irqsave(substream, flags); dev-adev.hwptr_done_capture += length; if (dev-adev.hwptr_done_capture = @@ -257,7 +259,7 @@ static void cx231xx_audio_bulkirq(struct urb *urb) runtime-period_size; period_elapsed = 1; } - snd_pcm_stream_unlock(substream); + snd_pcm_stream_unlock_irqrestore(substream,flags); } if (period_elapsed) snd_pcm_period_elapsed(substream); diff --git a/drivers/media/usb/cx231xx/cx231xx-core.c b/drivers/media/usb/cx231xx/cx231xx-core.c index 4ba3ce0..593b397 100644 --- a/drivers/media/usb/cx231xx/cx231xx-core.c +++ b/drivers/media/usb/cx231xx/cx231xx-core.c @@ -798,6 +798,7 @@ static void cx231xx_isoc_irq_callback(struct urb *urb) container_of(dma_q, struct cx231xx_video_mode, vidq); struct cx231xx *dev = container_of(vmode, struct cx231xx, video_mode); int i; + unsigned long flags; switch (urb-status) { case 0: /* success */ @@ -813,9 +814,9 @@ static void cx231xx_isoc_irq_callback(struct urb *urb) } /* Copy data from URB */ - spin_lock(dev-video_mode.slock); + spin_lock_irqsave(dev-video_mode.slock, flags); dev-video_mode.isoc_ctl.isoc_copy(dev, urb); - spin_unlock(dev-video_mode.slock); + spin_unlock_irqrestore(dev-video_mode.slock, flags); /* Reset urb buffers */ for (i = 0; i urb-number_of_packets; i++) { @@ -842,6 +843,7 @@ static void cx231xx_bulk_irq_callback(struct urb *urb) struct cx231xx_video_mode *vmode = container_of(dma_q, struct cx231xx_video_mode, vidq); struct cx231xx *dev = container_of(vmode, struct cx231xx, video_mode); + unsigned long flags; switch (urb-status) { case 0: /* success */ @@ -857,9 +859,9 @@ static void cx231xx_bulk_irq_callback(struct urb *urb) } /* Copy data from URB */ - spin_lock(dev-video_mode.slock); + spin_lock_irqsave(dev-video_mode.slock, flags); dev-video_mode.bulk_ctl.bulk_copy(dev, urb); -
[PATCH] net: usbnet: update addr_assign_type if appropriate
This module generates a common default address on init, using eth_random_addr. Set addr_assign_type to let userspace know the address is random unless it was overridden by the minidriver. Signed-off-by: Bjørn Mork bj...@mork.no --- drivers/net/usb/usbnet.c |4 1 file changed, 4 insertions(+) diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c index e4811d7..74a8e3c 100644 --- a/drivers/net/usb/usbnet.c +++ b/drivers/net/usb/usbnet.c @@ -1629,6 +1629,10 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod) dev-rx_urb_size = dev-hard_mtu; dev-maxpacket = usb_maxpacket (dev-udev, dev-out, 1); + /* let userspace know we have a random address */ + if (ether_addr_equal(net-dev_addr, node_id)) + net-addr_assign_type = NET_ADDR_RANDOM; + if ((dev-driver_info-flags FLAG_WLAN) != 0) SET_NETDEV_DEVTYPE(net, wlan_type); if ((dev-driver_info-flags FLAG_WWAN) != 0) -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 23/49] hid: usbhid: prepare for enabling irq in complete()
On Sun, Aug 18, 2013 at 12:24 AM, Ming Lei ming@canonical.com wrote: Complete() will be run with interrupt enabled, so change to spin_lock_irqsave(). Cc: Jiri Kosina jkos...@suse.cz Cc: linux-in...@vger.kernel.org Signed-off-by: Ming Lei ming@canonical.com Jiri, could you review and give an Ack? Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 24/49] BT: btusb: prepare for enabling irq in complete()
On Sun, Aug 18, 2013 at 12:24 AM, Ming Lei ming@canonical.com wrote: Complete() will be run with interrupt enabled, so change to spin_lock_irqsave(). Cc: Marcel Holtmann mar...@holtmann.org Cc: Gustavo Padovan gust...@padovan.org Cc: Johan Hedberg johan.hedb...@gmail.com Cc: linux-blueto...@vger.kernel.org Signed-off-by: Ming Lei ming@canonical.com Marcel, Gustavo and Johan, could you review and give an Ack if this one and 25/49 are OK so they can be merged via Greg's USB tree? Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 40/49] media: usb: tlg2300: prepare for enabling irq in complete()
On Sat 17 August 2013 18:25:05 Ming Lei wrote: Complete() will be run with interrupt enabled, so change to spin_lock_irqsave(). Cc: Mauro Carvalho Chehab mche...@redhat.com Cc: linux-me...@vger.kernel.org Signed-off-by: Ming Lei ming@canonical.com Acked-by: Hans Verkuil hans.verk...@cisco.com Regards, Hans --- drivers/media/usb/tlg2300/pd-alsa.c |5 +++-- drivers/media/usb/tlg2300/pd-video.c |5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/media/usb/tlg2300/pd-alsa.c b/drivers/media/usb/tlg2300/pd-alsa.c index 3f3e141..65c46a2 100644 --- a/drivers/media/usb/tlg2300/pd-alsa.c +++ b/drivers/media/usb/tlg2300/pd-alsa.c @@ -141,6 +141,7 @@ static inline void handle_audio_data(struct urb *urb, int *period_elapsed) int len = urb-actual_length / stride; unsigned char *cp = urb-transfer_buffer; unsigned int oldptr = pa-rcv_position; + unsigned long flags; if (urb-actual_length == AUDIO_BUF_SIZE - 4) len -= (AUDIO_TRAILER_SIZE / stride); @@ -156,7 +157,7 @@ static inline void handle_audio_data(struct urb *urb, int *period_elapsed) memcpy(runtime-dma_area + oldptr * stride, cp, len * stride); /* update the statas */ - snd_pcm_stream_lock(pa-capture_pcm_substream); + snd_pcm_stream_lock_irqsave(pa-capture_pcm_substream, flags); pa-rcv_position+= len; if (pa-rcv_position = runtime-buffer_size) pa-rcv_position -= runtime-buffer_size; @@ -166,7 +167,7 @@ static inline void handle_audio_data(struct urb *urb, int *period_elapsed) pa-copied_position -= runtime-period_size; *period_elapsed = 1; } - snd_pcm_stream_unlock(pa-capture_pcm_substream); + snd_pcm_stream_unlock_irqrestore(pa-capture_pcm_substream, flags); } static void complete_handler_audio(struct urb *urb) diff --git a/drivers/media/usb/tlg2300/pd-video.c b/drivers/media/usb/tlg2300/pd-video.c index 8df668d..4e5bd07 100644 --- a/drivers/media/usb/tlg2300/pd-video.c +++ b/drivers/media/usb/tlg2300/pd-video.c @@ -151,11 +151,12 @@ static void init_copy(struct video_data *video, bool index) static bool get_frame(struct front_face *front, int *need_init) { struct videobuf_buffer *vb = front-curr_frame; + unsigned long flags; if (vb) return true; - spin_lock(front-queue_lock); + spin_lock_irqsave(front-queue_lock, flags); if (!list_empty(front-active)) { vb = list_entry(front-active.next, struct videobuf_buffer, queue); @@ -164,7 +165,7 @@ static bool get_frame(struct front_face *front, int *need_init) front-curr_frame = vb; list_del_init(vb-queue); } - spin_unlock(front-queue_lock); + spin_unlock_irqrestore(front-queue_lock, flags); return !!vb; } -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 38/49] media: usb: em28xx: prepare for enabling irq in complete()
On Sat 17 August 2013 18:25:03 Ming Lei wrote: Complete() will be run with interrupt enabled, so change to spin_lock_irqsave(). Cc: Mauro Carvalho Chehab mche...@redhat.com Cc: linux-me...@vger.kernel.org Reviewed-by: Devin Heitmueller dheitmuel...@kernellabs.com Tested-by: Devin Heitmueller dheitmuel...@kernellabs.com Signed-off-by: Ming Lei ming@canonical.com Acked-by: Hans Verkuil hans.verk...@cisco.com Regards, Hans --- drivers/media/usb/em28xx/em28xx-core.c |5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c index fc157af..0d698f9 100644 --- a/drivers/media/usb/em28xx/em28xx-core.c +++ b/drivers/media/usb/em28xx/em28xx-core.c @@ -941,6 +941,7 @@ static void em28xx_irq_callback(struct urb *urb) { struct em28xx *dev = urb-context; int i; + unsigned long flags; switch (urb-status) { case 0: /* success */ @@ -956,9 +957,9 @@ static void em28xx_irq_callback(struct urb *urb) } /* Copy data from URB */ - spin_lock(dev-slock); + spin_lock_irqsave(dev-slock, flags); dev-usb_ctl.urb_data_copy(dev, urb); - spin_unlock(dev-slock); + spin_unlock_irqrestore(dev-slock, flags); /* Reset urb buffers */ for (i = 0; i urb-number_of_packets; i++) { -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 39/49] media: usb: sn9x102: prepare for enabling irq in complete()
On Sat 17 August 2013 18:25:04 Ming Lei wrote: Complete() will be run with interrupt enabled, so change to spin_lock_irqsave(). Cc: Mauro Carvalho Chehab mche...@redhat.com Cc: linux-me...@vger.kernel.org Signed-off-by: Ming Lei ming@canonical.com Acked-by: Hans Verkuil hans.verk...@cisco.com Regards, Hans --- drivers/media/usb/sn9c102/sn9c102_core.c |7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/media/usb/sn9c102/sn9c102_core.c b/drivers/media/usb/sn9c102/sn9c102_core.c index 2cb44de..33dc595 100644 --- a/drivers/media/usb/sn9c102/sn9c102_core.c +++ b/drivers/media/usb/sn9c102/sn9c102_core.c @@ -784,12 +784,14 @@ end_of_frame: cam-sensor.pix_format.pixelformat == V4L2_PIX_FMT_JPEG) eof)) { u32 b; + unsigned long flags; b = (*f)-buf.bytesused; (*f)-state = F_DONE; (*f)-buf.sequence= ++cam-frame_count; - spin_lock(cam-queue_lock); + spin_lock_irqsave(cam-queue_lock, + flags); list_move_tail((*f)-frame, cam-outqueue); if (!list_empty(cam-inqueue)) @@ -799,7 +801,8 @@ end_of_frame: frame ); else (*f) = NULL; - spin_unlock(cam-queue_lock); + spin_unlock_irqrestore(cam-queue_lock, +flags); memcpy(cam-sysfs.frame_header, cam-sof.header, soflen); -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 42/49] media: dvb-core: prepare for enabling irq in complete()
On Sat 17 August 2013 18:25:07 Ming Lei wrote: Complete() will be run with interrupt enabled, so change to spin_lock_irqsave(). These functions may be called inside URB-complete(), so use spin_lock_irqsave(). Cc: Mauro Carvalho Chehab mche...@redhat.com Cc: linux-me...@vger.kernel.org Signed-off-by: Ming Lei ming@canonical.com Acked-by: Hans Verkuil hans.verk...@cisco.com Note: Mauro needs to Ack this as well. It looks good to me, but I don't maintain dvb code. Regards, Hans --- drivers/media/dvb-core/dvb_demux.c | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/media/dvb-core/dvb_demux.c b/drivers/media/dvb-core/dvb_demux.c index 3485655..58de441 100644 --- a/drivers/media/dvb-core/dvb_demux.c +++ b/drivers/media/dvb-core/dvb_demux.c @@ -476,7 +476,9 @@ static void dvb_dmx_swfilter_packet(struct dvb_demux *demux, const u8 *buf) void dvb_dmx_swfilter_packets(struct dvb_demux *demux, const u8 *buf, size_t count) { - spin_lock(demux-lock); + unsigned long flags; + + spin_lock_irqsave(demux-lock, flags); while (count--) { if (buf[0] == 0x47) @@ -484,7 +486,7 @@ void dvb_dmx_swfilter_packets(struct dvb_demux *demux, const u8 *buf, buf += 188; } - spin_unlock(demux-lock); + spin_unlock_irqrestore(demux-lock, flags); } EXPORT_SYMBOL(dvb_dmx_swfilter_packets); @@ -519,8 +521,9 @@ static inline void _dvb_dmx_swfilter(struct dvb_demux *demux, const u8 *buf, { int p = 0, i, j; const u8 *q; + unsigned long flags; - spin_lock(demux-lock); + spin_lock_irqsave(demux-lock, flags); if (demux-tsbufp) { /* tsbuf[0] is now 0x47. */ i = demux-tsbufp; @@ -564,7 +567,7 @@ static inline void _dvb_dmx_swfilter(struct dvb_demux *demux, const u8 *buf, } bailout: - spin_unlock(demux-lock); + spin_unlock_irqrestore(demux-lock, flags); } void dvb_dmx_swfilter(struct dvb_demux *demux, const u8 *buf, size_t count) @@ -581,11 +584,13 @@ EXPORT_SYMBOL(dvb_dmx_swfilter_204); void dvb_dmx_swfilter_raw(struct dvb_demux *demux, const u8 *buf, size_t count) { - spin_lock(demux-lock); + unsigned long flags; + + spin_lock_irqsave(demux-lock, flags); demux-feed-cb.ts(buf, count, NULL, 0, demux-feed-feed.ts, DMX_OK); - spin_unlock(demux-lock); + spin_unlock_irqrestore(demux-lock, flags); } EXPORT_SYMBOL(dvb_dmx_swfilter_raw); -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 41/49] media: usb: tm6000: prepare for enabling irq in complete()
On Sat 17 August 2013 18:25:06 Ming Lei wrote: Complete() will be run with interrupt enabled, so change to spin_lock_irqsave(). Cc: Mauro Carvalho Chehab mche...@redhat.com Cc: linux-me...@vger.kernel.org Signed-off-by: Ming Lei ming@canonical.com Acked-by: Hans Verkuil hans.verk...@cisco.com Regards, Hans --- drivers/media/usb/tm6000/tm6000-video.c |5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/media/usb/tm6000/tm6000-video.c b/drivers/media/usb/tm6000/tm6000-video.c index cc1aa14..8bb440f 100644 --- a/drivers/media/usb/tm6000/tm6000-video.c +++ b/drivers/media/usb/tm6000/tm6000-video.c @@ -434,6 +434,7 @@ static void tm6000_irq_callback(struct urb *urb) struct tm6000_dmaqueue *dma_q = urb-context; struct tm6000_core *dev = container_of(dma_q, struct tm6000_core, vidq); int i; + unsigned long flags; switch (urb-status) { case 0: @@ -450,9 +451,9 @@ static void tm6000_irq_callback(struct urb *urb) break; } - spin_lock(dev-slock); + spin_lock_irqsave(dev-slock, flags); tm6000_isoc_copy(urb); - spin_unlock(dev-slock); + spin_unlock_irqrestore(dev-slock, flags); /* Reset urb buffers */ for (i = 0; i urb-number_of_packets; i++) { -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 43/49] media: usb: em28xx: prepare for enabling irq in complete()
On Sat 17 August 2013 18:25:08 Ming Lei wrote: Complete() will be run with interrupt enabled, so add local_irq_save() before acquiring the lock without irqsave(). Cc: Mauro Carvalho Chehab mche...@redhat.com Cc: linux-me...@vger.kernel.org Signed-off-by: Ming Lei ming@canonical.com Acked-by: Hans Verkuil hans.verk...@cisco.com Regards, Hans --- drivers/media/usb/em28xx/em28xx-audio.c |5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/media/usb/em28xx/em28xx-audio.c b/drivers/media/usb/em28xx/em28xx-audio.c index 2fdb66e..7fd1b2a 100644 --- a/drivers/media/usb/em28xx/em28xx-audio.c +++ b/drivers/media/usb/em28xx/em28xx-audio.c @@ -113,6 +113,7 @@ static void em28xx_audio_isocirq(struct urb *urb) stride = runtime-frame_bits 3; for (i = 0; i urb-number_of_packets; i++) { + unsigned long flags; int length = urb-iso_frame_desc[i].actual_length / stride; cp = (unsigned char *)urb-transfer_buffer + @@ -134,7 +135,7 @@ static void em28xx_audio_isocirq(struct urb *urb) length * stride); } - snd_pcm_stream_lock(substream); + snd_pcm_stream_lock_irqsave(substream, flags); dev-adev.hwptr_done_capture += length; if (dev-adev.hwptr_done_capture = @@ -150,7 +151,7 @@ static void em28xx_audio_isocirq(struct urb *urb) period_elapsed = 1; } - snd_pcm_stream_unlock(substream); + snd_pcm_stream_unlock_irqrestore(substream, flags); } if (period_elapsed) snd_pcm_period_elapsed(substream); -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 3/3] usb: don't use bNbrPorts after initialization
After successful initialization hub-descriptor-bNbrPorts and hub-hdev-maxchild are equal, but using hub-hdev-maxchild is preferred because that value is explicitly used for initialization of hub-ports[]. Signed-off-by: Krzysztof Mazur krzys...@podlesie.net Acked-by: Alan Stern st...@rowland.harvard.edu --- drivers/usb/core/hub.c | 12 +--- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 292ffa8..86e353f 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -451,7 +451,7 @@ static void led_work (struct work_struct *work) if (hdev-state != USB_STATE_CONFIGURED || hub-quiescing) return; - for (i = 0; i hub-descriptor-bNbrPorts; i++) { + for (i = 0; i hdev-maxchild; i++) { unsignedselector, mode; /* 30%-50% duty cycle */ @@ -500,7 +500,7 @@ static void led_work (struct work_struct *work) } if (!changed blinkenlights) { cursor++; - cursor %= hub-descriptor-bNbrPorts; + cursor %= hdev-maxchild; set_port_led(hub, cursor + 1, HUB_LED_GREEN); hub-indicator[cursor] = INDICATOR_CYCLE; changed++; @@ -826,7 +826,7 @@ static unsigned hub_power_on(struct usb_hub *hub, bool do_delay) else dev_dbg(hub-intfdev, trying to enable port power on non-switchable hub\n); - for (port1 = 1; port1 = hub-descriptor-bNbrPorts; port1++) + for (port1 = 1; port1 = hub-hdev-maxchild; port1++) if (hub-ports[port1 - 1]-power_is_on) set_port_feature(hub-hdev, port1, USB_PORT_FEAT_POWER); else @@ -4655,9 +4655,7 @@ static void hub_events(void) hub_dev = hub-intfdev; intf = to_usb_interface(hub_dev); dev_dbg(hub_dev, state %d ports %d chg %04x evt %04x\n, - hdev-state, hub-descriptor - ? hub-descriptor-bNbrPorts - : 0, + hdev-state, hdev-maxchild, /* NOTE: expects max 15 ports... */ (u16) hub-change_bits[0], (u16) hub-event_bits[0]); @@ -4702,7 +4700,7 @@ static void hub_events(void) } /* deal with port status changes */ - for (i = 1; i = hub-descriptor-bNbrPorts; i++) { + for (i = 1; i = hdev-maxchild; i++) { if (test_bit(i, hub-busy_bits)) continue; connect_change = test_bit(i, hub-change_bits); -- 1.8.4.rc4.527.g303b16c -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/3] usb: fix cleanup after failure in hub_configure()
If the hub_configure() fails after setting the hdev-maxchild the hub-ports might be NULL or point to uninitialized kzallocated memory causing NULL pointer dereference in hub_quiesce() during cleanup. Now after such error the hdev-maxchild is set to 0 to avoid cleanup of uninitialized ports. Signed-off-by: Krzysztof Mazur krzys...@podlesie.net Acked-by: Alan Stern st...@rowland.harvard.edu --- drivers/usb/core/hub.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 558313d..affed11 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -1568,6 +1568,7 @@ static int hub_configure(struct usb_hub *hub, return 0; fail: + hdev-maxchild = 0; dev_err (hub_dev, config failed, %s (err %d)\n, message, ret); /* hub_disconnect() frees urb and descriptor */ -- 1.8.4.rc4.527.g303b16c -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/3] usb: fix hub_configure() error handling
Hi, this series fixes hub_configure() error handling that causes hub-ports[i] NULL pointer dereferences. Changes in v3: Added Acked-by from Alan Stern. Changes in v2: After review by Alan Stern. The new label has been placed in other place to avoid changes in existing code. Third patch usb: don't use bNbrPorts after initialization has been added. The first version has been posted here: https://lkml.org/lkml/2013/8/20/469 Krzysiek Krzysztof Mazur (3): usb: fix cleanup after failure in hub_configure() usb: fail on usb_hub_create_port_device() errors usb: don't use bNbrPorts after initialization drivers/usb/core/hub.c | 23 ++- 1 file changed, 14 insertions(+), 9 deletions(-) -- 1.8.4.rc4.527.g303b16c -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/3] usb: fail on usb_hub_create_port_device() errors
Ignoring usb_hub_create_port_device() errors cause later NULL pointer deference when uninitialized hub-ports[i] entries are dereferenced after port memory allocation error. Signed-off-by: Krzysztof Mazur krzys...@podlesie.net Acked-by: Alan Stern st...@rowland.harvard.edu --- drivers/usb/core/hub.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index affed11..292ffa8 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -1557,10 +1557,15 @@ static int hub_configure(struct usb_hub *hub, if (hub-has_indicators blinkenlights) hub-indicator [0] = INDICATOR_CYCLE; - for (i = 0; i hdev-maxchild; i++) - if (usb_hub_create_port_device(hub, i + 1) 0) + for (i = 0; i hdev-maxchild; i++) { + ret = usb_hub_create_port_device(hub, i + 1); + if (ret 0) { dev_err(hub-intfdev, couldn't create port%d device.\n, i + 1); + hdev-maxchild = i; + goto fail_keep_maxchild; + } + } usb_hub_adjust_deviceremovable(hdev, hub-descriptor); @@ -1569,6 +1574,7 @@ static int hub_configure(struct usb_hub *hub, fail: hdev-maxchild = 0; +fail_keep_maxchild: dev_err (hub_dev, config failed, %s (err %d)\n, message, ret); /* hub_disconnect() frees urb and descriptor */ -- 1.8.4.rc4.527.g303b16c -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] HID: New hid-cp2112 driver.
This patch adds support for the Silicon Labs CP2112 Single-Chip HID USB to SMBus Master Bridge. I wrote this to support a USB temperature and humidity sensor I've been working on. It's been tested by using SMBus byte-read, byte-data-read/write, and word-data-read transfer modes to talk to an I2C sensor. The other transfer modes have not been tested. Signed-off-by: David Barksdale dbarksd...@uplogix.com --- drivers/hid/Kconfig | 6 + drivers/hid/Makefile | 1 + drivers/hid/hid-core.c | 3 + drivers/hid/hid-cp2112.c | 504 +++ 4 files changed, 514 insertions(+) diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig index 14ef6ab..1833948 100644 --- a/drivers/hid/Kconfig +++ b/drivers/hid/Kconfig @@ -175,6 +175,12 @@ config HID_PRODIKEYS multimedia keyboard, but will lack support for the musical keyboard and some additional multimedia keys. +config HID_CP2112 + tristate Silicon Labs CP2112 HID USB-to-SMBus Bridge support + depends on USB_HID + ---help--- + Support for Silicon Labs CP2112 HID USB to SMBus Master Bridge. + config HID_CYPRESS tristate Cypress mouse and barcode readers if EXPERT depends on HID diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile index 6f68728..a88a5c4 100644 --- a/drivers/hid/Makefile +++ b/drivers/hid/Makefile @@ -41,6 +41,7 @@ obj-$(CONFIG_HID_AUREAL)+= hid-aureal.o obj-$(CONFIG_HID_BELKIN) += hid-belkin.o obj-$(CONFIG_HID_CHERRY) += hid-cherry.o obj-$(CONFIG_HID_CHICONY) += hid-chicony.o +obj-$(CONFIG_HID_CP2112) += hid-cp2112.o obj-$(CONFIG_HID_CYPRESS) += hid-cypress.o obj-$(CONFIG_HID_DRAGONRISE) += hid-dr.o obj-$(CONFIG_HID_EMS_FF) += hid-emsff.o diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 36668d1..b472a0d 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1568,6 +1568,9 @@ static const struct hid_device_id hid_have_special_driver[] = { { HID_USB_DEVICE(USB_VENDOR_ID_CHICONY, USB_DEVICE_ID_CHICONY_WIRELESS2) }, { HID_USB_DEVICE(USB_VENDOR_ID_CHICONY, USB_DEVICE_ID_CHICONY_AK1D) }, { HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, USB_DEVICE_ID_PRODIKEYS_PCMIDI) }, +#if IS_ENABLED(CONFIG_HID_CP2112) + { HID_USB_DEVICE(USB_VENDOR_ID_CYGNAL, 0xEA90) }, +#endif { HID_USB_DEVICE(USB_VENDOR_ID_CYPRESS, USB_DEVICE_ID_CYPRESS_BARCODE_1) }, { HID_USB_DEVICE(USB_VENDOR_ID_CYPRESS, USB_DEVICE_ID_CYPRESS_BARCODE_2) }, { HID_USB_DEVICE(USB_VENDOR_ID_CYPRESS, USB_DEVICE_ID_CYPRESS_BARCODE_3) }, diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c new file mode 100644 index 000..8737d18 --- /dev/null +++ b/drivers/hid/hid-cp2112.c @@ -0,0 +1,504 @@ +/* + hid-cp2112.c - Silicon Labs HID USB to SMBus master bridge + Copyright (c) 2013 Uplogix, Inc. + David Barksdale dbarksd...@uplogix.com + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + */ + +#include linux/hid.h +#include linux/i2c.h +#include linux/module.h +#include hid-ids.h + +enum { + GET_VERSION_INFO = 0x05, + SMBUS_CONFIG = 0x06, + DATA_READ_REQUEST = 0x10, + DATA_WRITE_READ_REQUEST = 0x11, + DATA_READ_FORCE_SEND = 0x12, + DATA_READ_RESPONSE = 0x13, + DATA_WRITE_REQUEST = 0x14, + TRANSFER_STATUS_REQUEST = 0x15, + TRANSFER_STATUS_RESPONSE = 0x16, + CANCEL_TRANSFER = 0x17, +}; + +enum { + STATUS0_IDLE = 0x00, + STATUS0_BUSY = 0x01, + STATUS0_COMPLETE = 0x02, + STATUS0_ERROR = 0x03, +}; + +enum { + STATUS1_TIMEOUT_NACK = 0x00, + STATUS1_TIMEOUT_BUS = 0x01, + STATUS1_ARBITRATION_LOST = 0x02, + STATUS1_READ_INCOMPLETE = 0x03, + STATUS1_WRITE_INCOMPLETE = 0x04, + STATUS1_SUCCESS = 0x05, +}; + +/* All values are in big-endian */ +struct __attribute__ ((__packed__)) smbus_config { + uint32_t clock_speed; /* Hz */ + uint8_t device_address; /* Stored in the upper 7 bits */ + uint8_t auto_send_read; /* 1 = enabled, 0 = disabled */ + uint16_t write_timeout; /* ms, 0 = no timeout */ + uint16_t read_timeout; /* ms, 0 = no timeout */ + uint8_t scl_low_timeout; /* 1 = enabled, 0 = disabled */ + uint16_t retry_time; /* # of retries, 0 = no limit */ +}; +
Re: [RFT RFC] USB: Fix USB device disconnects on resume.
On Wed, 21 Aug 2013, Sarah Sharp wrote: Possible fixes -- The USB core obviously needs to be changed to check the port status after the TRSMRCY timeout, and continue to wait if the port is still in the resuming state. I will have to study the EHCI port status diagrams in detail to figure out how the USB core can do this. As far as EHCI is concerned, this is a non-problem. The closest analogy to the RExit-U0 transition is in the description of the Force Port Resume bit (bit 6) in Table 2-16 of the EHCI spec, where it says that the host controller must complete the transition to the high-speed idle state within 2 milliseconds of software setting the bit to a zero (which happens when the hub driver does its Get-Port-Status call). Thus, as soon as the TRSMRCY delay is finished, the device and the port are supposed to be ready. In fact, the hardware doesn't provide any means of telling whether they are ready or not. I can easily do this without the USB core being involved, by changing the xHCI driver to either: 1. Busy wait with xhci_handshake() in the xHCI get port status until the port is in U0. 2. Add a completion per xHCI port. In xHCI get port status, initiate U0 entry, and wait on the port's completion for up to 20 ms. In the port status change event code, complete that port's completion when the port is in U0 and the bus_state-resuming_ports bit is set. I would expect either of those to be adequate. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Trying to develop a Pixelsense kernel driver
On Thu, 22 Aug 2013, Florian Echtler wrote: Hello everyone, I recently wanted to pick up again on developing a kernel input driver for the Microsoft Pixelsense (formerly Surface 2.0). I already have a working user space driver (see also [1]). However - just like last year [2] - I quickly ran into an issue with a corrupted device firmware after some brief experimentation with the kernel driver, while the user space driver didn't and doesn't show any such issues. This happened on two separate Pixelsense devices, with different kernel versions. So even though userspace and kernel driver look like they perform exactly the same operations on the device, there must be some difference I don't understand - perhaps libusb does some kind of internal error handling which the kernel doesn't do? Userspace version: // for control commands usb_control_msg(handle, 0xC0, cmd, 0x00, index, (char*)buf, len, 1000); // for bulk data transfer uint8_t buffer[512]; result = usb_bulk_read( handle, 0x86, (char*)(buffer), sizeof(buffer), 1000 ); Kernel version: // for control commands #define surface_command(dev, command, index, buffer, size) usb_control_msg (dev-usbdev, usb_rcvctrlpipe (dev-usbdev, 0), command, USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_IN, 0x00, index, buffer, size, 1000) This isn't equivalent to the userspace version. Here the recipient is USB_RECIP_ENDPOINT, but in the userspace version it is USB_RECIP_DEVICE. To put it another way, here the bRequestType value is 0xC2 and there it is 0xC0. // for bulk data transfer result = usb_bulk_msg (surface-usbdev, usb_rcvbulkpipe (surface-usbdev, 0x86), surface-bulk_in_buffer, 512, bulk_read, 1000); This is equivalent to the version above. Can anybody venture a guess what kind of functional difference there is between these code snippets? It's hard to say exactly. Probably one of the control messages will work and the other one won't. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB Card reader constantly resets with xhci
On Thu, 22 Aug 2013, Joseph Yasi wrote: Hi, I just recently moved an internal USB card reader from an ehci machine to an xhci machine, and dmesg is filled with this message over and over again: [ 1596.047684] usb 1-8: reset high-speed USB device number 4 using xhci_hcd [ 1596.090902] xhci_hcd :00:14.0: xHCI xhci_drop_endpoint called with disabled ep 88041b06d700 [ 1596.090904] xhci_hcd :00:14.0: xHCI xhci_drop_endpoint called with disabled ep 88041b06d740 This did not happen on the ehci machine. Oddly, the card reader still works, but it just keeps printing that message in the log. Can you post usbmon traces for the card reader from the two machines? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 0/5] usbfs: Add support for bulk streams
Hi, On 08/22/2013 12:54 AM, Sarah Sharp wrote: On Thu, Aug 15, 2013 at 10:33:42PM +0200, Hans de Goede wrote: Hi, On 08/15/2013 12:42 PM, Hans de Goede wrote: snip What device did you find? I have yet to see a shipping device with streams... I don't know about streams, I'm hoping that having a uasp device means it will also use streams. So far I've been unable to get my hands on anything doing uasp, but it seems the tide is turning and now I've found multiple. Now lets hope that: 1) They really support uasp 2) they use streams (not sure if that is optional or mandatory with uasp). This is the one I've ordered: http://www.amazon.co.uk/Anker%C2%AE-Uspeed-Enclosure-2-5-Inch-Support/dp/B005B5NLZ0/ Correction, this one just arrived today and it despite its product description claiming it does, it does *not* support uasp. This one also looks nice, and plugable is known to be a Linux friendly company, and they should be available in the US: http://www.amazon.co.uk/Plugable-Lay-Flat-Docking-Station-ASM1053E/dp/B00D399JU2 I've ordered this one now ... I just ordered that one as well, and it looks like it doesn't have UAS exposed, even though the descriptors mention it: snip I wonder if this needs a vendor specific command to disconnect and reconnect in UAS mode? Or perhaps a firmware blob? I'll have to connect it under Windows with a USB bus analyzer and see what the driver does. Mine arrived yesterday and it does have UAS support (as alternate setting 1 for interface 0) in its descriptors, without needing any special commands. On a hunch I tried to reproduce your experience with my plugable dock, and if I turn it on without a harddisk in there it does not advertise uasp either, so likely it only advertises uasp if the harddisk is tcq capable (or some such), and without a harddisk it cannot determine that ... I wrote a small libusb program to get the super speed endpoint companion descriptors, and it can do up to 4 bulk streams on the uas data endpoints, so it seems like a perfect device to test uasp and bulk streams with. BTW we really ought to extend lsusb to list the number of streams an endpoint can handle in the endpoint info it prints with -v. I can load the Linux uas driver, after which I hit: https://bugzilla.kernel.org/show_bug.cgi?id=51031 So I'm happy to hear you've one too, I hope it will do uasp once you insert a harddisk, because then hopefully you can fix that bug :) If inserting a harddisk fails, ASMedia controllers have upgradable firmware inside them, so the first thing to try and get uasp working on your model is to try the firmware upgrade: http://plugable.com/2013/04/22/usb3-sata-uasp1-sleep-and-large-volume-firmware-update Note I did not need to do this, but judging from the descriptors you do seem to have slightly different firmware, yours has a manufacturer string of ASM1053E, where as mine has a manufacturer string of Plugable. I've attached my full descriptors (from when a harddisk is inserted into the dock). If that does not work I guess you should contact plugable about this, as said before various people in the FOSS community have good experience with them, they are quite FOSS / Linux friendly. Regards, Hans Bus 007 Device 003: ID 174c:55aa ASMedia Technology Inc. ASMedia 2105 SATA bridge Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 3.00 bDeviceClass0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 9 idVendor 0x174c ASMedia Technology Inc. idProduct 0x55aa ASMedia 2105 SATA bridge bcdDevice1.00 iManufacturer 2 Plugable iProduct3 USB3-SATA-UASP1 iSerial 1 0015 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 121 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xc0 Self Powered MaxPower 36mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 2 bInterfaceClass 8 Mass Storage bInterfaceSubClass 6 SCSI bInterfaceProtocol 80 Bulk-Only iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0400 1x 1024 bytes bInterval 0 bMaxBurst 15 Endpoint Descriptor: bLength 7 bDescriptorType 5
[RFC v2 2/2] xhci: refactor handle_cmd_completion() switch branches into functions
This patch refactors the switch statement in handle_cmd_completion() by creating a separate function for each branch, named 'xhci_handle_cmd_type'. Other changes introduced by this patch are: - Renaming the already existed functions by adding the prefix 'xhci_handle_cmd_' for consistency. Also, for handle_stopped_endpoint() the order of arguments was changed too, so that the prototype of 'xhci_handle_cmd_' be similar and easy to follow. - Additional local variables were added, such as cmd_trb, cmd_comp_code, cmd_type, add_flags and drop_flags, and the label 'update_ring', mainly to reduce code dublication and line length. - The variable ep_ring, that was assigned in the case TRB_CONFIG_EP but never used, was removed. Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com --- Differences from version 1: Create separate functions for each switch case branch, instead of a single function holding the entire switch drivers/usb/host/xhci-ring.c | 310 ++- 1 file changed, 185 insertions(+), 125 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index ddbda35..bc9ce03 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -755,8 +755,8 @@ static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci, * 2. Otherwise, we turn all the TRBs in the TD into No-op TRBs (with the chain * bit cleared) so that the HW will skip over them. */ -static void handle_stopped_endpoint(struct xhci_hcd *xhci, - union xhci_trb *trb, struct xhci_event_cmd *event) +static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, + struct xhci_event_cmd *event, union xhci_trb *trb) { unsigned int slot_id; unsigned int ep_index; @@ -1063,9 +1063,8 @@ static void update_ring_for_set_deq_completion(struct xhci_hcd *xhci, * endpoint doorbell to restart the ring, but only if there aren't more * cancellations pending. */ -static void handle_set_deq_completion(struct xhci_hcd *xhci, - struct xhci_event_cmd *event, - union xhci_trb *trb) +static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, + struct xhci_event_cmd *event, union xhci_trb *trb) { unsigned int slot_id; unsigned int ep_index; @@ -1157,9 +1156,8 @@ static void handle_set_deq_completion(struct xhci_hcd *xhci, ring_doorbell_for_active_rings(xhci, slot_id, ep_index); } -static void handle_reset_ep_completion(struct xhci_hcd *xhci, - struct xhci_event_cmd *event, - union xhci_trb *trb) +static void xhci_handle_cmd_reset_ep(struct xhci_hcd *xhci, + struct xhci_event_cmd *event, union xhci_trb *trb) { int slot_id; unsigned int ep_index; @@ -1372,21 +1370,160 @@ static int handle_stopped_cmd_ring(struct xhci_hcd *xhci, return cur_trb_is_good; } -static void handle_cmd_completion(struct xhci_hcd *xhci, +static void xhci_handle_cmd_enable_slot(struct xhci_hcd *xhci, + struct xhci_event_cmd *event, u32 cmd_comp_code) +{ + int slot_id; + + slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event-flags)); + if (cmd_comp_code == COMP_SUCCESS) + xhci-slot_id = slot_id; + else + xhci-slot_id = 0; + complete(xhci-addr_dev); +} + +static void xhci_handle_cmd_disable_slot(struct xhci_hcd *xhci, struct xhci_event_cmd *event) { - int slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event-flags)); - u64 cmd_dma; - dma_addr_t cmd_dequeue_dma; - struct xhci_input_control_ctx *ctrl_ctx; + int slot_id; struct xhci_virt_device *virt_dev; + + slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event-flags)); + virt_dev = xhci-devs[slot_id]; + if (!virt_dev) + return; + if (xhci-quirks XHCI_EP_LIMIT_QUIRK) + /* Delete default control endpoint resources */ + xhci_free_device_endpoint_resources(xhci, virt_dev, true); + xhci_free_virt_device(xhci, slot_id); +} + +static void xhci_handle_cmd_config_ep(struct xhci_hcd *xhci, + struct xhci_event_cmd *event, u32 cmd_comp_code) +{ + int slot_id; + struct xhci_virt_device *virt_dev; + struct xhci_input_control_ctx *ctrl_ctx; unsigned int ep_index; - struct xhci_ring *ep_ring; unsigned int ep_state; + u32 add_flags, drop_flags; + + slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event-flags)); + virt_dev = xhci-devs[slot_id]; + if (handle_cmd_in_cmd_wait_list(xhci, virt_dev, event)) + return; + /* +* Configure endpoint commands can come from the USB core +* configuration or alt setting changes, or because the HW +* needed an extra configure endpoint command after a reset +* endpoint command or streams were being configured. +* If the command was for a halted endpoint, the
Re: Trying to develop a Pixelsense kernel driver
Hello Alan, thanks for the quick response. On 22.08.2013 16:49, Alan Stern wrote: On Thu, 22 Aug 2013, Florian Echtler wrote: So even though userspace and kernel driver look like they perform exactly the same operations on the device, there must be some difference I don't understand - perhaps libusb does some kind of internal error handling which the kernel doesn't do? Userspace version: // for control commands usb_control_msg(handle, 0xC0, cmd, 0x00, index, (char*)buf, len, 1000); Kernel version: // for control commands #define surface_command(dev, command, index, buffer, size) usb_control_msg (dev-usbdev, usb_rcvctrlpipe (dev-usbdev, 0), command, USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_IN, 0x00, index, buffer, size, 1000) This isn't equivalent to the userspace version. Here the recipient is USB_RECIP_ENDPOINT, but in the userspace version it is USB_RECIP_DEVICE. To put it another way, here the bRequestType value is 0xC2 and there it is 0xC0. Great, thanks for spotting this. I thought I had correctly interpreted the original value with respect to the kernel constants - apparently not. // for bulk data transfer result = usb_bulk_msg (surface-usbdev, usb_rcvbulkpipe (surface-usbdev, 0x86), surface-bulk_in_buffer, 512, bulk_read, 1000); This is equivalent to the version above. OK. Can anybody venture a guess what kind of functional difference there is between these code snippets? It's hard to say exactly. Probably one of the control messages will work and the other one won't. The (wrong?) control messages in the kernel still work (in the sense that they don't trigger any detectable errors during the init sequence). However, as mentioned above, after a few loads and unloads of the kernel driver, the firmware EEPROM is trashed. (Which leads to some speculation about the quality of said firmware, regardless of my poor driver writing skills.) Once I work up the nerve to give it another try with the updated driver, I'll report back. Florian -- SENT FROM MY DEC VT50 TERMINAL signature.asc Description: OpenPGP digital signature
[RFC] xhci: add trace for missed periodic transfers
This patch adds a trace event to the class 'xhci_log_msg', called 'xhci_dbg_missed_periodic_tx', that traces the debug statements which signal that the xHC is unable to service an isochronous endpoint within its service interval either because the endpoint's ring is full and can not receive further data, for IN endpoints, or because the endpoint's ring is empty, for OUT endpoints, or due to xHC internal buffers' overrun or underrun caused by excessive latency on the transfer path. Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com --- drivers/usb/host/xhci-ring.c | 19 --- drivers/usb/host/xhci-trace.h | 5 + 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index bc9ce03..06d3a8f 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2510,18 +2510,22 @@ static int handle_tx_event(struct xhci_hcd *xhci, * a Ring Overrun Event for IN Isoch endpoint or Ring * Underrun Event for OUT Isoch endpoint. */ - xhci_dbg(xhci, underrun event on endpoint\n); + xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx, + underrun event on endpoint); if (!list_empty(ep_ring-td_list)) - xhci_dbg(xhci, Underrun Event for slot %d ep %d - still with TDs queued?\n, + xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx, + Underrun Event for slot %d ep %d + still with TDs queued?, TRB_TO_SLOT_ID(le32_to_cpu(event-flags)), ep_index); goto cleanup; case COMP_OVERRUN: - xhci_dbg(xhci, overrun event on endpoint\n); + xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx, + overrun event on endpoint); if (!list_empty(ep_ring-td_list)) - xhci_dbg(xhci, Overrun Event for slot %d ep %d - still with TDs queued?\n, + xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx, + Overrun Event for slot %d ep %d + still with TDs queued?, TRB_TO_SLOT_ID(le32_to_cpu(event-flags)), ep_index); goto cleanup; @@ -2537,7 +2541,8 @@ static int handle_tx_event(struct xhci_hcd *xhci, * short transfer when process the ep_ring next time. */ ep-skip = true; - xhci_dbg(xhci, Miss service interval error, set skip flag\n); + xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx, + Miss service interval error, set skip flag); goto cleanup; default: if (xhci_is_vendor_info_code(xhci, trb_comp_code)) { diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h index 20364cc..c156685 100644 --- a/drivers/usb/host/xhci-trace.h +++ b/drivers/usb/host/xhci-trace.h @@ -67,6 +67,11 @@ DEFINE_EVENT(xhci_log_msg, xhci_dbg_ring_expansion, TP_ARGS(vaf) ); +DEFINE_EVENT(xhci_log_msg, xhci_dbg_missed_periodic_tx, + TP_PROTO(struct va_format *vaf), + TP_ARGS(vaf) +); + DECLARE_EVENT_CLASS(xhci_log_ctx, TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx, unsigned int ep_num), -- 1.8.3.4 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v2 1/2] xhci: remove unused argument from xhci_giveback_urb_in_irq()
This patch removes the adjective argument from xhci_giveback_urb_in_irq(), since it is not used in the function anymore. Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com --- drivers/usb/host/xhci-ring.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 7b35af1..ddbda35 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -716,7 +716,7 @@ static void xhci_stop_watchdog_timer_in_irq(struct xhci_hcd *xhci, /* Must be called with xhci-lock held in interrupt context */ static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci, - struct xhci_td *cur_td, int status, char *adjective) + struct xhci_td *cur_td, int status) { struct usb_hcd *hcd; struct urb *urb; @@ -877,7 +877,7 @@ remove_finished_td: /* Doesn't matter what we pass for status, since the core will * just overwrite it (because the URB has been unlinked). */ - xhci_giveback_urb_in_irq(xhci, cur_td, 0, cancelled); + xhci_giveback_urb_in_irq(xhci, cur_td, 0); /* Stop processing the cancelled list if the watchdog timer is * running. @@ -987,7 +987,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg) if (!list_empty(cur_td-cancelled_td_list)) list_del_init(cur_td-cancelled_td_list); xhci_giveback_urb_in_irq(xhci, cur_td, - -ESHUTDOWN, killed); + -ESHUTDOWN); } while (!list_empty(temp_ep-cancelled_td_list)) { cur_td = list_first_entry( @@ -996,7 +996,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg) cancelled_td_list); list_del_init(cur_td-cancelled_td_list); xhci_giveback_urb_in_irq(xhci, cur_td, - -ESHUTDOWN, killed); + -ESHUTDOWN); } } } -- 1.8.3.4 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] usb: implement AMD remote wakeup quirk
Hi Sarah, Alan, The following patches are required to resolve remote wake issues with certain devices. Issue description: If the remote wake is issued from the device in a specific timing condition while the system is entering sleep state then it may cause system to auto wake on subsequent sleep cycle. Root Cause: Host controller rebroadcasts the Resume signal 100 µseconds after receiving the original resume event from the device. For proper function, some devices may require the rebroadcast of resume event within the USB spec of 100µS. Without this quirk, some AMD platform doesn't hold the resume right away when there is a resume signal from LS device like mouse. So it should reset the port which attached with mouse. Patch 1 is the implementation for xHCI driver. Patch 2 is the implementation for OHCI driver. They are generated on gregkh/usb usb-next. Thanks, Rui Huang Rui (2): usb: xhci: implement AMD remote wakeup quirk usb: ohci: add AMD remote wakeup quirk into ohci driver drivers/usb/core/hub.c| 28 drivers/usb/host/ohci-hub.c | 42 + drivers/usb/host/ohci-pci.c | 13 + drivers/usb/host/ohci.h | 23 drivers/usb/host/pci-quirks.c | 17 +++- drivers/usb/host/pci-quirks.h | 1 + drivers/usb/host/xhci-pci.c | 4 +++ drivers/usb/host/xhci.c | 61 +++ drivers/usb/host/xhci.h | 25 +- include/linux/usb.h | 1 + 10 files changed, 191 insertions(+), 24 deletions(-) -- 1.7.11.7 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] usb: xhci: implement AMD remote wakeup quirk
The following patch is required to resolve remote wake issues with certain devices. Issue description: If the remote wake is issued from the device in a specific timing condition while the system is entering sleep state then it may cause system to auto wake on subsequent sleep cycle. Root cause: Host controller rebroadcasts the Resume signal 100 µseconds after receiving the original resume event from the device. For proper function, some devices may require the rebroadcast of resume event within the USB spec of 100µS. This patch is only for xHCI driver and the quirk will be also added into OHCI driver too. This should be backported to kernels as old as 3.9, that contrain the commit 3f5eb14135ba9d97ba4b8514fc7ef5e0dac2abf4 usb: add find_raw_port_number callback to struct hc_driver(). Cc sta...@vger.kernel.org Signed-off-by: Huang Rui ray.hu...@amd.com --- drivers/usb/core/hub.c| 28 drivers/usb/host/pci-quirks.c | 17 +++- drivers/usb/host/pci-quirks.h | 1 + drivers/usb/host/xhci-pci.c | 4 +++ drivers/usb/host/xhci.c | 61 +++ drivers/usb/host/xhci.h | 25 +- include/linux/usb.h | 1 + 7 files changed, 124 insertions(+), 13 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 175179e..117196c 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -27,6 +27,7 @@ #include linux/freezer.h #include linux/random.h #include linux/pm_qos.h +#include linux/hid.h #include asm/uaccess.h #include asm/byteorder.h @@ -5437,3 +5438,30 @@ acpi_handle usb_get_hub_port_acpi_handle(struct usb_device *hdev, return DEVICE_ACPI_HANDLE(hub-ports[port1 - 1]-dev); } #endif + +bool is_usb_mouse(struct usb_device *udev) +{ + struct usb_interface *intf; + struct usb_interface_descriptor intf_desc; + + if (!udev) { + dev_warn(udev-dev, Warn: no device attached!\n); + return false; + } + + intf = usb_ifnum_to_if(udev, 0); + intf_desc = intf-cur_altsetting-desc; + if (intf_desc.bInterfaceClass == USB_INTERFACE_CLASS_HID + intf_desc.bInterfaceSubClass == + USB_INTERFACE_SUBCLASS_BOOT + intf_desc.bInterfaceProtocol == + USB_INTERFACE_PROTOCOL_MOUSE) { + dev_dbg(udev-dev, The attached usb device is +mouse\n); + return true; + } + + dev_dbg(udev-dev, The attached usb device is NOT mouse\n); + return false; +} +EXPORT_SYMBOL_GPL(is_usb_mouse); diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c index 2c76ef1..218e421 100644 --- a/drivers/usb/host/pci-quirks.c +++ b/drivers/usb/host/pci-quirks.c @@ -140,9 +140,11 @@ int usb_amd_find_chipset_info(void) rev = info.smbus_dev-revision; if (rev = 0x11 rev = 0x18) info.sb_type = 2; + if (rev = 0x39 rev = 0x3a) + info.sb_type = 4; } - if (info.sb_type == 0) { + if (info.sb_type == 0 || info.sb_type == 4) { if (info.smbus_dev) { pci_dev_put(info.smbus_dev); info.smbus_dev = NULL; @@ -197,6 +199,19 @@ commit: } EXPORT_SYMBOL_GPL(usb_amd_find_chipset_info); +int usb_amd_remote_wakeup_quirk(void) +{ + u8 ret; + if (amd_chipset.sb_type == 4) { + ret = 1; + printk(KERN_DEBUG QUIRK: Enable AMD remote wakup +fix\n); + } else + ret = 0; + return ret; +} +EXPORT_SYMBOL_GPL(usb_amd_remote_wakeup_quirk); + /* * The hardware normally enables the A-link power management feature, which * lets the system lower the power consumption in idle states. diff --git a/drivers/usb/host/pci-quirks.h b/drivers/usb/host/pci-quirks.h index ed6700d..e28bbbe 100644 --- a/drivers/usb/host/pci-quirks.h +++ b/drivers/usb/host/pci-quirks.h @@ -5,6 +5,7 @@ void uhci_reset_hc(struct pci_dev *pdev, unsigned long base); int uhci_check_and_reset_hc(struct pci_dev *pdev, unsigned long base); int usb_amd_find_chipset_info(void); +int usb_amd_remote_wakeup_quirk(void); void usb_amd_dev_put(void); void usb_amd_quirk_pll_disable(void); void usb_amd_quirk_pll_enable(void); diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index c2d4950..cc2ff7e 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -90,6 +90,10 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) /* AMD PLL quirk */ if (pdev-vendor == PCI_VENDOR_ID_AMD usb_amd_find_chipset_info()) xhci-quirks |= XHCI_AMD_PLL_FIX; + + if (pdev-vendor == PCI_VENDOR_ID_AMD usb_amd_remote_wakeup_quirk()) + xhci-quirks |= XHCI_AMD_REMOTE_WAKEUP_QUIRK; +
[PATCH 2/2] usb: ohci: add AMD remote wakeup quirk into ohci driver
The following patch is required to resolve remote wake issues with certain devices. Issue description: If the remote wake is issued from the device in a specific timing condition while the system is entering sleep state then it may cause system to auto wake on subsequent sleep cycle. Root Cause: Host controller rebroadcasts the Resume signal 100 µseconds after receiving the original resume event from the device. For proper function, some devices may require the rebroadcast of resume event within the USB spec of 100µS. This patch is for OHCI driver to add AMD remote wakeup fix. This should be backported to kernels as old as 3.9, that contrain the commit 3f5eb14135ba9d97ba4b8514fc7ef5e0dac2abf4 usb: add find_raw_port_number callback to struct hc_driver() and the last patch that AMD remote wakeup quirk for xhci. Cc: sta...@vger.kernel.org Signed-off-by: Huang Rui ray.hu...@amd.com --- drivers/usb/host/ohci-hub.c | 42 ++ drivers/usb/host/ohci-pci.c | 13 + drivers/usb/host/ohci.h | 23 --- 3 files changed, 67 insertions(+), 11 deletions(-) diff --git a/drivers/usb/host/ohci-hub.c b/drivers/usb/host/ohci-hub.c index 2347ab8..0af45e3 100644 --- a/drivers/usb/host/ohci-hub.c +++ b/drivers/usb/host/ohci-hub.c @@ -41,6 +41,7 @@ static void dl_done_list (struct ohci_hcd *); static void finish_unlinks (struct ohci_hcd *, u16); +static inline int root_port_reset (struct ohci_hcd *, unsigned); #ifdef CONFIG_PM static int ohci_rh_suspend (struct ohci_hcd *ohci, int autostop) @@ -293,6 +294,44 @@ static int ohci_bus_suspend (struct usb_hcd *hcd) return rc; } +/* + * Reset port which attached with mouse. + * + * If the remote wake is issued from the device in a specific timing + * condition while the system is entering sleep state then it may + * cause system to auto wake on subsequent sleep cycle. + * + * Host controller rebroadcasts the Resume signal 100 µseconds after + * receiving the original resume event from the device. For proper + * function, some devices may require the rebroadcast of resume event + * within the USB spec of 100µS. + * + * Without this quirk, some AMD platform doesn't hold the resume right + * away when there is a resume signal from LS device like mouse. So it + * should reset the port which attached with mouse. + */ +static int ohci_reset_port_by_mouse(struct ohci_hcd *ohci) +{ + u32 temp; + struct usb_device *hdev, *child; + int port1, wIndex; + + hdev = hcd_to_bus(ohci_to_hcd(ohci))-root_hub; + + usb_hub_for_each_child(hdev, port1, child) { + wIndex = port1 - 1; + temp = roothub_portstatus (ohci, wIndex); + dbg_port(ohci, Get port status, wIndex, temp); + if (is_usb_mouse(child)) { + ohci_dbg(ohci, Connencted mouse on port1%d.\n, + wIndex); + root_port_reset(ohci, wIndex); + } + } + + return 0; +} + static int ohci_bus_resume (struct usb_hcd *hcd) { struct ohci_hcd *ohci = hcd_to_ohci (hcd); @@ -309,6 +348,9 @@ static int ohci_bus_resume (struct usb_hcd *hcd) rc = ohci_rh_resume (ohci); spin_unlock_irq (ohci-lock); + if (ohci-flags OHCI_QUIRK_AMD_REMOTE_WAKEUP) + ohci_reset_port_by_mouse(ohci); + /* poll until we know a device is connected or we autostop */ if (rc == 0) usb_hcd_poll_rh_status(hcd); diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c index 08613e2..f69e2f3 100644 --- a/drivers/usb/host/ohci-pci.c +++ b/drivers/usb/host/ohci-pci.c @@ -175,6 +175,15 @@ static int ohci_quirk_amd700(struct usb_hcd *hcd) return 0; } +static int ohci_quirk_remote_wakeup(struct usb_hcd *hcd) +{ + struct ohci_hcd *ohci = hcd_to_ohci(hcd); + if (usb_amd_remote_wakeup_quirk()) { + ohci-flags |= OHCI_QUIRK_AMD_REMOTE_WAKEUP; + } + return 0; +} + /* List of quirks for OHCI */ static const struct pci_device_id ohci_pci_quirks[] = { { @@ -225,6 +234,10 @@ static const struct pci_device_id ohci_pci_quirks[] = { PCI_DEVICE(PCI_VENDOR_ID_ATI, 0x4399), .driver_data = (unsigned long)ohci_quirk_amd700, }, + { + PCI_DEVICE(PCI_VENDOR_ID_AMD, 0x7807), + .driver_data = (unsigned long)ohci_quirk_remote_wakeup, + }, /* FIXME for some of the early AMD 760 southbridges, OHCI * won't work at all. blacklist them. diff --git a/drivers/usb/host/ohci.h b/drivers/usb/host/ohci.h index e2e5faa..0d53f32 100644 --- a/drivers/usb/host/ohci.h +++ b/drivers/usb/host/ohci.h @@ -394,17 +394,18 @@ struct ohci_hcd { unsignedautostop:1; /* rh auto stopping/stopped */ unsigned long flags; /* for HC bugs */ -#define
Re: [RFC v2 2/2] xhci: refactor handle_cmd_completion() switch branches into functions
On Thu, Aug 22, 2013 at 06:20:14PM +0300, Xenia Ragiadakou wrote: This patch refactors the switch statement in handle_cmd_completion() by creating a separate function for each branch, named 'xhci_handle_cmd_type'. Other changes introduced by this patch are: - Renaming the already existed functions by adding the prefix 'xhci_handle_cmd_' for consistency. Also, for handle_stopped_endpoint() the order of arguments was changed too, so that the prototype of 'xhci_handle_cmd_' be similar and easy to follow. - Additional local variables were added, such as cmd_trb, cmd_comp_code, cmd_type, add_flags and drop_flags, and the label 'update_ring', mainly to reduce code dublication and line length. - The variable ep_ring, that was assigned in the case TRB_CONFIG_EP but never used, was removed. Can you break these changes into several patches, in order to make it easier to review? Something like: 1. Rename existing functions. 2. Change ordering of arguments for consistency. 3. Remove the unused variable from the TRB_CONFIG_EP case. 4. Several patches to refactor the code. For the refactoring, break each refactoring into a separate patch. I.e. refactoring and creating xhci_handle_cmd_enable_slot goes in one patch, xhci_handle_cmd_config_ep goes in another patch. This patch is nearing the 400-line fatigue point of most reviewers, see the article on Greg's google plus post for an interesting analysis: https://plus.google.com/111049168280159033135/posts/2KcFNwzzEgC Sarah Sharp Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com --- Differences from version 1: Create separate functions for each switch case branch, instead of a single function holding the entire switch drivers/usb/host/xhci-ring.c | 310 ++- 1 file changed, 185 insertions(+), 125 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index ddbda35..bc9ce03 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -755,8 +755,8 @@ static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci, * 2. Otherwise, we turn all the TRBs in the TD into No-op TRBs (with the chain * bit cleared) so that the HW will skip over them. */ -static void handle_stopped_endpoint(struct xhci_hcd *xhci, - union xhci_trb *trb, struct xhci_event_cmd *event) +static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, + struct xhci_event_cmd *event, union xhci_trb *trb) { unsigned int slot_id; unsigned int ep_index; @@ -1063,9 +1063,8 @@ static void update_ring_for_set_deq_completion(struct xhci_hcd *xhci, * endpoint doorbell to restart the ring, but only if there aren't more * cancellations pending. */ -static void handle_set_deq_completion(struct xhci_hcd *xhci, - struct xhci_event_cmd *event, - union xhci_trb *trb) +static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, + struct xhci_event_cmd *event, union xhci_trb *trb) { unsigned int slot_id; unsigned int ep_index; @@ -1157,9 +1156,8 @@ static void handle_set_deq_completion(struct xhci_hcd *xhci, ring_doorbell_for_active_rings(xhci, slot_id, ep_index); } -static void handle_reset_ep_completion(struct xhci_hcd *xhci, - struct xhci_event_cmd *event, - union xhci_trb *trb) +static void xhci_handle_cmd_reset_ep(struct xhci_hcd *xhci, + struct xhci_event_cmd *event, union xhci_trb *trb) { int slot_id; unsigned int ep_index; @@ -1372,21 +1370,160 @@ static int handle_stopped_cmd_ring(struct xhci_hcd *xhci, return cur_trb_is_good; } -static void handle_cmd_completion(struct xhci_hcd *xhci, +static void xhci_handle_cmd_enable_slot(struct xhci_hcd *xhci, + struct xhci_event_cmd *event, u32 cmd_comp_code) +{ + int slot_id; + + slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event-flags)); + if (cmd_comp_code == COMP_SUCCESS) + xhci-slot_id = slot_id; + else + xhci-slot_id = 0; + complete(xhci-addr_dev); +} + +static void xhci_handle_cmd_disable_slot(struct xhci_hcd *xhci, struct xhci_event_cmd *event) { - int slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event-flags)); - u64 cmd_dma; - dma_addr_t cmd_dequeue_dma; - struct xhci_input_control_ctx *ctrl_ctx; + int slot_id; struct xhci_virt_device *virt_dev; + + slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event-flags)); + virt_dev = xhci-devs[slot_id]; + if (!virt_dev) + return; + if (xhci-quirks XHCI_EP_LIMIT_QUIRK) + /* Delete default control endpoint resources */ + xhci_free_device_endpoint_resources(xhci, virt_dev, true); + xhci_free_virt_device(xhci, slot_id); +} + +static void xhci_handle_cmd_config_ep(struct xhci_hcd *xhci, + struct
[PATCH 3/4] staging: ozwpan: Increment reference counter.
Increment PD reference counter, on every timer event so that we do not loose PD object by mistake. Signed-off-by: Rupesh Gujare rupesh.guj...@atmel.com --- drivers/staging/ozwpan/ozproto.c |5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/staging/ozwpan/ozproto.c b/drivers/staging/ozwpan/ozproto.c index 8c2200b..d8ac823 100644 --- a/drivers/staging/ozwpan/ozproto.c +++ b/drivers/staging/ozwpan/ozproto.c @@ -497,7 +497,7 @@ void oz_pd_heartbeat_handler(unsigned long data) spin_unlock_bh(g_polling_lock); if (apps) oz_pd_heartbeat(pd, apps); - + oz_pd_put(pd); } /*-- @@ -519,6 +519,7 @@ void oz_pd_timeout_handler(unsigned long data) oz_pd_stop(pd); break; } + oz_pd_put(pd); } /*-- @@ -531,6 +532,7 @@ enum hrtimer_restart oz_pd_heartbeat_event(struct hrtimer *timer) pd = container_of(timer, struct oz_pd, heartbeat); hrtimer_forward_now(timer, ktime_set(pd-pulse_period / MSEC_PER_SEC, (pd-pulse_period % MSEC_PER_SEC) * NSEC_PER_MSEC)); + oz_pd_get(pd); tasklet_schedule(pd-heartbeat_tasklet); return HRTIMER_RESTART; } @@ -543,6 +545,7 @@ enum hrtimer_restart oz_pd_timeout_event(struct hrtimer *timer) struct oz_pd *pd; pd = container_of(timer, struct oz_pd, timeout); + oz_pd_get(pd); tasklet_schedule(pd-timeout_tasklet); return HRTIMER_NORESTART; } -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] staging: ozwpan: Create deferred work to destroy PD object.
Currently we call oz_pd_destroy() from softirq context, where we try to destroy relevant data structures, as well we kill a tasklet which always result in following kernel warning. [12279.262194] Attempt to kill tasklet from interrupt [12279.262202] Attempt to kill tasklet from interrupt This patch defers deallocation of data structures to work queue. Signed-off-by: Rupesh Gujare rupesh.guj...@atmel.com --- drivers/staging/ozwpan/ozpd.c | 28 +++- drivers/staging/ozwpan/ozpd.h |1 + 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/drivers/staging/ozwpan/ozpd.c b/drivers/staging/ozwpan/ozpd.c index 2514d79..06004c8 100644 --- a/drivers/staging/ozwpan/ozpd.c +++ b/drivers/staging/ozwpan/ozpd.c @@ -204,18 +204,16 @@ struct oz_pd *oz_pd_alloc(const u8 *mac_addr) /*-- * Context: softirq or process */ -void oz_pd_destroy(struct oz_pd *pd) +void oz_pd_free(struct work_struct *work) { struct list_head *e; struct oz_tx_frame *f; struct oz_isoc_stream *st; struct oz_farewell *fwell; + struct oz_pd *pd; oz_pd_dbg(pd, ON, Destroying PD\n); - if (hrtimer_active(pd-timeout)) - hrtimer_cancel(pd-timeout); - if (hrtimer_active(pd-heartbeat)) - hrtimer_cancel(pd-heartbeat); + pd = container_of(work, struct oz_pd, workitem); /*Disable timer tasklets*/ tasklet_kill(pd-heartbeat_tasklet); tasklet_kill(pd-timeout_tasklet); @@ -259,6 +257,26 @@ void oz_pd_destroy(struct oz_pd *pd) } /*-- + * Context: softirq or Process + */ +void oz_pd_destroy(struct oz_pd *pd) +{ + int ret; + + if (hrtimer_active(pd-timeout)) + hrtimer_cancel(pd-timeout); + if (hrtimer_active(pd-heartbeat)) + hrtimer_cancel(pd-heartbeat); + + memset(pd-workitem, 0, sizeof(pd-workitem)); + INIT_WORK(pd-workitem, oz_pd_free); + ret = schedule_work(pd-workitem); + + if (ret) + oz_pd_dbg(pd, ON, failed to schedule workitem\n); +} + +/*-- * Context: softirq-serialized */ int oz_services_start(struct oz_pd *pd, u16 apps, int resume) diff --git a/drivers/staging/ozwpan/ozpd.h b/drivers/staging/ozwpan/ozpd.h index 996ef65..12c7129 100644 --- a/drivers/staging/ozwpan/ozpd.h +++ b/drivers/staging/ozwpan/ozpd.h @@ -99,6 +99,7 @@ struct oz_pd { u8 timeout_type; struct tasklet_struct heartbeat_tasklet; struct tasklet_struct timeout_tasklet; + struct work_struct workitem; }; #define OZ_MAX_QUEUED_FRAMES 4 -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] ozwpan: staging: Fix crash for race condition.
Do not allocate a port to new device or process URB when its status is yet to be read. This avoids race condition when USB core read hub status a bit late, while new device tries to acquire port. Signed-off-by: Rupesh Gujare rupesh.guj...@atmel.com --- drivers/staging/ozwpan/ozhcd.c |5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c index 2b67107..4682d78 100644 --- a/drivers/staging/ozwpan/ozhcd.c +++ b/drivers/staging/ozwpan/ozhcd.c @@ -685,7 +685,7 @@ struct oz_port *oz_hcd_pd_arrived(void *hpd) struct oz_port *port = ozhcd-ports[i]; spin_lock(port-port_lock); - if (!(port-flags OZ_PORT_F_PRESENT)) { + if (!(port-flags (OZ_PORT_F_PRESENT | OZ_PORT_F_CHANGED))) { oz_acquire_port(port, hpd); spin_unlock(port-port_lock); break; @@ -1818,7 +1818,8 @@ static int oz_hcd_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, port = ozhcd-ports[port_ix]; if (port == NULL) return -EPIPE; - if ((port-flags OZ_PORT_F_PRESENT) == 0) { + if (!(port-flags OZ_PORT_F_PRESENT) || + (port-flags OZ_PORT_F_CHANGED)) { oz_dbg(ON, Refusing URB port_ix = %d devnum = %d\n, port_ix, urb-dev-devnum); return -EPIPE; -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB Card reader constantly resets with xhci
On Thu, 22 Aug 2013, Joseph Yasi wrote: Here's one from the xhci machine where the card reader is having the problems. The ehci machine doesn't have a power supply right now. I'll try to cobble it together tonight or over the weekend to get a log there. I don't see anything especially suspicious. It just looks like the card reader's firmware crashes every now and then, sending back STALL in response to every packet on the bulk-IN endpoint. It may be an electrical issue -- insufficient power or something like that. Or it could be a bug in the host controller hardware. The only way to tell for sure whether the problem lies in the device or in the computer is to use a USB bus analyzer. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] staging: ozwpan: Check error condition before creating endpoint.
Check if interface number is correct before creating an end point. Signed-off-by: Rupesh Gujare rupesh.guj...@atmel.com --- drivers/staging/ozwpan/ozhcd.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c index 4682d78..d5a3900 100644 --- a/drivers/staging/ozwpan/ozhcd.c +++ b/drivers/staging/ozwpan/ozhcd.c @@ -1247,6 +1247,8 @@ static int oz_build_endpoints_for_interface(struct usb_hcd *hcd, int request_heartbeat = 0; oz_dbg(ON, interface[%d] = %p\n, if_ix, intf); + if (if_ix = port-num_iface || port-iface == NULL) + return -ENOMEM; for (i = 0; i intf-desc.bNumEndpoints; i++) { struct usb_host_endpoint *hep = intf-endpoint[i]; u8 ep_addr = hep-desc.bEndpointAddress; -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] usb: implement AMD remote wakeup quirk
On Thu, 22 Aug 2013, Huang Rui wrote: Hi Sarah, Alan, The following patches are required to resolve remote wake issues with certain devices. Issue description: If the remote wake is issued from the device in a specific timing condition while the system is entering sleep state then it may cause system to auto wake on subsequent sleep cycle. Root Cause: Host controller rebroadcasts the Resume signal 100 µseconds after receiving the original resume event from the device. For proper function, some devices may require the rebroadcast of resume event within the USB spec of 100µS. Without this quirk, some AMD platform doesn't hold the resume right away when there is a resume signal from LS device like mouse. So it should reset the port which attached with mouse. What's special about LS devices? Or mice? Shouldn't the resume signal be sent within 100 us for every device? Patch 1 is the implementation for xHCI driver. Patch 2 is the implementation for OHCI driver. They are generated on gregkh/usb usb-next. You should have put the changes to core/hub.c and pci-quirks into a third patch, instead of mixing them in with the xHCI changes. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb: ohci: add AMD remote wakeup quirk into ohci driver
On Thu, 22 Aug 2013, Huang Rui wrote: The following patch is required to resolve remote wake issues with certain devices. Issue description: If the remote wake is issued from the device in a specific timing condition while the system is entering sleep state then it may cause system to auto wake on subsequent sleep cycle. Root Cause: Host controller rebroadcasts the Resume signal 100 µseconds after receiving the original resume event from the device. For proper function, some devices may require the rebroadcast of resume event within the USB spec of 100µS. This patch is for OHCI driver to add AMD remote wakeup fix. Here you need to put a description of what the fix is and how it works. This should be backported to kernels as old as 3.9, that contrain the commit 3f5eb14135ba9d97ba4b8514fc7ef5e0dac2abf4 usb: add find_raw_port_number callback to struct hc_driver() and the last patch that AMD remote wakeup quirk for xhci. Cc: sta...@vger.kernel.org Signed-off-by: Huang Rui ray.hu...@amd.com --- drivers/usb/host/ohci-hub.c | 42 ++ drivers/usb/host/ohci-pci.c | 13 + drivers/usb/host/ohci.h | 23 --- 3 files changed, 67 insertions(+), 11 deletions(-) diff --git a/drivers/usb/host/ohci-hub.c b/drivers/usb/host/ohci-hub.c index 2347ab8..0af45e3 100644 --- a/drivers/usb/host/ohci-hub.c +++ b/drivers/usb/host/ohci-hub.c @@ -41,6 +41,7 @@ static void dl_done_list (struct ohci_hcd *); static void finish_unlinks (struct ohci_hcd *, u16); +static inline int root_port_reset (struct ohci_hcd *, unsigned); #ifdef CONFIG_PM static int ohci_rh_suspend (struct ohci_hcd *ohci, int autostop) @@ -293,6 +294,44 @@ static int ohci_bus_suspend (struct usb_hcd *hcd) return rc; } +/* + * Reset port which attached with mouse. + * + * If the remote wake is issued from the device in a specific timing + * condition while the system is entering sleep state then it may + * cause system to auto wake on subsequent sleep cycle. + * + * Host controller rebroadcasts the Resume signal 100 µseconds after + * receiving the original resume event from the device. For proper + * function, some devices may require the rebroadcast of resume event + * within the USB spec of 100µS. + * + * Without this quirk, some AMD platform doesn't hold the resume right + * away when there is a resume signal from LS device like mouse. So it + * should reset the port which attached with mouse. + */ +static int ohci_reset_port_by_mouse(struct ohci_hcd *ohci) +{ + u32 temp; + struct usb_device *hdev, *child; + int port1, wIndex; + + hdev = hcd_to_bus(ohci_to_hcd(ohci))-root_hub; + + usb_hub_for_each_child(hdev, port1, child) { + wIndex = port1 - 1; + temp = roothub_portstatus (ohci, wIndex); + dbg_port(ohci, Get port status, wIndex, temp); + if (is_usb_mouse(child)) { + ohci_dbg(ohci, Connencted mouse on port1%d.\n, + wIndex); + root_port_reset(ohci, wIndex); + } + } + + return 0; +} Without more explanation, this seems completely wrong. Why do you want to reset the port? What happens if you don't reset the port? Wouldn't it be simpler to disable the port? Then usbcore would do the reset for you. That certainly is safer than a reset. Instead of doing this always for all mice, wouldn't it be better to test first and see whether it really is needed? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] staging: ozwpan: Create deferred work to destroy PD object.
Hello. On 08/22/2013 08:38 PM, Rupesh Gujare wrote: Currently we call oz_pd_destroy() from softirq context, where we try to destroy relevant data structures, as well we kill a tasklet which always result in following kernel warning. [12279.262194] Attempt to kill tasklet from interrupt [12279.262202] Attempt to kill tasklet from interrupt This patch defers deallocation of data structures to work queue. Signed-off-by: Rupesh Gujare rupesh.guj...@atmel.com --- drivers/staging/ozwpan/ozpd.c | 28 +++- drivers/staging/ozwpan/ozpd.h |1 + 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/drivers/staging/ozwpan/ozpd.c b/drivers/staging/ozwpan/ozpd.c index 2514d79..06004c8 100644 --- a/drivers/staging/ozwpan/ozpd.c +++ b/drivers/staging/ozwpan/ozpd.c [...] @@ -259,6 +257,26 @@ void oz_pd_destroy(struct oz_pd *pd) } /*-- + * Context: softirq or Process + */ +void oz_pd_destroy(struct oz_pd *pd) +{ + int ret; + + if (hrtimer_active(pd-timeout)) + hrtimer_cancel(pd-timeout); + if (hrtimer_active(pd-heartbeat)) + hrtimer_cancel(pd-heartbeat); + + memset(pd-workitem, 0, sizeof(pd-workitem)); + INIT_WORK(pd-workitem, oz_pd_free); Hm, memset(), then INIT_WORK()? Is memset() necessary? + ret = schedule_work(pd-workitem); + Don't think empty line is needed here. + if (ret) + oz_pd_dbg(pd, ON, failed to schedule workitem\n); +} + +/*-- * Context: softirq-serialized */ int oz_services_start(struct oz_pd *pd, u16 apps, int resume) diff --git a/drivers/staging/ozwpan/ozpd.h b/drivers/staging/ozwpan/ozpd.h index 996ef65..12c7129 100644 --- a/drivers/staging/ozwpan/ozpd.h +++ b/drivers/staging/ozwpan/ozpd.h @@ -99,6 +99,7 @@ struct oz_pd { u8 timeout_type; struct tasklet_struct heartbeat_tasklet; struct tasklet_struct timeout_tasklet; + struct work_struct workitem; Er, other field names seem aligned, what about this one? }; WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: acm gadget: Null termintate strings table
The gadget strings table should be null terminated. usb_gadget_get_string() loops through the table expecting a null at the end of the list. Signed-off-by: Graham gwi...@broadcom.com --- drivers/usb/gadget/f_acm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/gadget/f_acm.c b/drivers/usb/gadget/f_acm.c index 4b7e33e..ab1065a 100644 --- a/drivers/usb/gadget/f_acm.c +++ b/drivers/usb/gadget/f_acm.c @@ -285,6 +285,7 @@ static struct usb_string acm_string_defs[] = { [ACM_CTRL_IDX].s = CDC Abstract Control Model (ACM), [ACM_DATA_IDX].s = CDC ACM Data, [ACM_IAD_IDX ].s = CDC Serial, + { } /* end of list */ }; static struct usb_gadget_strings acm_string_table = { -- 1.8.0.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC v2 2/2] xhci: refactor handle_cmd_completion() switch branches into functions
From: Xenia Ragiadakou Sent: Thursday, August 22, 2013 8:20 AM This patch refactors the switch statement in handle_cmd_completion() by creating a separate function for each branch, named 'xhci_handle_cmd_type'. Other changes introduced by this patch are: - Renaming the already existed functions by adding the prefix 'xhci_handle_cmd_' for consistency. Also, for handle_stopped_endpoint() the order of arguments was changed too, so that the prototype of 'xhci_handle_cmd_' be similar and easy to follow. - Additional local variables were added, such as cmd_trb, cmd_comp_code, cmd_type, add_flags and drop_flags, and the label 'update_ring', mainly to reduce code dublication and line length. - The variable ep_ring, that was assigned in the case TRB_CONFIG_EP but never used, was removed. Just a small suggestion, I think it would be better to name these functions to indicate they handle completion events, instead of commands. So maybe xhci_handle_stop_ep_complete, xhci_handle_set_deq_complete, xhci_handle_reset_ep_complete etc? Feel free to ignore me if you disagree :) -- Paul -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] USB: EHCI: disable IAA_WATCHDOG and START_UNLINK_INTR if they needn't to be handled
On Thu, 22 Aug 2013, Ming Lei wrote: This seems to be the most important factor. When you think about it, though, does it really minimize the changes? Consider all the other adjustments we had to make to ehci-hcd: the interrupt QH unlink change and the isochronous stream stuff (which also affects the usb-audio drivers). For other HCDs, this changes on interrupt QH unlink may not need at all if there is no much cost about relink. But for some HCDs, it will be needed. About isochronous stream stuff, the only change with moving giveback in tasklet is that iso_stream_schedule() may see list_empty(stream-td_list) when underrun, and we can solve it easily, can't we? No, it's not so easy. I have done some work on it, and it's more complicated than you might think. Not to mention that the snd-usb-audio driver (and perhaps others) will also need to be modified. Also I remembered that you said the isochronous stream stuff needn't to consider on other HCDs. No, I didn't say that. It will have to be considered for ohci-hcd and uhci-hcd. So maybe the change on ehci HCD is a bit much, but for other HCDs, the change is just a little. The other HCDs will have to change too. Maybe not quite as much as ehci-hcd, but more than you seem to think. Another good point with moving giveback only to tasklet is deadlock immune during resubmissions, as you mentioned in below link: http://www.spinics.net/lists/linux-usb/msg88710.html That referred to giveback during submissions. It turns out that they aren't necessary in any case, although I didn't realize it at the time. I'm starting to think that moving the entire handler to a tasklet would have been better. Not sure, if so: - one thing is that the HCD private lock can't be held in interrupt handler any more, so new lock need to introduce, not mention each hard irq handler need to split to two parts. No new lock is needed -- the interrupt handler runs okay without one. Yes, the handler has to be split into two parts. But that's small and self-contained, and very few other changes are needed. - also it should have been better to avoid resubmissions in its giveback handler. Why? We are all set up to allow them now. Ruling them out won't make things much easier than they are already. Also moving only the giveback routine into a tasklet can avoid dropping HCD private lock during irq handler, which may simplify HCD code, and I have figured out ehci cleanup patches for this. I don't think we should make these changes. It's okay to keep the private lock during the giveback call, but let's not remove the other things. My feeling is that at some point we may indeed want to move the entire handler to a tasklet or a threaded IRQ. If that happens, all those simplifications would need to be undone. Better not to do them in the first place, since they add very little overhead. I think making HCDs simpler should have been welcome, :-) But it isn't simpler! Look at the extra code you had to add to ehci-hcd. It is now more complicated than it used to be, and the isochronous changes will make it even more so. Ruling out submissions during givebacks will make very little difference. Also one potential big problem of moving entire handler to tasklet is the hardware race, previously the entire hander is run with interrupt disabled and the big HCD private lock held, but after the conversion, both are not true, so each HCD change for this conversion might be big. If only moving giveback into tasklet, there is no such problem. I don't understand what you mean. In any case, the necessary conversion is not big. I wrote it (for ehci-hcd) today. I will post it as a series of 3 RFCs following this email, so you can see how straightforward it is. The patches apply to a kernel with none of the existing tasklet changes. (There is one awkward aspect. It turns out that ehci_urb_done() can sometimes be called in hardirq context. This can happen when the I/O watchdog hrtimer routine completes an URB, or when an URB is unlinked while waiting for a Clear-TT-Buffer request. It's rare, but not impossible, and when it happens we are forced to leave interrupts disabled during the entire giveback.) Would you like to try testing these patches? I don't many high-speed devices here right now, but you can compare the performance with the measurements you made earlier. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC 1/3] EHCI: store the interrupt mask in memory
This patch adds a new field to the ehci-hcd private structure, for holding the value of the EHCI Interrupt Enable register, together with an inline routine for setting this value. The ehci-intr-enable field will used in a following patch, when a tasklet's bottom-half handler needs to restore the correct interrupt mask. Not-yet-signed-off-by: Alan Stern st...@rowland.harvard.edu --- drivers/usb/host/ehci-hcd.c | 21 ++--- drivers/usb/host/ehci-hub.c |6 +++--- drivers/usb/host/ehci-timer.c |2 +- drivers/usb/host/ehci.h |1 + 4 files changed, 19 insertions(+), 11 deletions(-) Index: usb-3.11/drivers/usb/host/ehci-hcd.c === --- usb-3.11.orig/drivers/usb/host/ehci-hcd.c +++ usb-3.11/drivers/usb/host/ehci-hcd.c @@ -134,6 +134,12 @@ static inline unsigned ehci_read_frame_i return ehci_readl(ehci, ehci-regs-frame_index); } +static inline void ehci_set_intr_enable(struct ehci_hcd *ehci, u32 mask) +{ + ehci-intr_mask = mask; + ehci_writel(ehci, mask, ehci-regs-intr_enable); +} + #include ehci-dbg.c /*-*/ @@ -194,7 +200,7 @@ static int ehci_halt (struct ehci_hcd *e spin_lock_irq(ehci-lock); /* disable any irqs left enabled by previous code */ - ehci_writel(ehci, 0, ehci-regs-intr_enable); + ehci_set_intr_enable(ehci, 0); if (ehci_is_TDI(ehci) !tdi_in_host_mode(ehci)) { spin_unlock_irq(ehci-lock); @@ -639,8 +645,9 @@ static int ehci_run (struct usb_hcd *hcd temp 8, temp 0xff, ignore_oc ? , overcurrent ignored : ); - ehci_writel(ehci, INTR_MASK, - ehci-regs-intr_enable); /* Turn On Interrupts */ + spin_lock_irq(ehci-lock); + ehci_set_intr_enable(ehci, INTR_MASK); /* Turn on interrupts */ + spin_unlock_irq(ehci-lock); /* GRR this is run-once init(), being done every time the HC starts. * So long as they're part of class devices, we can't do it init() @@ -817,7 +824,7 @@ dead: ehci-rh_state = EHCI_RH_STOPPING; ehci-command = ~(CMD_RUN | CMD_ASE | CMD_PSE); ehci_writel(ehci, ehci-command, ehci-regs-command); - ehci_writel(ehci, 0, ehci-regs-intr_enable); + ehci_set_intr_enable(ehci, 0); ehci_handle_controller_death(ehci); /* Handle completions when the controller stops */ @@ -1080,7 +1087,7 @@ int ehci_suspend(struct usb_hcd *hcd, bo ehci_prepare_ports_for_controller_suspend(ehci, do_wakeup); spin_lock_irq(ehci-lock); - ehci_writel(ehci, 0, ehci-regs-intr_enable); + ehci_set_intr_enable(ehci, 0); (void) ehci_readl(ehci, ehci-regs-intr_enable); clear_bit(HCD_FLAG_HW_ACCESSIBLE, hcd-flags); @@ -,7 +1118,7 @@ int ehci_resume(struct usb_hcd *hcd, boo */ if (ehci_readl(ehci, ehci-regs-configured_flag) == FLAG_CF !hibernated) { - int mask = INTR_MASK; + u32 mask = INTR_MASK; ehci_prepare_ports_for_controller_resume(ehci); @@ -1121,7 +1128,7 @@ int ehci_resume(struct usb_hcd *hcd, boo if (!hcd-self.root_hub-do_remote_wakeup) mask = ~STS_PCD; - ehci_writel(ehci, mask, ehci-regs-intr_enable); + ehci_set_intr_enable(ehci, mask); ehci_readl(ehci, ehci-regs-intr_enable); skip: spin_unlock_irq(ehci-lock); Index: usb-3.11/drivers/usb/host/ehci-hub.c === --- usb-3.11.orig/drivers/usb/host/ehci-hub.c +++ usb-3.11/drivers/usb/host/ehci-hub.c @@ -352,7 +352,7 @@ static int ehci_bus_suspend (struct usb_ mask = INTR_MASK; if (!hcd-self.root_hub-do_remote_wakeup) mask = ~STS_PCD; - ehci_writel(ehci, mask, ehci-regs-intr_enable); + ehci_set_intr_enable(ehci, mask); ehci_readl(ehci, ehci-regs-intr_enable); done: @@ -401,7 +401,7 @@ static int ehci_bus_resume (struct usb_h /* at least some APM implementations will try to deliver * IRQs right away, so delay them until we're ready. */ - ehci_writel(ehci, 0, ehci-regs-intr_enable); + ehci_set_intr_enable(ehci, 0); /* re-init operational registers */ ehci_writel(ehci, 0, ehci-regs-segment); @@ -495,7 +495,7 @@ static int ehci_bus_resume (struct usb_h spin_lock_irq(ehci-lock); if (ehci-shutdown) goto shutdown; - ehci_writel(ehci, INTR_MASK, ehci-regs-intr_enable); + ehci_set_intr_enable(ehci, INTR_MASK); (void) ehci_readl(ehci, ehci-regs-intr_enable); spin_unlock_irq(ehci-lock); Index: usb-3.11/drivers/usb/host/ehci.h
[RFC 2/3] EHCI: convert the IRQ handler to a tasklet
This patch divides ehci-hcd's interrupt handler into a top half and a bottom half, using a tasklet to execute the latter. The conversion is very straightforward. The only subtle point is that we have to ignore interrupts that arrive while the tasklet is running (i.e., from another device on a shared IRQ line). Not-yet-signed-off-by: Alan Stern st...@rowland.harvard.edu --- drivers/usb/host/ehci-hcd.c | 56 drivers/usb/host/ehci.h |4 +++ 2 files changed, 51 insertions(+), 9 deletions(-) Index: usb-3.11/drivers/usb/host/ehci.h === --- usb-3.11.orig/drivers/usb/host/ehci.h +++ usb-3.11/drivers/usb/host/ehci.h @@ -98,6 +98,10 @@ enum ehci_hrtimer_event { #define EHCI_HRTIMER_NO_EVENT 99 struct ehci_hcd { /* one per controller */ + /* tasklet and IRQ support */ + struct tasklet_struct tasklet; + boolirqs_are_masked; + /* timing support */ enum ehci_hrtimer_event next_hrtimer_event; unsignedenabled_hrtimer_events; Index: usb-3.11/drivers/usb/host/ehci-hcd.c === --- usb-3.11.orig/drivers/usb/host/ehci-hcd.c +++ usb-3.11/drivers/usb/host/ehci-hcd.c @@ -140,6 +140,8 @@ static inline void ehci_set_intr_enable( ehci_writel(ehci, mask, ehci-regs-intr_enable); } +static void ehci_tasklet_routine(unsigned long _ehci); + #include ehci-dbg.c /*-*/ @@ -218,6 +220,7 @@ static int ehci_halt (struct ehci_hcd *e spin_unlock_irq(ehci-lock); synchronize_irq(ehci_to_hcd(ehci)-irq); + tasklet_kill(ehci-tasklet); return ehci_handshake(ehci, ehci-regs-status, STS_HALT, STS_HALT, 16 * 125); @@ -468,6 +471,8 @@ static int ehci_init(struct usb_hcd *hcd struct ehci_qh_hw *hw; spin_lock_init(ehci-lock); + tasklet_init(ehci-tasklet, ehci_tasklet_routine, + (unsigned long) ehci); /* * keep io watchdog by default, those good HCDs could turn off it later @@ -691,13 +696,40 @@ EXPORT_SYMBOL_GPL(ehci_setup); /*-*/ -static irqreturn_t ehci_irq (struct usb_hcd *hcd) +static irqreturn_t ehci_irq(struct usb_hcd *hcd) { struct ehci_hcd *ehci = hcd_to_ehci (hcd); + u32 status, masked_status; + + if (ehci-irqs_are_masked) + return IRQ_NONE; + + /* +* We don't use STS_FLR, but some controllers don't like it to +* remain on, so mask it out along with the other status bits. +*/ + status = ehci_readl(ehci, ehci-regs-status); + masked_status = status (INTR_MASK | STS_FLR); + + /* Shared IRQ? */ + if (!masked_status || unlikely(ehci-rh_state == EHCI_RH_HALTED)) + return IRQ_NONE; + + /* Mask IRQs and let the tasklet do the work */ + ehci_writel(ehci, 0, ehci-regs-intr_enable); + ehci-irqs_are_masked = true; + tasklet_hi_schedule(ehci-tasklet); + return IRQ_HANDLED; +} + +static void ehci_tasklet_routine(unsigned long _ehci) +{ + struct ehci_hcd *ehci = (struct ehci_hcd *) _ehci; + struct usb_hcd *hcd = ehci_to_hcd(ehci); u32 status, masked_status, pcd_status = 0, cmd; int bh; - spin_lock (ehci-lock); + spin_lock_irq(ehci-lock); status = ehci_readl(ehci, ehci-regs-status); @@ -713,11 +745,9 @@ static irqreturn_t ehci_irq (struct usb_ */ masked_status = status (INTR_MASK | STS_FLR); - /* Shared IRQ? */ - if (!masked_status || unlikely(ehci-rh_state == EHCI_RH_HALTED)) { - spin_unlock(ehci-lock); - return IRQ_NONE; - } + /* IRQ already handled? */ + if (!masked_status || unlikely(ehci-rh_state == EHCI_RH_HALTED)) + goto done; /* clear (just) interrupts */ ehci_writel(ehci, masked_status, ehci-regs-status); @@ -833,10 +863,16 @@ dead: if (bh) ehci_work (ehci); - spin_unlock (ehci-lock); + + done: + /* Unmask IRQs again */ + ehci-irqs_are_masked = false; + smp_wmb(); /* Make sure ehci_irq() sees that assignment */ + ehci_writel(ehci, ehci-intr_mask, ehci-regs-intr_enable); + + spin_unlock_irq(ehci-lock); if (pcd_status) usb_hcd_poll_rh_status(hcd); - return IRQ_HANDLED; } /*-*/ @@ -1093,6 +1129,8 @@ int ehci_suspend(struct usb_hcd *hcd, bo clear_bit(HCD_FLAG_HW_ACCESSIBLE, hcd-flags);
[RFC 3/3] EHCI: enable interrupts during URB giveback
This patch modifies the usb_hcd_giveback_urb() routine in usbcore to insure that interrupts will be disabled when an URB's completion handler is called, even if they were enabled initially. It also modifies ehci-hcd to enable interrupts when giving back URBs, if possible. Unfortunately it isn't always possible, because there are pathways that can give back URBs in hardirq context (during the I/O watchdog hrtimer routine, for example). Signed-off-by: Alan Stern st...@rowland.harvard.edu --- drivers/usb/core/hcd.c|4 drivers/usb/host/ehci-q.c | 12 ++-- 2 files changed, 14 insertions(+), 2 deletions(-) Index: usb-3.11/drivers/usb/core/hcd.c === --- usb-3.11.orig/drivers/usb/core/hcd.c +++ usb-3.11/drivers/usb/core/hcd.c @@ -1667,6 +1667,8 @@ int usb_hcd_unlink_urb (struct urb *urb, */ void usb_hcd_giveback_urb(struct usb_hcd *hcd, struct urb *urb, int status) { + unsigned long flags; + urb-hcpriv = NULL; if (unlikely(urb-unlinked)) status = urb-unlinked; @@ -1681,7 +1683,9 @@ void usb_hcd_giveback_urb(struct usb_hcd /* pass ownership to the completion handler */ urb-status = status; + local_irq_save(flags); urb-complete (urb); + local_irq_restore(flags); atomic_dec (urb-use_count); if (unlikely(atomic_read(urb-reject))) wake_up (usb_kill_urb_queue); Index: usb-3.11/drivers/usb/host/ehci-q.c === --- usb-3.11.orig/drivers/usb/host/ehci-q.c +++ usb-3.11/drivers/usb/host/ehci-q.c @@ -257,6 +257,8 @@ ehci_urb_done(struct ehci_hcd *ehci, str __releases(ehci-lock) __acquires(ehci-lock) { + boolenable_ints = !in_irq(); + if (usb_pipetype(urb-pipe) == PIPE_INTERRUPT) { /* ... update hc-wide periodic stats */ ehci_to_hcd(ehci)-self.bandwidth_int_reqs--; @@ -283,9 +285,15 @@ __acquires(ehci-lock) /* complete() can reenter this HCD */ usb_hcd_unlink_urb_from_ep(ehci_to_hcd(ehci), urb); - spin_unlock (ehci-lock); + + /* This routine can be called from hardirq or softirq context */ + spin_unlock(ehci-lock); + if (enable_ints) + local_irq_enable(); usb_hcd_giveback_urb(ehci_to_hcd(ehci), urb, status); - spin_lock (ehci-lock); + if (enable_ints) + local_irq_disable(); + spin_lock(ehci-lock); } static int qh_schedule (struct ehci_hcd *ehci, struct ehci_qh *qh); -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/3] usb: phy: Add Qualcomm SS-USB and HS-USB drivers for DWC3 core
On Wed, Aug 21, 2013 at 10:13:28AM -0500, Kumar Gala wrote: On Aug 21, 2013, at 8:06 AM, Ivan T. Ivanov wrote: On Tue, 2013-08-20 at 18:01 +0100, Pawel Moll wrote: On Tue, 2013-08-20 at 16:06 +0100, Pawel Moll wrote: On Tue, 2013-08-20 at 16:01 +0100, Kumar Gala wrote: On Aug 20, 2013, at 9:54 AM, Ivan T. Ivanov wrote: Hi, On Tue, 2013-08-20 at 09:33 -0500, Felipe Balbi wrote: On Tue, Aug 20, 2013 at 05:09:11PM +0300, Ivan T. Ivanov wrote: Hi, On Tue, 2013-08-20 at 08:37 -0500, Felipe Balbi wrote: Hi, On Tue, Aug 20, 2013 at 04:32:23PM +0300, Ivan T. Ivanov wrote: On Tue, Aug 20, 2013 at 12:56:04PM +0300, Ivan T. Ivanov wrote: From: Ivan T. Ivanov iiva...@mm-sol.com These drivers handles control and configuration of the HS and SS USB PHY transceivers. They are part of the driver which manage Synopsys DesignWare USB3 controller stack inside Qualcomm SoC's. Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com --- drivers/usb/phy/Kconfig | 11 ++ drivers/usb/phy/Makefile |2 + drivers/usb/phy/phy-msm-dwc3-hs.c | 327 drivers/usb/phy/phy-msm-dwc3-ss.c | 374 + please rename these PHY drivers, they have nothing to do with DWC3. PHYs don't care about the USB controller. I think they are SNPS DesignWare PHY's, additionally wrapped with Qualcomm logic. I could substitute dwc3 with just dw, which will be more correct. alright, thank you. Let's add Paul to the loop since he might have very good insight in the synopsys PHYs. mental note: if any other platform shows up with Synopsys PHY, ask them to use this driver instead :-) I really doubt that this will bi possible. Control of the PHY's is not directly trough ULPI, UTMI or PIPE3 interfaces, but trough QSCRATCH registers, which of course is highly Qualcomm specific. isn't it a memory mapped IP ? doesn't synopsys provide their own set of registers ? From what I see it is not directly mapped. How QSCRATCH write and reads transactions are translated to DW IP is unclear to me. I think the question is how does SW access them? I afraid the answer may be: it depends on the SOC. In my past I had to initialize a (SATA) PHY by implementing a software JTAG state machine, as the PHY's registers were not memory mapped *at all*. And the IP itself came from Synopsys, Cadence or yet another EDA company... Having said all that... If the PHY's spec at least defined layout of the registers in question and driver was using regmap API to talk to the device (initially regmap-mmio), it has some chances to become universal. The SOCs designed like my one would have to provide a custom regmap implementation. Sound reasonable. Unfortunately I don't have PHY's IP spec. Looking into this it appears that the register wrapper around the IP is most likely highly specific to qualcomm as I'm not seeing a register interface around the DWC HS PHY. Then it's set, we'll go with this driver :-) -- balbi signature.asc Description: Digital signature
RE: [PATCH v4 2/3] usb: phy: Add Qualcomm SS-USB and HS-USB drivers for DWC3 core
From: Ivan T. Ivanov [mailto:iiva...@mm-sol.com] Sent: Tuesday, August 20, 2013 8:26 AM On Tue, 2013-08-20 at 10:01 -0500, Kumar Gala wrote: On Aug 20, 2013, at 9:54 AM, Ivan T. Ivanov wrote: Hi, On Tue, 2013-08-20 at 09:33 -0500, Felipe Balbi wrote: On Tue, Aug 20, 2013 at 05:09:11PM +0300, Ivan T. Ivanov wrote: On Tue, 2013-08-20 at 08:37 -0500, Felipe Balbi wrote: On Tue, Aug 20, 2013 at 04:32:23PM +0300, Ivan T. Ivanov wrote: I think they are SNPS DesignWare PHY's, additionally wrapped with Qualcomm logic. I could substitute dwc3 with just dw, which will be more correct. alright, thank you. Let's add Paul to the loop since he might have very good insight in the synopsys PHYs. mental note: if any other platform shows up with Synopsys PHY, ask them to use this driver instead :-) I really doubt that this will bi possible. Control of the PHY's is not directly trough ULPI, UTMI or PIPE3 interfaces, but trough QSCRATCH registers, which of course is highly Qualcomm specific. isn't it a memory mapped IP ? doesn't synopsys provide their own set of registers ? From what I see it is not directly mapped. How QSCRATCH write and reads transactions are translated to DW IP is unclear to me. I think the question is how does SW access them? USB QSCRATCH Hardware registers don't ask me what is this :-) or like Pawel says: it depends on the SOC . To answer the question doesn't synopsys provide their own set of registers, we provide registers in our USB cores to access the PHYs through I2C, ULPI/UTMI, or PIPE3 interfaces. But if someone wants to use our PHY with some other controller that doesn't provide that, then they may need to implement their own register set, as Qualcomm has apparently done. -- Paul
Re: [RFT RFC] USB: Fix USB device disconnects on resume.
On Thu, Aug 22, 2013 at 10:42:49AM -0400, Alan Stern wrote: On Wed, 21 Aug 2013, Sarah Sharp wrote: Possible fixes -- The USB core obviously needs to be changed to check the port status after the TRSMRCY timeout, and continue to wait if the port is still in the resuming state. I will have to study the EHCI port status diagrams in detail to figure out how the USB core can do this. As far as EHCI is concerned, this is a non-problem. The closest analogy to the RExit-U0 transition is in the description of the Force Port Resume bit (bit 6) in Table 2-16 of the EHCI spec, where it says that the host controller must complete the transition to the high-speed idle state within 2 milliseconds of software setting the bit to a zero (which happens when the hub driver does its Get-Port-Status call). Thus, as soon as the TRSMRCY delay is finished, the device and the port are supposed to be ready. In fact, the hardware doesn't provide any means of telling whether they are ready or not. Well, shoot, I thought I had solved world hunger, or at least USB power management issues. :) So basically it sounds like this is an xHCI specific issue, and probably not the root cause of the USB device disconnects we see under EHCI hosts. I guess the xHCI hardware engineers just assumed software would always wait for the interrupt from the port status change event, rather than using a simple 10 ms timer. I bet they didn't even realize that that the transition took longer than 10ms, because Windows waited for the port status change event. I can easily do this without the USB core being involved, by changing the xHCI driver to either: 1. Busy wait with xhci_handshake() in the xHCI get port status until the port is in U0. 2. Add a completion per xHCI port. In xHCI get port status, initiate U0 entry, and wait on the port's completion for up to 20 ms. In the port status change event code, complete that port's completion when the port is in U0 and the bus_state-resuming_ports bit is set. I would expect either of those to be adequate. I right, I think I'll do the busy wait, since 71% of the time it should return immediately. Completions are overkill here. Should I print a debugging message if the xHCI host exceeds 10ms? I would be nice to let hardware engineers know they're out of spec. Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2 2/2] xhci: refactor handle_cmd_completion() switch branches into functions
On Thu, Aug 22, 2013 at 08:06:17PM +, Paul Zimmerman wrote: From: Xenia Ragiadakou Sent: Thursday, August 22, 2013 8:20 AM This patch refactors the switch statement in handle_cmd_completion() by creating a separate function for each branch, named 'xhci_handle_cmd_type'. Other changes introduced by this patch are: - Renaming the already existed functions by adding the prefix 'xhci_handle_cmd_' for consistency. Also, for handle_stopped_endpoint() the order of arguments was changed too, so that the prototype of 'xhci_handle_cmd_' be similar and easy to follow. - Additional local variables were added, such as cmd_trb, cmd_comp_code, cmd_type, add_flags and drop_flags, and the label 'update_ring', mainly to reduce code dublication and line length. - The variable ep_ring, that was assigned in the case TRB_CONFIG_EP but never used, was removed. Just a small suggestion, I think it would be better to name these functions to indicate they handle completion events, instead of commands. So maybe xhci_handle_stop_ep_complete, xhci_handle_set_deq_complete, xhci_handle_reset_ep_complete etc? Feel free to ignore me if you disagree :) I think that would be confusing, because some of the functions that handle command _completion_ events then go on to _complete_ the xhci_command-completion struct. So let's just drop the word complete from the function names at all. You queue xHCI commands with the xhci_queue_cmd functions, and you handle the commands being done in xhci_handle_cmd_cmd functions. Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFT RFC] USB: Fix USB device disconnects on resume.
On Thu, Aug 22, 2013 at 02:49:07PM -0700, Sarah Sharp wrote: On Thu, Aug 22, 2013 at 10:42:49AM -0400, Alan Stern wrote: On Wed, 21 Aug 2013, Sarah Sharp wrote: Possible fixes -- The USB core obviously needs to be changed to check the port status after the TRSMRCY timeout, and continue to wait if the port is still in the resuming state. I will have to study the EHCI port status diagrams in detail to figure out how the USB core can do this. As far as EHCI is concerned, this is a non-problem. The closest analogy to the RExit-U0 transition is in the description of the Force Port Resume bit (bit 6) in Table 2-16 of the EHCI spec, where it says that the host controller must complete the transition to the high-speed idle state within 2 milliseconds of software setting the bit to a zero (which happens when the hub driver does its Get-Port-Status call). Thus, as soon as the TRSMRCY delay is finished, the device and the port are supposed to be ready. In fact, the hardware doesn't provide any means of telling whether they are ready or not. Well, shoot, I thought I had solved world hunger, or at least USB power management issues. :) So basically it sounds like this is an xHCI specific issue, and probably not the root cause of the USB device disconnects we see under EHCI hosts. I guess the xHCI hardware engineers just assumed software would always wait for the interrupt from the port status change event, rather than using a simple 10 ms timer. I bet they didn't even realize that that the transition took longer than 10ms, because Windows waited for the port status change event. Why can't Linux do the same thing, and not worry about any timeout at all? Should I print a debugging message if the xHCI host exceeds 10ms? I would be nice to let hardware engineers know they're out of spec. Sure, but the odds of anyone of them enabling debugging, and then noticing this are slim to none. But if it makes us feel better pointing out hardware bugs (I know it makes me feel good), please do so. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: xhci: implement AMD remote wakeup quirk
Hi Huang, Thanks for notifying us of this issue and submitting a patch to attempt to fix it. However, the patches look very odd to me, and I don't understand the details of what they're trying to fix, so you will have to help me understand what you're trying to do. On Thu, Aug 22, 2013 at 11:56:22PM +0800, Huang Rui wrote: The following patch is required to resolve remote wake issues with certain devices. Issue description: If the remote wake is issued from the device in a specific timing condition while the system is entering sleep state then it may cause system to auto wake on subsequent sleep cycle. What do you mean by subsequent sleep cycle? Do you mean that if I enable a device to wake the system on resume, and the system goes into S3 while the device signals a wake, then the system doesn't wakeup? And then the system wakes up erroneously on the next sleep cycle? I'm confused, please explain this issue in more detail. Root cause: Host controller rebroadcasts the Resume signal 100 µseconds after receiving the original resume event from the device. For proper function, some devices may require the rebroadcast of resume event within the USB spec of 100µS. As Alan said, why only _some_ devices? The host isn't compliant with the spec, and you see that behavior happen on low speed mice. That doesn't mean the issue won't occur with other USB devices that implement remote wakeup. Did you test with, say, a high speed 3G modem or a USB bluetooth adapter to see if you can trigger the remote wakeup issue? This patch is only for xHCI driver and the quirk will be also added into OHCI driver too. This should be backported to kernels as old as 3.9, that contrain the commit 3f5eb14135ba9d97ba4b8514fc7ef5e0dac2abf4 usb: add find_raw_port_number callback to struct hc_driver(). Why that kernel in particular? That commit seems to have nothing to do with your current patch, aside from the fact that it will make it harder to back port. This problem will show up in any kernel with AMD xHCI host support, right? So why aren't you suggesting backporting to kernels that added older AMD quirks? More comments below. Cc sta...@vger.kernel.org Signed-off-by: Huang Rui ray.hu...@amd.com --- drivers/usb/core/hub.c| 28 drivers/usb/host/pci-quirks.c | 17 +++- drivers/usb/host/pci-quirks.h | 1 + drivers/usb/host/xhci-pci.c | 4 +++ drivers/usb/host/xhci.c | 61 +++ drivers/usb/host/xhci.h | 25 +- include/linux/usb.h | 1 + 7 files changed, 124 insertions(+), 13 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 175179e..117196c 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -27,6 +27,7 @@ #include linux/freezer.h #include linux/random.h #include linux/pm_qos.h +#include linux/hid.h #include asm/uaccess.h #include asm/byteorder.h @@ -5437,3 +5438,30 @@ acpi_handle usb_get_hub_port_acpi_handle(struct usb_device *hdev, return DEVICE_ACPI_HANDLE(hub-ports[port1 - 1]-dev); } #endif + +bool is_usb_mouse(struct usb_device *udev) +{ + struct usb_interface *intf; + struct usb_interface_descriptor intf_desc; + + if (!udev) { + dev_warn(udev-dev, Warn: no device attached!\n); + return false; + } + + intf = usb_ifnum_to_if(udev, 0); + intf_desc = intf-cur_altsetting-desc; + if (intf_desc.bInterfaceClass == USB_INTERFACE_CLASS_HID + intf_desc.bInterfaceSubClass == + USB_INTERFACE_SUBCLASS_BOOT + intf_desc.bInterfaceProtocol == + USB_INTERFACE_PROTOCOL_MOUSE) { + dev_dbg(udev-dev, The attached usb device is + mouse\n); + return true; + } + + dev_dbg(udev-dev, The attached usb device is NOT mouse\n); + return false; +} +EXPORT_SYMBOL_GPL(is_usb_mouse); diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c index 2c76ef1..218e421 100644 --- a/drivers/usb/host/pci-quirks.c +++ b/drivers/usb/host/pci-quirks.c @@ -140,9 +140,11 @@ int usb_amd_find_chipset_info(void) rev = info.smbus_dev-revision; if (rev = 0x11 rev = 0x18) info.sb_type = 2; + if (rev = 0x39 rev = 0x3a) + info.sb_type = 4; } - if (info.sb_type == 0) { + if (info.sb_type == 0 || info.sb_type == 4) { if (info.smbus_dev) { pci_dev_put(info.smbus_dev); info.smbus_dev = NULL; @@ -197,6 +199,19 @@ commit: } EXPORT_SYMBOL_GPL(usb_amd_find_chipset_info); +int usb_amd_remote_wakeup_quirk(void) +{ + u8 ret; + if (amd_chipset.sb_type == 4) { + ret = 1; + printk(KERN_DEBUG QUIRK:
Re: [RFT RFC] USB: Fix USB device disconnects on resume.
On Thu, 22 Aug 2013, Sarah Sharp wrote: As far as EHCI is concerned, this is a non-problem. The closest analogy to the RExit-U0 transition is in the description of the Force Port Resume bit (bit 6) in Table 2-16 of the EHCI spec, where it says that the host controller must complete the transition to the high-speed idle state within 2 milliseconds of software setting the bit to a zero (which happens when the hub driver does its Get-Port-Status call). Thus, as soon as the TRSMRCY delay is finished, the device and the port are supposed to be ready. In fact, the hardware doesn't provide any means of telling whether they are ready or not. Well, shoot, I thought I had solved world hunger, or at least USB power management issues. :) So basically it sounds like this is an xHCI specific issue, and probably not the root cause of the USB device disconnects we see under EHCI hosts. Probably not. After all, USB-2 does not have the elaborate link training and other link management features that USB-3 does. What device disconnects do you see under EHCI? I don't recall hearing about them. I guess the xHCI hardware engineers just assumed software would always wait for the interrupt from the port status change event, rather than using a simple 10 ms timer. I bet they didn't even realize that that the transition took longer than 10ms, because Windows waited for the port status change event. What do you mean? EHCI doesn't have any port-status change event at the end of the 10-ms TRSMRCY delay. The only port-status change event in the hardware is right at the beginning, when the resume is initiated. The same is true of UHCI, but OHCI is different. I can easily do this without the USB core being involved, by changing the xHCI driver to either: 1. Busy wait with xhci_handshake() in the xHCI get port status until the port is in U0. 2. Add a completion per xHCI port. In xHCI get port status, initiate U0 entry, and wait on the port's completion for up to 20 ms. In the port status change event code, complete that port's completion when the port is in U0 and the bus_state-resuming_ports bit is set. I would expect either of those to be adequate. I right, I think I'll do the busy wait, since 71% of the time it should return immediately. Completions are overkill here. Although that will be adequate, it may not be ideal. You'll still get a 10-ms TRSMRCY delay following the busy wait. Unless you change the hub driver to eliminate that delay for xHCI host controllers. Should I print a debugging message if the xHCI host exceeds 10ms? I would be nice to let hardware engineers know they're out of spec. It can't hurt. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: xhci: implement AMD remote wakeup quirk
On Thu, 22 Aug 2013, Sarah Sharp wrote: So, basically, whenever the xHCI host resumes from S3, you're going to reset mice attached to the ports. Again, why only mice? Alan, is there a way to force all devices under a host to have a reset resume quirk? That's not really what you want, because this issue arises only when waking up from a system suspend, not from a runtime suspend. Besides, it's still not clear (to me, at least) that resetting the device is necessary. However, if you really want to, you can force every device on a bus to go through reset-resume during a system resume by calling usb_root_hub_lost_power() when the controller or root hub resumes. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] fs: supply inode uid/gid setting interface
This patchset implements an accessor functions to set uid/gid in inode struct. Just finish code clean up. Rui Xiang (2): fs: implement inode uid/gid setting function fs: use inode_set_user to set uid/gid of inode arch/ia64/kernel/perfmon.c| 3 +-- arch/powerpc/platforms/cell/spufs/inode.c | 3 +-- arch/s390/hypfs/inode.c | 3 +-- drivers/infiniband/hw/qib/qib_fs.c| 3 +-- drivers/usb/gadget/f_fs.c | 3 +-- drivers/usb/gadget/inode.c| 5 +++-- fs/9p/vfs_inode.c | 6 ++ fs/adfs/inode.c | 3 +-- fs/affs/inode.c | 6 ++ fs/afs/inode.c| 6 ++ fs/anon_inodes.c | 3 +-- fs/autofs4/inode.c| 4 ++-- fs/befs/linuxvfs.c| 8 fs/ceph/caps.c| 5 +++-- fs/ceph/inode.c | 8 fs/cifs/inode.c | 6 ++ fs/configfs/inode.c | 3 +-- fs/debugfs/inode.c| 3 +-- fs/devpts/inode.c | 7 +++ fs/ext2/ialloc.c | 3 +-- fs/ext3/ialloc.c | 3 +-- fs/ext4/ialloc.c | 3 +-- fs/fat/inode.c| 6 ++ fs/fuse/control.c | 3 +-- fs/fuse/inode.c | 4 ++-- fs/hfs/inode.c| 6 ++ fs/hfsplus/inode.c| 3 +-- fs/hpfs/inode.c | 3 +-- fs/hpfs/namei.c | 12 fs/hugetlbfs/inode.c | 3 +-- fs/inode.c| 7 +++ fs/isofs/inode.c | 3 +-- fs/isofs/rock.c | 3 +-- fs/ncpfs/inode.c | 3 +-- fs/nfs/inode.c| 4 ++-- fs/ntfs/inode.c | 12 fs/ntfs/mft.c | 3 +-- fs/ntfs/super.c | 3 +-- fs/ocfs2/refcounttree.c | 3 +-- fs/omfs/inode.c | 3 +-- fs/pipe.c | 3 +-- fs/proc/base.c| 15 +-- fs/proc/fd.c | 8 fs/proc/inode.c | 3 +-- fs/proc/self.c| 3 +-- fs/stack.c| 3 +-- fs/sysfs/inode.c | 3 +-- fs/xfs/xfs_iops.c | 4 ++-- include/linux/fs.h| 1 + ipc/mqueue.c | 3 +-- kernel/cgroup.c | 3 +-- mm/shmem.c| 3 +-- net/socket.c | 3 +-- 53 files changed, 94 insertions(+), 142 deletions(-) -- 1.8.2.2 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] fs: implement inode uid/gid setting function
Supply a interface inode_set_user to set uid/gid of inode structs. Signed-off-by: Rui Xiang rui.xi...@huawei.com --- fs/inode.c | 7 +++ include/linux/fs.h | 1 + 2 files changed, 8 insertions(+) diff --git a/fs/inode.c b/fs/inode.c index e315c0a..3f90499 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -343,6 +343,13 @@ void inc_nlink(struct inode *inode) } EXPORT_SYMBOL(inc_nlink); +void inode_set_user(struct inode *inode, kuid_t uid, kgid_t gid) +{ + inode-i_uid = uid; + inode-i_gid = gid; +} +EXPORT_SYMBOL(inode_set_user); + void address_space_init_once(struct address_space *mapping) { memset(mapping, 0, sizeof(*mapping)); diff --git a/include/linux/fs.h b/include/linux/fs.h index 729e81b..36ac51b 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2619,6 +2619,7 @@ void __inode_sub_bytes(struct inode *inode, loff_t bytes); void inode_sub_bytes(struct inode *inode, loff_t bytes); loff_t inode_get_bytes(struct inode *inode); void inode_set_bytes(struct inode *inode, loff_t bytes); +void inode_set_user(struct inode *inode, kuid_t uid, kgid_t gid); extern int vfs_readdir(struct file *, filldir_t, void *); extern int iterate_dir(struct file *, struct dir_context *); -- 1.8.2.2 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] fs: use inode_set_user to set uid/gid of inode
Use the new interface to set i_uid/i_gid in inode struct. Signed-off-by: Rui Xiang rui.xi...@huawei.com --- arch/ia64/kernel/perfmon.c| 3 +-- arch/powerpc/platforms/cell/spufs/inode.c | 3 +-- arch/s390/hypfs/inode.c | 3 +-- drivers/infiniband/hw/qib/qib_fs.c| 3 +-- drivers/usb/gadget/f_fs.c | 3 +-- drivers/usb/gadget/inode.c| 5 +++-- fs/9p/vfs_inode.c | 6 ++ fs/adfs/inode.c | 3 +-- fs/affs/inode.c | 6 ++ fs/afs/inode.c| 6 ++ fs/anon_inodes.c | 3 +-- fs/autofs4/inode.c| 4 ++-- fs/befs/linuxvfs.c| 8 fs/ceph/caps.c| 5 +++-- fs/ceph/inode.c | 8 fs/cifs/inode.c | 6 ++ fs/configfs/inode.c | 3 +-- fs/debugfs/inode.c| 3 +-- fs/devpts/inode.c | 7 +++ fs/ext2/ialloc.c | 3 +-- fs/ext3/ialloc.c | 3 +-- fs/ext4/ialloc.c | 3 +-- fs/fat/inode.c| 6 ++ fs/fuse/control.c | 3 +-- fs/fuse/inode.c | 4 ++-- fs/hfs/inode.c| 6 ++ fs/hfsplus/inode.c| 3 +-- fs/hpfs/inode.c | 3 +-- fs/hpfs/namei.c | 12 fs/hugetlbfs/inode.c | 3 +-- fs/isofs/inode.c | 3 +-- fs/isofs/rock.c | 3 +-- fs/ncpfs/inode.c | 3 +-- fs/nfs/inode.c| 4 ++-- fs/ntfs/inode.c | 12 fs/ntfs/mft.c | 3 +-- fs/ntfs/super.c | 3 +-- fs/ocfs2/refcounttree.c | 3 +-- fs/omfs/inode.c | 3 +-- fs/pipe.c | 3 +-- fs/proc/base.c| 15 +-- fs/proc/fd.c | 8 fs/proc/inode.c | 3 +-- fs/proc/self.c| 3 +-- fs/stack.c| 3 +-- fs/sysfs/inode.c | 3 +-- fs/xfs/xfs_iops.c | 4 ++-- ipc/mqueue.c | 3 +-- kernel/cgroup.c | 3 +-- mm/shmem.c| 3 +-- net/socket.c | 3 +-- 51 files changed, 86 insertions(+), 142 deletions(-) diff --git a/arch/ia64/kernel/perfmon.c b/arch/ia64/kernel/perfmon.c index 5a9ff1c..73e1e55 100644 --- a/arch/ia64/kernel/perfmon.c +++ b/arch/ia64/kernel/perfmon.c @@ -2202,8 +2202,7 @@ pfm_alloc_file(pfm_context_t *ctx) DPRINT((new inode ino=%ld @%p\n, inode-i_ino, inode)); inode-i_mode = S_IFCHR|S_IRUGO; - inode-i_uid = current_fsuid(); - inode-i_gid = current_fsgid(); + inode_set_user(inode, current_fsuid(), current_fsgid()); /* * allocate a new dcache entry diff --git a/arch/powerpc/platforms/cell/spufs/inode.c b/arch/powerpc/platforms/cell/spufs/inode.c index 87ba7cf..4580c9b 100644 --- a/arch/powerpc/platforms/cell/spufs/inode.c +++ b/arch/powerpc/platforms/cell/spufs/inode.c @@ -101,8 +101,7 @@ spufs_new_inode(struct super_block *sb, umode_t mode) inode-i_ino = get_next_ino(); inode-i_mode = mode; - inode-i_uid = current_fsuid(); - inode-i_gid = current_fsgid(); + inode_set_user(inode, current_fsuid(), current_fsgid()); inode-i_atime = inode-i_mtime = inode-i_ctime = CURRENT_TIME; out: return inode; diff --git a/arch/s390/hypfs/inode.c b/arch/s390/hypfs/inode.c index 7a539f4..742e430 100644 --- a/arch/s390/hypfs/inode.c +++ b/arch/s390/hypfs/inode.c @@ -103,8 +103,7 @@ static struct inode *hypfs_make_inode(struct super_block *sb, umode_t mode) struct hypfs_sb_info *hypfs_info = sb-s_fs_info; ret-i_ino = get_next_ino(); ret-i_mode = mode; - ret-i_uid = hypfs_info-uid; - ret-i_gid = hypfs_info-gid; + inode_set_user(ret, hypfs_info-uid, hypfs_info-gid); ret-i_atime = ret-i_mtime = ret-i_ctime = CURRENT_TIME; if (S_ISDIR(mode)) set_nlink(ret, 2); diff --git a/drivers/infiniband/hw/qib/qib_fs.c b/drivers/infiniband/hw/qib/qib_fs.c index f247fc6e..6683837 100644 --- a/drivers/infiniband/hw/qib/qib_fs.c +++ b/drivers/infiniband/hw/qib/qib_fs.c @@ -61,13 +61,12 @@ static int qibfs_mknod(struct inode *dir, struct
Re: [PATCH v1] USB: EHCI: disable IAA_WATCHDOG and START_UNLINK_INTR if they needn't to be handled
On Fri, Aug 23, 2013 at 4:36 AM, Alan Stern st...@rowland.harvard.edu wrote: On Thu, 22 Aug 2013, Ming Lei wrote: This seems to be the most important factor. When you think about it, though, does it really minimize the changes? Consider all the other adjustments we had to make to ehci-hcd: the interrupt QH unlink change and the isochronous stream stuff (which also affects the usb-audio drivers). For other HCDs, this changes on interrupt QH unlink may not need at all if there is no much cost about relink. But for some HCDs, it will be needed. Which HCDs need them? Even for EHCI HCD, the interrupt QH unlink change isn't necessary, and the patch is only for improving the situation, isn't it? For other HCDs, basically unlink interrupt queue need't the wait time, so don't need the change. About isochronous stream stuff, the only change with moving giveback in tasklet is that iso_stream_schedule() may see list_empty(stream-td_list) when underrun, and we can solve it easily, can't we? No, it's not so easy. I have done some work on it, and it's more complicated than you might think. Not to mention that the snd-usb-audio driver (and perhaps others) will also need to be modified. As I said, the only change introduced with running giveback in tasklet is that iso_stream_schedule() may see list_empty(stream-td_list) when underrun, and we can handle it inside HCD and usbcore, nothing to do with drivers. OK, I give you a simple solution in which only one line change is needed for these HCDs, see attachment patch. Also I remembered that you said the isochronous stream stuff needn't to consider on other HCDs. No, I didn't say that. It will have to be considered for ohci-hcd and uhci-hcd. Maybe I understand it incorrectly: We also don't have to change the isochronous code in every HCD http://www.spinics.net/lists/linux-usb/msg89826.html So maybe the change on ehci HCD is a bit much, but for other HCDs, the change is just a little. The other HCDs will have to change too. Maybe not quite as much as ehci-hcd, but more than you seem to think. Per my solution, only one line change for the affected HCD is enough. Another good point with moving giveback only to tasklet is deadlock immune during resubmissions, as you mentioned in below link: http://www.spinics.net/lists/linux-usb/msg88710.html That referred to giveback during submissions. It turns out that they aren't necessary in any case, although I didn't realize it at the time. I'm starting to think that moving the entire handler to a tasklet would have been better. Not sure, if so: - one thing is that the HCD private lock can't be held in interrupt handler any more, so new lock need to introduce, not mention each hard irq handler need to split to two parts. No new lock is needed -- the interrupt handler runs okay without one. Yes, the handler has to be split into two parts. But that's small and self-contained, and very few other changes are needed. How can you make sure the lockless hw operations in hard interrupt handler won't cause race with all other hw operations on host controller, anyway, previously hcd lock is held to operate on host controller, and no such race. We should be careful since the change might depends on each hardware internal things. - also it should have been better to avoid resubmissions in its giveback handler. Why? We are all set up to allow them now. Ruling them out won't make things much easier than they are already. Only for one example, we needn't consider race between qh unlink and completion inside HCD anymore, so for ehci, the below variable and related code can be removed: ehci-async_unlinking ehci-intr_unlinking qh-exception QH_STATE_COMPLETING Doesn't it make HCD simpler? Also moving only the giveback routine into a tasklet can avoid dropping HCD private lock during irq handler, which may simplify HCD code, and I have figured out ehci cleanup patches for this. I don't think we should make these changes. It's okay to keep the private lock during the giveback call, but let's not remove the other things. My feeling is that at some point we may indeed want to move the entire handler to a tasklet or a threaded IRQ. If that happens, all those simplifications would need to be undone. Better not to do them in the first place, since they add very little overhead. I think making HCDs simpler should have been welcome, :-) But it isn't simpler! Look at the extra code you had to add to ehci-hcd. It is now more complicated than it used to be, and the isochronous changes will make it even more so. Ruling out submissions during givebacks will make very little difference. See above. Also one potential big problem of moving entire handler to tasklet is the hardware race, previously the entire hander is run with interrupt disabled and the big HCD
[PATCH v3 3/4] staging: usbip: properly handle -l / --log option
This option is in the long options list, and it's handled in the option processing loop, but the optstring didn't include it. Signed-off-by: Anthony Foiani anthony.foi...@gmail.com --- drivers/staging/usbip/userspace/src/usbip.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/usbip/userspace/src/usbip.c b/drivers/staging/usbip/userspace/src/usbip.c index 69ac4b5..b4f299e 100644 --- a/drivers/staging/usbip/userspace/src/usbip.c +++ b/drivers/staging/usbip/userspace/src/usbip.c @@ -152,7 +152,7 @@ int main(int argc, char *argv[]) usbip_use_stderr = 1; opterr = 0; for (;;) { - opt = getopt_long(argc, argv, +dt:, opts, NULL); + opt = getopt_long(argc, argv, +dlt:, opts, NULL); if (opt == -1) break; -- 1.8.1.4 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 4/4] staging: usbip: fix up alignment in long options list
Re-align the option lists. Signed-off-by: Anthony Foiani anthony.foi...@gmail.com --- drivers/staging/usbip/userspace/src/usbip.c | 8 drivers/staging/usbip/userspace/src/usbipd.c | 12 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/staging/usbip/userspace/src/usbip.c b/drivers/staging/usbip/userspace/src/usbip.c index b4f299e..04a5f20 100644 --- a/drivers/staging/usbip/userspace/src/usbip.c +++ b/drivers/staging/usbip/userspace/src/usbip.c @@ -139,10 +139,10 @@ static int run_command(const struct command *cmd, int argc, char *argv[]) int main(int argc, char *argv[]) { static const struct option opts[] = { - { debug, no_argument, NULL, 'd' }, - { log, no_argument, NULL, 'l' }, - { tcp-port, required_argument, NULL, 't' }, - { NULL,0, NULL, 0 } + { debug,no_argument, NULL, 'd' }, + { log, no_argument, NULL, 'l' }, + { tcp-port, required_argument, NULL, 't' }, + { NULL, 0, NULL, 0 } }; char *cmd; diff --git a/drivers/staging/usbip/userspace/src/usbipd.c b/drivers/staging/usbip/userspace/src/usbipd.c index f41ba5b..1c76cfd 100644 --- a/drivers/staging/usbip/userspace/src/usbipd.c +++ b/drivers/staging/usbip/userspace/src/usbipd.c @@ -560,13 +560,13 @@ static int do_standalone_mode(int daemonize) int main(int argc, char *argv[]) { static const struct option longopts[] = { - { daemon, no_argument, NULL, 'D' }, - { debug, no_argument, NULL, 'd' }, - { pid, optional_argument, NULL, 'P' }, + { daemon, no_argument, NULL, 'D' }, + { debug,no_argument, NULL, 'd' }, + { pid, optional_argument, NULL, 'P' }, { tcp-port, required_argument, NULL, 't' }, - { help,no_argument, NULL, 'h' }, - { version, no_argument, NULL, 'v' }, - { NULL, 0, NULL, 0 } + { help, no_argument, NULL, 'h' }, + { version, no_argument, NULL, 'v' }, + { NULL, 0, NULL, 0 } }; enum { -- 1.8.1.4 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] fs: supply inode uid/gid setting interface
On Fri, Aug 23, 2013 at 10:48:36AM +0800, Rui Xiang wrote: This patchset implements an accessor functions to set uid/gid in inode struct. Just finish code clean up. Why? -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Cash Awaiting Pick Up
This is to re-notify you of the 500,000.00(Pounds) was deposited here by the Heritage Lottery Board in the western union office in your name is available for pickup. Contact this email: wutransfer...@xtra.co.nz for your M.T.C.N Numbers.Contact Person: Mrs. Hillary Florence Email: wutransfer...@xtra.co.nz Mr. Greg Steve Western Union Branch Coordinator. Transaction Managing Director. Phone: +447-035-927-183 . -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] fs: use inode_set_user to set uid/gid of inode
From: Rui Xiang rui.xi...@huawei.com Date: Fri, 23 Aug 2013 10:48:38 +0800 Use the new interface to set i_uid/i_gid in inode struct. Signed-off-by: Rui Xiang rui.xi...@huawei.com For the networking bits: Acked-by: David S. Miller da...@davemloft.net -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html