[PATCH] (11/43) Kconfig fix (infiniband and PCI)

[PATCH] (11/43) Kconfig fix (infiniband and PCI)

[Al Viro]

infiniband uses PCI helpers all over the place (including the core parts) and
won't build without PCI.

Signed-off-by: Al Viro <viro@parcelfarce.linux.theplanet.co.uk>
----
diff -urN RC13-rc6-git13-m32r-pagealloc/drivers/infiniband/Kconfig RC13-rc6-git13-infiniband/drivers/infiniband/Kconfig
--- RC13-rc6-git13-m32r-pagealloc/drivers/infiniband/Kconfig	2005-08-10 10:37:48.000000000 -0400
+++ RC13-rc6-git13-infiniband/drivers/infiniband/Kconfig	2005-08-21 13:16:54.000000000 -0400
@@ -1,6 +1,7 @@
 menu "InfiniBand support"
 
 config INFINIBAND
+	depends on PCI || BROKEN
 	tristate "InfiniBand support"
 	---help---
 	  Core support for InfiniBand (IB).  Make sure to also select

Re: [PATCH] (11/43) Kconfig fix (infiniband and PCI)

[Adrian Bunk]

On Tue, Aug 23, 2005 at 10:45:41PM +0100, Al Viro wrote:

> infiniband uses PCI helpers all over the place (including the core parts) and
> won't build without PCI.
>...

CONFIG_INFINIBAND=y and CONFIG_PCI=n compiles for me on i386.

Can you post the compile error you got?

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] (11/43) Kconfig fix (infiniband and PCI)

[Al Viro]

On Wed, Aug 24, 2005 at 01:26:55PM +0200, Adrian Bunk wrote:
> On Tue, Aug 23, 2005 at 10:45:41PM +0100, Al Viro wrote:
> 
> > infiniband uses PCI helpers all over the place (including the core parts) and
> > won't build without PCI.
> >...
> 
> CONFIG_INFINIBAND=y and CONFIG_PCI=n compiles for me on i386.
> 
> Can you post the compile error you got?

On which platform?  There's a lot of them on the architectures that do not
have PCI at all - same situation as with firewire.  Note that you won't
get any low-level drivers on PCI-less config even on i386, so while I
agree that more accurate dependency would be nice here (as well as for
drivers/ieee1394), for all practical purposes the same dependency works
here.

BTW, this is more general question - do we expect pci helpers to be present
on all platforms and do we consider their use acceptable in code that does
not depend on PCI?

Again, for infiniband and ieee1394 it's an academical question, since neither
actually has any hardware it could talk to in .config without CONFIG_PCI...

Re: [PATCH] (11/43) Kconfig fix (infiniband and PCI)

[Adrian Bunk]

On Wed, Aug 24, 2005 at 03:57:36PM +0100, Al Viro wrote:
> On Wed, Aug 24, 2005 at 01:26:55PM +0200, Adrian Bunk wrote:
> > On Tue, Aug 23, 2005 at 10:45:41PM +0100, Al Viro wrote:
> > 
> > > infiniband uses PCI helpers all over the place (including the core parts) and
> > > won't build without PCI.
> > >...
> > 
> > CONFIG_INFINIBAND=y and CONFIG_PCI=n compiles for me on i386.
> > 
> > Can you post the compile error you got?
> 
> On which platform?  There's a lot of them on the architectures that do not

As I said, on i386.

> have PCI at all - same situation as with firewire.  Note that you won't
> get any low-level drivers on PCI-less config even on i386, so while I
> agree that more accurate dependency would be nice here (as well as for
> drivers/ieee1394), for all practical purposes the same dependency works
> here.
> 
> BTW, this is more general question - do we expect pci helpers to be present
> on all platforms and do we consider their use acceptable in code that does
> not depend on PCI?
>...

Are you talking about the ones that already have dummy functions for the 
PCI=n case in include/linux/pci.h, or about other functions?

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] (11/43) Kconfig fix (infiniband and PCI)

[Al Viro]

On Wed, Aug 24, 2005 at 05:26:09PM +0200, Adrian Bunk wrote:
> As I said, on i386.

Builds due to stubs being still present, doesn't do anything since it
doesn't even try to look for any hardware in that case.

> > have PCI at all - same situation as with firewire.  Note that you won't
> > get any low-level drivers on PCI-less config even on i386, so while I
> > agree that more accurate dependency would be nice here (as well as for
> > drivers/ieee1394), for all practical purposes the same dependency works
> > here.
> > 
> > BTW, this is more general question - do we expect pci helpers to be present
> > on all platforms and do we consider their use acceptable in code that does
> > not depend on PCI?
> >...
> 
> Are you talking about the ones that already have dummy functions for the 
> PCI=n case in include/linux/pci.h, or about other functions?

pci_map.../pci_unmap..., for one thing.

Re: [PATCH] (11/43) Kconfig fix (infiniband and PCI)

[Roland Dreier]

    Al> infiniband uses PCI helpers all over the place (including the
    Al> core parts) and won't build without PCI.

I don't think this is the right fix.  The only PCI helpers used in
code that is enabled with CONFIG_PCI=n are pci_unmap_addr_set() and
pci_unmap_addr().  And they're only used because no one has added
dma_unmap_addr_set() and dma_unmap_addr() -- the core code is properly
using the general dma_xxx API wherever possible.

There actually is non-PCI InfiniBand hardware coming, so we'll have to
fix this properly at some point.

 - R.

Re: [PATCH] (11/43) Kconfig fix (infiniband and PCI)

[Al Viro]

On Wed, Aug 24, 2005 at 09:22:27AM -0700, Roland Dreier wrote:
>     Al> infiniband uses PCI helpers all over the place (including the
>     Al> core parts) and won't build without PCI.
> 
> I don't think this is the right fix.  The only PCI helpers used in
> code that is enabled with CONFIG_PCI=n are pci_unmap_addr_set() and
> pci_unmap_addr().  And they're only used because no one has added
> dma_unmap_addr_set() and dma_unmap_addr() -- the core code is properly
> using the general dma_xxx API wherever possible.
> 
> There actually is non-PCI InfiniBand hardware coming, so we'll have to
> fix this properly at some point.

I'm all for it and removing BROKEN from Kconfig when fixes happen is
obviously not a problem at all ;-)

Re: [PATCH] (11/43) Kconfig fix (infiniband and PCI)

[Al Viro]

On Wed, Aug 24, 2005 at 05:31:34PM +0100, Al Viro wrote:
> On Wed, Aug 24, 2005 at 09:22:27AM -0700, Roland Dreier wrote:
> >     Al> infiniband uses PCI helpers all over the place (including the
> >     Al> core parts) and won't build without PCI.
> > 
> > I don't think this is the right fix.  The only PCI helpers used in
> > code that is enabled with CONFIG_PCI=n are pci_unmap_addr_set() and
> > pci_unmap_addr().  And they're only used because no one has added
> > dma_unmap_addr_set() and dma_unmap_addr() -- the core code is properly
> > using the general dma_xxx API wherever possible.
> > 
> > There actually is non-PCI InfiniBand hardware coming, so we'll have to
> > fix this properly at some point.
> 
> I'm all for it and removing BROKEN from Kconfig when fixes happen is
> obviously not a problem at all ;-)

PS: note that it's not
	depends on PCI
it's
	depends on PCI || BROKEN
which
	a) documents that something is wrong
	b) leaves all setups usable now intact
	c) prevents broken setups from being picked.

I certainly agree that proper fix is to switch to dma_... - no arguments here.
BTW, another dubious thing is use of DECLARE_PCI_UNMAP_ADDR() in infiniband
core - it's fine in PCI drivers (which is how it's used elsewhere), but not
in generic data structures.

Re: [PATCH] (11/43) Kconfig fix (infiniband and PCI)

[Roland Dreier]

    Al> PS: note that it's not depends on PCI it's depends on PCI ||
    Al> BROKEN which a) documents that something is wrong b) leaves
    Al> all setups usable now intact c) prevents broken setups from
    Al> being picked.

Yes, I agree that this makes sense for now.

    Al> I certainly agree that proper fix is to switch to dma_... - no
    Al> arguments here.  BTW, another dubious thing is use of
    Al> DECLARE_PCI_UNMAP_ADDR() in infiniband core - it's fine in PCI
    Al> drivers (which is how it's used elsewhere), but not in generic
    Al> data structures.

Yes, I guess we want DECLARE_DMA_UNMAP_ADDR() as well.  If I can
untangle the maze of .h files well enough to see where
dma_unmap_addr() et al. belong, I'll post a patch adding them.

 - R.