Re: [PATCH 06/13] USB: serial: ch341: fix initial line settings

[Johan Hovold <johan@kernel.org>]

On Tue, Dec 20, 2016 at 07:31:55AM -0800, Russell Senior wrote:
> On Tue, Dec 20, 2016 at 1:13 AM, Johan Hovold <johan@kernel.org> wrote:
> > Perhaps you could give the attached vendor driver a quick spin just to
> > confirm that? It's a rebased version against usb-next.
> 
> This is plain jane usb-next with your vendor patch, which I applied in
> a local branch:
> 
>  00001-gfadd29d:
> 
> Dec 20 07:18:36 willard kernel: PKCS#7 signature not signed with a trusted key
> Dec 20 07:18:36 willard kernel: ch341: module verification failed:
> signature and/or required key missing - tainting kernel
> Dec 20 07:18:36 willard kernel: usbcore: registered new interface driver ch341
> Dec 20 07:18:36 willard kernel: usbserial: USB Serial support
> registered for ch34x
> Dec 20 07:18:44 willard kernel: usb 6-2: new full-speed USB device
> number 12 using uhci_hcd
> Dec 20 07:18:44 willard kernel: usb 6-2: New USB device found,
> idVendor=1a86, idProduct=7523
> Dec 20 07:18:44 willard kernel: usb 6-2: New USB device strings:
> Mfr=0, Product=2, SerialNumber=0
> Dec 20 07:18:44 willard kernel: usb 6-2: Product: USB2.0-Ser!
> Dec 20 07:18:44 willard kernel: ch341 6-2:1.0: ch34x converter detected
> Dec 20 07:18:44 willard kernel: ------------[ cut here ]------------
> Dec 20 07:18:44 willard kernel: WARNING: CPU: 1 PID: 30168 at
> /home/russell/src/usb-serial/drivers/usb/core/hcd.c:1584
> usb_hcd_map_urb_for_dma+0x382/0x560
> Dec 20 07:18:44 willard kernel: transfer buffer not dma capable

Yeah, you would get that with 4.9 and x86 as the driver is using a
buffer on stack for to control transfers (reads) at probe. Should not
affect device behaviour, but if you want you can kzalloc buf in
ch34x_attach() (patch below) and try again.
 
> and, it didn't work.  Got 0xff's out on the pl2303, got nothing
> visible when characters sent in the other direction after setting
> 115200 8N1 on both sides.

Ok, so that sounds like the same behaviour you see when using usb-next
with the initial LCR write removed. This device does not seem to support
using the init-vendor command to change line settings.

> And, fwiw, I've been rmmod ch341 ; insmod
> $path/ch341.ko, where $path is the kernel module tree for the commit
> build, all from a 00013-gc510871 base kernel without rebooting.

That should be fine, but you probably want to reboot when you're done
testing the vendor driver.

> Not sure what the complaint about keys is about, I had not seen that
> before.

Related to module signing, not sure why you only see it with the vendor
driver. Did you apply it to my usb-next branch or Greg's?

Thanks,
Johan


>From 7779ac8871d36d48a0ee3d41f148ca046523f556 Mon Sep 17 00:00:00 2001
From: Johan Hovold <johan@kernel.org>
Date: Tue, 20 Dec 2016 16:48:52 +0100
Subject: [PATCH] tmp: ch341: fix DMA to stack

---
 drivers/usb/serial/ch341.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index b193b78c27bb..cd128e99843d 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -706,7 +706,7 @@ static int ch34x_attach(struct usb_serial *serial)
 {
 	struct ch34x_private *priv;
 	int i;
-	char buf[8];
+	char *buf;
 
 	dev_dbg(&serial->interface->dev, "%s", __func__);
 
@@ -724,6 +724,10 @@ static int ch34x_attach(struct usb_serial *serial)
 		usb_set_serial_port_data(serial->port[i], priv);
 	}
 
+	buf = kzalloc(8, GFP_KERNEL);
+	if (!buf)
+		goto cleanup;
+
 	ch34x_vendor_read(VENDOR_VERSION, 0x0000, 0x0000,
 			serial, buf, 0x02);
 	ch34x_vendor_write(VENDOR_SERIAL_INIT, 0x0000, 0x0000,
@@ -739,6 +743,8 @@ static int ch34x_attach(struct usb_serial *serial)
 	ch34x_vendor_write(VENDOR_MODEM_OUT, 0x009F, 0x0000,
 			serial, NULL, 0x00);
 
+	kfree(buf);
+
 	return 0;
 
 cleanup:
-- 
2.10.2