Re: [PATCH v9 06/10] serial: sh-sci: Use private port ID

Geert Uytterhoeven
2025-05-23T09:45:58Z

Hi Wolfram,

On Wed, 21 May 2025 at 13:00, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> On Thu, May 15, 2025 at 04:18:21PM +0200, Thierry Bultel wrote:
> > New port types cannot be added in serial_core.h, which is shared with
> > userspace.
> > In order to support new port types, the coming new ones will have
> > BIT(7) set in the id value, and in this case, uartport->type is
> > set to PORT_GENERIC.
> > This commit therefore changes all the places where the port type is
> > read, by not relying on uartport->type but on the private
> > value stored in struct sci_port.
>
> I quite like this approach to become independent of serial_core.h by
> adding a driver-local type. Because it changes only access to the
> variables but not much the logic of this driver. Two high level comments
> I do have:
>
> - I'd go for bit 31 as the flag, though. It is extremly unlikely that we
> ever need a number in serial_core.h again, but if, it could likely be
> > 127

Actually I asked Thierry to use bit 7, so both type and regtype can
fit in the existing hole in struct sci_port.

AFAIU, there is a hard moratorium on adding new public numbers to
serial_core.h. I doubt this field has ever been used by userspace
for anything other than 8250 and derivatives. The modern way to
discover the serial port type would be to look into sysfs anyway.

> - whatever bit numer we choose, it should be hidden as a constant. My
> suggestion:
>
> #define SCI_LOCAL_PORT_FLAG BIT(x)
>
> ?

I agree that would be good to have.

Alternatively:

enum {
SCI_PORT_LOCAL = 128,
SCI_PORT_RSCI,
};

and

#define SCI_PUBLIC_PORT_ID(port) (((port) >= SCI_PORT_LOCAL) ?
PORT_GENERIC : (port))

and 128 can be changed easily when the need ever arises?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds