Re: [PATCH v2 1/2] Revert "USB: serial: ch341: add new Product ID for CH341A"

On 5/12/21 4:55 AM, Johan Hovold wrote:

On Tue, May 11, 2021 at 08:07:31PM -0500, Frank Zago wrote:

quoted

Hello,

On 5/10/21 2:40 AM, Johan Hovold wrote:

quoted

On Thu, Apr 22, 2021 at 07:28:51PM -0500, Frank Zago wrote:

quoted

From: frank zago <frank@zago.net>

The 0x5512 USB PID is for the I2C/GPIO/SPI interfaces. UART is
 still present but only the TX and RX pins are available; DTS,
DTR, ... are used for other things. Remove the PID, and let a
I2C driver bind to it.

Existing CH341 boards usually have physical jumpers to switch 
between the 3 modes.

This reverts commit 46ee4abb10a07bd8f8ce910ee6b4ae6a947d7f63.

You can't just revert something which people clearly depend on
and therefore added to the kernel in the first place.

That device in UART mode was already supported by the serial
driver. The original submitter just had to move a jumper on his
board. There was no need to patch the kernel.

How do you know that the author used a dev board? And are you really 
sure that there are no devices out there which always operate in
this mode?

The author of commit 46ee4abb10a07bd8 put a link to his device. I have 
the same one (or a clone) and it works fine in serial mode without the patch.
I have a different model that works the same way. A jumper selects the mode.

I can't be sure that no one has ever built a board with that chip, selecting
the wrong mode. But the chip is about 10 years old now; someone would have noticed.

quoted

That product ID also supports UART but in a limited way, as only
the RX and TX pins are available. However it is the only one that
supports i2c/spi/gpio, and that's why I have to revert the patch.

I understand why you did it. My point is that you cannot just claim
that PID and say that it's only to be used for I2C/SPI without even
trying to make a case for why that should be ok.

That's the only PID that works for I2C/SPI/GPIO. Right now the serial driver is 
claiming it. I don't know what else to say. If I can't revert that patch, my driver
can't be used without blacklisting the serial driver.

quoted

If that's desired, the new driver could add support for that as
well, but I don't think it's worth the effort.

We obviously don't want a second serial driver for these devices.

quoted

quoted

Can you reprogram the device with a newly allocated PID to be
used for i2c-only instead?

It is possible if the device has an SPI flash connected to it, but
none of the cheap boards have that.

That's unfortunate. In principle, your approach is the right one,
that is, to use a dedicated PID do determine when to configure an
alternate mode. But since we already know that some people are using
the PID in question in serial mode, it's not that clear cut.

How do you intend to switch between i2c and spi mode?

i2c, spi and gpio can all be used simultaneously. I have a working spi implementation,
but I'm still testing it. Basically if a user wants to use spi, then 3 specific gpios
will be reserved for MOSI/MISO/CLK (using gpiochip_request_own_desc), with possibly 
one or more used for the chip select. 
How a user books spi is up in the air right now. That might be done through a sysfs command.

Frank.