CDC ACM nusb/serial driver

CDC ACM nusb/serial driver

[Eli Cohen]

[-- Attachment #1: Type: text/plain, Size: 867 bytes --]

I've cleaned up this driver to closely reflect the OpenBSD umodem
driver and address the issue of changing serial.c in a cleaner way.

cinap, we talked about having to support multiple serial interfaces on
one device, from the looks of it the OpenBSD driver may actually *not*
support this. this patch attempts to do so, but I have no such devices
to test it with...

the nusb/serial driver still needs work. I'm mounting 9p over the
serial port and it's flaky, it eventually screws up. I also did this
with an FTDI device and that was flaky too, before I made any changes.
I'm not sure what's going on, in kprint I see 9p errors when it screws
up... dropped data maybe...?

the devices I'm working with, Arduino Megas, work pretty well with
nusb/serial, both FTDI and with this patch CDC ACM devices, but like I
said above, I have no other devices to test this with

[-- Attachment #2: serial.patch --]
[-- Type: text/x-patch, Size: 9547 bytes --]

diff -r 23bc1d0e2dd2 sys/src/cmd/nusb/serial/cdc.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/sys/src/cmd/nusb/serial/cdc.c	Thu Jul 30 21:06:29 2020 -0700
@@ -0,0 +1,301 @@
+#include <u.h>
+#include <libc.h>
+#include <thread.h>
+#include <fcall.h>
+#include <9p.h>
+#include "usb.h"
+#include "serial.h"
+
+enum {
+	Kyocera		= 0x0482,
+	AHK3001V	= 0x0203,
+};
+
+enum {
+	ScACM	= 2,
+	AT		= 1,
+	None	= 0,
+	LineCoding	= 0x20,
+	SetBreak	= 0x23,
+	ControlState = 0x22,
+};
+
+enum {
+	ClassCDCData = 0x0a,
+	SubclassData = 0,
+};
+
+enum {
+	SubCDCCM	= 1,
+	SubCDCACM	= 2,
+	SubCDCUnion	= 6,
+};
+
+typedef struct raw_desc {
+	uchar bLength;
+	uchar bDescriptorType;
+	uchar bDescriptorSubtype;
+	uchar bbytes[1];
+} raw_desc;
+
+typedef struct cm_desc {
+	uchar bLength;
+	uchar bDescriptorType;
+	uchar bDescriptorSubtype;
+	uchar bmCapabilities;
+	uchar bDataInterface;
+} cm_desc;
+
+enum {
+	CMDoesCM	= 1,
+	CMOverData	= 2,
+};
+
+typedef struct acm_desc {
+	uchar bLength;
+	uchar bDescriptorType;
+	uchar bDescriptorSubtype;
+	uchar bmCapabilities;
+} acm_desc;
+
+enum {
+	ACMHasFeature	= 1,
+	ACMHasLine		= 2,
+	ACMHasBreak		= 4,
+	ACMHasNetwork	= 8,
+};
+
+typedef struct union_desc {
+	uchar bLength;
+	uchar bDescriptorType;
+	uchar bDescriptorSubtype;
+	uchar bMasterInterface;
+	uchar bSlaveInterface[1];
+} union_desc;
+
+typedef struct cdc_notify {
+	uchar bmRequestType;
+	uchar bNotification;
+	ushort wValue;
+	ushort wIndex;
+	ushort wLength;
+	uchar data[16];
+} cdc_notify;
+
+enum {
+	Notification = 0xa1,
+	SerialState = 0x20,
+};
+
+enum {
+	DcdStatus	= 0x01,
+	DsrStatus	= 0x02,
+	RingStatus	= 0x08,
+};
+
+static int
+cdcsetparam(Serialport *p)
+{
+	int res;
+	uchar vals[7];
+
+	PUT4(vals, p->baud);
+
+	vals[4] = 0;
+	if(p->stop == 1)
+		vals[4] = 0;
+	if(p->stop == 15)
+		vals[4] = 1;
+	if(p->stop == 2)
+		vals[4] = 2;
+
+	vals[5] = p->parity;
+
+	vals[6] = p->bits;
+
+	res = usbcmd(p->s->dev, Rh2d | Rclass | Riface, LineCoding, 0, 0, vals, 7);
+
+	return res;
+}
+
+void
+getcaps(Serialport *p)
+{
+	int i, j;
+	Desc *desc;
+	Iface *iface;
+	raw_desc ddesc;
+	cm_desc dcm;
+	acm_desc dacm;
+	union_desc dunion;
+	int cur_ifc;
+
+	for (i = 0; i < Niface; i++) {
+		iface = p->s->dev->usb->conf[0]->iface[i];
+		if (iface == nil)
+			continue;
+
+		cur_ifc = iface->id;
+		if (cur_ifc != p->ctl_ifc && Class(iface->csp) == ClassCDCData && Subclass(iface->csp) == SubclassData && p->data_ifc == -1)
+			p->data_ifc = cur_ifc;
+
+		if (cur_ifc == p->ctl_ifc)
+			for (j = 0; j < Nddesc; j++) {
+				desc = p->s->dev->usb->ddesc[j];
+				if (desc == nil)
+					continue;
+
+				ddesc = (raw_desc)desc->data;
+				if (ddesc.bDescriptorType == Dfunction) {
+					switch (ddesc.bDescriptorSubtype) {
+					case SubCDCCM:
+						dcm = (cm_desc)ddesc;
+						p->data_ifc = dcm.bDataInterface;
+						break;
+					case SubCDCACM:
+						dacm = (acm_desc)ddesc;
+						p->hasbreak = dacm.bmCapabilities & ACMHasBreak;
+						break;
+					case SubCDCUnion:
+						dunion = (union_desc)ddesc;
+						p->data_ifc = dunion.bSlaveInterface[0];
+						break;
+					}
+				}
+			}
+	}
+}
+
+void
+statusreader(void *u)
+{
+	Serialport *p = u;
+	uchar buf[sizeof(cdc_notify)];
+	cdc_notify *notify;
+
+	notify = (cdc_notify*)buf;
+	threadsetname("statusreaderproc");
+
+	while(read(p->epintr->dfd, buf, sizeof(cdc_notify)) > 0) {
+		if (notify->bmRequestType == Notification && notify->bNotification == SerialState && notify->wLength == 2) {
+			p->dcd = notify->data[0] & DcdStatus;
+			p->dsr = notify->data[0] & DsrStatus;
+			p->ring = notify->data[0] & RingStatus;
+		} else {
+			fprint(2, "serial: unknown intr message: reqtype:%02x notify:%02x len:%d state:%02x\n", notify->bmRequestType, notify->bNotification, notify->wLength, notify->data[0]);
+		}
+	}
+
+	fprint(2, "serial: statusreader exiting: %r\n");
+	closedev(p->s->dev);
+}
+
+static int
+cdcinit(Serialport *p)
+{
+	p->baud = 115200;
+	p->bits = 8;
+	p->parity = 0;
+	p->stop = 1;
+	cdcsetparam(p);
+
+	incref(p->s->dev);
+	proccreate(statusreader, p, 8*1024);
+
+	return 0;
+}
+
+static int
+cdcsetbreak(Serialport *p, int val)
+{
+	if(p->hasbreak == 0)
+		return 0;
+
+	return usbcmd(p->s->dev, Rh2d | Rclass | Riface, SetBreak, (val != 0? 0xffff: 0x0000), 0, nil, 0);
+}
+
+static int
+cdcsendlines(Serialport *p)
+{
+	if(p->rts)
+		p->ctlstate |= CtlRTS;
+	else
+		p->ctlstate &= ~CtlRTS;
+	if(p->dtr)
+		p->ctlstate |= CtlDTR;
+	else
+		p->ctlstate &= ~CtlDTR;
+
+	return usbcmd(p->s->dev, Rh2d | Rclass | Riface, ControlState, p->ctlstate, 0, nil, 0);
+}
+
+static int
+cdcclearpipes(Serialport *p)
+{
+	if(unstall(p->s->dev, p->epout, Eout) < 0)
+		dprint(2, "serial: unstall epout: %r\n");
+	if(unstall(p->s->dev, p->epin, Ein) < 0)
+		dprint(2, "serial: unstall epin: %r\n");
+	if(unstall(p->s->dev, p->epintr, Ein) < 0)
+		dprint(2, "serial: unstall epintr: %r\n");
+
+	return 0;
+}
+
+static int
+cdcwait4data(Serialport *p, uchar *data, int count)
+{
+	int n;
+
+	qunlock(p->s);
+	while ((n = read(p->epin->dfd, data, count)) == 0)
+		;
+	qlock(p->s);
+
+	return n;
+}
+
+static Serialops cdcops = {
+	.init		= cdcinit,
+	.setparam	= cdcsetparam,
+	.setbreak	= cdcsetbreak,
+	.sendlines	= cdcsendlines,
+	.clearpipes	= cdcclearpipes,
+	.wait4data	= cdcwait4data,
+};
+
+int
+cdcprobe(Serial *ser)
+{
+	int i, ifcs;
+	Usbdev *ud = ser->dev->usb;
+	Iface *iface;
+	ulong csp;
+
+	ifcs = 0;
+	if (ud->vid == Kyocera || ud->did == AHK3001V)
+		ifcs++;
+	else for (i = 0; i < Niface; i++) {
+		iface = ud->conf[0]->iface[i];
+		if (iface == nil)
+			continue;
+
+		csp = iface->csp;
+		if (Class(csp) == Clcomms && Subclass(csp) == ScACM && (Proto(csp) == AT || Proto(csp) == None)) {
+			ser->p[ifcs].ctl_ifc = i;
+			ser->p[ifcs].s = ser;
+			getcaps(&ser->p[ifcs]);
+			ifcs++;
+		}
+	}
+
+	if (ifcs == 0)
+		return -1;
+
+	ser->nifcs = ifcs;
+	ser->isacm = 1;
+	ser->outhdrsz = 0;
+	ser->hasepintr = 1;
+	ser->Serialops = cdcops;
+	return 0;
+}
diff -r 23bc1d0e2dd2 sys/src/cmd/nusb/serial/mkfile
--- a/sys/src/cmd/nusb/serial/mkfile	Tue Jun 09 12:23:24 2020 -0700
+++ b/sys/src/cmd/nusb/serial/mkfile	Thu Jul 30 21:06:29 2020 -0700
@@ -1,7 +1,7 @@
 </$objtype/mkfile
 
 TARG=serial
-OFILES=ftdi.$O serial.$O prolific.$O ucons.$O silabs.$O ch340.$O
+OFILES=ftdi.$O serial.$O prolific.$O ucons.$O silabs.$O ch340.$O cdc.$O
 HFILES=\
 	../lib/usb.h\
 	serial.h\
diff -r 23bc1d0e2dd2 sys/src/cmd/nusb/serial/serial.c
--- a/sys/src/cmd/nusb/serial/serial.c	Tue Jun 09 12:23:24 2020 -0700
+++ b/sys/src/cmd/nusb/serial/serial.c	Thu Jul 30 21:06:29 2020 -0700
@@ -624,23 +624,46 @@
 
 	epintr = epin = epout = -1;
 
-	/*
-	 * interfc 0 means start from the start which is equiv to
-	 * iterate through endpoints probably, could be done better
-	 */
-	eps = ser->dev->usb->conf[0]->iface[ifc]->ep;
+	if(ser->isacm){
+		eps = ser->dev->usb->conf[0]->iface[ser->p[ifc].ctl_ifc]->ep;
 
-	for(i = 0; i < Nep; i++){
-		if((ep = eps[i]) == nil)
-			continue;
-		if(ser->hasepintr && ep->type == Eintr &&
-		    ep->dir == Ein && epintr == -1)
-			epintr = ep->id;
-		if(ep->type == Ebulk){
-			if((ep->dir == Ein || ep->dir == Eboth) && epin == -1)
-				epin = ep->id;
-			if((ep->dir == Eout || ep->dir == Eboth) && epout == -1)
-				epout = ep->id;
+		for(i = 0; i < Nep; i++){
+			if((ep = eps[i]) == nil)
+				continue;
+			if(ep->type == Eintr && ep->dir == Ein && epintr == -1)
+				epintr = ep->id;
+		}
+
+		eps = ser->dev->usb->conf[0]->iface[ser->p[ifc].data_ifc]->ep;
+		for(i = 0; i < Nep; i++){
+			if((ep = eps[i]) == nil)
+				continue;
+			if(ep->type == Ebulk){
+				if((ep->dir == Ein || ep->dir == Eboth) && epin == -1)
+					epin = ep->id;
+				if((ep->dir == Eout || ep->dir == Eboth) && epout == -1)
+					epout = ep->id;
+			}
+		}
+	} else {
+		/*
+		 * interfc 0 means start from the start which is equiv to
+		 * iterate through endpoints probably, could be done better
+		 */
+		eps = ser->dev->usb->conf[0]->iface[ifc]->ep;
+
+		for(i = 0; i < Nep; i++){
+			if((ep = eps[i]) == nil)
+				continue;
+			if(ser->hasepintr && ep->type == Eintr &&
+			    ep->dir == Ein && epintr == -1)
+				epintr = ep->id;
+			if(ep->type == Ebulk){
+				if((ep->dir == Ein || ep->dir == Eboth) && epin == -1)
+					epin = ep->id;
+				if((ep->dir == Eout || ep->dir == Eboth) && epout == -1)
+					epout = ep->id;
+			}
 		}
 	}
 	dprint(2, "serial[%d]: ep ids: in %d out %d intr %d\n", ifc, epin, epout, epintr);
@@ -714,6 +737,7 @@
 extern int slprobe(Serial *ser);
 extern int chprobe(Serial *ser);
 extern int uconsprobe(Serial *ser);
+extern int cdcprobe(Serial *ser);
 
 void
 threadmain(int argc, char* argv[])
@@ -750,7 +774,8 @@
 	&& ftprobe(ser)
 	&& slprobe(ser)
 	&& plprobe(ser)
-	&& chprobe(ser))
+	&& chprobe(ser)
+	&& cdcprobe(ser))
 		sysfatal("no serial devices found");
 
 	for(i = 0; i < ser->nifcs; i++){
diff -r 23bc1d0e2dd2 sys/src/cmd/nusb/serial/serial.h
--- a/sys/src/cmd/nusb/serial/serial.h	Tue Jun 09 12:23:24 2020 -0700
+++ b/sys/src/cmd/nusb/serial/serial.h	Thu Jul 30 21:06:29 2020 -0700
@@ -60,6 +60,10 @@
 
 	int	interfc;	/* interfc on the device for ftdi */
 
+	int hasbreak;	/* CDC ACM may not have break */
+	int ctl_ifc;	/* epintr iface for CDC ACM */
+	int data_ifc;	/* epin/out iface for CDC ACM */
+
 	Channel *w4data;
 	Channel *gotdata;
 	int	ndata;
@@ -81,6 +85,7 @@
 
 	int	jtag;		/* index of jtag interface, -1 none */
 	int	nifcs;		/* # of serial interfaces, including JTAG */
+	int isacm; 		/* CDC ACM interfaces handled differently */
 	Serialport p[Maxifc];
 	int	maxrtrans;
 	int	maxwtrans;

Re: CDC ACM nusb/serial driver

[Eli Cohen]

[-- Attachment #1: Type: text/plain, Size: 93 bytes --]

I already noticed a few tiny changes. these flags should be set to 1
or 0, not the bit value

[-- Attachment #2: serial.patch --]
[-- Type: text/x-patch, Size: 9571 bytes --]

diff -r 23bc1d0e2dd2 sys/src/cmd/nusb/serial/cdc.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/sys/src/cmd/nusb/serial/cdc.c	Thu Jul 30 21:06:29 2020 -0700
@@ -0,0 +1,301 @@
+#include <u.h>
+#include <libc.h>
+#include <thread.h>
+#include <fcall.h>
+#include <9p.h>
+#include "usb.h"
+#include "serial.h"
+
+enum {
+	Kyocera		= 0x0482,
+	AHK3001V	= 0x0203,
+};
+
+enum {
+	ScACM	= 2,
+	AT		= 1,
+	None	= 0,
+	LineCoding	= 0x20,
+	SetBreak	= 0x23,
+	ControlState = 0x22,
+};
+
+enum {
+	ClassCDCData = 0x0a,
+	SubclassData = 0,
+};
+
+enum {
+	SubCDCCM	= 1,
+	SubCDCACM	= 2,
+	SubCDCUnion	= 6,
+};
+
+typedef struct raw_desc {
+	uchar bLength;
+	uchar bDescriptorType;
+	uchar bDescriptorSubtype;
+	uchar bbytes[1];
+} raw_desc;
+
+typedef struct cm_desc {
+	uchar bLength;
+	uchar bDescriptorType;
+	uchar bDescriptorSubtype;
+	uchar bmCapabilities;
+	uchar bDataInterface;
+} cm_desc;
+
+enum {
+	CMDoesCM	= 1,
+	CMOverData	= 2,
+};
+
+typedef struct acm_desc {
+	uchar bLength;
+	uchar bDescriptorType;
+	uchar bDescriptorSubtype;
+	uchar bmCapabilities;
+} acm_desc;
+
+enum {
+	ACMHasFeature	= 1,
+	ACMHasLine		= 2,
+	ACMHasBreak		= 4,
+	ACMHasNetwork	= 8,
+};
+
+typedef struct union_desc {
+	uchar bLength;
+	uchar bDescriptorType;
+	uchar bDescriptorSubtype;
+	uchar bMasterInterface;
+	uchar bSlaveInterface[1];
+} union_desc;
+
+typedef struct cdc_notify {
+	uchar bmRequestType;
+	uchar bNotification;
+	ushort wValue;
+	ushort wIndex;
+	ushort wLength;
+	uchar data[16];
+} cdc_notify;
+
+enum {
+	Notification = 0xa1,
+	SerialState = 0x20,
+};
+
+enum {
+	DcdStatus	= 0x01,
+	DsrStatus	= 0x02,
+	RingStatus	= 0x08,
+};
+
+static int
+cdcsetparam(Serialport *p)
+{
+	int res;
+	uchar vals[7];
+
+	PUT4(vals, p->baud);
+
+	vals[4] = 0;
+	if(p->stop == 1)
+		vals[4] = 0;
+	if(p->stop == 15)
+		vals[4] = 1;
+	if(p->stop == 2)
+		vals[4] = 2;
+
+	vals[5] = p->parity;
+
+	vals[6] = p->bits;
+
+	res = usbcmd(p->s->dev, Rh2d | Rclass | Riface, LineCoding, 0, 0, vals, 7);
+
+	return res;
+}
+
+void
+getcaps(Serialport *p)
+{
+	int i, j;
+	Desc *desc;
+	Iface *iface;
+	raw_desc ddesc;
+	cm_desc dcm;
+	acm_desc dacm;
+	union_desc dunion;
+	int cur_ifc;
+
+	for (i = 0; i < Niface; i++) {
+		iface = p->s->dev->usb->conf[0]->iface[i];
+		if (iface == nil)
+			continue;
+
+		cur_ifc = iface->id;
+		if (cur_ifc != p->ctl_ifc && Class(iface->csp) == ClassCDCData && Subclass(iface->csp) == SubclassData && p->data_ifc == -1)
+			p->data_ifc = cur_ifc;
+
+		if (cur_ifc == p->ctl_ifc)
+			for (j = 0; j < Nddesc; j++) {
+				desc = p->s->dev->usb->ddesc[j];
+				if (desc == nil)
+					continue;
+
+				ddesc = (raw_desc)desc->data;
+				if (ddesc.bDescriptorType == Dfunction) {
+					switch (ddesc.bDescriptorSubtype) {
+					case SubCDCCM:
+						dcm = (cm_desc)ddesc;
+						p->data_ifc = dcm.bDataInterface;
+						break;
+					case SubCDCACM:
+						dacm = (acm_desc)ddesc;
+						p->hasbreak = dacm.bmCapabilities & ACMHasBreak? 1: 0;
+						break;
+					case SubCDCUnion:
+						dunion = (union_desc)ddesc;
+						p->data_ifc = dunion.bSlaveInterface[0];
+						break;
+					}
+				}
+			}
+	}
+}
+
+void
+statusreader(void *u)
+{
+	Serialport *p = u;
+	uchar buf[sizeof(cdc_notify)];
+	cdc_notify *notify;
+
+	notify = (cdc_notify*)buf;
+	threadsetname("statusreaderproc");
+
+	while(read(p->epintr->dfd, buf, sizeof(cdc_notify)) > 0) {
+		if (notify->bmRequestType == Notification && notify->bNotification == SerialState && notify->wLength == 2) {
+			p->dcd = notify->data[0] & DcdStatus? 1: 0;
+			p->dsr = notify->data[0] & DsrStatus? 1: 0;
+			p->ring = notify->data[0] & RingStatus? 1: 0;
+		} else {
+			fprint(2, "serial: unknown intr message: reqtype:%02x notify:%02x len:%d state:%02x\n", notify->bmRequestType, notify->bNotification, notify->wLength, notify->data[0]);
+		}
+	}
+
+	fprint(2, "serial: statusreader exiting: %r\n");
+	closedev(p->s->dev);
+}
+
+static int
+cdcinit(Serialport *p)
+{
+	p->baud = 115200;
+	p->bits = 8;
+	p->parity = 0;
+	p->stop = 1;
+	cdcsetparam(p);
+
+	incref(p->s->dev);
+	proccreate(statusreader, p, 8*1024);
+
+	return 0;
+}
+
+static int
+cdcsetbreak(Serialport *p, int val)
+{
+	if(p->hasbreak == 0)
+		return 0;
+
+	return usbcmd(p->s->dev, Rh2d | Rclass | Riface, SetBreak, (val != 0? 0xffff: 0x0000), 0, nil, 0);
+}
+
+static int
+cdcsendlines(Serialport *p)
+{
+	if(p->rts)
+		p->ctlstate |= CtlRTS;
+	else
+		p->ctlstate &= ~CtlRTS;
+	if(p->dtr)
+		p->ctlstate |= CtlDTR;
+	else
+		p->ctlstate &= ~CtlDTR;
+
+	return usbcmd(p->s->dev, Rh2d | Rclass | Riface, ControlState, p->ctlstate, 0, nil, 0);
+}
+
+static int
+cdcclearpipes(Serialport *p)
+{
+	if(unstall(p->s->dev, p->epout, Eout) < 0)
+		dprint(2, "serial: unstall epout: %r\n");
+	if(unstall(p->s->dev, p->epin, Ein) < 0)
+		dprint(2, "serial: unstall epin: %r\n");
+	if(unstall(p->s->dev, p->epintr, Ein) < 0)
+		dprint(2, "serial: unstall epintr: %r\n");
+
+	return 0;
+}
+
+static int
+cdcwait4data(Serialport *p, uchar *data, int count)
+{
+	int n;
+
+	qunlock(p->s);
+	while ((n = read(p->epin->dfd, data, count)) == 0)
+		;
+	qlock(p->s);
+
+	return n;
+}
+
+static Serialops cdcops = {
+	.init		= cdcinit,
+	.setparam	= cdcsetparam,
+	.setbreak	= cdcsetbreak,
+	.sendlines	= cdcsendlines,
+	.clearpipes	= cdcclearpipes,
+	.wait4data	= cdcwait4data,
+};
+
+int
+cdcprobe(Serial *ser)
+{
+	int i, ifcs;
+	Usbdev *ud = ser->dev->usb;
+	Iface *iface;
+	ulong csp;
+
+	ifcs = 0;
+	if (ud->vid == Kyocera || ud->did == AHK3001V)
+		ifcs++;
+	else for (i = 0; i < Niface; i++) {
+		iface = ud->conf[0]->iface[i];
+		if (iface == nil)
+			continue;
+
+		csp = iface->csp;
+		if (Class(csp) == Clcomms && Subclass(csp) == ScACM && (Proto(csp) == AT || Proto(csp) == None)) {
+			ser->p[ifcs].ctl_ifc = i;
+			ser->p[ifcs].s = ser;
+			getcaps(&ser->p[ifcs]);
+			ifcs++;
+		}
+	}
+
+	if (ifcs == 0)
+		return -1;
+
+	ser->nifcs = ifcs;
+	ser->isacm = 1;
+	ser->outhdrsz = 0;
+	ser->hasepintr = 1;
+	ser->Serialops = cdcops;
+	return 0;
+}
diff -r 23bc1d0e2dd2 sys/src/cmd/nusb/serial/mkfile
--- a/sys/src/cmd/nusb/serial/mkfile	Tue Jun 09 12:23:24 2020 -0700
+++ b/sys/src/cmd/nusb/serial/mkfile	Thu Jul 30 21:06:29 2020 -0700
@@ -1,7 +1,7 @@
 </$objtype/mkfile
 
 TARG=serial
-OFILES=ftdi.$O serial.$O prolific.$O ucons.$O silabs.$O ch340.$O
+OFILES=ftdi.$O serial.$O prolific.$O ucons.$O silabs.$O ch340.$O cdc.$O
 HFILES=\
 	../lib/usb.h\
 	serial.h\
diff -r 23bc1d0e2dd2 sys/src/cmd/nusb/serial/serial.c
--- a/sys/src/cmd/nusb/serial/serial.c	Tue Jun 09 12:23:24 2020 -0700
+++ b/sys/src/cmd/nusb/serial/serial.c	Thu Jul 30 21:06:29 2020 -0700
@@ -624,23 +624,46 @@
 
 	epintr = epin = epout = -1;
 
-	/*
-	 * interfc 0 means start from the start which is equiv to
-	 * iterate through endpoints probably, could be done better
-	 */
-	eps = ser->dev->usb->conf[0]->iface[ifc]->ep;
+	if(ser->isacm){
+		eps = ser->dev->usb->conf[0]->iface[ser->p[ifc].ctl_ifc]->ep;
 
-	for(i = 0; i < Nep; i++){
-		if((ep = eps[i]) == nil)
-			continue;
-		if(ser->hasepintr && ep->type == Eintr &&
-		    ep->dir == Ein && epintr == -1)
-			epintr = ep->id;
-		if(ep->type == Ebulk){
-			if((ep->dir == Ein || ep->dir == Eboth) && epin == -1)
-				epin = ep->id;
-			if((ep->dir == Eout || ep->dir == Eboth) && epout == -1)
-				epout = ep->id;
+		for(i = 0; i < Nep; i++){
+			if((ep = eps[i]) == nil)
+				continue;
+			if(ep->type == Eintr && ep->dir == Ein && epintr == -1)
+				epintr = ep->id;
+		}
+
+		eps = ser->dev->usb->conf[0]->iface[ser->p[ifc].data_ifc]->ep;
+		for(i = 0; i < Nep; i++){
+			if((ep = eps[i]) == nil)
+				continue;
+			if(ep->type == Ebulk){
+				if((ep->dir == Ein || ep->dir == Eboth) && epin == -1)
+					epin = ep->id;
+				if((ep->dir == Eout || ep->dir == Eboth) && epout == -1)
+					epout = ep->id;
+			}
+		}
+	} else {
+		/*
+		 * interfc 0 means start from the start which is equiv to
+		 * iterate through endpoints probably, could be done better
+		 */
+		eps = ser->dev->usb->conf[0]->iface[ifc]->ep;
+
+		for(i = 0; i < Nep; i++){
+			if((ep = eps[i]) == nil)
+				continue;
+			if(ser->hasepintr && ep->type == Eintr &&
+			    ep->dir == Ein && epintr == -1)
+				epintr = ep->id;
+			if(ep->type == Ebulk){
+				if((ep->dir == Ein || ep->dir == Eboth) && epin == -1)
+					epin = ep->id;
+				if((ep->dir == Eout || ep->dir == Eboth) && epout == -1)
+					epout = ep->id;
+			}
 		}
 	}
 	dprint(2, "serial[%d]: ep ids: in %d out %d intr %d\n", ifc, epin, epout, epintr);
@@ -714,6 +737,7 @@
 extern int slprobe(Serial *ser);
 extern int chprobe(Serial *ser);
 extern int uconsprobe(Serial *ser);
+extern int cdcprobe(Serial *ser);
 
 void
 threadmain(int argc, char* argv[])
@@ -750,7 +774,8 @@
 	&& ftprobe(ser)
 	&& slprobe(ser)
 	&& plprobe(ser)
-	&& chprobe(ser))
+	&& chprobe(ser)
+	&& cdcprobe(ser))
 		sysfatal("no serial devices found");
 
 	for(i = 0; i < ser->nifcs; i++){
diff -r 23bc1d0e2dd2 sys/src/cmd/nusb/serial/serial.h
--- a/sys/src/cmd/nusb/serial/serial.h	Tue Jun 09 12:23:24 2020 -0700
+++ b/sys/src/cmd/nusb/serial/serial.h	Thu Jul 30 21:06:29 2020 -0700
@@ -60,6 +60,10 @@
 
 	int	interfc;	/* interfc on the device for ftdi */
 
+	int hasbreak;	/* CDC ACM may not have break */
+	int ctl_ifc;	/* epintr iface for CDC ACM */
+	int data_ifc;	/* epin/out iface for CDC ACM */
+
 	Channel *w4data;
 	Channel *gotdata;
 	int	ndata;
@@ -81,6 +85,7 @@
 
 	int	jtag;		/* index of jtag interface, -1 none */
 	int	nifcs;		/* # of serial interfaces, including JTAG */
+	int isacm; 		/* CDC ACM interfaces handled differently */
 	Serialport p[Maxifc];
 	int	maxrtrans;
 	int	maxwtrans;

Re: [9front] Re: CDC ACM nusb/serial driver

[ori]

Nice work! Haven't read in depth, but at first glance, the code
looks pretty nice.

Re: [9front] CDC ACM nusb/serial driver

[Ethan Gardener]

On Thu, Jul 30, 2020, at 10:18 PM, Eli Cohen wrote:
> 
> I'm mounting 9p over the
> serial port and it's flaky, it eventually screws up.

i remember being told you can't use 9p over serial directly, you need another layer. i can't clearly remember what that layer should do, but i think it was preserve message boundaries. this was all long ago, before 9front.

Re: [9front] CDC ACM nusb/serial driver

[Anthony Martin]

Ethan Gardener <eekee57@fastmail.fm> once said:
> i remember being told you can't use 9p over serial directly, you need
> another layer. i can't clearly remember what that layer should do,
> but i think it was preserve message boundaries. this was all long ago,
> before 9front.

Before the 9P2000 rewrite, the kernel required the underlying transport
protocol to preserve message boundaries. You needed a little something
extra between 9P and TCP, for example, but not for IL.

To quote Charles, "all you need is a transport protocol that reliably
preserves content and order".

Cheers,
  Anthony

Re: [9front] CDC ACM nusb/serial driver

[Ethan Gardener]

On Sat, Aug 1, 2020, at 6:35 AM, Anthony Martin wrote:
> Ethan Gardener <eekee57@fastmail.fm> once said:
> > i remember being told you can't use 9p over serial directly, you need
> > another layer. i can't clearly remember what that layer should do,
> > but i think it was preserve message boundaries. this was all long ago,
> > before 9front.
> 
> Before the 9P2000 rewrite, the kernel required the underlying transport
> protocol to preserve message boundaries. You needed a little something
> extra between 9P and TCP, for example, but not for IL.
> 
> To quote Charles, "all you need is a transport protocol that reliably
> preserves content and order".

Ah, that's good to know. Thanks.

Re: [9front] CDC ACM nusb/serial driver

[Eli Cohen]

[-- Attachment #1: Type: text/plain, Size: 203 bytes --]

cinap -
here is a new patch addressing your annotations

Ethan and Anthony -
I remember the same or a very similar conversation. interestingly, I
have had no problems mounting 9P over serial on Linux...

[-- Attachment #2: serial.patch --]
[-- Type: text/x-patch, Size: 8242 bytes --]

diff -r 23bc1d0e2dd2 sys/src/cmd/nusb/serial/cdc.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/sys/src/cmd/nusb/serial/cdc.c	Sat Aug 01 16:37:56 2020 -0700
@@ -0,0 +1,276 @@
+#include <u.h>
+#include <libc.h>
+#include <thread.h>
+#include <fcall.h>
+#include <9p.h>
+#include "usb.h"
+#include "serial.h"
+
+enum {
+	Kyocera		= 0x0482,
+	AHK3001V	= 0x0203,
+};
+
+enum {
+	ScACM	= 2,
+	AT		= 1,
+	None	= 0,
+};
+
+enum {
+	LineCoding	= 0x20,
+	SetBreak	= 0x23,
+	ControlState = 0x22,
+};
+
+enum {
+	ClassCDCData = 0x0a,
+	SubclassData = 0,
+};
+
+enum {
+	DescSubCDCCM	= 1,
+	DescSubCDCACM	= 2,
+	DescSubCDCUnion	= 6,
+};
+
+enum {
+	CMDoesCM	= 1,
+	CMOverData	= 2,
+};
+
+enum {
+	ACMHasFeature	= 1,
+	ACMHasLine		= 2,
+	ACMHasBreak		= 4,
+	ACMHasNetwork	= 8,
+};
+
+typedef struct CDCNotify {
+	uchar bmRequestType;
+	uchar bNotification;
+	ushort wValue;
+	ushort wIndex;
+	ushort wLength;
+	uchar data[16];
+} CDCNotify;
+
+enum {
+	Notification = 0xa1,
+	SerialState = 0x20,
+};
+
+enum {
+	DcdStatus	= 0x01,
+	DsrStatus	= 0x02,
+	BreakStatus	= 0x04,
+	RingStatus	= 0x08,
+};
+
+static int
+cdcsetparam(Serialport *p)
+{
+	int res;
+	uchar vals[7];
+
+	PUT4(vals, p->baud);
+
+	vals[4] = 0;
+	if(p->stop == 1)
+		vals[4] = 0;
+	if(p->stop == 15)
+		vals[4] = 1;
+	if(p->stop == 2)
+		vals[4] = 2;
+
+	vals[5] = p->parity;
+
+	vals[6] = p->bits;
+
+	res = usbcmd(p->s->dev, Rh2d | Rclass | Riface, LineCoding, 0, p->ctl_ifc, vals, 7);
+
+	return res;
+}
+
+void
+getcaps(Serialport *p)
+{
+	int i, j;
+	Desc *desc;
+	Iface *iface;
+	DDesc *ddesc;
+	int cur_ifc;
+
+	for (i = 0; i < Niface; i++) {
+		iface = p->s->dev->usb->conf[0]->iface[i];
+		if (iface == nil)
+			continue;
+
+		cur_ifc = iface->id;
+		if (cur_ifc != p->ctl_ifc && Class(iface->csp) == ClassCDCData && Subclass(iface->csp) == SubclassData && p->data_ifc == -1)
+			p->data_ifc = cur_ifc;
+
+		if (cur_ifc == p->ctl_ifc) {
+			for (j = 0; j < Nddesc; j++) {
+				desc = p->s->dev->usb->ddesc[j];
+				if (desc == nil)
+					continue;
+
+				ddesc = &desc->data;
+				if (ddesc->bDescriptorType == Dfunction) {
+					switch (ddesc->bbytes[0]) {
+					case DescSubCDCCM:
+						p->data_ifc = ddesc->bbytes[2];
+						break;
+					case DescSubCDCACM:
+						p->hasbreak = ddesc->bbytes[1] & ACMHasBreak? 1: 0;
+						break;
+					case DescSubCDCUnion:
+						p->data_ifc = ddesc->bbytes[2];
+						break;
+					}
+				}
+			}
+		}
+	}
+}
+
+void
+statusreader(void *u)
+{
+	Serialport *p = u;
+	uchar buf[24];
+	CDCNotify notify;
+
+	threadsetname("statusreaderproc");
+
+	while(readn(p->epintr->dfd, buf, sizeof(buf)) == sizeof(buf)) {
+		notify.bmRequestType = buf[0];
+		notify.bNotification = buf[1];
+		notify.wValue = buf[2] | (buf[3] << 8);
+		notify.wIndex = buf[4] | (buf[5] << 8);
+		notify.wLength = buf[6] | (buf[7] << 8);
+		memcpy(notify.data, &buf[8], 16);
+
+		if (notify.bmRequestType == Notification && notify.bNotification == SerialState && notify.wLength == 2) {
+			p->dcd = notify.data[0] & DcdStatus? 1: 0;
+			p->dsr = notify.data[0] & DsrStatus? 1: 0;
+			p->ring = notify.data[0] & RingStatus? 1: 0;
+		} else {
+			fprint(2, "serial: unknown intr message: reqtype:%02x notify:%02x len:%d state:%02x\n", notify.bmRequestType, notify.bNotification, notify.wLength, notify.data[0]);
+		}
+	}
+
+	fprint(2, "serial: statusreader exiting: %r\n");
+	closedev(p->s->dev);
+}
+
+static int
+cdcinit(Serialport *p)
+{
+	p->baud = 115200;
+	p->bits = 8;
+	p->parity = 0;
+	p->stop = 1;
+	cdcsetparam(p);
+
+	incref(p->s->dev);
+	proccreate(statusreader, p, 8*1024);
+
+	return 0;
+}
+
+static int
+cdcsetbreak(Serialport *p, int val)
+{
+	if(p->hasbreak == 0)
+		return 0;
+
+	return usbcmd(p->s->dev, Rh2d | Rclass | Riface, SetBreak, (val != 0? 0xffff: 0x0000), p->ctl_ifc, nil, 0);
+}
+
+static int
+cdcsendlines(Serialport *p)
+{
+	if(p->rts)
+		p->ctlstate |= CtlRTS;
+	else
+		p->ctlstate &= ~CtlRTS;
+	if(p->dtr)
+		p->ctlstate |= CtlDTR;
+	else
+		p->ctlstate &= ~CtlDTR;
+
+	return usbcmd(p->s->dev, Rh2d | Rclass | Riface, ControlState, p->ctlstate, p->ctl_ifc, nil, 0);
+}
+
+static int
+cdcclearpipes(Serialport *p)
+{
+	if(unstall(p->s->dev, p->epout, Eout) < 0)
+		dprint(2, "serial: unstall epout: %r\n");
+	if(unstall(p->s->dev, p->epin, Ein) < 0)
+		dprint(2, "serial: unstall epin: %r\n");
+	if(unstall(p->s->dev, p->epintr, Ein) < 0)
+		dprint(2, "serial: unstall epintr: %r\n");
+
+	return 0;
+}
+
+static int
+cdcwait4data(Serialport *p, uchar *data, int count)
+{
+	int n;
+
+	qunlock(p->s);
+	while ((n = read(p->epin->dfd, data, count)) == 0)
+		;
+	qlock(p->s);
+
+	return n;
+}
+
+static Serialops cdcops = {
+	.init		= cdcinit,
+	.setparam	= cdcsetparam,
+	.setbreak	= cdcsetbreak,
+	.sendlines	= cdcsendlines,
+	.clearpipes	= cdcclearpipes,
+	.wait4data	= cdcwait4data,
+};
+
+int
+cdcprobe(Serial *ser)
+{
+	int i, ifcs;
+	Usbdev *ud = ser->dev->usb;
+	Iface *iface;
+	ulong csp;
+
+	ifcs = 0;
+	if (ud->vid == Kyocera || ud->did == AHK3001V)
+		ifcs++;
+	else for (i = 0; i < Niface; i++) {
+		iface = ud->conf[0]->iface[i];
+		if (iface == nil)
+			continue;
+
+		csp = iface->csp;
+		if (Class(csp) == Clcomms && Subclass(csp) == ScACM && (Proto(csp) == AT || Proto(csp) == None)) {
+			ser->p[ifcs].ctl_ifc = i;
+			ser->p[ifcs].s = ser;
+			getcaps(&ser->p[ifcs]);
+			ifcs++;
+		}
+	}
+
+	if (ifcs == 0)
+		return -1;
+
+	ser->nifcs = ifcs;
+	ser->isacm = 1;
+	ser->outhdrsz = 0;
+	ser->hasepintr = 1;
+	ser->Serialops = cdcops;
+	return 0;
+}
diff -r 23bc1d0e2dd2 sys/src/cmd/nusb/serial/mkfile
--- a/sys/src/cmd/nusb/serial/mkfile	Tue Jun 09 12:23:24 2020 -0700
+++ b/sys/src/cmd/nusb/serial/mkfile	Sat Aug 01 16:37:56 2020 -0700
@@ -1,7 +1,7 @@
 </$objtype/mkfile
 
 TARG=serial
-OFILES=ftdi.$O serial.$O prolific.$O ucons.$O silabs.$O ch340.$O
+OFILES=ftdi.$O serial.$O prolific.$O ucons.$O silabs.$O ch340.$O cdc.$O
 HFILES=\
 	../lib/usb.h\
 	serial.h\
diff -r 23bc1d0e2dd2 sys/src/cmd/nusb/serial/serial.c
--- a/sys/src/cmd/nusb/serial/serial.c	Tue Jun 09 12:23:24 2020 -0700
+++ b/sys/src/cmd/nusb/serial/serial.c	Sat Aug 01 16:37:56 2020 -0700
@@ -624,18 +624,32 @@
 
 	epintr = epin = epout = -1;
 
+	if(ser->isacm){
+		eps = ser->dev->usb->conf[0]->iface[ser->p[ifc].ctl_ifc]->ep;
+
+		for(i = 0; i < Nep; i++){
+			if((ep = eps[i]) == nil)
+				continue;
+			if(ep->type == Eintr && ep->dir == Ein && epintr == -1)
+				epintr = ep->id;
+		}
+	}
+
 	/*
 	 * interfc 0 means start from the start which is equiv to
 	 * iterate through endpoints probably, could be done better
 	 */
-	eps = ser->dev->usb->conf[0]->iface[ifc]->ep;
+	if (!ser->isacm)
+		eps = ser->dev->usb->conf[0]->iface[ifc]->ep;
+	else
+		eps = ser->dev->usb->conf[0]->iface[ser->p[ifc].data_ifc]->ep;
 
 	for(i = 0; i < Nep; i++){
 		if((ep = eps[i]) == nil)
 			continue;
-		if(ser->hasepintr && ep->type == Eintr &&
-		    ep->dir == Ein && epintr == -1)
-			epintr = ep->id;
+		if(!ser->isacm && ser->hasepintr &&
+			ep->type == Eintr && ep->dir == Ein && epintr == -1)
+				epintr = ep->id;
 		if(ep->type == Ebulk){
 			if((ep->dir == Ein || ep->dir == Eboth) && epin == -1)
 				epin = ep->id;
@@ -714,6 +728,7 @@
 extern int slprobe(Serial *ser);
 extern int chprobe(Serial *ser);
 extern int uconsprobe(Serial *ser);
+extern int cdcprobe(Serial *ser);
 
 void
 threadmain(int argc, char* argv[])
@@ -750,7 +765,8 @@
 	&& ftprobe(ser)
 	&& slprobe(ser)
 	&& plprobe(ser)
-	&& chprobe(ser))
+	&& chprobe(ser)
+	&& cdcprobe(ser))
 		sysfatal("no serial devices found");
 
 	for(i = 0; i < ser->nifcs; i++){
diff -r 23bc1d0e2dd2 sys/src/cmd/nusb/serial/serial.h
--- a/sys/src/cmd/nusb/serial/serial.h	Tue Jun 09 12:23:24 2020 -0700
+++ b/sys/src/cmd/nusb/serial/serial.h	Sat Aug 01 16:37:56 2020 -0700
@@ -60,6 +60,10 @@
 
 	int	interfc;	/* interfc on the device for ftdi */
 
+	int	hasbreak;	/* CDC ACM may not have break */
+	int	ctl_ifc;	/* epintr iface for CDC ACM */
+	int	data_ifc;	/* epin/out iface for CDC ACM */
+
 	Channel *w4data;
 	Channel *gotdata;
 	int	ndata;
@@ -81,6 +85,7 @@
 
 	int	jtag;		/* index of jtag interface, -1 none */
 	int	nifcs;		/* # of serial interfaces, including JTAG */
+	int	isacm; 		/* CDC ACM interfaces handled differently */
 	Serialport p[Maxifc];
 	int	maxrtrans;
 	int	maxwtrans;

Re: [9front] CDC ACM nusb/serial driver

[Ethan Gardener]

On Sat, Aug 1, 2020, at 5:45 PM, Eli Cohen wrote:
> 
> Ethan and Anthony -
> I remember the same or a very similar conversation. interestingly, I
> have had no problems mounting 9P over serial on Linux...

I guess someone back then had forgotten about the 9P2000 changes. I of course remembered that conversation and not anything else.

Re: [9front] CDC ACM nusb/serial driver

[cinap_lenrek]

are you using hardware flow control?

--
cinap

Re: [9front] CDC ACM nusb/serial driver

[Eli Cohen]

I'm not sure... I was trying to just do everything the OpenBSD umodem
driver does: https://raw.githubusercontent.com/openbsd/src/master/sys/dev/usb/umodem.c

Re: [9front] CDC ACM nusb/serial driver

[Eli Cohen]

[-- Attachment #1: Type: text/plain, Size: 201 bytes --]

I was finally looking at this again today. it does use hardware flow
control, in the sendlines function.

it works well here, but I have only tested it with an arduino mega
2560 connected to an x230 t

[-- Attachment #2: serial.patch --]
[-- Type: text/x-patch, Size: 7721 bytes --]

diff -r bd503458654d sys/src/cmd/nusb/serial/cdc.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/sys/src/cmd/nusb/serial/cdc.c	Mon Oct 05 14:03:47 2020 -0700
@@ -0,0 +1,259 @@
+#include <u.h>
+#include <libc.h>
+#include <thread.h>
+#include <fcall.h>
+#include <9p.h>
+#include "usb.h"
+#include "serial.h"
+
+enum {
+	Kyocera		= 0x0482,
+	AHK3001V	= 0x0203,
+};
+
+enum {
+	ScACM	= 2,
+	AT		= 1,
+	None	= 0,
+};
+
+enum {
+	LineCoding	= 0x20,
+	SetBreak	= 0x23,
+	ControlState = 0x22,
+};
+
+enum {
+	ClassCDCData = 0x0a,
+	SubclassData = 0,
+};
+
+enum {
+	DescSubCDCCM	= 1,
+	DescSubCDCACM	= 2,
+	DescSubCDCUnion	= 6,
+};
+
+enum {
+	CMDoesCM	= 1,
+	CMOverData	= 2,
+};
+
+enum {
+	ACMHasFeature	= 1,
+	ACMHasLine		= 2,
+	ACMHasBreak		= 4,
+	ACMHasNetwork	= 8,
+};
+
+enum {
+	Notification = 0xa1,
+	SerialState = 0x20,
+};
+
+enum {
+	DcdStatus	= 0x01,
+	DsrStatus	= 0x02,
+	BreakStatus	= 0x04,
+	RingStatus	= 0x08,
+};
+
+static int
+cdcsetparam(Serialport *p)
+{
+	int res;
+	uchar vals[7];
+
+	PUT4(vals, p->baud);
+
+	vals[4] = 0;
+	if(p->stop == 1)
+		vals[4] = 0;
+	if(p->stop == 15)
+		vals[4] = 1;
+	if(p->stop == 2)
+		vals[4] = 2;
+
+	vals[5] = p->parity;
+
+	vals[6] = p->bits;
+
+	res = usbcmd(p->s->dev, Rh2d | Rclass | Riface, LineCoding, 0, p->ctl_ifc, vals, 7);
+
+	return res;
+}
+
+void
+getcaps(Serialport *p)
+{
+	int i, j;
+	Desc *desc;
+	Iface *iface;
+	DDesc *ddesc;
+	int cur_ifc;
+
+	for (i = 0; i < Niface; i++) {
+		iface = p->s->dev->usb->conf[0]->iface[i];
+		if (iface == nil)
+			continue;
+
+		cur_ifc = iface->id;
+		if (cur_ifc != p->ctl_ifc && Class(iface->csp) == ClassCDCData && Subclass(iface->csp) == SubclassData && p->data_ifc == -1)
+			p->data_ifc = cur_ifc;
+
+		if (cur_ifc == p->ctl_ifc) {
+			for (j = 0; j < Nddesc; j++) {
+				desc = p->s->dev->usb->ddesc[j];
+				if (desc == nil)
+					continue;
+
+				ddesc = &desc->data;
+				if (ddesc->bDescriptorType == Dfunction) {
+					switch (ddesc->bbytes[0]) {
+					case DescSubCDCCM:
+						p->data_ifc = ddesc->bbytes[2];
+						break;
+					case DescSubCDCACM:
+						p->hasbreak = ddesc->bbytes[1] & ACMHasBreak? 1: 0;
+						break;
+					case DescSubCDCUnion:
+						p->data_ifc = ddesc->bbytes[2];
+						break;
+					}
+				}
+			}
+		}
+	}
+}
+
+void
+statusreader(void *u)
+{
+	Serialport *p = u;
+	uchar buf[24];
+
+	threadsetname("statusreaderproc");
+
+	while(readn(p->epintr->dfd, buf, sizeof(buf)) == sizeof(buf)) {
+		if (buf[0] == Notification && buf[1] == SerialState && buf[6] == 2 && buf[7] == 0) {
+			p->dcd = buf[8] & DcdStatus? 1: 0;
+			p->dsr = buf[8] & DsrStatus? 1: 0;
+			p->ring = buf[8] & RingStatus? 1: 0;
+		} else {
+			fprint(2, "serial: unknown intr message: reqtype:%02x notify:%02x state:%02x\n", buf[0], buf[1], buf[8]);
+		}
+	}
+
+	fprint(2, "serial: statusreader exiting: %r\n");
+	closedev(p->s->dev);
+}
+
+static int
+cdcinit(Serialport *p)
+{
+	p->baud = 115200;
+	p->bits = 8;
+	p->parity = 0;
+	p->stop = 1;
+	cdcsetparam(p);
+
+	incref(p->s->dev);
+	proccreate(statusreader, p, 8*1024);
+
+	return 0;
+}
+
+static int
+cdcsetbreak(Serialport *p, int val)
+{
+	if(p->hasbreak == 0)
+		return 0;
+
+	return usbcmd(p->s->dev, Rh2d | Rclass | Riface, SetBreak, (val != 0? 0xffff: 0x0000), p->ctl_ifc, nil, 0);
+}
+
+static int
+cdcsendlines(Serialport *p)
+{
+	if(p->rts)
+		p->ctlstate |= CtlRTS;
+	else
+		p->ctlstate &= ~CtlRTS;
+	if(p->dtr)
+		p->ctlstate |= CtlDTR;
+	else
+		p->ctlstate &= ~CtlDTR;
+
+	return usbcmd(p->s->dev, Rh2d | Rclass | Riface, ControlState, p->ctlstate, p->ctl_ifc, nil, 0);
+}
+
+static int
+cdcclearpipes(Serialport *p)
+{
+	if(unstall(p->s->dev, p->epout, Eout) < 0)
+		dprint(2, "serial: unstall epout: %r\n");
+	if(unstall(p->s->dev, p->epin, Ein) < 0)
+		dprint(2, "serial: unstall epin: %r\n");
+	if(unstall(p->s->dev, p->epintr, Ein) < 0)
+		dprint(2, "serial: unstall epintr: %r\n");
+
+	return 0;
+}
+
+static int
+cdcwait4data(Serialport *p, uchar *data, int count)
+{
+	int n;
+
+	qunlock(p->s);
+	while ((n = read(p->epin->dfd, data, count)) == 0)
+		;
+	qlock(p->s);
+
+	return n;
+}
+
+static Serialops cdcops = {
+	.init		= cdcinit,
+	.setparam	= cdcsetparam,
+	.setbreak	= cdcsetbreak,
+	.sendlines	= cdcsendlines,
+	.clearpipes	= cdcclearpipes,
+	.wait4data	= cdcwait4data,
+};
+
+int
+cdcprobe(Serial *ser)
+{
+	int i, ifcs;
+	Usbdev *ud = ser->dev->usb;
+	Iface *iface;
+	ulong csp;
+
+	ifcs = 0;
+	if (ud->vid == Kyocera || ud->did == AHK3001V)
+		ifcs++;
+	else for (i = 0; i < Niface; i++) {
+		iface = ud->conf[0]->iface[i];
+		if (iface == nil)
+			continue;
+
+		csp = iface->csp;
+		if (Class(csp) == Clcomms && Subclass(csp) == ScACM && (Proto(csp) == AT || Proto(csp) == None)) {
+			ser->p[ifcs].ctl_ifc = i;
+			ser->p[ifcs].s = ser;
+			getcaps(&ser->p[ifcs]);
+			ifcs++;
+		}
+	}
+
+	if (ifcs == 0)
+		return -1;
+
+	ser->nifcs = ifcs;
+	ser->isacm = 1;
+	ser->outhdrsz = 0;
+	ser->hasepintr = 1;
+	ser->Serialops = cdcops;
+	return 0;
+}
diff -r bd503458654d sys/src/cmd/nusb/serial/mkfile
--- a/sys/src/cmd/nusb/serial/mkfile	Sun Oct 04 22:45:22 2020 +0200
+++ b/sys/src/cmd/nusb/serial/mkfile	Mon Oct 05 14:03:47 2020 -0700
@@ -1,7 +1,7 @@
 </$objtype/mkfile
 
 TARG=serial
-OFILES=ftdi.$O serial.$O prolific.$O ucons.$O silabs.$O ch340.$O
+OFILES=ftdi.$O serial.$O prolific.$O ucons.$O silabs.$O ch340.$O cdc.$O
 HFILES=\
 	../lib/usb.h\
 	serial.h\
diff -r bd503458654d sys/src/cmd/nusb/serial/serial.c
--- a/sys/src/cmd/nusb/serial/serial.c	Sun Oct 04 22:45:22 2020 +0200
+++ b/sys/src/cmd/nusb/serial/serial.c	Mon Oct 05 14:03:47 2020 -0700
@@ -624,18 +624,32 @@
 
 	epintr = epin = epout = -1;
 
+	if(ser->isacm){
+		eps = ser->dev->usb->conf[0]->iface[ser->p[ifc].ctl_ifc]->ep;
+
+		for(i = 0; i < Nep; i++){
+			if((ep = eps[i]) == nil)
+				continue;
+			if(ep->type == Eintr && ep->dir == Ein && epintr == -1)
+				epintr = ep->id;
+		}
+	}
+
 	/*
 	 * interfc 0 means start from the start which is equiv to
 	 * iterate through endpoints probably, could be done better
 	 */
-	eps = ser->dev->usb->conf[0]->iface[ifc]->ep;
+	if (!ser->isacm)
+		eps = ser->dev->usb->conf[0]->iface[ifc]->ep;
+	else
+		eps = ser->dev->usb->conf[0]->iface[ser->p[ifc].data_ifc]->ep;
 
 	for(i = 0; i < Nep; i++){
 		if((ep = eps[i]) == nil)
 			continue;
-		if(ser->hasepintr && ep->type == Eintr &&
-		    ep->dir == Ein && epintr == -1)
-			epintr = ep->id;
+		if(!ser->isacm && ser->hasepintr &&
+			ep->type == Eintr && ep->dir == Ein && epintr == -1)
+				epintr = ep->id;
 		if(ep->type == Ebulk){
 			if((ep->dir == Ein || ep->dir == Eboth) && epin == -1)
 				epin = ep->id;
@@ -714,6 +728,7 @@
 extern int slprobe(Serial *ser);
 extern int chprobe(Serial *ser);
 extern int uconsprobe(Serial *ser);
+extern int cdcprobe(Serial *ser);
 
 void
 threadmain(int argc, char* argv[])
@@ -750,7 +765,8 @@
 	&& ftprobe(ser)
 	&& slprobe(ser)
 	&& plprobe(ser)
-	&& chprobe(ser))
+	&& chprobe(ser)
+	&& cdcprobe(ser))
 		sysfatal("no serial devices found");
 
 	for(i = 0; i < ser->nifcs; i++){
diff -r bd503458654d sys/src/cmd/nusb/serial/serial.h
--- a/sys/src/cmd/nusb/serial/serial.h	Sun Oct 04 22:45:22 2020 +0200
+++ b/sys/src/cmd/nusb/serial/serial.h	Mon Oct 05 14:03:47 2020 -0700
@@ -60,6 +60,10 @@
 
 	int	interfc;	/* interfc on the device for ftdi */
 
+	int	hasbreak;	/* CDC ACM may not have break */
+	int	ctl_ifc;	/* epintr iface for CDC ACM */
+	int	data_ifc;	/* epin/out iface for CDC ACM */
+
 	Channel *w4data;
 	Channel *gotdata;
 	int	ndata;
@@ -81,6 +85,7 @@
 
 	int	jtag;		/* index of jtag interface, -1 none */
 	int	nifcs;		/* # of serial interfaces, including JTAG */
+	int	isacm; 		/* CDC ACM interfaces handled differently */
 	Serialport p[Maxifc];
 	int	maxrtrans;
 	int	maxwtrans;

Re: [9front] CDC ACM nusb/serial driver

[cinap_lenrek]

here goes some annotations for problems:

diff -r 23bc1d0e2dd2 sys/src/cmd/nusb/serial/cdc.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/sys/src/cmd/nusb/serial/cdc.c	Thu Jul 30 21:06:29 2020 -0700
@@ -0,0 +1,301 @@
+#include <u.h>
+#include <libc.h>
+#include <thread.h>
+#include <fcall.h>
+#include <9p.h>
+#include "usb.h"
+#include "serial.h"
+
+enum {
+	Kyocera		= 0x0482,
+	AHK3001V	= 0x0203,
+};
+
+enum {
+	ScACM	= 2,
+	AT		= 1,
+	None	= 0,
+	LineCoding	= 0x20,
+	SetBreak	= 0x23,
+	ControlState = 0x22,
+};
+
+enum {
+	ClassCDCData = 0x0a,
+	SubclassData = 0,
+};
+
+enum {
+	SubCDCCM	= 1,
+	SubCDCACM	= 2,
+	SubCDCUnion	= 6,
+};


these seem quite generic, is it worth dealing
with a struct here for a 3 byte header?

+typedef struct raw_desc {
+	uchar bLength;
+	uchar bDescriptorType;
+	uchar bDescriptorSubtype;
+	uchar bbytes[1];
+} raw_desc;
+

this thing is essentially just, 2 bytes:

+typedef struct cm_desc {
+	uchar bLength;
+	uchar bDescriptorType;
+	uchar bDescriptorSubtype;
+	uchar bmCapabilities;
+	uchar bDataInterface;
+} cm_desc;
+
+enum {
+	CMDoesCM	= 1,
+	CMOverData	= 2,
+};

this thing one one byte:

+typedef struct acm_desc {
+	uchar bLength;
+	uchar bDescriptorType;
+	uchar bDescriptorSubtype;
+	uchar bmCapabilities;
+} acm_desc;
+
+enum {
+	ACMHasFeature	= 1,
+	ACMHasLine		= 2,
+	ACMHasBreak		= 4,
+	ACMHasNetwork	= 8,
+};

this thing is two byte:

+typedef struct union_desc {
+	uchar bLength;
+	uchar bDescriptorType;
+	uchar bDescriptorSubtype;
+	uchar bMasterInterface;
+	uchar bSlaveInterface[1];
+} union_desc;

first interesting stuct, but you can't use it like you do.

+typedef struct cdc_notify {
+	uchar bmRequestType;
+	uchar bNotification;
+	ushort wValue;
+	ushort wIndex;
+	ushort wLength;
+	uchar data[16];
+} cdc_notify;

here we might have a problem, you read this structure directly
using sizeof(). wValue, WIndex and wLength are 2 bytes, so this
only works on little endian machines. also, the compiler might
pad your structure resulting in wrong size and field offsets.
you need to read the data as bytes, and then parse it... or
use only byte arrays and a constant for the binary length
instead of sizeof(). and for acccess the two byte fields,
you shift and or the two bytes.

+enum {
+	Notification = 0xa1,
+	SerialState = 0x20,
+};
+
+enum {
+	DcdStatus	= 0x01,
+	DsrStatus	= 0x02,
+	RingStatus	= 0x08,
+};
+
+static int
+cdcsetparam(Serialport *p)
+{
+	int res;
+	uchar vals[7];
+
+	PUT4(vals, p->baud);
+
+	vals[4] = 0;
+	if(p->stop == 1)
+		vals[4] = 0;
+	if(p->stop == 15)
+		vals[4] = 1;
+	if(p->stop == 2)
+		vals[4] = 2;
+
+	vals[5] = p->parity;
+
+	vals[6] = p->bits;
+
+	res = usbcmd(p->s->dev, Rh2d | Rclass | Riface, LineCoding, 0, 0, vals, 7);
+
+	return res;
+}
+
+void
+getcaps(Serialport *p)
+{
+	int i, j;
+	Desc *desc;
+	Iface *iface;
+	raw_desc ddesc;
+	cm_desc dcm;
+	acm_desc dacm;
+	union_desc dunion;
+	int cur_ifc;
+
+	for (i = 0; i < Niface; i++) {
+		iface = p->s->dev->usb->conf[0]->iface[i];
+		if (iface == nil)
+			continue;
+
+		cur_ifc = iface->id;
+		if (cur_ifc != p->ctl_ifc && Class(iface->csp) == ClassCDCData && Subclass(iface->csp) == SubclassData && p->data_ifc == -1)
+			p->data_ifc = cur_ifc;
+
+		if (cur_ifc == p->ctl_ifc)
+			for (j = 0; j < Nddesc; j++) {
+				desc = p->s->dev->usb->ddesc[j];
+				if (desc == nil)
+					continue;
+
+				ddesc = (raw_desc)desc->data;

wtf, just cast the pointers, man. 

+				if (ddesc.bDescriptorType == Dfunction) {
+					switch (ddesc.bDescriptorSubtype) {
+					case SubCDCCM:
+						dcm = (cm_desc)ddesc;

no! dont copy the whole struct. this is just wrong.

+						p->data_ifc = dcm.bDataInterface;
+						break;
+					case SubCDCACM:
+						dacm = (acm_desc)ddesc;
+						p->hasbreak = dacm.bmCapabilities & ACMHasBreak;
+						break;
+					case SubCDCUnion:
+						dunion = (union_desc)ddesc;
+						p->data_ifc = dunion.bSlaveInterface[0];
+						break;
+					}
+				}
+			}
+	}
+}
+
+void
+statusreader(void *u)
+{
+	Serialport *p = u;
+	uchar buf[sizeof(cdc_notify)];
+	cdc_notify *notify;
+
+	notify = (cdc_notify*)buf;
+	threadsetname("statusreaderproc");
+

see comment above about the struct:

+	while(read(p->epintr->dfd, buf, sizeof(cdc_notify)) > 0) {
+		if (notify->bmRequestType == Notification && notify->bNotification == SerialState && notify->wLength == 2) {
+			p->dcd = notify->data[0] & DcdStatus;
+			p->dsr = notify->data[0] & DsrStatus;
+			p->ring = notify->data[0] & RingStatus;
+		} else {
+			fprint(2, "serial: unknown intr message: reqtype:%02x notify:%02x len:%d state:%02x\n", notify->bmRequestType, notify->bNotification, notify->wLength, notify->data[0]);
+		}
+	}
+
+	fprint(2, "serial: statusreader exiting: %r\n");
+	closedev(p->s->dev);
+}
+
+static int
+cdcinit(Serialport *p)
+{
+	p->baud = 115200;
+	p->bits = 8;
+	p->parity = 0;
+	p->stop = 1;
+	cdcsetparam(p);
+
+	incref(p->s->dev);
+	proccreate(statusreader, p, 8*1024);
+
+	return 0;
+}
+
+static int
+cdcsetbreak(Serialport *p, int val)
+{
+	if(p->hasbreak == 0)
+		return 0;
+
+	return usbcmd(p->s->dev, Rh2d | Rclass | Riface, SetBreak, (val != 0? 0xffff: 0x0000), 0, nil, 0);
+}
+
+static int
+cdcsendlines(Serialport *p)
+{
+	if(p->rts)
+		p->ctlstate |= CtlRTS;
+	else
+		p->ctlstate &= ~CtlRTS;
+	if(p->dtr)
+		p->ctlstate |= CtlDTR;
+	else
+		p->ctlstate &= ~CtlDTR;
+
+	return usbcmd(p->s->dev, Rh2d | Rclass | Riface, ControlState, p->ctlstate, 0, nil, 0);
+}
+
+static int
+cdcclearpipes(Serialport *p)
+{
+	if(unstall(p->s->dev, p->epout, Eout) < 0)
+		dprint(2, "serial: unstall epout: %r\n");
+	if(unstall(p->s->dev, p->epin, Ein) < 0)
+		dprint(2, "serial: unstall epin: %r\n");
+	if(unstall(p->s->dev, p->epintr, Ein) < 0)
+		dprint(2, "serial: unstall epintr: %r\n");
+
+	return 0;
+}
+
+static int
+cdcwait4data(Serialport *p, uchar *data, int count)
+{
+	int n;
+
+	qunlock(p->s);
+	while ((n = read(p->epin->dfd, data, count)) == 0)
+		;
+	qlock(p->s);
+
+	return n;
+}
+
+static Serialops cdcops = {
+	.init		= cdcinit,
+	.setparam	= cdcsetparam,
+	.setbreak	= cdcsetbreak,
+	.sendlines	= cdcsendlines,
+	.clearpipes	= cdcclearpipes,
+	.wait4data	= cdcwait4data,
+};
+
+int
+cdcprobe(Serial *ser)
+{
+	int i, ifcs;
+	Usbdev *ud = ser->dev->usb;
+	Iface *iface;
+	ulong csp;
+
+	ifcs = 0;
+	if (ud->vid == Kyocera || ud->did == AHK3001V)
+		ifcs++;
+	else for (i = 0; i < Niface; i++) {
+		iface = ud->conf[0]->iface[i];
+		if (iface == nil)
+			continue;
+
+		csp = iface->csp;
+		if (Class(csp) == Clcomms && Subclass(csp) == ScACM && (Proto(csp) == AT || Proto(csp) == None)) {
+			ser->p[ifcs].ctl_ifc = i;
+			ser->p[ifcs].s = ser;
+			getcaps(&ser->p[ifcs]);
+			ifcs++;
+		}
+	}
+
+	if (ifcs == 0)
+		return -1;
+
+	ser->nifcs = ifcs;
+	ser->isacm = 1;
+	ser->outhdrsz = 0;
+	ser->hasepintr = 1;
+	ser->Serialops = cdcops;
+	return 0;
+}
diff -r 23bc1d0e2dd2 sys/src/cmd/nusb/serial/mkfile
--- a/sys/src/cmd/nusb/serial/mkfile	Tue Jun 09 12:23:24 2020 -0700
+++ b/sys/src/cmd/nusb/serial/mkfile	Thu Jul 30 21:06:29 2020 -0700
@@ -1,7 +1,7 @@
 </$objtype/mkfile
 
 TARG=serial
-OFILES=ftdi.$O serial.$O prolific.$O ucons.$O silabs.$O ch340.$O
+OFILES=ftdi.$O serial.$O prolific.$O ucons.$O silabs.$O ch340.$O cdc.$O
 HFILES=\
 	../lib/usb.h\
 	serial.h\
diff -r 23bc1d0e2dd2 sys/src/cmd/nusb/serial/serial.c
--- a/sys/src/cmd/nusb/serial/serial.c	Tue Jun 09 12:23:24 2020 -0700
+++ b/sys/src/cmd/nusb/serial/serial.c	Thu Jul 30 21:06:29 2020 -0700
@@ -624,23 +624,46 @@
 
 	epintr = epin = epout = -1;
 
-	/*
-	 * interfc 0 means start from the start which is equiv to
-	 * iterate through endpoints probably, could be done better
-	 */
-	eps = ser->dev->usb->conf[0]->iface[ifc]->ep;
+	if(ser->isacm){
+		eps = ser->dev->usb->conf[0]->iface[ser->p[ifc].ctl_ifc]->ep;
 
-	for(i = 0; i < Nep; i++){
-		if((ep = eps[i]) == nil)
-			continue;
-		if(ser->hasepintr && ep->type == Eintr &&
-		    ep->dir == Ein && epintr == -1)
-			epintr = ep->id;
-		if(ep->type == Ebulk){
-			if((ep->dir == Ein || ep->dir == Eboth) && epin == -1)
-				epin = ep->id;
-			if((ep->dir == Eout || ep->dir == Eboth) && epout == -1)
-				epout = ep->id;

both of these for-loops (ser->isacm cases) appears to be identical
except the search for the intr endpoint, which you could easily
override (ser->hasintr = 0) in the cdc driver, no?

+		for(i = 0; i < Nep; i++){
+			if((ep = eps[i]) == nil)
+				continue;
+			if(ep->type == Eintr && ep->dir == Ein && epintr == -1)
+				epintr = ep->id;
+		}
+
+		eps = ser->dev->usb->conf[0]->iface[ser->p[ifc].data_ifc]->ep;
+		for(i = 0; i < Nep; i++){
+			if((ep = eps[i]) == nil)
+				continue;
+			if(ep->type == Ebulk){
+				if((ep->dir == Ein || ep->dir == Eboth) && epin == -1)
+					epin = ep->id;
+				if((ep->dir == Eout || ep->dir == Eboth) && epout == -1)
+					epout = ep->id;
+			}
+		}
+	} else {
+		/*
+		 * interfc 0 means start from the start which is equiv to
+		 * iterate through endpoints probably, could be done better
+		 */
+		eps = ser->dev->usb->conf[0]->iface[ifc]->ep;
+
+		for(i = 0; i < Nep; i++){
+			if((ep = eps[i]) == nil)
+				continue;
+			if(ser->hasepintr && ep->type == Eintr &&
+			    ep->dir == Ein && epintr == -1)
+				epintr = ep->id;
+			if(ep->type == Ebulk){
+				if((ep->dir == Ein || ep->dir == Eboth) && epin == -1)
+					epin = ep->id;
+				if((ep->dir == Eout || ep->dir == Eboth) && epout == -1)
+					epout = ep->id;
+			}
 		}
 	}
 	dprint(2, "serial[%d]: ep ids: in %d out %d intr %d\n", ifc, epin, epout, epintr);
@@ -714,6 +737,7 @@
 extern int slprobe(Serial *ser);
 extern int chprobe(Serial *ser);
 extern int uconsprobe(Serial *ser);
+extern int cdcprobe(Serial *ser);
 
 void
 threadmain(int argc, char* argv[])
@@ -750,7 +774,8 @@
 	&& ftprobe(ser)
 	&& slprobe(ser)
 	&& plprobe(ser)
-	&& chprobe(ser))
+	&& chprobe(ser)
+	&& cdcprobe(ser))
 		sysfatal("no serial devices found");
 
 	for(i = 0; i < ser->nifcs; i++){
diff -r 23bc1d0e2dd2 sys/src/cmd/nusb/serial/serial.h
--- a/sys/src/cmd/nusb/serial/serial.h	Tue Jun 09 12:23:24 2020 -0700
+++ b/sys/src/cmd/nusb/serial/serial.h	Thu Jul 30 21:06:29 2020 -0700
@@ -60,6 +60,10 @@
 
 	int	interfc;	/* interfc on the device for ftdi */
 
+	int hasbreak;	/* CDC ACM may not have break */
+	int ctl_ifc;	/* epintr iface for CDC ACM */
+	int data_ifc;	/* epin/out iface for CDC ACM */
+
 	Channel *w4data;
 	Channel *gotdata;
 	int	ndata;
@@ -81,6 +85,7 @@
 
 	int	jtag;		/* index of jtag interface, -1 none */
 	int	nifcs;		/* # of serial interfaces, including JTAG */
+	int isacm; 		/* CDC ACM interfaces handled differently */
 	Serialport p[Maxifc];
 	int	maxrtrans;
 	int	maxwtrans;

--
cinap