All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] serial ports: add ability to suppress raising DTR & RTS on open
@ 2022-05-27 22:27 Mychaela N. Falconia
  2022-05-30 13:26 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 4+ messages in thread
From: Mychaela N. Falconia @ 2022-05-27 22:27 UTC (permalink / raw)
  To: Johan Hovold, Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-serial, linux-usb, mychaela.falconia

Back in 1970s UNIX a poor design decision was made regarding serial port
handling; this bad design has been codified into standards (POSIX, SUS)
and persists into present-day Linux.  In 2021 FreeBSD 13.0 became the
first Unix-style OS to fix the problem; the present patch series aims
to implement a similar solution in Linux.

The root of the problem is the POSIX/SUS-codified design decision to
always automatically assert both DTR and RTS modem control signals on
the initial open of a serial port, without giving userspace any ability
to say "no, please don't do it".  This design is flawed on a fundamental
philosophical level: an OS kernel needs to provide a mechanism for
applications to operate on hardware, rather than insert its own mind
in the middle.  If a user wishes to open any kind of hardware device
and perform arbitrary operations on that device, the kernel needs to
allow this access unobstructed, provided that the user has the necessary
permissions.  If the user is asking to merely open a hardware device,
but the kernel refuses to provide such "simple open" access without
mandatorily imposing some specific hardware action along with that
open, this behaviour needs to be considered a defect, a design bug.

Because the design bug is codified in POSIX, SUS etc, it cannot be
simply fixed - a change to default behaviour would violate standards
and break bazillion existing applications.  Instead the only possible
solutions at this point in time have to take the form of creative
workarounds - and by necessity, such creative workarounds should NOT
be expected to be "pretty" or architecturally clean.  The
architecturally-clean boat has sailed a few decades ago - in the
present time, non-pretty workarounds are required.

The solution implemented in FreeBSD relies on a feature of that OS
which does not exist in Linux: initial-state devices.  In FreeBSD
each ttyXX device also has a corresponding ttyXX.init device node;
the purpose of this .init device is to set the initial termios state
that will apply every time the regular ttyXX device is opened.
Furthermore, these .init devices have an additional property: opening
an init device does not raise modem control signals, unlike opening
of regular tty devices.  Having these init devices already available
(implemented a long time ago for unrelated reasons) made it possible
for FreeBSD to implement the option of suppressing automatic assertion
of DTR & RTS on open by way of a simple termios flag: they defined a
new termios flag CNO_RTSDTR (added to cflags) which needs to be set
on the /dev/ttyXX.init or /dev/cuaXX.init device _before_ opening
/dev/ttyXX or /dev/cuaXX for actual serial communication.

However, because Linux does not have any equivalent to FreeBSD's
/dev/ttyXX.init devices, FreeBSD's CNO_RTSDTR solution cannot be
copied to Linux verbatim.  A naive approach of copying FreeBSD's
termios flag and expecting users to set it on the regular /dev/ttyXX
device (lacking /dev/ttyXX.init) would not work: the act of opening
/dev/ttyXX for the purpose of setting the flag would in itself
assert DTR & RTS, which on some hardware devices can cause irreparable
harm to the user.  Instead the closest thing that can be implemented
in Linux would be a sysfs attribute attached to tty<x> serial devices,
and this sysfs approach is what the present patch series implements.

With this approach, a serial device control application (typically
developed together with the custom hw device in which DTR and RTS
are repurposed for non-standard uses) that seeks to support both
Linux and FreeBSD can treat both OSes in a unified manner, using
an abstraction function to hide the small difference between the two.
An application can implement an abstraction function like
set_initial_rtsdtr_mode(bool autoraise) with only the guts of this
function different between Linux and FreeBSD: the Linux version
would open /sys/class/tty/ttyXX/manual_rtsdtr and write into that
attribute file, whereas the FreeBSD version would open /dev/ttyXX.init
and set or clear CNO_RTSDTR termios flag.

It needs to be emphasized that the present patch series is a purely
additive change, merely adding a new optional mode of operation that
needs to be explicitly invoked by those users who desire it.  There
is NO change to any existing serial port behaviour or to anything
else pre-existing: until and unless a user explicitly writes into
the new /sys/class/tty/ttyXX/manual_rtsdtr attribute and sets it to 1,
absolutely nothing changes, exactly zero impact.

There is also one other possibility: there exist some hardware devices
in which a USB-serial converter chip and the application circuit behind
that chip (which repurposes DTR & RTS for non-standard uses) are
integrated into a single monolithic device, with a custom USB VID:PID
identifying the hardware device as a whole.  Because they are custom
ID codes not known at all to "naive" OS kernels, adding Linux support
for any such hw device will necessarily require adding knowledge of
that custom VID:PID to the appropriate USB-serial driver - and if it
is *known* that this paricular hardware device is wired in such a way
that requires the manual_rtsdtr flag to be set, then it makes the most
sense for the USB-serial driver to set the flag in the device-specific
quirk.  The present patch series adds support for one such device.

Mychaela N. Falconia (6):
  tty: add port flag to suppress raising DTR & RTS on open
  serial: core: add sysfs attribute to suppress raising DTR & RTS on
    open
  serial: core: fully suppress raising DTR & RTS on open if
    manual_rtsdtr
  USB: serial: add sysfs attribute to suppress raising DTR & RTS on open
  USB: serial: ftdi_sio: pass port to quirk port_probe functions
  USB: serial: ftdi_sio: add support for FreeCalypso DUART28C adapter

 Documentation/ABI/testing/sysfs-tty | 10 +++++++++
 drivers/tty/serial/serial_core.c    | 30 ++++++++++++++++++++++++-
 drivers/tty/tty_port.c              |  2 +-
 drivers/usb/serial/bus.c            | 36 +++++++++++++++++++++++++++--
 drivers/usb/serial/ftdi_sio.c       | 45 ++++++++++++++++++++++++++++++-------
 drivers/usb/serial/ftdi_sio_ids.h   |  1 +
 include/linux/tty_port.h            | 11 +++++++++
 7 files changed, 123 insertions(+), 12 deletions(-)

-- 
2.9.0

base-commit: 25e02ba60f0fbe65ba07553b5b2b8867726273c4

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

* Re: [PATCH 0/6] serial ports: add ability to suppress raising DTR & RTS on open
  2022-05-27 22:27 [PATCH 0/6] serial ports: add ability to suppress raising DTR & RTS on open Mychaela N. Falconia
@ 2022-05-30 13:26 ` Greg Kroah-Hartman
  2022-05-30 16:05   ` Mychaela Falconia
  0 siblings, 1 reply; 4+ messages in thread
From: Greg Kroah-Hartman @ 2022-05-30 13:26 UTC (permalink / raw)
  To: Mychaela N. Falconia
  Cc: Johan Hovold, Jiri Slaby, linux-serial, linux-usb, mychaela.falconia

On Fri, May 27, 2022 at 10:27:03PM +0000, Mychaela N. Falconia wrote:
> Back in 1970s UNIX a poor design decision was made regarding serial port
> handling; this bad design has been codified into standards (POSIX, SUS)
> and persists into present-day Linux.

Odd that you state it this way, as this IS the RS-232 standard
requirement that was released in 1968, so codifying it into the POSIX
standard and using it in Linux today was a good thing so that we can use
Linux on hardware built for the standard.

To ignore the public, accepted, standard is to become an operating
system that does not follow the standard, which would not be a good
thing at all.

> In 2021 FreeBSD 13.0 became the
> first Unix-style OS to fix the problem; the present patch series aims
> to implement a similar solution in Linux.

Again, this is not a "problem", it is a "let us use these pins for
something that they were not designed to be used for" type of thing.
You are wanting to ignore the well known and very common standard of the
past 53 years because you wish to reuse a UART pin as a GPIO pin, which
is not any sort of standardized thing at all.

> The root of the problem is the POSIX/SUS-codified design decision to
> always automatically assert both DTR and RTS modem control signals on
> the initial open of a serial port, without giving userspace any ability
> to say "no, please don't do it".

Again, that is the standard, why wouldn't you want to do that?  To not
do that would be to break interoperability with millions of devices out
there (remember modems?)

> This design is flawed on a fundamental
> philosophical level: an OS kernel needs to provide a mechanism for
> applications to operate on hardware, rather than insert its own mind
> in the middle.  If a user wishes to open any kind of hardware device
> and perform arbitrary operations on that device, the kernel needs to
> allow this access unobstructed, provided that the user has the necessary
> permissions.  If the user is asking to merely open a hardware device,
> but the kernel refuses to provide such "simple open" access without
> mandatorily imposing some specific hardware action along with that
> open, this behaviour needs to be considered a defect, a design bug.

Again, this is not a design bug, but the requirement that the OS follow
the 50+ year old documented and accepted standard for this hardware
interface.  This is designed properly.

> Because the design bug is codified in POSIX, SUS etc, it cannot be
> simply fixed - a change to default behaviour would violate standards
> and break bazillion existing applications.  Instead the only possible
> solutions at this point in time have to take the form of creative
> workarounds - and by necessity, such creative workarounds should NOT
> be expected to be "pretty" or architecturally clean.  The
> architecturally-clean boat has sailed a few decades ago - in the
> present time, non-pretty workarounds are required.

Again, you are wanting a workaround for a limitation of your hardware
design, not a limitation of the RS-232 standard.  You are creating
non-compliant hardware and wish to control it in a way the hardware was
not originally intended to be controlled.

And that's fine, but don't speak of it as if we somehow messed up 53
years ago and no one has noticed it since then.  That's just
condencending to all of us who have maintained and worked with this
codebase and standard for these 50+ years.

On a personal note, I've been working on UARTs and RS-232 since "only"
1992 on a paid basis, and I have never heard of anyone objecting to this
portion of the RS-232 standard in a way to make it sound like it was
designed incorrectly this way.  But then again, I've only been doing
this for 30 years now, maybe I'm too new at this :)

> The solution implemented in FreeBSD relies on a feature of that OS
> which does not exist in Linux: initial-state devices.

Linux dropped those a long time ago for good reasons, let's not revisit
that design decision again please.

> There is also one other possibility: there exist some hardware devices
> in which a USB-serial converter chip and the application circuit behind
> that chip (which repurposes DTR & RTS for non-standard uses) are
> integrated into a single monolithic device, with a custom USB VID:PID
> identifying the hardware device as a whole.  Because they are custom
> ID codes not known at all to "naive" OS kernels, adding Linux support
> for any such hw device will necessarily require adding knowledge of
> that custom VID:PID to the appropriate USB-serial driver - and if it
> is *known* that this paricular hardware device is wired in such a way
> that requires the manual_rtsdtr flag to be set, then it makes the most
> sense for the USB-serial driver to set the flag in the device-specific
> quirk.  The present patch series adds support for one such device.
> 
> Mychaela N. Falconia (6):
>   tty: add port flag to suppress raising DTR & RTS on open
>   serial: core: add sysfs attribute to suppress raising DTR & RTS on
>     open
>   serial: core: fully suppress raising DTR & RTS on open if
>     manual_rtsdtr
>   USB: serial: add sysfs attribute to suppress raising DTR & RTS on open
>   USB: serial: ftdi_sio: pass port to quirk port_probe functions
>   USB: serial: ftdi_sio: add support for FreeCalypso DUART28C adapter

From what I recall with the original patch series, Johan is the author
of these, not you.  Rebasing and forwarding on is great, but please
never drop original authorship of patches, that's just rude, and in
some cases, ripe for legal worries.

Can you fix that all up, tone down the "this is all wrong" verbage, and
properly resend the series as a joined patch series (your emails are not
threaded properly at all and our tools can not find them correct, just
use 'git send-email' and that would solve it.) and then after the merge
window, I'll reconsider this series.

Also please document what has changed since Johan's original submission
to now, and why it should now be accepted when it was rejected then.

thanks,

greg k-h

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

* Re: [PATCH 0/6] serial ports: add ability to suppress raising DTR & RTS on open
  2022-05-30 13:26 ` Greg Kroah-Hartman
@ 2022-05-30 16:05   ` Mychaela Falconia
  2022-05-30 16:23     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 4+ messages in thread
From: Mychaela Falconia @ 2022-05-30 16:05 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Johan Hovold, Jiri Slaby, linux-serial, linux-usb

Greg K-H wrote:

> To ignore the public, accepted, standard is to become an operating
> system that does not follow the standard, which would not be a good
> thing at all.

So is FreeBSD 13.x a bad OS then, because it offers an *option* of
suppressing this particular standard-mandated behaviour?

> Again, that is the standard, why wouldn't you want to do that?  To not
> do that would be to break interoperability with millions of devices out
> there (remember modems?)

I don't need to "remember" modems, I use them almost every day in my
test lab - but none of my proposed patch versions (nor FreeBSD's recent
CNO_RTSDTR feature addition) break interoperability with anything,
instead both FreeBSD's solution (for their OS) and my proposed Linux
patches merely provide an *option* for more specialized hw devices
that require different handling.

> > The solution implemented in FreeBSD relies on a feature of that OS
> > which does not exist in Linux: initial-state devices.
>
> Linux dropped those a long time ago for good reasons, let's not revisit
> that design decision again please.

Dropped?  Are you saying that Linux once had them at some point in the
past?  If so, I was unaware of such history - I thought Linux never
had them to begin with, just like Classic UNIX (4.3BSD etc) never had
them either.  But in any case, was I ever asking for ttyXX.init
devices to be added to Linux?  No, I was not - instead I was merely
pointing out why a naive "straightforward" port of the new CNO_RTSDTR
termios flag from FreeBSD to Linux wouldn't work.

> From what I recall with the original patch series, Johan is the author
> of these, not you.  Rebasing and forwarding on is great, but please
> never drop original authorship of patches, that's just rude, and in
> some cases, ripe for legal worries.

In the case of the 3 patches which originate from Johan (1/6, 2/6 and
4/6), I submitted them with the following attribution:

From: me
[...]
Co-developed-by: Johan
Signed-off-by: Johan
Signed-off-by: me

My reading of Documentation/process/submitting-patches.rst told me
this was the correct protocol - but if I got it wrong, what is the
correct way then?  Specifically, what is the correct protocol when
(in this chronological order):

1) Developer A prepares a patch, and then drops it, i.e., does not
continue fighting to get it mainlined;

2) Developer B picks up A's patch and makes further modifications to
it, modifications which A might disagree with.

> Can you fix that all up, tone down the "this is all wrong" verbage, and
> properly resend the series as a joined patch series (your emails are not
> threaded properly at all and our tools can not find them correct, just
> use 'git send-email' and that would solve it.) and then after the merge
> window, I'll reconsider this series.

Please accept my apologies for missing the thread-linking headers - I
didn't realize until after I sent the series that they were critical
for reviewers.  I can resend the series with those thread-linking
headers added, and also changing the cover letter comments to be more
in line with your worldview - but first I need a clarification on the
authorship attribution issue above, as it is one thing which, if wrong
indeed, would need to be fixed right away.

> Also please document what has changed since Johan's original submission
> to now, and why it should now be accepted when it was rejected then.

The main change is not in the patch code, but in circumstances: in
between then and now, I have discovered that FreeBSD's solution
actually works correctly (unlike Johan's superficially-similar-sounding
termios flag proposal for Linux) in that it does NOT tell users to
"please suffer just once" on the very initial open of a freshly
plugged-in USB serial device.  Also FreeBSD's release process is
slower than Linux', and as a result of this slowness, the feature
addition in question only appeared in a release in 2021-04 - some
months after my previous attempts here - even though it was committed
to FreeBSD HEAD in 2019.  This new development in FreeBSD shines a
flashlight at Linux being the backward one here, and I was hoping that
this new change in circumstances would help create an impetus for
making the fix in Linux too.

As far as actual code changes between Johan's original patch version
and my current modified version, there is just one code change and one
significant comment change:

Code change: I have renamed the sysfs attribute from nordy to
manual_rtsdtr, with a similar name change for the internal flag, which
IMO makes it clearer what it is actually happening and why/when this
special mode might be needed.

Comment change: I removed all references to the termios flag idea
(which was Johan's repeatedly-stated longer-term goal and desire), as
that one can never work acceptably for the present problem in Linux,
or in any OS that lacks initial-state devices.

Oh, and there was one place in the code (DTR/RTS assertion in
serial_core.c, applying only to "hard" serial ports and not USB) which
Johan missed in his patch series, an omission which I only noticed
when preparing my current series - hence patch 3/6 is my original
work.

M~

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

* Re: [PATCH 0/6] serial ports: add ability to suppress raising DTR & RTS on open
  2022-05-30 16:05   ` Mychaela Falconia
@ 2022-05-30 16:23     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 4+ messages in thread
From: Greg Kroah-Hartman @ 2022-05-30 16:23 UTC (permalink / raw)
  To: Mychaela Falconia; +Cc: Johan Hovold, Jiri Slaby, linux-serial, linux-usb

On Mon, May 30, 2022 at 08:05:00AM -0800, Mychaela Falconia wrote:
> Greg K-H wrote:
> 
> > To ignore the public, accepted, standard is to become an operating
> > system that does not follow the standard, which would not be a good
> > thing at all.
> 
> So is FreeBSD 13.x a bad OS then, because it offers an *option* of
> suppressing this particular standard-mandated behaviour?

I never said that, please do not be disengenous, that will only get you
added to people's email filters to be ignored.

> > Again, that is the standard, why wouldn't you want to do that?  To not
> > do that would be to break interoperability with millions of devices out
> > there (remember modems?)
> 
> I don't need to "remember" modems, I use them almost every day in my
> test lab - but none of my proposed patch versions (nor FreeBSD's recent
> CNO_RTSDTR feature addition) break interoperability with anything,
> instead both FreeBSD's solution (for their OS) and my proposed Linux
> patches merely provide an *option* for more specialized hw devices
> that require different handling.

That's fine, but again, you were ranting against the existing standard
as if that was the thing that is wrong and broken here.  Not your
one-off hardware implementation that does not follow the existing
standard.  Please read the context you cut out.

> > > The solution implemented in FreeBSD relies on a feature of that OS
> > > which does not exist in Linux: initial-state devices.
> >
> > Linux dropped those a long time ago for good reasons, let's not revisit
> > that design decision again please.
> 
> Dropped?  Are you saying that Linux once had them at some point in the
> past?

Yes we had much the same thing, but they might have worked a bit
differently.  Check the 2.2 kernel days or earlier.

> > From what I recall with the original patch series, Johan is the author
> > of these, not you.  Rebasing and forwarding on is great, but please
> > never drop original authorship of patches, that's just rude, and in
> > some cases, ripe for legal worries.
> 
> In the case of the 3 patches which originate from Johan (1/6, 2/6 and
> 4/6), I submitted them with the following attribution:
> 
> From: me
> [...]
> Co-developed-by: Johan
> Signed-off-by: Johan
> Signed-off-by: me
> 
> My reading of Documentation/process/submitting-patches.rst told me
> this was the correct protocol - but if I got it wrong, what is the
> correct way then?  Specifically, what is the correct protocol when
> (in this chronological order):

"From:" would be from Johan as he wrote the commit.

thanks,

greg k-h

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

end of thread, other threads:[~2022-05-30 16:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-27 22:27 [PATCH 0/6] serial ports: add ability to suppress raising DTR & RTS on open Mychaela N. Falconia
2022-05-30 13:26 ` Greg Kroah-Hartman
2022-05-30 16:05   ` Mychaela Falconia
2022-05-30 16:23     ` Greg Kroah-Hartman

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.