All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] a family of FTDI-based devices that need ftdi_sio quirks
@ 2020-09-16  1:56 Mychaela N. Falconia
  2020-09-29 10:13 ` Johan Hovold
  0 siblings, 1 reply; 10+ messages in thread
From: Mychaela N. Falconia @ 2020-09-16  1:56 UTC (permalink / raw)
  To: Johan Hovold; +Cc: linux-usb, mychaela.falconia

Hello Johan,

This patch series adds support for a family of FTDI-based devices that
need special quirks in the ftdi_sio driver.  The devices in question
are produced by my hobby company Falconia Partners LLC, and I got a
block of PIDs from FTDI (from their VID), so I assign custom USB IDs
when I program FT2232 EEPROMs on my boards.  These custom USD IDs are
intended to facilitate conditionalized quirks in the ftdi_sio driver,
as implemented in this patch series.

The 3 devices (3 USB IDs) for which this patch series adds support
consist of two JTAG+UART adapters and one dual UART adapter.  Support
for the two JTAG+UART adapters consists of nothing more than adding
them to the USB ID table with the already existing ftdi_jtag_quirk
applied, but the dual UART adapter (DUART28C) is more involved.

The special quirk with DUART28C is that FT2232D BDBUS2 and BDBUS4
outputs (which would normally be Channel B RTS and DTR, respectively)
have been repurposed to drive power and reset controls to Calypso
targets, and these power and reset controls must not be triggered
on their own - instead their control needs to be left to special
userspace applications.  The problem is that the standard behaviour
of all tty devices under Linux (not just ftdi_sio and not just USB
serial, but apparently all ttys) is that DTR and RTS are always
unconditionally asserted as soon as the tty device (ttyUSB in the
present case) is opened, and userspace does not get a chance to
intervene.  It is not my place to debate whether this behaviour is
good or not for true serial ports where DTR/RTS really are DTR/RTS,
but it is a total killer for non-standard hardware where the USB to
serial chip's DTR & RTS outputs have been repurposed to do something
disruptive like power and reset control.

The solution that works beautifully in my own hardware lab is contained
in the present patch series: the new DUART28C-specific quirk simply
suppresses automatic assertion of DTR&RTS for Channel B on this device.
Userspace can then trigger PWON and RESET actions as needed with
TIOCMBIS ioctls (followed by a delay and TIOCMBIC to make a pulse),
and everything works beautifully.  If I could get this DUART28C quirk
(conditionalized on the USB ID which FTDI officially allocated to me)
mainlined, it would spare my users the pain of having to apply this
patch themselves locally.

TIA,
Mychaela

Mychaela N. Falconia (3):
  USB: serial: ftdi_sio: add support for FreeCalypso JTAG+UART adapters
  USB: serial: ftdi_sio: pass port to quirk port_probe functions
  USB: serial: ftdi_sio: add support for FreeCalypso DUART28C adapter

 drivers/usb/serial/ftdi_sio.c     | 72 +++++++++++++++++++++++++++++++++------
 drivers/usb/serial/ftdi_sio_ids.h |  8 +++++
 2 files changed, 70 insertions(+), 10 deletions(-)

-- 
2.9.0

base-commit: 2bb70f0a4b238323e4e2f392fc3ddeb5b7208c9e

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/3] a family of FTDI-based devices that need ftdi_sio quirks
  2020-09-16  1:56 [PATCH 0/3] a family of FTDI-based devices that need ftdi_sio quirks Mychaela N. Falconia
@ 2020-09-29 10:13 ` Johan Hovold
  2020-09-29 19:40   ` Mychaela Falconia
  0 siblings, 1 reply; 10+ messages in thread
From: Johan Hovold @ 2020-09-29 10:13 UTC (permalink / raw)
  To: Mychaela N. Falconia; +Cc: Johan Hovold, linux-usb, mychaela.falconia

On Wed, Sep 16, 2020 at 01:56:21AM +0000, Mychaela N. Falconia wrote:
> Hello Johan,
> 
> This patch series adds support for a family of FTDI-based devices that
> need special quirks in the ftdi_sio driver.  The devices in question
> are produced by my hobby company Falconia Partners LLC, and I got a
> block of PIDs from FTDI (from their VID), so I assign custom USB IDs
> when I program FT2232 EEPROMs on my boards.  These custom USD IDs are
> intended to facilitate conditionalized quirks in the ftdi_sio driver,
> as implemented in this patch series.
> 
> The 3 devices (3 USB IDs) for which this patch series adds support
> consist of two JTAG+UART adapters and one dual UART adapter.  Support
> for the two JTAG+UART adapters consists of nothing more than adding
> them to the USB ID table with the already existing ftdi_jtag_quirk
> applied, but the dual UART adapter (DUART28C) is more involved.
> 
> The special quirk with DUART28C is that FT2232D BDBUS2 and BDBUS4
> outputs (which would normally be Channel B RTS and DTR, respectively)
> have been repurposed to drive power and reset controls to Calypso
> targets, and these power and reset controls must not be triggered
> on their own - instead their control needs to be left to special
> userspace applications.  The problem is that the standard behaviour
> of all tty devices under Linux (not just ftdi_sio and not just USB
> serial, but apparently all ttys) is that DTR and RTS are always
> unconditionally asserted as soon as the tty device (ttyUSB in the
> present case) is opened, and userspace does not get a chance to
> intervene.  It is not my place to debate whether this behaviour is
> good or not for true serial ports where DTR/RTS really are DTR/RTS,
> but it is a total killer for non-standard hardware where the USB to
> serial chip's DTR & RTS outputs have been repurposed to do something
> disruptive like power and reset control.
> 
> The solution that works beautifully in my own hardware lab is contained
> in the present patch series: the new DUART28C-specific quirk simply
> suppresses automatic assertion of DTR&RTS for Channel B on this device.
> Userspace can then trigger PWON and RESET actions as needed with
> TIOCMBIS ioctls (followed by a delay and TIOCMBIC to make a pulse),
> and everything works beautifully.  If I could get this DUART28C quirk
> (conditionalized on the USB ID which FTDI officially allocated to me)
> mainlined, it would spare my users the pain of having to apply this
> patch themselves locally.

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.

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):

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?

> Mychaela N. Falconia (3):
>   USB: serial: ftdi_sio: add support for FreeCalypso JTAG+UART adapters
>   USB: serial: ftdi_sio: pass port to quirk port_probe functions
>   USB: serial: ftdi_sio: add support for FreeCalypso DUART28C adapter
> 
>  drivers/usb/serial/ftdi_sio.c     | 72 +++++++++++++++++++++++++++++++++------
>  drivers/usb/serial/ftdi_sio_ids.h |  8 +++++
>  2 files changed, 70 insertions(+), 10 deletions(-)

Johan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/3] a family of FTDI-based devices that need ftdi_sio quirks
  2020-09-29 10:13 ` Johan Hovold
@ 2020-09-29 19:40   ` Mychaela Falconia
  2020-10-05 10:57     ` Johan Hovold
  0 siblings, 1 reply; 10+ messages in thread
From: Mychaela Falconia @ 2020-09-29 19:40 UTC (permalink / raw)
  To: Johan Hovold; +Cc: linux-usb

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~

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/3] a family of FTDI-based devices that need ftdi_sio quirks
  2020-09-29 19:40   ` Mychaela Falconia
@ 2020-10-05 10:57     ` Johan Hovold
  2020-10-05 20:02       ` Mychaela Falconia
  0 siblings, 1 reply; 10+ messages in thread
From: Johan Hovold @ 2020-10-05 10:57 UTC (permalink / raw)
  To: Mychaela Falconia; +Cc: Johan Hovold, linux-usb

On Tue, Sep 29, 2020 at 11:40:47AM -0800, Mychaela Falconia wrote:

> > 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.

Yeah, your application would then need to deassert after open.

I believe some arduino boards use a capacitor to trigger a reset pulse
whenever DTR/RTS is asserted instead.

I wasn't expecting you to support "any serial port"; my point was that
when possible it's better to accommodate the standard behaviour of these
signals and have it work on any OS (version) rather than requiring
patching drivers.

> 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.

Right. But let's see where we end up on this, the more general the
solution we can come up with the better.

> 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.

Good.

> > 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?

You're quirk flag was named no-auto-dtr-rts or similar, so it doesn't
seem unreasonable to honour it also in case someone, perhaps
unknowingly, sets CRTSCTS.

But a less-intrusive patch for this is of course preferred so we can
probably leave CRTSCTS handling unchanged. But then the same should be
done for B0 I think.

> 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?

Right. But we should still try to aim at general solution that can be
reused by other devices if needed (cf. the jtag quirk).

Let me give this some more thought.

Johan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/3] a family of FTDI-based devices that need ftdi_sio quirks
  2020-10-05 10:57     ` Johan Hovold
@ 2020-10-05 20:02       ` Mychaela Falconia
  2020-10-27 20:18         ` Mychaela Falconia
  2020-11-30 15:13         ` Johan Hovold
  0 siblings, 2 replies; 10+ messages in thread
From: Mychaela Falconia @ 2020-10-05 20:02 UTC (permalink / raw)
  To: Johan Hovold; +Cc: linux-usb

Hi Johan,

> Yeah, your application would then need to deassert after open.
>
> I believe some arduino boards use a capacitor to trigger a reset pulse
> whenever DTR/RTS is asserted instead.

But you are still asking me to modify the hardware in ways that would
make the product worse for the end user.  Just consider the current
situation:

* Calypso target board (FC Caramel2) can run either with or without a
host computer connected via the DUART28 USB-serial interface.  When
there is no host computer connected to either of the two UARTs, the
board can be usefully operated via its local LCD and keypad UI
elements, provided that the user loaded the necessary firmware into
flash when she did previously connect her host PC via DUART28.

* The board has human-finger-operated PWON and RESET pushbutton
switches on it, allowing the user to command the needed signals to the
power & reset control chip without requiring a connected host computer.
Each of the two pushbutton switches is wired between the corresponding
chipset signal and GND.

* The user can freely connect and disconnect the DUART28 adapter (both
the USB host to DUART28 connection and the connection from DUART28 to
the Calypso target board can be freely made and broken) with the
Calypso target board is either switched-on or switched-off state, and
NONE of these connect/disconnect manipulations will trigger any resets
or on/off switching actions on the Calypso.

* If the user has applied the necessary ftdi_sio driver patch,
programmed the DUART28 on-board EEPROM to the 'C' configuration
(assigning DUART28C custom USB PID) and connected the wires from
DUART28 open drain outputs CTL1 and CTL2 to Calypso target board PWON
and RESET, then DUART28 CTL1 becomes a programmatic PWON and CTL2
becomes a programmatic RESET.  Running fc-pulse-rts /dev/ttyUSB1 is
now equivalent to pressing PWON with one's finger, and running
fc-pulse-dtr /dev/ttyUSB1 is equivalent to pressing the RESET button
instead.  Note the selective pulsing of just one signal, either RTS
or DTR, but not both!  There are also more complex "macro" operations
that combine boot control pulsing with actual serial communication:
for example, fc-loadtool -Pdtr /dev/ttyUSB1 will pulse DTR (intending
a reset) while sending "beacons" on the serial port every 13 ms,
seeking to catch the attention of the chip's mask ROM bootloader.

* When no boot control pulsing is desired, the user can freely run
ordinary serial communication programs on both UARTs (standard AT
command software on /dev/ttyUSB0 and rvinterf on /dev/ttyUSB1) WITHOUT
any resets or PWON pulses being triggered ever.

I don't see how any of your proposed alternative hardware designs
would ever accomplish the same objectives of having NO resets or PWON
actions glitch-triggered when the user merely plugs in or unplugs the
USB cable, or when the user desires to run an ordinary serial
communication program *without* triggering either RESET or PWON
controls.  Oh, and don't forget the ability to selectively trigger
either one of PWON and RESET as desired, without producing even a
slightest glitch on the other control line.

> I wasn't expecting you to support "any serial port"; my point was that
> when possible it's better to accommodate the standard behaviour of these
> signals and have it work on any OS (version) rather than requiring
> patching drivers.

Please be more careful with phrases like "standard behaviour of these
signals" and "any OS": I argue that what Linux does with DTR & RTS is
NOT universally standard, and is in fact *defective* compared to what
Windows does.  Linux exhibits a fundamental philosophical design bug
in that whenever a serial port is opened, DTR and RTS are automatically
asserted without ever giving the userspace program a chance to say
"no, please don't do it" - that's a bug, not a feature.  I know that
Linux got this behaviour (which I hereby argue to be a philosophical
design bug) from POSIX, and that POSIX in turn got it from Original
UNIX - being a retrocomputing enthusiast, I happen to know that
Ancient UNIX exhibits the exact same misfeature of unconditional
DTR & RTS assertion on open - but just because it has been a very
long-standing tradition going all the way back to original 1970s UNIX
does not make it right.

If you look at a real independent competitor to Linux, namely Windows,
a totally independent OS design not hindered by those particular
traditions, you will see that it does NOT exhibit the same philosophical
design bug: under Windows one can open a serial port with a
CreateFile("COM1", ...) API call, and this action in itself will NOT
cause any of the modem control signals to change state.  Instead the
userspace program gets the complete freedom to manipulate modem control
signals *as desired* with a SetCommState() API call, *without* the OS
and its drivers producing an unstoppable, unpreventable glitch on DTR
and RTS immediately on open.

Of course the most proper fix would be to correct the Unix/POSIX/Linux
serial port handling philosophical design bug at its root, adding a
flag like O_NODTR to the open syscall that would prevent automatic
assertion of DTR & RTS on open, just like including O_NONBLOCK in the
open flags prevents a hang waiting for DCD to become asserted.  But
producing such a patch and getting it accepted would be far beyond my
capabilities, and even if such a change were to be made in current
mainline, it would not help practical end users: it would be too much
of a "new feature" change to get into stable kernels, end users of
older kernels would need to apply the patch locally, and making a
purely local application of a big change like adding a new open flag
(propagating it into /usr/include headers seen by userspace) is not
something I can reasonably ask my end users to do.

Hence I am going for the minimally invasive surgical approach instead:
suppress the unwanted on-open assertion of DTR & RTS in the ftdi_sio
driver, conditionalized on my custom USB ID so no one else will be
affected except my custom hardware, and the patch is small and
surgical such that end users should be able to apply it locally with
minimal pain.

> You're quirk flag was named no-auto-dtr-rts or similar, so it doesn't
> seem unreasonable to honour it also in case someone, perhaps
> unknowingly, sets CRTSCTS.

The flag is named no_auto_dtr_rts, and the associated comment in the
struct member definition reads:

	int no_auto_dtr_rts;	/* if non-zero, suppress automatic assertion
				   of DTR & RTS on device open */

The flag means no automatic assertion of DTR & RTS *on device open*,
i.e., producing the same behaviour which saner OSes like Windows
provide out of the box.  Once the open hurdle has been cleared, the
user has complete freedom of control: she can assert DTR and/or RTS
with TIOCMBIS, she can clear either or both of them with TIOCMBIC, and
she can also enable CRTSCTS if she so chooses - so why in the world
should the driver artificially block CRTSCTS?

If someone mistakenly enables CRTSCTS on Channel B (/dev/ttyUSB1 in
the absence of other ttyUSB devices) of a DUART28C, the result will be
bogus pulses on the RTS-turned-PWON line, and the resulting system
behaviour would be the same as if that user's unattended child were to
wander into the lab and playfully press the PWON button on the board
with her little pinky finger.  It is not the kernel's job to guard
against the latter scenario, so why does it need to go out of its way
to guard against the former?

Oh, and setting CRTSCTS on Channel B of a DUART28C would also block
all serial communication on that channel, as the FT2232D chip will see
its CTS input as being always negated: the chip's BDBUS3 pin is
physically unconnected (due to being unused) in the hardware, and the
FT2232D chip's built-in pull-ups cause unused/unconnected inputs to be
sensed as CMOS high.  CMOS high equals RS-232 inactive, thus CTS is
always sensed as being inactive, and setting CRTSCTS would stop all
communication - thus there is NO valid use case for setting CRTSCTS on
DUART28 Channel B.

> But a less-intrusive patch for this is of course preferred so we can
> probably leave CRTSCTS handling unchanged.

The actual implementation of CRTSCTS functionality resides in FTDI
chip hardware, the driver merely enables it with
FTDI_SIO_SET_FLOW_CTRL_REQUEST - and I don't see any valid reason to
artificially suppress this function when there is a quirk that says
"please don't twiddle control lines immediately on open without
userspace asking for it" - CRTSCTS in contrast *is* an explicit
request from userspace.

> But then the same should be done for B0 I think.

The whole concept of B0 meaning "hang up" is another bizarre legacy
from Ancient UNIX that crept its way into Linux through POSIX - before
TIOCMBIS and TIOCMBIC were invented, this B0 hack was provides as a
means for applications that needed to assert and negate DTR (RTS was
rarely used back then) without having to close and reopen the tty port.

When it comes to the no_auto_dtr_rts quirk, the important part is that
automatic assertion of DTR & RTS upon coming out of B0 state MUST be
suppressed - because having this suppression is the only way to prevent
unwanted automatic assertion of the signals if the previous state
(before userspace opening this serial port and operating on it) was B0.

What about going the other way: when userspace explicitly sets B0,
should the driver negate DTR & RTS?  I would be OK either way,
whichever way would allow my hardware support patch to be accepted,
but it seems more philosophically correct to me for the driver to
negate DTR & RTS upon userspace setting B0: it is an explicit request
from userspace, not an automatic unrequested action, and thus it
should honored.  This way userspace explicitly setting B0 is no
different from an explicit TIOCMBIC.

> Right. But we should still try to aim at general solution that can be
> reused by other devices if needed (cf. the jtag quirk).

It is indeed quite possible that other hardware engineers (or end
users of hardware that was designed for Windows) will run into the
same issue: someone has wired up DTR and/or RTS for a purpose that
does not tolerate glitches, everything works like a charm under
Windows, but Linux assininely insists on always asserting DTR & RTS on
device open whether the user wants it or not, and the hardware
glitches badly as a result.

If you let my current patch into the kernel and then someone else runs
into a similar situation, they should be able to set the exact same
no_auto_dtr_rts flag in their ID-code-specific quirk as appropriate
for their hw (DUART28C quirk needs this flag set for Channel B but not
for Channel A; other people's hw has every right to be different), and
it should allow those people to exercise full arbitrary control over
both DTR and RTS from userspace as appropriate for their application,
even if their hw specifics are quite different from mine:

* Absolutely no signal state change will happen upon the mere act of
opening the ttyUSB device.

* If someone wants to have some signal state changes upon open, they
can trivially follow their open syscall with TIOCMBIS and/or TIOCMBIC
ioctls as appropriate for their hw.

* No signal state change will happen when going from B0 to Bxxx, and
this behaviour is required in order to avoid unstoppable control signal
glitches if the initial state prior to userspace open was B0.

* If userspace is explicitly setting B0, it is an explicit userspace
request to drop DTR & RTS just like TIOCMBIC, so it gets honored.

* Whether or not DTR & RTS will be dropped upon port close will depend
on whether HUPCL has been set or not.  Because HUPCL is a userspace-
controlled flag, someone could argue that if userspace set HUPCL, then
userspace wants DTR & RTS to be negated on port close, and that this
request should be honored - hence the current behaviour of my present
patch is defended for both drop-on-close and drop-on-B0 cases.

* CRTSCTS still works like it always did - thus if someone else's hw
is wired differently from mine in that they have an RTS signal that
works as flow control, but they still need to block automatic
assertion of RTS and/or DTR on device open, then they can open the
serial port, optionally manipulate DTR with TIOCMBIS or TIOCMBIC as
needed, and then set CRTSCTS if that is what their application calls
for.

> Let me give this some more thought.

Please tell me if you would like me to make some specific changes to
my proposed patch, or if I should let my current v3 stand as it is
while you think about it.

M~

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/3] a family of FTDI-based devices that need ftdi_sio quirks
  2020-10-05 20:02       ` Mychaela Falconia
@ 2020-10-27 20:18         ` Mychaela Falconia
  2020-10-28  7:17           ` Johan Hovold
  2020-11-30 15:13         ` Johan Hovold
  1 sibling, 1 reply; 10+ messages in thread
From: Mychaela Falconia @ 2020-10-27 20:18 UTC (permalink / raw)
  To: Johan Hovold; +Cc: linux-usb

Hi Johan,

Our last exchange regarding my proposed ftdi_sio driver patch adding
the needed quirk for DUART28C took place 3 weeks ago on October 5.
Your last argument against my patch was this one:

https://marc.info/?l=linux-usb&m=160189545216969&w=2

And here is my rebuttal to that argument made the same day:

https://marc.info/?l=linux-usb&m=160192817717108&w=2

In your October 5 argument you wrote:

> Let me give this some more thought.

It has been 3 weeks - do you have any more thoughts that address my
not yet answered rebuttal arguments in defense of my patch?  My key
arguments are:

* The "standard" behaviour of Linux and other Unix-derived OSes of
unconditionally asserting DTR & RTS on tty port open (or upon leaving
B0 state) without giving userspace any ability to say "no, please
don't do it" is a philosophical design bug, one that goes all the way
back to original 1970s UNIX - but long-standing tradition does not
make right.

* Particularly in the present age of USB-serial adapters with LVCMOS
rather than RS-232 electrical signals, this Unix/POSIX/Linux serial
port handling philosophical design bug is hampering hardware engineers'
ability to produce otherwise clean and elegant circuit designs.

* Because Windows does NOT exhibit the same philosophical design bug
in this regard as Unix/POSIX/Linux, there may be hw devices that were
made for use with Windows, that depend on glitch-free DTR and/or RTS,
and which will fail to work correctly with current Linux.  When such
cases occur, the party at fault is Linux and not the hardware design -
the hardware engineers were merely exercising their natural right to
make simple and elegant circuit designs that work well with OSes that
are free of philosophical design bugs, it is not our fault as hw
engineers that Linux inherited a philosophical design bug from Ancient
UNIX by way of POSIX.

* A minimally invasive surgical solution in the form of driver quirks
that suppress the traditional DTR & RTS behaviour for those specific
hw devices for which it is unacceptable is more practical than trying
to fix the root-cause Unix philosophical design bug 45 y too late.

I would appreciate a response to my arguments.

Sincerely,
Mychaela,
custom hardware designer,
she/her/hers

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/3] a family of FTDI-based devices that need ftdi_sio quirks
  2020-10-27 20:18         ` Mychaela Falconia
@ 2020-10-28  7:17           ` Johan Hovold
  0 siblings, 0 replies; 10+ messages in thread
From: Johan Hovold @ 2020-10-28  7:17 UTC (permalink / raw)
  To: Mychaela Falconia; +Cc: Johan Hovold, linux-usb

On Tue, Oct 27, 2020 at 12:18:13PM -0800, Mychaela Falconia wrote:

> In your October 5 argument you wrote:
> 
> > Let me give this some more thought.
> 
> It has been 3 weeks - do you have any more thoughts that address my
> not yet answered rebuttal arguments in defense of my patch?

No, sorry, I haven't read your last mail yet. But don't worry it's not
lost, and I've started processing patches again now that rc1 is out.

Johan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/3] a family of FTDI-based devices that need ftdi_sio quirks
  2020-10-05 20:02       ` Mychaela Falconia
  2020-10-27 20:18         ` Mychaela Falconia
@ 2020-11-30 15:13         ` Johan Hovold
  2020-11-30 20:15           ` Mychaela Falconia
  1 sibling, 1 reply; 10+ messages in thread
From: Johan Hovold @ 2020-11-30 15:13 UTC (permalink / raw)
  To: Mychaela Falconia; +Cc: Johan Hovold, linux-usb

Hi Mychaela, 

and sorry about the late follow-up on this.

I took a fresh look at this before going back and reading you last
mails and found that I'd landed in more or less the same conclusions as
you. Please see below.

On Mon, Oct 05, 2020 at 12:02:55PM -0800, Mychaela Falconia wrote:
> Hi Johan,
> 
> > Yeah, your application would then need to deassert after open.
> >
> > I believe some arduino boards use a capacitor to trigger a reset pulse
> > whenever DTR/RTS is asserted instead.
> 
> But you are still asking me to modify the hardware in ways that would
> make the product worse for the end user.  Just consider the current
> situation:

> I don't see how any of your proposed alternative hardware designs
> would ever accomplish the same objectives of having NO resets or PWON
> actions glitch-triggered when the user merely plugs in or unplugs the
> USB cable, or when the user desires to run an ordinary serial
> communication program *without* triggering either RESET or PWON
> controls.  Oh, and don't forget the ability to selectively trigger
> either one of PWON and RESET as desired, without producing even a
> slightest glitch on the other control line.

Yeah, you're right.

> > I wasn't expecting you to support "any serial port"; my point was that
> > when possible it's better to accommodate the standard behaviour of these
> > signals and have it work on any OS (version) rather than requiring
> > patching drivers.
> 
> Please be more careful with phrases like "standard behaviour of these
> signals" and "any OS": I argue that what Linux does with DTR & RTS is
> NOT universally standard, and is in fact *defective* compared to what
> Windows does.  Linux exhibits a fundamental philosophical design bug
> in that whenever a serial port is opened, DTR and RTS are automatically
> asserted without ever giving the userspace program a chance to say
> "no, please don't do it" - that's a bug, not a feature.  I know that
> Linux got this behaviour (which I hereby argue to be a philosophical
> design bug) from POSIX, and that POSIX in turn got it from Original
> UNIX - being a retrocomputing enthusiast, I happen to know that
> Ancient UNIX exhibits the exact same misfeature of unconditional
> DTR & RTS assertion on open - but just because it has been a very
> long-standing tradition going all the way back to original 1970s UNIX
> does not make it right.

I'd still call it standard behaviour since Linux has always handled it
this way (and so has earlier systems as you point out), but I see your
point.

> If you look at a real independent competitor to Linux, namely Windows,
> a totally independent OS design not hindered by those particular
> traditions, you will see that it does NOT exhibit the same philosophical
> design bug: under Windows one can open a serial port with a
> CreateFile("COM1", ...) API call, and this action in itself will NOT
> cause any of the modem control signals to change state.  Instead the
> userspace program gets the complete freedom to manipulate modem control
> signals *as desired* with a SetCommState() API call, *without* the OS
> and its drivers producing an unstoppable, unpreventable glitch on DTR
> and RTS immediately on open.

From the API docs I found, it seems the behaviour on open (CreateFile())
is configurable after opening the port. Perhaps the default can be set
through some other means too, unless it's driver dependent.

> Of course the most proper fix would be to correct the Unix/POSIX/Linux
> serial port handling philosophical design bug at its root, adding a
> flag like O_NODTR to the open syscall that would prevent automatic
> assertion of DTR & RTS on open, just like including O_NONBLOCK in the
> open flags prevents a hang waiting for DCD to become asserted.  But
> producing such a patch and getting it accepted would be far beyond my
> capabilities, and even if such a change were to be made in current
> mainline, it would not help practical end users: it would be too much
> of a "new feature" change to get into stable kernels, end users of
> older kernels would need to apply the patch locally, and making a
> purely local application of a big change like adding a new open flag
> (propagating it into /usr/include headers seen by userspace) is not
> something I can reasonably ask my end users to do.

Yeah, that may not be feasible.

> Hence I am going for the minimally invasive surgical approach instead:
> suppress the unwanted on-open assertion of DTR & RTS in the ftdi_sio
> driver, conditionalized on my custom USB ID so no one else will be
> affected except my custom hardware, and the patch is small and
> surgical such that end users should be able to apply it locally with
> minimal pain.

Right, but I still think this should be generalised somewhat.

> > But then the same should be done for B0 I think.
> 
> The whole concept of B0 meaning "hang up" is another bizarre legacy
> from Ancient UNIX that crept its way into Linux through POSIX - before
> TIOCMBIS and TIOCMBIC were invented, this B0 hack was provides as a
> means for applications that needed to assert and negate DTR (RTS was
> rarely used back then) without having to close and reopen the tty port.
> 
> When it comes to the no_auto_dtr_rts quirk, the important part is that
> automatic assertion of DTR & RTS upon coming out of B0 state MUST be
> suppressed - because having this suppression is the only way to prevent
> unwanted automatic assertion of the signals if the previous state
> (before userspace opening this serial port and operating on it) was B0.
> 
> What about going the other way: when userspace explicitly sets B0,
> should the driver negate DTR & RTS?  I would be OK either way,
> whichever way would allow my hardware support patch to be accepted,
> but it seems more philosophically correct to me for the driver to
> negate DTR & RTS upon userspace setting B0: it is an explicit request
> from userspace, not an automatic unrequested action, and thus it
> should honored.  This way userspace explicitly setting B0 is no
> different from an explicit TIOCMBIC.

I don't think we need to change the B0 handling. It will never be set
unless explicitly requested by the user.

> > Right. But we should still try to aim at general solution that can be
> > reused by other devices if needed (cf. the jtag quirk).
> 
> It is indeed quite possible that other hardware engineers (or end
> users of hardware that was designed for Windows) will run into the
> same issue: someone has wired up DTR and/or RTS for a purpose that
> does not tolerate glitches, everything works like a charm under
> Windows, but Linux assininely insists on always asserting DTR & RTS on
> device open whether the user wants it or not, and the hardware
> glitches badly as a result.

Right. This has come up the in past, and there are other more or less
established applications such has HAM radio that use these lines for
other purposes. So I agree, we should try to support it.

> If you let my current patch into the kernel and then someone else runs
> into a similar situation, they should be able to set the exact same
> no_auto_dtr_rts flag in their ID-code-specific quirk as appropriate
> for their hw (DUART28C quirk needs this flag set for Channel B but not
> for Channel A; other people's hw has every right to be different), and
> it should allow those people to exercise full arbitrary control over
> both DTR and RTS from userspace as appropriate for their application,
> even if their hw specifics are quite different from mine:
> 
> * Absolutely no signal state change will happen upon the mere act of
> opening the ttyUSB device.
> 
> * If someone wants to have some signal state changes upon open, they
> can trivially follow their open syscall with TIOCMBIS and/or TIOCMBIC
> ioctls as appropriate for their hw.
> 
> * No signal state change will happen when going from B0 to Bxxx, and
> this behaviour is required in order to avoid unstoppable control signal
> glitches if the initial state prior to userspace open was B0.
> 
> * If userspace is explicitly setting B0, it is an explicit userspace
> request to drop DTR & RTS just like TIOCMBIC, so it gets honored.
> 
> * Whether or not DTR & RTS will be dropped upon port close will depend
> on whether HUPCL has been set or not.  Because HUPCL is a userspace-
> controlled flag, someone could argue that if userspace set HUPCL, then
> userspace wants DTR & RTS to be negated on port close, and that this
> request should be honored - hence the current behaviour of my present
> patch is defended for both drop-on-close and drop-on-B0 cases.
> 
> * CRTSCTS still works like it always did - thus if someone else's hw
> is wired differently from mine in that they have an RTS signal that
> works as flow control, but they still need to block automatic
> assertion of RTS and/or DTR on device open, then they can open the
> serial port, optionally manipulate DTR with TIOCMBIS or TIOCMBIC as
> needed, and then set CRTSCTS if that is what their application calls
> for.
> 
> > Let me give this some more thought.

I've prepared a series adding generic support for use-cases such as
yours and respun your last two patches on top so that they use the
generic implementation instead.

I'm adding a new tty port flag that can be used to suppress the
assertion of DTR/RTS to signal DTE readiness on open named NORDY.
Eventually we can expose it through termios to match HUPCL that controls
the behaviour on final close. For now I'm only adding a sysfs interface
but that also allows control over these signals on first open.

Your FTDI quirk now only needs to set this flag on probe to override the
default behaviour. The end result with respect to your device should be
the same except for B0, which is still handled as before.

I'll post the series shortly.

Johan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/3] a family of FTDI-based devices that need ftdi_sio quirks
  2020-11-30 15:13         ` Johan Hovold
@ 2020-11-30 20:15           ` Mychaela Falconia
  2020-12-01  9:00             ` Johan Hovold
  0 siblings, 1 reply; 10+ messages in thread
From: Mychaela Falconia @ 2020-11-30 20:15 UTC (permalink / raw)
  To: Johan Hovold; +Cc: linux-usb

Hi Johan,

Thank you for finally taking the time to address the issue at hand.
Your generalized solution and your reimplementation of my DUART28C
quirk using your new tty port flag would indeed solve my immediate
problem of getting mainline Linux support for FreeCalypso DUART28C, so
I would be perfectly happy if your new patch series gets mainlined.

> I don't think we need to change the B0 handling. It will never be set
> unless explicitly requested by the user.

I originally thought that DTR & RTS assertion upon change from B0 to
Bxxx had to be suppressed because I thought that the initial termios
state of a completely cold ttyUSB port (created upon USB device plug-in
but never opened) had B0 in it - but now I see that this cold initial
state is B9600 rather than B0, so indeed you are right in that B0 can
only be set by users and thus can be ignored for purposes of
legacy-gunk-free custom hw like mine.

> I'm adding a new tty port flag that can be used to suppress the
> assertion of DTR/RTS to signal DTE readiness on open named NORDY.
> Eventually we can expose it through termios to match HUPCL that controls
> the behaviour on final close. For now I'm only adding a sysfs interface
> but that also allows control over these signals on first open.

I don't see how exposing this new flag via termios would be of any use
(it's a chicken and egg problem: one needs to open the tty device in
order to do termios ioctls on it, and if that initial open triggers
DTR/RTS hardware actions, then the end user is still screwed), but
making this flag accessible for setting and clearing via sysfs is a
good idea.

For hardware engineers like me who design and build their own boards
with the USB-serial chip fully embedded and who have their own custom
USB IDs, applying a driver quirk tied to that custom USB ID is still
the best solution in terms of ultimate friendliness to the lowly end
user of the finished hardware.  But for users who don't have the
luxury of a custom USB ID, i.e., users of a non-DTR-RTS-glitch-tolerant
RS-232 device connected to a standard PC COM port or via a generic
off-the-shelf USB to serial DE9M cable, being able to set the needed
quirk via sysfs can be a life saver.

> Your FTDI quirk now only needs to set this flag on probe to override the
> default behaviour. The end result with respect to your device should be
> the same except for B0, which is still handled as before.

LGTM,
Mychaela

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/3] a family of FTDI-based devices that need ftdi_sio quirks
  2020-11-30 20:15           ` Mychaela Falconia
@ 2020-12-01  9:00             ` Johan Hovold
  0 siblings, 0 replies; 10+ messages in thread
From: Johan Hovold @ 2020-12-01  9:00 UTC (permalink / raw)
  To: Mychaela Falconia; +Cc: Johan Hovold, linux-usb

On Mon, Nov 30, 2020 at 12:15:51PM -0800, Mychaela Falconia wrote:

> > I'm adding a new tty port flag that can be used to suppress the
> > assertion of DTR/RTS to signal DTE readiness on open named NORDY.
> > Eventually we can expose it through termios to match HUPCL that controls
> > the behaviour on final close. For now I'm only adding a sysfs interface
> > but that also allows control over these signals on first open.
> 
> I don't see how exposing this new flag via termios would be of any use
> (it's a chicken and egg problem: one needs to open the tty device in
> order to do termios ioctls on it, and if that initial open triggers
> DTR/RTS hardware actions, then the end user is still screwed), but
> making this flag accessible for setting and clearing via sysfs is a
> good idea.

Yeah, as I mentioned in the other thread a termios flag would not be
sufficient for your use case, but there could be other applications that
are more tolerant but still want control after first open.

I believe this is also the way Win32, which you referred to earlier,
works.

Johan

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-12-01  9:28 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-16  1:56 [PATCH 0/3] a family of FTDI-based devices that need ftdi_sio quirks Mychaela N. Falconia
2020-09-29 10:13 ` Johan Hovold
2020-09-29 19:40   ` Mychaela Falconia
2020-10-05 10:57     ` Johan Hovold
2020-10-05 20:02       ` Mychaela Falconia
2020-10-27 20:18         ` Mychaela Falconia
2020-10-28  7:17           ` Johan Hovold
2020-11-30 15:13         ` Johan Hovold
2020-11-30 20:15           ` Mychaela Falconia
2020-12-01  9:00             ` Johan Hovold

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.