On Mon, Nov 25, 2024 at 02:50:56PM +0100, Arnd Bergmann wrote:
> On Mon, Nov 25, 2024, at 12:26, Andy Shevchenko wrote:
> > On Mon, Nov 25, 2024 at 12:06:16PM +0100, Arnd Bergmann wrote:
...
> >> What I suspect is going on with the f4c23a140d80 commit
> >> is the same bug I mentioned earlier in this thread, where
> >> __serial8250_isa_init_ports() just always registers
> >> 'nr_uarts' (CONFIG_SERIAL_8250_RUNTIME_UARTS) ports,
> >> unlike any other serial driver.
> >
> > But the configuration can give less than old_serial_port contains.
> > See dozens of the explicit settings in the defconfigs.
>
> I don't see any of the upstream defconfigs doing this
> though, the only ones setting CONFIG_SERIAL_8250_RUNTIME_UARTS
> are those that have an empty old_serial_port[].
A-ha, a good catch. I haven't checked the actual contents of the
old_serial_port for those configurations.
> Note that SERIAL_PORT_DFNS is only defined on x86, alpha
> and m68k (for q40), which are the main PC-like platforms.
> I see that all three have identical definitions of
> SERIAL_PORT_DFNS, so I think these should just be moved
> next to the __serial8250_isa_init_ports definition, with
> the entire thing moved into a separate ISA driver or
> an #ifdef around it. This is of course not the problem
> at hand, but it would help separate the x86/isa and
> non-x86 platform device cases further.
It's nice idea, but yes, we can think about it later.
> >> This used to be required before 9d86719f8769 ("serial:
> >> 8250: Allow using ports higher than SERIAL_8250_RUNTIME_UARTS"),
> >> but I don't see why this is still a thing now, other than
> >> for using setserial on i486-class PCs with nonstandard ISA
> >> ports.
> >>
> >> On non-x86 machines, it only ever seems to create extra
> >> ports that are likely to crash the system if opened, either
> >> because they lack proper serial_in/serial_out callbacks,
> >> or because the default UPIO_PORT callbacks end up poking
> >> unmapped memory.
> >>
> >> Do you see any reason why we can't just do the version below?
> >
> > Perhaps we may do this way (it seems better to me than previous
> > suggestions), but it also needs to be carefully checked against
> > those configurations that set it explicitly.
>
> Yes, at least to make sure that the numbering of the uarts
> does not change. I expect it's actually the same, but don't
> know for sure.
Me neither. And the issue with NULL pointer dereference needs to be retested.
--
With Best Regards,
Andy Shevchenko
