Re: [PATCH v2 0/6] USB: serial: ch341: Add support for limited chips (was: Add support for HL340 devices)

From: Johan Hovold <johan@kernel.org>
To: Michael Hanselmann <public@hansmi.ch>
Cc: linux-usb@vger.kernel.org, Johan Hovold <johan@kernel.org>,
	Michael Dreher <michael@5dot1.de>,
	Jonathan Olds <jontio@i4free.co.nz>
Subject: Re: [PATCH v2 0/6] USB: serial: ch341: Add support for limited chips (was: Add support for HL340 devices)
Date: Thu, 14 May 2020 16:02:20 +0200	[thread overview]
Message-ID: <20200514140220.GC25962@localhost> (raw)
In-Reply-To: <cover.1585697281.git.public@hansmi.ch>

Hi Michel,

and sorry about the late feedback on this one.

On Tue, Mar 31, 2020 at 11:37:16PM +0000, Michael Hanselmann wrote:
> A subset of CH341 devices does not support all features, namely the
> prescaler is limited to a reduced precision and there is no support for
> sending a RS232 break condition.
> 
> These devices can usually be identified by an imprint of "340" on the
> turquoise-colored plug. They're also sometimes called "HL340", but not
> all such devices are affected.
> 
> This series takes multiple steps to support the limited chips:
> 
> * Remove special cases from clock prescaler computation
> * Detect limited devices by trying to read a register related to sending
>   a break condition
> * Amend clock prescaler computation to only use working values on
>   limited chips
> * Simulate an RS232 break condition by temporarily lowering the baud
>   rate and sending a NUL byte

Good summary, and generally clean overall. I know you replied to my
earlier comments mentioning what you had done for v2, but its good
always include a short changelog here when resending a series.

> Michael Hanselmann (6):
>   USB: serial: ch341: Reduce special cases in clock calculation
>   USB: serial: ch341: Add basis for quirk detection
>   USB: serial: ch341: Limit prescaler on quirky chips
>   USB: serial: ch341: Name prescaler, divisor registers
>   USB: serial: ch341: Compute minimum baud rate
>   USB: serial: ch341: Simulate break condition if not supported
> 
>  drivers/usb/serial/ch341.c | 294 ++++++++++++++++++++++++++++++-------
>  1 file changed, 244 insertions(+), 50 deletions(-)

Johan