On Fri, 4 Oct 2024, Niklas Schnelle wrote:
> > Can we just provide dummy prototypes with `__attribute__((error("...")))'
> > instead? This will surely prevent us from having to bend backwards so as
> > to make sure the compiler won't see any spurious references to these
> > inexistent functions or macros. We already have a `__compiletime_error()'
> > macro for this purpose even.
>
> This is already done in the final patch of my series when disabling
> inb()/outb() and friends.
Good!
> > I agree. Enthusiastically.
>
> I think there was also a bit of a misunderstanding. My argument that
> this would be very ugly in the general case was really meant as general
> case outside of drivers like 8250 that deals with both I/O port and
> MMIO i.e. we can't warn/error when !HAS_IOPORT deactivates a whole
> driver because seeing an I/O port BAR in common PCI code doesn't mean
> that it is required for use of the device.
Absolutely. Just seeing an I/O bar does not mean it's ever going to be
used. Even conventional PCI hardware is often dual-mapped and it's up to
the driver to choose which mapping to use.
> I'm working on a new proposal for 8250 now. Basically I think we can
> put the warning/error in serial8250_pci_setup_port(). And then for
> those 8250_pci.c "sub drivers" that require I/O ports instead of just
> ifdeffing out their setup/init/exit we can define anything but setup to
> NULL and setup to pci_default_setup() such that the latter will find
> the I/O port BAR via FL_GET_BASE() and subsequently cause the error
> print in serial8250_pci_setup_port(). It's admittedly a bit odd but it
> also keeps the #ifdefs to just around the code that wouldn't compile.
I'd rather you did what Arnd example patch does and just made original
handlers bail out right away unless IS_ENABLED(CONFIG_HAS_IOPORT). I do
hope it will make no #ifdef necessary in 8250_pci.c at all.
Maciej
