mbox series

[v6,0/2] usb: serial: add support for CH348

Message ID 20230628133834.1527941-1-clabbe@baylibre.com (mailing list archive)
Headers show
Series usb: serial: add support for CH348 | expand

Message

Corentin LABBE June 28, 2023, 1:38 p.m. UTC
Hello

The CH348 is an octo serial to USB adapter.
The following patch adds a driver for supporting it.
Since there is no public datasheet, unfortunatly it remains some magic values.

It was tested with a large range of baud from 1200 to 1500000 and used with
success in one of our kernel CI testlab.

Regards

Changes since v1:
- use a data structure for encoding/decoding messages.
- check if needed endpoints exists
- fix URB leak in ch348_allocate_status_read error case
- test for maximum baud rate as stated by datasheet

Changes since v2:
- specify ch348_rxbuf data length
- Use correct speed_t dwDTERate instead of __le32
- test for maximum baud rate supported according to datasheet
- Use a define for CH348_TX_HDRSIZE

Changes since v3
- Fixed all reported problem from https://lore.kernel.org/lkml/Y5NDwEakGJbmB6+b@Red/T/#mb6234d0427cfdabf412190565e215995a41482dd
  Mostly reworked the endpoint mux to be the same than mx_uport

Changes since v4:
- The V4 was sent against stable and next have ch348_set_termios ktermios
  parameter const that I forgot to change

Changes since v5:
- Fixed all reported problem from https://lore.kernel.org/lkml/20230106135338.643951-1-clabbe@baylibre.com/T/#m044aab24dfb652ea34aa06f8ef704da9d6a2e036
- Major change is dropping of all status handling which was unused.
  It will be probably necessary to bring it back when using GPIO.
  This will be done when I will finish my next devboard.

Corentin Labbe (2):
  usb: serial: add support for CH348
  usb: serial: add myself as maintainer of CH348

 MAINTAINERS                 |   5 +
 drivers/usb/serial/Kconfig  |   9 +
 drivers/usb/serial/Makefile |   1 +
 drivers/usb/serial/ch348.c  | 491 ++++++++++++++++++++++++++++++++++++
 4 files changed, 506 insertions(+)
 create mode 100644 drivers/usb/serial/ch348.c

Comments

David Heidelberg July 1, 2023, 7:16 p.m. UTC | #1
Tested-by: David Heidelberg <david@ixit.cz>
Reviewed-by: David Heidelberg <david@ixit.cz>

Thank you

On 28/06/2023 15:38, Corentin Labbe wrote:
> Hello
>
> The CH348 is an octo serial to USB adapter.
> The following patch adds a driver for supporting it.
> Since there is no public datasheet, unfortunatly it remains some magic values.
>
> It was tested with a large range of baud from 1200 to 1500000 and used with
> success in one of our kernel CI testlab.
>
> Regards
>
> Changes since v1:
> - use a data structure for encoding/decoding messages.
> - check if needed endpoints exists
> - fix URB leak in ch348_allocate_status_read error case
> - test for maximum baud rate as stated by datasheet
>
> Changes since v2:
> - specify ch348_rxbuf data length
> - Use correct speed_t dwDTERate instead of __le32
> - test for maximum baud rate supported according to datasheet
> - Use a define for CH348_TX_HDRSIZE
>
> Changes since v3
> - Fixed all reported problem from https://lore.kernel.org/lkml/Y5NDwEakGJbmB6+b@Red/T/#mb6234d0427cfdabf412190565e215995a41482dd
>    Mostly reworked the endpoint mux to be the same than mx_uport
>
> Changes since v4:
> - The V4 was sent against stable and next have ch348_set_termios ktermios
>    parameter const that I forgot to change
>
> Changes since v5:
> - Fixed all reported problem from https://lore.kernel.org/lkml/20230106135338.643951-1-clabbe@baylibre.com/T/#m044aab24dfb652ea34aa06f8ef704da9d6a2e036
> - Major change is dropping of all status handling which was unused.
>    It will be probably necessary to bring it back when using GPIO.
>    This will be done when I will finish my next devboard.
>
> Corentin Labbe (2):
>    usb: serial: add support for CH348
>    usb: serial: add myself as maintainer of CH348
>
>   MAINTAINERS                 |   5 +
>   drivers/usb/serial/Kconfig  |   9 +
>   drivers/usb/serial/Makefile |   1 +
>   drivers/usb/serial/ch348.c  | 491 ++++++++++++++++++++++++++++++++++++
>   4 files changed, 506 insertions(+)
>   create mode 100644 drivers/usb/serial/ch348.c
>
David Heidelberg Aug. 16, 2023, 11:34 p.m. UTC | #2
Hello Johan,

Any chance to get CH348 into 6.5?

I want to highlight that from all serials I have around this is the only 
one which support 1500000 baud (needed for Rockchip devices).

Thank you!
David


On 28/06/2023 15:38, Corentin Labbe wrote:
> Hello
>
> The CH348 is an octo serial to USB adapter.
> The following patch adds a driver for supporting it.
> Since there is no public datasheet, unfortunatly it remains some magic values.
>
> It was tested with a large range of baud from 1200 to 1500000 and used with
> success in one of our kernel CI testlab.
>
> Regards
>
> Changes since v1:
> - use a data structure for encoding/decoding messages.
> - check if needed endpoints exists
> - fix URB leak in ch348_allocate_status_read error case
> - test for maximum baud rate as stated by datasheet
>
> Changes since v2:
> - specify ch348_rxbuf data length
> - Use correct speed_t dwDTERate instead of __le32
> - test for maximum baud rate supported according to datasheet
> - Use a define for CH348_TX_HDRSIZE
>
> Changes since v3
> - Fixed all reported problem from https://lore.kernel.org/lkml/Y5NDwEakGJbmB6+b@Red/T/#mb6234d0427cfdabf412190565e215995a41482dd
>    Mostly reworked the endpoint mux to be the same than mx_uport
>
> Changes since v4:
> - The V4 was sent against stable and next have ch348_set_termios ktermios
>    parameter const that I forgot to change
>
> Changes since v5:
> - Fixed all reported problem from https://lore.kernel.org/lkml/20230106135338.643951-1-clabbe@baylibre.com/T/#m044aab24dfb652ea34aa06f8ef704da9d6a2e036
> - Major change is dropping of all status handling which was unused.
>    It will be probably necessary to bring it back when using GPIO.
>    This will be done when I will finish my next devboard.
>
> Corentin Labbe (2):
>    usb: serial: add support for CH348
>    usb: serial: add myself as maintainer of CH348
>
>   MAINTAINERS                 |   5 +
>   drivers/usb/serial/Kconfig  |   9 +
>   drivers/usb/serial/Makefile |   1 +
>   drivers/usb/serial/ch348.c  | 491 ++++++++++++++++++++++++++++++++++++
>   4 files changed, 506 insertions(+)
>   create mode 100644 drivers/usb/serial/ch348.c
>
Nicolas Frattaroli Aug. 30, 2023, 5:43 p.m. UTC | #3
On Mittwoch, 28. Juni 2023 15:38:32 CEST Corentin Labbe wrote:
> Hello
> 
> The CH348 is an octo serial to USB adapter.
> The following patch adds a driver for supporting it.
> Since there is no public datasheet, unfortunatly it remains some magic values.
> 
> It was tested with a large range of baud from 1200 to 1500000 and used with
> success in one of our kernel CI testlab.
> 
> Regards
> 
> [...]

Hello,

thank you for your work on this. I recently made myself a CH348
board and used this patchset with a small test application[1]
to see how it performs. Specifically, I ran this on an RK3566
single board computer, connecting one serial adapter to the
other, with the test as follows:

 ./serialtest /dev/ttyUSB0 9600 # UART0 of 1st CH348 board
 ./serialtest /dev/ttyUSB8 9600 # UART0 of 2nd CH348 board

One problem I've noticed is that writes to the tty fd never
seem to block. On two CH340 adapters I have, they do seem to
block, whereas here, you can see from the statistics at the
end that magnitudes more bytes were written than read, with
seemingly most of them being discarded. From my reading of
the termios parameters I set, this shouldn't be the case,
right?

You can see from the error percentage that it gets less
bad as you increase the serial baudrate; I've tested up
to 6 mbaud like this. I assume that's because less written
bytes get discarded.

Any ideas on whether I'm relying on weird driver behaviour
with the blocking here or if this driver actually has a
defect whereby it never signals to userspace that less
bytes were written than have been submitted?

Kind regards,
Nicolas Frattaroli

[1]: https://github.com/CounterPillow/serialtest
David Heidelberg Sept. 27, 2023, 12:09 p.m. UTC | #4
Hello Corentin, light reminder about this patchset and Nicolas question :)

Thank you
David

On 30/08/2023 23:13, Nicolas Frattaroli wrote:
> On Mittwoch, 28. Juni 2023 15:38:32 CEST Corentin Labbe wrote:
>> Hello
>>
>> The CH348 is an octo serial to USB adapter.
>> The following patch adds a driver for supporting it.
>> Since there is no public datasheet, unfortunatly it remains some magic values.
>>
>> It was tested with a large range of baud from 1200 to 1500000 and used with
>> success in one of our kernel CI testlab.
>>
>> Regards
>>
>> [...]
> Hello,
>
> thank you for your work on this. I recently made myself a CH348
> board and used this patchset with a small test application[1]
> to see how it performs. Specifically, I ran this on an RK3566
> single board computer, connecting one serial adapter to the
> other, with the test as follows:
>
>   ./serialtest /dev/ttyUSB0 9600 # UART0 of 1st CH348 board
>   ./serialtest /dev/ttyUSB8 9600 # UART0 of 2nd CH348 board
>
> One problem I've noticed is that writes to the tty fd never
> seem to block. On two CH340 adapters I have, they do seem to
> block, whereas here, you can see from the statistics at the
> end that magnitudes more bytes were written than read, with
> seemingly most of them being discarded. From my reading of
> the termios parameters I set, this shouldn't be the case,
> right?
>
> You can see from the error percentage that it gets less
> bad as you increase the serial baudrate; I've tested up
> to 6 mbaud like this. I assume that's because less written
> bytes get discarded.
>
> Any ideas on whether I'm relying on weird driver behaviour
> with the blocking here or if this driver actually has a
> defect whereby it never signals to userspace that less
> bytes were written than have been submitted?
>
> Kind regards,
> Nicolas Frattaroli
>
> [1]: https://github.com/CounterPillow/serialtest
>
>
Corentin LABBE Nov. 20, 2023, 12:29 p.m. UTC | #5
Le Wed, Aug 30, 2023 at 07:43:03PM +0200, Nicolas Frattaroli a écrit :
> On Mittwoch, 28. Juni 2023 15:38:32 CEST Corentin Labbe wrote:
> > Hello
> > 
> > The CH348 is an octo serial to USB adapter.
> > The following patch adds a driver for supporting it.
> > Since there is no public datasheet, unfortunatly it remains some magic values.
> > 
> > It was tested with a large range of baud from 1200 to 1500000 and used with
> > success in one of our kernel CI testlab.
> > 
> > Regards
> > 
> > [...]
> 
> Hello,
> 
> thank you for your work on this. I recently made myself a CH348
> board and used this patchset with a small test application[1]
> to see how it performs. Specifically, I ran this on an RK3566
> single board computer, connecting one serial adapter to the
> other, with the test as follows:
> 
>  ./serialtest /dev/ttyUSB0 9600 # UART0 of 1st CH348 board
>  ./serialtest /dev/ttyUSB8 9600 # UART0 of 2nd CH348 board
> 
> One problem I've noticed is that writes to the tty fd never
> seem to block. On two CH340 adapters I have, they do seem to
> block, whereas here, you can see from the statistics at the
> end that magnitudes more bytes were written than read, with
> seemingly most of them being discarded. From my reading of
> the termios parameters I set, this shouldn't be the case,
> right?
> 
> You can see from the error percentage that it gets less
> bad as you increase the serial baudrate; I've tested up
> to 6 mbaud like this. I assume that's because less written
> bytes get discarded.
> 
> Any ideas on whether I'm relying on weird driver behaviour
> with the blocking here or if this driver actually has a
> defect whereby it never signals to userspace that less
> bytes were written than have been submitted?
> 
> Kind regards,
> Nicolas Frattaroli
> 
> [1]: https://github.com/CounterPillow/serialtest
> 

Hello

Sorry for the very long delay of the answer.
I have reproduced the problem on my board.
My reproducer is https://github.com/montjoie/lava-tests/blob/master/test2a2.py

This problem seems to be here since the v1 of my patchset.
The vendor driver seems to work so it is not an hardware problem.

I have no clue at the moment, it is hard to diff with vendor driver since it create tty directly and do not use usbserial.

Regards
David Heidelberg April 3, 2024, 1:27 p.m. UTC | #6
Hello Corentin,

is there chance you find some time to upstream this?
In the worst case I would try to find some time to look into this, so it 
would have chance to get merged.

Thanks
David

On 20/11/2023 13:29, Corentin LABBE wrote:
> Le Wed, Aug 30, 2023 at 07:43:03PM +0200, Nicolas Frattaroli a écrit :
>> On Mittwoch, 28. Juni 2023 15:38:32 CEST Corentin Labbe wrote:
>>> Hello
>>>
>>> The CH348 is an octo serial to USB adapter.
>>> The following patch adds a driver for supporting it.
>>> Since there is no public datasheet, unfortunatly it remains some magic values.
>>>
>>> It was tested with a large range of baud from 1200 to 1500000 and used with
>>> success in one of our kernel CI testlab.
>>>
>>> Regards
>>>
>>> [...]
>> Hello,
>>
>> thank you for your work on this. I recently made myself a CH348
>> board and used this patchset with a small test application[1]
>> to see how it performs. Specifically, I ran this on an RK3566
>> single board computer, connecting one serial adapter to the
>> other, with the test as follows:
>>
>>   ./serialtest /dev/ttyUSB0 9600 # UART0 of 1st CH348 board
>>   ./serialtest /dev/ttyUSB8 9600 # UART0 of 2nd CH348 board
>>
>> One problem I've noticed is that writes to the tty fd never
>> seem to block. On two CH340 adapters I have, they do seem to
>> block, whereas here, you can see from the statistics at the
>> end that magnitudes more bytes were written than read, with
>> seemingly most of them being discarded. From my reading of
>> the termios parameters I set, this shouldn't be the case,
>> right?
>>
>> You can see from the error percentage that it gets less
>> bad as you increase the serial baudrate; I've tested up
>> to 6 mbaud like this. I assume that's because less written
>> bytes get discarded.
>>
>> Any ideas on whether I'm relying on weird driver behaviour
>> with the blocking here or if this driver actually has a
>> defect whereby it never signals to userspace that less
>> bytes were written than have been submitted?
>>
>> Kind regards,
>> Nicolas Frattaroli
>>
>> [1]: https://github.com/CounterPillow/serialtest
>>
> Hello
>
> Sorry for the very long delay of the answer.
> I have reproduced the problem on my board.
> My reproducer is https://github.com/montjoie/lava-tests/blob/master/test2a2.py
>
> This problem seems to be here since the v1 of my patchset.
> The vendor driver seems to work so it is not an hardware problem.
>
> I have no clue at the moment, it is hard to diff with vendor driver since it create tty directly and do not use usbserial.
>
> Regards
Corentin LABBE April 10, 2024, 8:04 p.m. UTC | #7
Le Wed, Apr 03, 2024 at 03:27:25PM +0200, David Heidelberg a écrit :
> Hello Corentin,
> 
> is there chance you find some time to upstream this?
> In the worst case I would try to find some time to look into this, so it 
> would have chance to get merged.
> 

Hello

Martin Blumenstingl worked a lot on fixing issues
You can see it at https://github.com/xdarklight/ch348/commits/main/

Currently I finish to test it to be able to do a v7.

We have an IRC channel to speak about it libera:#ch348

Regards