On Mon, Oct 26, 2020 at 09:12:17AM +0100, Stefan Huber wrote:
> Hey all,
>
> I recently got hands on an USB-TTY converter that is not (yet) supported by
> OpenBSD:
>
> > addr 03: 067b:23a3 Prolific Technology Inc., USB-Serial Controller
>
> Simply adding the device ID to usbdevs and the uplcom driver did not do the
> trick, as apparently it is what the linux driver calls a "HXN" chip, which
> is (as of now) not yet supported by the uplcom driver (also, not in NetBSD
> where the uplcom driver seems to come from). Some hours (days) later, I have
> produced the following diff, butchering both the uplcom and the linux
> driver. I know it looks like shit, but it works (for now and for me). I will
> try to "streamline" the driver, and am happy for any feedback (since you may
> see watching the code I am not a good kernel hacker yet).
>
> I was also thinking of adding all the other IDs from the linux driver to
> uplcom, but then I can't test them since I only have the two chips 067b:2303
> and 067b:23a3. So I'd rather not merge the other IDs to prevent the kernel
> from doing nasty things to them (or was it the other way around?)
>
> The diff's below. Works for me. Not looking for okay or a merge, mostly
> feedback and maybe a merge for a future version of this diff.
First of all, we need unified diffs. Usually you have a CVS checkout
and then run cvs diff -u. Yours don't look like you used cvs, was that
a diff -r? Well, even then you should add -u, otherwise diffs become
unreadable.
Second, only OpenBSD account holders can ask for OKs. :)
So, please resend with diff -u, otherwise no one will have a look at it
since the diff is hard to follow.
Thanks,
Patrick
> Happy Hacking,
> Stefan
>
>
> Common subdirectories: /home/src_orig/sys/dev/usb/CVS and
> /usr/src/sys/dev/usb/CVS
> Common subdirectories: /home/src_orig/sys/dev/usb/dwc2 and
> /usr/src/sys/dev/usb/dwc2
> diff /home/src_orig/sys/dev/usb/uplcom.c /usr/src/sys/dev/usb/uplcom.c
> 66c66
> < #define DPRINTF(x) DPRINTFN(0, x)
> ---
> > #define DPRINTF(x) printf x;
> 71a72,74
> > #define UPLCOM_SET_NREQUEST 0x80
> > #define UPLCOM_READ_REQUEST 0x01
> > #define UPLCOM_READ_NREQUEST 0x81
> 77a81,91
> > #define UPLCOM_READ_HX_STATUS 0x8080
> > #define UPLCOM_HXN_RESET_REG 0x07
> > #define UPLCOM_HXN_RESET_UPSTREAM_PIPE 0x02
> > #define UPLCOM_HXN_RESET_DOWNSTREAM_PIPE 0x01
> >
> > #define UPLCOM_HXN_FLOWCTRL_REG 0x0a
> > #define UPLCOM_HXN_FLOWCTRL_MASK 0x1c
> > #define UPLCOM_HXN_FLOWCTRL_NONE 0x1c
> > #define UPLCOM_HXN_FLOWCTRL_RTS_CTS 0x18
> > #define UPLCOM_HXN_FLOWCTRL_XON_XOFF 0x0c
> >
> 98a113
> > int sc_type_hxn; /* HXN variant */
> 108a124,127
> > usbd_status uplcom_update_reg(struct uplcom_softc *sc, char reg, char
> > mask, char val);
> > usbd_status uplcom_vendor_read(struct uplcom_softc *sc, char val, char
> > buf[1]);
> > usbd_status uplcom_vendor_write(struct uplcom_softc *sc, char val, char
> > index);
> > int uplcom_test_hxn(struct uplcom_softc *);
> 153a173
> > { USB_VENDOR_PROLIFIC, USB_PRODUCT_PROLIFIC_PL23a3 },
> 250c270,272
> < if (ddesc->bDeviceClass == 0x02)
> ---
> > printf("[DEBUG] DeviceClass: %x\n", ddesc->bDeviceClass);
> > printf("[DEBUG] MaxPacketSize: %x\n", ddesc->bMaxPacketSize);
> > if (ddesc->bDeviceClass == 0x02) { /* type 0 */
> 252c274,276
> < else if (ddesc->bMaxPacketSize == 0x40)
> ---
> > sc->sc_type_hxn = 0;
> > }
> > else if (ddesc->bMaxPacketSize == 0x40) { /* type hx */
> 254c278,280
> < else
> ---
> > sc->sc_type_hxn = 0;
> > }
> > else { /* probably type 1 */
> 255a282,283
> > sc->sc_type_hxn = 0;
> > }
> 256a285,298
> > /*
> > * The Linux driver also distinguishes HXN variant...
> > * Let's see if that is needed for the 23a3 variant.
> > */
> > if (sc->sc_type_hx == 1) {
> > printf("[DEBUG] seems to be HX type. Checking for HXN type.\n");
> > if (uplcom_test_hxn(sc)) {
> > printf("[DEBUG] Probably HXN.\n");
> > sc->sc_type_hxn = 1;
> > } else {
> > printf("[DEBUG] Probably not HXN.\n");
> > }
> > }
> >
> 259c301,303
> < if (sc->sc_type_hx) {
> ---
> > if (sc->sc_type_hxn) {
> > DPRINTF(("uplcom_attach: chiptype 2303XN\n"));
> > } else if (sc->sc_type_hx) {
> 419a464,471
> >
> > if (sc->sc_type_hxn) {
> > DPRINTF(("[DEBUG] trying to reset hxn\n"));
> > req.bmRequestType = UT_WRITE_VENDOR_DEVICE;
> > req.bRequest = UPLCOM_SET_NREQUEST;
> > USETW(req.wValue, 0x07);
> > USETW(req.wIndex, UPLCOM_HXN_RESET_UPSTREAM_PIPE |
> > UPLCOM_HXN_RESET_DOWNSTREAM_PIPE);
> > USETW(req.wLength, 0);
> 421,425c473,484
> < req.bmRequestType = UT_WRITE_VENDOR_DEVICE;
> < req.bRequest = UPLCOM_SET_REQUEST;
> < USETW(req.wValue, 0);
> < USETW(req.wIndex, sc->sc_iface_number);
> < USETW(req.wLength, 0);
> ---
> > err = usbd_do_request(sc->sc_udev, &req, 0);
> > if (err) {
> > DPRINTF(("[DEBUG] uplcom_reset_hxn err=%d\n", err));
> > return (err);
> > }
> > } else {
> > DPRINTF(("[DEBUG] trying to reset non-hxn device\n"));
> > req.bmRequestType = UT_WRITE_VENDOR_DEVICE;
> > req.bRequest = UPLCOM_SET_REQUEST;
> > USETW(req.wValue, 8);
> > USETW(req.wIndex, 0);
> > USETW(req.wLength, 0);
> 427,429c486,495
> < err = usbd_do_request(sc->sc_udev, &req, 0);
> < if (err)
> < return (EIO);
> ---
> > err = usbd_do_request(sc->sc_udev, &req, 0);
> > if (err) {
> > DPRINTF(("[DEBUG] uplcom_reset_8 err=%s\n",
> > usbd_errstr(err)));
> > return (err);
> > }
> > req.bmRequestType = UT_WRITE_VENDOR_DEVICE;
> > req.bRequest = UPLCOM_SET_REQUEST;
> > USETW(req.wValue, 9);
> > USETW(req.wIndex, 0);
> > USETW(req.wLength, 0);
> 430a497,503
> > err = usbd_do_request(sc->sc_udev, &req, 0);
> > if (err) {
> > DPRINTF(("[DEBUG] uplcom_reset_9 err=%s\n",
> > usbd_errstr(err)));
> > return (err);
> > }
> > }
> >
> 528a602,610
> > if (sc->sc_type_hxn) {
> > uplcom_update_reg(sc, UPLCOM_HXN_FLOWCTRL_REG,
> > UPLCOM_HXN_FLOWCTRL_MASK, UPLCOM_HXN_FLOWCTRL_XON_XOFF);
> > } else {
> > req.bmRequestType = UT_WRITE_VENDOR_DEVICE;
> > req.bRequest = UPLCOM_SET_REQUEST;
> > USETW(req.wValue, 0);
> > USETW(req.wIndex,
> > (sc->sc_type_hx ? UPLCOM_HX_SET_CRTSCTS :
> > UPLCOM_SET_CRTSCTS));
> > USETW(req.wLength, 0);
> 530,541c612,617
> < req.bmRequestType = UT_WRITE_VENDOR_DEVICE;
> < req.bRequest = UPLCOM_SET_REQUEST;
> < USETW(req.wValue, 0);
> < USETW(req.wIndex,
> < (sc->sc_type_hx ? UPLCOM_HX_SET_CRTSCTS : UPLCOM_SET_CRTSCTS));
> < USETW(req.wLength, 0);
> <
> < err = usbd_do_request(sc->sc_udev, &req, 0);
> < if (err) {
> < DPRINTF(("uplcom_set_crtscts: failed, err=%s\n",
> < usbd_errstr(err)));
> < return (err);
> ---
> > err = usbd_do_request(sc->sc_udev, &req, 0);
> > if (err) {
> > DPRINTF(("uplcom_set_crtscts: failed, err=%s\n",
> > usbd_errstr(err)));
> > return (err);
> > }
> 658c734,738
> < if (sc->sc_type_hx == 1) {
> ---
> > if (sc->sc_type_hxn) {
> > uplcom_vendor_write(sc, UPLCOM_HXN_RESET_REG,
> > UPLCOM_HXN_RESET_UPSTREAM_PIPE |
> > UPLCOM_HXN_RESET_DOWNSTREAM_PIPE);
> > } else if (sc->sc_type_hx == 1) {
> 768a849,984
> > }
> >
> > /*
> > * uplcom_test_hxn()
> > * will return 1 if attached device is HXN device,
> > * will return 0 else.
> > */
> > int
> > uplcom_test_hxn(struct uplcom_softc *sc)
> > {
> > /*
> > * res = usb_control_msg(serial->dev,
> > * usb_rcvctrlpipe(serial->dev, 0),
> > * VENDOR_READ_REQUEST, VENDOR_READ_REQUEST_TYPE,
> > * PL2303_READ_TYPE_HX_STATUS, 0, buf, 1, 100);
> > * if (res != 1)
> > * type = TYPE_HXN;
> > *
> > * int usb_control_msg(
> > * struct usb_device * dev,
> > * unsigned int pipe,
> > * __u8 request,
> > * __u8 requesttype,
> > * __u16 value,
> > * __u16 index,
> > * void * data,
> > * __u16 size,
> > * int timeout);
> > *
> > * VENDOR_READ_REQUEST 0x01
> > * VENDOR_READ_REQUEST_TYPE 0xc0
> > * PL2303_READ_TYPE_HX_STATUS 0x8080
> > */
> > usb_device_request_t req;
> > usbd_status err;
> >
> > req.bmRequestType = 0xc0;
> > req.bRequest = 0x01;
> > USETW(req.wValue, 0x8080);
> > USETW(req.wIndex, 0);
> > USETW(req.wLength, 1);
> >
> > void* buf = malloc(1, M_USBDEV, M_WAITOK);
> > err = usbd_do_request(sc->sc_udev, &req, buf);
> > DPRINTF(("uplcom_test_hxn buf: %02x\n", (*(uint8_t*) buf)));
> > if (err) {
> > /* If it stalls, it's HXN */
> > DPRINTF(("uplcom_test_hxn err=%s\n", usbd_errstr(err)));
> > return (1);
> > }
> >
> > return (0);
> > }
> >
> > usbd_status
> > uplcom_update_reg(struct uplcom_softc *sc, char reg, char mask, char
> > val)
> > {
> > char* buf = malloc(1, M_USBDEV, M_WAITOK);
> > usbd_status err;
> > /* read value */
> > if (sc->sc_type_hxn) {
> > err = uplcom_vendor_read(sc, reg, buf);
> > if (err) {
> > DPRINTF(("%s: err=%s\n", __func__, usbd_errstr(err)));
> > return (err);
> > }
> > } else {
> > err = uplcom_vendor_read(sc, reg | 0x80, buf);
> > if (err) {
> > DPRINTF(("%s: err=%s\n", __func__, usbd_errstr(err)));
> > return (err);
> > }
> > }
> >
> > /* update value */
> > *buf &= ~mask;
> > *buf |= val & mask;
> >
> > /* write new value */
> > err = uplcom_vendor_write(sc, reg, *buf);
> > if (err) {
> > DPRINTF(("%s: err=%s\n", __func__, usbd_errstr(err)));
> > return (err);
> > }
> >
> > return USBD_NORMAL_COMPLETION;
> > }
> >
> > usbd_status
> > uplcom_vendor_read(struct uplcom_softc *sc, char val, char buf[1])
> > {
> > usbd_status err;
> > usb_device_request_t req;
> > req.bmRequestType = UT_READ_VENDOR_DEVICE;
> > if (sc->sc_type_hxn) {
> > req.bRequest = UPLCOM_READ_NREQUEST;
> > } else {
> > req.bRequest = UPLCOM_READ_REQUEST;
> > }
> > USETW(req.wValue, val);
> > USETW(req.wIndex, 0); /* read: index is always zero */
> > USETW(req.wLength, 1); /* read: read always one byte */
> >
> > err = usbd_do_request(sc->sc_udev, &req, &buf);
> > if (err) {
> > DPRINTF(("%s: failed to read: %s\n",
> > __func__, usbd_errstr(err)));
> > return (err);
> > }
> >
> > return USBD_NORMAL_COMPLETION; /* should return 0 if everything worked
> > */
> > }
> >
> > usbd_status
> > uplcom_vendor_write(struct uplcom_softc *sc, char val, char index)
> > {
> > usbd_status err;
> > usb_device_request_t req;
> > req.bmRequestType = UT_READ_VENDOR_DEVICE;
> > if (sc->sc_type_hxn) {
> > req.bRequest = UPLCOM_SET_NREQUEST;
> > } else {
> > req.bRequest = UPLCOM_SET_REQUEST;
> > }
> > USETW(req.wValue, val);
> > USETW(req.wIndex, index);
> > USETW(req.wLength, 0);
> >
> > err = usbd_do_request(sc->sc_udev, &req, 0);
> > if (err) {
> > DPRINTF(("%s: failed to write: %s\n",
> > __func__, usbd_errstr(err)));
> > return (err);
> > }
> >
> > return USBD_NORMAL_COMPLETION; /* should return 0 if everything worked
> > */
> diff /home/src_orig/sys/dev/usb/usbdevs /usr/src/sys/dev/usb/usbdevs
> 3552a3553
> > product PROLIFIC PL23a3 0x23a3 PL2303 Serial
> diff /home/src_orig/sys/dev/usb/usbdevs.h /usr/src/sys/dev/usb/usbdevs.h
> 1c1
> < /* $OpenBSD: usbdevs.h,v 1.732 2020/08/03 14:25:56 deraadt Exp $ */
> ---
> > /* $OpenBSD$ */
> 3559a3560
> > #define USB_PRODUCT_PROLIFIC_PL23a3 0x23a3 /* PL2303
> > Serial */
> diff /home/src_orig/sys/dev/usb/usbdevs_data.h
> /usr/src/sys/dev/usb/usbdevs_data.h
> 1c1
> < /* $OpenBSD: usbdevs_data.h,v 1.726 2020/08/03 14:25:56 deraadt Exp $
> */
> ---
> > /* $OpenBSD$ */
> 8777a8778,8781
> > "PL2303 Serial",
> > },
> > {
> > USB_VENDOR_PROLIFIC, USB_PRODUCT_PROLIFIC_PL23a3,
>
