[PATCH] fix compile break caused by iomap: make IOPORT/PCI mapping functions conditional

[PATCH] fix compile break caused by iomap: make IOPORT/PCI mapping functions conditional

[James Bottomley]

The problem in

commit fea80311a939a746533a6d7e7c3183729d6a3faf
Author: Randy Dunlap <rdunlap@xenotime.net>
Date:   Sun Jul 24 11:39:14 2011 -0700

    iomap: make IOPORT/PCI mapping functions conditional


is that if your architecture supplies pci_iomap/pci_iounmap, it expects
always to supply them.  Adding empty body defitions in the !CONFIG_PCI
case, which is what this patch does, breaks the parisc compile because
the functions become doubly defined.  It took us a while to spot this,
because we don't actually build !CONFIG_PCI very often (only if someone
is brave enough to test the snake/asp machines).

Since the note in the commit log says this is to fix a
CONFIG_GENERIC_IOMAP issue (which it does because CONFIG_GENERIC_IOMAP
supplies pci_iounmap only if CONFIG_PCI is set), there should actually
have been a condition upon this.  This should make sure no other
architecture's !CONFIG_PCI compile breaks in the same way as parisc.

The fix had to be updated to take account of the GENERIC_PCI_IOMAP
separation.

Reported-by: Rolf Eike Beer <eike@sf-mail.de>
Signed-off-by: James Bottomley <JBottomley@Parallels.com>

---

diff --git a/include/asm-generic/iomap.h b/include/asm-generic/iomap.h
index 8a3d4fd..6afd7d6 100644
--- a/include/asm-generic/iomap.h
+++ b/include/asm-generic/iomap.h
@@ -70,7 +70,7 @@ extern void ioport_unmap(void __iomem *);
 /* Destroy a virtual mapping cookie for a PCI BAR (memory or IO) */
 struct pci_dev;
 extern void pci_iounmap(struct pci_dev *dev, void __iomem *);
-#else
+#elif defined(CONFIG_GENERIC_IOMAP)
 struct pci_dev;
 static inline void pci_iounmap(struct pci_dev *dev, void __iomem *addr)
 { }
diff --git a/include/asm-generic/pci_iomap.h b/include/asm-generic/pci_iomap.h
index 8de4b73..217eb3d 100644
--- a/include/asm-generic/pci_iomap.h
+++ b/include/asm-generic/pci_iomap.h
@@ -15,7 +15,7 @@ struct pci_dev;
 #ifdef CONFIG_PCI
 /* Create a virtual mapping cookie for a PCI BAR (memory or IO) */
 extern void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long max);
-#else
+#elif defined(CONFIG_GENERIC_PCI_IOMAP)
 static inline void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long max)
 {
 	return NULL;

Re: [PATCH] fix compile break caused by iomap: make IOPORT/PCI mapping functions conditional

[Rolf Eike Beer]

[-- Attachment #1: Type: text/plain, Size: 1548 bytes --]

Am Montag 30 Januar 2012, 10:40:47 schrieb James Bottomley:
> The problem in
> 
> commit fea80311a939a746533a6d7e7c3183729d6a3faf
> Author: Randy Dunlap <rdunlap@xenotime.net>
> Date:   Sun Jul 24 11:39:14 2011 -0700
> 
>     iomap: make IOPORT/PCI mapping functions conditional
> 
> 
> is that if your architecture supplies pci_iomap/pci_iounmap, it expects
> always to supply them.  Adding empty body defitions in the !CONFIG_PCI
> case, which is what this patch does, breaks the parisc compile because
> the functions become doubly defined.  It took us a while to spot this,
> because we don't actually build !CONFIG_PCI very often (only if someone
> is brave enough to test the snake/asp machines).
> 
> Since the note in the commit log says this is to fix a
> CONFIG_GENERIC_IOMAP issue (which it does because CONFIG_GENERIC_IOMAP
> supplies pci_iounmap only if CONFIG_PCI is set), there should actually
> have been a condition upon this.  This should make sure no other
> architecture's !CONFIG_PCI compile breaks in the same way as parisc.
> 
> The fix had to be updated to take account of the GENERIC_PCI_IOMAP
> separation.

So this means we end up still building the PA-RISC PCI code even if the config 
says no PCI. That doesn't really sound consistent to me. I really would have 
expected that we do not build any non-void PCI code then.

> Reported-by: Rolf Eike Beer <eike@sf-mail.de>

I used the wrong email account when sending out the last patch where you took 
that from, please change that to the address used in this mail.

Eike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] fix compile break caused by iomap: make IOPORT/PCI mapping functions conditional

[Rolf Eike Beer]

[-- Attachment #1: Type: text/plain, Size: 1548 bytes --]

Am Montag 30 Januar 2012, 10:40:47 schrieb James Bottomley:
> The problem in
> 
> commit fea80311a939a746533a6d7e7c3183729d6a3faf
> Author: Randy Dunlap <rdunlap@xenotime.net>
> Date:   Sun Jul 24 11:39:14 2011 -0700
> 
>     iomap: make IOPORT/PCI mapping functions conditional
> 
> 
> is that if your architecture supplies pci_iomap/pci_iounmap, it expects
> always to supply them.  Adding empty body defitions in the !CONFIG_PCI
> case, which is what this patch does, breaks the parisc compile because
> the functions become doubly defined.  It took us a while to spot this,
> because we don't actually build !CONFIG_PCI very often (only if someone
> is brave enough to test the snake/asp machines).
> 
> Since the note in the commit log says this is to fix a
> CONFIG_GENERIC_IOMAP issue (which it does because CONFIG_GENERIC_IOMAP
> supplies pci_iounmap only if CONFIG_PCI is set), there should actually
> have been a condition upon this.  This should make sure no other
> architecture's !CONFIG_PCI compile breaks in the same way as parisc.
> 
> The fix had to be updated to take account of the GENERIC_PCI_IOMAP
> separation.

So this means we end up still building the PA-RISC PCI code even if the config 
says no PCI. That doesn't really sound consistent to me. I really would have 
expected that we do not build any non-void PCI code then.

> Reported-by: Rolf Eike Beer <eike@sf-mail.de>

I used the wrong email account when sending out the last patch where you took 
that from, please change that to the address used in this mail.

Eike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] fix compile break caused by iomap: make IOPORT/PCI mapping functions conditional

[James Bottomley]

On Mon, 2012-01-30 at 21:10 +0100, Rolf Eike Beer wrote:
> Am Montag 30 Januar 2012, 10:40:47 schrieb James Bottomley:
> > The problem in
> > 
> > commit fea80311a939a746533a6d7e7c3183729d6a3faf
> > Author: Randy Dunlap <rdunlap@xenotime.net>
> > Date:   Sun Jul 24 11:39:14 2011 -0700
> > 
> >     iomap: make IOPORT/PCI mapping functions conditional
> > 
> > 
> > is that if your architecture supplies pci_iomap/pci_iounmap, it expects
> > always to supply them.  Adding empty body defitions in the !CONFIG_PCI
> > case, which is what this patch does, breaks the parisc compile because
> > the functions become doubly defined.  It took us a while to spot this,
> > because we don't actually build !CONFIG_PCI very often (only if someone
> > is brave enough to test the snake/asp machines).
> > 
> > Since the note in the commit log says this is to fix a
> > CONFIG_GENERIC_IOMAP issue (which it does because CONFIG_GENERIC_IOMAP
> > supplies pci_iounmap only if CONFIG_PCI is set), there should actually
> > have been a condition upon this.  This should make sure no other
> > architecture's !CONFIG_PCI compile breaks in the same way as parisc.
> > 
> > The fix had to be updated to take account of the GENERIC_PCI_IOMAP
> > separation.
> 
> So this means we end up still building the PA-RISC PCI code even if the config 
> says no PCI. That doesn't really sound consistent to me. I really would have 
> expected that we do not build any non-void PCI code then.

The first object is to fix the build breakage which likely affects more
than just parisc. Having a non-stub for the pci functions is just a few
spurious bytes in a kernel that's megabytes big (and it's what we were
doing previously).

James

Re: [PATCH] fix compile break caused by iomap: make IOPORT/PCI mapping functions conditional

[James Bottomley]

On Mon, 2012-01-30 at 21:10 +0100, Rolf Eike Beer wrote:
> Am Montag 30 Januar 2012, 10:40:47 schrieb James Bottomley:
> > The problem in
> > 
> > commit fea80311a939a746533a6d7e7c3183729d6a3faf
> > Author: Randy Dunlap <rdunlap@xenotime.net>
> > Date:   Sun Jul 24 11:39:14 2011 -0700
> > 
> >     iomap: make IOPORT/PCI mapping functions conditional
> > 
> > 
> > is that if your architecture supplies pci_iomap/pci_iounmap, it expects
> > always to supply them.  Adding empty body defitions in the !CONFIG_PCI
> > case, which is what this patch does, breaks the parisc compile because
> > the functions become doubly defined.  It took us a while to spot this,
> > because we don't actually build !CONFIG_PCI very often (only if someone
> > is brave enough to test the snake/asp machines).
> > 
> > Since the note in the commit log says this is to fix a
> > CONFIG_GENERIC_IOMAP issue (which it does because CONFIG_GENERIC_IOMAP
> > supplies pci_iounmap only if CONFIG_PCI is set), there should actually
> > have been a condition upon this.  This should make sure no other
> > architecture's !CONFIG_PCI compile breaks in the same way as parisc.
> > 
> > The fix had to be updated to take account of the GENERIC_PCI_IOMAP
> > separation.
> 
> So this means we end up still building the PA-RISC PCI code even if the config 
> says no PCI. That doesn't really sound consistent to me. I really would have 
> expected that we do not build any non-void PCI code then.

The first object is to fix the build breakage which likely affects more
than just parisc. Having a non-stub for the pci functions is just a few
spurious bytes in a kernel that's megabytes big (and it's what we were
doing previously).

James