| Message ID | 20170512123052.6265-1-shrirang.bagul@canonical.com |
|---|---|
| State | New |
| Headers | show |
On Fri, May 12, 2017 at 08:30:52PM +0800, Shrirang Bagul wrote: > BugLink: https://bugs.launchpad.net/bugs/1690362 > > The XR21V1412 ports were using the hardware power-on default of 9600bps > after resume from S3/S4. This patch re-initialises the baud rate and > fixes the re-connection issues after S3/S4. > > Changelog: > Version 1B, 11/6/2015 > Fixed Bug: The conditional logic to support kernel 3.9 was incorrect(line 396 in xr_usb_serial_common.c). > > Version 1A, 1/9/2015 > This driver will work with any USB UART function in these Exar devices: > XR21V1410/1412/1414 > XR21B1411 > XR21B1420/1422/1424 > XR22801/802/804 > > The source code has been tested on various Linux kernels from 3.6.x to 3.17.x. > This may also work with newer kernels as well. > > Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com> > --- > ubuntu/xr-usb-serial/Makefile | 3 ++- > ubuntu/xr-usb-serial/xr_usb_serial_common.c | 14 ++++++++++++-- > 2 files changed, 14 insertions(+), 3 deletions(-) > > diff --git a/ubuntu/xr-usb-serial/Makefile b/ubuntu/xr-usb-serial/Makefile > index 1a323a62b348..533988cede12 100644 > --- a/ubuntu/xr-usb-serial/Makefile > +++ b/ubuntu/xr-usb-serial/Makefile > @@ -1,6 +1,7 @@ > obj-m := xr_usb_serial_common.o > > -KERNELDIR ?= /lib/modules/$(shell uname -r)/build > +#KERNELDIR ?= /lib/modules/$(shell uname -r)/build > +KERNELDIR ?=$(shell pwd)/../parts/kernel/build > PWD := $(shell pwd) > > EXTRA_CFLAGS := -DDEBUG=0 > diff --git a/ubuntu/xr-usb-serial/xr_usb_serial_common.c b/ubuntu/xr-usb-serial/xr_usb_serial_common.c > index 5d049855fb1a..09d21e63606e 100644 > --- a/ubuntu/xr-usb-serial/xr_usb_serial_common.c > +++ b/ubuntu/xr-usb-serial/xr_usb_serial_common.c > @@ -1146,7 +1146,7 @@ static void xr_usb_serial_tty_set_termios(struct tty_struct *tty, > } > > > - if (memcmp(&xr_usb_serial->line, &newline, sizeof newline)) > + //if (memcmp(&xr_usb_serial->line, &newline, sizeof newline)) > { > memcpy(&xr_usb_serial->line, &newline, sizeof newline); > /* > @@ -1819,11 +1819,21 @@ static int xr_usb_serial_resume(struct usb_interface *intf) > { > struct xr_usb_serial *xr_usb_serial = usb_get_intfdata(intf); > struct xr_usb_serial_wb *wb; > +#if 0 > + struct tty_struct *tty = xr_usb_serial->port.tty; > +#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 7, 0) > + struct ktermios *termios = tty->termios; > +#else > + struct ktermios *termios = &tty->termios; > +#endif > +#endif > int rv = 0; > int cnt; > > xr_usb_serial_pre_setup(xr_usb_serial); > - > + xr_usb_serial_disable(xr_usb_serial); > + xr_usb_serial_set_line(xr_usb_serial, &xr_usb_serial->line); > + xr_usb_serial_enable(xr_usb_serial); > spin_lock_irq(&xr_usb_serial->read_lock); > xr_usb_serial->susp_count -= 1; > cnt = xr_usb_serial->susp_count; This patch is sloppy and has some obviously unrelated changes (those to the makefile and adding in code that is commented out). So it's not very nice, but for this particular driver I guess it isn't that important so long as the changes work. There's no comments about testing on the bug; I assume it has been tested?
On 17/05/17 15:33, Seth Forshee wrote: > On Fri, May 12, 2017 at 08:30:52PM +0800, Shrirang Bagul wrote: >> BugLink: https://bugs.launchpad.net/bugs/1690362 >> >> The XR21V1412 ports were using the hardware power-on default of 9600bps >> after resume from S3/S4. This patch re-initialises the baud rate and >> fixes the re-connection issues after S3/S4. >> >> Changelog: >> Version 1B, 11/6/2015 >> Fixed Bug: The conditional logic to support kernel 3.9 was incorrect(line 396 in xr_usb_serial_common.c). >> >> Version 1A, 1/9/2015 >> This driver will work with any USB UART function in these Exar devices: >> XR21V1410/1412/1414 >> XR21B1411 >> XR21B1420/1422/1424 >> XR22801/802/804 >> >> The source code has been tested on various Linux kernels from 3.6.x to 3.17.x. >> This may also work with newer kernels as well. >> >> Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com> >> --- >> ubuntu/xr-usb-serial/Makefile | 3 ++- >> ubuntu/xr-usb-serial/xr_usb_serial_common.c | 14 ++++++++++++-- >> 2 files changed, 14 insertions(+), 3 deletions(-) >> >> diff --git a/ubuntu/xr-usb-serial/Makefile b/ubuntu/xr-usb-serial/Makefile >> index 1a323a62b348..533988cede12 100644 >> --- a/ubuntu/xr-usb-serial/Makefile >> +++ b/ubuntu/xr-usb-serial/Makefile >> @@ -1,6 +1,7 @@ >> obj-m := xr_usb_serial_common.o >> >> -KERNELDIR ?= /lib/modules/$(shell uname -r)/build >> +#KERNELDIR ?= /lib/modules/$(shell uname -r)/build >> +KERNELDIR ?=$(shell pwd)/../parts/kernel/build >> PWD := $(shell pwd) >> >> EXTRA_CFLAGS := -DDEBUG=0 >> diff --git a/ubuntu/xr-usb-serial/xr_usb_serial_common.c b/ubuntu/xr-usb-serial/xr_usb_serial_common.c >> index 5d049855fb1a..09d21e63606e 100644 >> --- a/ubuntu/xr-usb-serial/xr_usb_serial_common.c >> +++ b/ubuntu/xr-usb-serial/xr_usb_serial_common.c >> @@ -1146,7 +1146,7 @@ static void xr_usb_serial_tty_set_termios(struct tty_struct *tty, >> } >> >> >> - if (memcmp(&xr_usb_serial->line, &newline, sizeof newline)) >> + //if (memcmp(&xr_usb_serial->line, &newline, sizeof newline)) >> { >> memcpy(&xr_usb_serial->line, &newline, sizeof newline); >> /* >> @@ -1819,11 +1819,21 @@ static int xr_usb_serial_resume(struct usb_interface *intf) >> { >> struct xr_usb_serial *xr_usb_serial = usb_get_intfdata(intf); >> struct xr_usb_serial_wb *wb; >> +#if 0 >> + struct tty_struct *tty = xr_usb_serial->port.tty; >> +#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 7, 0) >> + struct ktermios *termios = tty->termios; >> +#else >> + struct ktermios *termios = &tty->termios; >> +#endif >> +#endif >> int rv = 0; >> int cnt; >> >> xr_usb_serial_pre_setup(xr_usb_serial); >> - >> + xr_usb_serial_disable(xr_usb_serial); >> + xr_usb_serial_set_line(xr_usb_serial, &xr_usb_serial->line); >> + xr_usb_serial_enable(xr_usb_serial); >> spin_lock_irq(&xr_usb_serial->read_lock); >> xr_usb_serial->susp_count -= 1; >> cnt = xr_usb_serial->susp_count; > > This patch is sloppy and has some obviously unrelated changes (those to > the makefile and adding in code that is commented out). So it's not very > nice, but for this particular driver I guess it isn't that important so > long as the changes work. There's no comments about testing on the bug; > I assume it has been tested? > personally speaking, I'd prefer it to be cleaned up first before we apply it to our repo. Colin
On Wed, 2017-05-17 at 09:33 -0500, Seth Forshee wrote: > On Fri, May 12, 2017 at 08:30:52PM +0800, Shrirang Bagul wrote: > > BugLink: https://bugs.launchpad.net/bugs/1690362 > > > > The XR21V1412 ports were using the hardware power-on default of 9600bps > > after resume from S3/S4. This patch re-initialises the baud rate and > > fixes the re-connection issues after S3/S4. > > > > Changelog: > > Version 1B, 11/6/2015 > > Fixed Bug: The conditional logic to support kernel 3.9 was incorrect(line 396 in > > xr_usb_serial_common.c). > > > > Version 1A, 1/9/2015 > > This driver will work with any USB UART function in these Exar devices: > > XR21V1410/1412/1414 > > XR21B1411 > > XR21B1420/1422/1424 > > XR22801/802/804 > > > > The source code has been tested on various Linux kernels from 3.6.x to 3.17.x. > > This may also work with newer kernels as well. > > > > Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com> > > --- > > ubuntu/xr-usb-serial/Makefile | 3 ++- > > ubuntu/xr-usb-serial/xr_usb_serial_common.c | 14 ++++++++++++-- > > 2 files changed, 14 insertions(+), 3 deletions(-) > > > > diff --git a/ubuntu/xr-usb-serial/Makefile b/ubuntu/xr-usb-serial/Makefile > > index 1a323a62b348..533988cede12 100644 > > --- a/ubuntu/xr-usb-serial/Makefile > > +++ b/ubuntu/xr-usb-serial/Makefile > > @@ -1,6 +1,7 @@ > > obj-m := xr_usb_serial_common.o > > > > -KERNELDIR ?= /lib/modules/$(shell uname -r)/build > > +#KERNELDIR ?= /lib/modules/$(shell uname -r)/build > > +KERNELDIR ?=$(shell pwd)/../parts/kernel/build > > PWD := $(shell pwd) > > > > EXTRA_CFLAGS := -DDEBUG=0 > > diff --git a/ubuntu/xr-usb-serial/xr_usb_serial_common.c b/ubuntu/xr-usb- > > serial/xr_usb_serial_common.c > > index 5d049855fb1a..09d21e63606e 100644 > > --- a/ubuntu/xr-usb-serial/xr_usb_serial_common.c > > +++ b/ubuntu/xr-usb-serial/xr_usb_serial_common.c > > @@ -1146,7 +1146,7 @@ static void xr_usb_serial_tty_set_termios(struct tty_struct > > *tty, > > } > > > > > > - if (memcmp(&xr_usb_serial->line, &newline, sizeof newline)) > > + //if (memcmp(&xr_usb_serial->line, &newline, sizeof newline)) > > { > > memcpy(&xr_usb_serial->line, &newline, sizeof newline); > > /* > > @@ -1819,11 +1819,21 @@ static int xr_usb_serial_resume(struct usb_interface > > *intf) > > { > > struct xr_usb_serial *xr_usb_serial = usb_get_intfdata(intf); > > struct xr_usb_serial_wb *wb; > > +#if 0 > > + struct tty_struct *tty = xr_usb_serial->port.tty; > > +#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 7, 0) > > + struct ktermios *termios = tty->termios; > > +#else > > + struct ktermios *termios = &tty->termios; > > +#endif > > +#endif > > int rv = 0; > > int cnt; > > > > xr_usb_serial_pre_setup(xr_usb_serial); > > - > > + xr_usb_serial_disable(xr_usb_serial); > > + xr_usb_serial_set_line(xr_usb_serial, &xr_usb_serial->line); > > + xr_usb_serial_enable(xr_usb_serial); > > spin_lock_irq(&xr_usb_serial->read_lock); > > xr_usb_serial->susp_count -= 1; > > cnt = xr_usb_serial->susp_count; > > This patch is sloppy and has some obviously unrelated changes (those to > the makefile and adding in code that is commented out). So it's not very > nice, but for this particular driver I guess it isn't that important so > long as the changes work. There's no comments about testing on the bug; > I assume it has been tested? This driver is from third party vendor and is merged as-is. It has been tested by Canonical QA on Dell IoT gateway which sports this EXAR USB-Serial chip.
On Wed, 2017-05-17 at 15:36 +0100, Colin Ian King wrote: > On 17/05/17 15:33, Seth Forshee wrote: > > On Fri, May 12, 2017 at 08:30:52PM +0800, Shrirang Bagul wrote: > > > BugLink: https://bugs.launchpad.net/bugs/1690362 > > > > > > The XR21V1412 ports were using the hardware power-on default of 9600bps > > > after resume from S3/S4. This patch re-initialises the baud rate and > > > fixes the re-connection issues after S3/S4. > > > > > > Changelog: > > > Version 1B, 11/6/2015 > > > Fixed Bug: The conditional logic to support kernel 3.9 was incorrect(line 396 > > > in xr_usb_serial_common.c). > > > > > > Version 1A, 1/9/2015 > > > This driver will work with any USB UART function in these Exar devices: > > > XR21V1410/1412/1414 > > > XR21B1411 > > > XR21B1420/1422/1424 > > > XR22801/802/804 > > > > > > The source code has been tested on various Linux kernels from 3.6.x to 3.17.x. > > > This may also work with newer kernels as well. > > > > > > Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com> > > > --- > > > ubuntu/xr-usb-serial/Makefile | 3 ++- > > > ubuntu/xr-usb-serial/xr_usb_serial_common.c | 14 ++++++++++++-- > > > 2 files changed, 14 insertions(+), 3 deletions(-) > > > > > > diff --git a/ubuntu/xr-usb-serial/Makefile b/ubuntu/xr-usb-serial/Makefile > > > index 1a323a62b348..533988cede12 100644 > > > --- a/ubuntu/xr-usb-serial/Makefile > > > +++ b/ubuntu/xr-usb-serial/Makefile > > > @@ -1,6 +1,7 @@ > > > obj-m := xr_usb_serial_common.o > > > > > > -KERNELDIR ?= /lib/modules/$(shell uname -r)/build > > > +#KERNELDIR ?= /lib/modules/$(shell uname -r)/build > > > +KERNELDIR ?=$(shell pwd)/../parts/kernel/build > > > PWD := $(shell pwd) > > > > > > EXTRA_CFLAGS := -DDEBUG=0 > > > diff --git a/ubuntu/xr-usb-serial/xr_usb_serial_common.c b/ubuntu/xr-usb- > > > serial/xr_usb_serial_common.c > > > index 5d049855fb1a..09d21e63606e 100644 > > > --- a/ubuntu/xr-usb-serial/xr_usb_serial_common.c > > > +++ b/ubuntu/xr-usb-serial/xr_usb_serial_common.c > > > @@ -1146,7 +1146,7 @@ static void xr_usb_serial_tty_set_termios(struct > > > tty_struct *tty, > > > } > > > > > > > > > - if (memcmp(&xr_usb_serial->line, &newline, sizeof newline)) > > > + //if (memcmp(&xr_usb_serial->line, &newline, sizeof newline)) > > > { > > > memcpy(&xr_usb_serial->line, &newline, sizeof newline); > > > /* > > > @@ -1819,11 +1819,21 @@ static int xr_usb_serial_resume(struct usb_interface > > > *intf) > > > { > > > struct xr_usb_serial *xr_usb_serial = usb_get_intfdata(intf); > > > struct xr_usb_serial_wb *wb; > > > +#if 0 > > > + struct tty_struct *tty = xr_usb_serial->port.tty; > > > +#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 7, 0) > > > + struct ktermios *termios = tty->termios; > > > +#else > > > + struct ktermios *termios = &tty->termios; > > > +#endif > > > +#endif > > > int rv = 0; > > > int cnt; > > > > > > xr_usb_serial_pre_setup(xr_usb_serial); > > > - > > > + xr_usb_serial_disable(xr_usb_serial); > > > + xr_usb_serial_set_line(xr_usb_serial, &xr_usb_serial->line); > > > + xr_usb_serial_enable(xr_usb_serial); > > > spin_lock_irq(&xr_usb_serial->read_lock); > > > xr_usb_serial->susp_count -= 1; > > > cnt = xr_usb_serial->susp_count; > > > > This patch is sloppy and has some obviously unrelated changes (those to > > the makefile and adding in code that is commented out). So it's not very > > nice, but for this particular driver I guess it isn't that important so > > long as the changes work. There's no comments about testing on the bug; > > I assume it has been tested? > > > > personally speaking, I'd prefer it to be cleaned up first before we > apply it to our repo. This has been discussed before, see https://lists.ubuntu.com/archives/kernel-team/201 6-November/081192.html Shrirang > > Colin
On Wed, May 17, 2017 at 10:40:32PM +0800, Shrirang Bagul wrote: > On Wed, 2017-05-17 at 09:33 -0500, Seth Forshee wrote: > > On Fri, May 12, 2017 at 08:30:52PM +0800, Shrirang Bagul wrote: > > > BugLink: https://bugs.launchpad.net/bugs/1690362 > > > > > > The XR21V1412 ports were using the hardware power-on default of 9600bps > > > after resume from S3/S4. This patch re-initialises the baud rate and > > > fixes the re-connection issues after S3/S4. > > > > > > Changelog: > > > Version 1B, 11/6/2015 > > > Fixed Bug: The conditional logic to support kernel 3.9 was incorrect(line 396 in > > > xr_usb_serial_common.c). > > > > > > Version 1A, 1/9/2015 > > > This driver will work with any USB UART function in these Exar devices: > > > XR21V1410/1412/1414 > > > XR21B1411 > > > XR21B1420/1422/1424 > > > XR22801/802/804 > > > > > > The source code has been tested on various Linux kernels from 3.6.x to 3.17.x. > > > This may also work with newer kernels as well. > > > > > > Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com> > > > --- > > > ubuntu/xr-usb-serial/Makefile | 3 ++- > > > ubuntu/xr-usb-serial/xr_usb_serial_common.c | 14 ++++++++++++-- > > > 2 files changed, 14 insertions(+), 3 deletions(-) > > > > > > diff --git a/ubuntu/xr-usb-serial/Makefile b/ubuntu/xr-usb-serial/Makefile > > > index 1a323a62b348..533988cede12 100644 > > > --- a/ubuntu/xr-usb-serial/Makefile > > > +++ b/ubuntu/xr-usb-serial/Makefile > > > @@ -1,6 +1,7 @@ > > > obj-m := xr_usb_serial_common.o > > > > > > -KERNELDIR ?= /lib/modules/$(shell uname -r)/build > > > +#KERNELDIR ?= /lib/modules/$(shell uname -r)/build > > > +KERNELDIR ?=$(shell pwd)/../parts/kernel/build > > > PWD := $(shell pwd) > > > > > > EXTRA_CFLAGS := -DDEBUG=0 > > > diff --git a/ubuntu/xr-usb-serial/xr_usb_serial_common.c b/ubuntu/xr-usb- > > > serial/xr_usb_serial_common.c > > > index 5d049855fb1a..09d21e63606e 100644 > > > --- a/ubuntu/xr-usb-serial/xr_usb_serial_common.c > > > +++ b/ubuntu/xr-usb-serial/xr_usb_serial_common.c > > > @@ -1146,7 +1146,7 @@ static void xr_usb_serial_tty_set_termios(struct tty_struct > > > *tty, > > > } > > > > > > > > > - if (memcmp(&xr_usb_serial->line, &newline, sizeof newline)) > > > + //if (memcmp(&xr_usb_serial->line, &newline, sizeof newline)) > > > { > > > memcpy(&xr_usb_serial->line, &newline, sizeof newline); > > > /* > > > @@ -1819,11 +1819,21 @@ static int xr_usb_serial_resume(struct usb_interface > > > *intf) > > > { > > > struct xr_usb_serial *xr_usb_serial = usb_get_intfdata(intf); > > > struct xr_usb_serial_wb *wb; > > > +#if 0 > > > + struct tty_struct *tty = xr_usb_serial->port.tty; > > > +#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 7, 0) > > > + struct ktermios *termios = tty->termios; > > > +#else > > > + struct ktermios *termios = &tty->termios; > > > +#endif > > > +#endif > > > int rv = 0; > > > int cnt; > > > > > > xr_usb_serial_pre_setup(xr_usb_serial); > > > - > > > + xr_usb_serial_disable(xr_usb_serial); > > > + xr_usb_serial_set_line(xr_usb_serial, &xr_usb_serial->line); > > > + xr_usb_serial_enable(xr_usb_serial); > > > spin_lock_irq(&xr_usb_serial->read_lock); > > > xr_usb_serial->susp_count -= 1; > > > cnt = xr_usb_serial->susp_count; > > > > This patch is sloppy and has some obviously unrelated changes (those to > > the makefile and adding in code that is commented out). So it's not very > > nice, but for this particular driver I guess it isn't that important so > > long as the changes work. There's no comments about testing on the bug; > > I assume it has been tested? > This driver is from third party vendor and is merged as-is. It has been tested by > Canonical QA on Dell IoT gateway which sports this EXAR USB-Serial chip. In that case, with reluctance: Acked-by: Seth Forshee <seth.forshee@canonical.com>
Same sentiment.
Applied to xenial master-next branch. Thanks. Cascardo.
diff --git a/ubuntu/xr-usb-serial/Makefile b/ubuntu/xr-usb-serial/Makefile index 1a323a62b348..533988cede12 100644 --- a/ubuntu/xr-usb-serial/Makefile +++ b/ubuntu/xr-usb-serial/Makefile @@ -1,6 +1,7 @@ obj-m := xr_usb_serial_common.o -KERNELDIR ?= /lib/modules/$(shell uname -r)/build +#KERNELDIR ?= /lib/modules/$(shell uname -r)/build +KERNELDIR ?=$(shell pwd)/../parts/kernel/build PWD := $(shell pwd) EXTRA_CFLAGS := -DDEBUG=0 diff --git a/ubuntu/xr-usb-serial/xr_usb_serial_common.c b/ubuntu/xr-usb-serial/xr_usb_serial_common.c index 5d049855fb1a..09d21e63606e 100644 --- a/ubuntu/xr-usb-serial/xr_usb_serial_common.c +++ b/ubuntu/xr-usb-serial/xr_usb_serial_common.c @@ -1146,7 +1146,7 @@ static void xr_usb_serial_tty_set_termios(struct tty_struct *tty, } - if (memcmp(&xr_usb_serial->line, &newline, sizeof newline)) + //if (memcmp(&xr_usb_serial->line, &newline, sizeof newline)) { memcpy(&xr_usb_serial->line, &newline, sizeof newline); /* @@ -1819,11 +1819,21 @@ static int xr_usb_serial_resume(struct usb_interface *intf) { struct xr_usb_serial *xr_usb_serial = usb_get_intfdata(intf); struct xr_usb_serial_wb *wb; +#if 0 + struct tty_struct *tty = xr_usb_serial->port.tty; +#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 7, 0) + struct ktermios *termios = tty->termios; +#else + struct ktermios *termios = &tty->termios; +#endif +#endif int rv = 0; int cnt; xr_usb_serial_pre_setup(xr_usb_serial); - + xr_usb_serial_disable(xr_usb_serial); + xr_usb_serial_set_line(xr_usb_serial, &xr_usb_serial->line); + xr_usb_serial_enable(xr_usb_serial); spin_lock_irq(&xr_usb_serial->read_lock); xr_usb_serial->susp_count -= 1; cnt = xr_usb_serial->susp_count;
BugLink: https://bugs.launchpad.net/bugs/1690362 The XR21V1412 ports were using the hardware power-on default of 9600bps after resume from S3/S4. This patch re-initialises the baud rate and fixes the re-connection issues after S3/S4. Changelog: Version 1B, 11/6/2015 Fixed Bug: The conditional logic to support kernel 3.9 was incorrect(line 396 in xr_usb_serial_common.c). Version 1A, 1/9/2015 This driver will work with any USB UART function in these Exar devices: XR21V1410/1412/1414 XR21B1411 XR21B1420/1422/1424 XR22801/802/804 The source code has been tested on various Linux kernels from 3.6.x to 3.17.x. This may also work with newer kernels as well. Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com> --- ubuntu/xr-usb-serial/Makefile | 3 ++- ubuntu/xr-usb-serial/xr_usb_serial_common.c | 14 ++++++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-)