[PATCH v2 0/6] serial ports: add ability to suppress raising DTR & RTS on open

[PATCH v2 0/6] serial ports: add ability to suppress raising DTR & RTS on open

[Mychaela N. Falconia]

There exist serial-attached specialty hardware devices, both RS-232
and USB-serial, in which DTR and/or RTS modem control outputs have
been repurposed for non-standard uses.  Many people will argue that
such devices shouldn't exist because they go against RS-232 standards,
but they exist primarily for reasons of practicality and economics.
If someone has an end application that consists of a data-leads-only
UART (TxD & RxD only, no flow control or modem control) plus one or two
ad hoc control lines (could be thought of as GPIOs, but unidirectional
from the host to the target), it is far more practical and economical
to repurpose otherwise unused DTR & RTS control lines in the already
given serial interface than to build extra hardware interfaces at
extra cost for the purpose of conveying the needed custom signaling.

As just one example, consider a device that consists of a data-leads-only
UART and two control buttons, Button1 and Button2.  Telling designers
of such specialty hw and their user communities that they should run a
second (or third, if there already is a second) separate serial port
to their target, and put an extra processor in the device that would
listen on that extra serial port and interpret ASCII '1' as a Button1
command, ASCII '2' as a Button2 command is a non-starter, when they
can instead wire up the circuit such that a pulse on RTS triggers
Button1 and a pulse on DTR triggers Button2.

Unfortunately such specialty hw applications were not considered back
in 1970s and 80s when UNIX serial port handling behaviour was codified
into standards, and it so happened that the standard POSIX/SUS
behaviour of automatically and unconditionally asserting both DTR and
RTS on serial port open is incompatible with specialty hw devices in
which these signals are repurposed for non-standard uses.

Changing standard serial port behaviour is not possible, hence the
only way to add support for non-standard hw devices is to implement
some completely optional, OS-specific mechanism by which an application
can ask the OS to override standard behaviour on a per-port basis
prior to opening the ttyXX serial device.

In 2021-04 FreeBSD 13.0 became the first Unix-style OS that makes it
possible for specialty applications to open a serial port (both
traditional PC serial ports and USB-serial are supported) without ever
asserting DTR & RTS.  However, the specific way in which this feature
is implemented in FreeBSD depends on their /dev/ttyXX.init devices,
which for better or worse do not exist in Linux.  Therefore, a
functionally comparable Linux implementation (giving the same
in-the-end capability to users) would have to make use of some other
OS-specific trick, this time Linux-specific.

The present patch series implements the desired capability and
functionality in Linux, providing two ways to mark a particular serial
device as subject to special non-standard operation:

1) For both "hard" serial ports (serial_core) and USB-serial, there is
   a new boolean sysfs attribute attached to the tty device.  Writing
   1 into this attribute activates the special mode of suppressing
   DTR & RTS on open, writing 0 restores standard operation.  In the
   present version of this patch series, this sysfs atttribute is
   named nordy - this name was chosen by Johan Hovold in late 2020.
   I personally dislike this choice of name, but I defer to more
   senior kernel developers and maintainers on the final choice of
   name, and will be happy to rename to whatever is chosen.  To end
   users, it is functionality that matters, not the name.

2) In the present day, it will often be the case that the user-visible
   interface to the specialty hw device is USB rather than RS-232, with
   a USB-serial converter chip integrated internally inside the custom
   device that repurposes its DTR and RTS outputs.  In such hardware
   situations, the EEPROM-assigned custom USB VID:PID on the USB-serial
   device identifies the hw product as a whole, including the part that
   repurposes DTR & RTS, rather than just the USB-serial converter part.
   At times when the described situation does hold and there does exist
   a custom USB VID:PID that explicitly and umambiguously identifies
   the device as one that requires the non-standard behaviour, then it
   is only proper that whenever support is added to a USB-serial driver
   to recognize the custom USB ID in question, a quirk should be
   applied at the same time, setting the flag for the required
   non-standard operation.

Changes from v1 to v2:

* Fix the notation for authorship attribution, making it clear that
  Johan Hovold is the author of 3 out of 6 patches in this series.

* Revert sysfs atttribute name to Johan's original choice of nordy,
  pending community consensus on the choice of name.

* For the 3 patches that originate from Johan, revert all commit
  comments to his original wording.

Johan Hovold (3):
  tty: add port flag to suppress ready signalling on open
  serial: core: add sysfs attribute to suppress ready signalling on open
  USB: serial: add sysfs attribute to suppress ready signalling on open

Mychaela N. Falconia (3):
  serial: core: fully suppress raising DTR & RTS on open if nordy is set
  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 |  7 ++++++
 drivers/tty/serial/serial_core.c    | 28 ++++++++++++++++++++++-
 drivers/tty/tty_port.c              |  2 +-
 drivers/usb/serial/bus.c            | 35 +++++++++++++++++++++++++++--
 drivers/usb/serial/ftdi_sio.c       | 45 ++++++++++++++++++++++++++++++-------
 drivers/usb/serial/ftdi_sio_ids.h   |  1 +
 include/linux/tty_port.h            | 11 +++++++++
 7 files changed, 117 insertions(+), 12 deletions(-)

-- 
2.9.0

base-commit: 25e02ba60f0fbe65ba07553b5b2b8867726273c4

[PATCH v2 1/6] tty: add port flag to suppress ready signalling on open

[Mychaela N. Falconia]

From: Johan Hovold <johan@kernel.org>

Add a NORDY port flag to suppress raising the modem-control lines on
open to signal DTE readiness.

This can be used to implement a NORDY termios control flag to complement
HUPCL, which controls lowering of the modem-control lines on final
close.

Initially drivers can export the flag through sysfs, which also allows
control over the lines on first open.

This can be used to prevent undesirable side-effects on open for
applications where the DTR and RTS lines are used for non-standard
purposes such as generating power-on and reset pulses.

Signed-off-by: Johan Hovold <johan@kernel.org>
[rebase: move from include/linux/tty.h to include/linux/tty_port.h]
Signed-off-by: Mychaela N. Falconia <falcon@freecalypso.org>
---
 drivers/tty/tty_port.c   |  2 +-
 include/linux/tty_port.h | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index 880608a65773..84831c1b6d10 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -441,7 +441,7 @@ EXPORT_SYMBOL(tty_port_carrier_raised);
  */
 void tty_port_raise_dtr_rts(struct tty_port *port)
 {
-	if (port->ops->dtr_rts)
+	if (port->ops->dtr_rts && !tty_port_nordy(port))
 		port->ops->dtr_rts(port, 1);
 }
 EXPORT_SYMBOL(tty_port_raise_dtr_rts);
diff --git a/include/linux/tty_port.h b/include/linux/tty_port.h
index 58e9619116b7..0c1581cbfe2b 100644
--- a/include/linux/tty_port.h
+++ b/include/linux/tty_port.h
@@ -133,6 +133,7 @@ struct tty_port {
 #define TTY_PORT_CHECK_CD	4	/* carrier detect enabled */
 #define TTY_PORT_KOPENED	5	/* device exclusively opened by
 					   kernel */
+#define TTY_PORT_NORDY		6	/* do not raise DTR/RTS on open */
 
 void tty_port_init(struct tty_port *port);
 void tty_port_link_device(struct tty_port *port, struct tty_driver *driver,
@@ -226,6 +227,16 @@ static inline void tty_port_set_kopened(struct tty_port *port, bool val)
 	assign_bit(TTY_PORT_KOPENED, &port->iflags, val);
 }
 
+static inline bool tty_port_nordy(const struct tty_port *port)
+{
+	return test_bit(TTY_PORT_NORDY, &port->iflags);
+}
+
+static inline void tty_port_set_nordy(struct tty_port *port, bool val)
+{
+	assign_bit(TTY_PORT_NORDY, &port->iflags, val);
+}
+
 struct tty_struct *tty_port_tty_get(struct tty_port *port);
 void tty_port_tty_set(struct tty_port *port, struct tty_struct *tty);
 int tty_port_carrier_raised(struct tty_port *port);
-- 
2.9.0

[PATCH v2 2/6] serial: core: add sysfs attribute to suppress ready signalling on open

[Mychaela N. Falconia]

From: Johan Hovold <johan@kernel.org>

Add a nordy sysfs attribute to suppress raising the modem-control lines
on open to signal DTE readiness.

This can be used to prevent undesirable side-effects on open for
applications where the DTR and RTS lines are used for non-standard
purposes such as generating power-on and reset pulses.

Signed-off-by: Johan Hovold <johan@kernel.org>
Signed-off-by: Mychaela N. Falconia <falcon@freecalypso.org>
---
 Documentation/ABI/testing/sysfs-tty |  7 +++++++
 drivers/tty/serial/serial_core.c    | 26 ++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-tty b/Documentation/ABI/testing/sysfs-tty
index 820e412d38a8..98cb5cf0af75 100644
--- a/Documentation/ABI/testing/sysfs-tty
+++ b/Documentation/ABI/testing/sysfs-tty
@@ -161,3 +161,10 @@ Contact:	Andy Shevchenko <andriy.shevchenko@linux.intel.com>
 Description:
 		 Allows user to detach or attach back the given device as
 		 kernel console. It shows and accepts a boolean variable.
+
+What:		/sys/class/tty/ttyS0/nordy
+Date:		November 2020
+Contact:	Johan Hovold <johan@kernel.org>
+Description:
+		 Show and store the port NORDY flag which suppresses raising
+		 the modem-control lines on open to signal DTE readiness.
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 9a85b41caa0a..a17ac4efaceb 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2870,6 +2870,30 @@ static ssize_t console_store(struct device *dev,
 	return ret < 0 ? ret : count;
 }
 
+static ssize_t nordy_show(struct device *dev, struct device_attribute *attr,
+				char *buf)
+{
+	struct tty_port *port = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d\n", tty_port_nordy(port));
+}
+
+static ssize_t nordy_store(struct device *dev, struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	struct tty_port *port = dev_get_drvdata(dev);
+	bool val;
+	int ret;
+
+	ret = kstrtobool(buf, &val);
+	if (ret)
+		return ret;
+
+	tty_port_set_nordy(port, val);
+
+	return count;
+}
+
 static DEVICE_ATTR_RO(uartclk);
 static DEVICE_ATTR_RO(type);
 static DEVICE_ATTR_RO(line);
@@ -2884,6 +2908,7 @@ static DEVICE_ATTR_RO(io_type);
 static DEVICE_ATTR_RO(iomem_base);
 static DEVICE_ATTR_RO(iomem_reg_shift);
 static DEVICE_ATTR_RW(console);
+static DEVICE_ATTR_RW(nordy);
 
 static struct attribute *tty_dev_attrs[] = {
 	&dev_attr_uartclk.attr,
@@ -2900,6 +2925,7 @@ static struct attribute *tty_dev_attrs[] = {
 	&dev_attr_iomem_base.attr,
 	&dev_attr_iomem_reg_shift.attr,
 	&dev_attr_console.attr,
+	&dev_attr_nordy.attr,
 	NULL
 };
 
-- 
2.9.0

[PATCH v2 3/6] serial: core: fully suppress raising DTR & RTS on open if nordy is set

[Mychaela N. Falconia]

When nordy sysfs attribute is written as 1, TTY_PORT_NORDY is set,
and the call to raise DTR & RTS in tty_port_raise_dtr_rts() is
suppressed.  However, there is one other place where these signals
are also raised on open: uart_port_startup() in
drivers/tty/serial/serial_core.c - this other point of raising
DTR & RTS also needs to be suppressed if TTY_PORT_NORDY is set.

Signed-off-by: Mychaela N. Falconia <falcon@freecalypso.org>
---
 drivers/tty/serial/serial_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index a17ac4efaceb..2a558813afa1 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -236,7 +236,7 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state,
 		 * Setup the RTS and DTR signals once the
 		 * port is open and ready to respond.
 		 */
-		if (init_hw && C_BAUD(tty))
+		if (init_hw && !tty_port_nordy(&state->port) && C_BAUD(tty))
 			uart_port_dtr_rts(uport, 1);
 	}
 
-- 
2.9.0

[PATCH v2 4/6] USB: serial: add sysfs attribute to suppress ready signalling on open

[Mychaela N. Falconia]

From: Johan Hovold <johan@kernel.org>

Add a nordy sysfs attribute to suppress raising the modem-control lines
on open to signal DTE readiness.

This can be used to prevent undesirable side-effects on open for
applications where the DTR and RTS lines are used for non-standard
purposes such as generating power-on and reset pulses.

Signed-off-by: Johan Hovold <johan@kernel.org>
Signed-off-by: Mychaela N. Falconia <falcon@freecalypso.org>
---
 drivers/usb/serial/bus.c | 35 +++++++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/serial/bus.c b/drivers/usb/serial/bus.c
index 9e38142acd38..25f8218b2bce 100644
--- a/drivers/usb/serial/bus.c
+++ b/drivers/usb/serial/bus.c
@@ -29,6 +29,37 @@ static int usb_serial_device_match(struct device *dev,
 	return 0;
 }
 
+static ssize_t nordy_show(struct device *dev, struct device_attribute *attr,
+				char *buf)
+{
+	struct usb_serial_port *port = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d\n", tty_port_nordy(&port->port));
+}
+
+static ssize_t nordy_store(struct device *dev, struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	struct usb_serial_port *port = dev_get_drvdata(dev);
+	bool val;
+	int ret;
+
+	ret = kstrtobool(buf, &val);
+	if (ret)
+		return ret;
+
+	tty_port_set_nordy(&port->port, val);
+
+	return count;
+}
+static DEVICE_ATTR_RW(nordy);
+
+static struct attribute *tty_attrs[] = {
+	&dev_attr_nordy.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(tty);
+
 static int usb_serial_device_probe(struct device *dev)
 {
 	struct usb_serial_port *port = to_usb_serial_port(dev);
@@ -50,8 +81,8 @@ static int usb_serial_device_probe(struct device *dev)
 	}
 
 	minor = port->minor;
-	tty_dev = tty_port_register_device(&port->port, usb_serial_tty_driver,
-					   minor, dev);
+	tty_dev = tty_port_register_device_attr(&port->port,
+			usb_serial_tty_driver, minor, dev, port, tty_groups);
 	if (IS_ERR(tty_dev)) {
 		retval = PTR_ERR(tty_dev);
 		goto err_port_remove;
-- 
2.9.0

[PATCH v2 5/6] USB: serial: ftdi_sio: pass port to quirk port_probe functions

[Mychaela N. Falconia]

The original code passed only the pointer to the ftdi_private struct
to quirk port_probe functions.  However, some quirks may need to be
applied conditionally only to some channels of a multichannel FT2232x
or FT4232H device, and if a given quirk's port_probe function needs
to figure out which channel of a multichannel device is currently
being considered, it needs access to the port pointer passed to the
ftdi_sio_port_probe() function, so it can traverse USB data structures
from there.

Signed-off-by: Mychaela N. Falconia <falcon@freecalypso.org>
---
 drivers/usb/serial/ftdi_sio.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 49c08f07c969..6523a36dcc45 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -88,15 +88,15 @@ struct ftdi_private {
 struct ftdi_sio_quirk {
 	int (*probe)(struct usb_serial *);
 	/* Special settings for probed ports. */
-	void (*port_probe)(struct ftdi_private *);
+	void (*port_probe)(struct usb_serial_port *);
 };
 
 static int   ftdi_jtag_probe(struct usb_serial *serial);
 static int   ftdi_NDI_device_setup(struct usb_serial *serial);
 static int   ftdi_stmclite_probe(struct usb_serial *serial);
 static int   ftdi_8u2232c_probe(struct usb_serial *serial);
-static void  ftdi_USB_UIRT_setup(struct ftdi_private *priv);
-static void  ftdi_HE_TIRA1_setup(struct ftdi_private *priv);
+static void  ftdi_USB_UIRT_setup(struct usb_serial_port *port);
+static void  ftdi_HE_TIRA1_setup(struct usb_serial_port *port);
 
 static const struct ftdi_sio_quirk ftdi_jtag_quirk = {
 	.probe	= ftdi_jtag_probe,
@@ -2238,11 +2238,11 @@ static int ftdi_sio_port_probe(struct usb_serial_port *port)
 
 	mutex_init(&priv->cfg_lock);
 
-	if (quirk && quirk->port_probe)
-		quirk->port_probe(priv);
-
 	usb_set_serial_port_data(port, priv);
 
+	if (quirk && quirk->port_probe)
+		quirk->port_probe(port);
+
 	ftdi_determine_type(port);
 	ftdi_set_max_packet_size(port);
 	if (read_latency_timer(port) < 0)
@@ -2263,8 +2263,10 @@ static int ftdi_sio_port_probe(struct usb_serial_port *port)
 /* Setup for the USB-UIRT device, which requires hardwired
  * baudrate (38400 gets mapped to 312500) */
 /* Called from usbserial:serial_probe */
-static void ftdi_USB_UIRT_setup(struct ftdi_private *priv)
+static void ftdi_USB_UIRT_setup(struct usb_serial_port *port)
 {
+	struct ftdi_private *priv = usb_get_serial_port_data(port);
+
 	priv->flags |= ASYNC_SPD_CUST;
 	priv->custom_divisor = 77;
 	priv->force_baud = 38400;
@@ -2273,8 +2275,10 @@ static void ftdi_USB_UIRT_setup(struct ftdi_private *priv)
 /* Setup for the HE-TIRA1 device, which requires hardwired
  * baudrate (38400 gets mapped to 100000) and RTS-CTS enabled.  */
 
-static void ftdi_HE_TIRA1_setup(struct ftdi_private *priv)
+static void ftdi_HE_TIRA1_setup(struct usb_serial_port *port)
 {
+	struct ftdi_private *priv = usb_get_serial_port_data(port);
+
 	priv->flags |= ASYNC_SPD_CUST;
 	priv->custom_divisor = 240;
 	priv->force_baud = 38400;
-- 
2.9.0

[PATCH v2 6/6] USB: serial: ftdi_sio: add support for FreeCalypso DUART28C adapter

[Mychaela N. Falconia]

FreeCalypso DUART28C is an FT2232D-based USB to dual UART adapter
with a special quirk: Channel B RTS and DTR outputs (BDBUS2 and BDBUS4
on the chip) have been repurposed to drive PWON and RESET controls
on Calypso targets.  The circuit is wired such that BDBUS[24] high
(RTS/DTR inactive) is the normal state with Iota VRPC controls
NOT activated, whereas BDBUS[24] low (RTS or DTR active) turn ON
the corresponding open drain control signal drivers.

A special ftdi_sio driver quirk is needed in order to suppress
automatic assertion of DTR & RTS on device open: this device's
special PWON and RESET control drivers MUST NOT be activated
when the port is ordinarily opened for plain serial communication,
instead they must only be activated when a special userspace
application explicitly requests such activation with a TIOCMBIS ioctl.
These special userspace applications are responsible for making the
needed pulse with a TIOCMBIS, delay, TIOCMBIC sequence.

The special quirk is conditionalized on the DUART28C adapter's custom
USB ID, and is further limited to FT2232D Channel B only: Channel A
is wired normally, with the chip's ADBUS2 and ADBUS4 outputs
actually being RTS and DTR rather than something else.

Signed-off-by: Mychaela N. Falconia <falcon@freecalypso.org>
---
 drivers/usb/serial/ftdi_sio.c     | 25 +++++++++++++++++++++++++
 drivers/usb/serial/ftdi_sio_ids.h |  1 +
 2 files changed, 26 insertions(+)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 6523a36dcc45..f62d9f804c73 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -97,6 +97,7 @@ static int   ftdi_stmclite_probe(struct usb_serial *serial);
 static int   ftdi_8u2232c_probe(struct usb_serial *serial);
 static void  ftdi_USB_UIRT_setup(struct usb_serial_port *port);
 static void  ftdi_HE_TIRA1_setup(struct usb_serial_port *port);
+static void  ftdi_duart28c_setup(struct usb_serial_port *port);
 
 static const struct ftdi_sio_quirk ftdi_jtag_quirk = {
 	.probe	= ftdi_jtag_probe,
@@ -122,6 +123,10 @@ static const struct ftdi_sio_quirk ftdi_8u2232c_quirk = {
 	.probe	= ftdi_8u2232c_probe,
 };
 
+static const struct ftdi_sio_quirk ftdi_duart28c_quirk = {
+	.port_probe = ftdi_duart28c_setup,
+};
+
 /*
  * The 8U232AM has the same API as the sio except for:
  * - it can support MUCH higher baudrates; up to:
@@ -1050,6 +1055,8 @@ static const struct usb_device_id id_table_combined[] = {
 		.driver_info = (kernel_ulong_t)&ftdi_jtag_quirk },
 	{ USB_DEVICE(FTDI_VID, FTDI_FALCONIA_JTAG_UNBUF_PID),
 		.driver_info = (kernel_ulong_t)&ftdi_jtag_quirk },
+	{ USB_DEVICE(FTDI_VID, FTDI_FALCONIA_DUART28C_PID),
+		.driver_info = (kernel_ulong_t)&ftdi_duart28c_quirk },
 	{ }					/* Terminating entry */
 };
 
@@ -2372,6 +2379,24 @@ static int ftdi_stmclite_probe(struct usb_serial *serial)
 	return 0;
 }
 
+/*
+ * FreeCalypso DUART28C is an FT2232D-based USB to dual UART adapter
+ * with a special quirk: Channel B RTS and DTR outputs (BDBUS2 and BDBUS4
+ * on the chip) have been repurposed to drive PWON and RESET controls.
+ *
+ * Only Channel B is subject to the quirk - Channel A needs to retain
+ * standard POSIX/SUS behaviour.
+ */
+static void ftdi_duart28c_setup(struct usb_serial_port *port)
+{
+	struct usb_serial *serial = port->serial;
+	struct usb_interface *intf = serial->interface;
+	int ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
+
+	if (ifnum == 1)
+		tty_port_set_nordy(&port->port, true);
+}
+
 static void ftdi_sio_port_remove(struct usb_serial_port *port)
 {
 	struct ftdi_private *priv = usb_get_serial_port_data(port);
diff --git a/drivers/usb/serial/ftdi_sio_ids.h b/drivers/usb/serial/ftdi_sio_ids.h
index d1a9564697a4..6ff2509e54a2 100644
--- a/drivers/usb/serial/ftdi_sio_ids.h
+++ b/drivers/usb/serial/ftdi_sio_ids.h
@@ -45,6 +45,7 @@
  */
 #define FTDI_FALCONIA_JTAG_BUF_PID	0x7150
 #define FTDI_FALCONIA_JTAG_UNBUF_PID	0x7151
+#define FTDI_FALCONIA_DUART28C_PID	0x7152
 
 /* Sienna Serial Interface by Secyourit GmbH */
 #define FTDI_SIENNA_PID		0x8348
-- 
2.9.0

Re: [PATCH v2 2/6] serial: core: add sysfs attribute to suppress ready signalling on open

[Jiri Slaby]

On 31. 05. 22, 6:36, Mychaela N. Falconia wrote:
> From: Johan Hovold <johan@kernel.org>
> 
> Add a nordy sysfs attribute to suppress raising the modem-control lines
> on open to signal DTE readiness.
> 
> This can be used to prevent undesirable side-effects on open for
> applications where the DTR and RTS lines are used for non-standard
> purposes such as generating power-on and reset pulses.
> 
> Signed-off-by: Johan Hovold <johan@kernel.org>
> Signed-off-by: Mychaela N. Falconia <falcon@freecalypso.org>
> ---
>   Documentation/ABI/testing/sysfs-tty |  7 +++++++
>   drivers/tty/serial/serial_core.c    | 26 ++++++++++++++++++++++++++
>   2 files changed, 33 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-tty b/Documentation/ABI/testing/sysfs-tty
> index 820e412d38a8..98cb5cf0af75 100644
> --- a/Documentation/ABI/testing/sysfs-tty
> +++ b/Documentation/ABI/testing/sysfs-tty
> @@ -161,3 +161,10 @@ Contact:	Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>   Description:
>   		 Allows user to detach or attach back the given device as
>   		 kernel console. It shows and accepts a boolean variable.
> +
> +What:		/sys/class/tty/ttyS0/nordy

s@ttyS0@ttyS<x>@

> +Date:		November 2020
> +Contact:	Johan Hovold <johan@kernel.org>
> +Description:
> +		 Show and store the port NORDY flag which suppresses raising
> +		 the modem-control lines on open to signal DTE readiness.
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 9a85b41caa0a..a17ac4efaceb 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -2870,6 +2870,30 @@ static ssize_t console_store(struct device *dev,
>   	return ret < 0 ? ret : count;
>   }
>   
> +static ssize_t nordy_show(struct device *dev, struct device_attribute *attr,
> +				char *buf)
> +{
> +	struct tty_port *port = dev_get_drvdata(dev);
> +
> +	return sprintf(buf, "%d\n", tty_port_nordy(port));

sysfs_emit()? (Even if I know %d won't overflow PAGE_SIZE :P.)


thanks,
-- 
js
suse labs

Re: [PATCH v2 2/6] serial: core: add sysfs attribute to suppress ready signalling on open

[Mychaela Falconia]

Jiri Slaby wrote:

> s@ttyS0@ttyS<x>@
> [...]
> sysfs_emit()? (Even if I know %d won't overflow PAGE_SIZE :P.)

Thanks for the feedback - saving it for v3 of my patch series.

Any comments, positive or negative, on the principal/philosophical
idea behind this proposed patch series?

M~

Re: [PATCH v2 2/6] serial: core: add sysfs attribute to suppress ready signalling on open

[Jiri Slaby]

On 02. 06. 22, 10:35, Mychaela Falconia wrote:
> Jiri Slaby wrote:
> 
>> s@ttyS0@ttyS<x>@
>> [...]
>> sysfs_emit()? (Even if I know %d won't overflow PAGE_SIZE :P.)
> 
> Thanks for the feedback - saving it for v3 of my patch series.
> 
> Any comments, positive or negative, on the principal/philosophical
> idea behind this proposed patch series?

Neutral. As long as you break no currently supported devices.

I've just noticed the double negative "!tty_port_nordy()" on both calls 
of that function. I guess there was already a discussion about the 
naming, but wouldn't it make more sense to dub it like tty_port_do_rtscts()?

thanks,
-- 
js
suse labs

Re: [PATCH v2 2/6] serial: core: add sysfs attribute to suppress ready signalling on open

[Mychaela Falconia]

Jiri Slaby wrote:

> As long as you break no currently supported devices.

This condition is a given, and has always been satisfied in all of my
patch proposals, including those patches which I currently instruct my
users to install locally to work with my GSM devices.

> I've just noticed the double negative "!tty_port_nordy()" on both calls of
> that function. I guess there was already a discussion about the naming,
> but wouldn't it make more sense to dub it like tty_port_do_rtscts()?

There are two aspects to this naming issue:

1) names of internal flags and functions that exist only inside the
kernel;

2) name of the sysfs attribute exported to userspace.

The latter part constitutes ABI, hence it is the one that calls for
serious reflection on the choice of name.  The sysfs portion of the
present patch series (as opposed to the USB VID:PID-keyed portion)
originates from Johan, and the nordy name for the sysfs attribute
(and for the internal flag and function for consistency) was his
choice.

My own preferred choice for the sysfs attribute name would be something
like manual_rtsdtr rather than nordy; I feel that a name such as
manual_rtsdtr conveys what is being done: asking the kernel to put
these modem control outputs under manual control (TIOCMBIS & TIOCMBIC)
instead of automatic assertion on open.  Johan argued a year and a
half ago that nordy was a better name as it indicated "please suppress
ready signaling" - it's a different perspective.  Johan, are you still
around?  Do you still favor nordy as the sysfs attribute name?

At the end of the day, I will be happy with _any_ name for the sysfs
attribute - to end users it's the functionality that matters, not the
name - and because it's ABI, once the sysfs attribute is implemented
with some given name, it will stay.  So, can some authoritative people
please weigh in on how this sysfs attribute should be named?  Should
it be nordy?  manual_rtsdtr?  Something else?

M~