RE: [PATCH v2 0/2] usb: renesas_usbhs: fix to avoid unexpected condition
Hi Felipe, > From: Yoshihiro Shimoda > Sent: Thursday, March 10, 2016 11:30 AM > > This patch set is based on the latest Felipe's usb.git / testing/next branch. > (commit id = ac5706631325ae3695bfa1527101ab2b2f64859f) Would you review the patch set? I confirmed that the patch set could be applied on the latest testing/next and testing/fixes branches. Best regards, Yoshihiro Shimoda > Changes from v1: > - Add "Fixes" and "Cc: . > - Revise the commit log of patch 2. > - consistent words: tx -> TX, irq -> IRQ > - typo: heppen -> happen > > Yoshihiro Shimoda (2): > usb: renesas_usbhs: avoid NULL pointer derefernce in > usbhsf_pkt_handler() > usb: renesas_usbhs: disable TX IRQ before starting TX DMAC transfer > > drivers/usb/renesas_usbhs/fifo.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > -- > 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
Re: [PATCH 1/2] usb: dwc3: core: Introduce dwc3_device_reinit()
Hi, John Youn writes: > [ text/plain ] > On 3/18/2016 12:17 PM, John Youn wrote: >> On 3/16/2016 6:56 AM, Felipe Balbi wrote: >>> >>> heh, +john >>> >>> Felipe Balbi writes: [ text/plain ] Hi, Roger Quadros writes: > [ text/plain ] > We will need this function for a workaround. > The function issues a softreset only to the device > controller and performs minimal re-initialization > so that the device controller can be usable. > > As some code is similar to dwc3_core_init() take out > common code into dwc3_get_gctl_quirks(). > > We add a new member (prtcap_mode) to struct dwc3 to > keep track of the current mode in the PRTCAPDIR register. > > Signed-off-by: Roger Quadros I must say, I don't like this at all :-p There's ONE known silicon which needs this because of a poor silicon integration which took an IP with a known erratum where it can't be made to work on lower speeds and STILL was integrated without a superspeed PHY. There's a reason why I never tried to push this upstream myself ;-) I'm really thinking we might be better off adding a quirk flag to skip the metastability workaround and allow this ONE silicon to set the controller to lower speed. John, can you check with your colleagues if we would ever fall into STAR#9000525659 if we set maximum speed to high speed during driver probe and never touch it again ? I would assume we don't really fall into the metastability workaround, right ? We're not doing any sort of PM for dwc3... > > Hi Felipe, > > Do you mean to keep DCFG.speed to SS and set dwc->maximum_speed to HS? > I don't see an issue with that as long as we always ignore > dwc->maximum_speed when programming DCFG.speed for all affected > versions of the core. As long as the DCFG.speed = SS, you should not > hit the STAR. I actually mean changing DCFG.speed during driver probe and never touching it again. Would that still cause problems ? -- balbi signature.asc Description: PGP signature
RE: [PATCH] usb: xhci: Fix incomplete PM resume operation due to XHCI commmand timeout
> -Original Message-
> From: Mathias Nyman [mailto:mathias.ny...@intel.com]
> Sent: Monday, March 21, 2016 2:46 PM
> To: Rajesh Bhagat ; Mathias Nyman
> ; linux-usb@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Cc: gre...@linuxfoundation.org; Sriram Dash
> Subject: Re: [PATCH] usb: xhci: Fix incomplete PM resume operation due to XHCI
> commmand timeout
>
> On 21.03.2016 06:18, Rajesh Bhagat wrote:
> >
> >
> >>
> >> Hi
> >>
> >> I think clearing the whole command ring is a bit too much in this case.
> >> It may cause issues for all attached devices when one command times out.
> >>
> >
> > Hi Mathias,
> >
> > I understand your point, But I want to understand how would completion
> > handler be called if a command is timed out and xhci_abort_cmd_ring is
> > successful. In this case all the code would be waiting on completion
> > handler forever.
> >
> >
> > 2. xhci_handle_command_timeout -> xhci_abort_cmd_ring(failure) ->
> > xhci_cleanup_command_queue -> xhci_complete_del_and_free_cmd
> >
> > In our case command is timed out, Hence we hit the case #2 but
> > xhci_abort_cmd_ring is success which does not calls complete.
>
> xhci_abort_cmd_ring() will write CA bit (CMD_RING_ABORT) to CRCR register.
> This will generate a command completion event with status "command aborted"
> for
> the pending command.
> This event is then followed by a "command ring stopped" command completion
> event.
>
> See xHCI specs 5.4.5 and 4.6.1.2
>
> handle_cmd_completion() will check if cmd_comp_code == COMP_CMD_ABORT, goto
> event_handled, and call xhci_complete_del_and_free_cmd(cmd, cmd_comp_code) for
> the aborted command.
>
> If xHCI already processed the aborted command, we might only get a command
> ring
> stopped event, in this case handle_cmd_completion() will call
> xhci_handle_stopped_cmd_ring(xhci, cmd), which will turn the commands that
> were
> tagged for "abort" that still remain on the command ring to NO-OP commands.
>
> The completion callback will be called for these NO-OP command later when we
> get a
> command completion event for them.
>
Thanks Mathias for detailed explanation. Now I understand how completion
handler is
supposed to be called in this scenario.
But in our case, somehow we are not getting any event and handle_cmd_completion
function
is not getting called even after successful xhci_abort_cmd_ring when command
timed out.
Now, my point here is code prior to this patch xhci: rework command timeout and
cancellation,
Code would have returned in case command timed out in xhci_alloc_dev itself.
- /* XXX: how much time for xHC slot assignment? */
- timeleft = wait_for_completion_interruptible_timeout(
- command->completion,
- XHCI_CMD_DEFAULT_TIMEOUT);
- if (timeleft <= 0) {
- xhci_warn(xhci, "%s while waiting for a slot\n",
- timeleft == 0 ? "Timeout" : "Signal");
- /* cancel the enable slot request */
- ret = xhci_cancel_cmd(xhci, NULL, command->command_trb);
- return ret;
- }
+ wait_for_completion(command->completion);
But after this patch, we are waiting for hardware event, which is somehow not
generated
and causing a hang scenario.
IMO, The assumption that "xhci_abort_cmd_ring would always generate an event
and handle_cmd_completion would be called" will not be always be true if HW is
in bad state.
Please share your opinion.
> >> What kernel version, and what xhci vendor was this triggered on?
> >>
> >
> > We are using 4.1.8 kernel
> >
>
> Are you able to try a more recent version?
>
Using a newer kernel version would be bit difficult, but I would surely try it.
> -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: 答复: 答复: 【xhci】suspend and resume core dump problem
On Tue, Mar 22, 2016 at 03:51:27AM +, Lipengcheng wrote: > Hi, >Thanks Lu Baolu, > >The core dump says "PC is at xhci_mem_cleanup+0x4e4/0x574 [xhci_hcd] ". > >But this call flow shows error happens in dma allocation. Which is correct? > >Or, anything I misunderstood? > Sorry . I am not description clear . > Because the interrupt makes the memory allocation fails, then enter the > error handling procedures(xhci_mem_cleanup).In the function,it is core dump. What is generating that interrupt? -- 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】suspend and resume core dump problem
On Tue, Mar 22, 2016 at 11:36:47AM +0800, Lu Baolu wrote: > Hi, > > On 03/22/2016 11:17 AM, Lipengcheng wrote: > > Hi, > > Thanks for your reply. > > When the suspend and resume process , the operation of press ctrl + c > > will produce an interruput and cause to allocate memory failed. It causes > > the usb3 module can not be used and core dump after resume process. > > Software call flow: > > |xhci_resume > > ||xhci_init > > |||xhci_mem_init > > dma_alloc_coherent > > |dma_alloc_from_contiguous > > In dma_alloc_from_contiguous return -4 (Interrupted system call). > > The core dump says "PC is at xhci_mem_cleanup+0x4e4/0x574 [xhci_hcd] ". > But this call flow shows error happens in dma allocation. Which is correct? > Or, anything I misunderstood? Why is that function failing? That seems suspicious. 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: 答复: 【xhci】suspend and resume core dump problem
On Tue, Mar 22, 2016 at 03:17:08AM +, Lipengcheng wrote: > Hi, > Thanks for your reply. > When the suspend and resume process , the operation of press ctrl + c will > produce an interruput and cause to allocate memory failed. It causes the usb3 > module can not be used and core dump after resume process. what are you pressing ctrl+c on? Why would that do anything on a USB keyboard to interrupt the host? > Software call flow: > |xhci_resume > ||xhci_init > |||xhci_mem_init > dma_alloc_coherent > |dma_alloc_from_contiguous > In dma_alloc_from_contiguous return -4 (Interrupted system call). Is this in some type of emulator? > > Core dump information： > > Vi Resume OK > PM resume OK > usb usb1: root hub lost power or was reset > usb usb2: root hub lost power or was reset > ^Cusb usb3: root hub lost power or was reset > usb usb4: root hub lost power or was reset > usb usb5: root hub lost power or was reset > usb usb6: root hub lost power or was reset > hiusb-xhci hiusb-xhci.0: Couldn't initialize memory > Unable to handle kernel NULL pointer dereference at virtual address > pgd = a7324000 > [] *pgd=27265831, *pte=, *ppte= > Internal error: Oops: 817 [#1] SMP ARM > Modules linked in: tntfs(PO) xhci_hcd ohci_hcd ehci_hcd hi_pmoc(O) hi_vi(O) > hi_keyled(O) hi_aenc(O) hi_venc(O) hi_png(O) hi_jpge(O) hi_jpeg(O) hi_ir(O) > hi_fb(O) hi_pwm(O) hi_mce(O) hi_avplay(O) hi_pvr(O) hi_sync(O) hi_vou(O) > hi_aiao(O) hi_adsp(O) hi_hdmi(O) hi_cipher(O) hi_vdec(O) hi_vpss(O) hi_pq(O) > hi_pdm(O) hi_svdec(O) hi_vfmw(O) hi_adec(O) hi_demux(O) hi_otp(O) hi_tde(O) > mali(O) hi_i2c(O) hi_gpio_i2c(O) hi_gpio(O) hi_common(O) hi_mmz(O) hi_media(O) > CPU: 0 PID: 1113 Comm: sample_pmoc Tainted: P O 3.10.0_s40 #1 Can you remove all of your "custom" and closed source kernel drivers? Also 3.10 is very old and obsolete, please work with the company that is forcing you to use such a kernel version, as you are paying for support from them. Does this happen on 4.5? 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: 答复: 【xhci】suspend and resume core dump problem
Hi, On 03/22/2016 11:17 AM, Lipengcheng wrote: > Hi, > Thanks for your reply. > When the suspend and resume process , the operation of press ctrl + c will > produce an interruput and cause to allocate memory failed. It causes the usb3 > module can not be used and core dump after resume process. > Software call flow: > |xhci_resume > ||xhci_init > |||xhci_mem_init > dma_alloc_coherent > |dma_alloc_from_contiguous > In dma_alloc_from_contiguous return -4 (Interrupted system call). The core dump says "PC is at xhci_mem_cleanup+0x4e4/0x574 [xhci_hcd] ". But this call flow shows error happens in dma allocation. Which is correct? Or, anything I misunderstood? Best regards, Baolu > > Core dump information： > > Vi Resume OK > PM resume OK > usb usb1: root hub lost power or was reset > usb usb2: root hub lost power or was reset > ^Cusb usb3: root hub lost power or was reset > usb usb4: root hub lost power or was reset > usb usb5: root hub lost power or was reset > usb usb6: root hub lost power or was reset > hiusb-xhci hiusb-xhci.0: Couldn't initialize memory > Unable to handle kernel NULL pointer dereference at virtual address > pgd = a7324000 > [] *pgd=27265831, *pte=, *ppte= > Internal error: Oops: 817 [#1] SMP ARM > Modules linked in: tntfs(PO) xhci_hcd ohci_hcd ehci_hcd hi_pmoc(O) hi_vi(O) > hi_keyled(O) hi_aenc(O) hi_venc(O) hi_png(O) hi_jpge(O) hi_jpeg(O) hi_ir(O) > hi_fb(O) hi_pwm(O) hi_mce(O) hi_avplay(O) hi_pvr(O) hi_sync(O) hi_vou(O) > hi_aiao(O) hi_adsp(O) hi_hdmi(O) hi_cipher(O) hi_vdec(O) hi_vpss(O) hi_pq(O) > hi_pdm(O) hi_svdec(O) hi_vfmw(O) hi_adec(O) hi_demux(O) hi_otp(O) hi_tde(O) > mali(O) hi_i2c(O) hi_gpio_i2c(O) hi_gpio(O) hi_common(O) hi_mmz(O) hi_media(O) > CPU: 0 PID: 1113 Comm: sample_pmoc Tainted: P O 3.10.0_s40 #1 > task: a7ba9a40 ti: a72e2000 task.ti: a72e2000 > PC is at xhci_mem_cleanup+0x4e4/0x574 [xhci_hcd] > LR is at 0x0 > pc : [<7f840468>]lr : [<>]psr: 8013 > sp : a72e3ca8 ip : a79aef18 fp : a72e3cdc > r10: r9 : 0002 r8 : 00100100 > r7 : 00200200 r6 : r5 : a79ae800 r4 : a691b000 > r3 : a79af000 r2 : a79ae800 r1 : a79aec00 r0 : a79af000 > Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user > Control: 10c53c7d Table: 2732406a DAC: 0015 > > SP: 0xa72e3c28: > 3c28 a72e3c5c a72e3c38 8002158c 80355df4 1000 a72e3d04 7f840468 8013 > 3c48 a72e3c94 a72e3cdc a72e3c60 80014c98 8000846c a79af000 a79aec00 > 3c68 a79ae800 a79af000 a691b000 a79ae800 00200200 00100100 0002 > 3c88 a72e3cdc a79aef18 a72e3ca8 7f840468 8013 > 3ca8 a68236a4 a7330a00 a72e3cdc a691b000 7f84ce88 a72e3d04 a68236a4 > 3cc8 0010 a7330a00 a72e3d34 a72e3ce0 7f841d54 7f83ff90 a691b000 > 3ce8 0338 00200200 00100100 00d0 0002 a72e3d44 > 3d08 7f8404d8 a691b000 a6823600 8086c0c0 0001 a68236a4 0010 a7330a00 > > IP: 0xa79aee98: > ee98 a79aeea0 a79aeea0 > eeb8 a79aeeb8 a79aeeb8 a79aeed0 a79aeed0 > eed8 a79aeee8 a79aeee8 > eef8 a79aef00 a79aef00 > ef18 a79aef18 a79aef18 > ef38 > ef58 > ef78 > > FP: 0xa72e3c5c: > 3c5c 8000846c a79af000 a79aec00 a79ae800 a79af000 a691b000 a79ae800 > 3c7c 00200200 00100100 0002 a72e3cdc a79aef18 a72e3ca8 > 3c9c 7f840468 8013 a68236a4 a7330a00 a72e3cdc a691b000 7f84ce88 > 3cbc a72e3d04 a68236a4 0010 a7330a00 a72e3d34 a72e3ce0 7f841d54 > 3cdc 7f83ff90 a691b000 0338 00200200 00100100 00d0 > 3cfc 0002 a72e3d44 7f8404d8 a691b000 a6823600 8086c0c0 0001 > 3d1c a68236a4 0010 a7330a00 a72e3d44 a72e3d38 7f83da28 7f840e94 a72e3d7c > 3d3c a72e3d48 7f83df78 7f83d9e4 2710 a72e3d58 803d977c a691b000 80352fe0 > > R0: 0xa79aef80: > ef80 > efa0 > efc0 > efe0 > f000 a79ae800 a72c7658 8005e52c > f020 > f040 > f060 > > R1: 0xa79aeb80: > e
Re: 【xhci】suspend and resume core dump problem
On Tue, Mar 22, 2016 at 01:31:40AM +, Lipengcheng wrote: >Hi, > we have a problem like that we can start kernel successfully without > processing any other operations, but failed after pressing ctrl + c. After > our analsys, it is because of that, ctrl + c will produce an interruput,it > will be dealed with in xhci_resume->xhci_init->xhci_mem_init so that > dma_alloc_coherent fail, finally it will be calm down in the FUNCITON > LIST_DEL(&tt->tt_list). > As the modification , set flag to GFP_ATOMIC, disabling interrupt requests > will not be core dump, could you please tell me can I modify like this? > > Thanks very much > > Best Regards, > > Pengcheng Li > > > Modify 1: > xhci->dcbaa = dma_alloc_coherent(dev, sizeof(*xhci->dcbaa), &dma, > - GFP_KERNEL); > + GFP_ATOMIC); > Modify 2: > xhci->erst.entries = dma_alloc_coherent(dev, > sizeof(struct xhci_erst_entry) * ERST_NUM_SEGS, &dma, > - GFP_KERNEL); > + GFP_ATOMIC); > Modify 3: > - retval = xhci_mem_init(xhci, GFP_KERNEL); > + retval = xhci_mem_init(xhci, GFP_ATOMIC); > xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Finished xhci_init"); Why isn't the 'flags' variable being use here instead of hard-coding GFP_KERNEL? Hm, it looks like that variable doesn't really make much sense as it's only ever called with GFP_KERNEL... Anyway, this is during init, there should not be any locks happening, so you can sleep, so using GFP_KERNEL is correct, I don't understand why GFP_ATOMIC is needed here. 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] musb: sunxi-glue: Fix sunxi-musb driver not auto-loading
On Mon, Mar 21, 2016 at 7:13 PM, Hans de Goede wrote: > Add a missing MODULE_DEVICE_TABLE() line which was causing the > sunxi-musb glue to not auto-load when build as a module. > > Signed-off-by: Hans de Goede > --- Reviewed-by: Javier Martinez Canillas Best regards, Javier -- 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 5/5] USB: serial: cp210x: Added more error handling to open()
Added error handling to register accesses made by open(), so it doesn't
succeed if anything goes wrong.
Signed-off-by: Konstantin Shkolnyy
---
drivers/usb/serial/cp210x.c | 41 -
1 file changed, 28 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index 58cffc9..785ce10 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -31,8 +31,8 @@
*/
static int cp210x_open(struct tty_struct *tty, struct usb_serial_port *);
static void cp210x_close(struct usb_serial_port *);
-static void cp210x_get_termios(struct tty_struct *, struct usb_serial_port *);
-static void cp210x_get_termios_port(struct usb_serial_port *port,
+static int cp210x_get_termios(struct tty_struct *, struct usb_serial_port *);
+static int cp210x_get_termios_port(struct usb_serial_port *port,
unsigned int *cflagp, unsigned int *baudp);
static void cp210x_change_speed(struct tty_struct *, struct usb_serial_port *,
struct ktermios *);
@@ -595,7 +595,9 @@ static int cp210x_open(struct tty_struct *tty, struct
usb_serial_port *port)
}
/* Configure the termios structure */
- cp210x_get_termios(tty, port);
+ result = cp210x_get_termios(tty, port);
+ if (result)
+ return result;
/* The baud rate must be initialised on cp2104 */
if (tty)
@@ -665,27 +667,29 @@ static bool cp210x_tx_empty(struct usb_serial_port *port)
* from the device, corrects any unsupported values, and configures the
* termios structure to reflect the state of the device
*/
-static void cp210x_get_termios(struct tty_struct *tty,
+static int cp210x_get_termios(struct tty_struct *tty,
struct usb_serial_port *port)
{
unsigned int baud;
+ int err;
if (tty) {
- cp210x_get_termios_port(tty->driver_data,
+ err = cp210x_get_termios_port(tty->driver_data,
&tty->termios.c_cflag, &baud);
tty_encode_baud_rate(tty, baud, baud);
} else {
unsigned int cflag;
cflag = 0;
- cp210x_get_termios_port(port, &cflag, &baud);
+ err = cp210x_get_termios_port(port, &cflag, &baud);
}
+ return err;
}
/*
* cp210x_get_termios_port
* This is the heart of cp210x_get_termios which always uses a
&usb_serial_port.
*/
-static void cp210x_get_termios_port(struct usb_serial_port *port,
+static int cp210x_get_termios_port(struct usb_serial_port *port,
unsigned int *cflagp, unsigned int *baudp)
{
struct device *dev = &port->dev;
@@ -694,15 +698,19 @@ static void cp210x_get_termios_port(struct
usb_serial_port *port,
u32 baud;
u16 old_bits;
u16 bits;
+ int err;
- cp210x_read_u32_reg(port, CP210X_GET_BAUDRATE, &baud);
-
+ err = cp210x_read_u32_reg(port, CP210X_GET_BAUDRATE, &baud);
+ if (err)
+ return err;
dev_dbg(dev, "%s - baud rate = %d\n", __func__, baud);
*baudp = baud;
cflag = *cflagp;
- cp210x_get_line_ctl(port, &bits);
+ err = cp210x_get_line_ctl(port, &bits);
+ if (err)
+ return err;
old_bits = bits;
cflag &= ~CSIZE;
switch (bits & BITS_DATA_MASK) {
@@ -785,11 +793,16 @@ static void cp210x_get_termios_port(struct
usb_serial_port *port,
break;
}
- if (bits != old_bits)
- cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits);
+ if (bits != old_bits) {
+ err = cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits);
+ if (err)
+ return err;
+ }
- cp210x_read_reg_block(port, CP210X_GET_FLOW, modem_ctl,
+ err = cp210x_read_reg_block(port, CP210X_GET_FLOW, modem_ctl,
sizeof(modem_ctl));
+ if (err)
+ return err;
if (modem_ctl[0] & 0x08) { /* if SERIAL_CTS_HANDSHAKE */
dev_dbg(dev, "%s - flow control = CRTSCTS\n", __func__);
cflag |= CRTSCTS;
@@ -799,6 +812,8 @@ static void cp210x_get_termios_port(struct usb_serial_port
*port,
}
*cflagp = cflag;
+
+ return 0;
}
/*
--
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
[PATCH 3/5] USB: serial: cp210x: Added comments to CRTSCT flag code.
Documented "magic numbers" used in the CRTSCT flag code in terms of
register bit names from the chip specification.
Signed-off-by: Konstantin Shkolnyy
---
drivers/usb/serial/cp210x.c | 38 ++
1 file changed, 34 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index ede5c52..b2321a7 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -790,7 +790,7 @@ static void cp210x_get_termios_port(struct usb_serial_port
*port,
cp210x_read_reg_block(port, CP210X_GET_FLOW, modem_ctl,
sizeof(modem_ctl));
- if (modem_ctl[0] & 0x08) {
+ if (modem_ctl[0] & 0x08) { /* if SERIAL_CTS_HANDSHAKE */
dev_dbg(dev, "%s - flow control = CRTSCTS\n", __func__);
cflag |= CRTSCTS;
} else {
@@ -952,17 +952,47 @@ static void cp210x_set_termios(struct tty_struct *tty,
__func__, modem_ctl[0], modem_ctl[4], modem_ctl[7]);
if (cflag & CRTSCTS) {
- modem_ctl[0] &= ~0x7B;
+ modem_ctl[0] &= ~0x7B; /* keep reserved D2 and D7 */
+ /*
+* D1-D0 SERIAL_DTR_MASK =01b: DTR is held active
+* D3 SERIAL_CTS_HANDSHAKE =1: CTS is a handshake line
+* D4 SERIAL_DSR_HANDSHAKE =0
+* D5 SERIAL_DCD_HANDSHAKE =0
+* D6 SERIAL_DSR_SENSITIVITY =0
+*/
modem_ctl[0] |= 0x09;
+ /*
+* D0 SERIAL_AUTO_TRANSMIT =0
+* D1 SERIAL_AUTO_RECEIVE =0
+* D2 SERIAL_ERROR_CHAR =0
+* D3 SERIAL_NULL_STRIPPING =0
+* D4 SERIAL_BREAK_CHAR =0
+* D7-D6 SERIAL_RTS_MASK =10b: RTS for rcv flow control
+*/
modem_ctl[4] = 0x80;
dev_dbg(dev, "%s - flow control = CRTSCTS\n", __func__);
} else {
- modem_ctl[0] &= ~0x7B;
+ modem_ctl[0] &= ~0x7B; /* keep reserved D2 and D7 */
+ /*
+* D1-D0 SERIAL_DTR_MASK = 01b: DTR is held active
+* D3 SERIAL_CTS_HANDSHAKE = 0: CTS is a status input.
+* D4 SERIAL_DSR_HANDSHAKE = 0
+* D5 SERIAL_DCD_HANDSHAKE = 0
+* D6 SERIAL_DSR_SENSITIVITY = 0
+*/
modem_ctl[0] |= 0x01;
+ /*
+* D0 SERIAL_AUTO_TRANSMIT = 0
+* D1 SERIAL_AUTO_RECEIVE = 0
+* D2 SERIAL_ERROR_CHAR = 0
+* D3 SERIAL_NULL_STRIPPING = 0
+* D4 SERIAL_BREAK_CHAR = 0
+* D7-D6 SERIAL_RTS_MASK =01b: RTS is statically active
+*/
modem_ctl[4] = 0x40;
dev_dbg(dev, "%s - flow control = NONE\n", __func__);
}
- modem_ctl[7] = 0;
+ modem_ctl[7] = 0; /* SERIAL_XOFF_CONTINUE = 0 */
dev_dbg(dev, "%s - write modem controls = %02x .. .. .. %02x ..
.. %02x\n",
__func__, modem_ctl[0], modem_ctl[4], modem_ctl[7]);
--
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
[PATCH 2/5] USB: serial: cp210x: Made sure SERIAL_XOFF_CONTINUE flag is clear.
The CRTCTS flag code intended to clear the SERIAL_XOFF_CONTINUE flag, but
did it inconsistently. This change is non-functional for existing chips
because the driver never set the flag and it's clear by default.
Signed-off-by: Konstantin Shkolnyy
---
drivers/usb/serial/cp210x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index de1606f..ede5c52 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -955,7 +955,6 @@ static void cp210x_set_termios(struct tty_struct *tty,
modem_ctl[0] &= ~0x7B;
modem_ctl[0] |= 0x09;
modem_ctl[4] = 0x80;
- modem_ctl[7] = 0;
dev_dbg(dev, "%s - flow control = CRTSCTS\n", __func__);
} else {
modem_ctl[0] &= ~0x7B;
@@ -963,6 +962,7 @@ static void cp210x_set_termios(struct tty_struct *tty,
modem_ctl[4] = 0x40;
dev_dbg(dev, "%s - flow control = NONE\n", __func__);
}
+ modem_ctl[7] = 0;
dev_dbg(dev, "%s - write modem controls = %02x .. .. .. %02x ..
.. %02x\n",
__func__, modem_ctl[0], modem_ctl[4], modem_ctl[7]);
--
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
[PATCH 4/5] USB: serial: cp210x: Prepared get_termios() for adding error handling
Replaced several register write calls with one, to simplify adding error
handling.
Signed-off-by: Konstantin Shkolnyy
---
drivers/usb/serial/cp210x.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index b2321a7..58cffc9 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -692,6 +692,7 @@ static void cp210x_get_termios_port(struct usb_serial_port
*port,
unsigned int cflag;
u8 modem_ctl[16];
u32 baud;
+ u16 old_bits;
u16 bits;
cp210x_read_u32_reg(port, CP210X_GET_BAUDRATE, &baud);
@@ -702,6 +703,7 @@ static void cp210x_get_termios_port(struct usb_serial_port
*port,
cflag = *cflagp;
cp210x_get_line_ctl(port, &bits);
+ old_bits = bits;
cflag &= ~CSIZE;
switch (bits & BITS_DATA_MASK) {
case BITS_DATA_5:
@@ -725,14 +727,12 @@ static void cp210x_get_termios_port(struct
usb_serial_port *port,
cflag |= CS8;
bits &= ~BITS_DATA_MASK;
bits |= BITS_DATA_8;
- cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits);
break;
default:
dev_dbg(dev, "%s - Unknown number of data bits, using 8\n",
__func__);
cflag |= CS8;
bits &= ~BITS_DATA_MASK;
bits |= BITS_DATA_8;
- cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits);
break;
}
@@ -763,7 +763,6 @@ static void cp210x_get_termios_port(struct usb_serial_port
*port,
dev_dbg(dev, "%s - Unknown parity mode, disabling parity\n",
__func__);
cflag &= ~PARENB;
bits &= ~BITS_PARITY_MASK;
- cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits);
break;
}
@@ -775,7 +774,6 @@ static void cp210x_get_termios_port(struct usb_serial_port
*port,
case BITS_STOP_1_5:
dev_dbg(dev, "%s - stop bits = 1.5 (not supported, using 1 stop
bit)\n", __func__);
bits &= ~BITS_STOP_MASK;
- cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits);
break;
case BITS_STOP_2:
dev_dbg(dev, "%s - stop bits = 2\n", __func__);
@@ -784,10 +782,12 @@ static void cp210x_get_termios_port(struct
usb_serial_port *port,
default:
dev_dbg(dev, "%s - Unknown number of stop bits, using 1 stop
bit\n", __func__);
bits &= ~BITS_STOP_MASK;
- cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits);
break;
}
+ if (bits != old_bits)
+ cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits);
+
cp210x_read_reg_block(port, CP210X_GET_FLOW, modem_ctl,
sizeof(modem_ctl));
if (modem_ctl[0] & 0x08) { /* if SERIAL_CTS_HANDSHAKE */
--
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
[PATCH 1/5] USB: serial: cp210x: Fixed RTS mode setting by the CRTSCTS flag.
A bug in the CRTSCT handling caused RTS to alternate between
CRTSCTS=0 => "RTS transmits active signal" and
CRTSCTS=1 => "RTS receives flow control"
instead of
CRTSCTS=0 => "RTS is statically active" and
CRTSCTS=1 => "RTS receives flow control"
Signed-off-by: Konstantin Shkolnyy
---
drivers/usb/serial/cp210x.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index c740592..de1606f 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -955,16 +955,12 @@ static void cp210x_set_termios(struct tty_struct *tty,
modem_ctl[0] &= ~0x7B;
modem_ctl[0] |= 0x09;
modem_ctl[4] = 0x80;
- /* FIXME - why clear reserved bits just read? */
- modem_ctl[5] = 0;
- modem_ctl[6] = 0;
modem_ctl[7] = 0;
dev_dbg(dev, "%s - flow control = CRTSCTS\n", __func__);
} else {
modem_ctl[0] &= ~0x7B;
modem_ctl[0] |= 0x01;
- /* FIXME - OR here instead of assignment looks wrong */
- modem_ctl[4] |= 0x40;
+ modem_ctl[4] = 0x40;
dev_dbg(dev, "%s - flow control = NONE\n", __func__);
}
--
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
[PATCH] musb: sunxi-glue: Fix sunxi-musb driver not auto-loading
Add a missing MODULE_DEVICE_TABLE() line which was causing the
sunxi-musb glue to not auto-load when build as a module.
Signed-off-by: Hans de Goede
---
drivers/usb/musb/sunxi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
index 4b472b8..9af9431 100644
--- a/drivers/usb/musb/sunxi.c
+++ b/drivers/usb/musb/sunxi.c
@@ -756,6 +756,7 @@ static const struct of_device_id sunxi_musb_match[] = {
{ .compatible = "allwinner,sun8i-a33-musb", },
{}
};
+MODULE_DEVICE_TABLE(of, sunxi_musb_match);
static struct platform_driver sunxi_musb_driver = {
.probe = sunxi_musb_probe,
--
2.7.3
--
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: "reset full-speed USB device number 6 using ehci-pci" with Dell Inspiron 15R 5537
On Mon, 2016-03-21 at 11:46 -0400, Alan Stern wrote: > On Mon, 21 Mar 2016, Oliver Neukum wrote: > > > On Fri, 2016-03-18 at 12:36 -0400, Alan Stern wrote: > > > > Almost. In case of reset_resume() it makes no sense to still > > clear a halt or execute a reset that had been ordered. The > > corresponding flags should be cleared. > > hid_post_reset() already clears the HID_RESET_PENDING flag. It also > needs to clear the HID_CLEAR_HALT flag; I'll add that in. > > > And now that I think of it, if while restarting IO an > > old error condition is detected shouldn't the restart wait for > > the error to be handled? > > Yes; we should avoid starting I/O if a reset is pending and we should > avoid starting the interrupt-IN URB if a clear-halt is pending. How > does this patch look? Perfect. 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: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM
On Mon, 21 Mar 2016 woojung@microchip.com wrote: > > > > But this leaves open the issue that querying the device too often will > > > > prevent it from going into autosuspend. It seems to me that the best > > > > way to deal with this is to make sure that the autosuspend timeout is > > > > shorter than the interal between queries, not to make the querying > > > > conditional on !runtime_auto. > > > > > > Short autosuspend timeout can affect performance. For instance our > > experiments showed that > > > shorter than 10sec timeout made Ethernet performance degrade because > > of wakeup delays. > > > So, just putting shorter timeout may have some side effects. > > > > Sure. This just means that you need a long statistics interval -- > > longer than the autosuspend timeout. That's why I suggested making the > > interval adjustable. > > What do you mean statistics interval? > Interval calling ndo_get_stats64 or another thread/timer or else getting > statistics? The time between calls to ndo_get_stats64, since that's the routine which queries the device. 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] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM
> > > But this leaves open the issue that querying the device too often will > > > prevent it from going into autosuspend. It seems to me that the best > > > way to deal with this is to make sure that the autosuspend timeout is > > > shorter than the interal between queries, not to make the querying > > > conditional on !runtime_auto. > > > > Short autosuspend timeout can affect performance. For instance our > experiments showed that > > shorter than 10sec timeout made Ethernet performance degrade because > of wakeup delays. > > So, just putting shorter timeout may have some side effects. > > Sure. This just means that you need a long statistics interval -- > longer than the autosuspend timeout. That's why I suggested making the > interval adjustable. What do you mean statistics interval? Interval calling ndo_get_stats64 or another thread/timer or else getting statistics? Thanks. Woojung -- 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: dwc3: core: Introduce dwc3_device_reinit()
On 3/18/2016 12:17 PM, John Youn wrote: > On 3/16/2016 6:56 AM, Felipe Balbi wrote: >> >> heh, +john >> >> Felipe Balbi writes: >>> [ text/plain ] >>> >>> Hi, >>> >>> Roger Quadros writes: [ text/plain ] We will need this function for a workaround. The function issues a softreset only to the device controller and performs minimal re-initialization so that the device controller can be usable. As some code is similar to dwc3_core_init() take out common code into dwc3_get_gctl_quirks(). We add a new member (prtcap_mode) to struct dwc3 to keep track of the current mode in the PRTCAPDIR register. Signed-off-by: Roger Quadros >>> >>> I must say, I don't like this at all :-p There's ONE known silicon which >>> needs this because of a poor silicon integration which took an IP with a >>> known erratum where it can't be made to work on lower speeds and STILL >>> was integrated without a superspeed PHY. >>> >>> There's a reason why I never tried to push this upstream myself ;-) >>> >>> I'm really thinking we might be better off adding a quirk flag to skip >>> the metastability workaround and allow this ONE silicon to set the >>> controller to lower speed. >>> >>> John, can you check with your colleagues if we would ever fall into >>> STAR#9000525659 if we set maximum speed to high speed during driver >>> probe and never touch it again ? I would assume we don't really fall >>> into the metastability workaround, right ? We're not doing any sort of >>> PM for dwc3... >>> Hi Felipe, Do you mean to keep DCFG.speed to SS and set dwc->maximum_speed to HS? I don't see an issue with that as long as we always ignore dwc->maximum_speed when programming DCFG.speed for all affected versions of the core. As long as the DCFG.speed = SS, you should not hit the STAR. John -- 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
FOR YOUR INFORMATION DEAR BENEFICIARY,
FOR YOUR INFORMATION DEAR BENEFICIARY, Your Over-due ATM Card payment of $1.5MUSD by the UN Office have deposited to the ATM MASTER CARD OFFICE. All you have to do now is to contact the Office Manager Dr. peter Akupa Boni at: (peterakupa...@gmail.com) and Phone number: +229 61 31 07 78 , he will give you direction on how you will receive your ATM MASTER CARD immediately. Contact person: Dr. peter Akupa, E-mail: (peterakupa...@gmail.com ) You have to also contact him with the following details: 1. Your full name: 2. Your contact cell phone number: 3. Your age: 4. your Email Thanks David Kedogo -- 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] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM
On Mon, 21 Mar 2016, Oliver Neukum wrote: > On Mon, 2016-03-21 at 14:24 -0400, Alan Stern wrote: > > On Mon, 21 Mar 2016, Oliver Neukum wrote: > > > > > On Mon, 2016-03-21 at 10:57 -0400, Alan Stern wrote: > > > > > > > One possible solution is to export a sysfs parameter to prevent > > > > statistics collection (or more generally, to change the interval at > > > > which it occurs). > > > > > > Surely, not performing a task can hardly be beaten in terms of power > > > consumption. That is not meant to be flippant, but I think these > > > issues are orthogonal. The question of how much to do doesn't > > > solve the question of doing efficiently what we do. > > > > In other words, what's the best way to collect the statistics without > > interfering with runtime PM, right? > > Yes. > > > If the device is suspended, presumably we know there's nothing to > > collect -- especially if we already collected the statistics at the > > time the device got suspended. Hence my suggestion to avoid querying > > the device while it is suspended. > > That is perfectly alright if we just collect statistics. > As a generic mechanism it is bad. Think about the polling > for media detection. True. Here I'm talking specifically about collecting statistics. Media detection has its own requirements. > > But this leaves open the issue that querying the device too often will > > prevent it from going into autosuspend. It seems to me that the best > > way to deal with this is to make sure that the autosuspend timeout is > > shorter than the interal between queries, not to make the querying > > conditional on !runtime_auto. > [..] > > > If we know when the next activity will come, why not pass this > > > information down? > > We have an autosuspend timeout because we think that IO, if it will > come at all, is likeliest to come soon. If, however, the IO is > periodic that heuristics is false. > To save most power the driver must either decide that the interval > is too short or suspend immediately. So if we are lucky enough > to have the frequency in the kernel, we should use that information. The autosuspend timeout is set by userspace. The kernel may assign a default value, but the user can always override it. Given this, I don't see how the kernel can use frequency information (and I'm not sure where that information would come from in the first place). 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] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM
On Mon, 21 Mar 2016 woojung@microchip.com wrote: > > But this leaves open the issue that querying the device too often will > > prevent it from going into autosuspend. It seems to me that the best > > way to deal with this is to make sure that the autosuspend timeout is > > shorter than the interal between queries, not to make the querying > > conditional on !runtime_auto. > > Short autosuspend timeout can affect performance. For instance our > experiments showed that > shorter than 10sec timeout made Ethernet performance degrade because of > wakeup delays. > So, just putting shorter timeout may have some side effects. Sure. This just means that you need a long statistics interval -- longer than the autosuspend timeout. That's why I suggested making the interval adjustable. Alternatively, you could automatically set the statistics interval to the autosuspend timeout (or 0 if autosuspend is disabled) plus some fixed value. 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] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM
On Mon, 2016-03-21 at 14:24 -0400, Alan Stern wrote: > On Mon, 21 Mar 2016, Oliver Neukum wrote: > > > On Mon, 2016-03-21 at 10:57 -0400, Alan Stern wrote: > > > > > One possible solution is to export a sysfs parameter to prevent > > > statistics collection (or more generally, to change the interval at > > > which it occurs). > > > > Surely, not performing a task can hardly be beaten in terms of power > > consumption. That is not meant to be flippant, but I think these > > issues are orthogonal. The question of how much to do doesn't > > solve the question of doing efficiently what we do. > > In other words, what's the best way to collect the statistics without > interfering with runtime PM, right? Yes. > If the device is suspended, presumably we know there's nothing to > collect -- especially if we already collected the statistics at the > time the device got suspended. Hence my suggestion to avoid querying > the device while it is suspended. That is perfectly alright if we just collect statistics. As a generic mechanism it is bad. Think about the polling for media detection. > But this leaves open the issue that querying the device too often will > prevent it from going into autosuspend. It seems to me that the best > way to deal with this is to make sure that the autosuspend timeout is > shorter than the interal between queries, not to make the querying > conditional on !runtime_auto. [..] > > If we know when the next activity will come, why not pass this > > information down? We have an autosuspend timeout because we think that IO, if it will come at all, is likeliest to come soon. If, however, the IO is periodic that heuristics is false. To save most power the driver must either decide that the interval is too short or suspend immediately. So if we are lucky enough to have the frequency in the kernel, we should use that information. 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: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM
> But this leaves open the issue that querying the device too often will > prevent it from going into autosuspend. It seems to me that the best > way to deal with this is to make sure that the autosuspend timeout is > shorter than the interal between queries, not to make the querying > conditional on !runtime_auto. Short autosuspend timeout can affect performance. For instance our experiments showed that shorter than 10sec timeout made Ethernet performance degrade because of wakeup delays. So, just putting shorter timeout may have some side effects. Woojung -- 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] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM
On Mon, 21 Mar 2016, Oliver Neukum wrote: > On Mon, 2016-03-21 at 10:57 -0400, Alan Stern wrote: > > > One possible solution is to export a sysfs parameter to prevent > > statistics collection (or more generally, to change the interval at > > which it occurs). > > Surely, not performing a task can hardly be beaten in terms of power > consumption. That is not meant to be flippant, but I think these > issues are orthogonal. The question of how much to do doesn't > solve the question of doing efficiently what we do. In other words, what's the best way to collect the statistics without interfering with runtime PM, right? If the device is suspended, presumably we know there's nothing to collect -- especially if we already collected the statistics at the time the device got suspended. Hence my suggestion to avoid querying the device while it is suspended. But this leaves open the issue that querying the device too often will prevent it from going into autosuspend. It seems to me that the best way to deal with this is to make sure that the autosuspend timeout is shorter than the interal between queries, not to make the querying conditional on !runtime_auto. > > But checking the runtime_auto flag is probably not a great idea. Even > > if it isn't set, collecting statistics is likely to wait up a device > > that otherwise would have remained suspended. > > > > Perhaps the best solution is to collect the statistics only when the > > device is not suspended or is about to suspend. > > If we know when the next activity will come, why not pass this > information down? I don't follow. 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] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM
On Mon, 2016-03-21 at 10:57 -0400, Alan Stern wrote: > One possible solution is to export a sysfs parameter to prevent > statistics collection (or more generally, to change the interval at > which it occurs). Surely, not performing a task can hardly be beaten in terms of power consumption. That is not meant to be flippant, but I think these issues are orthogonal. The question of how much to do doesn't solve the question of doing efficiently what we do. > But checking the runtime_auto flag is probably not a great idea. Even > if it isn't set, collecting statistics is likely to wait up a device > that otherwise would have remained suspended. > > Perhaps the best solution is to collect the statistics only when the > device is not suspended or is about to suspend. If we know when the next activity will come, why not pass this information down? 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 v8 1/5] leds: core: add generic support for RGB LED's
Add generic support for RGB LED's. Basic idea is to use enum led_brightness also for the hue and saturation color components.This allows to implement the color extension w/o changes to struct led_classdev. Select LEDS_CLASS_RGB to enable building drivers using the RGB extension. Flag LED_SET_HUE_SAT allows to specify that hue / saturation should be overridden even if the provided values are zero. Some examples for writing values to /sys/class/leds//brightness: (now also hex notation can be used) 255 -> set full brightness and keep existing color if set 0 -> switch LED off but keep existing color so that it can be restored if the LED is switched on again later 0x100 -> switch LED off and set also hue and saturation to 0 0x00 -> set full brightness, full saturation and set hue to 0 (red) Signed-off-by: Heiner Kallweit --- v2: - move extension-specific code into a separate source file and introduce config symbol LEDS_HSV for it - create separate patch for the extension to sysfs - use variable name led_cdev as in the rest if the core - rename to_hsv to led_validate_brightness - rename LED_BRIGHTNESS_SET_COLOR to LED_SET_HSV - introduce helper is_off for checking whether V part of a HSV value is zero v3: - change Kconfig to use depend instead of select, add help message, and change config symbol to LEDS_COLOR - change LED core object file name in Makefile - rename flag LED_SET_HSV to LED_SET_COLOR - rename is_off to led_is_off - rename led_validate-brightness to led_confine_brightness - rename variable in led_confine_brightness - add dummy enum led_brightness value to enforce 32bit enum - rename led-hsv-core.c to led-color-core.c - move check of provided brightness value to led_confine_brightness v4: - change config symbol name to LEDS_RGB - change name of new file to led-rgb-core.c - factor out part of led_confine_brightness - change led_is_off to __is_set_brightness - in led_set_software_blink pass led_cdev->max_brightness instead of LED_FULL - rename LED_SET_COLOR to LED_SET_HUE_SAT v5: - change "RGB Color LED" to "RGB LED" in Kconfig - move definition of LED_HUE_SAT_MASK to drivers/leds/leds.h - rename LED_DEV_CAP_HSV to LED_DEV_CAP_RGB v6: - no change v7: - removed "Color" from RGB Color LED in commit message and title - don't include linux/kernel.h in led-rgb-core.c - keep definition of LED_DEV_CAP_[] flags together v8: - rename LEDS_RGB to LEDS_CLASS_RGB --- drivers/leds/Kconfig| 12 drivers/leds/Makefile | 4 +++- drivers/leds/led-class.c| 2 +- drivers/leds/led-core.c | 16 +--- drivers/leds/led-rgb-core.c | 39 +++ drivers/leds/leds.h | 18 ++ include/linux/leds.h| 11 ++- 7 files changed, 92 insertions(+), 10 deletions(-) create mode 100644 drivers/leds/led-rgb-core.c diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index 7f940c2..20697a2 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -19,6 +19,14 @@ config LEDS_CLASS This option enables the led sysfs class in /sys/class/leds. You'll need this to do anything useful with LEDs. If unsure, say N. +config LEDS_CLASS_RGB + bool "RGB LED Class Support" + depends on LEDS_CLASS + help + This option enables support for RGB LED devices. + Sysfs attribute brightness is interpreted as a HSV color value. + For details see Documentation/leds/leds-class.txt. + config LEDS_CLASS_FLASH tristate "LED Flash Class Support" depends on LEDS_CLASS @@ -29,6 +37,10 @@ config LEDS_CLASS_FLASH for the flash related features of a LED device. It can be built as a module. +if LEDS_CLASS_RGB +comment "RGB LED drivers" +endif # LEDS_CLASS_RGB + comment "LED drivers" config LEDS_88PM860X diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index e9d5309..7f1b2be 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -1,6 +1,8 @@ # LED Core -obj-$(CONFIG_NEW_LEDS) += led-core.o +obj-$(CONFIG_NEW_LEDS) += led-core-objs.o +led-core-objs-y:= led-core.o +led-core-objs-$(CONFIG_LEDS_CLASS_RGB) += led-rgb-core.o obj-$(CONFIG_LEDS_CLASS) += led-class.o obj-$(CONFIG_LEDS_CLASS_FLASH) += led-class-flash.o obj-$(CONFIG_LEDS_TRIGGERS)+= led-triggers.o diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c index aa84e5b..007a5b3 100644 --- a/drivers/leds/led-class.c +++ b/drivers/leds/led-class.c @@ -53,7 +53,7 @@ static ssize_t brightness_store(struct device *dev, if (ret) goto unlock; - if (state == LED_OFF) + if (!__is_brightness_set(state)) led_trigger_remove(led_cdev); led_set_brightness(led_cdev, state); diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c index 3495d5d..e75b0c8 100644 --- a/drivers/leds/l
[PATCH v3] usb: phy: mxs: Add DT bindings to configure TX settings
The TX settings can be calibrated for particular hardware. The
phy is reset by Linux, so this cannot be handled by the bootloader.
The TRM mentions that the maximum resistance should be used for the
DN/DP calibration in order to pass USB certification.
The values for the TX registers are poorly described in the TRM.
The meanings of the register values were taken from another
Freescale-provided document:
https://community.freescale.com/message/566147#comment-566912
Signed-off-by: Jaret Cantu
---
v3. Added unit suffix (-ohms) to tx-cal-45-d*
v2. Copying devicetree list
Removed prettifying extra whitespace
Removed unnecessary register rewrite on resume
Use min and max constants for clarity
.../devicetree/bindings/phy/mxs-usb-phy.txt| 10
drivers/usb/phy/phy-mxs-usb.c | 58
2 files changed, 68 insertions(+)
diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
index 379b84a..1d25b04 100644
--- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
+++ b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
@@ -12,6 +12,16 @@ Required properties:
- interrupts: Should contain phy interrupt
- fsl,anatop: phandle for anatop register, it is only for imx6 SoC series
+Optional properties:
+- fsl,tx-cal-45-dn-ohms: Integer [30-55]. Resistance (in ohms) of switchable
+ high-speed trimming resistor connected in parallel with the 45 ohm resistor
+ that terminates the DN output signal. Default: 45
+- fsl,tx-cal-45-dp-ohms: Integer [30-55]. Resistance (in ohms) of switchable
+ high-speed trimming resistor connected in parallel with the 45 ohm resistor
+ that terminates the DP output signal. Default: 45
+- fsl,tx-d-cal: Integer [79-119]. Current trimming value (as a percentage) of
+ the 17.78mA TX reference current. Default: 100
+
Example:
usbphy1: usbphy@020c9000 {
compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index 00bfea0..6c6b12c 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -27,6 +27,7 @@
#define DRIVER_NAME "mxs_phy"
#define HW_USBPHY_PWD 0x00
+#define HW_USBPHY_TX 0x10
#define HW_USBPHY_CTRL 0x30
#define HW_USBPHY_CTRL_SET 0x34
#define HW_USBPHY_CTRL_CLR 0x38
@@ -38,6 +39,10 @@
#define HW_USBPHY_IP_SET 0x94
#define HW_USBPHY_IP_CLR 0x98
+#define GM_USBPHY_TX_TXCAL45DP(x)(((x) & 0xf) << 16)
+#define GM_USBPHY_TX_TXCAL45DN(x)(((x) & 0xf) << 8)
+#define GM_USBPHY_TX_D_CAL(x)(((x) & 0xf) << 0)
+
#define BM_USBPHY_CTRL_SFTRST BIT(31)
#define BM_USBPHY_CTRL_CLKGATE BIT(30)
#define BM_USBPHY_CTRL_OTG_ID_VALUEBIT(27)
@@ -115,6 +120,12 @@
*/
#define MXS_PHY_NEED_IP_FIXBIT(3)
+/* Minimum and maximum values for device tree entries */
+#define MXS_PHY_TX_CAL45_MIN 30
+#define MXS_PHY_TX_CAL45_MAX 55
+#define MXS_PHY_TX_D_CAL_MIN 79
+#define MXS_PHY_TX_D_CAL_MAX 119
+
struct mxs_phy_data {
unsigned int flags;
};
@@ -164,6 +175,8 @@ struct mxs_phy {
const struct mxs_phy_data *data;
struct regmap *regmap_anatop;
int port_id;
+ u32 tx_reg_set;
+ u32 tx_reg_mask;
};
static inline bool is_imx6q_phy(struct mxs_phy *mxs_phy)
@@ -185,6 +198,20 @@ static void mxs_phy_clock_switch_delay(void)
usleep_range(300, 400);
}
+static void mxs_phy_tx_init(struct mxs_phy *mxs_phy)
+{
+ void __iomem *base = mxs_phy->phy.io_priv;
+ u32 phytx;
+
+ /* Update TX register if there is anything to write */
+ if (mxs_phy->tx_reg_mask) {
+ phytx = readl(base + HW_USBPHY_TX);
+ phytx &= ~mxs_phy->tx_reg_mask;
+ phytx |= mxs_phy->tx_reg_set;
+ writel(phytx, base + HW_USBPHY_TX);
+ }
+}
+
static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
{
int ret;
@@ -214,6 +241,8 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
if (mxs_phy->data->flags & MXS_PHY_NEED_IP_FIX)
writel(BM_USBPHY_IP_FIX, base + HW_USBPHY_IP_SET);
+ mxs_phy_tx_init(mxs_phy);
+
return 0;
}
@@ -459,6 +488,7 @@ static int mxs_phy_probe(struct platform_device *pdev)
int ret;
const struct of_device_id *of_id;
struct device_node *np = pdev->dev.of_node;
+ u32 val;
of_id = of_match_device(mxs_phy_dt_ids, &pdev->dev);
if (!of_id)
@@ -491,6 +521,34 @@ static int mxs_phy_probe(struct platform_device *pdev)
}
}
+ /* Precompute which bits of the TX register are to be updated, if any */
+ if (
Re: [PATCH] usb: Wait before re-enabling a port that has been disabled due to EMI
Consider also a couple of very common non-telecom EMI sources: vehicle engines and lifts starting up. Both of them have a typical duration which I suppose is of 1, 2 or at most 3 seconds (although I do not have exact figures or references to them). What would really help here is an ad-hoc study on typical duration of EMI inside buildings... In the meanwhile, I believe 1000 milliseconds (plus configurability) to be safe. Regards, Guido Il 21 marzo 2016 16:42:14 CET, Guido Trentalancia ha scritto: >Hello Alan, > >thanks for getting back on this... > >Il 21 marzo 2016 16:01:17 CET, Alan Stern >ha scritto: >>On Sun, 20 Mar 2016, Guido Trentalancia wrote: >> >>> Hello. >>> >>> Considering that EM interference can last for a while (generally up >>to >>> seconds), I am currently testing the following patch for module >>usbcore >>> so that it is possible to specify an amount of time to wait before >>> trying to re-enable a port that has been previously disabled by the >>hub >>> (supposedly because of EM interference). >>> >>> Hopefully, setting the right positive value (for example, 2000 >>> milliseconds) would help overcome situations such as the following: >>> >>> [ 1295.575679] usb 6-2: FTDI USB Serial Device converter now >attached >>> to ttyUSB1 >>> [ 1302.204285] usb usb6-port2: disabled by hub (EMI?), >re-enabling... >>> >>> *** NOTE: EMI is probably still present here *** >>> >>> [ 1303.205202] usb 6-2: USB disconnect, device number 6 >>> [ 1303.205907] ftdi_sio ttyUSB1: FTDI USB Serial Device converter >now >>> disconnected from ttyUSB1 >>> [ 1303.205950] ftdi_sio 6-2:1.0: device disconnected >>> [ 1303.414089] usb 6-2: new full-speed USB device number 7 using >>> uhci_hcd >>> [ 1303.526226] usb 6-2: device descriptor read/64, error -71 >>> [ 1303.894228] usb 6-2: new full-speed USB device number 8 using >>> uhci_hcd >>> [ 1304.006185] usb 6-2: device descriptor read/64, error -71 >>> [ 1304.219089] usb 6-2: device descriptor read/64, error -71 >>> [ 1304.422107] usb 6-2: new full-speed USB device number 9 using >>> uhci_hcd >>> [ 1304.640020] usb 6-2: device not accepting address 9, error -71 >>> [ 1304.752024] usb 6-2: new full-speed USB device number 10 using >>> uhci_hcd >>> [ 1305.160020] usb 6-2: device not accepting address 10, error -71 >>> [ 1305.160038] hub 6-0:1.0: unable to enumerate USB device on port 2 >>> >>> *** NOTE: Device is permanently disabled at this point *** >>> >>> I don't know whether my analysis is correct (and therefore the >>proposed >>> solution appropriate), as reproducing the problem is very >>difficult... >> >>Would it be better instead to provide a mechanism for re-enabling the >>port after it has been "permanently" disabled? After all, you don't >>know how long the EMI is going to last, and so the kernel has to give >>up at some point. This means increasing the delay will make problems >>more infrequent but won't eliminate them completely. >> >>Alan Stern > >It should be automatic (e.g. for servers). The manual solution doesn't >need a patch: it's hardware, i. >e. unplug then plug-again ;) > >In some cases it is possibile to predict how long interference lasts >(for example, the source is co-located and known). This is not that >uncommon, I suppose... > >When the source is not known then only statistical assumptions can be >made regarding the duration of the interference: a value between 1000 >and 2000 would probably suit most cases and also seems in line with >other hard-coded waiting times in the driver. > >As you correctly noted, further increasing the value will make problems >more infrequent but not necessarily would eliminate them completely >(consider, for example, a very long USB cable picking up interference >from a long wireless phone call lasting minutes, although even in such >case there will usually be fades of the order of several milliseconds >or 1-2 seconds). > >There is no "one fits all" solution, I suppose: only statistical >assumptions and optimal solutions based upon them (and/or full >configurability such as in a kernel parameter). > >Consider at the moment, we have least reliability against such EMI >issues, as they all have finite duration. > >Regards, > >Guido >-- >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: "reset full-speed USB device number 6 using ehci-pci" with Dell Inspiron 15R 5537
On Mon, 21 Mar 2016, Oliver Neukum wrote:
> On Fri, 2016-03-18 at 12:36 -0400, Alan Stern wrote:
>
> > @@ -1584,10 +1581,8 @@ static int hid_resume(struct usb_interfa
> > static int hid_reset_resume(struct usb_interface *intf)
> > {
> > struct hid_device *hid = usb_get_intfdata(intf);
> > - struct usbhid_device *usbhid = hid->driver_data;
> > int status;
> >
> > - clear_bit(HID_SUSPENDED, &usbhid->iofl);
> > status = hid_post_reset(intf);
> > if (status >= 0 && hid->driver && hid->driver->reset_resume) {
> > int ret = hid->driver->reset_resume(hid);
>
> Almost. In case of reset_resume() it makes no sense to still
> clear a halt or execute a reset that had been ordered. The
> corresponding flags should be cleared.
hid_post_reset() already clears the HID_RESET_PENDING flag. It also
needs to clear the HID_CLEAR_HALT flag; I'll add that in.
> And now that I think of it, if while restarting IO an
> old error condition is detected shouldn't the restart wait for
> the error to be handled?
Yes; we should avoid starting I/O if a reset is pending and we should
avoid starting the interrupt-IN URB if a clear-halt is pending. How
does this patch look?
Alan Stern
Index: usb-4.4/drivers/hid/usbhid/hid-core.c
===
--- usb-4.4.orig/drivers/hid/usbhid/hid-core.c
+++ usb-4.4/drivers/hid/usbhid/hid-core.c
@@ -951,14 +951,6 @@ static int usbhid_output_report(struct h
return ret;
}
-static void usbhid_restart_queues(struct usbhid_device *usbhid)
-{
- if (usbhid->urbout && !test_bit(HID_OUT_RUNNING, &usbhid->iofl))
- usbhid_restart_out_queue(usbhid);
- if (!test_bit(HID_CTRL_RUNNING, &usbhid->iofl))
- usbhid_restart_ctrl_queue(usbhid);
-}
-
static void hid_free_buffers(struct usb_device *dev, struct hid_device *hid)
{
struct usbhid_device *usbhid = hid->driver_data;
@@ -1404,6 +1396,37 @@ static void hid_cease_io(struct usbhid_d
usb_kill_urb(usbhid->urbout);
}
+static void hid_restart_io(struct hid_device *hid)
+{
+ struct usbhid_device *usbhid = hid->driver_data;
+ int clear_halt = test_bit(HID_CLEAR_HALT, &usbhid->iofl);
+ int reset_pending = test_bit(HID_RESET_PENDING, &usbhid->iofl);
+
+ spin_lock_irq(&usbhid->lock);
+ clear_bit(HID_SUSPENDED, &usbhid->iofl);
+ usbhid_mark_busy(usbhid);
+
+ if (clear_halt || reset_pending)
+ schedule_work(&usbhid->reset_work);
+ usbhid->retry_delay = 0;
+ spin_unlock_irq(&usbhid->lock);
+
+ if (reset_pending || !test_bit(HID_STARTED, &usbhid->iofl))
+ return;
+
+ if (!clear_halt) {
+ if (hid_start_in(hid) < 0)
+ hid_io_error(hid);
+ }
+
+ spin_lock_irq(&usbhid->lock);
+ if (usbhid->urbout && !test_bit(HID_OUT_RUNNING, &usbhid->iofl))
+ usbhid_restart_out_queue(usbhid);
+ if (!test_bit(HID_CTRL_RUNNING, &usbhid->iofl))
+ usbhid_restart_ctrl_queue(usbhid);
+ spin_unlock_irq(&usbhid->lock);
+}
+
/* Treat USB reset pretty much the same as suspend/resume */
static int hid_pre_reset(struct usb_interface *intf)
{
@@ -1453,14 +1476,14 @@ static int hid_post_reset(struct usb_int
return 1;
}
+ /* No need to do another reset or clear a halted endpoint */
spin_lock_irq(&usbhid->lock);
clear_bit(HID_RESET_PENDING, &usbhid->iofl);
+ clear_bit(HID_CLEAR_HALT, &usbhid->iofl);
spin_unlock_irq(&usbhid->lock);
hid_set_idle(dev, intf->cur_altsetting->desc.bInterfaceNumber, 0, 0);
- status = hid_start_in(hid);
- if (status < 0)
- hid_io_error(hid);
- usbhid_restart_queues(usbhid);
+
+ hid_restart_io(hid);
return 0;
}
@@ -1483,25 +1506,9 @@ void usbhid_put_power(struct hid_device
#ifdef CONFIG_PM
static int hid_resume_common(struct hid_device *hid, bool driver_suspended)
{
- struct usbhid_device *usbhid = hid->driver_data;
- int status;
-
- spin_lock_irq(&usbhid->lock);
- clear_bit(HID_SUSPENDED, &usbhid->iofl);
- usbhid_mark_busy(usbhid);
-
- if (test_bit(HID_CLEAR_HALT, &usbhid->iofl) ||
- test_bit(HID_RESET_PENDING, &usbhid->iofl))
- schedule_work(&usbhid->reset_work);
- usbhid->retry_delay = 0;
-
- usbhid_restart_queues(usbhid);
- spin_unlock_irq(&usbhid->lock);
-
- status = hid_start_in(hid);
- if (status < 0)
- hid_io_error(hid);
+ int status = 0;
+ hid_restart_io(hid);
if (driver_suspended && hid->driver && hid->driver->resume)
status = hid->driver->resume(hid);
return status;
@@ -1570,12 +1577,8 @@ static int hid_suspend(struct usb_interf
static int hid_resume(struct usb_interface *intf)
{
struct hid_device *hid = usb_get_intfdata (intf);
- str
Re: [PATCH] usb: Wait before re-enabling a port that has been disabled due to EMI
Hello Alan, thanks for getting back on this... Il 21 marzo 2016 16:01:17 CET, Alan Stern ha scritto: >On Sun, 20 Mar 2016, Guido Trentalancia wrote: > >> Hello. >> >> Considering that EM interference can last for a while (generally up >to >> seconds), I am currently testing the following patch for module >usbcore >> so that it is possible to specify an amount of time to wait before >> trying to re-enable a port that has been previously disabled by the >hub >> (supposedly because of EM interference). >> >> Hopefully, setting the right positive value (for example, 2000 >> milliseconds) would help overcome situations such as the following: >> >> [ 1295.575679] usb 6-2: FTDI USB Serial Device converter now attached >> to ttyUSB1 >> [ 1302.204285] usb usb6-port2: disabled by hub (EMI?), re-enabling... >> >> *** NOTE: EMI is probably still present here *** >> >> [ 1303.205202] usb 6-2: USB disconnect, device number 6 >> [ 1303.205907] ftdi_sio ttyUSB1: FTDI USB Serial Device converter now >> disconnected from ttyUSB1 >> [ 1303.205950] ftdi_sio 6-2:1.0: device disconnected >> [ 1303.414089] usb 6-2: new full-speed USB device number 7 using >> uhci_hcd >> [ 1303.526226] usb 6-2: device descriptor read/64, error -71 >> [ 1303.894228] usb 6-2: new full-speed USB device number 8 using >> uhci_hcd >> [ 1304.006185] usb 6-2: device descriptor read/64, error -71 >> [ 1304.219089] usb 6-2: device descriptor read/64, error -71 >> [ 1304.422107] usb 6-2: new full-speed USB device number 9 using >> uhci_hcd >> [ 1304.640020] usb 6-2: device not accepting address 9, error -71 >> [ 1304.752024] usb 6-2: new full-speed USB device number 10 using >> uhci_hcd >> [ 1305.160020] usb 6-2: device not accepting address 10, error -71 >> [ 1305.160038] hub 6-0:1.0: unable to enumerate USB device on port 2 >> >> *** NOTE: Device is permanently disabled at this point *** >> >> I don't know whether my analysis is correct (and therefore the >proposed >> solution appropriate), as reproducing the problem is very >difficult... > >Would it be better instead to provide a mechanism for re-enabling the >port after it has been "permanently" disabled? After all, you don't >know how long the EMI is going to last, and so the kernel has to give >up at some point. This means increasing the delay will make problems >more infrequent but won't eliminate them completely. > >Alan Stern It should be automatic (e.g. for servers). The manual solution doesn't need a patch: it's hardware, i. e. unplug then plug-again ;) In some cases it is possibile to predict how long interference lasts (for example, the source is co-located and known). This is not that uncommon, I suppose... When the source is not known then only statistical assumptions can be made regarding the duration of the interference: a value between 1000 and 2000 would probably suit most cases and also seems in line with other hard-coded waiting times in the driver. As you correctly noted, further increasing the value will make problems more infrequent but not necessarily would eliminate them completely (consider, for example, a very long USB cable picking up interference from a long wireless phone call lasting minutes, although even in such case there will usually be fades of the order of several milliseconds or 1-2 seconds). There is no "one fits all" solution, I suppose: only statistical assumptions and optimal solutions based upon them (and/or full configurability such as in a kernel parameter). Consider at the moment, we have least reliability against such EMI issues, as they all have finite duration. Regards, Guido -- 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: Fix incomplete PM resume operation due to XHCI commmand timeout
On Mon, 21 Mar 2016, Rajesh Bhagat wrote: > I agree to your point. Actually the intent was to check the return status of > reset_device which > is currently being ignored. I encountered the reset_device failure case in > resume operation (STR) > which is increasing the time of resume and causing unexpected crashes if > return value is not checked. > > Do you agree it should be checked here? If yes, I can rework this patch. I don't know. You should ask the xHCI maintainter, Mathias. 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] usb: Wait before re-enabling a port that has been disabled due to EMI
On Sun, 20 Mar 2016, Guido Trentalancia wrote: > Hello. > > Considering that EM interference can last for a while (generally up to > seconds), I am currently testing the following patch for module usbcore > so that it is possible to specify an amount of time to wait before > trying to re-enable a port that has been previously disabled by the hub > (supposedly because of EM interference). > > Hopefully, setting the right positive value (for example, 2000 > milliseconds) would help overcome situations such as the following: > > [ 1295.575679] usb 6-2: FTDI USB Serial Device converter now attached > to ttyUSB1 > [ 1302.204285] usb usb6-port2: disabled by hub (EMI?), re-enabling... > > *** NOTE: EMI is probably still present here *** > > [ 1303.205202] usb 6-2: USB disconnect, device number 6 > [ 1303.205907] ftdi_sio ttyUSB1: FTDI USB Serial Device converter now > disconnected from ttyUSB1 > [ 1303.205950] ftdi_sio 6-2:1.0: device disconnected > [ 1303.414089] usb 6-2: new full-speed USB device number 7 using > uhci_hcd > [ 1303.526226] usb 6-2: device descriptor read/64, error -71 > [ 1303.894228] usb 6-2: new full-speed USB device number 8 using > uhci_hcd > [ 1304.006185] usb 6-2: device descriptor read/64, error -71 > [ 1304.219089] usb 6-2: device descriptor read/64, error -71 > [ 1304.422107] usb 6-2: new full-speed USB device number 9 using > uhci_hcd > [ 1304.640020] usb 6-2: device not accepting address 9, error -71 > [ 1304.752024] usb 6-2: new full-speed USB device number 10 using > uhci_hcd > [ 1305.160020] usb 6-2: device not accepting address 10, error -71 > [ 1305.160038] hub 6-0:1.0: unable to enumerate USB device on port 2 > > *** NOTE: Device is permanently disabled at this point *** > > I don't know whether my analysis is correct (and therefore the proposed > solution appropriate), as reproducing the problem is very difficult... Would it be better instead to provide a mechanism for re-enabling the port after it has been "permanently" disabled? After all, you don't know how long the EMI is going to last, and so the kernel has to give up at some point. This means increasing the delay will make problems more infrequent but won't eliminate them completely. 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] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM
> -Original Message-
> From: Guenter Roeck [mailto:li...@roeck-us.net]
> Sent: Sunday, March 20, 2016 6:59 PM
> To: Geert Uytterhoeven; Woojung Huh - C21699; UNGLinuxDriver; David S.
> Miller
> Cc: Rafael J. Wysocki; net...@vger.kernel.org; linux-usb@vger.kernel.org;
> linux...@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef
> CONFIG_PM
>
> Not that it matters anymore since David reverted the original patch,
> but my reason for not sending a similar patch was that I wasn't sure
> if .runtime_auto should be accessed from drivers in the first place,
> or if there is some logical problem with the code.
>
Because driver can enable/disable autosuspend by usb_enable_autosuspend() and
usb_disable_autosuspend(), it would be nice to have helper to find out
autosuspend status.
The code path was to utilize autosuspend power saving, when it is enabled.
Woojung
N�r��yb�X��ǧv�^�){.n�+{��^n�r���z���h�&�� �G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�
Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM
On Mon, 21 Mar 2016, Oliver Neukum wrote:
> On Sun, 2016-03-20 at 11:43 +0100, Geert Uytterhoeven wrote:
> > If CONFIG_PM=n:
> >
> > drivers/net/usb/lan78xx.c: In function ‘lan78xx_get_stats64’:
> > drivers/net/usb/lan78xx.c:3274: error: ‘struct dev_pm_info’ has no
> > member named ‘runtime_auto’
> >
> > If PM is disabled, the runtime_auto flag is not available, but auto
> > suspend is not enabled anyway. Hence protect the check for
> > runtime_auto
> > by #ifdef CONFIG_PM to fix this.
> >
> > Fixes: a59f8c5b048dc938 ("lan78xx: add ndo_get_stats64")
> > Reported-by: Guenter Roeck
> > Signed-off-by: Geert Uytterhoeven
> > ---
> > Alternatively, we can add a dev_pm_runtime_auto_is_enabled() wrapper
> > to
> > include/linux/pm.h, which always return false if CONFIG_PM is
> > disabled.
>
> That is what we do for almost everything else in include/pm_runtime.h
> So it is the best solution to add the stub.
Guenter's question about whether drivers should try to access
runtime_auto in the first place was a good one. A similar problem
arises in the block layer: When a block device has removable media, the
block layer probes at 1-second intervals looking for media changes.
This defeats autosuspend in the same way as we're talking about here.
One possible solution is to export a sysfs parameter to prevent
statistics collection (or more generally, to change the interval at
which it occurs).
But checking the runtime_auto flag is probably not a great idea. Even
if it isn't set, collecting statistics is likely to wait up a device
that otherwise would have remained suspended.
Perhaps the best solution is to collect the statistics only when the
device is not suspended or is about to suspend.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] digi_acceleport: do sanity checking for the number of ports
The driver can be crashed with devices that expose crafted
descriptors with too few endpoints.
See:
http://seclists.org/bugtraq/2016/Mar/61
Signed-off-by: Oliver Neukum
v1 - added sanity checks
v2 - moved them to probe() to fix problems Johan pointed out
---
drivers/usb/serial/digi_acceleport.c | 24 +++-
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/serial/digi_acceleport.c
b/drivers/usb/serial/digi_acceleport.c
index 12b0e67..dab1dcf 100644
--- a/drivers/usb/serial/digi_acceleport.c
+++ b/drivers/usb/serial/digi_acceleport.c
@@ -1252,7 +1252,8 @@ static int digi_port_init(struct usb_serial_port *port,
unsigned port_num)
static int digi_startup(struct usb_serial *serial)
{
struct digi_serial *serial_priv;
- int ret;
+ int ret = -ENODEV;
+ int i;
serial_priv = kzalloc(sizeof(*serial_priv), GFP_KERNEL);
if (!serial_priv)
@@ -1260,18 +1261,31 @@ static int digi_startup(struct usb_serial *serial)
spin_lock_init(&serial_priv->ds_serial_lock);
serial_priv->ds_oob_port_num = serial->type->num_ports;
+
+ /* Check whether the expected number of ports matches the device */
+ if (serial->num_ports < serial_priv->ds_oob_port_num)
+ goto error;
+ /* all features must be present */
+ for (i = 0; i < serial->type->num_ports + 1 ; i++) {
+ if (!serial->port[i]->read_urb)
+ goto error;
+ if (!serial->port[i]->write_urb)
+ goto error;
+ }
+
serial_priv->ds_oob_port = serial->port[serial_priv->ds_oob_port_num];
ret = digi_port_init(serial_priv->ds_oob_port,
serial_priv->ds_oob_port_num);
- if (ret) {
- kfree(serial_priv);
- return ret;
- }
+ if (ret)
+ goto error;
usb_set_serial_data(serial, serial_priv);
return 0;
+error:
+ kfree(serial_priv);
+ return ret;
}
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM
> -Original Message- > From: Oliver Neukum [mailto:oneu...@suse.com] > Sent: Monday, March 21, 2016 4:36 AM > To: Geert Uytterhoeven > Cc: Woojung Huh - C21699; UNGLinuxDriver; David S. Miller; Guenter Roeck; > Rafael J. Wysocki; net...@vger.kernel.org; linux-usb@vger.kernel.org; > linux...@vger.kernel.org; linux-ker...@vger.kernel.org > Subject: Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef > CONFIG_PM Thanks for all comments. Will look into it and submit new patch. - Woojung
Re: [PATCH] uas: Limit qdepth at the scsi-host level
On Sat, Mar 19, 2016 at 10:06:16AM -0700, James Bottomley wrote: > Help me understand this bug a bit more. Are you saying that the commit > you identify is causing the block layer to queue more commands than > you've set the per-lun limit to? In which case we have a serious > problem for more than just UAS. Or are you saying that UAS always had > a global command limit, but it just didn't get set correctly; however, > it mostly worked until the above commit exposed the problem? I did mishandle uas in my patch to make the host wide tags the default and didn't notice that uas set a different limit for the host wide tag limit and ->can_queue. With the old code we'd apply the lower of the two limits, which was the host wide tag limit, but my patch removed that one and now purely relies on can_queue. Now most uas devices only have a single lun, and the per-lun limit was set to what the global limit should be as well, so most users didn't really notice this. -- 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: Limit qdepth at the scsi-host level
On Sat, Mar 19, 2016 at 09:59:12AM +0100, Hans de Goede wrote:
> Commit 64d513ac31bd ("scsi: use host wide tags by default") causes
> the scsi-core to queue more cmnds then we can handle on devices with
> multiple LUNs, limit the qdepth at the scsi-host level instead of
> per slave to fix this.
>
> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1315013
> Cc: sta...@vger.kernel.org # 4.4.x and 4.5.x
> Signed-off-by: Hans de Goede
Looks good,
Reviewed-by: Christoph Hellwig
--
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] option.c: Added support for Gemalto's Cinterion PH8 and AHxx products
Added support for Gemalto's Cinterion PH8 and AHxx products
with 2 RmNet Interfaces and products with 1 RmNet + 1 USB Audio interface.
The RmNet and USB Audio interfaces are blacklisted because they will be
handled by other drivers.
In addition some minor renaming and formatting.
Signed-off-by: Hans-Christoph Schemmel
---
drivers/usb/serial/option.c | 39 ---
1 file changed, 32 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index 348e198..b1e7820 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -375,18 +375,22 @@ static void option_instat_callback(struct urb *urb);
#define HAIER_PRODUCT_CE81B0x10f8
#define HAIER_PRODUCT_CE1000x2009
-/* Cinterion (formerly Siemens) products */
-#define SIEMENS_VENDOR_ID 0x0681
-#define CINTERION_VENDOR_ID0x1e2d
+/* Gemalto's Cinterion products (formerly Siemens)*/
+#define SIEMENS_VENDOR_ID 0x0681
+#define CINTERION_VENDOR_ID0x1e2d
#define CINTERION_PRODUCT_HC25_MDM 0x0047
-#define CINTERION_PRODUCT_HC25_MDMNET 0x0040
+#define CINTERION_PRODUCT_HC25_MDMNET 0x0040
#define CINTERION_PRODUCT_HC28_MDM 0x004C
-#define CINTERION_PRODUCT_HC28_MDMNET 0x004A /* same for HC28J */
+#define CINTERION_PRODUCT_HC28_MDMNET 0x004A /* same for HC28J */
#define CINTERION_PRODUCT_EU3_E0x0051
#define CINTERION_PRODUCT_EU3_P0x0052
#define CINTERION_PRODUCT_PH8 0x0053
#define CINTERION_PRODUCT_AHXX 0x0055
#define CINTERION_PRODUCT_PLXX 0x0060
+#define CINTERION_PRODUCT_PH8_2RMNET 0x0082
+#define CINTERION_PRODUCT_PH8_AUDIO0x0083
+#define CINTERION_PRODUCT_AHXX_2RMNET 0x0084
+#define CINTERION_PRODUCT_AHXX_AUDIO 0x0085
/* Olivetti products */
#define OLIVETTI_VENDOR_ID 0x0b3c
@@ -600,6 +604,18 @@ static const struct option_blacklist_info
net_intf6_blacklist = {
.reserved = BIT(6),
};
+static const struct option_blacklist_info cinterion_rmnet1_blacklist = {
+ .reserved = BIT(4) | BIT(5),
+};
+
+static const struct option_blacklist_info cinterion_rmnet2_blacklist = {
+ .reserved = BIT(4) | BIT(5) | BIT(6) | BIT(7),
+};
+
+static const struct option_blacklist_info cinterion_audio_blacklist = {
+ .reserved = BIT(4) | BIT(5) | BIT(6) | BIT(7) | BIT(8),
+};
+
static const struct option_blacklist_info zte_mf626_blacklist = {
.sendsetup = BIT(0) | BIT(1),
.reserved = BIT(4),
@@ -1708,8 +1724,17 @@ static const struct usb_device_id option_ids[] = {
{ USB_DEVICE(CINTERION_VENDOR_ID, CINTERION_PRODUCT_EU3_E) },
{ USB_DEVICE(CINTERION_VENDOR_ID, CINTERION_PRODUCT_EU3_P) },
{ USB_DEVICE(CINTERION_VENDOR_ID, CINTERION_PRODUCT_PH8),
- .driver_info = (kernel_ulong_t)&net_intf4_blacklist },
- { USB_DEVICE_INTERFACE_CLASS(CINTERION_VENDOR_ID,
CINTERION_PRODUCT_AHXX, 0xff) },
+ .driver_info = (kernel_ulong_t)&cinterion_rmnet1_blacklist },
+ { USB_DEVICE(CINTERION_VENDOR_ID, CINTERION_PRODUCT_AHXX),
+ .driver_info = (kernel_ulong_t)&cinterion_rmnet1_blacklist },
+ { USB_DEVICE(CINTERION_VENDOR_ID, CINTERION_PRODUCT_PH8_2RMNET),
+ .driver_info = (kernel_ulong_t)&cinterion_rmnet2_blacklist },
+ { USB_DEVICE(CINTERION_VENDOR_ID, CINTERION_PRODUCT_PH8_AUDIO),
+ .driver_info = (kernel_ulong_t)&cinterion_audio_blacklist },
+ { USB_DEVICE(CINTERION_VENDOR_ID, CINTERION_PRODUCT_AHXX_2RMNET),
+ .driver_info = (kernel_ulong_t)&cinterion_rmnet2_blacklist },
+ { USB_DEVICE(CINTERION_VENDOR_ID, CINTERION_PRODUCT_AHXX_AUDIO),
+ .driver_info = (kernel_ulong_t)&cinterion_audio_blacklist },
{ USB_DEVICE(CINTERION_VENDOR_ID, CINTERION_PRODUCT_PLXX),
.driver_info = (kernel_ulong_t)&net_intf4_blacklist },
{ USB_DEVICE(CINTERION_VENDOR_ID, CINTERION_PRODUCT_HC28_MDM) },
smime.p7s
Description: S/MIME cryptographic signature
Fwd: Re: [libusb] Potential xHCI regression upgrading kernel 3.19.0-60 to 4.2.0-30
Good day, I've noticed what I suspect to be a possible xHCI regression when upgrading from kernel 3.19.51 to 4.2.0-30 generic SMP x64 which causes various USB3.0 cameras to no longer stream data. It appears to be limited to Intel 9 Series on-board controllers and the occasional NEC PCIe USB3.0 host controller I come across. I've tested a slew of about a dozen different controllers so far, and these are the only ones that are problematic. I don't see any warnings/errors in the kernel log output via `dmesg', and I've sent off a very verbose run-time log to the libusb mailing list. Is there anything further I can do, save for a physical trace, to further diagnose this issue? Thank you. Matthew Giassa Forwarded Message Subject: Re: [libusb] Potential xHCI regression upgrading kernel 3.19.0-60 to 4.2.0-30 Date: Sat, 19 Mar 2016 10:25:11 -0700 From: Matthew Giassa To: libusb Developer Mailing List As a follow-up, I've pulled out a bunch of USB3.0 host controllers to test against, and seem to only have issues with some NEC and Intel controller. No issues: * Renesas PCIe controller (latest firmware flashed to chip). * Fresco PCIe controller. * Intel 7 Series on-board USB3.0 controller. Encountered issues as described in previous message: * Intel 9 Series on-board controller. * NEC USB3.0 PCIe controller. Matthew Giassa On 03/18/16 14:47, Matthew Giassa wrote: Good day, I have some Linux USB-related issues with some USB3.0 cameras that previously worked without issue in Ubuntu (kernel 3.16.0-60 on 14.04.2, and kernel 3.19.0-51 on 14.04.3), and suddenly everything is no longer functional upon upgrading to kernel 4.2.0-30 on Ubuntu 14.04.4. In all these cases, I am testing against the x64 generic SMP kernel. I don't see any warnings in `dmesg` logs like I've seen with either xHCI regressions or potential libusb issues in the past, and the application being tested is a (relatively) simple example just to grab frames from a camera: no post-processing or other components to cause the USB core code to fail. The application reports that it simply isn't receiving data from the camera after sending the appropriate control sequence. I have attached the console debug log, and wanted to know if there are any glaring red flags that would point out if it's a kernel regression, hidden bug in code I'm working on, or something else entirely. Thank you, by the way. Matthew Giassa -- Transform Data into Opportunity. Accelerate data analysis in your applications with Intel Data Analytics Acceleration Library. Click to learn more. http://pubads.g.doubleclick.net/gampad/clk?id=278785231&iu=/4140 ___ libusb-devel mailing list libusb-de...@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/libusb-devel -- Transform Data into Opportunity. Accelerate data analysis in your applications with Intel Data Analytics Acceleration Library. Click to learn more. http://pubads.g.doubleclick.net/gampad/clk?id=278785231&iu=/4140 ___ libusb-devel mailing list libusb-de...@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/libusb-devel -- 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: Wait before re-enabling a port that has been disabled due to EMI
On Sun, Mar 20, 2016 at 06:57:30PM +0100, Guido Trentalancia wrote: > Hello Greg ! > > On dom, 2016-03-20 at 10:34 -0700, Greg KH wrote: > > On Sun, Mar 20, 2016 at 06:09:57PM +0100, Guido Trentalancia wrote: > > > > > > [ 1295.575679] usb 6-2: FTDI USB Serial Device converter now > > > attached > > > to ttyUSB1 > > > [ 1302.204285] usb usb6-port2: disabled by hub (EMI?), re- > > > enabling... > > > > > > *** NOTE: EMI is probably still present here *** > > > > > > [ 1303.205202] usb 6-2: USB disconnect, device number 6 > > > [ 1303.205907] ftdi_sio ttyUSB1: FTDI USB Serial Device converter > > > now > > > disconnected from ttyUSB1 > > > [ 1303.205950] ftdi_sio 6-2:1.0: device disconnected > > > [ 1303.414089] usb 6-2: new full-speed USB device number 7 using > > > uhci_hcd > > > [ 1303.526226] usb 6-2: device descriptor read/64, error -71 > > > [ 1303.894228] usb 6-2: new full-speed USB device number 8 using > > > uhci_hcd > > > [ 1304.006185] usb 6-2: device descriptor read/64, error -71 > > > [ 1304.219089] usb 6-2: device descriptor read/64, error -71 > > > [ 1304.422107] usb 6-2: new full-speed USB device number 9 using > > > uhci_hcd > > > [ 1304.640020] usb 6-2: device not accepting address 9, error -71 > > > [ 1304.752024] usb 6-2: new full-speed USB device number 10 using > > > uhci_hcd > > > [ 1305.160020] usb 6-2: device not accepting address 10, error -71 > > > [ 1305.160038] hub 6-0:1.0: unable to enumerate USB device on port > > > 2 > > > > > > *** NOTE: Device is permanently disabled at this point *** > > > > > > I don't know whether my analysis is correct (and therefore the > > > proposed > > > solution appropriate), as reproducing the problem is very > > > difficult... > > > > > > Module parameters are "icky", isn't there some way we can just > > dynamically determine this (i.e. fall back to longer and longer wait > > times?) > > No, we cannot in general predict how long EMI lasts. > > But, for example, I have a known source of EMI close to the computer > (RF transmitter) and I know it lasts a couple of seconds, therefore I > am able to set up the parameter correctly. > > Also, I believe once it fails the first time, the device is permanently > disabled by the USB driver (I am no USB driver expert, do you confirm > ?). Therefore, by the way the driver is actually designed, it is not > possible to try again with a longer wait interval. And it's much more > complicate, plus you still have to hard-code a maximum value I > suppose... Yes, this needs more work, and again, don't use a module parameter please, make it "automatic" if at all possible. 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
[PATCH v2] mct_u232: sanity checking in probe
An attack using the lack of sanity checking in probe
is known. This patch checks for the existance of a
second port.
CVE-2016-3136
Signed-off-by: Oliver Neukum
CC: sta...@vger.kernel.org
v1 - add sanity check for presence of a second port
v2 - add sanity check for an interrupt endpoint
---
drivers/usb/serial/mct_u232.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/usb/serial/mct_u232.c b/drivers/usb/serial/mct_u232.c
index 4446b8d..3e64538 100644
--- a/drivers/usb/serial/mct_u232.c
+++ b/drivers/usb/serial/mct_u232.c
@@ -378,6 +378,10 @@ static int mct_u232_port_probe(struct usb_serial_port
*port)
{
struct mct_u232_private *priv;
+ /* check first to simplify error handling */
+ if (!port->serial->port[1] || !port->serial->port[1]->interrupt_in_urb)
+ return -ENODEV;
+
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mct_u232: sanity checking in probe
On Thu, 2016-03-17 at 10:47 -0400, Johan Hovold wrote: > This looks like you just resent v1 (again, please include the revision > in the mail Subject). Indeed. Wrong directory. Sorry 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] mct_u232: sanity checking in probe
An attack using the lack of sanity checking in probe
is known. This patch checks for the existance of a
second port.
CVE-2016-3136
Signed-off-by: Oliver Neukum
CC: sta...@vger.kernel.org
v1 - add sanity check for presence of a second port
v2 - add sanity check for an interrupt endpoint
---
drivers/usb/serial/mct_u232.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/usb/serial/mct_u232.c b/drivers/usb/serial/mct_u232.c
index 4446b8d..3e64538 100644
--- a/drivers/usb/serial/mct_u232.c
+++ b/drivers/usb/serial/mct_u232.c
@@ -378,6 +378,10 @@ static int mct_u232_port_probe(struct usb_serial_port
*port)
{
struct mct_u232_private *priv;
+ /* check first to simplify error handling */
+ if (!port->serial->port[1] || !port->serial->port[1]->interrupt_in_urb)
+ return -ENODEV;
+
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: "reset full-speed USB device number 6 using ehci-pci" with Dell Inspiron 15R 5537
On Fri, 2016-03-18 at 12:36 -0400, Alan Stern wrote:
> @@ -1584,10 +1581,8 @@ static int hid_resume(struct usb_interfa
> static int hid_reset_resume(struct usb_interface *intf)
> {
> struct hid_device *hid = usb_get_intfdata(intf);
> - struct usbhid_device *usbhid = hid->driver_data;
> int status;
>
> - clear_bit(HID_SUSPENDED, &usbhid->iofl);
> status = hid_post_reset(intf);
> if (status >= 0 && hid->driver && hid->driver->reset_resume) {
> int ret = hid->driver->reset_resume(hid);
Almost. In case of reset_resume() it makes no sense to still
clear a halt or execute a reset that had been ordered. The
corresponding flags should be cleared.
And now that I think of it, if while restarting IO an
old error condition is detected shouldn't the restart wait for
the error to be handled?
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: Yet another Seagate quirk for unusual_uas.h : Correction
Hi David, On 21-03-16 10:08, David Webb wrote: I am sorry but the 't' option (i.e. US_FL_NO_ATA_1X) may not be robust enough. Yesterday after leaving the system running for a few hours the usb disk would no longer connect. Today when I restarted the system with the 't' option, again the disk would not connect. I then tried repeatedly disconnecting and connecting the usb disk without restarting the system. After four failures the disk mounted correctly. I continued disconnecting and connecting the disk and it worked every time. I repeated the tests with the 'u' option after restarting the system. Amongst the output is the message: kernel: usb 2-7: UAS is blacklisted for this device, using usb-storage instead And the connection worked every time. On this basis I recommend using the flag US_FL_IGNORE_UAS as there must be some other potential conflict which is not handled by the flag US_FL_NO_ATA_1X. The change does not seem to have a large effect on the write speed - which is still of order 100 MB/s. /* Reported-by: David Webb */ UNUSUAL_DEV(0x0bc2, 0x331a, 0x, 0x, "Seagate", "Expansion Desk", USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_IGNORE_UAS), US_FL_IGNORE_UAS is a really big hammer, and there are significant performance benefits to using uas (mostly for random r/w access). Can you try again with the t quirk and when the drive fails to detect, collect error messages from "dmesg" and include these error messages in your next mail ? Thanks & 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: Wait before re-enabling a port that has been disabled due to EMI
Hi Oliver,
that would be a misconfiguration issue, however I think we can set an upper
limit for the new parameter... Also consider that the other
"initial_descriptor_timeout" parameter is not limited by an upper limit...
And by the way, I should preferably modify the patch so that it correctly
documents Documentation/kernel-parameters.txt.
Regards,
Guido
Il 21 marzo 2016 09:52:39 CET, Oliver Neukum ha scritto:
>On Sun, 2016-03-20 at 18:09 +0100, Guido Trentalancia wrote:
>> Hello.
>>
>> Considering that EM interference can last for a while (generally up
>to
>> seconds), I am currently testing the following patch for module
>usbcore
>> so that it is possible to specify an amount of time to wait before
>> trying to re-enable a port that has been previously disabled by the
>hub
>> (supposedly because of EM interference).
>>
>> Hopefully, setting the right positive value (for example, 2000
>> milliseconds) would help overcome situations such as the following:
>>
>
>The idea seems sound to me, but the implementation is problematic.
>
>
>> @@ -4960,6 +4967,7 @@ static void port_event(struct usb_hub *h
>> if (!(portstatus & USB_PORT_STAT_ENABLE)
>> && !connect_change && udev) {
>> dev_err(&port_dev->dev, "disabled by hub (EMI?),
>re-enabling...\n");
>> +msleep(emi_recover_timer);
>
>You cannot just stall here for seconds.
>
> 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
--
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: Fix incomplete PM resume operation due to XHCI commmand timeout
On 21.03.2016 06:18, Rajesh Bhagat wrote: Hi I think clearing the whole command ring is a bit too much in this case. It may cause issues for all attached devices when one command times out. Hi Mathias, I understand your point, But I want to understand how would completion handler be called if a command is timed out and xhci_abort_cmd_ring is successful. In this case all the code would be waiting on completion handler forever. 2. xhci_handle_command_timeout -> xhci_abort_cmd_ring(failure) -> xhci_cleanup_command_queue -> xhci_complete_del_and_free_cmd In our case command is timed out, Hence we hit the case #2 but xhci_abort_cmd_ring is success which does not calls complete. xhci_abort_cmd_ring() will write CA bit (CMD_RING_ABORT) to CRCR register. This will generate a command completion event with status "command aborted" for the pending command. This event is then followed by a "command ring stopped" command completion event. See xHCI specs 5.4.5 and 4.6.1.2 handle_cmd_completion() will check if cmd_comp_code == COMP_CMD_ABORT, goto event_handled, and call xhci_complete_del_and_free_cmd(cmd, cmd_comp_code) for the aborted command. If xHCI already processed the aborted command, we might only get a command ring stopped event, in this case handle_cmd_completion() will call xhci_handle_stopped_cmd_ring(xhci, cmd), which will turn the commands that were tagged for "abort" that still remain on the command ring to NO-OP commands. The completion callback will be called for these NO-OP command later when we get a command completion event for them. What kernel version, and what xhci vendor was this triggered on? We are using 4.1.8 kernel Are you able to try a more recent version? -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
Yet another Seagate quirk for unusual_uas.h : Correction
I am sorry but the 't' option (i.e. US_FL_NO_ATA_1X) may not be robust enough. Yesterday after leaving the system running for a few hours the usb disk would no longer connect. Today when I restarted the system with the 't' option, again the disk would not connect. I then tried repeatedly disconnecting and connecting the usb disk without restarting the system. After four failures the disk mounted correctly. I continued disconnecting and connecting the disk and it worked every time. I repeated the tests with the 'u' option after restarting the system. Amongst the output is the message: kernel: usb 2-7: UAS is blacklisted for this device, using usb-storage instead And the connection worked every time. On this basis I recommend using the flag US_FL_IGNORE_UAS as there must be some other potential conflict which is not handled by the flag US_FL_NO_ATA_1X. The change does not seem to have a large effect on the write speed - which is still of order 100 MB/s. /* Reported-by: David Webb */ UNUSUAL_DEV(0x0bc2, 0x331a, 0x, 0x, "Seagate", "Expansion Desk", USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_IGNORE_UAS), Regards, David Webb. -- 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] Input: gtco: fix crash on detecting device without endpoints
>> The full report of this issue can be found here: >> http://seclists.org/bugtraq/2016/Mar/86 The subject and the body of the above message are not matching and the message does not describe the gtco driver bug. The full correct report of this issue can be found in the public Red Hat Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1283385 Best regards, Vladis Dronov | Red Hat, Inc. | Product Security Engineer -- 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: Wait before re-enabling a port that has been disabled due to EMI
On Sun, 2016-03-20 at 18:09 +0100, Guido Trentalancia wrote:
> Hello.
>
> Considering that EM interference can last for a while (generally up to
> seconds), I am currently testing the following patch for module usbcore
> so that it is possible to specify an amount of time to wait before
> trying to re-enable a port that has been previously disabled by the hub
> (supposedly because of EM interference).
>
> Hopefully, setting the right positive value (for example, 2000
> milliseconds) would help overcome situations such as the following:
>
The idea seems sound to me, but the implementation is problematic.
> @@ -4960,6 +4967,7 @@ static void port_event(struct usb_hub *h
> if (!(portstatus & USB_PORT_STAT_ENABLE)
> && !connect_change && udev) {
> dev_err(&port_dev->dev, "disabled by hub (EMI?),
> re-enabling...\n");
> + msleep(emi_recover_timer);
You cannot just stall here for seconds.
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] usb: gadget: udc: atmel: don't disable enpdoints we don't own
Hi,
>UDC driver should NEVER do anything behind
>udc-core's back, so let's stop disabling endpoints
>we don't exactly own - rather we provide as
>resources for gadget drivers. This fixes the
>regression reported by Gil.
>
>Reported-by: Gil Weber
>Signed-off-by: Felipe Balbi
All is working for me now.
Tested-by: Gil Weber
Many thanks,
Gil
>---
>drivers/usb/gadget/udc/atmel_usba_udc.c | 14 --
> 1 file changed, 14 deletions(-)
>
>diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
>b/drivers/usb/gadget/udc/atmel_usba_udc.c
>index 8755b2c2aada..5c96b9b0f48d 100644
>--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
>+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
>@@ -1045,20 +1045,6 @@ static void reset_all_endpoints(struct usba_udc *udc)
>list_del_init(&req->queue);
> request_complete(ep, req, -ECONNRESET);
>}
>-
>- /* NOTE: normally, the next call to the gadget driver is in
>- * charge of disabling endpoints... usually disconnect().
>- * The exception would be entering a high speed test mode.
>- *
>- * FIXME remove this code ... and retest thoroughly.
>- */
>- list_for_each_entry(ep, &udc->gadget.ep_list, ep.ep_list) {
>- if (ep->ep.desc) {
>- spin_unlock(&udc->lock);
>- usba_ep_disable(&ep->ep);
>- spin_lock(&udc->lock);
>- }
>- }
>}
>
>static struct usba_ep *get_ep_by_addr(struct usba_udc *udc, u16 wIndex)
>--
>2.8.0.rc2
>
--
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] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM
On Sun, 2016-03-20 at 11:43 +0100, Geert Uytterhoeven wrote:
> If CONFIG_PM=n:
>
> drivers/net/usb/lan78xx.c: In function ‘lan78xx_get_stats64’:
> drivers/net/usb/lan78xx.c:3274: error: ‘struct dev_pm_info’ has no
> member named ‘runtime_auto’
>
> If PM is disabled, the runtime_auto flag is not available, but auto
> suspend is not enabled anyway. Hence protect the check for
> runtime_auto
> by #ifdef CONFIG_PM to fix this.
>
> Fixes: a59f8c5b048dc938 ("lan78xx: add ndo_get_stats64")
> Reported-by: Guenter Roeck
> Signed-off-by: Geert Uytterhoeven
> ---
> Alternatively, we can add a dev_pm_runtime_auto_is_enabled() wrapper
> to
> include/linux/pm.h, which always return false if CONFIG_PM is
> disabled.
That is what we do for almost everything else in include/pm_runtime.h
So it is the best solution to add the stub.
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] usb: gadget: udc: atmel: don't disable enpdoints we don't own
UDC driver should NEVER do anything behind
udc-core's back, so let's stop disabling endpoints
we don't exactly own - rather we provide as
resources for gadget drivers. This fixes the
regression reported by Gil.
Reported-by: Gil Weber
Signed-off-by: Felipe Balbi
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 8755b2c2aada..5c96b9b0f48d 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -1045,20 +1045,6 @@ static void reset_all_endpoints(struct usba_udc *udc)
list_del_init(&req->queue);
request_complete(ep, req, -ECONNRESET);
}
-
- /* NOTE: normally, the next call to the gadget driver is in
-* charge of disabling endpoints... usually disconnect().
-* The exception would be entering a high speed test mode.
-*
-* FIXME remove this code ... and retest thoroughly.
-*/
- list_for_each_entry(ep, &udc->gadget.ep_list, ep.ep_list) {
- if (ep->ep.desc) {
- spin_unlock(&udc->lock);
- usba_ep_disable(&ep->ep);
- spin_lock(&udc->lock);
- }
- }
}
static struct usba_ep *get_ep_by_addr(struct usba_udc *udc, u16 wIndex)
--
2.8.0.rc2
--
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: dwc3: core: Introduce dwc3_device_reinit()
Hi, John Youn writes: > [ text/plain ] > On 3/16/2016 6:56 AM, Felipe Balbi wrote: >> >> heh, +john >> >> Felipe Balbi writes: >>> [ text/plain ] >>> >>> Hi, >>> >>> Roger Quadros writes: [ text/plain ] We will need this function for a workaround. The function issues a softreset only to the device controller and performs minimal re-initialization so that the device controller can be usable. As some code is similar to dwc3_core_init() take out common code into dwc3_get_gctl_quirks(). We add a new member (prtcap_mode) to struct dwc3 to keep track of the current mode in the PRTCAPDIR register. Signed-off-by: Roger Quadros >>> >>> I must say, I don't like this at all :-p There's ONE known silicon which >>> needs this because of a poor silicon integration which took an IP with a >>> known erratum where it can't be made to work on lower speeds and STILL >>> was integrated without a superspeed PHY. >>> >>> There's a reason why I never tried to push this upstream myself ;-) >>> >>> I'm really thinking we might be better off adding a quirk flag to skip >>> the metastability workaround and allow this ONE silicon to set the >>> controller to lower speed. >>> >>> John, can you check with your colleagues if we would ever fall into >>> STAR#9000525659 if we set maximum speed to high speed during driver >>> probe and never touch it again ? I would assume we don't really fall >>> into the metastability workaround, right ? We're not doing any sort of >>> PM for dwc3... >>> > > Hi Felipe, > > I don't know much about this but I will check with the engineers and > get back to you. Thank you John ;-) -- balbi signature.asc Description: PGP signature
