On Wed, Jun 11, 2025 at 07:32:57AM -0700, [email protected] wrote:
>
> >Synopsis: uchcom(4) seems to not flush received data
> >Category: kernel
> >Environment:
> System : OpenBSD 7.7
> Details : OpenBSD 7.7 (GENERIC.MP) #625: Sun Apr 13 08:30:20 MDT
> 2025
>
> [email protected]:/usr/src/sys/arch/amd64/compile/GENERIC.MP
>
> Architecture: OpenBSD.amd64
> Machine : amd64
> >Description:
> i have a device which presents its serial line as a CH341:
>
> uchcom0 at uhub1 port 2 configuration 1 interface 0 "QinHeng
> Electronics USB2.0-Serial" rev 1.10/2.54 addr 5
> uchcom0: CH341
> ucom2 at uchcom0: usb0.2.00002.0
>
> the other end of it is attached to a linux SBC, running a normal
> getty/login/shell.
>
> when i type, and the remote sends something, the information is not
> flushed immediately and visible, but it seems somehow still stuck in a
> buffer
> somewhere. for example, i would normally expect:
>
> mischief@megrez:~$ # press enter
> mischief@megrez:~$ # full shell prompt line appears
>
> but on this serial adapter, instead i get something like:
>
> mischief@megrez:~$ # press enter
> mischief@megr # line from remote is not fully flushed,
> press enter again
>
> mischief@megrez:~$ # the previous line now fully appears
> mischie # subsequent line not fully flushed again
>
> this also happens for non-newlines, such as just typing regular
> characters at the shell. for example, if i just type 'a', nothing
> appears until
> i type several 'a' in a row.
>
> it happens with both cu(1) and picocom from packages.
>
> in testing, the 'flush' of data seems to happen at exactly a 32-byte
> boundary, e.g., if i type some characters, when they finally appear, if
> i type
> exactly 32 more, it is then flushed and appears, which would make sense
> as the
> size of a buffer for the data. however, the same device does not
> exhibit the
> same behavior when using cu or picocom under linux, so my suspicion was
> that
> this is an issue with the uchcom(4) driver, and not a problem with the
> device
> itself or cu/etc.
>
> the device in question is listed below from usbdevs, with vid/did
> 1a86:7523.
>
> >How-To-Repeat:
> cu -s 115200 -l /dev/cuaU2, try to type
> >Fix:
> unsure, but the linux driver may reveal a clue.
When I added support for the CH343, I didn't touch any of the CH341/CH340
code paths. I also tested my CH341 with several SBCs, unfortunately I wasn't
able to reproduce your issue.
uchcom0 at uhub1 port 1 configuration 1 interface 0 "QinHeng Electronics
USB2.0-Serial" rev 1.10/2.63 addr 5
uchcom0: CH341
ucom0 at uchcom0: usb1.0.00001.0
$ usbdevs | grep 7523
addr 05: 1a86:7523 QinHeng Electronics, USB2.0-Serial
Since the CH340/CH341 seem to share many common parts with the CH343,
could you try the diff below to see if it fixes your issue? Thanks.
Index: sys/dev/usb/uchcom.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uchcom.c,v
diff -u -p -u -p -r1.37 uchcom.c
--- sys/dev/usb/uchcom.c 22 Oct 2024 21:50:02 -0000 1.37
+++ sys/dev/usb/uchcom.c 11 Jul 2025 02:14:29 -0000
@@ -116,14 +116,8 @@ int uchcomdebug = 0;
#define UCHCOM_T 0x08
#define UCHCOM_CL 0x04
#define UCHCOM_CT 0x80
-/*
- * XXX - these magic numbers come from Linux (drivers/usb/serial/ch341.c).
- * The manufacturer was unresponsive when asked for documentation.
- */
-#define UCHCOM_RESET_VALUE 0x501F /* line mode? */
-#define UCHCOM_RESET_INDEX 0xD90A /* baud rate? */
-#define UCHCOMOBUFSIZE 256
+#define UCHCOMOBUFSIZE 256
#define UCHCOM_TYPE_CH343 1
@@ -158,29 +152,6 @@ struct uchcom_endpoints {
int ep_intr_size;
};
-struct uchcom_divider {
- uint8_t dv_prescaler;
- uint8_t dv_div;
- uint8_t dv_mod;
-};
-
-struct uchcom_divider_record {
- uint32_t dvr_high;
- uint32_t dvr_low;
- uint32_t dvr_base_clock;
- struct uchcom_divider dvr_divider;
-};
-
-static const struct uchcom_divider_record dividers[] =
-{
- { 307200, 307200, UCHCOM_BASE_UNKNOWN, { 7, 0xD9, 0 } },
- { 921600, 921600, UCHCOM_BASE_UNKNOWN, { 7, 0xF3, 0 } },
- { 2999999, 23530, 6000000, { 3, 0, 0 } },
- { 23529, 2942, 750000, { 2, 0, 0 } },
- { 2941, 368, 93750, { 1, 0, 0 } },
- { 367, 1, 11719, { 0, 0, 0 } },
-};
-
void uchcom_get_status(void *, int, u_char *, u_char *);
void uchcom_set(void *, int, int, int);
int uchcom_param(void *, int, struct termios *);
@@ -211,16 +182,12 @@ int uchcom_update_status(struct uchcom_
int uchcom_set_dtrrts(struct uchcom_softc *, int, int);
int uchcom_set_break(struct uchcom_softc *, int);
int uchcom_set_break_ch343(struct uchcom_softc *, int);
-void uchcom_calc_baudrate_ch343(uint32_t, uint8_t *, uint8_t *);
-int uchcom_calc_divider_settings(struct uchcom_divider *, uint32_t);
-int uchcom_set_dte_rate_ch343(struct uchcom_softc *, uint32_t,
- uint16_t);
-int uchcom_set_dte_rate(struct uchcom_softc *, uint32_t);
+void uchcom_calc_baudrate(struct uchcom_softc *, uint32_t, uint8_t *,
+ uint8_t *);
+int uchcom_set_dte_rate(struct uchcom_softc *, uint32_t, uint16_t);
uint16_t uchcom_set_line_control(struct uchcom_softc *, tcflag_t,
uint16_t *);
int uchcom_clear_chip(struct uchcom_softc *);
-int uchcom_reset_chip(struct uchcom_softc *);
-int uchcom_setup_comm(struct uchcom_softc *);
int uchcom_setup_intr_pipe(struct uchcom_softc *);
@@ -576,9 +543,6 @@ uchcom_update_version(struct uchcom_soft
void
uchcom_convert_status(struct uchcom_softc *sc, uint8_t cur)
{
- sc->sc_dtr = !(cur & UCHCOM_DTR_MASK);
- sc->sc_rts = !(cur & UCHCOM_RTS_MASK);
-
cur = ~cur & 0x0F;
sc->sc_msr = (cur << 4) | ((sc->sc_msr >> 4) ^ cur);
}
@@ -595,6 +559,10 @@ uchcom_update_status(struct uchcom_softc
sc->sc_dev.dv_xname, usbd_errstr(err));
return EIO;
}
+
+ sc->sc_dtr = !(cur & UCHCOM_DTR_MASK);
+ sc->sc_rts = !(cur & UCHCOM_RTS_MASK);
+
uchcom_convert_status(sc, cur);
return 0;
@@ -666,11 +634,12 @@ uchcom_set_break_ch343(struct uchcom_sof
}
void
-uchcom_calc_baudrate_ch343(uint32_t rate, uint8_t *divisor, uint8_t *factor)
+uchcom_calc_baudrate(struct uchcom_softc *sc, uint32_t rate, uint8_t *divisor,
+ uint8_t *factor)
{
uint32_t clk = 12000000;
- if (rate >= 256000)
+ if (rate >= 921600 && rate != 1500000)
*divisor = 7;
else if (rate > 23529) {
clk /= 2;
@@ -686,56 +655,21 @@ uchcom_calc_baudrate_ch343(uint32_t rate
*divisor = 0;
}
- *factor = 256 - clk / rate;
-}
-
-int
-uchcom_calc_divider_settings(struct uchcom_divider *dp, uint32_t rate)
-{
- int i;
- const struct uchcom_divider_record *rp;
- uint32_t div, rem, mod;
-
- /* find record */
- for (i=0; i<nitems(dividers); i++) {
- if (dividers[i].dvr_high >= rate &&
- dividers[i].dvr_low <= rate) {
- rp = ÷rs[i];
- goto found;
- }
- }
- return -1;
-
-found:
- dp->dv_prescaler = rp->dvr_divider.dv_prescaler;
- if (rp->dvr_base_clock == UCHCOM_BASE_UNKNOWN)
- dp->dv_div = rp->dvr_divider.dv_div;
- else {
- div = rp->dvr_base_clock / rate;
- rem = rp->dvr_base_clock % rate;
- if (div==0 || div>=0xFF)
- return -1;
- if ((rem<<1) >= rate)
- div += 1;
- dp->dv_div = (uint8_t)-div;
- }
-
- mod = UCHCOM_BPS_MOD_BASE/rate + UCHCOM_BPS_MOD_BASE_OFS;
- mod = mod + mod/2;
-
- dp->dv_mod = mod / 0x100;
-
- return 0;
+ if (rate == 921600 && sc->sc_type != UCHCOM_TYPE_CH343)
+ *factor = 243;
+ else
+ *factor = 256 - clk / rate;
}
int
-uchcom_set_dte_rate_ch343(struct uchcom_softc *sc, uint32_t rate, uint16_t val)
+uchcom_set_dte_rate(struct uchcom_softc *sc, uint32_t rate, uint16_t val)
{
usbd_status err;
uint16_t idx;
uint8_t factor, div;
- uchcom_calc_baudrate_ch343(rate, &div, &factor);
+ uchcom_calc_baudrate(sc, rate, &div, &factor);
+ div |= (sc->sc_type != UCHCOM_TYPE_CH343) ? 0x80 : 0;
idx = (factor << 8) | div;
err = uchcom_generic_control_out(sc, UCHCOM_REQ_SET_BAUDRATE, val, idx);
@@ -748,34 +682,10 @@ uchcom_set_dte_rate_ch343(struct uchcom_
return 0;
}
-int
-uchcom_set_dte_rate(struct uchcom_softc *sc, uint32_t rate)
-{
- usbd_status err;
- struct uchcom_divider dv;
-
- if (uchcom_calc_divider_settings(&dv, rate))
- return EINVAL;
-
- if ((err = uchcom_write_reg(sc,
- UCHCOM_REG_BPS_PRE, dv.dv_prescaler,
- UCHCOM_REG_BPS_DIV, dv.dv_div)) ||
- (err = uchcom_write_reg(sc,
- UCHCOM_REG_BPS_MOD, dv.dv_mod,
- UCHCOM_REG_BPS_PAD, 0))) {
- printf("%s: cannot set DTE rate: %s\n",
- sc->sc_dev.dv_xname, usbd_errstr(err));
- return EIO;
- }
-
- return 0;
-}
-
uint16_t
uchcom_set_line_control(struct uchcom_softc *sc, tcflag_t cflag, uint16_t *val)
{
- usbd_status err;
- uint8_t lcr = 0, lcr2 = 0;
+ uint8_t lcr = 0;
if (sc->sc_release == UCHCOM_REV_CH340) {
/*
@@ -797,16 +707,6 @@ uchcom_set_line_control(struct uchcom_so
return 0;
}
- if (sc->sc_type != UCHCOM_TYPE_CH343) {
- err = uchcom_read_reg(sc, UCHCOM_REG_LCR, &lcr, UCHCOM_REG_LCR2,
- &lcr2);
- if (err) {
- printf("%s: cannot get LCR: %s\n",
- sc->sc_dev.dv_xname, usbd_errstr(err));
- return EIO;
- }
- }
-
lcr = UCHCOM_LCR_RXE | UCHCOM_LCR_TXE;
switch (ISSET(cflag, CSIZE)) {
@@ -834,16 +734,8 @@ uchcom_set_line_control(struct uchcom_so
lcr |= UCHCOM_LCR_STOPB;
}
- if (sc->sc_type != UCHCOM_TYPE_CH343) {
- err = uchcom_write_reg(sc, UCHCOM_REG_LCR, lcr, UCHCOM_REG_LCR2,
- lcr2);
- if (err) {
- printf("%s: cannot set LCR: %s\n",
- sc->sc_dev.dv_xname, usbd_errstr(err));
- return EIO;
- }
- } else
- *val = UCHCOM_T | UCHCOM_CL | UCHCOM_CT | lcr << 8;
+ *val = UCHCOM_T | UCHCOM_CL | UCHCOM_CT | lcr << 8;
+ *val |= (sc->sc_type != UCHCOM_TYPE_CH343) ? 0x10 : 0;
return 0;
}
@@ -865,59 +757,6 @@ uchcom_clear_chip(struct uchcom_softc *s
}
int
-uchcom_reset_chip(struct uchcom_softc *sc)
-{
- usbd_status err;
-
- DPRINTF(("%s: reset\n", sc->sc_dev.dv_xname));
-
- err = uchcom_generic_control_out(sc, UCHCOM_REQ_RESET,
- UCHCOM_RESET_VALUE,
- UCHCOM_RESET_INDEX);
- if (err)
- goto failed;
-
- return 0;
-
-failed:
- printf("%s: cannot reset: %s\n",
- sc->sc_dev.dv_xname, usbd_errstr(err));
- return EIO;
-}
-
-int
-uchcom_setup_comm(struct uchcom_softc *sc)
-{
- int ret;
-
- ret = uchcom_clear_chip(sc);
- if (ret)
- return ret;
-
- ret = uchcom_set_dte_rate(sc, TTYDEF_SPEED);
- if (ret)
- return ret;
-
- ret = uchcom_set_line_control(sc, CS8, 0);
- if (ret)
- return ret;
-
- ret = uchcom_update_status(sc);
- if (ret)
- return ret;
-
- ret = uchcom_reset_chip(sc);
- if (ret)
- return ret;
-
- ret = uchcom_set_dte_rate(sc, TTYDEF_SPEED); /* XXX */
- if (ret)
- return ret;
-
- return 0;
-}
-
-int
uchcom_setup_intr_pipe(struct uchcom_softc *sc)
{
usbd_status err;
@@ -1008,14 +847,16 @@ uchcom_param(void *arg, int portno, stru
if (usbd_is_dying(sc->sc_udev))
return 0;
+ if (t->c_ospeed <= 0 ||
+ (t->c_ospeed > 921600 && sc->sc_type != UCHCOM_TYPE_CH343) ||
+ (t->c_ospeed > 6000000 && sc->sc_type == UCHCOM_TYPE_CH343))
+ return EINVAL;
+
ret = uchcom_set_line_control(sc, t->c_cflag, &val);
if (ret)
return ret;
- if (sc->sc_type == UCHCOM_TYPE_CH343)
- ret = uchcom_set_dte_rate_ch343(sc, t->c_ospeed, val);
- else
- ret = uchcom_set_dte_rate(sc, t->c_ospeed);
+ ret = uchcom_set_dte_rate(sc, t->c_ospeed, val);
if (ret)
return ret;
@@ -1025,8 +866,8 @@ uchcom_param(void *arg, int portno, stru
int
uchcom_open(void *arg, int portno)
{
- int ret;
struct uchcom_softc *sc = arg;
+ int ret;
if (usbd_is_dying(sc->sc_udev))
return EIO;
@@ -1039,10 +880,13 @@ uchcom_open(void *arg, int portno)
if (ret)
return ret;
- if (sc->sc_type == UCHCOM_TYPE_CH343)
- ret = uchcom_update_status(sc);
- else
- ret = uchcom_setup_comm(sc);
+ if (sc->sc_type != UCHCOM_TYPE_CH343) {
+ ret = uchcom_clear_chip(sc);
+ if (ret)
+ return ret;
+ }
+
+ ret = uchcom_update_status(sc);
if (ret)
return ret;
