On Tue, Mar 31, 2020 at 11:37:20PM +0000, Michael Hanselmann wrote: > Add constants for the prescaler and divisor registers. ...and document and name register 0x25, and put the LCR define to more use. I still thinks this should go in its own patch as your not replacing all magic register constants (or at least be mentioned in the commit message). > The 0x25 register is only used by CH341 chips before version 0x30 and is > involved in configuring the line control parameters. It's not known to > the author whether there any such chips in the wild, and the driver > never supported them (other registers are also treated differently). The > alternative would've been to not set the register, but that may have > unintended effects. How did you come to those conclusions? I see this register being written the value zero in some older version of the vendor driver, but not in more recent ones. Are you sure it's at all related to LCR? > Signed-off-by: Michael Hanselmann <public@xxxxxxxxx> > --- > drivers/usb/serial/ch341.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c > index 67a5d4c3df42..9407e12d9fbc 100644 > --- a/drivers/usb/serial/ch341.c > +++ b/drivers/usb/serial/ch341.c > @@ -61,7 +61,11 @@ > #define CH341_REQ_MODEM_CTRL 0xA4 > > #define CH341_REG_BREAK 0x05 > +#define CH341_REG_PRESCALER 0x12 > +#define CH341_REG_DIVISOR 0x13 > #define CH341_REG_LCR 0x18 > +#define CH341_REG_LCR2 0x25 > + > #define CH341_NBREAK_BITS 0x01 > > #define CH341_LCR_ENABLE_RX 0x80 > @@ -294,11 +298,19 @@ static int ch341_set_baudrate_lcr(struct usb_device *dev, > */ > val |= BIT(7); > > - r = ch341_control_out(dev, CH341_REQ_WRITE_REG, 0x1312, val); > + r = ch341_control_out(dev, CH341_REQ_WRITE_REG, > + CH341_REG_DIVISOR << 8 | CH341_REG_PRESCALER, > + val); > if (r) > return r; > > - r = ch341_control_out(dev, CH341_REQ_WRITE_REG, 0x2518, lcr); > + /* > + * Chip versions before version 0x30 (read using > + * CH341_REQ_READ_VERSION) used separate registers for line control. > + * 0x30 and above use CH341_REG_LCR only. > + */ > + r = ch341_control_out(dev, CH341_REQ_WRITE_REG, > + CH341_REG_LCR2 << 8 | CH341_REG_LCR, lcr); Keeping "0x25" here to indicate that it's purpose is still not known should be ok too. > if (r) > return r; Johan
