Re: [PATCH 0/3] a family of FTDI-based devices that need ftdi_sio quirks

[Mychaela Falconia <mychaela.falconia@gmail.com>]

Hi Johan,

> Thanks for the well-argued series. I've applied the first one now after
> dropping the PID for the dual-port device, which isn't used until the
> last patch.

Thank you for applying the JTAG patch: having that one mainlined means
that users of these JTAG+UART adapters will get the UART channel
working "out of the box", and there is now an unambiguous place in
ftdi_sio_ids.h and in the USB ID table for where DUART28C will need to
be added.

> Did you try inverting the signals so that they can be used on any serial
> port to power on and release reset on open, and then just clear HUPCL if
> you want the connected device to remain powered after the port is closed?

The first problem with asking for a hardware design change is that the
hardware has already been physically built - here are some photos:

https://www.freecalypso.org/members/falcon/pictures/duart28/

I got 20 fully assembled boards plus 20 more unpopulated PCBs, and
asking our community to throw this hardware out is a non-starter.

Also the idea of "so that they can be used on any serial port" is
meaningless in the context of this application: there is no "any
serial port", instead there exists a custom piece of hardware
(depicted above) that contains an FT2232D chip as one of its
components.  FTDI chips are powerful multifunction beasts, they are a
lot more than just serial ports, and on this particular board the I/O
channels coming out of the FT2232D chip are used in a mixed manner:
ADBUS0 through ADBUS7 form UART channel 0, BDBUS0 and BDBUS1 (data
leads only) form UART channel 1 (no flow control or modem control),
and BDBUS2 and BDBUS4 (which would otherwise be Channel B RTS & DTR)
are used essentially as GPIOs that have nothing to do with either of
the two serial ports.

Aside from already built hardware, inverting the sense of the signals
running between FT2232D and the open drain buffer (replacing the non-
inverting 74LVC2G07 buffer with an inverting one) would not work: in
that case both control signals would be immediately asserted as soon
as the USB cable is connected between the adapter board and the host
computer, resulting in the connected Calypso target being held down
both in reset and in the PWON pulse - bad.

I also realize that I may have been a little unclear in my original
explanation as to what the hardware signals really do.  FT2232D BDBUS2
and BDBUS4 outputs are conventionally defined to be RTS and DTR,
respectively, but their CMOS logic levels are the opposite of RS-232:
RS-232 DTR & RTS asserted means CMOS low on BDBUS[24], and RS-232 DTR
& RTS negated means CMOS high on BDBUS[24].  Both signals are CMOS
high initially on USB power application, and with my patch they become
CMOS low only as a result of an explicit TIOCMBIS ioctl setting DTR or
RTS.  These signals go to a Nexperia 74LVC2G07 non-inverting open
drain buffer IC (tiny U7 chip visible in the photos linked above), and
the open drain outputs are wired to J5 header pins that are labeled
CTL1 and CTL2 on the PCB silk screen - so they are not really DTR and
RTS at all as far as this custom hw is concerned, instead they are
special open drain pulse outputs.

CTL1 will be connected to PWON and CTL2 will be connected to RESET on
the Calypso target board.  The pulse nature of these two open drain
outputs is crucial: PWON is not a power on/off control where one
steady state means on and the other steady state means off, instead it
is a pulse control.  Virtually every handheld battery-powered device
has a "soft power" button that is physically wired between the power
management chip's dedicated PWON pin and ground, and Calypso is no
different: the pulse put out on CTL1 by the DUART28C adapter (produced
by userspace doing TIOCMBIS, delay, TIOCMBIC) simulates the action of
a user pressing the red power button on her dumbphone.  RESET works
the same way: having the target held down in reset long-term is bad,
instead whenever a reset pulse is needed, we do a TIOCMBIS, delay,
TIOCMBIC sequence.

In short, I will not change our hardware design, and I will continue
running with my patch whether it is mainlined or not.

> As you probably expected, I'm a bit reluctant to adding quirks like this
> (e.g. as it makes the code harder to read and maintain):

Right now I am instructing my user community to apply the needed patch
locally.  I am making my best-faith attempt to get this patch mainlined
as a matter of due diligence - had I not even bothered to try, my users
would be bound to ask "why don't you get this patch mainlined".  If
our patch is rejected, I will document this rejection in the user
documentation for the hardware product, with links to the relevant
messages in the linux-usb mailing list archive, so that my users will
know that I have honestly tried and that I am not the one to blame -
it is all I can do.

I am going to prepare a second version of this patch series (down to
just two patches now) with the changes you have requested for 2/3 and
3/3.

Oh, and a couple of comments from 3/3 that are worth addressing:

> If we are to add this, then you shouldn't allow for automatic deassert
> either.

Because of the pulse nature of CTL1 and CTL2 outputs, automatic
deassertion (in case a userspace program exited without having done
TIOCMBIC) makes good sense here - but I am perfectly fine with
disabling automatic deassertion for this quirk if it would help
getting the patch mainlined.  If the patch gets mainlined with
automatic deassertion disabled, the resulting system would be a little
less robust against the contrived corner case of a userspace program
crashing in the middle of a pulse, but it is not a deal breaker - if
that very unlikely corner case happens, just unplug the USB cable and
plug it back in to release the open drain outputs and to restore the
internal signals to CMOS high.  I can also add a trivial "clean up"
utility to our userspace software suite that opens the serial port,
clears both controls with TIOCMBIC and exits.

> And then there's CRTSCTS of course...

The second serial channel whose RTS & DTR outputs are being "stolen"
and repurposed for a use not related to this serial channel per se is
a data leads only UART with *no* flow control and no modem control -
thus if a userspace program were to set CRTSCTS on this channel, it
would be totally invalid usage, no different from a user trying to
run, say, a modem dialer program (one that talks AT commands) against
a /dev/ttyUSBx device connected to, say, an RS-232 thermometer, and
expecting it to work - surely it is not the kernel's job to guard
against such completely invalid usage?

I realize that all of my answers are highly specific to my one
particular custom hardware toy and aren't generalized in the
slightest, but in my defense, the quirk which I am asking to mainline
is conditionalized on one specific USB VID:PID, a specific USB VID:PID
that belongs only to my specific hardware piece and to no one else -
so where is the harm?

M~