Thread (107 messages) 107 messages, 11 authors, 2013-01-03

[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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help