Skip to contentSkip to conversation

tile: switch to generic pci_iomap

PATCH Superseded Michael S. Tsirkin 5 participants 23 messages Nov 29, 2011 – Dec 5, 2011

Subsystems (3)
PCI Library code Generic include/asm header files
lore
Share & tools
lore

Michael S. Tsirkin

author
#0 lore raw

cover tile: switch to generic pci_iomap

Here's a completely untested patch to switch tile
over to the new pci_iomap (in -mm).

This is on top of pci: use generic pci_iomap in most architectures, and converts just the tile architecture.

Seems straightforward enough but completely untested - could someone with access to appropriate systems respond and say whether the first patch in the series is OK?
The second one should then be trivial.

Signature · 1 line
1.7.5.53.gc233e
author
#1 lore raw

PATCH 1/2 tile: don't panic on iomap

I think panic on iomap is there just for debugging. If we return NULL instead, the generic pci_iomap will DTRT so we don't need to roll our own.

Signed-off-by Michael S. Tsirkin <[email protected]>
---
 arch/tile/include/asm/io.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
arch/tile/include/asm/io.h +2 -1
index c9ea165..be6090d 100644
@@ -204,7 +204,8 @@ static inline long ioport_panic(void)
 
 static inline void __iomem *ioport_map(unsigned long port, unsigned int len)
 {
-	return (void __iomem *) ioport_panic();
+	pr_info("Trying to map an IO resource - it does not exit on tile.\n");
+	return NULL;
 }
 
 static inline void ioport_unmap(void __iomem *addr)
Signature · 1 line
1.7.5.53.gc233e
maintainer
#2 lore raw

Re: [PATCH-RFC 1/2] tile: don't panic on iomap

Quoted 3 lines · Michael S. Tsirkin

I think panic on iomap is there just for debugging. If we return NULL instead, the generic pci_iomap will DTRT so we don't need to roll our own.

Just to be explicit about what "doing the right thing" means, here's what I think is changing (I think the new behavior is OK, but it *is* different):

Old behavior: Caller calls pci_iomap(), which panics in ioport_map().

New behavior: Caller calls pci_iomap(), ioport_map() returns NULL, pci_iomap() returns NULL (failure), caller may check for failure. If caller does not check for failure and passes the NULL to ioread()/iowrite(), we WARN in bad_io_access().

Quoted 5 lines

 static inline void __iomem *ioport_map(unsigned long port, unsigned int len) {
-       return (void __iomem *) ioport_panic();
+       pr_info("Trying to map an IO resource - it does not exit on tile.\n");
+       return NULL;

s/exit/exist/

Since we only expect to see this message during debugging, maybe it could be more informative, e.g., use dump_stack() to identify the offending driver? I don't think either the "Trying to map" message or the "Bad IO access" message is enough to actually make progress in debugging.

Bjorn

author
#3 lore raw

Re: [PATCH-RFC 1/2] tile: don't panic on iomap

Quoted 8 lines · Bjorn Helgaas
Quoted 3 lines · Michael S. Tsirkin

I think panic on iomap is there just for debugging. If we return NULL instead, the generic pci_iomap will DTRT so we don't need to roll our own.

Just to be explicit about what "doing the right thing" means, here's what I think is changing (I think the new behavior is OK, but it *is* different):

I think the change is that anyone calling ioport_map *directly* will fail. pci_iomap callers are mostly unaffected.

Quoted 2 lines

Old behavior: Caller calls pci_iomap(), which panics in ioport_map().

Not really, the old pci_iomap simply returned NULL in this case, it did not call ioport_map.

Quoted 13 lines

New behavior: Caller calls pci_iomap(), ioport_map() returns NULL, pci_iomap() returns NULL (failure), caller may check for failure. If caller does not check for failure and passes the NULL to ioread()/iowrite(), we WARN in bad_io_access().

Quoted 5 lines

 static inline void __iomem *ioport_map(unsigned long port, unsigned int len) {
-       return (void __iomem *) ioport_panic();
+       pr_info("Trying to map an IO resource - it does not exit on tile.\n");
+       return NULL;

s/exit/exist/

Quoted 7 lines

Since we only expect to see this message during debugging, maybe it could be more informative, e.g., use dump_stack() to identify the offending driver? I don't think either the "Trying to map" message or the "Bad IO access" message is enough to actually make progress in debugging.

Bjorn

As explained above, only direct callers of ioport_map get a changed behaviour. If we start dumping stack there we will hurt users of pci_iomap which used to get a graceful failure and will start getting scary messages. Is does not seem to be worth doing to simplify debugging, right? How about sticking the function name in the pr_info message?
A simple grep for ioport_map will then get you the culprit ...
Like this:
+       pr_info("ioport_map: mapping IO resources is unsupported on tile.\n"); ?

Signature · 1 line
MST
maintainer
#4 lore raw

Re: [PATCH-RFC 1/2] tile: don't panic on iomap

In reply to Michael S. Tsirkin

arch/tile/include/asm/io.h static inline long ioport_panic(void) · line 209

Quoted 12 lines · Michael S. Tsirkin
arch/tile/include/asm/io.h +2 -1
index c9ea165..be6090d 100644
@@ -204,7 +204,8 @@ static inline long ioport_panic(void)
 
 static inline void __iomem *ioport_map(unsigned long port, unsigned int len)
 {
-       return (void __iomem *) ioport_panic();
+       pr_info("Trying to map an IO resource - it does not exit on tile.\n");
+       return NULL;
 }

Why not just set CONFIG_NO_IOPORT and make this unavailable at compile time?

Arnd

author
#5 lore raw

Re: [PATCH-RFC 1/2] tile: don't panic on iomap

Quoted 16 lines · Arnd Bergmann

arch/tile/include/asm/io.h static inline long ioport_panic(void) · line 209

Quoted 12 lines · Michael S. Tsirkin
arch/tile/include/asm/io.h +2 -1
index c9ea165..be6090d 100644
@@ -204,7 +204,8 @@ static inline long ioport_panic(void)
 
 static inline void __iomem *ioport_map(unsigned long port, unsigned int len)
 {
-       return (void __iomem *) ioport_panic();
+       pr_info("Trying to map an IO resource - it does not exit on tile.\n");
+       return NULL;
 }

Why not just set CONFIG_NO_IOPORT and make this unavailable at compile time?

Quoted 1 line

Arnd

Well I think CONFIG_NO_IOPORT only has effect if you pull in asm-generic/io.h. This might work but I have no idea. Presumably whoever wrote that architecture considered and discarded setting GENERIC_IOMAP.

Signature · 1 line
MST
maintainer
#6 lore raw

Re: [PATCH-RFC 1/2] tile: don't panic on iomap

Quoted 18 lines · Michael S. Tsirkin
Quoted 16 lines · Arnd Bergmann

arch/tile/include/asm/io.h static inline long ioport_panic(void) · line 209

Quoted 12 lines · Michael S. Tsirkin
arch/tile/include/asm/io.h +2 -1
index c9ea165..be6090d 100644
@@ -204,7 +204,8 @@ static inline long ioport_panic(void)
 
 static inline void __iomem *ioport_map(unsigned long port, unsigned int len)
 {
-       return (void __iomem *) ioport_panic();
+       pr_info("Trying to map an IO resource - it does not exit on tile.\n");
+       return NULL;
 }

Why not just set CONFIG_NO_IOPORT and make this unavailable at compile time?

Quoted 4 lines
Quoted 1 line

Arnd

Well I think CONFIG_NO_IOPORT only has effect if you pull in asm-generic/io.h. This might work but I have no idea.

NO_IOPORT ha nothing to do with asm-generic, it just makes lib/iomap.c leave out the calls to ioport_map, and prevent all drivers that call ioport_map from being built.

Quoted 1 line

Presumably whoever wrote that architecture considered and discarded setting GENERIC_IOMAP.

The tile PCI controller is apparently special in that it does not map the PIO region into MMIO space, or make it available in any other way, so generic iomap with IOCOND is pointless. Setting NO_IPORT will still mean that drivers using ioport_map will not be built. This mostly impacts the legacy (non-AHCI) ATA controllers, which I assume are not used on tile anyway.

Arnd

author
#7 lore raw

Re: [PATCH-RFC 1/2] tile: don't panic on iomap

Quoted 19 lines · Arnd Bergmann
Quoted 18 lines · Michael S. Tsirkin
Quoted 16 lines · Arnd Bergmann

arch/tile/include/asm/io.h static inline long ioport_panic(void) · line 209

Quoted 12 lines · Michael S. Tsirkin
arch/tile/include/asm/io.h +2 -1
index c9ea165..be6090d 100644
@@ -204,7 +204,8 @@ static inline long ioport_panic(void)
 
 static inline void __iomem *ioport_map(unsigned long port, unsigned int len)
 {
-       return (void __iomem *) ioport_panic();
+       pr_info("Trying to map an IO resource - it does not exit on tile.\n");
+       return NULL;
 }

Why not just set CONFIG_NO_IOPORT and make this unavailable at compile time?

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

Arnd

Well I think CONFIG_NO_IOPORT only has effect if you pull in asm-generic/io.h. This might work but I have no idea.

NO_IOPORT ha nothing to do with asm-generic, it just makes lib/iomap.c leave out the calls to ioport_map, and prevent all drivers that call ioport_map from being built.

Yes. But lib/iomap.c isn't even linked unless GENERIC_IOMAP is set.

Quoted 9 lines
Quoted 1 line

Presumably whoever wrote that architecture considered and discarded setting GENERIC_IOMAP.

The tile PCI controller is apparently special in that it does not map the PIO region into MMIO space, or make it available in any other way, so generic iomap with IOCOND is pointless. Setting NO_IPORT will still mean that drivers using ioport_map will not be built. This mostly impacts the legacy (non-AHCI) ATA controllers, which I assume are not used on tile anyway.

Arnd

I don't object to setting NO_IPORT at all, but this won't help address the problem this patch is fixing, which is to use a generic pci_iomap on tile.

Signature · 1 line
MST
maintainer
#8 lore raw

Re: [PATCH-RFC 1/2] tile: don't panic on iomap

Quoted 11 lines · Michael S. Tsirkin
Quoted 7 lines
Quoted 1 line

Presumably whoever wrote that architecture considered and discarded setting GENERIC_IOMAP.

The tile PCI controller is apparently special in that it does not map the PIO region into MMIO space, or make it available in any other way, so generic iomap with IOCOND is pointless. Setting NO_IPORT will still mean that drivers using ioport_map will not be built. This mostly impacts the legacy (non-AHCI) ATA controllers, which I assume are not used on tile anyway.

I don't object to setting NO_IPORT at all, but this won't help address the problem this patch is fixing, which is to use a generic pci_iomap on tile.

Ah, right. I didn't realize that the generic pci_iomap still attempts to call ioport_map(). It would probably make sense to enclose the ioport_map() call in pci_iomap() inside of #ifdef CONFIG_HAS_IOPORT. It's not exactly beautiful, but probably the most correct solution so that we can make any call to ioport_map() a build-time error on architectures that set CONFIG_NO_IOPORT.

Arnd

author
#9 lore raw

Re: [PATCH-RFC 1/2] tile: don't panic on iomap

Quoted 13 lines · Arnd Bergmann
Quoted 11 lines · Michael S. Tsirkin
Quoted 7 lines
Quoted 1 line

Presumably whoever wrote that architecture considered and discarded setting GENERIC_IOMAP.

The tile PCI controller is apparently special in that it does not map the PIO region into MMIO space, or make it available in any other way, so generic iomap with IOCOND is pointless. Setting NO_IPORT will still mean that drivers using ioport_map will not be built. This mostly impacts the legacy (non-AHCI) ATA controllers, which I assume are not used on tile anyway.

I don't object to setting NO_IPORT at all, but this won't help address the problem this patch is fixing, which is to use a generic pci_iomap on tile.

Quoted 8 lines

Ah, right. I didn't realize that the generic pci_iomap still attempts to call ioport_map(). It would probably make sense to enclose the ioport_map() call in pci_iomap() inside of #ifdef CONFIG_HAS_IOPORT. It's not exactly beautiful, but probably the most correct solution so that we can make any call to ioport_map() a build-time error on architectures that set CONFIG_NO_IOPORT.

Arnd

I'm not sure why do you want to do that.

Signature · 1 line
MST
maintainer
#10 lore raw

Re: [PATCH-RFC 1/2] tile: don't panic on iomap

Quoted 11 lines · Michael S. Tsirkin
Quoted 6 lines · Arnd Bergmann

Ah, right. I didn't realize that the generic pci_iomap still attempts to call ioport_map(). It would probably make sense to enclose the ioport_map() call in pci_iomap() inside of #ifdef CONFIG_HAS_IOPORT. It's not exactly beautiful, but probably the most correct solution so that we can make any call to ioport_map() a build-time error on architectures that set CONFIG_NO_IOPORT.

I'm not sure why do you want to do that.

The problem is that any definition of ioport_map on architectures that can't do it is potentially harmful. Calling panic() is bad style as you pointed out, but simply returning NULL can also be harmful because it's likely that some drivers are written under the (false) assumption that ioport_map can never fail.

Getting a build-time error would be more helpful here IMHO.

Arnd

author
#11 lore raw

Re: [PATCH-RFC 1/2] tile: don't panic on iomap

Quoted 13 lines · Arnd Bergmann
Quoted 11 lines · Michael S. Tsirkin
Quoted 6 lines · Arnd Bergmann

Ah, right. I didn't realize that the generic pci_iomap still attempts to call ioport_map(). It would probably make sense to enclose the ioport_map() call in pci_iomap() inside of #ifdef CONFIG_HAS_IOPORT. It's not exactly beautiful, but probably the most correct solution so that we can make any call to ioport_map() a build-time error on architectures that set CONFIG_NO_IOPORT.

I'm not sure why do you want to do that.

Quoted 8 lines

The problem is that any definition of ioport_map on architectures that can't do it is potentially harmful. Calling panic() is bad style as you pointed out, but simply returning NULL can also be harmful because it's likely that some drivers are written under the (false) assumption that ioport_map can never fail. Getting a build-time error would be more helpful here IMHO.

Arnd

Yes but uglifying these users is also bad, ifdefs in code are incredibly fragile. Isn't it enough to declare ioport_map __must_check?

Signature · 1 line
MST
maintainer
#12 lore raw

Re: [PATCH-RFC 1/2] tile: don't panic on iomap

Quoted 21 lines · Michael S. Tsirkin
Quoted 13 lines · Arnd Bergmann
Quoted 11 lines · Michael S. Tsirkin
Quoted 6 lines · Arnd Bergmann

Ah, right. I didn't realize that the generic pci_iomap still attempts to call ioport_map(). It would probably make sense to enclose the ioport_map() call in pci_iomap() inside of #ifdef CONFIG_HAS_IOPORT. It's not exactly beautiful, but probably the most correct solution so that we can make any call to ioport_map() a build-time error on architectures that set CONFIG_NO_IOPORT.

I'm not sure why do you want to do that.

Quoted 6 lines

The problem is that any definition of ioport_map on architectures that can't do it is potentially harmful. Calling panic() is bad style as you pointed out, but simply returning NULL can also be harmful because it's likely that some drivers are written under the (false) assumption that ioport_map can never fail. Getting a build-time error would be more helpful here IMHO.

Quoted 2 lines

Yes but uglifying these users is also bad, ifdefs in code are incredibly fragile. Isn't it enough to declare ioport_map __must_check?

I guess we already wasted too many electrons over this triviality, I don't actually care all that much, and will probably have to touch the same code again when I get to submit my patch to make inb/outb optional for architectures. Just pick any solution (including your original one). I'll revisit this when I'm bothered by the presence of the ioport_map function and will keep you in the loop on those patches.

Arnd

#13 lore raw

PATCH 1/1 lib/devres.c: allow specifying NO_IOPORT while using PCI

In reply to Arnd Bergmann

The tile architecture does not support IOPORT, but it does support PCI and IOMEM. We were not specifying NO_IOPORT, though, because it was causing a couple of functions (pcim_iomap_regions and pcim_iomap_table) that are needed to not get provided. Moving the #ifdef fixes this so that those functions and some related ones are now always provided if CONFIG_PCI is true.

Signed-off-by Chris Metcalf <[email protected]>

---
This commit was inspired by the recent linux-arch discussion about what to do with ioport_map on architectures that can't do it, so I went back to figure out why I hadn't set NO_IOPORT in the first place. I'm willing to take this into the tile tree if the PCI folks are OK with it, or if someone else can pick it up, that's fine too.

 arch/tile/Kconfig |    2 +-
 lib/devres.c      |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
arch/tile/Kconfig +1 -1
index 70a0de4..e7b1e07 100644
@@ -333,7 +333,7 @@ config NO_IOMEM
 	def_bool !PCI
 
 config NO_IOPORT
-	def_bool !PCI
+	def_bool y
 
 source "drivers/pci/Kconfig"
lib/devres.c +1 -1
index 7c0e953..5f36cfd 100644
@@ -144,6 +144,7 @@ void devm_ioport_unmap(struct device *dev, void __iomem *addr)
 			       devm_ioport_map_match, (void *)addr));
 }
 EXPORT_SYMBOL(devm_ioport_unmap);
+#endif
 
 #ifdef CONFIG_PCI
 /*

@@ -349,4 +350,3 @@ void pcim_iounmap_regions(struct pci_dev *pdev, u16 mask)
 }
 EXPORT_SYMBOL(pcim_iounmap_regions);
 #endif
-#endif
Signature · 1 line
1.6.5.2
Acked-by on v1 1/1
#14 lore raw

Re: [PATCH] lib/devres.c: allow specifying NO_IOPORT while using PCI

Hello,

Quoted 8 lines · Chris Metcalf

The tile architecture does not support IOPORT, but it does support PCI and IOMEM. We were not specifying NO_IOPORT, though, because it was causing a couple of functions (pcim_iomap_regions and pcim_iomap_table) that are needed to not get provided. Moving the #ifdef fixes this so that those functions and some related ones are now always provided if CONFIG_PCI is true.

Signed-off-by Chris Metcalf <[email protected]>
Acked-by Tejun Heo <[email protected]>

Please feel free to route the change through tile. If not, Andrew, can you please pick this up?

Thanks.

Signature · 1 line
tejun
#15 lore raw

Re: [PATCH] lib/devres.c: allow specifying NO_IOPORT while using PCI

Quoted 10 lines · Tejun Heo

Hello,

Quoted 7 lines · Chris Metcalf

The tile architecture does not support IOPORT, but it does support PCI and IOMEM. We were not specifying NO_IOPORT, though, because it was causing a couple of functions (pcim_iomap_regions and pcim_iomap_table) that are needed to not get provided. Moving the #ifdef fixes this so that those functions and some related ones are now always provided if CONFIG_PCI is true.

Quoted 5 lines
Quoted 1 line
Signed-off-by Chris Metcalf <[email protected]>
Acked-by Tejun Heo <[email protected]>

Please feel free to route the change through tile. If not, Andrew, can you please pick this up?

Thanks. I'll plan to push this to Linus through the tile tree with your Acked-by.

Signature · 2 lines
Chris Metcalf, Tilera Corp.
http://www.tilera.com
maintainer
#16 lore raw

Re: [PATCH] lib/devres.c: allow specifying NO_IOPORT while using PCI

In reply to Chris Metcalf
Quoted 8 lines · On Thursday 01 December 2011 12:59:19 Chris Metcalf wrote:

The tile architecture does not support IOPORT, but it does support PCI and IOMEM. We were not specifying NO_IOPORT, though, because it was causing a couple of functions (pcim_iomap_regions and pcim_iomap_table) that are needed to not get provided. Moving the #ifdef fixes this so that those functions and some related ones are now always provided if CONFIG_PCI is true.

Signed-off-by Chris Metcalf <[email protected]>

Hmm, I'm sure that Al Viro intentionally did it the way it is, but I'm not sure exactly why that would be.

Any driver using the functions you now expose for NO_IOPORT is probably prepared to handle both MMIO and PIO BARs on PCI, which you can consider broken in the case where a card actually requires PIO.

On tile, it would be just as broken as other drivers that directly use inb/outb to do the port access, but on platforms that actually have PIO but are not able to map them (like arm/mach-rpc), you would now enable people to write silently broken drivers.

However, assuming they check the return value of the *iomap, the drivers could still easily detect this case at runtime. Also, at least on tile, you already provide a pci_iomap function, which has the exact same problem as the devres functions.

One area where your patch might actually break existing code is architectures that allow the devres functions to be built *and* have PCI support but don't provide a pci_iomap function, because they do not support mapping the PIO range into addresses. I'm not sure if that's actually still the case anywhere after Michael's recent patches, but it used to be the case on RPC and probably others.

Arnd

#17 lore raw

Re: [PATCH] lib/devres.c: allow specifying NO_IOPORT while using PCI

Quoted 26 lines · Arnd Bergmann
Quoted 8 lines · On Thursday 01 December 2011 12:59:19 Chris Metcalf wrote:

The tile architecture does not support IOPORT, but it does support PCI and IOMEM. We were not specifying NO_IOPORT, though, because it was causing a couple of functions (pcim_iomap_regions and pcim_iomap_table) that are needed to not get provided. Moving the #ifdef fixes this so that those functions and some related ones are now always provided if CONFIG_PCI is true.

Signed-off-by Chris Metcalf <[email protected]>

Hmm, I'm sure that Al Viro intentionally did it the way it is, but I'm not sure exactly why that would be.

Any driver using the functions you now expose for NO_IOPORT is probably prepared to handle both MMIO and PIO BARs on PCI, which you can consider broken in the case where a card actually requires PIO.

On tile, it would be just as broken as other drivers that directly use inb/outb to do the port access, but on platforms that actually have PIO but are not able to map them (like arm/mach-rpc), you would now enable people to write silently broken drivers.

However, assuming they check the return value of the *iomap, the drivers could still easily detect this case at runtime. Also, at least on tile, you already provide a pci_iomap function, which has the exact same problem as the devres functions.

Quoted 6 lines

One area where your patch might actually break existing code is architectures that allow the devres functions to be built *and* have PCI support but don't provide a pci_iomap function, because they do not support mapping the PIO range into addresses. I'm not sure if that's actually still the case anywhere after Michael's recent patches, but it used to be the case on RPC and probably others.

Sounds sufficiently subtle that I'll plan to drop the patch unless someone else (maybe Al Viro, now cc'ed) wants to weigh in. For tile this just means we'll continue not to set NO_IOPORT, at the cost of a few hundred bytes of dead code; no big deal.

The specific issue I saw was calls to pcim_iomap_{regions,table} from drivers/ata/libata-sff.c and drivers/ata/sata_sil24.c; we use the sata_sil24 in one of our platforms (TILEmpower).

Signature · 2 lines
Chris Metcalf, Tilera Corp.
http://www.tilera.com
maintainer
#18 lore raw

Re: [PATCH] lib/devres.c: allow specifying NO_IOPORT while using PCI

Quoted 3 lines · Chris Metcalf

The specific issue I saw was calls to pcim_iomap_{regions,table} from drivers/ata/libata-sff.c and drivers/ata/sata_sil24.c; we use the sata_sil24 in one of our platforms (TILEmpower).

Hmm, so sata_sil24 does not actually use PIO, so we can probably find a way to make that work. Why do you have to build libata-sff? Shouldn't that only be needed if you actually have an sff-type controller?

Arnd

#19 lore raw

Re: [PATCH] lib/devres.c: allow specifying NO_IOPORT while using PCI

Quoted 6 lines · Arnd Bergmann
Quoted 3 lines · Chris Metcalf

The specific issue I saw was calls to pcim_iomap_{regions,table} from drivers/ata/libata-sff.c and drivers/ata/sata_sil24.c; we use the sata_sil24 in one of our platforms (TILEmpower).

Hmm, so sata_sil24 does not actually use PIO, so we can probably find a way to make that work.

OK, that would be good. I'm not familiar enough with the driver (or indeed the devres stuff) to really take an effective swing at this myself, though.

Quoted 2 lines

Why do you have to build libata-sff? Shouldn't
that only be needed if you actually have an sff-type controller?

Yes, I think this is an accidental legacy of the original development work for PCI root complex three years ago. I disabled ATA_SFF and booted up a number of our inhouse platforms, and all the devices appear to work fine without it. I've now turned it off in our standard configuration.

Signature · 2 lines
Chris Metcalf, Tilera Corp.
http://www.tilera.com
author
#20 lore raw

PATCH v2 1/2 tile: don't panic on iomap

In reply to Michael S. Tsirkin

I think panic on iomap is there just for debugging. If we return NULL instead, the generic pci_iomap will do the same thing as the custom one that tile currently has (that is, return NULL on an IO BAR)
so tile won't need to roll its own anymore.

Signed-off-by Michael S. Tsirkin <[email protected]>

---

Note: the other patch in the series is unchanged. Changes from v1:
- tweaked pr_info message

 arch/tile/include/asm/io.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
arch/tile/include/asm/io.h +2 -1
index c9ea165..d2152de 100644
@@ -204,7 +204,8 @@ static inline long ioport_panic(void)
 
 static inline void __iomem *ioport_map(unsigned long port, unsigned int len)
 {
-	return (void __iomem *) ioport_panic();
+	pr_info("ioport_map: mapping IO resources is unsupported on tile.\n");
+	return NULL;
 }
 
 static inline void ioport_unmap(void __iomem *addr)
Signature · 1 line
1.7.5.53.gc233e
author
#21 lore raw

PATCH 2/2 tile: switch to GENERIC_PCI_IOMAP

In reply to Michael S. Tsirkin

tile now has working stubs for ioport_map and ioremap such that the generic pci_iomap will DTRT: cast to pointer on memory and return NULL and log message on IO map.

Switch it over to GENERIC_PCI_IOMAP.

Signed-off-by Michael S. Tsirkin <[email protected]>
---
 arch/tile/Kconfig           |    1 +
 arch/tile/include/asm/pci.h |    2 +-
 arch/tile/kernel/pci.c      |   21 ---------------------
 3 files changed, 2 insertions(+), 22 deletions(-)
arch/tile/Kconfig +1 -0
index 70a0de4..11270ca 100644
@@ -321,6 +321,7 @@ config PCI
 	bool "PCI support"
 	default y
 	select PCI_DOMAINS
+	select GENERIC_PCI_IOMAP
 	---help---
 	  Enable PCI root complex support, so PCIe endpoint devices can
 	  be attached to the Tile chip.  Many, but not all, PCI devices
arch/tile/include/asm/pci.h +1 -1
index 7f03cef..1d25fea 100644
@@ -16,6 +16,7 @@
 #define _ASM_TILE_PCI_H
 
 #include <linux/pci.h>
+#include <asm-generic/pci_iomap.h>
 
 /*
  * Structure of a PCI controller (host bridge)

@@ -49,7 +50,6 @@ struct pci_controller {
 int __devinit tile_pci_init(void);
 int __devinit pcibios_init(void);
 
-void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long max);
 static inline void pci_iounmap(struct pci_dev *dev, void __iomem *addr) {}
 
 void __devinit pcibios_fixup_bus(struct pci_bus *bus);
arch/tile/kernel/pci.c +0 -21
index 2a8014c..1b6244e 100644
@@ -465,27 +465,6 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
 	return 0;
 }
 
-void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long max)
-{
-	unsigned long start = pci_resource_start(dev, bar);
-	unsigned long len = pci_resource_len(dev, bar);
-	unsigned long flags = pci_resource_flags(dev, bar);
-
-	if (!len)
-		return NULL;
-	if (max && len > max)
-		len = max;
-
-	if (!(flags & IORESOURCE_MEM)) {
-		pr_info("PCI: Trying to map invalid resource %#lx\n", flags);
-		start = 0;
-	}
-
-	return (void __iomem *)start;
-}
-EXPORT_SYMBOL(pci_iomap);
-
-
 /****************************************************************
  *
  * Tile PCI config space read/write routines
Signature · 1 line
1.7.5.53.gc233e
Acked-by on v1 2/2
#22 lore raw

Re: [PATCH-RFC 2/2] tile: switch to GENERIC_PCI_IOMAP

Quoted 4 lines · Michael S. Tsirkin

tile now has working stubs for ioport_map and ioremap such that the generic pci_iomap will DTRT: cast to pointer on memory and return NULL and log message on IO map.

Quoted 8 lines

Switch it over to GENERIC_PCI_IOMAP.

Signed-off-by Michael S. Tsirkin <[email protected]>
---
 arch/tile/Kconfig           |    1 +
 arch/tile/include/asm/pci.h |    2 +-
 arch/tile/kernel/pci.c      |   21 ---------------------
 3 files changed, 2 insertions(+), 22 deletions(-)
Acked-by Chris Metcalf <[email protected]>
Signature · 2 lines
Chris Metcalf, Tilera Corp.
http://www.tilera.com