diff mbox series

[1/5] tty: add port flag to suppress ready signalling on open

Message ID 20201130153742.9163-2-johan@kernel.org
State New
Headers show
Series tty: add flag to suppress ready signalling on open | expand

Commit Message

Johan Hovold Nov. 30, 2020, 3:37 p.m. UTC
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 use 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>
---
 drivers/tty/tty_port.c |  2 +-
 include/linux/tty.h    | 14 ++++++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

Comments

Mychaela Falconia Nov. 30, 2020, 11:36 p.m. UTC | #1
> 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 use 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>

Reviewed-by: Mychaela N. Falconia <falcon@freecalypso.org>
Jiri Slaby Dec. 1, 2020, 5:49 a.m. UTC | #2
On 30. 11. 20, 16:37, Johan Hovold wrote:
> --- a/include/linux/tty.h
> +++ b/include/linux/tty.h
> @@ -683,6 +684,19 @@ static inline void tty_port_set_kopened(struct tty_port *port, bool val)
>   		clear_bit(TTY_PORT_KOPENED, &port->iflags);
>   }
>   
> +static inline bool tty_port_nordy(struct tty_port *port)

port can be const here.

> +{
> +	return test_bit(TTY_PORT_NORDY, &port->iflags);
> +}
> +
> +static inline void tty_port_set_nordy(struct tty_port *port, bool val)
> +{
> +	if (val)
> +		set_bit(TTY_PORT_NORDY, &port->iflags);
> +	else
> +		clear_bit(TTY_PORT_NORDY, &port->iflags);

We have assign_bit() for these cases these days.

thanks,
Mychaela Falconia Dec. 1, 2020, 7:09 a.m. UTC | #3
On 11/30/20, Jiri Slaby <jirislaby@kernel.org> wrote:
> port can be const here.
> [...]
> We have assign_bit() for these cases these days.

Johan's patch adding test and set accessor inline functions for the
new flag follows the style of the existing accessor inline functions
for previously existing flags, for the sake of consistency. If we are
going to use the new style (const for test functions, assign_bit() for
set functions) for the new flag, then we should also change all
existing ones for consistency. In terms of patch splitting, would it
be most kosher to have one patch that updates the style of existing
accessor inline functions, and then the interesting patch that adds
the new flag?

M~
Jiri Slaby Dec. 1, 2020, 7:16 a.m. UTC | #4
On 01. 12. 20, 8:09, Mychaela Falconia wrote:
> On 11/30/20, Jiri Slaby <jirislaby@kernel.org> wrote:

>> port can be const here.

>> [...]

>> We have assign_bit() for these cases these days.

> 

> Johan's patch adding test and set accessor inline functions for the

> new flag follows the style of the existing accessor inline functions

> for previously existing flags, for the sake of consistency. If we are

> going to use the new style (const for test functions, assign_bit() for

> set functions) for the new flag, then we should also change all

> existing ones for consistency. In terms of patch splitting, would it

> be most kosher to have one patch that updates the style of existing

> accessor inline functions, and then the interesting patch that adds

> the new flag?


Yes. Or the other way around. Add this new using const+assign_bit and 
convert the rest on the top of the series.

thanks,
-- 
js
Johan Hovold Dec. 1, 2020, 8:46 a.m. UTC | #5
On Tue, Dec 01, 2020 at 06:49:07AM +0100, Jiri Slaby wrote:
> On 30. 11. 20, 16:37, Johan Hovold wrote:
> > --- a/include/linux/tty.h
> > +++ b/include/linux/tty.h
> > @@ -683,6 +684,19 @@ static inline void tty_port_set_kopened(struct tty_port *port, bool val)
> >   		clear_bit(TTY_PORT_KOPENED, &port->iflags);
> >   }
> >   
> > +static inline bool tty_port_nordy(struct tty_port *port)
> 
> port can be const here.

Sure, but see below.

> > +{
> > +	return test_bit(TTY_PORT_NORDY, &port->iflags);
> > +}
> > +
> > +static inline void tty_port_set_nordy(struct tty_port *port, bool val)
> > +{
> > +	if (val)
> > +		set_bit(TTY_PORT_NORDY, &port->iflags);
> > +	else
> > +		clear_bit(TTY_PORT_NORDY, &port->iflags);
> 
> We have assign_bit() for these cases these days.

Right, but for both your comments this follows the pattern used by the
other port-flag helpers.

I can add a preparatory patch updating the current helpers, but I don't
think this needs to be a blocker.

Johan
diff mbox series

Patch

diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index ea80bf872f54..2613debc1d06 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -415,7 +415,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.h b/include/linux/tty.h
index a99e9b8e4e31..31414efd8661 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -267,6 +267,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 */
 
 /*
  * Where all of the state associated with a tty is kept while the tty
@@ -683,6 +684,19 @@  static inline void tty_port_set_kopened(struct tty_port *port, bool val)
 		clear_bit(TTY_PORT_KOPENED, &port->iflags);
 }
 
+static inline bool tty_port_nordy(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)
+{
+	if (val)
+		set_bit(TTY_PORT_NORDY, &port->iflags);
+	else
+		clear_bit(TTY_PORT_NORDY, &port->iflags);
+}
+
 extern struct tty_struct *tty_port_tty_get(struct tty_port *port);
 extern void tty_port_tty_set(struct tty_port *port, struct tty_struct *tty);
 extern int tty_port_carrier_raised(struct tty_port *port);