[RFC v1 01/16] lib: devres: don't enclose pcim_*() functions in CONFIG_HAS_IOPORT
From: arnd@arndb.de (Arnd Bergmann)
Date: 2012-12-11 22:34:50
Also in:
lkml
On Tuesday 11 December 2012, Russell King - ARM Linux wrote:
On Tue, Dec 11, 2012 at 04:15:02PM +0000, Arnd Bergmann wrote:quoted
On Tuesday 11 December 2012, Thomas Petazzoni wrote:quoted
On Tue, 11 Dec 2012 10:43:49 +0000, Arnd Bergmann wrote:quoted
On Friday 07 December 2012, Thomas Petazzoni wrote:quoted
The pcim_*() functions are used by the libata-sff subsystem, and this subsystem is used for many SATA drivers on ARM platforms that do not necessarily have I/O ports.I think this one is wrong as the CONFIG_HAS_IOPORT does not refer to the presence of PIO ports but to whether or not they provide an ioport_map function. If there is no ioport_map(), devm_pci_iomap will fail to link as far as I can tell.The problem is that on ARCH_MULTI_V7, ARCH_VEXPRESS is forcefully enabled. And ARCH_VEXPRESS selects NO_IOPORT.. so you don't have the pcim_*() functions, and therefore libata-sff.c (needed for many SATA drivers) will not build. How do you solve this?What you describe here are probable two bugs, and we should fix both: * ARCH_VEXPRESS should not select NO_IOPORT. It's generally wrong to select this in combination with ARCH_MULTIPLATFORM, when some of the other platforms you may enable actually have IOPORT mapping support.No. ARCH_VEXPRESS selects NO_IOPORT because it does not support PCI/ISA IO space. That in itself is reasonable, but what isn't reasonable is the negative logic being used. Negative logic in the config system always tends to provoke this kind of sillyness because you're selecting something to be excluded which another platform may require.
Exactly, that is what I meant. Sorry if I wasn't clear enough.
We should instead have HAVE_IOPORT and platforms which need ISA/PCI IO space support should select this symbol instead - so it becomes an inclusive feature rather than an exclusive feature.
Right. Note that HAS_IOPORT already exists and is defined as
config HAS_IOPORT
boolean
depends on HAS_IOMEM && !NO_IOPORT
default y
If we change it to
config HAS_IOPORT
boolean
depends on HAS_IOMEM
default !NO_IOPORT
then we can actually select both NO_IOPORT and HAS_IOPORT with the result
of getting HAS_IOPORT. It is a bit confusing though to have both enabled,
so we might still want to use an approach where we only select NO_IOPORT
if we are sure that we can't have it.
Arnd