Re: [PATCH 4/5] serial: core, 8250: Add a hook for extra port property reporting

From: Maciej W. Rozycki
Date: Sat Jun 26 2021 - 00:12:43 EST


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