On Tue, 2024-10-15 at 11:10 -0700, Adam Williamson wrote:
>
> 7b192ec4c rejigged the serial port detection code when no port
> is explicitly specified. Before 7b192ec4c we did
> grub_serial_find ("com0") in this case, which on *any* platform
> would return a port called "com0" if one was found, otherwise it
> fell through to the later blocks about ns8250 and ieee1275.
> 7b192ec4c changed this so we do grub_serial_find ("auto"), and
> added a block to handle the "auto" string which tries to detect
> a port from the SPCR by calling grub_ns8250_spcr_init (), then
> falls through to com0 if that doesn't work.
>
> However, that block was placed much later than a call with "com0"
> would be handled, and was wrapped in two ifdefs that limit its
> scope. This means that we would no longer default to com0 unless
> the requirements of *both* ifdefs are met.
>
> To fix this, move the handling of "auto" up to happen right after
> the exact port name match (and before the mips/x86/EMU/ARC ifdef
> kicks in), and change the ifdef that is meant to block use of
> grub_ns8250_spcr_init on inappropriate platforms to *only* wrap
> the call to grub_ns8250_spcr_init, not *also* wrap the fallback
> to com0.
I am not entirely sure what the "moving up" changes in practice. The
various top-level if () blocks in grub_serial_find() all operates on
the name.
You are basically moving the
if (name is "auto")
From after to before the
if (name starts with "port")
I don't think that in itself has any impact or changes anything other
than being moved outside of the confines of the ifdef. So I assume
that's what you are after.
Correct me if I'm wrong but the only "issue" here is that if you are on
a target for which:
#if (defined(__i386__) || defined(__x86_64__)) &&
!defined(GRUB_MACHINE_IEEE1275) && !defined(GRUB_MACHINE_QEMU)
Isn't true, then the new "default" of "auto" will fail to match an existing
port in the list called "com0" right ?
In which case a simpler patch would have been maybe to just add to
the very top of the function insteadinstead
if (grub_strcmp (name, "auto") == 0)
{
#if (defined(__i386__) || defined(__x86_64__)) &&
!defined(GRUB_MACHINE_IEEE1275) && !defined(GRUB_MACHINE_QEMU)
/* Look for an SPCR if any. If not, default to com0. */
port = grub_ns8250_spcr_init ();
if (port != NULL)
return port;
#endif
/* Legacy default */
name = "com0";
}
Don't you think ?
Cheers,
Ben.
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
