Add a hook for `uart_report_port' to let serial ports report extra
properties beyond `irq' and `base_baud'. Use it with the 8250 backend
to report extra baud rates supported above the base rate for ports with
the UPF_MAGIC_MULTIPLIER property, so that people have a way to find out
that they are supported with their system, e.g.:
Serial: 8250/16550 driver, 5 ports, IRQ sharing enabled
printk: console [ttyS0] disabled
serial8250.0: ttyS0 at I/O 0x3f8 (irq = 4, base_baud = 115200 [+230400, 460800]) is a 16550A
printk: console [ttyS0] enabled
printk: console [ttyS0] enabled
printk: bootconsole [uart8250] disabled
printk: bootconsole [uart8250] disabled
serial8250.0: ttyS1 at I/O 0x2f8 (irq = 3, base_baud = 115200 [+230400, 460800]) is a 16550A
serial8250.0: ttyS2 at MMIO 0x1f000900 (irq = 20, base_baud = 230400) is a 16550A
Otherwise there is no clear way to figure this out.
Signed-off-by: Maciej W. Rozycki <[email protected]>
---
drivers/tty/serial/8250/8250_core.c | 10 ++++++++++
drivers/tty/serial/serial_core.c | 11 +++++++++--
include/linux/serial_core.h | 3 +++
3 files changed, 22 insertions(+), 2 deletions(-)
linux-serial-core-baud-extra.diff
Index: linux-malta-cbus-uart/drivers/tty/serial/8250/8250_core.c
===================================================================
--- linux-malta-cbus-uart.orig/drivers/tty/serial/8250/8250_core.c
+++ linux-malta-cbus-uart/drivers/tty/serial/8250/8250_core.c
@@ -952,6 +952,13 @@ static struct uart_8250_port *serial8250
return NULL;
}
+static void serial8250_report_magic(struct uart_port *port,
+ char *report_buf, size_t report_size)
+{
+ snprintf(report_buf, report_size,
+ " [+%d, %d]", port->uartclk / 8, port->uartclk / 4);
+}
+
static void serial_8250_overrun_backoff_work(struct work_struct *work)
{
struct uart_8250_port *up =
@@ -1048,6 +1055,9 @@ int serial8250_register_8250_port(struct
}
}
+ if (up->port.flags & UPF_MAGIC_MULTIPLIER)
+ uart->port.report_extra = serial8250_report_magic;
+
serial8250_set_defaults(uart);
/* Possibly override default I/O functions. */
Index: linux-malta-cbus-uart/drivers/tty/serial/serial_core.c
===================================================================
--- linux-malta-cbus-uart.orig/drivers/tty/serial/serial_core.c
+++ linux-malta-cbus-uart/drivers/tty/serial/serial_core.c
@@ -2309,6 +2309,7 @@ int uart_resume_port(struct uart_driver
static inline void
uart_report_port(struct uart_driver *drv, struct uart_port *port)
{
+ char report_extra[64];
char address[64];
switch (port->iotype) {
@@ -2333,11 +2334,17 @@ uart_report_port(struct uart_driver *drv
break;
}
- pr_info("%s%s%s at %s (irq = %d, base_baud = %d) is a %s\n",
+ if (port->report_extra)
+ port->report_extra(port, report_extra, sizeof(report_extra));
+ else
+ report_extra[0] = '\0';
+
+ pr_info("%s%s%s at %s (irq = %d, base_baud = %d%s) is a %s\n",
port->dev ? dev_name(port->dev) : "",
port->dev ? ": " : "",
port->name,
- address, port->irq, port->uartclk / 16, uart_type(port));
+ address, port->irq, port->uartclk / 16, report_extra,
+ uart_type(port));
}
static void
Index: linux-malta-cbus-uart/include/linux/serial_core.h
===================================================================
--- linux-malta-cbus-uart.orig/include/linux/serial_core.h
+++ linux-malta-cbus-uart/include/linux/serial_core.h
@@ -135,6 +135,9 @@ struct uart_port {
struct serial_rs485 *rs485);
int (*iso7816_config)(struct uart_port *,
struct serial_iso7816 *iso7816);
+ void (*report_extra)(struct uart_port *port,
+ char *report_buf,
+ size_t report_size);
unsigned int irq; /* irq number */
unsigned long irqflags; /* irq flags */
unsigned int uartclk; /* base uart clock */
On Thu, Jun 10, 2021 at 08:38:44PM +0200, Maciej W. Rozycki wrote:
> Add a hook for `uart_report_port' to let serial ports report extra
> properties beyond `irq' and `base_baud'. Use it with the 8250 backend
> to report extra baud rates supported above the base rate for ports with
> the UPF_MAGIC_MULTIPLIER property, so that people have a way to find out
> that they are supported with their system, e.g.:
>
> Serial: 8250/16550 driver, 5 ports, IRQ sharing enabled
> printk: console [ttyS0] disabled
> serial8250.0: ttyS0 at I/O 0x3f8 (irq = 4, base_baud = 115200 [+230400, 460800]) is a 16550A
> printk: console [ttyS0] enabled
> printk: console [ttyS0] enabled
> printk: bootconsole [uart8250] disabled
> printk: bootconsole [uart8250] disabled
> serial8250.0: ttyS1 at I/O 0x2f8 (irq = 3, base_baud = 115200 [+230400, 460800]) is a 16550A
> serial8250.0: ttyS2 at MMIO 0x1f000900 (irq = 20, base_baud = 230400) is a 16550A
>
> Otherwise there is no clear way to figure this out.
>
> Signed-off-by: Maciej W. Rozycki <[email protected]>
> ---
> drivers/tty/serial/8250/8250_core.c | 10 ++++++++++
> drivers/tty/serial/serial_core.c | 11 +++++++++--
> include/linux/serial_core.h | 3 +++
> 3 files changed, 22 insertions(+), 2 deletions(-)
>
> linux-serial-core-baud-extra.diff
> Index: linux-malta-cbus-uart/drivers/tty/serial/8250/8250_core.c
> ===================================================================
> --- linux-malta-cbus-uart.orig/drivers/tty/serial/8250/8250_core.c
> +++ linux-malta-cbus-uart/drivers/tty/serial/8250/8250_core.c
> @@ -952,6 +952,13 @@ static struct uart_8250_port *serial8250
> return NULL;
> }
>
> +static void serial8250_report_magic(struct uart_port *port,
> + char *report_buf, size_t report_size)
> +{
> + snprintf(report_buf, report_size,
> + " [+%d, %d]", port->uartclk / 8, port->uartclk / 4);
> +}
> +
> static void serial_8250_overrun_backoff_work(struct work_struct *work)
> {
> struct uart_8250_port *up =
> @@ -1048,6 +1055,9 @@ int serial8250_register_8250_port(struct
> }
> }
>
> + if (up->port.flags & UPF_MAGIC_MULTIPLIER)
> + uart->port.report_extra = serial8250_report_magic;
> +
> serial8250_set_defaults(uart);
>
> /* Possibly override default I/O functions. */
> Index: linux-malta-cbus-uart/drivers/tty/serial/serial_core.c
> ===================================================================
> --- linux-malta-cbus-uart.orig/drivers/tty/serial/serial_core.c
> +++ linux-malta-cbus-uart/drivers/tty/serial/serial_core.c
> @@ -2309,6 +2309,7 @@ int uart_resume_port(struct uart_driver
> static inline void
> uart_report_port(struct uart_driver *drv, struct uart_port *port)
> {
> + char report_extra[64];
> char address[64];
>
> switch (port->iotype) {
> @@ -2333,11 +2334,17 @@ uart_report_port(struct uart_driver *drv
> break;
> }
>
> - pr_info("%s%s%s at %s (irq = %d, base_baud = %d) is a %s\n",
> + if (port->report_extra)
> + port->report_extra(port, report_extra, sizeof(report_extra));
> + else
> + report_extra[0] = '\0';
> +
> + pr_info("%s%s%s at %s (irq = %d, base_baud = %d%s) is a %s\n",
> port->dev ? dev_name(port->dev) : "",
> port->dev ? ": " : "",
> port->name,
> - address, port->irq, port->uartclk / 16, uart_type(port));
> + address, port->irq, port->uartclk / 16, report_extra,
> + uart_type(port));
Ick, really? What relies on this print message? Why do we need a whole
new uart port hook for this?
Isn't there some other way for your specific variant to print out
another message if you really want to do something "odd" like this?
And you did not document what your new change did anywhere in the tree,
so people are going to be confused.
I've taken the other patches here, but not this one.
thanks,
greg k-h
On Tue, 15 Jun 2021, Greg Kroah-Hartman wrote:
> > Add a hook for `uart_report_port' to let serial ports report extra
> > properties beyond `irq' and `base_baud'. Use it with the 8250 backend
> > to report extra baud rates supported above the base rate for ports with
> > the UPF_MAGIC_MULTIPLIER property, so that people have a way to find out
> > that they are supported with their system, e.g.:
[...]
> Ick, really? What relies on this print message? Why do we need a whole
> new uart port hook for this?
As I noted, perhaps too briefly, in the commit description (see the last
sentence above) people need to be made aware of the extra baud rates above
`base_baud' supported, or otherwise they'll have no way to figure out they
can use them.
Reporting tweaked `base_baud' would I think cause confusion from the
inconsistency with the TIOCGSERIAL/TIOCSSERIAL ioctls (e.g. `setserial'),
and the sysfs flags:
$ cat /sys/class/tty/ttyS[0-2]/flags
0x10010040
0x10010040
0x90000040
$
(here from a Malta board) are IMO too obscure for anyone to figure this
out (bit 16 means the two extra baud rates are supported).
As explained with 1/5 we could set `base_baud' to 460800 instead and
hardcode bit 15 of the divisor to 1, relying on undocumented Super I/O IC
behaviour, but that would require more exhaustive verification than I am
able to do with just a single board and IC type and revision.
> Isn't there some other way for your specific variant to print out
> another message if you really want to do something "odd" like this?
There's always another way. :) How about?
serial8250.0: ttyS0 at I/O 0x3f8 (irq = 4, base_baud = 115200) is a 16550A
serial8250.0: ttyS0 extra baud rates supported: 230400, 460800
> And you did not document what your new change did anywhere in the tree,
> so people are going to be confused.
We've been somewhat terse about things, but you're probably right here.
Sorry about that.
> I've taken the other patches here, but not this one.
Thanks. I've posted an alternative printing a message like above, with
some commentary. Let me know if that makes you feel more convinced.
Maciej