[PATCH 00/15] random tty fixes

[PATCH 00/15] random tty fixes

[Jiri Slaby (SUSE)]

This is a collection of random fixes for tty I did while crawling
through the code. Mostly done for readability and understandability. No
behavior change intended (except for Documentation fixes).

Jiri Slaby (SUSE) (15):
  tty: n_tty: use 'retval' instead of 'c'
  tty: n_tty: rename and retype 'retval' in n_tty_ioctl()
  tty: n_tty: use min3() in copy_from_read_buf()
  tty: n_tty: invert the condition in copy_from_read_buf()
  tty: n_tty: use do-while in n_tty_check_{,un}throttle()
  tty: switch tty_{,un}throttle_safe() to return a bool
  tty: invert return values of tty_{,un}throttle_safe()
  tty: fix up and plug in tty_ioctl kernel-doc
  tty: fix kernel-doc for functions in tty.h
  tty: stop using ndash in kernel-doc
  tty: tty_buffer: use bool for 'restart' in
    tty_buffer_unlock_exclusive()
  tty: convert THROTTLE constants into enum
  tty: early return from send_break() on TTY_DRIVER_HARDWARE_BREAK
  tty: don't check for signal_pending() in send_break()
  tty: use 'if' in send_break() instead of 'goto'

 Documentation/driver-api/tty/index.rst     |   1 +
 Documentation/driver-api/tty/tty_ioctl.rst |  10 +
 drivers/tty/n_tty.c                        |  77 ++++---
 drivers/tty/tty.h                          |  13 +-
 drivers/tty/tty_buffer.c                   |   5 +-
 drivers/tty/tty_io.c                       |  36 ++--
 drivers/tty/tty_ioctl.c                    | 234 ++++++++++-----------
 drivers/tty/tty_port.c                     |   6 +-
 drivers/tty/vt/consolemap.c                |   2 +-
 drivers/tty/vt/vc_screen.c                 |   4 +-
 drivers/tty/vt/vt.c                        |   4 +-
 include/linux/tty.h                        |  25 +--
 12 files changed, 209 insertions(+), 208 deletions(-)
 create mode 100644 Documentation/driver-api/tty/tty_ioctl.rst

-- 
2.42.0

[PATCH 01/15] tty: n_tty: use 'retval' instead of 'c'

[Jiri Slaby (SUSE)]

In n_tty_read(), there is a separate int variable 'c' and is used only
to hold an int value returned from job_control(). There is also a
'retval' variable typed ssize_t. So drop this single occurrence of 'c'
and reuse 'retval' which is used on all other places to hold the value
returned from n_tty_read().

Note that 'retval' needs not be initialized now. Drop that.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/n_tty.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 6c9a408d67cd..71aa898b077a 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -2154,9 +2154,8 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file, u8 *kbuf,
 	struct n_tty_data *ldata = tty->disc_data;
 	u8 *kb = kbuf;
 	DEFINE_WAIT_FUNC(wait, woken_wake_function);
-	int c;
 	int minimum, time;
-	ssize_t retval = 0;
+	ssize_t retval;
 	long timeout;
 	bool packet;
 	size_t old_tail;
@@ -2192,9 +2191,9 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file, u8 *kbuf,
 		return kb - kbuf;
 	}
 
-	c = job_control(tty, file);
-	if (c < 0)
-		return c;
+	retval = job_control(tty, file);
+	if (retval < 0)
+		return retval;
 
 	/*
 	 *	Internal serialization of reads.
-- 
2.42.0

[PATCH 02/15] tty: n_tty: rename and retype 'retval' in n_tty_ioctl()

[Jiri Slaby (SUSE)]

The value stored to the current 'retval' is number of characters. It is
both obtained and put to user as unsigned. So make its type unsigned.
And provided it's not a "return value" per se, rename it to 'num'.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/n_tty.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 71aa898b077a..e917faa0b84c 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -2498,7 +2498,7 @@ static int n_tty_ioctl(struct tty_struct *tty, unsigned int cmd,
 		       unsigned long arg)
 {
 	struct n_tty_data *ldata = tty->disc_data;
-	int retval;
+	unsigned int num;
 
 	switch (cmd) {
 	case TIOCOUTQ:
@@ -2506,11 +2506,11 @@ static int n_tty_ioctl(struct tty_struct *tty, unsigned int cmd,
 	case TIOCINQ:
 		down_write(&tty->termios_rwsem);
 		if (L_ICANON(tty) && !L_EXTPROC(tty))
-			retval = inq_canon(ldata);
+			num = inq_canon(ldata);
 		else
-			retval = read_cnt(ldata);
+			num = read_cnt(ldata);
 		up_write(&tty->termios_rwsem);
-		return put_user(retval, (unsigned int __user *) arg);
+		return put_user(num, (unsigned int __user *) arg);
 	default:
 		return n_tty_ioctl_helper(tty, cmd, arg);
 	}
-- 
2.42.0

[PATCH 03/15] tty: n_tty: use min3() in copy_from_read_buf()

[Jiri Slaby (SUSE)]

n is a minimum of:
* available chars in the ring buffer
* available chars in the ring buffer till the end of the ring buffer
* requested number (*nr)

We can use min3() for that instead of two min()s.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/n_tty.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index e917faa0b84c..6a112910c058 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1965,8 +1965,7 @@ static bool copy_from_read_buf(const struct tty_struct *tty, u8 **kbp,
 	size_t head = smp_load_acquire(&ldata->commit_head);
 	size_t tail = MASK(ldata->read_tail);
 
-	n = min(head - ldata->read_tail, N_TTY_BUF_SIZE - tail);
-	n = min(*nr, n);
+	n = min3(head - ldata->read_tail, N_TTY_BUF_SIZE - tail, *nr);
 	if (n) {
 		u8 *from = read_buf_addr(ldata, tail);
 		memcpy(*kbp, from, n);
-- 
2.42.0

[PATCH 04/15] tty: n_tty: invert the condition in copy_from_read_buf()

[Jiri Slaby (SUSE)]

Make "no numbers available" a fast quit from the function. And do the
heavy work outside the 'if'. This makes the code more understandable and
conforming to the common kernel coding style.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/n_tty.c | 38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 6a112910c058..922fb61b587a 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1966,24 +1966,26 @@ static bool copy_from_read_buf(const struct tty_struct *tty, u8 **kbp,
 	size_t tail = MASK(ldata->read_tail);
 
 	n = min3(head - ldata->read_tail, N_TTY_BUF_SIZE - tail, *nr);
-	if (n) {
-		u8 *from = read_buf_addr(ldata, tail);
-		memcpy(*kbp, from, n);
-		is_eof = n == 1 && *from == EOF_CHAR(tty);
-		tty_audit_add_data(tty, from, n);
-		zero_buffer(tty, from, n);
-		smp_store_release(&ldata->read_tail, ldata->read_tail + n);
-		/* Turn single EOF into zero-length read */
-		if (L_EXTPROC(tty) && ldata->icanon && is_eof &&
-		    (head == ldata->read_tail))
-			return false;
-		*kbp += n;
-		*nr -= n;
-
-		/* If we have more to copy, let the caller know */
-		return head != ldata->read_tail;
-	}
-	return false;
+	if (!n)
+		return false;
+
+	u8 *from = read_buf_addr(ldata, tail);
+	memcpy(*kbp, from, n);
+	is_eof = n == 1 && *from == EOF_CHAR(tty);
+	tty_audit_add_data(tty, from, n);
+	zero_buffer(tty, from, n);
+	smp_store_release(&ldata->read_tail, ldata->read_tail + n);
+
+	/* Turn single EOF into zero-length read */
+	if (L_EXTPROC(tty) && ldata->icanon && is_eof &&
+	    head == ldata->read_tail)
+		return false;
+
+	*kbp += n;
+	*nr -= n;
+
+	/* If we have more to copy, let the caller know */
+	return head != ldata->read_tail;
 }
 
 /**
-- 
2.42.0

Re: [PATCH 04/15] tty: n_tty: invert the condition in copy_from_read_buf()

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2137 bytes --]

On Tue, 19 Sep 2023, Jiri Slaby (SUSE) wrote:

> Make "no numbers available" a fast quit from the function. And do the

Did you really intend to write "numbers" and not e.g. characters?

The change itself looks good,

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

> heavy work outside the 'if'. This makes the code more understandable and
> conforming to the common kernel coding style.
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> ---
>  drivers/tty/n_tty.c | 38 ++++++++++++++++++++------------------
>  1 file changed, 20 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
> index 6a112910c058..922fb61b587a 100644
> --- a/drivers/tty/n_tty.c
> +++ b/drivers/tty/n_tty.c
> @@ -1966,24 +1966,26 @@ static bool copy_from_read_buf(const struct tty_struct *tty, u8 **kbp,
>  	size_t tail = MASK(ldata->read_tail);
>  
>  	n = min3(head - ldata->read_tail, N_TTY_BUF_SIZE - tail, *nr);
> -	if (n) {
> -		u8 *from = read_buf_addr(ldata, tail);
> -		memcpy(*kbp, from, n);
> -		is_eof = n == 1 && *from == EOF_CHAR(tty);
> -		tty_audit_add_data(tty, from, n);
> -		zero_buffer(tty, from, n);
> -		smp_store_release(&ldata->read_tail, ldata->read_tail + n);
> -		/* Turn single EOF into zero-length read */
> -		if (L_EXTPROC(tty) && ldata->icanon && is_eof &&
> -		    (head == ldata->read_tail))
> -			return false;
> -		*kbp += n;
> -		*nr -= n;
> -
> -		/* If we have more to copy, let the caller know */
> -		return head != ldata->read_tail;
> -	}
> -	return false;
> +	if (!n)
> +		return false;
> +
> +	u8 *from = read_buf_addr(ldata, tail);
> +	memcpy(*kbp, from, n);
> +	is_eof = n == 1 && *from == EOF_CHAR(tty);
> +	tty_audit_add_data(tty, from, n);
> +	zero_buffer(tty, from, n);
> +	smp_store_release(&ldata->read_tail, ldata->read_tail + n);
> +
> +	/* Turn single EOF into zero-length read */
> +	if (L_EXTPROC(tty) && ldata->icanon && is_eof &&
> +	    head == ldata->read_tail)
> +		return false;
> +
> +	*kbp += n;
> +	*nr -= n;
> +
> +	/* If we have more to copy, let the caller know */
> +	return head != ldata->read_tail;
>  }
>  
>  /**
> 

Re: [PATCH 04/15] tty: n_tty: invert the condition in copy_from_read_buf()

[Jiri Slaby]

On 19. 09. 23, 11:54, Ilpo Järvinen wrote:
> On Tue, 19 Sep 2023, Jiri Slaby (SUSE) wrote:
> 
>> Make "no numbers available" a fast quit from the function. And do the
> 
> Did you really intend to write "numbers" and not e.g. characters?

What was I thinking? "n must be numbers", apparently. But I definitely 
meant "characters". Funny, that I missed it now, while re-reading and 
preparing the commit logs for submission.

thanks,
-- 
js
suse labs

[PATCH 05/15] tty: n_tty: use do-while in n_tty_check_{,un}throttle()

[Jiri Slaby (SUSE)]

This change gets rid of the complicated exit from the loops. It can be
done much easier using do-while loops.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/n_tty.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 922fb61b587a..b34e6612aef6 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -249,15 +249,12 @@ static void n_tty_check_throttle(struct tty_struct *tty)
 	if (ldata->icanon && ldata->canon_head == ldata->read_tail)
 		return;
 
-	while (1) {
-		int throttled;
+	do {
 		tty_set_flow_change(tty, TTY_THROTTLE_SAFE);
 		if (N_TTY_BUF_SIZE - read_cnt(ldata) >= TTY_THRESHOLD_THROTTLE)
 			break;
-		throttled = tty_throttle_safe(tty);
-		if (!throttled)
-			break;
-	}
+	} while (tty_throttle_safe(tty));
+
 	__tty_set_flow_change(tty, 0);
 }
 
@@ -279,16 +276,14 @@ static void n_tty_check_unthrottle(struct tty_struct *tty)
 	 * we won't get any more characters.
 	 */
 
-	while (1) {
-		int unthrottled;
+	do {
 		tty_set_flow_change(tty, TTY_UNTHROTTLE_SAFE);
 		if (chars_in_buffer(tty) > TTY_THRESHOLD_UNTHROTTLE)
 			break;
+
 		n_tty_kick_worker(tty);
-		unthrottled = tty_unthrottle_safe(tty);
-		if (!unthrottled)
-			break;
-	}
+	} while (tty_unthrottle_safe(tty));
+
 	__tty_set_flow_change(tty, 0);
 }
 
-- 
2.42.0

[PATCH 06/15] tty: switch tty_{,un}throttle_safe() to return a bool

[Jiri Slaby (SUSE)]

They return 0 or 1 -- a boolean value, so make it clear than noone
should expect negative or other values.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/tty_ioctl.c | 18 ++++++++----------
 include/linux/tty.h     |  4 ++--
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index 7958bf6d27c4..ba60fcf518e0 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -124,17 +124,16 @@ EXPORT_SYMBOL(tty_unthrottle);
  *	conditions when throttling is conditional on factors evaluated prior to
  *	throttling.
  *
- *	Returns 0 if tty is throttled (or was already throttled)
+ *	Returns false if tty is throttled (or was already throttled)
  */
-
-int tty_throttle_safe(struct tty_struct *tty)
+bool tty_throttle_safe(struct tty_struct *tty)
 {
-	int ret = 0;
+	bool ret = false;
 
 	mutex_lock(&tty->throttle_mutex);
 	if (!tty_throttled(tty)) {
 		if (tty->flow_change != TTY_THROTTLE_SAFE)
-			ret = 1;
+			ret = true;
 		else {
 			set_bit(TTY_THROTTLED, &tty->flags);
 			if (tty->ops->throttle)
@@ -155,17 +154,16 @@ int tty_throttle_safe(struct tty_struct *tty)
  *	unthrottle due to race conditions when unthrottling is conditional
  *	on factors evaluated prior to unthrottling.
  *
- *	Returns 0 if tty is unthrottled (or was already unthrottled)
+ *	Returns false if tty is unthrottled (or was already unthrottled)
  */
-
-int tty_unthrottle_safe(struct tty_struct *tty)
+bool tty_unthrottle_safe(struct tty_struct *tty)
 {
-	int ret = 0;
+	bool ret = false;
 
 	mutex_lock(&tty->throttle_mutex);
 	if (tty_throttled(tty)) {
 		if (tty->flow_change != TTY_UNTHROTTLE_SAFE)
-			ret = 1;
+			ret = true;
 		else {
 			clear_bit(TTY_THROTTLED, &tty->flags);
 			if (tty->ops->unthrottle)
diff --git a/include/linux/tty.h b/include/linux/tty.h
index f002d0f25db7..59d675f345e9 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -416,8 +416,8 @@ unsigned int tty_chars_in_buffer(struct tty_struct *tty);
 unsigned int tty_write_room(struct tty_struct *tty);
 void tty_driver_flush_buffer(struct tty_struct *tty);
 void tty_unthrottle(struct tty_struct *tty);
-int tty_throttle_safe(struct tty_struct *tty);
-int tty_unthrottle_safe(struct tty_struct *tty);
+bool tty_throttle_safe(struct tty_struct *tty);
+bool tty_unthrottle_safe(struct tty_struct *tty);
 int tty_do_resize(struct tty_struct *tty, struct winsize *ws);
 int tty_get_icount(struct tty_struct *tty,
 		struct serial_icounter_struct *icount);
-- 
2.42.0

[PATCH 07/15] tty: invert return values of tty_{,un}throttle_safe()

[Jiri Slaby (SUSE)]

If tty_{,un}throttle_safe() returned true on success (similar to
*_trylock()), it would make the conditions in callers more obvious. So
perform the switch to these inverted values (and fix the callers).

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/n_tty.c     |  4 ++--
 drivers/tty/tty_ioctl.c | 12 ++++++------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index b34e6612aef6..f252d0b5a434 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -253,7 +253,7 @@ static void n_tty_check_throttle(struct tty_struct *tty)
 		tty_set_flow_change(tty, TTY_THROTTLE_SAFE);
 		if (N_TTY_BUF_SIZE - read_cnt(ldata) >= TTY_THRESHOLD_THROTTLE)
 			break;
-	} while (tty_throttle_safe(tty));
+	} while (!tty_throttle_safe(tty));
 
 	__tty_set_flow_change(tty, 0);
 }
@@ -282,7 +282,7 @@ static void n_tty_check_unthrottle(struct tty_struct *tty)
 			break;
 
 		n_tty_kick_worker(tty);
-	} while (tty_unthrottle_safe(tty));
+	} while (!tty_unthrottle_safe(tty));
 
 	__tty_set_flow_change(tty, 0);
 }
diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index ba60fcf518e0..fb2f1ac5172f 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -124,16 +124,16 @@ EXPORT_SYMBOL(tty_unthrottle);
  *	conditions when throttling is conditional on factors evaluated prior to
  *	throttling.
  *
- *	Returns false if tty is throttled (or was already throttled)
+ *	Returns true if tty is throttled (or was already throttled)
  */
 bool tty_throttle_safe(struct tty_struct *tty)
 {
-	bool ret = false;
+	bool ret = true;
 
 	mutex_lock(&tty->throttle_mutex);
 	if (!tty_throttled(tty)) {
 		if (tty->flow_change != TTY_THROTTLE_SAFE)
-			ret = true;
+			ret = false;
 		else {
 			set_bit(TTY_THROTTLED, &tty->flags);
 			if (tty->ops->throttle)
@@ -154,16 +154,16 @@ bool tty_throttle_safe(struct tty_struct *tty)
  *	unthrottle due to race conditions when unthrottling is conditional
  *	on factors evaluated prior to unthrottling.
  *
- *	Returns false if tty is unthrottled (or was already unthrottled)
+ *	Returns true if tty is unthrottled (or was already unthrottled)
  */
 bool tty_unthrottle_safe(struct tty_struct *tty)
 {
-	bool ret = false;
+	bool ret = true;
 
 	mutex_lock(&tty->throttle_mutex);
 	if (tty_throttled(tty)) {
 		if (tty->flow_change != TTY_UNTHROTTLE_SAFE)
-			ret = true;
+			ret = false;
 		else {
 			clear_bit(TTY_THROTTLED, &tty->flags);
 			if (tty->ops->unthrottle)
-- 
2.42.0

[PATCH 08/15] tty: fix up and plug in tty_ioctl kernel-doc

[Jiri Slaby (SUSE)]

The ioctl helpers are well documented, except they are not plugged in
the Documentation. So fix up the minor issues in the kernel-doc and plug
it in.

The minor issues include:
* bad \t on every line (sphinx misinterprets the description otherwise)
* missing colon after Return
* superfluous \n after the comment
* make some struct members and constants a hyperlink
* and so on

Perhaps better to use --word-diff if one wants to see the "real"
changes.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 Documentation/driver-api/tty/index.rst     |   1 +
 Documentation/driver-api/tty/tty_ioctl.rst |   7 +
 drivers/tty/tty_ioctl.c                    | 218 ++++++++++-----------
 3 files changed, 115 insertions(+), 111 deletions(-)
 create mode 100644 Documentation/driver-api/tty/tty_ioctl.rst

diff --git a/Documentation/driver-api/tty/index.rst b/Documentation/driver-api/tty/index.rst
index 2d32606a4278..b490da11f257 100644
--- a/Documentation/driver-api/tty/index.rst
+++ b/Documentation/driver-api/tty/index.rst
@@ -36,6 +36,7 @@ In-detail description of the named TTY structures is in separate documents:
    tty_struct
    tty_ldisc
    tty_buffer
+   tty_ioctl
    tty_internals
 
 Writing TTY Driver
diff --git a/Documentation/driver-api/tty/tty_ioctl.rst b/Documentation/driver-api/tty/tty_ioctl.rst
new file mode 100644
index 000000000000..9b0be79fc15e
--- /dev/null
+++ b/Documentation/driver-api/tty/tty_ioctl.rst
@@ -0,0 +1,7 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=================
+TTY IOCTL Helpers
+=================
+
+.. kernel-doc:: drivers/tty/tty_ioctl.c
diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index fb2f1ac5172f..42c793e9d131 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -38,16 +38,13 @@
 #define TERMIOS_TERMIO	BIT(2)
 #define TERMIOS_OLD	BIT(3)
 
-
 /**
- *	tty_chars_in_buffer	-	characters pending
- *	@tty: terminal
+ * tty_chars_in_buffer - characters pending
+ * @tty: terminal
  *
- *	Return the number of bytes of data in the device private
- *	output queue. If no private method is supplied there is assumed
- *	to be no queue on the device.
+ * Returns: the number of bytes of data in the device private output queue. If
+ * no private method is supplied there is assumed to be no queue on the device.
  */
-
 unsigned int tty_chars_in_buffer(struct tty_struct *tty)
 {
 	if (tty->ops->chars_in_buffer)
@@ -57,16 +54,15 @@ unsigned int tty_chars_in_buffer(struct tty_struct *tty)
 EXPORT_SYMBOL(tty_chars_in_buffer);
 
 /**
- *	tty_write_room		-	write queue space
- *	@tty: terminal
+ * tty_write_room - write queue space
+ * @tty: terminal
  *
- *	Return the number of bytes that can be queued to this device
- *	at the present time. The result should be treated as a guarantee
- *	and the driver cannot offer a value it later shrinks by more than
- *	the number of bytes written. If no method is provided 2K is always
- *	returned and data may be lost as there will be no flow control.
+ * Returns: the number of bytes that can be queued to this device at the present
+ * time. The result should be treated as a guarantee and the driver cannot
+ * offer a value it later shrinks by more than the number of bytes written. If
+ * no method is provided, 2K is always returned and data may be lost as there
+ * will be no flow control.
  */
- 
 unsigned int tty_write_room(struct tty_struct *tty)
 {
 	if (tty->ops->write_room)
@@ -76,12 +72,12 @@ unsigned int tty_write_room(struct tty_struct *tty)
 EXPORT_SYMBOL(tty_write_room);
 
 /**
- *	tty_driver_flush_buffer	-	discard internal buffer
- *	@tty: terminal
+ * tty_driver_flush_buffer - discard internal buffer
+ * @tty: terminal
  *
- *	Discard the internal output buffer for this device. If no method
- *	is provided then either the buffer cannot be hardware flushed or
- *	there is no buffer driver side.
+ * Discard the internal output buffer for this device. If no method is provided,
+ * then either the buffer cannot be hardware flushed or there is no buffer
+ * driver side.
  */
 void tty_driver_flush_buffer(struct tty_struct *tty)
 {
@@ -91,18 +87,17 @@ void tty_driver_flush_buffer(struct tty_struct *tty)
 EXPORT_SYMBOL(tty_driver_flush_buffer);
 
 /**
- *	tty_unthrottle		-	flow control
- *	@tty: terminal
+ * tty_unthrottle - flow control
+ * @tty: terminal
  *
- *	Indicate that a tty may continue transmitting data down the stack.
- *	Takes the termios rwsem to protect against parallel throttle/unthrottle
- *	and also to ensure the driver can consistently reference its own
- *	termios data at this point when implementing software flow control.
+ * Indicate that a @tty may continue transmitting data down the stack. Takes
+ * the &tty_struct->termios_rwsem to protect against parallel
+ * throttle/unthrottle and also to ensure the driver can consistently reference
+ * its own termios data at this point when implementing software flow control.
  *
- *	Drivers should however remember that the stack can issue a throttle,
- *	then change flow control method, then unthrottle.
+ * Drivers should however remember that the stack can issue a throttle, then
+ * change flow control method, then unthrottle.
  */
-
 void tty_unthrottle(struct tty_struct *tty)
 {
 	down_write(&tty->termios_rwsem);
@@ -115,16 +110,15 @@ void tty_unthrottle(struct tty_struct *tty)
 EXPORT_SYMBOL(tty_unthrottle);
 
 /**
- *	tty_throttle_safe	-	flow control
- *	@tty: terminal
+ * tty_throttle_safe - flow control
+ * @tty: terminal
  *
- *	Indicate that a tty should stop transmitting data down the stack.
- *	tty_throttle_safe will only attempt throttle if tty->flow_change is
- *	TTY_THROTTLE_SAFE. Prevents an accidental throttle due to race
- *	conditions when throttling is conditional on factors evaluated prior to
- *	throttling.
+ * Indicate that a @tty should stop transmitting data down the stack.
+ * tty_throttle_safe() will only attempt throttle if @tty->flow_change is
+ * %TTY_THROTTLE_SAFE. Prevents an accidental throttle due to race conditions
+ * when throttling is conditional on factors evaluated prior to throttling.
  *
- *	Returns true if tty is throttled (or was already throttled)
+ * Returns: %true if @tty is throttled (or was already throttled)
  */
 bool tty_throttle_safe(struct tty_struct *tty)
 {
@@ -146,15 +140,15 @@ bool tty_throttle_safe(struct tty_struct *tty)
 }
 
 /**
- *	tty_unthrottle_safe	-	flow control
- *	@tty: terminal
+ * tty_unthrottle_safe - flow control
+ * @tty: terminal
  *
- *	Similar to tty_unthrottle() but will only attempt unthrottle
- *	if tty->flow_change is TTY_UNTHROTTLE_SAFE. Prevents an accidental
- *	unthrottle due to race conditions when unthrottling is conditional
- *	on factors evaluated prior to unthrottling.
+ * Similar to tty_unthrottle() but will only attempt unthrottle if
+ * @tty->flow_change is %TTY_UNTHROTTLE_SAFE. Prevents an accidental unthrottle
+ * due to race conditions when unthrottling is conditional on factors evaluated
+ * prior to unthrottling.
  *
- *	Returns true if tty is unthrottled (or was already unthrottled)
+ * Returns: %true if @tty is unthrottled (or was already unthrottled)
  */
 bool tty_unthrottle_safe(struct tty_struct *tty)
 {
@@ -176,14 +170,14 @@ bool tty_unthrottle_safe(struct tty_struct *tty)
 }
 
 /**
- *	tty_wait_until_sent	-	wait for I/O to finish
- *	@tty: tty we are waiting for
- *	@timeout: how long we will wait
+ * tty_wait_until_sent - wait for I/O to finish
+ * @tty: tty we are waiting for
+ * @timeout: how long we will wait
  *
- *	Wait for characters pending in a tty driver to hit the wire, or
- *	for a timeout to occur (eg due to flow control)
+ * Wait for characters pending in a tty driver to hit the wire, or for a
+ * timeout to occur (eg due to flow control).
  *
- *	Locking: none
+ * Locking: none
  */
 
 void tty_wait_until_sent(struct tty_struct *tty, long timeout)
@@ -229,16 +223,15 @@ static void unset_locked_termios(struct tty_struct *tty, const struct ktermios *
 }
 
 /**
- *	tty_termios_copy_hw	-	copy hardware settings
- *	@new: New termios
- *	@old: Old termios
+ * tty_termios_copy_hw - copy hardware settings
+ * @new: new termios
+ * @old: old termios
  *
- *	Propagate the hardware specific terminal setting bits from
- *	the old termios structure to the new one. This is used in cases
- *	where the hardware does not support reconfiguration or as a helper
- *	in some cases where only minimal reconfiguration is supported
+ * Propagate the hardware specific terminal setting bits from the @old termios
+ * structure to the @new one. This is used in cases where the hardware does not
+ * support reconfiguration or as a helper in some cases where only minimal
+ * reconfiguration is supported.
  */
-
 void tty_termios_copy_hw(struct ktermios *new, const struct ktermios *old)
 {
 	/* The bits a dumb device handles in software. Smart devices need
@@ -251,14 +244,15 @@ void tty_termios_copy_hw(struct ktermios *new, const struct ktermios *old)
 EXPORT_SYMBOL(tty_termios_copy_hw);
 
 /**
- *	tty_termios_hw_change	-	check for setting change
- *	@a: termios
- *	@b: termios to compare
+ * tty_termios_hw_change - check for setting change
+ * @a: termios
+ * @b: termios to compare
  *
- *	Check if any of the bits that affect a dumb device have changed
- *	between the two termios structures, or a speed change is needed.
+ * Check if any of the bits that affect a dumb device have changed between the
+ * two termios structures, or a speed change is needed.
+ *
+ * Returns: %true if change is needed
  */
-
 bool tty_termios_hw_change(const struct ktermios *a, const struct ktermios *b)
 {
 	if (a->c_ispeed != b->c_ispeed || a->c_ospeed != b->c_ospeed)
@@ -270,11 +264,10 @@ bool tty_termios_hw_change(const struct ktermios *a, const struct ktermios *b)
 EXPORT_SYMBOL(tty_termios_hw_change);
 
 /**
- *	tty_get_char_size	-	get size of a character
- *	@cflag: termios cflag value
+ * tty_get_char_size - get size of a character
+ * @cflag: termios cflag value
  *
- *	Get the size (in bits) of a character depending on @cflag's %CSIZE
- *	setting.
+ * Returns: size (in bits) of a character depending on @cflag's %CSIZE setting
  */
 unsigned char tty_get_char_size(unsigned int cflag)
 {
@@ -293,13 +286,14 @@ unsigned char tty_get_char_size(unsigned int cflag)
 EXPORT_SYMBOL_GPL(tty_get_char_size);
 
 /**
- *	tty_get_frame_size	-	get size of a frame
- *	@cflag: termios cflag value
+ * tty_get_frame_size - get size of a frame
+ * @cflag: termios cflag value
  *
- *	Get the size (in bits) of a frame depending on @cflag's %CSIZE, %CSTOPB,
- *	and %PARENB setting. The result is a sum of character size, start and
- *	stop bits -- one bit each -- second stop bit (if set), and parity bit
- *	(if set).
+ * Get the size (in bits) of a frame depending on @cflag's %CSIZE, %CSTOPB, and
+ * %PARENB setting. The result is a sum of character size, start and stop bits
+ * -- one bit each -- second stop bit (if set), and parity bit (if set).
+ *
+ * Returns: size (in bits) of a frame depending on @cflag's setting.
  */
 unsigned char tty_get_frame_size(unsigned int cflag)
 {
@@ -317,16 +311,15 @@ unsigned char tty_get_frame_size(unsigned int cflag)
 EXPORT_SYMBOL_GPL(tty_get_frame_size);
 
 /**
- *	tty_set_termios		-	update termios values
- *	@tty: tty to update
- *	@new_termios: desired new value
+ * tty_set_termios - update termios values
+ * @tty: tty to update
+ * @new_termios: desired new value
  *
- *	Perform updates to the termios values set on this terminal.
- *	A master pty's termios should never be set.
+ * Perform updates to the termios values set on this @tty. A master pty's
+ * termios should never be set.
  *
- *	Locking: termios_rwsem
+ * Locking: &tty_struct->termios_rwsem
  */
-
 int tty_set_termios(struct tty_struct *tty, struct ktermios *new_termios)
 {
 	struct ktermios old_termios;
@@ -439,18 +432,19 @@ __weak int kernel_termios_to_user_termios(struct termios __user *u,
 #endif /* TCGETS2 */
 
 /**
- *	set_termios		-	set termios values for a tty
- *	@tty: terminal device
- *	@arg: user data
- *	@opt: option information
+ * set_termios - set termios values for a tty
+ * @tty: terminal device
+ * @arg: user data
+ * @opt: option information
  *
- *	Helper function to prepare termios data and run necessary other
- *	functions before using tty_set_termios to do the actual changes.
+ * Helper function to prepare termios data and run necessary other functions
+ * before using tty_set_termios() to do the actual changes.
  *
- *	Locking:
- *		Called functions take ldisc and termios_rwsem locks
+ * Locking: called functions take &tty_struct->ldisc_sem and
+ * &tty_struct->termios_rwsem locks
+ *
+ * Returns: 0 on success, an error otherwise
  */
-
 static int set_termios(struct tty_struct *tty, void __user *arg, int opt)
 {
 	struct ktermios tmp_termios;
@@ -622,16 +616,16 @@ static void set_sgflags(struct ktermios *termios, int flags)
 }
 
 /**
- *	set_sgttyb		-	set legacy terminal values
- *	@tty: tty structure
- *	@sgttyb: pointer to old style terminal structure
+ * set_sgttyb - set legacy terminal values
+ * @tty: tty structure
+ * @sgttyb: pointer to old style terminal structure
  *
- *	Updates a terminal from the legacy BSD style terminal information
- *	structure.
+ * Updates a terminal from the legacy BSD style terminal information structure.
  *
- *	Locking: termios_rwsem
+ * Locking: &tty_struct->termios_rwsem
+ *
+ * Returns: 0 on success, an error otherwise
  */
-
 static int set_sgttyb(struct tty_struct *tty, struct sgttyb __user *sgttyb)
 {
 	int retval;
@@ -733,14 +727,17 @@ static int set_ltchars(struct tty_struct *tty, struct ltchars __user *ltchars)
 #endif
 
 /**
- *	tty_change_softcar	-	carrier change ioctl helper
- *	@tty: tty to update
- *	@enable: enable/disable CLOCAL
+ * tty_change_softcar - carrier change ioctl helper
+ * @tty: tty to update
+ * @enable: enable/disable %CLOCAL
  *
- *	Perform a change to the CLOCAL state and call into the driver
- *	layer to make it visible. All done with the termios rwsem
+ * Perform a change to the %CLOCAL state and call into the driver layer to make
+ * it visible.
+ *
+ * Locking: &tty_struct->termios_rwsem.
+ *
+ * Returns: 0 on success, an error otherwise
  */
-
 static int tty_change_softcar(struct tty_struct *tty, bool enable)
 {
 	int ret = 0;
@@ -760,16 +757,15 @@ static int tty_change_softcar(struct tty_struct *tty, bool enable)
 }
 
 /**
- *	tty_mode_ioctl		-	mode related ioctls
- *	@tty: tty for the ioctl
- *	@cmd: command
- *	@arg: ioctl argument
+ * tty_mode_ioctl - mode related ioctls
+ * @tty: tty for the ioctl
+ * @cmd: command
+ * @arg: ioctl argument
  *
- *	Perform non line discipline specific mode control ioctls. This
- *	is designed to be called by line disciplines to ensure they provide
- *	consistent mode setting.
+ * Perform non-line discipline specific mode control ioctls. This is designed
+ * to be called by line disciplines to ensure they provide consistent mode
+ * setting.
  */
-
 int tty_mode_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
 {
 	struct tty_struct *real_tty;
-- 
2.42.0

[PATCH 09/15] tty: fix kernel-doc for functions in tty.h

[Jiri Slaby (SUSE)]

tty_kref_get() is already included in Documentation, but is not properly
formatted. Fix this.

tty_get_baud_rate() is neither properly formatted, nor is included. Fix
both.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 Documentation/driver-api/tty/tty_ioctl.rst |  3 +++
 include/linux/tty.h                        | 21 +++++++++------------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/Documentation/driver-api/tty/tty_ioctl.rst b/Documentation/driver-api/tty/tty_ioctl.rst
index 9b0be79fc15e..3ff1ac5e07f1 100644
--- a/Documentation/driver-api/tty/tty_ioctl.rst
+++ b/Documentation/driver-api/tty/tty_ioctl.rst
@@ -5,3 +5,6 @@ TTY IOCTL Helpers
 =================
 
 .. kernel-doc:: drivers/tty/tty_ioctl.c
+
+.. kernel-doc:: include/linux/tty.h
+   :identifiers: tty_get_baud_rate
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 59d675f345e9..4b6340ac2af2 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -390,14 +390,12 @@ int vcs_init(void);
 extern const struct class tty_class;
 
 /**
- *	tty_kref_get		-	get a tty reference
- *	@tty: tty device
+ * tty_kref_get - get a tty reference
+ * @tty: tty device
  *
- *	Return a new reference to a tty object. The caller must hold
- *	sufficient locks/counts to ensure that their existing reference cannot
- *	go away
+ * Returns: a new reference to a tty object. The caller must hold sufficient
+ * locks/counts to ensure that their existing reference cannot go away
  */
-
 static inline struct tty_struct *tty_kref_get(struct tty_struct *tty)
 {
 	if (tty)
@@ -435,14 +433,13 @@ void tty_encode_baud_rate(struct tty_struct *tty, speed_t ibaud,
 		speed_t obaud);
 
 /**
- *	tty_get_baud_rate	-	get tty bit rates
- *	@tty: tty to query
+ * tty_get_baud_rate - get tty bit rates
+ * @tty: tty to query
  *
- *	Returns the baud rate as an integer for this terminal. The
- *	termios lock must be held by the caller and the terminal bit
- *	flags may be updated.
+ * Returns: the baud rate as an integer for this terminal. The termios lock
+ * must be held by the caller and the terminal bit flags may be updated.
  *
- *	Locking: none
+ * Locking: none
  */
 static inline speed_t tty_get_baud_rate(struct tty_struct *tty)
 {
-- 
2.42.0

Re: [PATCH 09/15] tty: fix kernel-doc for functions in tty.h

[Ilpo Järvinen]

On Tue, 19 Sep 2023, Jiri Slaby (SUSE) wrote:

> tty_kref_get() is already included in Documentation, but is not properly
> formatted. Fix this.
> 
> tty_get_baud_rate() is neither properly formatted, nor is included. Fix
> both.
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> ---
>  Documentation/driver-api/tty/tty_ioctl.rst |  3 +++
>  include/linux/tty.h                        | 21 +++++++++------------
>  2 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/driver-api/tty/tty_ioctl.rst b/Documentation/driver-api/tty/tty_ioctl.rst
> index 9b0be79fc15e..3ff1ac5e07f1 100644
> --- a/Documentation/driver-api/tty/tty_ioctl.rst
> +++ b/Documentation/driver-api/tty/tty_ioctl.rst
> @@ -5,3 +5,6 @@ TTY IOCTL Helpers
>  =================
>  
>  .. kernel-doc:: drivers/tty/tty_ioctl.c
> +
> +.. kernel-doc:: include/linux/tty.h
> +   :identifiers: tty_get_baud_rate
> diff --git a/include/linux/tty.h b/include/linux/tty.h
> index 59d675f345e9..4b6340ac2af2 100644
> --- a/include/linux/tty.h
> +++ b/include/linux/tty.h
> @@ -390,14 +390,12 @@ int vcs_init(void);
>  extern const struct class tty_class;
>  
>  /**
> - *	tty_kref_get		-	get a tty reference
> - *	@tty: tty device
> + * tty_kref_get - get a tty reference
> + * @tty: tty device
>   *
> - *	Return a new reference to a tty object. The caller must hold
> - *	sufficient locks/counts to ensure that their existing reference cannot
> - *	go away
> + * Returns: a new reference to a tty object. The caller must hold sufficient
> + * locks/counts to ensure that their existing reference cannot go away

Shouldn't this have also Locking: entry instead of hiding the details into 
Return?

>   */
> -
>  static inline struct tty_struct *tty_kref_get(struct tty_struct *tty)
>  {
>  	if (tty)
> @@ -435,14 +433,13 @@ void tty_encode_baud_rate(struct tty_struct *tty, speed_t ibaud,
>  		speed_t obaud);
>  
>  /**
> - *	tty_get_baud_rate	-	get tty bit rates
> - *	@tty: tty to query
> + * tty_get_baud_rate - get tty bit rates
> + * @tty: tty to query
>   *
> - *	Returns the baud rate as an integer for this terminal. The
> - *	termios lock must be held by the caller and the terminal bit
> - *	flags may be updated.
> + * Returns: the baud rate as an integer for this terminal. The termios lock
> + * must be held by the caller and the terminal bit flags may be updated.
>   *
> - *	Locking: none
> + * Locking: none

Eh, the paragraph above says, "The termios lock must be held by the 
caller" so this "Locking: none" seems pretty bogus.

>   */
>  static inline speed_t tty_get_baud_rate(struct tty_struct *tty)
>  {
> 

-- 
 i.

Re: [PATCH 09/15] tty: fix kernel-doc for functions in tty.h

[Jiri Slaby]

On 19. 09. 23, 12:07, Ilpo Järvinen wrote:
> On Tue, 19 Sep 2023, Jiri Slaby (SUSE) wrote:
> 
>> tty_kref_get() is already included in Documentation, but is not properly
>> formatted. Fix this.
>>
>> tty_get_baud_rate() is neither properly formatted, nor is included. Fix
>> both.
>>
>> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
>> ---
>>   Documentation/driver-api/tty/tty_ioctl.rst |  3 +++
>>   include/linux/tty.h                        | 21 +++++++++------------
>>   2 files changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/Documentation/driver-api/tty/tty_ioctl.rst b/Documentation/driver-api/tty/tty_ioctl.rst
>> index 9b0be79fc15e..3ff1ac5e07f1 100644
>> --- a/Documentation/driver-api/tty/tty_ioctl.rst
>> +++ b/Documentation/driver-api/tty/tty_ioctl.rst
>> @@ -5,3 +5,6 @@ TTY IOCTL Helpers
>>   =================
>>   
>>   .. kernel-doc:: drivers/tty/tty_ioctl.c
>> +
>> +.. kernel-doc:: include/linux/tty.h
>> +   :identifiers: tty_get_baud_rate
>> diff --git a/include/linux/tty.h b/include/linux/tty.h
>> index 59d675f345e9..4b6340ac2af2 100644
>> --- a/include/linux/tty.h
>> +++ b/include/linux/tty.h
>> @@ -390,14 +390,12 @@ int vcs_init(void);
>>   extern const struct class tty_class;
>>   
>>   /**
>> - *	tty_kref_get		-	get a tty reference
>> - *	@tty: tty device
>> + * tty_kref_get - get a tty reference
>> + * @tty: tty device
>>    *
>> - *	Return a new reference to a tty object. The caller must hold
>> - *	sufficient locks/counts to ensure that their existing reference cannot
>> - *	go away
>> + * Returns: a new reference to a tty object. The caller must hold sufficient
>> + * locks/counts to ensure that their existing reference cannot go away
> 
> Shouldn't this have also Locking: entry instead of hiding the details into
> Return?

/me left to fix both in a separate patch.

thanks,
-- 
js
suse labs

Re: [PATCH 09/15] tty: fix kernel-doc for functions in tty.h

[Jiri Slaby]

On 19. 09. 23, 12:45, Jiri Slaby wrote:
> On 19. 09. 23, 12:07, Ilpo Järvinen wrote:
>> On Tue, 19 Sep 2023, Jiri Slaby (SUSE) wrote:
>>
>>> tty_kref_get() is already included in Documentation, but is not properly
>>> formatted. Fix this.
>>>
>>> tty_get_baud_rate() is neither properly formatted, nor is included. Fix
>>> both.
>>>
>>> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
>>> ---
>>>   Documentation/driver-api/tty/tty_ioctl.rst |  3 +++
>>>   include/linux/tty.h                        | 21 +++++++++------------
>>>   2 files changed, 12 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/Documentation/driver-api/tty/tty_ioctl.rst 
>>> b/Documentation/driver-api/tty/tty_ioctl.rst
>>> index 9b0be79fc15e..3ff1ac5e07f1 100644
>>> --- a/Documentation/driver-api/tty/tty_ioctl.rst
>>> +++ b/Documentation/driver-api/tty/tty_ioctl.rst
>>> @@ -5,3 +5,6 @@ TTY IOCTL Helpers
>>>   =================
>>>   .. kernel-doc:: drivers/tty/tty_ioctl.c
>>> +
>>> +.. kernel-doc:: include/linux/tty.h
>>> +   :identifiers: tty_get_baud_rate
>>> diff --git a/include/linux/tty.h b/include/linux/tty.h
>>> index 59d675f345e9..4b6340ac2af2 100644
>>> --- a/include/linux/tty.h
>>> +++ b/include/linux/tty.h
>>> @@ -390,14 +390,12 @@ int vcs_init(void);
>>>   extern const struct class tty_class;
>>>   /**
>>> - *    tty_kref_get        -    get a tty reference
>>> - *    @tty: tty device
>>> + * tty_kref_get - get a tty reference
>>> + * @tty: tty device
>>>    *
>>> - *    Return a new reference to a tty object. The caller must hold
>>> - *    sufficient locks/counts to ensure that their existing 
>>> reference cannot
>>> - *    go away
>>> + * Returns: a new reference to a tty object. The caller must hold 
>>> sufficient
>>> + * locks/counts to ensure that their existing reference cannot go away
>>
>> Shouldn't this have also Locking: entry instead of hiding the details 
>> into
>> Return?
> 
> /me left to fix both in a separate patch.

Ah, no. The Locking Alan introduced means what _this_ function locks. I 
am not sure we want to extend the meaning to _expected_ locks?

thanks,
-- 
js
suse labs

Re: [PATCH 09/15] tty: fix kernel-doc for functions in tty.h

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2550 bytes --]

On Tue, 19 Sep 2023, Jiri Slaby wrote:

> On 19. 09. 23, 12:45, Jiri Slaby wrote:
> > On 19. 09. 23, 12:07, Ilpo Järvinen wrote:
> > > On Tue, 19 Sep 2023, Jiri Slaby (SUSE) wrote:
> > > 
> > > > tty_kref_get() is already included in Documentation, but is not properly
> > > > formatted. Fix this.
> > > > 
> > > > tty_get_baud_rate() is neither properly formatted, nor is included. Fix
> > > > both.
> > > > 
> > > > Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> > > > ---
> > > >   Documentation/driver-api/tty/tty_ioctl.rst |  3 +++
> > > >   include/linux/tty.h                        | 21 +++++++++------------
> > > >   2 files changed, 12 insertions(+), 12 deletions(-)
> > > > 
> > > > diff --git a/Documentation/driver-api/tty/tty_ioctl.rst
> > > > b/Documentation/driver-api/tty/tty_ioctl.rst
> > > > index 9b0be79fc15e..3ff1ac5e07f1 100644
> > > > --- a/Documentation/driver-api/tty/tty_ioctl.rst
> > > > +++ b/Documentation/driver-api/tty/tty_ioctl.rst
> > > > @@ -5,3 +5,6 @@ TTY IOCTL Helpers
> > > >   =================
> > > >   .. kernel-doc:: drivers/tty/tty_ioctl.c
> > > > +
> > > > +.. kernel-doc:: include/linux/tty.h
> > > > +   :identifiers: tty_get_baud_rate
> > > > diff --git a/include/linux/tty.h b/include/linux/tty.h
> > > > index 59d675f345e9..4b6340ac2af2 100644
> > > > --- a/include/linux/tty.h
> > > > +++ b/include/linux/tty.h
> > > > @@ -390,14 +390,12 @@ int vcs_init(void);
> > > >   extern const struct class tty_class;
> > > >   /**
> > > > - *    tty_kref_get        -    get a tty reference
> > > > - *    @tty: tty device
> > > > + * tty_kref_get - get a tty reference
> > > > + * @tty: tty device
> > > >    *
> > > > - *    Return a new reference to a tty object. The caller must hold
> > > > - *    sufficient locks/counts to ensure that their existing reference
> > > > cannot
> > > > - *    go away
> > > > + * Returns: a new reference to a tty object. The caller must hold
> > > > sufficient
> > > > + * locks/counts to ensure that their existing reference cannot go away
> > > 
> > > Shouldn't this have also Locking: entry instead of hiding the details into
> > > Return?
> > 
> > /me left to fix both in a separate patch.
> 
> Ah, no. The Locking Alan introduced means what _this_ function locks. I am not
> sure we want to extend the meaning to _expected_ locks?

There are plenty of examples with "Must be called with" or "Caller holds"
in Locking. This is for humans anyway so one reading should understand it.


-- 
 i.

[PATCH 10/15] tty: stop using ndash in kernel-doc

[Jiri Slaby (SUSE)]

An ndash used instead of a single dash renders a bullet to the result.
So use only single dashes in kernel-doc.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/tty_io.c        | 8 ++++----
 drivers/tty/tty_port.c      | 6 +++---
 drivers/tty/vt/consolemap.c | 2 +-
 drivers/tty/vt/vc_screen.c  | 4 ++--
 drivers/tty/vt/vt.c         | 4 ++--
 5 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 8a94e5a43c6d..2ed12ca7c832 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -3300,7 +3300,7 @@ void tty_unregister_device(struct tty_driver *driver, unsigned index)
 EXPORT_SYMBOL(tty_unregister_device);
 
 /**
- * __tty_alloc_driver -- allocate tty driver
+ * __tty_alloc_driver - allocate tty driver
  * @lines: count of lines this driver can handle at most
  * @owner: module which is responsible for this driver
  * @flags: some of %TTY_DRIVER_ flags, will be set in driver->flags
@@ -3393,7 +3393,7 @@ static void destruct_tty_driver(struct kref *kref)
 }
 
 /**
- * tty_driver_kref_put -- drop a reference to a tty driver
+ * tty_driver_kref_put - drop a reference to a tty driver
  * @driver: driver of which to drop the reference
  *
  * The final put will destroy and free up the driver.
@@ -3405,7 +3405,7 @@ void tty_driver_kref_put(struct tty_driver *driver)
 EXPORT_SYMBOL(tty_driver_kref_put);
 
 /**
- * tty_register_driver -- register a tty driver
+ * tty_register_driver - register a tty driver
  * @driver: driver to register
  *
  * Called by a tty driver to register itself.
@@ -3470,7 +3470,7 @@ int tty_register_driver(struct tty_driver *driver)
 EXPORT_SYMBOL(tty_register_driver);
 
 /**
- * tty_unregister_driver -- unregister a tty driver
+ * tty_unregister_driver - unregister a tty driver
  * @driver: driver to unregister
  *
  * Called by a tty driver to unregister itself.
diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index 624d104bd145..63c125250961 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -79,7 +79,7 @@ const struct tty_port_client_operations tty_port_default_client_ops = {
 EXPORT_SYMBOL_GPL(tty_port_default_client_ops);
 
 /**
- * tty_port_init -- initialize tty_port
+ * tty_port_init - initialize tty_port
  * @port: tty_port to initialize
  *
  * Initializes the state of struct tty_port. When a port was initialized using
@@ -267,7 +267,7 @@ void tty_port_free_xmit_buf(struct tty_port *port)
 EXPORT_SYMBOL(tty_port_free_xmit_buf);
 
 /**
- * tty_port_destroy -- destroy inited port
+ * tty_port_destroy - destroy inited port
  * @port: tty port to be destroyed
  *
  * When a port was initialized using tty_port_init(), one has to destroy the
@@ -297,7 +297,7 @@ static void tty_port_destructor(struct kref *kref)
 }
 
 /**
- * tty_port_put -- drop a reference to tty_port
+ * tty_port_put - drop a reference to tty_port
  * @port: port to drop a reference of (can be NULL)
  *
  * The final put will destroy and free up the @port using
diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index f02d21e2a96e..5e39a4f430ee 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -205,7 +205,7 @@ static enum translation_map inv_translate[MAX_NR_CONSOLES];
 				 FIELD_PREP(UNI_GLYPH_BITS, (glyph)))
 
 /**
- * struct uni_pagedict -- unicode directory
+ * struct uni_pagedict - unicode directory
  *
  * @uni_pgdir: 32*32*64 table with glyphs
  * @refcount: reference count of this structure
diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
index 829c4be66f3b..99c8e39d91b4 100644
--- a/drivers/tty/vt/vc_screen.c
+++ b/drivers/tty/vt/vc_screen.c
@@ -174,7 +174,7 @@ vcs_poll_data_get(struct file *file)
 }
 
 /**
- * vcs_vc -- return VC for @inode
+ * vcs_vc - return VC for @inode
  * @inode: inode for which to return a VC
  * @viewed: returns whether this console is currently foreground (viewed)
  *
@@ -199,7 +199,7 @@ static struct vc_data *vcs_vc(struct inode *inode, bool *viewed)
 }
 
 /**
- * vcs_size -- return size for a VC in @vc
+ * vcs_size - return size for a VC in @vc
  * @vc: which VC
  * @attr: does it use attributes?
  * @unicode: is it unicode?
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 5c47f77804f0..f5004231cb6a 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -2588,7 +2588,7 @@ static inline int vc_translate_ascii(const struct vc_data *vc, int c)
 
 
 /**
- * vc_sanitize_unicode -- Replace invalid Unicode code points with U+FFFD
+ * vc_sanitize_unicode - Replace invalid Unicode code points with U+FFFD
  * @c: the received character, or U+FFFD for invalid sequences.
  */
 static inline int vc_sanitize_unicode(const int c)
@@ -2600,7 +2600,7 @@ static inline int vc_sanitize_unicode(const int c)
 }
 
 /**
- * vc_translate_unicode -- Combine UTF-8 into Unicode in @vc_utf_char
+ * vc_translate_unicode - Combine UTF-8 into Unicode in @vc_utf_char
  * @vc: virtual console
  * @c: character to translate
  * @rescan: we return true if we need more (continuation) data
-- 
2.42.0

[PATCH 11/15] tty: tty_buffer: use bool for 'restart' in tty_buffer_unlock_exclusive()

[Jiri Slaby (SUSE)]

It's a boolean value, so no need for 'int' there.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/tty_buffer.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 5f6d0cf67571..f8883afbeeba 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -69,12 +69,11 @@ EXPORT_SYMBOL_GPL(tty_buffer_lock_exclusive);
 void tty_buffer_unlock_exclusive(struct tty_port *port)
 {
 	struct tty_bufhead *buf = &port->buf;
-	int restart;
-
-	restart = buf->head->commit != buf->head->read;
+	bool restart = buf->head->commit != buf->head->read;
 
 	atomic_dec(&buf->priority);
 	mutex_unlock(&buf->lock);
+
 	if (restart)
 		queue_work(system_unbound_wq, &buf->work);
 }
-- 
2.42.0

[PATCH 12/15] tty: convert THROTTLE constants into enum

[Jiri Slaby (SUSE)]

And make an explicit constant for zero too. This allows for easier type
checking of the parameter.

Note: tty_struct::flow_change is kept as int because include/tty.h
(tty_struct) doesn't see tty/tty.h (this enum).

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/tty.h       | 13 +++++++++----
 drivers/tty/tty_ioctl.c |  2 +-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/tty.h b/drivers/tty/tty.h
index 50862f98273e..93cf5ef1e857 100644
--- a/drivers/tty/tty.h
+++ b/drivers/tty/tty.h
@@ -41,15 +41,20 @@ enum {
 };
 
 /* Values for tty->flow_change */
-#define TTY_THROTTLE_SAFE	1
-#define TTY_UNTHROTTLE_SAFE	2
+enum tty_flow_change {
+	TTY_FLOW_NO_CHANGE,
+	TTY_THROTTLE_SAFE,
+	TTY_UNTHROTTLE_SAFE,
+};
 
-static inline void __tty_set_flow_change(struct tty_struct *tty, int val)
+static inline void __tty_set_flow_change(struct tty_struct *tty,
+					 enum tty_flow_change val)
 {
 	tty->flow_change = val;
 }
 
-static inline void tty_set_flow_change(struct tty_struct *tty, int val)
+static inline void tty_set_flow_change(struct tty_struct *tty,
+				       enum tty_flow_change val)
 {
 	tty->flow_change = val;
 	smp_mb();
diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index 42c793e9d131..4b499301a3db 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -104,7 +104,7 @@ void tty_unthrottle(struct tty_struct *tty)
 	if (test_and_clear_bit(TTY_THROTTLED, &tty->flags) &&
 	    tty->ops->unthrottle)
 		tty->ops->unthrottle(tty);
-	tty->flow_change = 0;
+	tty->flow_change = TTY_FLOW_NO_CHANGE;
 	up_write(&tty->termios_rwsem);
 }
 EXPORT_SYMBOL(tty_unthrottle);
-- 
2.42.0

Re: [PATCH 12/15] tty: convert THROTTLE constants into enum

[Ilpo Järvinen]

On Tue, 19 Sep 2023, Jiri Slaby (SUSE) wrote:

> And make an explicit constant for zero too. This allows for easier type
> checking of the parameter.
> 
> Note: tty_struct::flow_change is kept as int because include/tty.h
> (tty_struct) doesn't see tty/tty.h (this enum).

And it cannot moved there because of what?

-- 
 i.


> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> ---
>  drivers/tty/tty.h       | 13 +++++++++----
>  drivers/tty/tty_ioctl.c |  2 +-
>  2 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/tty/tty.h b/drivers/tty/tty.h
> index 50862f98273e..93cf5ef1e857 100644
> --- a/drivers/tty/tty.h
> +++ b/drivers/tty/tty.h
> @@ -41,15 +41,20 @@ enum {
>  };
>  
>  /* Values for tty->flow_change */
> -#define TTY_THROTTLE_SAFE	1
> -#define TTY_UNTHROTTLE_SAFE	2
> +enum tty_flow_change {
> +	TTY_FLOW_NO_CHANGE,
> +	TTY_THROTTLE_SAFE,
> +	TTY_UNTHROTTLE_SAFE,
> +};
>  
> -static inline void __tty_set_flow_change(struct tty_struct *tty, int val)
> +static inline void __tty_set_flow_change(struct tty_struct *tty,
> +					 enum tty_flow_change val)
>  {
>  	tty->flow_change = val;
>  }
>  
> -static inline void tty_set_flow_change(struct tty_struct *tty, int val)
> +static inline void tty_set_flow_change(struct tty_struct *tty,
> +				       enum tty_flow_change val)
>  {
>  	tty->flow_change = val;
>  	smp_mb();
> diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
> index 42c793e9d131..4b499301a3db 100644
> --- a/drivers/tty/tty_ioctl.c
> +++ b/drivers/tty/tty_ioctl.c
> @@ -104,7 +104,7 @@ void tty_unthrottle(struct tty_struct *tty)
>  	if (test_and_clear_bit(TTY_THROTTLED, &tty->flags) &&
>  	    tty->ops->unthrottle)
>  		tty->ops->unthrottle(tty);
> -	tty->flow_change = 0;
> +	tty->flow_change = TTY_FLOW_NO_CHANGE;
>  	up_write(&tty->termios_rwsem);
>  }
>  EXPORT_SYMBOL(tty_unthrottle);
> 

Re: [PATCH 12/15] tty: convert THROTTLE constants into enum

[Jiri Slaby]

On 19. 09. 23, 12:10, Ilpo Järvinen wrote:
> On Tue, 19 Sep 2023, Jiri Slaby (SUSE) wrote:
> 
>> And make an explicit constant for zero too. This allows for easier type
>> checking of the parameter.
>>
>> Note: tty_struct::flow_change is kept as int because include/tty.h
>> (tty_struct) doesn't see tty/tty.h (this enum).
> 
> And it cannot moved there because of what?

It's possible, but would make the constants "public". I hoped for 
forward enum declaration, but since enum is no longer a simple int, the 
compilation then fails with incomplete type of tty_struct.

thanks,
-- 
js
suse labs

[PATCH 13/15] tty: early return from send_break() on TTY_DRIVER_HARDWARE_BREAK

[Jiri Slaby (SUSE)]

If the driver sets TTY_DRIVER_HARDWARE_BREAK, we leave ops->break_ctl()
to the driver and return from send_break(). But we do it using a local
variable and keep the code flowing through the end of the function.
Instead, do 'return' immediately with the ops->break_ctl()'s return
value.

This way, we don't have to stuff the 'else' branch of the 'if' with the
software break handling. And we can re-indent the function too.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/tty_io.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 2ed12ca7c832..87bb5094e0bb 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2475,22 +2475,24 @@ static int send_break(struct tty_struct *tty, unsigned int duration)
 		return 0;
 
 	if (tty->driver->flags & TTY_DRIVER_HARDWARE_BREAK)
-		retval = tty->ops->break_ctl(tty, duration);
-	else {
-		/* Do the work ourselves */
-		if (tty_write_lock(tty, false) < 0)
-			return -EINTR;
-		retval = tty->ops->break_ctl(tty, -1);
-		if (retval)
-			goto out;
-		if (!signal_pending(current))
-			msleep_interruptible(duration);
-		retval = tty->ops->break_ctl(tty, 0);
+		return tty->ops->break_ctl(tty, duration);
+
+	/* Do the work ourselves */
+	if (tty_write_lock(tty, false) < 0)
+		return -EINTR;
+
+	retval = tty->ops->break_ctl(tty, -1);
+	if (retval)
+		goto out;
+	if (!signal_pending(current))
+		msleep_interruptible(duration);
+	retval = tty->ops->break_ctl(tty, 0);
 out:
-		tty_write_unlock(tty);
-		if (signal_pending(current))
-			retval = -EINTR;
-	}
+	tty_write_unlock(tty);
+
+	if (signal_pending(current))
+		retval = -EINTR;
+
 	return retval;
 }
 
-- 
2.42.0

[PATCH 14/15] tty: don't check for signal_pending() in send_break()

[Jiri Slaby (SUSE)]

msleep_interruptible() will check on its own. So no need to do the check
in send_break() before calling the above.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/tty_io.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 87bb5094e0bb..24833b31b81c 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2484,8 +2484,7 @@ static int send_break(struct tty_struct *tty, unsigned int duration)
 	retval = tty->ops->break_ctl(tty, -1);
 	if (retval)
 		goto out;
-	if (!signal_pending(current))
-		msleep_interruptible(duration);
+	msleep_interruptible(duration);
 	retval = tty->ops->break_ctl(tty, 0);
 out:
 	tty_write_unlock(tty);
-- 
2.42.0

Re: [PATCH 14/15] tty: don't check for signal_pending() in send_break()

[Ilpo Järvinen]

On Tue, 19 Sep 2023, Jiri Slaby (SUSE) wrote:

> msleep_interruptible() will check on its own. So no need to do the check

For clarity:

... will check !signal_pending() on its own.

> in send_break() before calling the above.
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> ---
>  drivers/tty/tty_io.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index 87bb5094e0bb..24833b31b81c 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -2484,8 +2484,7 @@ static int send_break(struct tty_struct *tty, unsigned int duration)
>  	retval = tty->ops->break_ctl(tty, -1);
>  	if (retval)
>  		goto out;
> -	if (!signal_pending(current))
> -		msleep_interruptible(duration);
> +	msleep_interruptible(duration);
>  	retval = tty->ops->break_ctl(tty, 0);
>  out:
>  	tty_write_unlock(tty);
> 

-- 
 i.

[PATCH 15/15] tty: use 'if' in send_break() instead of 'goto'

[Jiri Slaby (SUSE)]

Now, the "jumped-over" code is simple enough to be put inside an 'if'.
Do so to make it 'goto'-less.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/tty_io.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 24833b31b81c..378257c4c41a 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2482,11 +2482,10 @@ static int send_break(struct tty_struct *tty, unsigned int duration)
 		return -EINTR;
 
 	retval = tty->ops->break_ctl(tty, -1);
-	if (retval)
-		goto out;
-	msleep_interruptible(duration);
-	retval = tty->ops->break_ctl(tty, 0);
-out:
+	if (!retval) {
+		msleep_interruptible(duration);
+		retval = tty->ops->break_ctl(tty, 0);
+	}
 	tty_write_unlock(tty);
 
 	if (signal_pending(current))
-- 
2.42.0

Re: [PATCH 00/15] random tty fixes

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2151 bytes --]

On Tue, 19 Sep 2023, Jiri Slaby (SUSE) wrote:

> This is a collection of random fixes for tty I did while crawling
> through the code. Mostly done for readability and understandability. No
> behavior change intended (except for Documentation fixes).
> 
> Jiri Slaby (SUSE) (15):
>   tty: n_tty: use 'retval' instead of 'c'
>   tty: n_tty: rename and retype 'retval' in n_tty_ioctl()
>   tty: n_tty: use min3() in copy_from_read_buf()
>   tty: n_tty: invert the condition in copy_from_read_buf()
>   tty: n_tty: use do-while in n_tty_check_{,un}throttle()
>   tty: switch tty_{,un}throttle_safe() to return a bool
>   tty: invert return values of tty_{,un}throttle_safe()
>   tty: fix up and plug in tty_ioctl kernel-doc
>   tty: fix kernel-doc for functions in tty.h
>   tty: stop using ndash in kernel-doc
>   tty: tty_buffer: use bool for 'restart' in
>     tty_buffer_unlock_exclusive()
>   tty: convert THROTTLE constants into enum
>   tty: early return from send_break() on TTY_DRIVER_HARDWARE_BREAK
>   tty: don't check for signal_pending() in send_break()
>   tty: use 'if' in send_break() instead of 'goto'
> 
>  Documentation/driver-api/tty/index.rst     |   1 +
>  Documentation/driver-api/tty/tty_ioctl.rst |  10 +
>  drivers/tty/n_tty.c                        |  77 ++++---
>  drivers/tty/tty.h                          |  13 +-
>  drivers/tty/tty_buffer.c                   |   5 +-
>  drivers/tty/tty_io.c                       |  36 ++--
>  drivers/tty/tty_ioctl.c                    | 234 ++++++++++-----------
>  drivers/tty/tty_port.c                     |   6 +-
>  drivers/tty/vt/consolemap.c                |   2 +-
>  drivers/tty/vt/vc_screen.c                 |   4 +-
>  drivers/tty/vt/vt.c                        |   4 +-
>  include/linux/tty.h                        |  25 +--
>  12 files changed, 209 insertions(+), 208 deletions(-)
>  create mode 100644 Documentation/driver-api/tty/tty_ioctl.rst

For this whole series except the patches I commented on:

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

...If you make the amendments I requested, please add the tag also to the 
patches I commented on.

-- 
 i.

Re: [PATCH 00/15] random tty fixes

[Greg Kroah-Hartman]

On Tue, Sep 19, 2023 at 01:17:49PM +0300, Ilpo Järvinen wrote:
> On Tue, 19 Sep 2023, Jiri Slaby (SUSE) wrote:
> 
> > This is a collection of random fixes for tty I did while crawling
> > through the code. Mostly done for readability and understandability. No
> > behavior change intended (except for Documentation fixes).
> > 
> > Jiri Slaby (SUSE) (15):
> >   tty: n_tty: use 'retval' instead of 'c'
> >   tty: n_tty: rename and retype 'retval' in n_tty_ioctl()
> >   tty: n_tty: use min3() in copy_from_read_buf()
> >   tty: n_tty: invert the condition in copy_from_read_buf()
> >   tty: n_tty: use do-while in n_tty_check_{,un}throttle()
> >   tty: switch tty_{,un}throttle_safe() to return a bool
> >   tty: invert return values of tty_{,un}throttle_safe()
> >   tty: fix up and plug in tty_ioctl kernel-doc
> >   tty: fix kernel-doc for functions in tty.h
> >   tty: stop using ndash in kernel-doc
> >   tty: tty_buffer: use bool for 'restart' in
> >     tty_buffer_unlock_exclusive()
> >   tty: convert THROTTLE constants into enum
> >   tty: early return from send_break() on TTY_DRIVER_HARDWARE_BREAK
> >   tty: don't check for signal_pending() in send_break()
> >   tty: use 'if' in send_break() instead of 'goto'
> > 
> >  Documentation/driver-api/tty/index.rst     |   1 +
> >  Documentation/driver-api/tty/tty_ioctl.rst |  10 +
> >  drivers/tty/n_tty.c                        |  77 ++++---
> >  drivers/tty/tty.h                          |  13 +-
> >  drivers/tty/tty_buffer.c                   |   5 +-
> >  drivers/tty/tty_io.c                       |  36 ++--
> >  drivers/tty/tty_ioctl.c                    | 234 ++++++++++-----------
> >  drivers/tty/tty_port.c                     |   6 +-
> >  drivers/tty/vt/consolemap.c                |   2 +-
> >  drivers/tty/vt/vc_screen.c                 |   4 +-
> >  drivers/tty/vt/vt.c                        |   4 +-
> >  include/linux/tty.h                        |  25 +--
> >  12 files changed, 209 insertions(+), 208 deletions(-)
> >  create mode 100644 Documentation/driver-api/tty/tty_ioctl.rst
> 
> For this whole series except the patches I commented on:
> 
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> 
> ...If you make the amendments I requested, please add the tag also to the 
> patches I commented on.

That's horrible, b4 will now just add this reviewed-by to all of them
when I run it :(

greg k-h

Re: [PATCH 00/15] random tty fixes

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2582 bytes --]

On Tue, 19 Sep 2023, Greg Kroah-Hartman wrote:

> On Tue, Sep 19, 2023 at 01:17:49PM +0300, Ilpo Järvinen wrote:
> > On Tue, 19 Sep 2023, Jiri Slaby (SUSE) wrote:
> > 
> > > This is a collection of random fixes for tty I did while crawling
> > > through the code. Mostly done for readability and understandability. No
> > > behavior change intended (except for Documentation fixes).
> > > 
> > > Jiri Slaby (SUSE) (15):
> > >   tty: n_tty: use 'retval' instead of 'c'
> > >   tty: n_tty: rename and retype 'retval' in n_tty_ioctl()
> > >   tty: n_tty: use min3() in copy_from_read_buf()
> > >   tty: n_tty: invert the condition in copy_from_read_buf()
> > >   tty: n_tty: use do-while in n_tty_check_{,un}throttle()
> > >   tty: switch tty_{,un}throttle_safe() to return a bool
> > >   tty: invert return values of tty_{,un}throttle_safe()
> > >   tty: fix up and plug in tty_ioctl kernel-doc
> > >   tty: fix kernel-doc for functions in tty.h
> > >   tty: stop using ndash in kernel-doc
> > >   tty: tty_buffer: use bool for 'restart' in
> > >     tty_buffer_unlock_exclusive()
> > >   tty: convert THROTTLE constants into enum
> > >   tty: early return from send_break() on TTY_DRIVER_HARDWARE_BREAK
> > >   tty: don't check for signal_pending() in send_break()
> > >   tty: use 'if' in send_break() instead of 'goto'
> > > 
> > >  Documentation/driver-api/tty/index.rst     |   1 +
> > >  Documentation/driver-api/tty/tty_ioctl.rst |  10 +
> > >  drivers/tty/n_tty.c                        |  77 ++++---
> > >  drivers/tty/tty.h                          |  13 +-
> > >  drivers/tty/tty_buffer.c                   |   5 +-
> > >  drivers/tty/tty_io.c                       |  36 ++--
> > >  drivers/tty/tty_ioctl.c                    | 234 ++++++++++-----------
> > >  drivers/tty/tty_port.c                     |   6 +-
> > >  drivers/tty/vt/consolemap.c                |   2 +-
> > >  drivers/tty/vt/vc_screen.c                 |   4 +-
> > >  drivers/tty/vt/vt.c                        |   4 +-
> > >  include/linux/tty.h                        |  25 +--
> > >  12 files changed, 209 insertions(+), 208 deletions(-)
> > >  create mode 100644 Documentation/driver-api/tty/tty_ioctl.rst
> > 
> > For this whole series except the patches I commented on:
> > 
> > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > 
> > ...If you make the amendments I requested, please add the tag also to the 
> > patches I commented on.
> 
> That's horrible, b4 will now just add this reviewed-by to all of them
> when I run it :(

Okay, I'll try to avoid it in the future.

-- 
 i.