On Fri, Dec 16, 2016 at 12:39:45PM +0000, Aidan Thornton wrote:
> On Thu, Dec 15, 2016 at 3:58 PM, Johan Hovold <johan@xxxxxxxxxx> wrote:
> > On Wed, Dec 14, 2016 at 04:28:07PM +0100, Johan Hovold wrote:
> >> Some CH341 devices appear to require the use of the init vendor command
> >> to set the baud rate and line-control register. Specifically, while
> >> using direct register updates for speed and LCR seem to update those
> >> settings correctly, not using the init command causes received data to
> >> be buffered until a full endpoint-size packet (32 bytes) have been
> >> received (i.e. the init command has some undocumented side-effect we
> >> need).
> >
> > Turns out it is bit 7 in the divisor register which needs to be set in
> > order for CH341A to not buffer incoming data this way. If set, direct
> > register updates works also for these devices.
> >
> > This bit is only set since 4e46c410e050 ("USB: serial: ch341:
> > reinitialize chip on reconfiguration") (in -next), which switched to
> > using the init-vendor command.
> >
> > So it seems we could use a common implementation after all...
>
> It would certainly simplify matters a lot if it turns out that was the
> only problem with using direct register writes after all.
>
> The removal of the second CH341_REQ_SERIAL_INIT by patch 2 of my
> series and subsequent removal of it from baudrate setting too does
> presumably mean that some register which was being set to a non-zero
> value isn't anymore. (The one cryptically described as "enable
> SFR_UART Control register and timer" in the vendor driver.)
I think I've managed to decipher that: the control register comment is
about setting bit 7 in wValue which causes the provided LCR value in the
high byte to take effect. If left unset, the LCR is instead reset to its
default value -- and this was in fact something that we unknowingly
relied on before your recent change that removed the second init.
Without, all devices would have been stuck at 5N1 due to that initial
LCR write.
The UART timer thing, may be about that buffering effect I describe
above, but could be something different (as it seems to apply to the
0x9c wValue, but who knows).
> While that doesn't seem to have any negative effect that I can spot so
> far, surely it must do something?
Heh. Possibly, but if we don't notice any difference I think it comes
down to whether we can get Russel's device to play nice with it somehow.
I've also asked if the vendor can provide some insight.
> > -static int ch341_init_set_baudrate(struct usb_device *dev,
> > +static int ch341_set_baudrate_lcr(struct usb_device *dev,
> > struct ch341_private *priv, unsigned ctrl)
>
> Also, since you're renaming a bunch of stuff it might be worth
> renaming the last parameter of this from "ctrl" to "lcr" too at some
> point for consistency, and the same with the corresponding variable in
> ch341_set_termios.
Good point, will do.
Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
