On Mon, Apr 14, 2014 at 07:54:17PM +0000, Karl Palsson wrote:
> Based on wireshark packet traces from a windows machine.
>
> ch340 and ch341 both seem to support all parity modes, but only the ch341
> appears to support variable data bits and variable stop bits, so those are left
> unimplemented, as before.
>
> Tested on a generic usb-rs485 dongle with the chip label scratched off, and
> some Modbus/RTU devices that required various parity settings.
>
> Signed-off-by: Karl Palsson <karlp@xxxxxxxxxxxx>
> ---
> drivers/usb/serial/ch341.c | 27 ++++++++++++++++++++++++++-
> 1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
> index 82371f6..b870f6f 100644
> --- a/drivers/usb/serial/ch341.c
> +++ b/drivers/usb/serial/ch341.c
> @@ -346,6 +346,7 @@ static void ch341_set_termios(struct tty_struct *tty,
> struct ch341_private *priv = usb_get_serial_port_data(port);
> unsigned baud_rate;
> unsigned long flags;
> + unsigned int par_flags;
>
> baud_rate = tty_get_baud_rate(tty);
>
> @@ -366,9 +367,33 @@ static void ch341_set_termios(struct tty_struct *tty,
>
> /* Unimplemented:
> * (cflag & CSIZE) : data bits [5, 8]
> - * (cflag & PARENB) : parity {NONE, EVEN, ODD}
> * (cflag & CSTOPB) : stop bits [1, 2]
> */
> + /* CH340 doesn't appear to support variable stop bits or data bits */
> + if (C_PARENB(tty)) {
> + if (C_PARODD(tty)) {
> + if (C_CMSPAR(tty)) {
Thanks for fixing the C_CMSPAR macro, but you didn't address my other
comments on v1.
> + dev_dbg(&port->dev, "parity = mark\n");
> + par_flags = 0xeb;
> + } else {
> + dev_dbg(&port->dev, "parity = odd\n");
> + par_flags = 0xcb;
> + }
> + } else {
> + if (C_CMSPAR(tty)) {
> + dev_dbg(&port->dev, "parity = space\n");
> + par_flags = 0xfb;
> + } else {
> + dev_dbg(&port->dev, "parity = even\n");
> + par_flags = 0xdb;
> + }
> + }
> + } else {
> + dev_dbg(&port->dev, "parity = none\n");
> + par_flags = 0xc3;
> + }
> + ch341_control_out(port->serial->dev, 0x9a, 0x2518, par_flags);
Specifically, I asked if you are able to make sense of the values of
register 0x2518. The reason I ask is that your patch changes the value
of that register from 0x50 (set in ch341_configure) to 0xc3 (when no
parity is used) and I want to make sure that you're not inadvertently
changing some other setting.
Do you know what those other bits do? Do they encode the number of data
and stop bits?
>From a quick glance it seems like
0xc0 parity mode (2 bits)
0x08 parity enable
but your patch now also sets bits 0x83 and clears bit 0x10.
> +
Again, you should remove this newline that your patch is adding.
> }
>
> static void ch341_break_ctl(struct tty_struct *tty, int break_state)
> --
> 1.9.0
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
