Re: [PATCH v2 2/3] term/serial: Add support for PCI serial devices

[Daniel Kiper]

On Mon, Feb 13, 2023 at 08:23:39AM +1100, Benjamin Herrenschmidt wrote:
> On Thu, 2023-02-09 at 15:52 +0100, Daniel Kiper wrote:
> > Adding another Ben's address...
>
> Ah ! I saw the previous one before breakfast the other day, and forgot
> to reply once properly caffeinated :-)
>
> > > Ben, you've lived a very long time in !x86 land, can you elucidate us on
> > > this? My thinking is that if someone sticks a 16550 on a PCI board, this
> > > driver should work on any PCI enabled platform, not just x86, rite?
> >
> > Ben, here ^^^ is the question for you...
>
> It's ... complicated :-)
>
> On most non-x86 platforms you don't have a 1:1 mapping between PCI BAR
> value and the corresponding CPU address. Now I think with ACPI and UEFI
> there are ways to convert but I don't remember them off the top of my
> mind, there are similar ways with Open Firmware or flat device-trees.
> But grub doesn't have the infrastructure for that.
>
> Also non-x86 don't have "IO ports" per se, they are mapped somewhere in
> the MMIO address space, and how that is done is very platform specific.
>
> So at this point it makes sense to leave that x86 only. Once can add
> support for other archs/platforms later with the appopriate address
> conversion trickery.
>
> Another issue is that grub  doesn't have any form of MMIO abstraction
> layer providing things like ordering enforcement (memory barriers)
> etc... so doing any kind of direct device access in grub on most non-
> x86 is chancy at best.

Ben, thank you for explanation. Taking into account above...

> --- grub.orig/grub-core/Makefile.core.def
> +++ grub/grub-core/Makefile.core.def
> @@ -2047,6 +2047,7 @@ module = {
>    ieee1275 = term/ieee1275/serial.c;
>    mips_arc = term/arc/serial.c;
>    efi = term/efi/serial.c;
> +  pci = term/pci/serial.c;

... I still think this should be "x86 = term/pci/serial.c".

Until you push me to leave it as "pci = term/pci/serial.c".
However, if you want to do that please bring in good arguments... :-)

Daniel