Re: [PATCH v2 2/3] usb: gadget: uvc: Set the V4L2 buffer field to V4L2_FIELD_NONE
On Fri, Mar 28, 2014 at 05:02:47PM +0100, Laurent Pinchart wrote: > The UVC gadget driver doesn't support interlaced video but left the > buffer field uninitialized. Set it to V4L2_FIELD_NONE. > > Signed-off-by: Laurent Pinchart Acked-by: Felipe Balbi > --- > drivers/usb/gadget/uvc_queue.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/usb/gadget/uvc_queue.c b/drivers/usb/gadget/uvc_queue.c > index 9ac4ffe1..305eb49 100644 > --- a/drivers/usb/gadget/uvc_queue.c > +++ b/drivers/usb/gadget/uvc_queue.c > @@ -380,6 +380,7 @@ static struct uvc_buffer *uvc_queue_next_buffer(struct > uvc_video_queue *queue, > else > nextbuf = NULL; > > + buf->buf.v4l2_buf.field = V4L2_FIELD_NONE; > buf->buf.v4l2_buf.sequence = queue->sequence++; > v4l2_get_timestamp(&buf->buf.v4l2_buf.timestamp); > > -- > 1.8.3.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 -- balbi signature.asc Description: Digital signature
INFORME.
Su contraseña caducará hoy para llenar formularios y enviar de inmediato para validar su dirección de e-mail. Nombre de usuario: . Contraseña anterior: . Nueva contraseña: gracias administrador del sistema -- 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/3] Fix USB deadlock caused by SCSI error handling
On Fri, 28 Mar 2014, James Bottomley wrote: > This is a set of three patches we agreed to a while ago to eliminate a > USB deadlock. I did rewrite the first patch, if it could be reviewed > and tested. I tested all three patches under the same conditions as before, and they all worked correctly. In the revised patch 1, the meaning of SCSI_EH_ABORT_SCHEDULED isn't entirely clear. This boils down to two questions, which I don't know the answers to: What should happen in scmd_eh_abort_handler() if scsi_host_eh_past_deadline() returns true and thereby prevents scsi_try_to_abort_cmd() from being called? The flag wouldn't get cleared then. What should happen if some other pathway manages to call scsi_try_to_abort_cmd() while scmd->abort_work is still sitting on the work queue? The command would be aborted and the flag would be cleared, but the queued work would remain. Can this ever happen? Maybe scmd_eh_abort_handler() should check the flag before doing anything. Is there any sort of sychronization to prevent the same incarnation of a command from being aborted twice (or by two different threads at the same time)? If there is, it isn't obvious. (Also, what's going on at the start of scsi_abort_command()? Contrary to what one might expect, the first part of the function _cancels_ a scheduled abort. And it does so without clearing the SCSI_EH_ABORT_SCHEDULED flag.) Maybe in all these scenarios, the command is doomed to fail anyway so these questions don't make any real difference. I don't know... 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: serial: fixed the product id typo
On Fri, 2014-03-28 at 15:25 +0800, AceLan Kao wrote:
> Fixed the product id typo and replace whitespace with tab.
>
> Signed-off-by: AceLan Kao
> ---
> drivers/usb/serial/sierra.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
> index a9eb622..b78fd4e 100644
> --- a/drivers/usb/serial/sierra.c
> +++ b/drivers/usb/serial/sierra.c
> @@ -291,7 +291,7 @@ static const struct usb_device_id id_table[] = {
> { USB_DEVICE(0x0f3d, 0x68A3), /* Airprime/Sierra Wireless Direct IP
> modems */
> .driver_info = (kernel_ulong_t)&direct_ip_interface_blacklist
> },
> - { USB_DEVICE(0x413C, 0x08133) }, /* Dell Computer Corp. Wireless 5720
> VZW Mobile Broadband (EVDO Rev-A) Minicard GPS Port */
> + { USB_DEVICE(0x413C, 0x8133) }, /* Dell Computer Corp. Wireless 5720
> VZW Mobile Broadband (EVDO Rev-A) Minicard GPS Port */
The Dell 5720 (Novatel E725) is not a Sierra device, so it should
certainly not remain in the 'sierra' driver and the original commit that
did this was in error. It's device ID should be moved to the 'option'
driver instead. So could you resend this patch, but move the device to
option.c and fix the typo there?
Thanks,
Dan
--
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] uas: fix GFP_NOIO under spinlock
On Fri, Mar 28, 2014 at 07:39:19PM +0100, Hans de Goede wrote: > Hi, > > On 03/28/2014 05:42 PM, Greg KH wrote: > > On Fri, Mar 28, 2014 at 09:48:00AM +0100, Hans de Goede wrote: > >> Hi, > >> > >> On 03/28/2014 09:45 AM, oli...@neukum.org wrote: > >>> From: Oliver Neukum > >>> > >>> Quote Dan: > >>> > >>> The patch e36e64930cff: "uas: Use GFP_NOIO rather then GFP_ATOMIC > >>> where possible" from Nov 7, 2013, leads to the following static > >>> checker warning: > >>> > >>> drivers/usb/storage/uas.c:806 uas_eh_task_mgmt() > >>> error: scheduling with locks held: 'spin_lock:lock' > >>> The fix is obvious. > >>> > >>> Signed-off-by: Oliver Neukum > >>> Reported-by: Dan Carpenter > >> > >> Dan, thanks for catching this, Oliver thanks for fixing this. > >> > >> Looks good to me, Greg can you please add this to usb-next? > >> It fixes a regression my uas set has introduced there. > > > > I'll queue it up after 3.15-rc1 is out, thanks. > > Great, thanks. Note Oliver has done a new version catching one > more instance (in essence reverting a patch I did where I > fsck-ed up). But I guess you've seen this already. Yeah, I'll work it out when it's time to start applying these patches, 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] uas: fix GFP_NOIO under spinlock
Hi, On 03/28/2014 05:42 PM, Greg KH wrote: > On Fri, Mar 28, 2014 at 09:48:00AM +0100, Hans de Goede wrote: >> Hi, >> >> On 03/28/2014 09:45 AM, oli...@neukum.org wrote: >>> From: Oliver Neukum >>> >>> Quote Dan: >>> >>> The patch e36e64930cff: "uas: Use GFP_NOIO rather then GFP_ATOMIC >>> where possible" from Nov 7, 2013, leads to the following static >>> checker warning: >>> >>> drivers/usb/storage/uas.c:806 uas_eh_task_mgmt() >>> error: scheduling with locks held: 'spin_lock:lock' >>> The fix is obvious. >>> >>> Signed-off-by: Oliver Neukum >>> Reported-by: Dan Carpenter >> >> Dan, thanks for catching this, Oliver thanks for fixing this. >> >> Looks good to me, Greg can you please add this to usb-next? >> It fixes a regression my uas set has introduced there. > > I'll queue it up after 3.15-rc1 is out, thanks. Great, thanks. Note Oliver has done a new version catching one more instance (in essence reverting a patch I did where I fsck-ed up). But I guess you've seen this already. Regards, Hans -- 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] USB:pl2303: add ids for Hewlett-Packard HP POS pole displays
Add device ids to pl2303 for the Hewlett-Packard HP POS pole displays:
LD960: 03f0:0B39
LCM220: 03f0:3139
LCM960: 03f0:3239
Signed-off-by: Aaron Sanders
diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
index 3b5ba4f..4d70809 100644
--- a/drivers/usb/serial/pl2303.c
+++ b/drivers/usb/serial/pl2303.c
@@ -82,6 +82,9 @@ static const struct usb_device_id id_table[] = {
{ USB_DEVICE(YCCABLE_VENDOR_ID, YCCABLE_PRODUCT_ID) },
{ USB_DEVICE(SUPERIAL_VENDOR_ID, SUPERIAL_PRODUCT_ID) },
{ USB_DEVICE(HP_VENDOR_ID, HP_LD220_PRODUCT_ID) },
+ { USB_DEVICE(HP_VENDOR_ID, HP_LD960_PRODUCT_ID) },
+ { USB_DEVICE(HP_VENDOR_ID, HP_LCM220_PRODUCT_ID) },
+ { USB_DEVICE(HP_VENDOR_ID, HP_LCM960_PRODUCT_ID) },
{ USB_DEVICE(CRESSI_VENDOR_ID, CRESSI_EDY_PRODUCT_ID) },
{ USB_DEVICE(ZEAGLE_VENDOR_ID, ZEAGLE_N2ITION3_PRODUCT_ID) },
{ USB_DEVICE(SONY_VENDOR_ID, SONY_QN3USB_PRODUCT_ID) },
diff --git a/drivers/usb/serial/pl2303.h b/drivers/usb/serial/pl2303.h
index c38b8c0..ec183d1 100644
--- a/drivers/usb/serial/pl2303.h
+++ b/drivers/usb/serial/pl2303.h
@@ -121,9 +121,12 @@
#define SUPERIAL_VENDOR_ID 0x5372
#define SUPERIAL_PRODUCT_ID0x2303
-/* Hewlett-Packard LD220-HP POS Pole Display */
-#define HP_VENDOR_ID 0x03f0
-#define HP_LD220_PRODUCT_ID0x3524
+//* Hewlett-Packard POS Pole Displays */
+ #define HP_VENDOR_ID 0x03f0
+ #define HP_LD220_PRODUCT_ID 0x3524
+ #define HP_LD960_PRODUCT_ID 0x0B39
+ #define HP_LCM220_PRODUCT_ID 0x3139
+ #define HP_LCM960_PRODUCT_ID 0x3239
/* Cressi Edy (diving computer) PC interface */
#define CRESSI_VENDOR_ID 0x04b8
--
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/serial/io_ti.c broken on BE systems
> At least if > you can verify that it still works on an LE system. Yes your patch works on a LE system (tested on 3.2.55) Thanks! -- Ludovic Drolez. http://www.aopensource.com - The Android Open Source Portal http://www.drolez.com - Personal site - Linux and Free Software -- 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/3] [SCSI] Fix abort state memory problem
The abort state is being stored persistently across commands, meaning if the
command times out a second time, the error handler thinks an abort is still
pending. Fix this by properly resetting the abort flag after the abort is
finished.
Signed-off-by: James Bottomley
---
drivers/scsi/scsi_error.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 771c16b..b9f3b07 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -869,10 +869,13 @@ static int scsi_try_bus_device_reset(struct scsi_cmnd
*scmd)
static int scsi_try_to_abort_cmd(struct scsi_host_template *hostt, struct
scsi_cmnd *scmd)
{
- if (!hostt->eh_abort_handler)
- return FAILED;
+ int retval = FAILED;
+
+ if (hostt->eh_abort_handler)
+ retval = hostt->eh_abort_handler(scmd);
- return hostt->eh_abort_handler(scmd);
+ scmd->eh_eflags &= ~SCSI_EH_ABORT_SCHEDULED;
+ return retval;
}
static void scsi_abort_eh_cmnd(struct scsi_cmnd *scmd)
--
1.9.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
[PATCH 2/3] [SCSI] Fix spurious request sense in error handling
We unconditionally execute scsi_eh_get_sense() to make sure all failed commands that should have sense attached, do. However, the routine forgets that some commands, because of the way they fail, will not have any sense code ... we should not bother them with a REQUEST_SENSE command. Fix this by testing to see if we actually got a CHECK_CONDITION return and skip asking for sense if we don't. Tested-by: Alan Stern Signed-off-by: James Bottomley --- drivers/scsi/scsi_error.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index b9f3b07..221a5bc 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -1160,6 +1160,15 @@ int scsi_eh_get_sense(struct list_head *work_q, __func__)); break; } + if (status_byte(scmd->result) != CHECK_CONDITION) + /* +* don't request sense if there's no check condition +* status because the error we're processing isn't one +* that has a sense code (and some devices get +* confused by sense requests out of the blue) +*/ + continue; + SCSI_LOG_ERROR_RECOVERY(2, scmd_printk(KERN_INFO, scmd, "%s: requesting sense\n", current->comm)); -- 1.9.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
[PATCH 3/3] [SCSI] Fix command result state propagation
From: Alan Stern
We're seeing a case where the contents of scmd->result isn't being reset after
a SCSI command encounters an error, is resubmitted, times out and then gets
handled. The error handler acts on the stale result of the previous error
instead of the timeout. Fix this by properly zeroing the scmd->status before
the command is resubmitted.
Signed-off-by: Alan Stern
Signed-off-by: James Bottomley
---
drivers/scsi/scsi_error.c | 1 +
drivers/scsi/scsi_lib.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 221a5bc..335eaf4 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -927,6 +927,7 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct
scsi_eh_save *ses,
memset(scmd->cmnd, 0, BLK_MAX_CDB);
memset(&scmd->sdb, 0, sizeof(scmd->sdb));
scmd->request->next_rq = NULL;
+ scmd->result = 0;
if (sense_bytes) {
scmd->sdb.length = min_t(unsigned, SCSI_SENSE_BUFFERSIZE,
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 5681c05..b1f4867 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -137,6 +137,7 @@ static void __scsi_queue_insert(struct scsi_cmnd *cmd, int
reason, int unbusy)
* lock such that the kblockd_schedule_work() call happens
* before blk_cleanup_queue() finishes.
*/
+ cmd->result = 0;
spin_lock_irqsave(q->queue_lock, flags);
blk_requeue_request(q, cmd->request);
kblockd_schedule_work(q, &device->requeue_work);
--
1.9.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
[PATCH 0/3] Fix USB deadlock caused by SCSI error handling
This is a set of three patches we agreed to a while ago to eliminate a USB deadlock. I did rewrite the first patch, if it could be reviewed and tested. Thanks, James --- Alan Stern (1): [SCSI] Fix command result state propagation James Bottomley (2): [SCSI] Fix abort state memory problem [SCSI] Fix spurious request sense in error handling drivers/scsi/scsi_error.c | 19 --- drivers/scsi/scsi_lib.c | 1 + 2 files changed, 17 insertions(+), 3 deletions(-) -- 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: XHCI ERROR: Transfer error on endpoint
On Thu, Mar 27, 2014 at 10:31:33AM +, Keerthy V wrote: > Hi, > > I am trying to send data to a USB device connected to a USB3.0 port over a > USB bulk end point using libusb APIs . It is failing failed with error > ERROR_IO. > > The same device and the test program works fine on a USB2.0 port. > > After enabling the XHCI debug options, I found that I am getting the > following errors: > "Endpoint xxx not halted, refusing to reset Transfer error on endpoint" Your userspace program is unconditionally sending a ClearHalt request when the endpoint is not stalled. That triggers a known bug in the xHCI driver, where the host controller's toggle bits and the device's toggle bits get out of sync. This then causes endless transfer errors when the host tries to communicate with that endpoint. The only way to fix this is to unplug and replug in the device. Mathias (xHCI maintainer) has plans to implement a fix for this, but it will probably not land until 3.16 or later. I suggest you remove any unconditional ClearHalt requests from your userspace program in the mean time. Sarah Sharp > I have tested in following kernel : > 3.13.7 > 3.11.0-18 > > Both gave the same results. The test has been performed on USB 3.0 host > controller. > > Can you please let me know the root cause and possible solutions? > > Attached is the debug log. > > Regards, > Keerthy > This message and any attachments may contain Cypress (or its subsidiaries) > confidential information. If it has been received in error, please advise the > sender and immediately delete this message. > -- > 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 -- 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 v2 3/3] usb: gadget: uvc: Set the vb2 queue timestamp flags
The vb2 queue timestamp_flags field must be set by drivers, as enforced by a WARN_ON in vb2_queue_init. The UVC gadget driver failed to do so. This resulted in the following warning. [2.104371] g_webcam gadget: uvc_function_bind [2.105567] [ cut here ] [2.105567] [ cut here ] [2.106779] WARNING: CPU: 0 PID: 1 at drivers/media/v4l2-core/videobuf2-core.c:2207 vb2_queue_init+0xa3/0x113() Fix it. Reported-by: Fengguang Wu Signed-off-by: Laurent Pinchart Reviewed-by: Hans Verkuil --- drivers/usb/gadget/uvc_queue.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/gadget/uvc_queue.c b/drivers/usb/gadget/uvc_queue.c index 305eb49..1c29bc9 100644 --- a/drivers/usb/gadget/uvc_queue.c +++ b/drivers/usb/gadget/uvc_queue.c @@ -137,6 +137,8 @@ static int uvc_queue_init(struct uvc_video_queue *queue, queue->queue.buf_struct_size = sizeof(struct uvc_buffer); queue->queue.ops = &uvc_queue_qops; queue->queue.mem_ops = &vb2_vmalloc_memops; + queue->queue.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC +| V4L2_BUF_FLAG_TSTAMP_SRC_EOF; ret = vb2_queue_init(&queue->queue); if (ret) return ret; -- 1.8.3.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 v2 2/3] usb: gadget: uvc: Set the V4L2 buffer field to V4L2_FIELD_NONE
The UVC gadget driver doesn't support interlaced video but left the buffer field uninitialized. Set it to V4L2_FIELD_NONE. Signed-off-by: Laurent Pinchart --- drivers/usb/gadget/uvc_queue.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/gadget/uvc_queue.c b/drivers/usb/gadget/uvc_queue.c index 9ac4ffe1..305eb49 100644 --- a/drivers/usb/gadget/uvc_queue.c +++ b/drivers/usb/gadget/uvc_queue.c @@ -380,6 +380,7 @@ static struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue, else nextbuf = NULL; + buf->buf.v4l2_buf.field = V4L2_FIELD_NONE; buf->buf.v4l2_buf.sequence = queue->sequence++; v4l2_get_timestamp(&buf->buf.v4l2_buf.timestamp); -- 1.8.3.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 v2 0/3] Miscellaneous fixes for UVC gadget driver
Hello,
These three patches fix miscellaneous issues in the UVC gadget driver. Patches
1 and 3 have already been posted as part of the "Clock fixes for UVC gadget
driver" series, and patch 2 is new.
The series is based on the latest media tree master branch as it depends on
commit 872484ce40881e295b046adf21f7211306477751 ("v4l: Add timestamp source
flags, mask and document them") queued for v3.15. It would thus be easier to
merge it through the media tree. Greg and Mauro, would that be fine ?
Alternatively I can rebase it on top of v3.15-rc1 when that version will be
tagged.
Laurent Pinchart (3):
usb: gadget: uvc: Switch to monotonic clock for buffer timestamps
usb: gadget: uvc: Set the V4L2 buffer field to V4L2_FIELD_NONE
usb: gadget: uvc: Set the vb2 queue timestamp flags
drivers/usb/gadget/uvc_queue.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
--
Regards,
Laurent Pinchart
--
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 v2 3/3] usb: gadget: uvc: Set the vb2 queue timestamp flags
On Fri, Mar 28, 2014 at 05:02:48PM +0100, Laurent Pinchart wrote: > The vb2 queue timestamp_flags field must be set by drivers, as enforced > by a WARN_ON in vb2_queue_init. The UVC gadget driver failed to do so. > This resulted in the following warning. > > [2.104371] g_webcam gadget: uvc_function_bind > [2.105567] [ cut here ] > [2.105567] [ cut here ] > [2.106779] WARNING: CPU: 0 PID: 1 at > drivers/media/v4l2-core/videobuf2-core.c:2207 vb2_queue_init+0xa3/0x113() > > Fix it. > > Reported-by: Fengguang Wu > Signed-off-by: Laurent Pinchart > Reviewed-by: Hans Verkuil Acked-by: Felipe Balbi > --- > drivers/usb/gadget/uvc_queue.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/usb/gadget/uvc_queue.c b/drivers/usb/gadget/uvc_queue.c > index 305eb49..1c29bc9 100644 > --- a/drivers/usb/gadget/uvc_queue.c > +++ b/drivers/usb/gadget/uvc_queue.c > @@ -137,6 +137,8 @@ static int uvc_queue_init(struct uvc_video_queue *queue, > queue->queue.buf_struct_size = sizeof(struct uvc_buffer); > queue->queue.ops = &uvc_queue_qops; > queue->queue.mem_ops = &vb2_vmalloc_memops; > + queue->queue.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC > + | V4L2_BUF_FLAG_TSTAMP_SRC_EOF; > ret = vb2_queue_init(&queue->queue); > if (ret) > return ret; > -- > 1.8.3.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 -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 2/3] usb: gadget: uvc: Set the V4L2 buffer field to V4L2_FIELD_NONE
On 03/28/2014 05:02 PM, Laurent Pinchart wrote: > The UVC gadget driver doesn't support interlaced video but left the > buffer field uninitialized. Set it to V4L2_FIELD_NONE. > > Signed-off-by: Laurent Pinchart Reviewed-by: Hans Verkuil Regards, Hans > --- > drivers/usb/gadget/uvc_queue.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/usb/gadget/uvc_queue.c b/drivers/usb/gadget/uvc_queue.c > index 9ac4ffe1..305eb49 100644 > --- a/drivers/usb/gadget/uvc_queue.c > +++ b/drivers/usb/gadget/uvc_queue.c > @@ -380,6 +380,7 @@ static struct uvc_buffer *uvc_queue_next_buffer(struct > uvc_video_queue *queue, > else > nextbuf = NULL; > > + buf->buf.v4l2_buf.field = V4L2_FIELD_NONE; > buf->buf.v4l2_buf.sequence = queue->sequence++; > v4l2_get_timestamp(&buf->buf.v4l2_buf.timestamp); > > -- 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
usb: chipidea: udc: ep_free_request & memleak
Hi,
I'm currently checking chipidea udc on our platform and I can see some
"freeing queued request" message (some bad behaviour from android adb
gadget driver).
After this message we exit without freeing anything :
static void ep_free_request(struct usb_ep *ep, struct usb_request *req)
{
(...)
if (ep == NULL || req == NULL) {
return;
} else if (!list_empty(&hwreq->queue)) {
dev_err(hwep->ci->dev, "freeing queued request\n");
return;
}
(...)
kfree(hwreq);
I checked on other gadget drivers:
- amd5536udc, at91_udc, lpc32xx_udc will BUG_ON(!list_empty(&req->queue));
- pch_udc.c will dev_err but not return
- a lot will WARN_ON (!list_empty (&req->queue));
- and some will not check it
As far as the gadget.h says, the caller of usb_ep_free_request
guarantees the request is not queued, and that it will no longer be
requeued (or otherwise used).
But maybe a WARN_ON should be a better strategy ? or am I lacking
something ?
Regards,
François MULLER
--
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: serial: add dell wwan card(413c:81a9)
On Fri, 2014-03-28 at 11:25 +0100, Bjørn Mork wrote:
> Johan Hovold writes:
>
> > [ +CC: Dan and Bjørn ]
> >
> > On Fri, Mar 28, 2014 at 03:25:06PM +0800, AceLan Kao wrote:
> >> This patch adds support for Dell Computer Corp. Wireless 5808e 4G LTE
> >> Mobile Broadband Card.
> >
> > Shouldn't this device be handled by the option driver?
> >
> > In fact, PID 0x8133 for 5720 VZW (which your clean-up patch touches)
> > is listed in both option and sierra.
> >
> > I think we need to revert 1ebca9dad5ab ("USB: serial: add usbid for dell
> > wwan card to sierra.c").
>
> Maybe. If nothing else, additional PID digit in that patch looks a bit
> silly :-)
>
> It would be good to have a bit more information about these modules. As
> you know, but the reporters may not be aware of, the fact that they are
> made by Sierra does not necessarily mean that they are supported by the
> sierra driver. That will depend on the chipset and firmware.
>
> Just as an example: My Sierra Wireless MC7710 works fine with the sierra
> driver if it is in "DirectIP" mode (using a 0x68a3 PID). But it fails
> the sierra_set_power_state() control message I configure it for "QMI"
> mode (using a 0x68a2 PID), adding a 5 second delay for every serial port
> probed. This delay isn't necessarily noticed by the reporter... The
> 0x68a2 PID was also originally added to the sierra driver, and it did
> sort of work.
>
> I think we'd like to see at least a dmesg snippet with time stamps
> showing the actual probing before accepting any new device into the
> "sierra" driver. We do not want to add any device which does not
> understand the Sierra vendor specific control messages.
>
> And it would also be preferable to have a lsusb -v listing or similar,
> so that we can match the different functions against other Sierra
> devices. Sierra have been using a system with fixed interface numbers
> for different vendor specific functions lately. The sierra driver has
> knowledge about this. That's what the &direct_ip_interface_blacklist is
> for. I'm pretty sure that any new Sierra module will need to have this
> blacklist enabled, even if it turns out that the sierra driver is
> correct.
The Dell drivers use the same configuration for PIDs:
81A2: Dell Wireless 5806 Gobi(TM) 4G LTE Mobile Broadband Card
81A3: Dell Wireless 5570 HSPA+ (42Mbps) Mobile Broadband Card
81A4: Dell Wireless 5570e HSPA+ (42Mbps) Mobile Broadband Card
81A8: Dell Wireless 5808 Gobi(TM) 4G LTE Mobile Broadband Card
81A9: Dell Wireless 5808e Gobi(TM) 4G LTE Mobile Broadband Card
These devices are all clearly Sierra devices, but are also definitely
Gobi-based. The A8 might be the MC7700/7710 and A9 is likely a MC7750.
>From DellGobi5kSetup.exe from the Dell drivers:
usbif0: serial/firmware loader?
usbif2: nmea
usbif3: modem/ppp
usbif8: net/QMI
If Bjorn is right about Sierra using the fixed interface numbers for the
different functions, then perhaps the other USB interfaces (1, 4, 5, 6,
7) would be active when the device is in Direct IP mode?
Dan
--
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: serial: fixed the product id typo
On Fri, 2014-03-28 at 11:37 -0500, Dan Williams wrote:
> On Fri, 2014-03-28 at 15:25 +0800, AceLan Kao wrote:
> > Fixed the product id typo and replace whitespace with tab.
> >
> > Signed-off-by: AceLan Kao
> > ---
> > drivers/usb/serial/sierra.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
> > index a9eb622..b78fd4e 100644
> > --- a/drivers/usb/serial/sierra.c
> > +++ b/drivers/usb/serial/sierra.c
> > @@ -291,7 +291,7 @@ static const struct usb_device_id id_table[] = {
> > { USB_DEVICE(0x0f3d, 0x68A3), /* Airprime/Sierra Wireless Direct IP
> > modems */
> > .driver_info = (kernel_ulong_t)&direct_ip_interface_blacklist
> > },
> > - { USB_DEVICE(0x413C, 0x08133) }, /* Dell Computer Corp. Wireless
> > 5720 VZW Mobile Broadband (EVDO Rev-A) Minicard GPS Port */
> > + { USB_DEVICE(0x413C, 0x8133) }, /* Dell Computer Corp. Wireless 5720
> > VZW Mobile Broadband (EVDO Rev-A) Minicard GPS Port */
>
> The Dell 5720 (Novatel E725) is not a Sierra device, so it should
> certainly not remain in the 'sierra' driver and the original commit that
> did this was in error. It's device ID should be moved to the 'option'
> driver instead. So could you resend this patch, but move the device to
> option.c and fix the typo there?
Nevermind, Johan nailed it; we should do the revert instead.
Dan
--
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: move ZTE CDMA device pid from zte_ev.c back to option.c
On Thu, Mar 27, 2014 at 02:40:23PM +0800, 刘磊 wrote: > dear linuxfoundation: "Linux Foundation" is a non-profit group, not a person :) And please don't send HTML email, the mailing list rejects it. > Because of the time difference, can not respond promptly. That's not an issue, don't worry about it. > May be mail service lead to patch is badly in last mail, now i use other > mailbox. > Two method provide the patch, the html format in mail and take the patch > file as attachment. > > Signed-off-by:lei liu This is still corrupted, can you please resend, with the proper information in the changelog area (as described in the Documentation/SubmittingPatches file) so that we can apply it? 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 2/2] USB: serial: add dell wwan card(413c:81a9)
On Fri, 2014-03-28 at 10:56 +0100, Johan Hovold wrote:
> [ +CC: Dan and Bjørn ]
>
> On Fri, Mar 28, 2014 at 03:25:06PM +0800, AceLan Kao wrote:
> > This patch adds support for Dell Computer Corp. Wireless 5808e 4G LTE
> > Mobile Broadband Card.
>
> Shouldn't this device be handled by the option driver?
>
> In fact, PID 0x8133 for 5720 VZW (which your clean-up patch touches)
> is listed in both option and sierra.
>
> I think we need to revert 1ebca9dad5ab ("USB: serial: add usbid for dell
> wwan card to sierra.c").
Yeah, you're right. The revert should be sufficient since like you say
it's already in option (which is the correct driver for this Novatel
product, which is obviously not a Sierra device).
Dan
> Thanks,
> Johan
>
> > Signed-off-by: AceLan Kao
> > ---
> > drivers/usb/serial/sierra.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
> > index b78fd4e..961d586 100644
> > --- a/drivers/usb/serial/sierra.c
> > +++ b/drivers/usb/serial/sierra.c
> > @@ -292,6 +292,7 @@ static const struct usb_device_id id_table[] = {
> > .driver_info = (kernel_ulong_t)&direct_ip_interface_blacklist
> > },
> > { USB_DEVICE(0x413C, 0x8133) }, /* Dell Computer Corp. Wireless 5720
> > VZW Mobile Broadband (EVDO Rev-A) Minicard GPS Port */
> > + { USB_DEVICE(0x413C, 0x81A9) }, /* Dell Computer Corp. Wireless 5808e
> > 4G LTE Mobile Broadband Card */
> >
> > { }
> > };
> --
> 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
--
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] uas: fix GFP_NOIO under spinlock
On Fri, Mar 28, 2014 at 09:48:00AM +0100, Hans de Goede wrote: > Hi, > > On 03/28/2014 09:45 AM, oli...@neukum.org wrote: > > From: Oliver Neukum > > > > Quote Dan: > > > > The patch e36e64930cff: "uas: Use GFP_NOIO rather then GFP_ATOMIC > > where possible" from Nov 7, 2013, leads to the following static > > checker warning: > > > > drivers/usb/storage/uas.c:806 uas_eh_task_mgmt() > > error: scheduling with locks held: 'spin_lock:lock' > > The fix is obvious. > > > > Signed-off-by: Oliver Neukum > > Reported-by: Dan Carpenter > > Dan, thanks for catching this, Oliver thanks for fixing this. > > Looks good to me, Greg can you please add this to usb-next? > It fixes a regression my uas set has introduced there. I'll queue it up after 3.15-rc1 is out, 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 v2 1/3] usb: gadget: uvc: Switch to monotonic clock for buffer timestamps
On Fri, Mar 28, 2014 at 05:02:46PM +0100, Laurent Pinchart wrote: > The wall time clock isn't useful for applications as it can jump around > due to time adjustement. Switch to the monotonic clock. > > Signed-off-by: Laurent Pinchart Acked-by: Felipe Balbi > --- > drivers/usb/gadget/uvc_queue.c | 9 ++--- > 1 file changed, 2 insertions(+), 7 deletions(-) > > Changes since v1: > > - Replace ktime_get_ts() with v4l2_get_timestamp() > > diff --git a/drivers/usb/gadget/uvc_queue.c b/drivers/usb/gadget/uvc_queue.c > index 0bb5d50..9ac4ffe1 100644 > --- a/drivers/usb/gadget/uvc_queue.c > +++ b/drivers/usb/gadget/uvc_queue.c > @@ -20,6 +20,7 @@ > #include > #include > > +#include > #include > > #include "uvc.h" > @@ -379,14 +380,8 @@ static struct uvc_buffer *uvc_queue_next_buffer(struct > uvc_video_queue *queue, > else > nextbuf = NULL; > > - /* > - * FIXME: with videobuf2, the sequence number or timestamp fields > - * are valid only for video capture devices and the UVC gadget usually > - * is a video output device. Keeping these until the specs are clear on > - * this aspect. > - */ > buf->buf.v4l2_buf.sequence = queue->sequence++; > - do_gettimeofday(&buf->buf.v4l2_buf.timestamp); > + v4l2_get_timestamp(&buf->buf.v4l2_buf.timestamp); > > vb2_set_plane_payload(&buf->buf, 0, buf->bytesused); > vb2_buffer_done(&buf->buf, VB2_BUF_STATE_DONE); > -- > 1.8.3.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 -- balbi signature.asc Description: Digital signature
[PATCH 1/1] usb: option driver, add support for Telit UE910v2
option driver, added VID/PID for Telit UE910v2 modem
Signed-off-by: Daniele Palmas
---
drivers/usb/serial/option.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index 68fc9fe..8811a5b 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -244,6 +244,7 @@ static void option_instat_callback(struct urb *urb);
#define TELIT_PRODUCT_CC864_SINGLE 0x1006
#define TELIT_PRODUCT_DE910_DUAL 0x1010
#define TELIT_PRODUCT_LE9200x1200
+#define TELIT_PRODUCT_UE910_V2 0x1012
/* ZTE PRODUCTS */
#define ZTE_VENDOR_ID 0x19d2
@@ -1043,6 +1044,7 @@ static const struct usb_device_id option_ids[] = {
{ USB_DEVICE(TELIT_VENDOR_ID, TELIT_PRODUCT_DE910_DUAL) },
{ USB_DEVICE(TELIT_VENDOR_ID, TELIT_PRODUCT_LE920),
.driver_info = (kernel_ulong_t)&telit_le920_blacklist },
+ { USB_DEVICE(TELIT_VENDOR_ID, TELIT_PRODUCT_UE910_V2) },
{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_MF622, 0xff,
0xff, 0xff) }, /* ZTE WCDMA products */
{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0x0002, 0xff, 0xff,
0xff),
.driver_info = (kernel_ulong_t)&net_intf1_blacklist },
--
1.7.5.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 v2 1/3] usb: gadget: uvc: Switch to monotonic clock for buffer timestamps
On 03/28/2014 05:02 PM, Laurent Pinchart wrote: > The wall time clock isn't useful for applications as it can jump around > due to time adjustement. Switch to the monotonic clock. > > Signed-off-by: Laurent Pinchart Reviewed-by: Hans Verkuil Regards, Hans > --- > drivers/usb/gadget/uvc_queue.c | 9 ++--- > 1 file changed, 2 insertions(+), 7 deletions(-) > > Changes since v1: > > - Replace ktime_get_ts() with v4l2_get_timestamp() > > diff --git a/drivers/usb/gadget/uvc_queue.c b/drivers/usb/gadget/uvc_queue.c > index 0bb5d50..9ac4ffe1 100644 > --- a/drivers/usb/gadget/uvc_queue.c > +++ b/drivers/usb/gadget/uvc_queue.c > @@ -20,6 +20,7 @@ > #include > #include > > +#include > #include > > #include "uvc.h" > @@ -379,14 +380,8 @@ static struct uvc_buffer *uvc_queue_next_buffer(struct > uvc_video_queue *queue, > else > nextbuf = NULL; > > - /* > - * FIXME: with videobuf2, the sequence number or timestamp fields > - * are valid only for video capture devices and the UVC gadget usually > - * is a video output device. Keeping these until the specs are clear on > - * this aspect. > - */ > buf->buf.v4l2_buf.sequence = queue->sequence++; > - do_gettimeofday(&buf->buf.v4l2_buf.timestamp); > + v4l2_get_timestamp(&buf->buf.v4l2_buf.timestamp); > > vb2_set_plane_payload(&buf->buf, 0, buf->bytesused); > vb2_buffer_done(&buf->buf, VB2_BUF_STATE_DONE); > -- 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: Deadlock in usb-storage error handling
On Thu, 2014-03-20 at 15:49 -0400, Alan Stern wrote:
> On Thu, 20 Mar 2014, James Bottomley wrote:
>
> > On Thu, 2014-03-20 at 12:34 -0400, Alan Stern wrote:
> > > On Thu, 20 Mar 2014, James Bottomley wrote:
> > >
> > > > OK, so I think we have three things to do
> > > >
> > > > 1. Investigate SCSI and fix it's abort state problem that's causing
> > > > it not to send the abort second time around
> > > > 2. Fix usb-storage to fail a reset it can't do (i.e. device reset
> > > > with outstanding commands)
> > > > 3. Find out why we're sending a spurious request sense.
> > > >
> > > > I can look at 1 and 3 if you want to take 2.
> > >
> > > It's a deal! Thanks for your help.
> >
> > OK, I think this is the fix for 1, if you could try it out.
> >
> > Thanks,
> >
> > James
> >
> > ---
> >
> > diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> > index 771c16b..c52bfb2 100644
> > --- a/drivers/scsi/scsi_error.c
> > +++ b/drivers/scsi/scsi_error.c
> > @@ -145,14 +145,14 @@ scmd_eh_abort_handler(struct work_struct *work)
> > "scmd %p retry "
> > "aborted command\n", scmd));
> > scsi_queue_insert(scmd, SCSI_MLQUEUE_EH_RETRY);
> > - return;
> > + goto out;
> > } else {
> > SCSI_LOG_ERROR_RECOVERY(3,
> > scmd_printk(KERN_WARNING, scmd,
> > "scmd %p finish "
> > "aborted command\n", scmd));
> > scsi_finish_command(scmd);
> > - return;
> > + goto out;
> > }
> > } else {
> > SCSI_LOG_ERROR_RECOVERY(3,
> > @@ -162,6 +162,8 @@ scmd_eh_abort_handler(struct work_struct *work)
> > }
> > }
> >
> > + scmd->eh_eflags &= ~SCSI_EH_ABORT_SCHEDULED;
> > +
> > if (!scsi_eh_scmd_add(scmd, 0)) {
> > SCSI_LOG_ERROR_RECOVERY(3,
> > scmd_printk(KERN_WARNING, scmd,
> > @@ -170,6 +172,10 @@ scmd_eh_abort_handler(struct work_struct *work)
> > scmd->result |= DID_TIME_OUT << 16;
> > scsi_finish_command(scmd);
> > }
> > + return;
> > + out:
> > + scmd->eh_eflags &= ~SCSI_EH_ABORT_SCHEDULED;
> > + return;
> > }
> >
> > /**
>
> This worked the first time. :-)
>
> But I wonder, is it safe to access scmd after calling
> scsi_finish_command()?
Agree, I've redone the patch integrated into the try_to_abort call
instead. Will post shortly.
James
--
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] Revert "USB: serial: add usbid for dell wwan card to sierra.c"
This reverts commit 1ebca9dad5abe8b2ed4dbd186cd657fb47c1f321.
This device was erroneously added to the sierra driver even though it's
not a Sierra device and was already handled by the option driver.
Cc: Richard Farina
Cc: sta...@vger.kernel.org
Signed-off-by: Johan Hovold
---
drivers/usb/serial/sierra.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
index a9eb6221a815..6b192e602ce0 100644
--- a/drivers/usb/serial/sierra.c
+++ b/drivers/usb/serial/sierra.c
@@ -291,7 +291,6 @@ static const struct usb_device_id id_table[] = {
{ USB_DEVICE(0x0f3d, 0x68A3), /* Airprime/Sierra Wireless Direct IP
modems */
.driver_info = (kernel_ulong_t)&direct_ip_interface_blacklist
},
- { USB_DEVICE(0x413C, 0x08133) }, /* Dell Computer Corp. Wireless 5720
VZW Mobile Broadband (EVDO Rev-A) Minicard GPS Port */
{ }
};
--
1.8.3.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
Re: [PATCH] usb: xhci: Correct last context entry calculation for Configure Endpoint
On 03/25/2014 08:42 PM, Julius Werner wrote: The current XHCI driver recalculates the Context Entries field in the Slot Context on every add_endpoint() and drop_endpoint() call. In the case of drop_endpoint(), it seems to assume that the add_flags will always contain every endpoint for the new configuration, which is not necessarily correct if you don't make assumptions about how the USB core uses the add_endpoint/drop_endpoint interface (add_flags only contains endpoints that are new additions in the new configuration). Furthermore, EP0_FLAG is not consistently set in add_flags throughout the lifetime of a device. This means that when all endpoints are dropped, the Context Entries field can be set to 0 (which is invalid and may cause a Parameter Error) or -1 (which is interpreted as 31 and causes the driver to keep using the old, incorrect value). The only surefire way to set this field right is to also take all existing endpoints into account, and to force the value to 1 (meaning only EP0 is active) if no other endpoint is found. This patch implements that as a single step in the final check_bandwidth() call and removes the intermediary calculations from add_endpoint() and drop_endpoint(). Hi I think this looks good in general, but before I really dig into it I'd like to know if this has triggered any problems so far? (e.g. slot context's context entries getting zeroed, or set to a smaller value than the last active endpoint context, causing trouble) -Mathias -- 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: serial: add dell wwan card(413c:81a9)
On Fri, Mar 28, 2014 at 11:25:23AM +0100, Bjørn Mork wrote:
> Johan Hovold writes:
> > [ +CC: Dan and Bjørn ]
> >
> > On Fri, Mar 28, 2014 at 03:25:06PM +0800, AceLan Kao wrote:
> >> This patch adds support for Dell Computer Corp. Wireless 5808e 4G LTE
> >> Mobile Broadband Card.
> >
> > Shouldn't this device be handled by the option driver?
> >
> > In fact, PID 0x8133 for 5720 VZW (which your clean-up patch touches)
> > is listed in both option and sierra.
> >
> > I think we need to revert 1ebca9dad5ab ("USB: serial: add usbid for dell
> > wwan card to sierra.c").
>
> Maybe. If nothing else, additional PID digit in that patch looks a bit
> silly :-)
Indeed. But lets decide what driver should actually be claiming the
device first. :)
> It would be good to have a bit more information about these modules. As
> you know, but the reporters may not be aware of, the fact that they are
> made by Sierra does not necessarily mean that they are supported by the
> sierra driver. That will depend on the chipset and firmware.
>
> Just as an example: My Sierra Wireless MC7710 works fine with the sierra
> driver if it is in "DirectIP" mode (using a 0x68a3 PID). But it fails
> the sierra_set_power_state() control message I configure it for "QMI"
> mode (using a 0x68a2 PID), adding a 5 second delay for every serial port
> probed. This delay isn't necessarily noticed by the reporter... The
> 0x68a2 PID was also originally added to the sierra driver, and it did
> sort of work.
Seems to me that some error messages are missing from the sierra driver
when sierra_set_power_state (and sierra_vsc_set_nmea) fails then.
> I think we'd like to see at least a dmesg snippet with time stamps
> showing the actual probing before accepting any new device into the
> "sierra" driver. We do not want to add any device which does not
> understand the Sierra vendor specific control messages.
AceLan, could you provide such logs from when plugging the device in
(with your patch applied). Please also include the output of
lsusb -vd 413c:81a9
> And it would also be preferable to have a lsusb -v listing or similar,
> so that we can match the different functions against other Sierra
> devices. Sierra have been using a system with fixed interface numbers
> for different vendor specific functions lately. The sierra driver has
> knowledge about this. That's what the &direct_ip_interface_blacklist is
> for. I'm pretty sure that any new Sierra module will need to have this
> blacklist enabled, even if it turns out that the sierra driver is
> correct.
I found lsusb output for 0x8133 (currently claimed by both sierra and
option):
Bus 004 Device 002: ID 413c:8133 Dell Computer Corp.
Device Descriptor:
bLength18
bDescriptorType 1
bcdUSB 1.10
bDeviceClass0 (Defined at Interface level)
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize064
idVendor 0x413c Dell Computer Corp.
idProduct 0x8133
bcdDevice0.00
iManufacturer 1
iProduct2
iSerial 0
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 62
bNumInterfaces 2
bConfigurationValue 1
iConfiguration 0
bmAttributes 0xe0
Self Powered
Remote Wakeup
MaxPower 100mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber0
bAlternateSetting 0
bNumEndpoints 3
bInterfaceClass 255 Vendor Specific Class
bInterfaceSubClass255 Vendor Specific Subclass
bInterfaceProtocol255 Vendor Specific Protocol
iInterface 3
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81 EP 1 IN
bmAttributes3
Transfer TypeInterrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0010 1x 16 bytes
bInterval 128
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x82 EP 2 IN
bmAttributes2
Transfer TypeBulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x02 EP 2 OUT
bmAttributes2
Transfer TypeBulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 0
Re: [PATCH] usb: xhci: Correct last context entry calculation for Configure Endpoint
Hi,
On Tue, Mar 25, 2014 at 11:42:43AM -0700, Julius Werner wrote:
> @@ -2723,8 +2697,19 @@ int xhci_check_bandwidth(struct usb_hcd *hcd, struct
> usb_device *udev)
> ctrl_ctx->drop_flags == 0)
> return 0;
>
> - xhci_dbg(xhci, "New Input Control Context:\n");
> + /* Fix up Context Entries field. Minimum value is EP0 == BIT(1). */
> slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->in_ctx);
> + for (i = 31; i >= 1; i--) {
> + u32 le32 = cpu_to_le32(BIT(i));
can you use __le32 instead of u32 here, just to make sure we can make
good use of sparse ?
--
balbi
signature.asc
Description: Digital signature
[PATCH v2 1/3] usb: gadget: uvc: Switch to monotonic clock for buffer timestamps
The wall time clock isn't useful for applications as it can jump around due to time adjustement. Switch to the monotonic clock. Signed-off-by: Laurent Pinchart --- drivers/usb/gadget/uvc_queue.c | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) Changes since v1: - Replace ktime_get_ts() with v4l2_get_timestamp() diff --git a/drivers/usb/gadget/uvc_queue.c b/drivers/usb/gadget/uvc_queue.c index 0bb5d50..9ac4ffe1 100644 --- a/drivers/usb/gadget/uvc_queue.c +++ b/drivers/usb/gadget/uvc_queue.c @@ -20,6 +20,7 @@ #include #include +#include #include #include "uvc.h" @@ -379,14 +380,8 @@ static struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue, else nextbuf = NULL; - /* -* FIXME: with videobuf2, the sequence number or timestamp fields -* are valid only for video capture devices and the UVC gadget usually -* is a video output device. Keeping these until the specs are clear on -* this aspect. -*/ buf->buf.v4l2_buf.sequence = queue->sequence++; - do_gettimeofday(&buf->buf.v4l2_buf.timestamp); + v4l2_get_timestamp(&buf->buf.v4l2_buf.timestamp); vb2_set_plane_payload(&buf->buf, 0, buf->bytesused); vb2_buffer_done(&buf->buf, VB2_BUF_STATE_DONE); -- 1.8.3.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
Re: problem with resume after s2ram
On Thu, 27 Mar 2014, Peter Münster wrote:
> Hi Alan,
>
> These are my commands:
>
> echo 8 >/proc/sys/kernel/printk
> echo on >/sys/bus/usb/devices/4-2/power/control
> sleep 1
> echo 0 >/sys/bus/usb/devices/4-2/bConfigurationValue
> sleep 1
> echo auto >/sys/bus/pci/devices/:00:12.1/power/control
> sleep 1
> echo auto >/sys/bus/usb/devices/4-2/power/control
> sleep 10
> echo on >/sys/bus/usb/devices/4-2/power/control
>
> Please find attached the kernel messages and the content of the
> registers file once per second.
This just raises more questions. It looks like it worked almost okay,
but not quite. Certainly it didn't hang, so it isn't an exact
reproduction of the problem. Can you run the same test again, and then
at the end, do:
echo 1 >/sys/bus/usb/devices/4-2/bConfigurationValue
(Actually this 1 should be whatever was in the file before you started
the test -- it probably was 1 but it might be something else.)
After that, the mouse should be working properly. Check it and see,
and post the kernel log. (Don't bother to check the frame number
values, since we already know they are changing.)
Back when you ran the system suspend test under the 3.9 kernel with
CONFIG_USB_SUSPEND turned off, you said the system was okay after the
resume. Does that mean the mouse and keyboard were working normally?
One more thing... The bad commit should have no effect on devices
enabled for wakeup -- and keyboards generally _are_ enabled for wakeup.
Can you check this? Under 3.12.x with no patches or reversions, plug
the mouse into a front port and the keyboard into its usual rear port.
Before going into suspend, make sure that the keyboard's power/wakeup
file (/sys/bus/usb/devices/4-3/power/wakeup) contains "enabled".
(Write it to the file if the file says "disabled".) Then test if the
suspend and resume work okay -- capture the log with netconsole and
let's see what it says.
Finally, I'd like to get more information about what causes the system
to hang when things go wrong. Below is a diagnostic patch for the 3.12
kernel -- you can keep the previous patch or remove it, it doesn't
matter. This patch is intended to print information to the console
every 10 seconds while the system is hung, so if it works then we
should get some useful clues. Plug the mouse into the rear port and
see what happens after a suspend.
Alan Stern
Index: usb-3.14/drivers/usb/host/ohci-hcd.c
===
--- usb-3.14.orig/drivers/usb/host/ohci-hcd.c
+++ usb-3.14/drivers/usb/host/ohci-hcd.c
@@ -79,6 +79,19 @@ static const charhcd_name [] = "ohci_hc
static void ohci_dump (struct ohci_hcd *ohci, int verbose);
static void ohci_stop (struct usb_hcd *hcd);
+static void alantimer_func(unsigned long _ohci)
+{
+ struct ohci_hcd *ohci = (struct ohci_hcd *) _ohci;
+
+ ohci_info(ohci, "Timer: count %d intr-en %x intr-stat %x frame %x\n",
+ ohci->alancount,
+ ohci_readl(ohci, &ohci->regs->intrenable),
+ ohci_readl(ohci, &ohci->regs->intrstatus),
+ ohci_frame_no(ohci));
+ if (ohci->alancount > 0)
+ mod_timer(&ohci->alantimer, jiffies + HZ*10);
+}
+
#include "ohci-hub.c"
#include "ohci-dbg.c"
#include "ohci-mem.c"
@@ -289,6 +302,10 @@ static int ohci_urb_dequeue(struct usb_h
* handed to us, flag it for unlink and giveback, and force
* some upcoming INTR_SF to call finish_unlinks()
*/
+ if (ohci->alancount == 0)
+ mod_timer(&ohci->alantimer, jiffies + HZ*10);
+ ohci_info(ohci, "Dequeue: %p count %d\n", urb,
+ ++ohci->alancount);
urb_priv = urb->hcpriv;
if (urb_priv) {
if (urb_priv->ed->state == ED_OPER)
@@ -299,6 +316,8 @@ static int ohci_urb_dequeue(struct usb_h
* with HC dead, we won't respect hc queue pointers
* any more ... just clean up every urb's memory.
*/
+ ohci_info(ohci, "Dequeue: %p count %d\n", urb,
+ ++ohci->alancount);
if (urb->hcpriv)
finish_urb(ohci, urb, status);
}
@@ -745,6 +764,7 @@ retry:
}
ohci_dump (ohci, 1);
+ setup_timer(&ohci->alantimer, alantimer_func, (unsigned long) ohci);
return 0;
}
@@ -801,6 +821,12 @@ static irqreturn_t ohci_irq (struct usb_
return IRQ_HANDLED;
}
+ if (ohci->alancount > 0)
+ ohci_info(ohci, "IRQ: count %d intr-en %x intr-stat %x frame
%x\n",
+ ohci->alancount,
+ ohci_readl(ohci, ®s->intrenable), ints,
+ ohci_frame_no(ohci));
+
/* We only care about interrupts that are enabled */
ints &= ohci
Re: [PATCH] pl2303: add ids for Hewlett-Packard HP POS pole displays
On Fri, Mar 28, 2014 at 01:36:51PM +, Sanders, Aaron wrote:
> Add device ids to pl2303 for the Hewlett-Packard HP POS pole displays:
> LD960: 03f0:0B39
> LCM220: 03f0:3139
> LCM960: 03f0:3239
>
> Signed-off-by: Aaron Sanders
Almost there. :)
Please add a "USB: " prefix to the subject above as well (i.e. "USB:
pl2303: add ...").
> diff -uNr old/pl2303.c new/pl2303.c
You need to generate the patch from the root of the source tree (as
described in Documentation/SubmittingPatches).
Thanks,
Johan
> --- old/pl2303.c 2014-03-26 11:06:20.526852963 -0700
> +++ new/pl2303.c 2014-03-26 11:06:32.154852611 -0700
> @@ -82,6 +82,9 @@
> { USB_DEVICE(YCCABLE_VENDOR_ID, YCCABLE_PRODUCT_ID) },
> { USB_DEVICE(SUPERIAL_VENDOR_ID, SUPERIAL_PRODUCT_ID) },
> { USB_DEVICE(HP_VENDOR_ID, HP_LD220_PRODUCT_ID) },
> + { USB_DEVICE(HP_VENDOR_ID, HP_LD960_PRODUCT_ID) },
> + { USB_DEVICE(HP_VENDOR_ID, HP_LCM220_PRODUCT_ID) },
> + { USB_DEVICE(HP_VENDOR_ID, HP_LCM960_PRODUCT_ID) },
> { USB_DEVICE(CRESSI_VENDOR_ID, CRESSI_EDY_PRODUCT_ID) },
> { USB_DEVICE(ZEAGLE_VENDOR_ID, ZEAGLE_N2ITION3_PRODUCT_ID) },
> { USB_DEVICE(SONY_VENDOR_ID, SONY_QN3USB_PRODUCT_ID) },
> diff -uNr old/pl2303.h new/pl2303.h
> --- old/pl2303.h 2014-03-26 11:06:11.030853251 -0700
> +++ new/pl2303.h 2014-03-26 11:06:37.462852450 -0700
> @@ -124,6 +124,9 @@
> /* Hewlett-Packard LD220-HP POS Pole Display */
> #define HP_VENDOR_ID 0x03f0
> #define HP_LD220_PRODUCT_ID 0x3524
> +#define HP_LD960_PRODUCT_ID 0x0B39
> +#define HP_LCM220_PRODUCT_ID 0x3139
> +#define HP_LCM960_PRODUCT_ID 0x3239
>
> /* Cressi Edy (diving computer) PC interface */
> #define CRESSI_VENDOR_ID 0x04b8
--
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] pl2303: add ids for Hewlett-Packard HP POS pole displays
Add device ids to pl2303 for the Hewlett-Packard HP POS pole displays:
LD960: 03f0:0B39
LCM220: 03f0:3139
LCM960: 03f0:3239
Signed-off-by: Aaron Sanders
diff -uNr old/pl2303.c new/pl2303.c
--- old/pl2303.c2014-03-26 11:06:20.526852963 -0700
+++ new/pl2303.c2014-03-26 11:06:32.154852611 -0700
@@ -82,6 +82,9 @@
{ USB_DEVICE(YCCABLE_VENDOR_ID, YCCABLE_PRODUCT_ID) },
{ USB_DEVICE(SUPERIAL_VENDOR_ID, SUPERIAL_PRODUCT_ID) },
{ USB_DEVICE(HP_VENDOR_ID, HP_LD220_PRODUCT_ID) },
+ { USB_DEVICE(HP_VENDOR_ID, HP_LD960_PRODUCT_ID) },
+ { USB_DEVICE(HP_VENDOR_ID, HP_LCM220_PRODUCT_ID) },
+ { USB_DEVICE(HP_VENDOR_ID, HP_LCM960_PRODUCT_ID) },
{ USB_DEVICE(CRESSI_VENDOR_ID, CRESSI_EDY_PRODUCT_ID) },
{ USB_DEVICE(ZEAGLE_VENDOR_ID, ZEAGLE_N2ITION3_PRODUCT_ID) },
{ USB_DEVICE(SONY_VENDOR_ID, SONY_QN3USB_PRODUCT_ID) },
diff -uNr old/pl2303.h new/pl2303.h
--- old/pl2303.h2014-03-26 11:06:11.030853251 -0700
+++ new/pl2303.h2014-03-26 11:06:37.462852450 -0700
@@ -124,6 +124,9 @@
/* Hewlett-Packard LD220-HP POS Pole Display */
#define HP_VENDOR_ID 0x03f0
#define HP_LD220_PRODUCT_ID0x3524
+#define HP_LD960_PRODUCT_ID0x0B39
+#define HP_LCM220_PRODUCT_ID 0x3139
+#define HP_LCM960_PRODUCT_ID 0x3239
/* Cressi Edy (diving computer) PC interface */
#define CRESSI_VENDOR_ID 0x04b8
--
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: uas_disconnect() and morphed devices after resumption
Hi, On 03/28/2014 01:53 PM, Oliver Neukum wrote: > Hi, > > I am looking at uas_disconnect() calling uas_free_streams() > unconditionally. I wonder what this does if a device has > morphed. It seems to me we'd try to free streams of endpoints > that might not even exist. If a device morphs we will get uas_disconnect() called before the kernel re-enumerates the device, so then we will still have all the old endpoints in the interface struct, and freeing the streams should work fine. Note that freeing the streams is an controller level operation, nothing is send to the device, and since at this point all our state still reflects the old descriptors of the device everything should be fine. Regards, Hans -- 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
uas_disconnect() and morphed devices after resumption
Hi, I am looking at uas_disconnect() calling uas_free_streams() unconditionally. I wonder what this does if a device has morphed. It seems to me we'd try to free streams of endpoints that might not even exist. Regards Oliver -- 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: XHCI ERROR: Transfer error on endpoint
On 03/27/2014 12:31 PM, Keerthy V wrote: Hi, I am trying to send data to a USB device connected to a USB3.0 port over a USB bulk end point using libusb APIs . It is failing failed with error ERROR_IO. The same device and the test program works fine on a USB2.0 port. After enabling the XHCI debug options, I found that I am getting the following errors: "Endpoint xxx not halted, refusing to reset Transfer error on endpoint" I have tested in following kernel : 3.13.7 3.11.0-18 Both gave the same results. The test has been performed on USB 3.0 host controller. Can you please let me know the root cause and possible solutions? Attached is the debug log. Hi xhci currently refuses to reset non-halted endpoints. Turns out that in some cases reset endpoint is used as a "soft reset" for non halted endpoint and we should do a configure-endpoint instead. I'll start working on that once other work is completed But in your case the error this message is probably just a symptom of some other cause. Without the actual debug log (please remember to attach) it's difficult to say anything. A really wild guess is that it might be Powermanagement related (USB3 LPM) but that is just because we've seen other IO error issues where we suspect LPM. -Mathias -- 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: gadget: uvc: Switch to monotonic clock for buffer timestamps
Hi Hans,
On Friday 28 March 2014 09:50:42 Hans Verkuil wrote:
> On 03/23/2014 04:32 PM, Laurent Pinchart wrote:
> > The wall time clock isn't useful for applications as it can jump around
> > due to time adjustement. Switch to the monotonic clock.
> >
> > Signed-off-by: Laurent Pinchart
> > ---
> >
> > drivers/usb/gadget/uvc_queue.c | 12 +---
> > 1 file changed, 5 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/uvc_queue.c
> > b/drivers/usb/gadget/uvc_queue.c index 0bb5d50..d4561ba 100644
> > --- a/drivers/usb/gadget/uvc_queue.c
> > +++ b/drivers/usb/gadget/uvc_queue.c
> > @@ -364,6 +364,7 @@ static struct uvc_buffer *uvc_queue_next_buffer(struct
> > uvc_video_queue *queue,
> > struct uvc_buffer *buf)
> > {
> > struct uvc_buffer *nextbuf;
> > + struct timespec ts;
> >
> > if ((queue->flags & UVC_QUEUE_DROP_INCOMPLETE) &&
> > buf->length != buf->bytesused) {
> > @@ -379,14 +380,11 @@ static struct uvc_buffer
> > *uvc_queue_next_buffer(struct uvc_video_queue *queue,>
> > else
> > nextbuf = NULL;
> >
> > - /*
> > -* FIXME: with videobuf2, the sequence number or timestamp fields
> > -* are valid only for video capture devices and the UVC gadget
> > usually
> > -* is a video output device. Keeping these until the specs are clear
> > on
> > -* this aspect.
> > -*/
> > + ktime_get_ts(&ts);
>
> Why not use the v4l2-common.c helper v4l2_get_timestamp()?
No reason at all. I'll fix that.
> > +
> > buf->buf.v4l2_buf.sequence = queue->sequence++;
> >
> > - do_gettimeofday(&buf->buf.v4l2_buf.timestamp);
> > + buf->buf.v4l2_buf.timestamp.tv_sec = ts.tv_sec;
> > + buf->buf.v4l2_buf.timestamp.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
>
> You should also add:
>
> buf->buf.v4l2_buf.field = V4L2_FIELD_NONE;
>
> I noticed that that was never set, which is wrong.
Good point. Will be fixed in v2.
> > vb2_set_plane_payload(&buf->buf, 0, buf->bytesused);
> > vb2_buffer_done(&buf->buf, VB2_BUF_STATE_DONE);
--
Regards,
Laurent Pinchart
--
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] uas: fix error handling during scsi_scan()
Hi, On 03/28/2014 11:29 AM, oli...@neukum.org wrote: > From: Oliver Neukum > > intfdata is set only after scsi_scan(). uas_pre_reset() however > needs intfdata to be valid and will follow the NULL pointer > killing khubd. intfdata must be preemptively set before the > host is registered and undone in the error case. > > Signed-off-by: Oliver Neukum Good catch, thanks: Reviewed-by: Hans de Goede Acked-by: Hans de Goede Regards, Hans > --- > drivers/usb/storage/uas.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c > index b2bd8ea..511b229 100644 > --- a/drivers/usb/storage/uas.c > +++ b/drivers/usb/storage/uas.c > @@ -1096,16 +1096,17 @@ static int uas_probe(struct usb_interface *intf, > const struct usb_device_id *id) > if (result) > goto free_streams; > > + usb_set_intfdata(intf, shost); > result = scsi_add_host(shost, &intf->dev); > if (result) > goto free_streams; > > scsi_scan_host(shost); > - usb_set_intfdata(intf, shost); > return result; > > free_streams: > uas_free_streams(devinfo); > + usb_set_intfdata(intf, NULL); > set_alt0: > usb_set_interface(udev, intf->altsetting[0].desc.bInterfaceNumber, 0); > if (shost) > -- 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] uas: fix deadlocky memory allocations
On Fri, 2014-03-28 at 11:15 +0100, Hans de Goede wrote: > Hi, > > On 03/28/2014 11:14 AM, oli...@neukum.org wrote: > > From: Oliver Neukum > > > > There is an allocation with GFP_NOIO with a spinlock > > held that the checker didn't catch. > > I hate to be a PITA, but this above part of the commit message is no longer > correct, can you amend the commit message and resend please ? Sure. No problem. Regards Oliver -- 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] uas: fix error handling during scsi_scan()
From: Oliver Neukum intfdata is set only after scsi_scan(). uas_pre_reset() however needs intfdata to be valid and will follow the NULL pointer killing khubd. intfdata must be preemptively set before the host is registered and undone in the error case. Signed-off-by: Oliver Neukum --- drivers/usb/storage/uas.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index b2bd8ea..511b229 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -1096,16 +1096,17 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) if (result) goto free_streams; + usb_set_intfdata(intf, shost); result = scsi_add_host(shost, &intf->dev); if (result) goto free_streams; scsi_scan_host(shost); - usb_set_intfdata(intf, shost); return result; free_streams: uas_free_streams(devinfo); + usb_set_intfdata(intf, NULL); set_alt0: usb_set_interface(udev, intf->altsetting[0].desc.bInterfaceNumber, 0); if (shost) -- 1.8.4.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 2/2] USB: serial: add dell wwan card(413c:81a9)
Johan Hovold writes:
> [ +CC: Dan and Bjørn ]
>
> On Fri, Mar 28, 2014 at 03:25:06PM +0800, AceLan Kao wrote:
>> This patch adds support for Dell Computer Corp. Wireless 5808e 4G LTE
>> Mobile Broadband Card.
>
> Shouldn't this device be handled by the option driver?
>
> In fact, PID 0x8133 for 5720 VZW (which your clean-up patch touches)
> is listed in both option and sierra.
>
> I think we need to revert 1ebca9dad5ab ("USB: serial: add usbid for dell
> wwan card to sierra.c").
Maybe. If nothing else, additional PID digit in that patch looks a bit
silly :-)
It would be good to have a bit more information about these modules. As
you know, but the reporters may not be aware of, the fact that they are
made by Sierra does not necessarily mean that they are supported by the
sierra driver. That will depend on the chipset and firmware.
Just as an example: My Sierra Wireless MC7710 works fine with the sierra
driver if it is in "DirectIP" mode (using a 0x68a3 PID). But it fails
the sierra_set_power_state() control message I configure it for "QMI"
mode (using a 0x68a2 PID), adding a 5 second delay for every serial port
probed. This delay isn't necessarily noticed by the reporter... The
0x68a2 PID was also originally added to the sierra driver, and it did
sort of work.
I think we'd like to see at least a dmesg snippet with time stamps
showing the actual probing before accepting any new device into the
"sierra" driver. We do not want to add any device which does not
understand the Sierra vendor specific control messages.
And it would also be preferable to have a lsusb -v listing or similar,
so that we can match the different functions against other Sierra
devices. Sierra have been using a system with fixed interface numbers
for different vendor specific functions lately. The sierra driver has
knowledge about this. That's what the &direct_ip_interface_blacklist is
for. I'm pretty sure that any new Sierra module will need to have this
blacklist enabled, even if it turns out that the sierra driver is
correct.
Bjørn
>> Signed-off-by: AceLan Kao
>> ---
>> drivers/usb/serial/sierra.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
>> index b78fd4e..961d586 100644
>> --- a/drivers/usb/serial/sierra.c
>> +++ b/drivers/usb/serial/sierra.c
>> @@ -292,6 +292,7 @@ static const struct usb_device_id id_table[] = {
>>.driver_info = (kernel_ulong_t)&direct_ip_interface_blacklist
>> },
>> { USB_DEVICE(0x413C, 0x8133) }, /* Dell Computer Corp. Wireless 5720
>> VZW Mobile Broadband (EVDO Rev-A) Minicard GPS Port */
>> +{ USB_DEVICE(0x413C, 0x81A9) }, /* Dell Computer Corp. Wireless 5808e
>> 4G LTE Mobile Broadband Card */
>>
>> { }
>> };
--
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] uas: fix deadlocky memory allocations
From: Oliver Neukum
There are also two allocations with GFP_KERNEL in the pre-/post_reset
code paths. That is no good because that is a part of the SCSI error handler.
Signed-off-by: Oliver Neukum
---
drivers/usb/storage/uas.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 8f42226..b2bd8ea 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -1030,7 +1030,7 @@ static int uas_configure_endpoints(struct uas_dev_info
*devinfo)
devinfo->use_streams = 0;
} else {
devinfo->qdepth = usb_alloc_streams(devinfo->intf, eps + 1,
- 3, 256, GFP_KERNEL);
+ 3, 256, GFP_NOIO);
if (devinfo->qdepth < 0)
return devinfo->qdepth;
devinfo->use_streams = 1;
@@ -1047,7 +1047,7 @@ static void uas_free_streams(struct uas_dev_info *devinfo)
eps[0] = usb_pipe_endpoint(udev, devinfo->status_pipe);
eps[1] = usb_pipe_endpoint(udev, devinfo->data_in_pipe);
eps[2] = usb_pipe_endpoint(udev, devinfo->data_out_pipe);
- usb_free_streams(devinfo->intf, eps, 3, GFP_KERNEL);
+ usb_free_streams(devinfo->intf, eps, 3, GFP_NOIO);
}
static int uas_probe(struct usb_interface *intf, const struct usb_device_id
*id)
--
1.8.4.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] uas: fix deadlocky memory allocations
Hi,
On 03/28/2014 11:14 AM, oli...@neukum.org wrote:
> From: Oliver Neukum
>
> There is an allocation with GFP_NOIO with a spinlock
> held that the checker didn't catch.
I hate to be a PITA, but this above part of the commit message is no longer
correct, can you amend the commit message and resend please ?
Thanks,
Hans
> There are also two allocations with GFP_KERNEL in the pre-/post_reset
> code paths. That is no good because that is a part of the SCSI error handler.
>
> Signed-off-by: Oliver Neukum
> ---
> drivers/usb/storage/uas.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
> index 8f42226..b2bd8ea 100644
> --- a/drivers/usb/storage/uas.c
> +++ b/drivers/usb/storage/uas.c
> @@ -1030,7 +1030,7 @@ static int uas_configure_endpoints(struct uas_dev_info
> *devinfo)
> devinfo->use_streams = 0;
> } else {
> devinfo->qdepth = usb_alloc_streams(devinfo->intf, eps + 1,
> - 3, 256, GFP_KERNEL);
> + 3, 256, GFP_NOIO);
> if (devinfo->qdepth < 0)
> return devinfo->qdepth;
> devinfo->use_streams = 1;
> @@ -1047,7 +1047,7 @@ static void uas_free_streams(struct uas_dev_info
> *devinfo)
> eps[0] = usb_pipe_endpoint(udev, devinfo->status_pipe);
> eps[1] = usb_pipe_endpoint(udev, devinfo->data_in_pipe);
> eps[2] = usb_pipe_endpoint(udev, devinfo->data_out_pipe);
> - usb_free_streams(devinfo->intf, eps, 3, GFP_KERNEL);
> + usb_free_streams(devinfo->intf, eps, 3, GFP_NOIO);
> }
>
> static int uas_probe(struct usb_interface *intf, const struct usb_device_id
> *id)
>
--
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] USB: serial: ftdi_sio: add id for Brainboxes VX-001 ExpressCard
On Tue, Mar 25, 2014 at 07:00:51PM +, Michele Baldessari wrote:
> Custom VID/PID for Brainboxes VX-001 ExpressCard RS232 as reported in
> https://bugzilla.redhat.com/show_bug.cgi?id=1071914
Thanks for the patch.
How about including the other PIDs listed in the report (in the attached
udev-rule) as well?
> Signed-off-by: Michele Baldessari
> ---
> drivers/usb/serial/ftdi_sio.c | 2 ++
> drivers/usb/serial/ftdi_sio_ids.h | 7 +++
> 2 files changed, 9 insertions(+)
>
> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> index 44ab12986805..bf72063d6f48 100644
> --- a/drivers/usb/serial/ftdi_sio.c
> +++ b/drivers/usb/serial/ftdi_sio.c
> @@ -909,6 +909,8 @@ static const struct usb_device_id id_table_combined[] = {
> { USB_DEVICE(FTDI_VID, FTDI_Z3X_PID) },
> /* Cressi Devices */
> { USB_DEVICE(FTDI_VID, FTDI_CRESSI_PID) },
> + /* Brainbox Devices */
> + { USB_DEVICE(BRAINBOXES_VID, BRAINBOXES_VX_001_PID) },
> { } /* Terminating entry */
> };
>
> diff --git a/drivers/usb/serial/ftdi_sio_ids.h
> b/drivers/usb/serial/ftdi_sio_ids.h
> index e599fbfcde5f..ae842a203371 100644
> --- a/drivers/usb/serial/ftdi_sio_ids.h
> +++ b/drivers/usb/serial/ftdi_sio_ids.h
> @@ -1326,3 +1326,10 @@
> * Manufacturer: Cressi
> */
> #define FTDI_CRESSI_PID 0x87d0
> +
> +/*
> + * Product: ExpressCard 1 Port RS2323
> + * Manufacturer: Brainboxes
> + */
Could you just make this a "Brainboxes devices"-header and then list any
product in a comment following the PID instead? That would "scale"
better when adding more Brainboxes PIDs.
> +#define BRAINBOXES_VID 0x05d1
> +#define BRAINBOXES_VX_001_PID0x1001
That is, something like:
/*
* Brainboxes devices
*/
#define BRAINBOXES_VID 0x05d1
#define BRAINBOXES_VX_001_PID 0x1001 /* ExpressCard 1 Port RS2323 */
Thanks,
Johan
--
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] uas: fix deadlocky memory allocations
From: Oliver Neukum
There is an allocation with GFP_NOIO with a spinlock
held that the checker didn't catch.
There are also two allocations with GFP_KERNEL in the pre-/post_reset
code paths. That is no good because that is a part of the SCSI error handler.
Signed-off-by: Oliver Neukum
---
drivers/usb/storage/uas.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 8f42226..b2bd8ea 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -1030,7 +1030,7 @@ static int uas_configure_endpoints(struct uas_dev_info
*devinfo)
devinfo->use_streams = 0;
} else {
devinfo->qdepth = usb_alloc_streams(devinfo->intf, eps + 1,
- 3, 256, GFP_KERNEL);
+ 3, 256, GFP_NOIO);
if (devinfo->qdepth < 0)
return devinfo->qdepth;
devinfo->use_streams = 1;
@@ -1047,7 +1047,7 @@ static void uas_free_streams(struct uas_dev_info *devinfo)
eps[0] = usb_pipe_endpoint(udev, devinfo->status_pipe);
eps[1] = usb_pipe_endpoint(udev, devinfo->data_in_pipe);
eps[2] = usb_pipe_endpoint(udev, devinfo->data_out_pipe);
- usb_free_streams(devinfo->intf, eps, 3, GFP_KERNEL);
+ usb_free_streams(devinfo->intf, eps, 3, GFP_NOIO);
}
static int uas_probe(struct usb_interface *intf, const struct usb_device_id
*id)
--
1.8.4.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] uas: fix GFP_NOIO under spinlock
Hi,
On 03/28/2014 11:10 AM, oli...@neukum.org wrote:
> From: Oliver Neukum
>
> Quote Dan:
>
> The patch e36e64930cff: "uas: Use GFP_NOIO rather then GFP_ATOMIC
> where possible" from Nov 7, 2013, leads to the following static
> checker warning:
>
> drivers/usb/storage/uas.c:806 uas_eh_task_mgmt()
> error: scheduling with locks held: 'spin_lock:lock'
>
> Some other allocations under spinlock are not caught.
> The fix essentially reverts e36e64930cffd94e1c37fdb82f35989384aa946b
>
> Signed-off-by: Oliver Neukum
> Reported-by: Dan Carpenter
Thanks!
Reviewed-by: Hans de Goede
Acked-by: Hans de Goede
Regards,
Hans
> ---
> drivers/usb/storage/uas.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
> index a7ac97c..8f42226 100644
> --- a/drivers/usb/storage/uas.c
> +++ b/drivers/usb/storage/uas.c
> @@ -137,7 +137,7 @@ static void uas_do_work(struct work_struct *work)
> if (!(cmdinfo->state & IS_IN_WORK_LIST))
> continue;
>
> - err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_NOIO);
> + err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_ATOMIC);
> if (!err)
> cmdinfo->state &= ~IS_IN_WORK_LIST;
> else
> @@ -803,7 +803,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd,
>
> devinfo->running_task = 1;
> memset(&devinfo->response, 0, sizeof(devinfo->response));
> - sense_urb = uas_submit_sense_urb(cmnd, GFP_NOIO,
> + sense_urb = uas_submit_sense_urb(cmnd, GFP_ATOMIC,
>devinfo->use_streams ? tag : 0);
> if (!sense_urb) {
> shost_printk(KERN_INFO, shost,
> @@ -813,7 +813,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd,
> spin_unlock_irqrestore(&devinfo->lock, flags);
> return FAILED;
> }
> - if (uas_submit_task_urb(cmnd, GFP_NOIO, function, tag)) {
> + if (uas_submit_task_urb(cmnd, GFP_ATOMIC, function, tag)) {
> shost_printk(KERN_INFO, shost,
>"%s: %s: submit task mgmt urb failed\n",
>__func__, fname);
>
--
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] uas: fix GFP_NOIO under spinlock
From: Oliver Neukum
Quote Dan:
The patch e36e64930cff: "uas: Use GFP_NOIO rather then GFP_ATOMIC
where possible" from Nov 7, 2013, leads to the following static
checker warning:
drivers/usb/storage/uas.c:806 uas_eh_task_mgmt()
error: scheduling with locks held: 'spin_lock:lock'
Some other allocations under spinlock are not caught.
The fix essentially reverts e36e64930cffd94e1c37fdb82f35989384aa946b
Signed-off-by: Oliver Neukum
Reported-by: Dan Carpenter
---
drivers/usb/storage/uas.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index a7ac97c..8f42226 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -137,7 +137,7 @@ static void uas_do_work(struct work_struct *work)
if (!(cmdinfo->state & IS_IN_WORK_LIST))
continue;
- err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_NOIO);
+ err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_ATOMIC);
if (!err)
cmdinfo->state &= ~IS_IN_WORK_LIST;
else
@@ -803,7 +803,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd,
devinfo->running_task = 1;
memset(&devinfo->response, 0, sizeof(devinfo->response));
- sense_urb = uas_submit_sense_urb(cmnd, GFP_NOIO,
+ sense_urb = uas_submit_sense_urb(cmnd, GFP_ATOMIC,
devinfo->use_streams ? tag : 0);
if (!sense_urb) {
shost_printk(KERN_INFO, shost,
@@ -813,7 +813,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd,
spin_unlock_irqrestore(&devinfo->lock, flags);
return FAILED;
}
- if (uas_submit_task_urb(cmnd, GFP_NOIO, function, tag)) {
+ if (uas_submit_task_urb(cmnd, GFP_ATOMIC, function, tag)) {
shost_printk(KERN_INFO, shost,
"%s: %s: submit task mgmt urb failed\n",
__func__, fname);
--
1.8.4.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: uas_do_work() and flush_work()
Hi, On 03/28/2014 10:45 AM, Oliver Neukum wrote: > Hans, > > it seems to me that there is a problem. > > uas_pre_reset() calls flush_work(). uas_do_work(), however > can reschedule itself if usb_submit_urb() fails. That opens > up the possibility that there's a permanent problem with > the device which causes an attempt to reset it and makes > usb_submit_urb() fail. What do you think? There are 2 reset paths here: 1) Reset from uas_eh_bus_reset_handler, in this case uas_abort_inflight() has run first, so there will be no work to be done at all, and the flush_work + usb_wait_anchor_empty_timeout are nops 2) Everything is working fine from the uas pov, but another driver for another interface of the device (ie usbhid) does a reset. This is why the block-reqs + flush_work + usb_wait_anchor_empty_timeout are there. To avoid in flight commands getting aborted in this case. Of course the problem you describe could still happen in case of 2) happening while at the same time for some reason usb_submit_urb() starts failing. This is not a problem either, as flush_work only guarantees finishing any work queued before calling it (*), so if uas_do_work re-queues itself because of submission failure, flush_work will still return after the initial uas_do_work run. And then usb_wait_anchor_empty_timeout will go into the timeout, and the reset can move along. Note that since we've the usb_wait_anchor_empty_timeout there (which we need anyways) the flush_work() is not really necessary, it is meant as a yield to uas_do_work to speed things up. Regards, Hans *) See: https://www.kernel.org/doc/htmldocs/device-drivers/API-flush-work.html -- 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: serial: add dell wwan card(413c:81a9)
[ +CC: Dan and Bjørn ]
On Fri, Mar 28, 2014 at 03:25:06PM +0800, AceLan Kao wrote:
> This patch adds support for Dell Computer Corp. Wireless 5808e 4G LTE
> Mobile Broadband Card.
Shouldn't this device be handled by the option driver?
In fact, PID 0x8133 for 5720 VZW (which your clean-up patch touches)
is listed in both option and sierra.
I think we need to revert 1ebca9dad5ab ("USB: serial: add usbid for dell
wwan card to sierra.c").
Thanks,
Johan
> Signed-off-by: AceLan Kao
> ---
> drivers/usb/serial/sierra.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
> index b78fd4e..961d586 100644
> --- a/drivers/usb/serial/sierra.c
> +++ b/drivers/usb/serial/sierra.c
> @@ -292,6 +292,7 @@ static const struct usb_device_id id_table[] = {
> .driver_info = (kernel_ulong_t)&direct_ip_interface_blacklist
> },
> { USB_DEVICE(0x413C, 0x8133) }, /* Dell Computer Corp. Wireless 5720
> VZW Mobile Broadband (EVDO Rev-A) Minicard GPS Port */
> + { USB_DEVICE(0x413C, 0x81A9) }, /* Dell Computer Corp. Wireless 5808e
> 4G LTE Mobile Broadband Card */
>
> { }
> };
--
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] uas: fix deadlocky memory allocations
Hi,
On 03/28/2014 10:27 AM, oli...@neukum.org wrote:
> From: Oliver Neukum
>
> There is an allocation with GFP_NOIO with a spinlock
> held that the checker didn't catch.
ACK, my bad. When I wrote this commit:
http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/usb/storage/uas.c?id=e36e64930cffd94e1c37fdb82f35989384aa946b
I did not take spinlocks into account, only the general context
in which the code-block was being called. My bad.
So combined with your previous patch in essence this means that
commit e36e64930cffd94e1c37fdb82f35989384aa946b should be reverted,
it might be better to do a new patch doing just that (so that history
clearly shows that commit was bad, and has been completely undone.
> There are also two allocations with GFP_KERNEL in the pre-/post_reset
> code paths. That is no good because that is a part of the SCSI error handler.
Ah right, those should indeed be GFP_NOIO.
Can you respin the 2 patches into a revert of e36e64930 + a separate patch
fixing the pre / post reset handlers? I think that would be cleaner.
Otherwise ACK.
Thanks & Regards,
Hans
>
> Signed-off-by: Oliver Neukum
> ---
> drivers/usb/storage/uas.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
> index befa0c0..b2bd8ea 100644
> --- a/drivers/usb/storage/uas.c
> +++ b/drivers/usb/storage/uas.c
> @@ -137,7 +137,7 @@ static void uas_do_work(struct work_struct *work)
> if (!(cmdinfo->state & IS_IN_WORK_LIST))
> continue;
>
> - err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_NOIO);
> + err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_ATOMIC);
> if (!err)
> cmdinfo->state &= ~IS_IN_WORK_LIST;
> else
> @@ -1030,7 +1030,7 @@ static int uas_configure_endpoints(struct uas_dev_info
> *devinfo)
> devinfo->use_streams = 0;
> } else {
> devinfo->qdepth = usb_alloc_streams(devinfo->intf, eps + 1,
> - 3, 256, GFP_KERNEL);
> + 3, 256, GFP_NOIO);
> if (devinfo->qdepth < 0)
> return devinfo->qdepth;
> devinfo->use_streams = 1;
> @@ -1047,7 +1047,7 @@ static void uas_free_streams(struct uas_dev_info
> *devinfo)
> eps[0] = usb_pipe_endpoint(udev, devinfo->status_pipe);
> eps[1] = usb_pipe_endpoint(udev, devinfo->data_in_pipe);
> eps[2] = usb_pipe_endpoint(udev, devinfo->data_out_pipe);
> - usb_free_streams(devinfo->intf, eps, 3, GFP_KERNEL);
> + usb_free_streams(devinfo->intf, eps, 3, GFP_NOIO);
> }
>
> static int uas_probe(struct usb_interface *intf, const struct usb_device_id
> *id)
>
--
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
uas_do_work() and flush_work()
Hans, it seems to me that there is a problem. uas_pre_reset() calls flush_work(). uas_do_work(), however can reschedule itself if usb_submit_urb() fails. That opens up the possibility that there's a permanent problem with the device which causes an attempt to reset it and makes usb_submit_urb() fail. What do you think? Regards Oliver -- 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: Linux Patch to pl2303 driver to add USB ID's for new HP Pole Displays
On Wed, Mar 26, 2014 at 08:43:09PM +, Sanders, Aaron wrote:
> Hi Johan,
>
> I found you name on the list of Maintainers for Linux for
> "drivers/usb/serial/". I have never submitted change before so I am
> looking for your help. I would like to submit a very simple patch to
> add some USB ID's for new HP Pole Displays to pl2303 driver. Attached
> is a patch file made using diff.
>
> What do I need to do? Thank you for your help!
The diff in itself looks good, but please have a look at
Documentation/SubmittingPatches on how to generate the patch and submit
it.
Specifically, you need to add a descriptive subject line and commit
message (have a look at the git log for usb-serial for inspiration,
e.g. commit 598f0b703506 ("USB: pl2303: add id for SMART device")), and
a Signed-off-by tag.
git format-patch (and git send-email) can also be quite handy.
And remember to CC the linux-usb list (added to CC).
Thanks,
Johan
> diff -uNr old/pl2303.c new/pl2303.c
> --- old/pl2303.c 2014-03-26 11:06:20.526852963 -0700
> +++ new/pl2303.c 2014-03-26 11:06:32.154852611 -0700
> @@ -82,6 +82,9 @@
> { USB_DEVICE(YCCABLE_VENDOR_ID, YCCABLE_PRODUCT_ID) },
> { USB_DEVICE(SUPERIAL_VENDOR_ID, SUPERIAL_PRODUCT_ID) },
> { USB_DEVICE(HP_VENDOR_ID, HP_LD220_PRODUCT_ID) },
> + { USB_DEVICE(HP_VENDOR_ID, HP_LD960_PRODUCT_ID) },
> + { USB_DEVICE(HP_VENDOR_ID, HP_LCM220_PRODUCT_ID) },
> + { USB_DEVICE(HP_VENDOR_ID, HP_LCM960_PRODUCT_ID) },
> { USB_DEVICE(CRESSI_VENDOR_ID, CRESSI_EDY_PRODUCT_ID) },
> { USB_DEVICE(ZEAGLE_VENDOR_ID, ZEAGLE_N2ITION3_PRODUCT_ID) },
> { USB_DEVICE(SONY_VENDOR_ID, SONY_QN3USB_PRODUCT_ID) },
> diff -uNr old/pl2303.h new/pl2303.h
> --- old/pl2303.h 2014-03-26 11:06:11.030853251 -0700
> +++ new/pl2303.h 2014-03-26 11:06:37.462852450 -0700
> @@ -124,6 +124,9 @@
> /* Hewlett-Packard LD220-HP POS Pole Display */
> #define HP_VENDOR_ID 0x03f0
> #define HP_LD220_PRODUCT_ID 0x3524
> +#define HP_LD960_PRODUCT_ID 0x0B39
> +#define HP_LCM220_PRODUCT_ID 0x3139
> +#define HP_LCM960_PRODUCT_ID 0x3239
>
> /* Cressi Edy (diving computer) PC interface */
> #define CRESSI_VENDOR_ID 0x04b8
--
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] uas: fix deadlocky memory allocations
From: Oliver Neukum
There is an allocation with GFP_NOIO with a spinlock
held that the checker didn't catch.
There are also two allocations with GFP_KERNEL in the pre-/post_reset
code paths. That is no good because that is a part of the SCSI error handler.
Signed-off-by: Oliver Neukum
---
drivers/usb/storage/uas.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index befa0c0..b2bd8ea 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -137,7 +137,7 @@ static void uas_do_work(struct work_struct *work)
if (!(cmdinfo->state & IS_IN_WORK_LIST))
continue;
- err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_NOIO);
+ err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_ATOMIC);
if (!err)
cmdinfo->state &= ~IS_IN_WORK_LIST;
else
@@ -1030,7 +1030,7 @@ static int uas_configure_endpoints(struct uas_dev_info
*devinfo)
devinfo->use_streams = 0;
} else {
devinfo->qdepth = usb_alloc_streams(devinfo->intf, eps + 1,
- 3, 256, GFP_KERNEL);
+ 3, 256, GFP_NOIO);
if (devinfo->qdepth < 0)
return devinfo->qdepth;
devinfo->use_streams = 1;
@@ -1047,7 +1047,7 @@ static void uas_free_streams(struct uas_dev_info *devinfo)
eps[0] = usb_pipe_endpoint(udev, devinfo->status_pipe);
eps[1] = usb_pipe_endpoint(udev, devinfo->data_in_pipe);
eps[2] = usb_pipe_endpoint(udev, devinfo->data_out_pipe);
- usb_free_streams(devinfo->intf, eps, 3, GFP_KERNEL);
+ usb_free_streams(devinfo->intf, eps, 3, GFP_NOIO);
}
static int uas_probe(struct usb_interface *intf, const struct usb_device_id
*id)
--
1.8.4.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 2/2] usb: gadget: uvc: Set the vb2 queue timestamp flags
On 03/23/2014 04:32 PM, Laurent Pinchart wrote: > The vb2 queue timestamp_flags field must be set by drivers, as enforced > by a WARN_ON in vb2_queue_init. The UVC gadget driver failed to do so. > This resulted in the following warning. > > [2.104371] g_webcam gadget: uvc_function_bind > [2.105567] [ cut here ] > [2.105567] [ cut here ] > [2.106779] WARNING: CPU: 0 PID: 1 at > drivers/media/v4l2-core/videobuf2-core.c:2207 vb2_queue_init+0xa3/0x113() > > Fix it. > > Reported-by: Fengguang Wu > Signed-off-by: Laurent Pinchart Reviewed-by: Hans Verkuil Regards, Hans > --- > drivers/usb/gadget/uvc_queue.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/usb/gadget/uvc_queue.c b/drivers/usb/gadget/uvc_queue.c > index d4561ba..4611e9c 100644 > --- a/drivers/usb/gadget/uvc_queue.c > +++ b/drivers/usb/gadget/uvc_queue.c > @@ -136,6 +136,8 @@ static int uvc_queue_init(struct uvc_video_queue *queue, > queue->queue.buf_struct_size = sizeof(struct uvc_buffer); > queue->queue.ops = &uvc_queue_qops; > queue->queue.mem_ops = &vb2_vmalloc_memops; > + queue->queue.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC > + | V4L2_BUF_FLAG_TSTAMP_SRC_EOF; > ret = vb2_queue_init(&queue->queue); > if (ret) > return ret; > -- 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: gadget: uvc: Switch to monotonic clock for buffer timestamps
Hi Laurent,
I have a few comments:
On 03/23/2014 04:32 PM, Laurent Pinchart wrote:
> The wall time clock isn't useful for applications as it can jump around
> due to time adjustement. Switch to the monotonic clock.
>
> Signed-off-by: Laurent Pinchart
> ---
> drivers/usb/gadget/uvc_queue.c | 12 +---
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/gadget/uvc_queue.c b/drivers/usb/gadget/uvc_queue.c
> index 0bb5d50..d4561ba 100644
> --- a/drivers/usb/gadget/uvc_queue.c
> +++ b/drivers/usb/gadget/uvc_queue.c
> @@ -364,6 +364,7 @@ static struct uvc_buffer *uvc_queue_next_buffer(struct
> uvc_video_queue *queue,
> struct uvc_buffer *buf)
> {
> struct uvc_buffer *nextbuf;
> + struct timespec ts;
>
> if ((queue->flags & UVC_QUEUE_DROP_INCOMPLETE) &&
>buf->length != buf->bytesused) {
> @@ -379,14 +380,11 @@ static struct uvc_buffer *uvc_queue_next_buffer(struct
> uvc_video_queue *queue,
> else
> nextbuf = NULL;
>
> - /*
> - * FIXME: with videobuf2, the sequence number or timestamp fields
> - * are valid only for video capture devices and the UVC gadget usually
> - * is a video output device. Keeping these until the specs are clear on
> - * this aspect.
> - */
> + ktime_get_ts(&ts);
Why not use the v4l2-common.c helper v4l2_get_timestamp()?
> +
> buf->buf.v4l2_buf.sequence = queue->sequence++;
> - do_gettimeofday(&buf->buf.v4l2_buf.timestamp);
> + buf->buf.v4l2_buf.timestamp.tv_sec = ts.tv_sec;
> + buf->buf.v4l2_buf.timestamp.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
You should also add:
buf->buf.v4l2_buf.field = V4L2_FIELD_NONE;
I noticed that that was never set, which is wrong.
Regards,
Hans
>
> vb2_set_plane_payload(&buf->buf, 0, buf->bytesused);
> vb2_buffer_done(&buf->buf, VB2_BUF_STATE_DONE);
>
--
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] uas: fix GFP_NOIO under spinlock
Hi,
On 03/28/2014 09:45 AM, oli...@neukum.org wrote:
> From: Oliver Neukum
>
> Quote Dan:
>
> The patch e36e64930cff: "uas: Use GFP_NOIO rather then GFP_ATOMIC
> where possible" from Nov 7, 2013, leads to the following static
> checker warning:
>
> drivers/usb/storage/uas.c:806 uas_eh_task_mgmt()
> error: scheduling with locks held: 'spin_lock:lock'
> The fix is obvious.
>
> Signed-off-by: Oliver Neukum
> Reported-by: Dan Carpenter
Dan, thanks for catching this, Oliver thanks for fixing this.
Looks good to me, Greg can you please add this to usb-next?
It fixes a regression my uas set has introduced there.
Reviewed-by: Hans de Goede
Regards,
Hans
> ---
> drivers/usb/storage/uas.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
> index a7ac97c..befa0c0 100644
> --- a/drivers/usb/storage/uas.c
> +++ b/drivers/usb/storage/uas.c
> @@ -803,7 +803,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd,
>
> devinfo->running_task = 1;
> memset(&devinfo->response, 0, sizeof(devinfo->response));
> - sense_urb = uas_submit_sense_urb(cmnd, GFP_NOIO,
> + sense_urb = uas_submit_sense_urb(cmnd, GFP_ATOMIC,
>devinfo->use_streams ? tag : 0);
> if (!sense_urb) {
> shost_printk(KERN_INFO, shost,
> @@ -813,7 +813,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd,
> spin_unlock_irqrestore(&devinfo->lock, flags);
> return FAILED;
> }
> - if (uas_submit_task_urb(cmnd, GFP_NOIO, function, tag)) {
> + if (uas_submit_task_urb(cmnd, GFP_ATOMIC, function, tag)) {
> shost_printk(KERN_INFO, shost,
>"%s: %s: submit task mgmt urb failed\n",
>__func__, fname);
>
--
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] uas: fix GFP_NOIO under spinlock
From: Oliver Neukum
Quote Dan:
The patch e36e64930cff: "uas: Use GFP_NOIO rather then GFP_ATOMIC
where possible" from Nov 7, 2013, leads to the following static
checker warning:
drivers/usb/storage/uas.c:806 uas_eh_task_mgmt()
error: scheduling with locks held: 'spin_lock:lock'
The fix is obvious.
Signed-off-by: Oliver Neukum
Reported-by: Dan Carpenter
---
drivers/usb/storage/uas.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index a7ac97c..befa0c0 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -803,7 +803,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd,
devinfo->running_task = 1;
memset(&devinfo->response, 0, sizeof(devinfo->response));
- sense_urb = uas_submit_sense_urb(cmnd, GFP_NOIO,
+ sense_urb = uas_submit_sense_urb(cmnd, GFP_ATOMIC,
devinfo->use_streams ? tag : 0);
if (!sense_urb) {
shost_printk(KERN_INFO, shost,
@@ -813,7 +813,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd,
spin_unlock_irqrestore(&devinfo->lock, flags);
return FAILED;
}
- if (uas_submit_task_urb(cmnd, GFP_NOIO, function, tag)) {
+ if (uas_submit_task_urb(cmnd, GFP_ATOMIC, function, tag)) {
shost_printk(KERN_INFO, shost,
"%s: %s: submit task mgmt urb failed\n",
__func__, fname);
--
1.8.4.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: uas: Use GFP_NOIO rather then GFP_ATOMIC where possible
Hello Hans de Goede,
The patch e36e64930cff: "uas: Use GFP_NOIO rather then GFP_ATOMIC
where possible" from Nov 7, 2013, leads to the following static
checker warning:
drivers/usb/storage/uas.c:806 uas_eh_task_mgmt()
error: scheduling with locks held: 'spin_lock:lock'
drivers/usb/storage/uas.c
789 spin_lock_irqsave(&devinfo->lock, flags);
790
791 if (devinfo->resetting) {
792 spin_unlock_irqrestore(&devinfo->lock, flags);
793 return FAILED;
794 }
795
796 if (devinfo->running_task) {
797 shost_printk(KERN_INFO, shost,
798 "%s: %s: error already running a task\n",
799 __func__, fname);
800 spin_unlock_irqrestore(&devinfo->lock, flags);
801 return FAILED;
802 }
803
804 devinfo->running_task = 1;
805 memset(&devinfo->response, 0, sizeof(devinfo->response));
806 sense_urb = uas_submit_sense_urb(cmnd, GFP_NOIO,
The original code had this as GFP_ATOMIC because we can't sleep while
we have spin_lock_irqsave() held.
807 devinfo->use_streams ? tag :
0);
808 if (!sense_urb) {
809 shost_printk(KERN_INFO, shost,
810 "%s: %s: submit sense urb failed\n",
811 __func__, fname);
812 devinfo->running_task = 0;
813 spin_unlock_irqrestore(&devinfo->lock, flags);
814 return FAILED;
815 }
816 if (uas_submit_task_urb(cmnd, GFP_NOIO, function, tag)) {
Same.
817 shost_printk(KERN_INFO, shost,
818 "%s: %s: submit task mgmt urb failed\n",
819 __func__, fname);
820 devinfo->running_task = 0;
821 spin_unlock_irqrestore(&devinfo->lock, flags);
822 usb_kill_urb(sense_urb);
823 return FAILED;
824 }
825 spin_unlock_irqrestore(&devinfo->lock, flags);
regards,
dan carpenter
--
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] USB: serial: add dell wwan card(413c:81a9)
This patch adds support for Dell Computer Corp. Wireless 5808e 4G LTE
Mobile Broadband Card.
Signed-off-by: AceLan Kao
---
drivers/usb/serial/sierra.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
index b78fd4e..961d586 100644
--- a/drivers/usb/serial/sierra.c
+++ b/drivers/usb/serial/sierra.c
@@ -292,6 +292,7 @@ static const struct usb_device_id id_table[] = {
.driver_info = (kernel_ulong_t)&direct_ip_interface_blacklist
},
{ USB_DEVICE(0x413C, 0x8133) }, /* Dell Computer Corp. Wireless 5720
VZW Mobile Broadband (EVDO Rev-A) Minicard GPS Port */
+ { USB_DEVICE(0x413C, 0x81A9) }, /* Dell Computer Corp. Wireless 5808e
4G LTE Mobile Broadband Card */
{ }
};
--
1.9.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
[PATCH 1/2] USB: serial: fixed the product id typo
Fixed the product id typo and replace whitespace with tab.
Signed-off-by: AceLan Kao
---
drivers/usb/serial/sierra.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
index a9eb622..b78fd4e 100644
--- a/drivers/usb/serial/sierra.c
+++ b/drivers/usb/serial/sierra.c
@@ -291,7 +291,7 @@ static const struct usb_device_id id_table[] = {
{ USB_DEVICE(0x0f3d, 0x68A3), /* Airprime/Sierra Wireless Direct IP
modems */
.driver_info = (kernel_ulong_t)&direct_ip_interface_blacklist
},
- { USB_DEVICE(0x413C, 0x08133) }, /* Dell Computer Corp. Wireless 5720
VZW Mobile Broadband (EVDO Rev-A) Minicard GPS Port */
+ { USB_DEVICE(0x413C, 0x8133) }, /* Dell Computer Corp. Wireless 5720
VZW Mobile Broadband (EVDO Rev-A) Minicard GPS Port */
{ }
};
--
1.9.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
