Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
Hi Rob, Nice work! On Fri, Jun 27, 2014 at 05:15:53PM +0100, Rob Herring wrote: > On 06/27/2014 07:49 AM, Will Deacon wrote: > > On Fri, Jun 27, 2014 at 12:03:34PM +0100, Arnd Bergmann wrote: > >> On Thursday 26 June 2014 19:44:21 Rob Herring wrote: > >>> I don't agree arm32 is harder than microblaze. Yes, converting ALL of > >>> arm would be, but that is not necessary. With Liviu's latest branch > >>> the hacks I previously needed are gone (thanks!), and this is all I > >>> need to get Versatile PCI working (under QEMU): > >> > >> I meant converting all of arm32 would be harder, but I agree we don't > >> have to do it. It would be nice to convert all of the drivers/pci/host > >> drivers though, iow all multiplatform-enabled ones, and leaving the > >> current arm32 pci implementation for the platforms we don't want to > >> convert to multiplatform anyway (footbridge, iop, ixp4xx, ks8695 (?), > >> pxa, sa1100). > > > > I'm more than happy to convert the generic host controller we merged > > recently, but I'd probably want the core changes merged first so that I > > know I'm not wasting my time! > > Something like this untested patch... > > Another issue I found still present is pci_ioremap_io needs some work > to unify with the arm implementation. That's a matter of changing the > function from an offset to i/o resource. That should be a pretty > mechanical change. > > Also, there is a potential for memory leak because there is no undo > for of_create_pci_host_bridge. Once Liviu reposts his series, I'll take a look at of_create_pci_host_bridge, as I'm not sure that it provides all the behaviour that we get from gen_pci_parse_request_of_pci_ranges right now (e.g. required a non-prefetchable mem resource). Will -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
Hi Rob, Nice work! On Fri, Jun 27, 2014 at 05:15:53PM +0100, Rob Herring wrote: On 06/27/2014 07:49 AM, Will Deacon wrote: On Fri, Jun 27, 2014 at 12:03:34PM +0100, Arnd Bergmann wrote: On Thursday 26 June 2014 19:44:21 Rob Herring wrote: I don't agree arm32 is harder than microblaze. Yes, converting ALL of arm would be, but that is not necessary. With Liviu's latest branch the hacks I previously needed are gone (thanks!), and this is all I need to get Versatile PCI working (under QEMU): I meant converting all of arm32 would be harder, but I agree we don't have to do it. It would be nice to convert all of the drivers/pci/host drivers though, iow all multiplatform-enabled ones, and leaving the current arm32 pci implementation for the platforms we don't want to convert to multiplatform anyway (footbridge, iop, ixp4xx, ks8695 (?), pxa, sa1100). I'm more than happy to convert the generic host controller we merged recently, but I'd probably want the core changes merged first so that I know I'm not wasting my time! Something like this untested patch... Another issue I found still present is pci_ioremap_io needs some work to unify with the arm implementation. That's a matter of changing the function from an offset to i/o resource. That should be a pretty mechanical change. Also, there is a potential for memory leak because there is no undo for of_create_pci_host_bridge. Once Liviu reposts his series, I'll take a look at of_create_pci_host_bridge, as I'm not sure that it provides all the behaviour that we get from gen_pci_parse_request_of_pci_ranges right now (e.g. required a non-prefetchable mem resource). Will -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On 06/27/2014 07:49 AM, Will Deacon wrote:
> On Fri, Jun 27, 2014 at 12:03:34PM +0100, Arnd Bergmann wrote:
>> On Thursday 26 June 2014 19:44:21 Rob Herring wrote:
>>> I don't agree arm32 is harder than microblaze. Yes, converting ALL of
>>> arm would be, but that is not necessary. With Liviu's latest branch
>>> the hacks I previously needed are gone (thanks!), and this is all I
>>> need to get Versatile PCI working (under QEMU):
>>
>> I meant converting all of arm32 would be harder, but I agree we don't
>> have to do it. It would be nice to convert all of the drivers/pci/host
>> drivers though, iow all multiplatform-enabled ones, and leaving the
>> current arm32 pci implementation for the platforms we don't want to
>> convert to multiplatform anyway (footbridge, iop, ixp4xx, ks8695 (?),
>> pxa, sa1100).
>
> I'm more than happy to convert the generic host controller we merged
> recently, but I'd probably want the core changes merged first so that I
> know I'm not wasting my time!
Something like this untested patch...
Another issue I found still present is pci_ioremap_io needs some work
to unify with the arm implementation. That's a matter of changing the
function from an offset to i/o resource. That should be a pretty
mechanical change.
Also, there is a potential for memory leak because there is no undo
for of_create_pci_host_bridge.
Rob
8<--
>From 301b402631b2867ced38d5533586da0bd888045c Mon Sep 17 00:00:00 2001
From: Rob Herring
Date: Fri, 27 Jun 2014 09:46:09 -0500
Subject: [PATCH] pci: generic-host: refactor to use common range parsing
Signed-off-by: Rob Herring
---
drivers/pci/host/pci-host-generic.c | 191 ++--
1 file changed, 29 insertions(+), 162 deletions(-)
diff --git a/drivers/pci/host/pci-host-generic.c
b/drivers/pci/host/pci-host-generic.c
index 44fe6aa..57fd1e1 100644
--- a/drivers/pci/host/pci-host-generic.c
+++ b/drivers/pci/host/pci-host-generic.c
@@ -32,16 +32,14 @@ struct gen_pci_cfg_bus_ops {
struct gen_pci_cfg_windows {
struct resource res;
- struct resource bus_range;
void __iomem**win;
const struct gen_pci_cfg_bus_ops*ops;
};
struct gen_pci {
- struct pci_host_bridge host;
+ struct pci_host_bridge *host;
struct gen_pci_cfg_windows cfg;
- struct list_headresources;
};
static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus,
@@ -50,7 +48,7 @@ static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus
*bus,
{
struct pci_sys_data *sys = bus->sysdata;
struct gen_pci *pci = sys->private_data;
- resource_size_t idx = bus->number - pci->cfg.bus_range.start;
+ resource_size_t idx = bus->number - bus->busn_res.start;
return pci->cfg.win[idx] + ((devfn << 8) | where);
}
@@ -66,7 +64,7 @@ static void __iomem *gen_pci_map_cfg_bus_ecam(struct pci_bus
*bus,
{
struct pci_sys_data *sys = bus->sysdata;
struct gen_pci *pci = sys->private_data;
- resource_size_t idx = bus->number - pci->cfg.bus_range.start;
+ resource_size_t idx = bus->number - bus->busn_res.start;
return pci->cfg.win[idx] + ((devfn << 12) | where);
}
@@ -138,154 +136,32 @@ static const struct of_device_id gen_pci_of_match[] = {
};
MODULE_DEVICE_TABLE(of, gen_pci_of_match);
-static int gen_pci_calc_io_offset(struct device *dev,
- struct of_pci_range *range,
- struct resource *res,
- resource_size_t *offset)
-{
- static atomic_t wins = ATOMIC_INIT(0);
- int err, idx, max_win;
- unsigned int window;
-
- if (!PAGE_ALIGNED(range->cpu_addr))
- return -EINVAL;
-
- max_win = (IO_SPACE_LIMIT + 1) / SZ_64K;
- idx = atomic_inc_return();
- if (idx > max_win)
- return -ENOSPC;
-
- window = (idx - 1) * SZ_64K;
- err = pci_ioremap_io(window, range->cpu_addr);
- if (err)
- return err;
-
- of_pci_range_to_resource(range, dev->of_node, res);
- res->start = window;
- res->end = res->start + range->size - 1;
- *offset = window - range->pci_addr;
- return 0;
-}
-
-static int gen_pci_calc_mem_offset(struct device *dev,
- struct of_pci_range *range,
- struct resource *res,
- resource_size_t *offset)
-{
- of_pci_range_to_resource(range, dev->of_node, res);
- *offset = range->cpu_addr - range->pci_addr;
- return 0;
-}
-
-static void gen_pci_release_of_pci_ranges(struct gen_pci *pci)
-{
- struct pci_host_bridge_window *win;
-
- list_for_each_entry(win, >resources, list)
-
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Fri, Jun 27, 2014 at 03:55:04PM +0100, Bjorn Helgaas wrote: > On Fri, Jun 27, 2014 at 8:14 AM, Catalin Marinas > wrote: > > ... > > With Liviu's latest version (not posted) and with > > of_create_pci_host_bridge() function moved to of_pci.c, I don't think > > there is much new functionality added to drivers/pci/. What I think we > > need is clarifying the domain_nr patch (and API change) and more users > > of the new generic code. As you said, it doesn't need to be a separate > > architecture but rather existing pci host drivers under drivers/pci. Of > > course, other arch conversion should follow shortly as well but even > > without an immediate conversion, I don't see too much additional > > maintenance burden for the core PCIe code (and code sharing between new > > PCIe host drivers is even more beneficial). > > Sorry, I haven't had time to follow this. It sounds like there are > several pieces we could get out of the way easily. How about posting > the actual patches again? Maybe re-order them so the easy pieces are > first so they can get applied even if there are issues with later > ones? OK, I will post a new series on Monday. Thanks, Liviu > > Bjorn > -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/¯ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Fri, Jun 27, 2014 at 8:14 AM, Catalin Marinas wrote: > ... > With Liviu's latest version (not posted) and with > of_create_pci_host_bridge() function moved to of_pci.c, I don't think > there is much new functionality added to drivers/pci/. What I think we > need is clarifying the domain_nr patch (and API change) and more users > of the new generic code. As you said, it doesn't need to be a separate > architecture but rather existing pci host drivers under drivers/pci. Of > course, other arch conversion should follow shortly as well but even > without an immediate conversion, I don't see too much additional > maintenance burden for the core PCIe code (and code sharing between new > PCIe host drivers is even more beneficial). Sorry, I haven't had time to follow this. It sounds like there are several pieces we could get out of the way easily. How about posting the actual patches again? Maybe re-order them so the easy pieces are first so they can get applied even if there are issues with later ones? Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Fri, Jun 27, 2014 at 01:44:21AM +0100, Rob Herring wrote: > On Thu, Jun 26, 2014 at 3:59 AM, Catalin Marinas > wrote: > > Although a bit late, I'm raising this now and hopefully we'll come to a > > conclusion soon. Delaying arm64 PCIe support even further is not a real > > option, which leaves us with: > > > > 1. Someone else (with enough PCIe knowledge) volunteering to take over > >soon or > > 2. Dropping Liviu's work and going for an arm64-specific implementation > >(most likely based on the arm32 implementation, see below) > > 3. Keeping Liviu's patches leaving some of the architecture specific > bits. I know Arnd and I both commented on it still needing more common > pieces, but compared to option 2 that would be way better. > > Let's look at the patches in question: > > 3e71867 pci: Introduce pci_register_io_range() helper function. > 6681dff pci: OF: Fix the conversion of IO ranges into IO resources. > > Both OF patches. I'll happily merge them. We just need to make sure they don't break other users of of_pci_range_to_resource() that the second patch introduces. > 2d5dd85 pci: Create pci_host_bridge before its associated bus in > pci_create_root_bus. > f6f2854 pci: Introduce a domain number for pci_host_bridge. > 524a9f5 pci: Export find_pci_host_bridge() function. > > These don't seem to be too controversial. I think here there were discussions around introducing domain_nr to pci_host_bridge, particularly to the pci_create_root_bus_in_domain() API change. I don't think we reached any conclusion. > fb75718 pci: of: Parse and map the IRQ when adding the PCI device. > > 6 LOC. Hardly controversial. I agree. > 920a685 pci: Add support for creating a generic host_bridge from device tree > > This function could be moved to drivers/of/of_pci.c if having it in > drivers/pci is too much maintenance burden. I think it makes sense. Currently drivers/pci/host-bridge.c doesn't have anything OF related, so of_pci.c looks more appropriate. > However, nearly the same > code is already being duplicated in every DT enabled ARM PCI host > driver and will continue as more PCI hosts are added. So this isn't > really a question of converting other architectures to common PCI host > infrastructure, but converting DT based PCI hosts to common > infrastructure. ARM is the only arch moving host drivers to > drivers/pci ATM. Until other architectures start doing that, > converting them is pointless. I agree. It's probably more important to convert some of the drivers/pci/host implementations to using the common parsing rather than a new architecture (this way we avoid even more code duplication). > bcf5c10 Fix ioport_map() for !CONFIG_GENERIC_IOMAP cases. > > Seems like an independent fix that should be applied regardless. Indeed. But it got stuck at the top of this series and hasn't been pushed upstream. > 7cfde80 arm64: Add architecture support for PCI > > What is here is really just a function of which option we pick. With Liviu's latest version (not posted) and with of_create_pci_host_bridge() function moved to of_pci.c, I don't think there is much new functionality added to drivers/pci/. What I think we need is clarifying the domain_nr patch (and API change) and more users of the new generic code. As you said, it doesn't need to be a separate architecture but rather existing pci host drivers under drivers/pci. Of course, other arch conversion should follow shortly as well but even without an immediate conversion, I don't see too much additional maintenance burden for the core PCIe code (and code sharing between new PCIe host drivers is even more beneficial). -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Fri, Jun 27, 2014 at 02:16:28PM +0100, Arnd Bergmann wrote: > On Friday 27 June 2014 13:49:49 Will Deacon wrote: > > On Fri, Jun 27, 2014 at 12:03:34PM +0100, Arnd Bergmann wrote: > > > On Thursday 26 June 2014 19:44:21 Rob Herring wrote: > > > > I don't agree arm32 is harder than microblaze. Yes, converting ALL of > > > > arm would be, but that is not necessary. With Liviu's latest branch > > > > the hacks I previously needed are gone (thanks!), and this is all I > > > > need to get Versatile PCI working (under QEMU): > > > > > > I meant converting all of arm32 would be harder, but I agree we don't > > > have to do it. It would be nice to convert all of the drivers/pci/host > > > drivers though, iow all multiplatform-enabled ones, and leaving the > > > current arm32 pci implementation for the platforms we don't want to > > > convert to multiplatform anyway (footbridge, iop, ixp4xx, ks8695 (?), > > > pxa, sa1100). > > > > I'm more than happy to convert the generic host controller we merged > > recently, but I'd probably want the core changes merged first so that I > > know I'm not wasting my time! > > That is definitely fine with me, but it's Bjorn's decision. Or the changes to the generic host controller can be part of Liviu's patch series (maybe together with other PCI host drivers like designware, as time allows). -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Friday 27 June 2014 13:49:49 Will Deacon wrote: > On Fri, Jun 27, 2014 at 12:03:34PM +0100, Arnd Bergmann wrote: > > On Thursday 26 June 2014 19:44:21 Rob Herring wrote: > > > I don't agree arm32 is harder than microblaze. Yes, converting ALL of > > > arm would be, but that is not necessary. With Liviu's latest branch > > > the hacks I previously needed are gone (thanks!), and this is all I > > > need to get Versatile PCI working (under QEMU): > > > > I meant converting all of arm32 would be harder, but I agree we don't > > have to do it. It would be nice to convert all of the drivers/pci/host > > drivers though, iow all multiplatform-enabled ones, and leaving the > > current arm32 pci implementation for the platforms we don't want to > > convert to multiplatform anyway (footbridge, iop, ixp4xx, ks8695 (?), > > pxa, sa1100). > > I'm more than happy to convert the generic host controller we merged > recently, but I'd probably want the core changes merged first so that I > know I'm not wasting my time! That is definitely fine with me, but it's Bjorn's decision. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Fri, Jun 27, 2014 at 12:03:34PM +0100, Arnd Bergmann wrote: > On Thursday 26 June 2014 19:44:21 Rob Herring wrote: > > I don't agree arm32 is harder than microblaze. Yes, converting ALL of > > arm would be, but that is not necessary. With Liviu's latest branch > > the hacks I previously needed are gone (thanks!), and this is all I > > need to get Versatile PCI working (under QEMU): > > I meant converting all of arm32 would be harder, but I agree we don't > have to do it. It would be nice to convert all of the drivers/pci/host > drivers though, iow all multiplatform-enabled ones, and leaving the > current arm32 pci implementation for the platforms we don't want to > convert to multiplatform anyway (footbridge, iop, ixp4xx, ks8695 (?), > pxa, sa1100). I'm more than happy to convert the generic host controller we merged recently, but I'd probably want the core changes merged first so that I know I'm not wasting my time! Will -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Thursday 26 June 2014 19:44:21 Rob Herring wrote: > > Although a bit late, I'm raising this now and hopefully we'll come to a > > conclusion soon. Delaying arm64 PCIe support even further is not a real > > option, which leaves us with: > > > > 1. Someone else (with enough PCIe knowledge) volunteering to take over > >soon or > > Well, I might have 2 months ago, but now I'm pretty booked up. > > > 2. Dropping Liviu's work and going for an arm64-specific implementation > >(most likely based on the arm32 implementation, see below) > > 3. Keeping Liviu's patches leaving some of the architecture specific > bits. I know Arnd and I both commented on it still needing more common > pieces, but compared to option 2 that would be way better. Agreed. > 920a685 pci: Add support for creating a generic host_bridge from device tree > > This function could be moved to drivers/of/of_pci.c if having it in > drivers/pci is too much maintenance burden. However, nearly the same > code is already being duplicated in every DT enabled ARM PCI host > driver and will continue as more PCI hosts are added. So this isn't > really a question of converting other architectures to common PCI host > infrastructure, but converting DT based PCI hosts to common > infrastructure. ARM is the only arch moving host drivers to > drivers/pci ATM. Until other architectures start doing that, > converting them is pointless. This is the most important part in my mind. Every implementation gets this code wrong at least initially, and we have to provide some generic helper to get the broken code out of host drivers. I don't care whether that helper is part of the PCI core or part of the OF core. > 7cfde80 arm64: Add architecture support for PCI > > What is here is really just a function of which option we pick. > > > > First option is ideal but there is work to do as laid out by Arnd here: > > > > http://article.gmane.org/gmane.linux.kernel/1679304 > > I don't agree arm32 is harder than microblaze. Yes, converting ALL of > arm would be, but that is not necessary. With Liviu's latest branch > the hacks I previously needed are gone (thanks!), and this is all I > need to get Versatile PCI working (under QEMU): I meant converting all of arm32 would be harder, but I agree we don't have to do it. It would be nice to convert all of the drivers/pci/host drivers though, iow all multiplatform-enabled ones, and leaving the current arm32 pci implementation for the platforms we don't want to convert to multiplatform anyway (footbridge, iop, ixp4xx, ks8695 (?), pxa, sa1100). Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Thursday 26 June 2014 19:44:21 Rob Herring wrote: Although a bit late, I'm raising this now and hopefully we'll come to a conclusion soon. Delaying arm64 PCIe support even further is not a real option, which leaves us with: 1. Someone else (with enough PCIe knowledge) volunteering to take over soon or Well, I might have 2 months ago, but now I'm pretty booked up. 2. Dropping Liviu's work and going for an arm64-specific implementation (most likely based on the arm32 implementation, see below) 3. Keeping Liviu's patches leaving some of the architecture specific bits. I know Arnd and I both commented on it still needing more common pieces, but compared to option 2 that would be way better. Agreed. 920a685 pci: Add support for creating a generic host_bridge from device tree This function could be moved to drivers/of/of_pci.c if having it in drivers/pci is too much maintenance burden. However, nearly the same code is already being duplicated in every DT enabled ARM PCI host driver and will continue as more PCI hosts are added. So this isn't really a question of converting other architectures to common PCI host infrastructure, but converting DT based PCI hosts to common infrastructure. ARM is the only arch moving host drivers to drivers/pci ATM. Until other architectures start doing that, converting them is pointless. This is the most important part in my mind. Every implementation gets this code wrong at least initially, and we have to provide some generic helper to get the broken code out of host drivers. I don't care whether that helper is part of the PCI core or part of the OF core. 7cfde80 arm64: Add architecture support for PCI What is here is really just a function of which option we pick. First option is ideal but there is work to do as laid out by Arnd here: http://article.gmane.org/gmane.linux.kernel/1679304 I don't agree arm32 is harder than microblaze. Yes, converting ALL of arm would be, but that is not necessary. With Liviu's latest branch the hacks I previously needed are gone (thanks!), and this is all I need to get Versatile PCI working (under QEMU): I meant converting all of arm32 would be harder, but I agree we don't have to do it. It would be nice to convert all of the drivers/pci/host drivers though, iow all multiplatform-enabled ones, and leaving the current arm32 pci implementation for the platforms we don't want to convert to multiplatform anyway (footbridge, iop, ixp4xx, ks8695 (?), pxa, sa1100). Arnd -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Fri, Jun 27, 2014 at 12:03:34PM +0100, Arnd Bergmann wrote: On Thursday 26 June 2014 19:44:21 Rob Herring wrote: I don't agree arm32 is harder than microblaze. Yes, converting ALL of arm would be, but that is not necessary. With Liviu's latest branch the hacks I previously needed are gone (thanks!), and this is all I need to get Versatile PCI working (under QEMU): I meant converting all of arm32 would be harder, but I agree we don't have to do it. It would be nice to convert all of the drivers/pci/host drivers though, iow all multiplatform-enabled ones, and leaving the current arm32 pci implementation for the platforms we don't want to convert to multiplatform anyway (footbridge, iop, ixp4xx, ks8695 (?), pxa, sa1100). I'm more than happy to convert the generic host controller we merged recently, but I'd probably want the core changes merged first so that I know I'm not wasting my time! Will -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Friday 27 June 2014 13:49:49 Will Deacon wrote: On Fri, Jun 27, 2014 at 12:03:34PM +0100, Arnd Bergmann wrote: On Thursday 26 June 2014 19:44:21 Rob Herring wrote: I don't agree arm32 is harder than microblaze. Yes, converting ALL of arm would be, but that is not necessary. With Liviu's latest branch the hacks I previously needed are gone (thanks!), and this is all I need to get Versatile PCI working (under QEMU): I meant converting all of arm32 would be harder, but I agree we don't have to do it. It would be nice to convert all of the drivers/pci/host drivers though, iow all multiplatform-enabled ones, and leaving the current arm32 pci implementation for the platforms we don't want to convert to multiplatform anyway (footbridge, iop, ixp4xx, ks8695 (?), pxa, sa1100). I'm more than happy to convert the generic host controller we merged recently, but I'd probably want the core changes merged first so that I know I'm not wasting my time! That is definitely fine with me, but it's Bjorn's decision. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Fri, Jun 27, 2014 at 02:16:28PM +0100, Arnd Bergmann wrote: On Friday 27 June 2014 13:49:49 Will Deacon wrote: On Fri, Jun 27, 2014 at 12:03:34PM +0100, Arnd Bergmann wrote: On Thursday 26 June 2014 19:44:21 Rob Herring wrote: I don't agree arm32 is harder than microblaze. Yes, converting ALL of arm would be, but that is not necessary. With Liviu's latest branch the hacks I previously needed are gone (thanks!), and this is all I need to get Versatile PCI working (under QEMU): I meant converting all of arm32 would be harder, but I agree we don't have to do it. It would be nice to convert all of the drivers/pci/host drivers though, iow all multiplatform-enabled ones, and leaving the current arm32 pci implementation for the platforms we don't want to convert to multiplatform anyway (footbridge, iop, ixp4xx, ks8695 (?), pxa, sa1100). I'm more than happy to convert the generic host controller we merged recently, but I'd probably want the core changes merged first so that I know I'm not wasting my time! That is definitely fine with me, but it's Bjorn's decision. Or the changes to the generic host controller can be part of Liviu's patch series (maybe together with other PCI host drivers like designware, as time allows). -- Catalin -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Fri, Jun 27, 2014 at 01:44:21AM +0100, Rob Herring wrote: On Thu, Jun 26, 2014 at 3:59 AM, Catalin Marinas catalin.mari...@arm.com wrote: Although a bit late, I'm raising this now and hopefully we'll come to a conclusion soon. Delaying arm64 PCIe support even further is not a real option, which leaves us with: 1. Someone else (with enough PCIe knowledge) volunteering to take over soon or 2. Dropping Liviu's work and going for an arm64-specific implementation (most likely based on the arm32 implementation, see below) 3. Keeping Liviu's patches leaving some of the architecture specific bits. I know Arnd and I both commented on it still needing more common pieces, but compared to option 2 that would be way better. Let's look at the patches in question: 3e71867 pci: Introduce pci_register_io_range() helper function. 6681dff pci: OF: Fix the conversion of IO ranges into IO resources. Both OF patches. I'll happily merge them. We just need to make sure they don't break other users of of_pci_range_to_resource() that the second patch introduces. 2d5dd85 pci: Create pci_host_bridge before its associated bus in pci_create_root_bus. f6f2854 pci: Introduce a domain number for pci_host_bridge. 524a9f5 pci: Export find_pci_host_bridge() function. These don't seem to be too controversial. I think here there were discussions around introducing domain_nr to pci_host_bridge, particularly to the pci_create_root_bus_in_domain() API change. I don't think we reached any conclusion. fb75718 pci: of: Parse and map the IRQ when adding the PCI device. 6 LOC. Hardly controversial. I agree. 920a685 pci: Add support for creating a generic host_bridge from device tree This function could be moved to drivers/of/of_pci.c if having it in drivers/pci is too much maintenance burden. I think it makes sense. Currently drivers/pci/host-bridge.c doesn't have anything OF related, so of_pci.c looks more appropriate. However, nearly the same code is already being duplicated in every DT enabled ARM PCI host driver and will continue as more PCI hosts are added. So this isn't really a question of converting other architectures to common PCI host infrastructure, but converting DT based PCI hosts to common infrastructure. ARM is the only arch moving host drivers to drivers/pci ATM. Until other architectures start doing that, converting them is pointless. I agree. It's probably more important to convert some of the drivers/pci/host implementations to using the common parsing rather than a new architecture (this way we avoid even more code duplication). bcf5c10 Fix ioport_map() for !CONFIG_GENERIC_IOMAP cases. Seems like an independent fix that should be applied regardless. Indeed. But it got stuck at the top of this series and hasn't been pushed upstream. 7cfde80 arm64: Add architecture support for PCI What is here is really just a function of which option we pick. With Liviu's latest version (not posted) and with of_create_pci_host_bridge() function moved to of_pci.c, I don't think there is much new functionality added to drivers/pci/. What I think we need is clarifying the domain_nr patch (and API change) and more users of the new generic code. As you said, it doesn't need to be a separate architecture but rather existing pci host drivers under drivers/pci. Of course, other arch conversion should follow shortly as well but even without an immediate conversion, I don't see too much additional maintenance burden for the core PCIe code (and code sharing between new PCIe host drivers is even more beneficial). -- Catalin -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Fri, Jun 27, 2014 at 8:14 AM, Catalin Marinas catalin.mari...@arm.com wrote: ... With Liviu's latest version (not posted) and with of_create_pci_host_bridge() function moved to of_pci.c, I don't think there is much new functionality added to drivers/pci/. What I think we need is clarifying the domain_nr patch (and API change) and more users of the new generic code. As you said, it doesn't need to be a separate architecture but rather existing pci host drivers under drivers/pci. Of course, other arch conversion should follow shortly as well but even without an immediate conversion, I don't see too much additional maintenance burden for the core PCIe code (and code sharing between new PCIe host drivers is even more beneficial). Sorry, I haven't had time to follow this. It sounds like there are several pieces we could get out of the way easily. How about posting the actual patches again? Maybe re-order them so the easy pieces are first so they can get applied even if there are issues with later ones? Bjorn -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Fri, Jun 27, 2014 at 03:55:04PM +0100, Bjorn Helgaas wrote: On Fri, Jun 27, 2014 at 8:14 AM, Catalin Marinas catalin.mari...@arm.com wrote: ... With Liviu's latest version (not posted) and with of_create_pci_host_bridge() function moved to of_pci.c, I don't think there is much new functionality added to drivers/pci/. What I think we need is clarifying the domain_nr patch (and API change) and more users of the new generic code. As you said, it doesn't need to be a separate architecture but rather existing pci host drivers under drivers/pci. Of course, other arch conversion should follow shortly as well but even without an immediate conversion, I don't see too much additional maintenance burden for the core PCIe code (and code sharing between new PCIe host drivers is even more beneficial). Sorry, I haven't had time to follow this. It sounds like there are several pieces we could get out of the way easily. How about posting the actual patches again? Maybe re-order them so the easy pieces are first so they can get applied even if there are issues with later ones? OK, I will post a new series on Monday. Thanks, Liviu Bjorn -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/¯ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On 06/27/2014 07:49 AM, Will Deacon wrote:
On Fri, Jun 27, 2014 at 12:03:34PM +0100, Arnd Bergmann wrote:
On Thursday 26 June 2014 19:44:21 Rob Herring wrote:
I don't agree arm32 is harder than microblaze. Yes, converting ALL of
arm would be, but that is not necessary. With Liviu's latest branch
the hacks I previously needed are gone (thanks!), and this is all I
need to get Versatile PCI working (under QEMU):
I meant converting all of arm32 would be harder, but I agree we don't
have to do it. It would be nice to convert all of the drivers/pci/host
drivers though, iow all multiplatform-enabled ones, and leaving the
current arm32 pci implementation for the platforms we don't want to
convert to multiplatform anyway (footbridge, iop, ixp4xx, ks8695 (?),
pxa, sa1100).
I'm more than happy to convert the generic host controller we merged
recently, but I'd probably want the core changes merged first so that I
know I'm not wasting my time!
Something like this untested patch...
Another issue I found still present is pci_ioremap_io needs some work
to unify with the arm implementation. That's a matter of changing the
function from an offset to i/o resource. That should be a pretty
mechanical change.
Also, there is a potential for memory leak because there is no undo
for of_create_pci_host_bridge.
Rob
8--
From 301b402631b2867ced38d5533586da0bd888045c Mon Sep 17 00:00:00 2001
From: Rob Herring r...@kernel.org
Date: Fri, 27 Jun 2014 09:46:09 -0500
Subject: [PATCH] pci: generic-host: refactor to use common range parsing
Signed-off-by: Rob Herring r...@kernel.org
---
drivers/pci/host/pci-host-generic.c | 191 ++--
1 file changed, 29 insertions(+), 162 deletions(-)
diff --git a/drivers/pci/host/pci-host-generic.c
b/drivers/pci/host/pci-host-generic.c
index 44fe6aa..57fd1e1 100644
--- a/drivers/pci/host/pci-host-generic.c
+++ b/drivers/pci/host/pci-host-generic.c
@@ -32,16 +32,14 @@ struct gen_pci_cfg_bus_ops {
struct gen_pci_cfg_windows {
struct resource res;
- struct resource bus_range;
void __iomem**win;
const struct gen_pci_cfg_bus_ops*ops;
};
struct gen_pci {
- struct pci_host_bridge host;
+ struct pci_host_bridge *host;
struct gen_pci_cfg_windows cfg;
- struct list_headresources;
};
static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus,
@@ -50,7 +48,7 @@ static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus
*bus,
{
struct pci_sys_data *sys = bus-sysdata;
struct gen_pci *pci = sys-private_data;
- resource_size_t idx = bus-number - pci-cfg.bus_range.start;
+ resource_size_t idx = bus-number - bus-busn_res.start;
return pci-cfg.win[idx] + ((devfn 8) | where);
}
@@ -66,7 +64,7 @@ static void __iomem *gen_pci_map_cfg_bus_ecam(struct pci_bus
*bus,
{
struct pci_sys_data *sys = bus-sysdata;
struct gen_pci *pci = sys-private_data;
- resource_size_t idx = bus-number - pci-cfg.bus_range.start;
+ resource_size_t idx = bus-number - bus-busn_res.start;
return pci-cfg.win[idx] + ((devfn 12) | where);
}
@@ -138,154 +136,32 @@ static const struct of_device_id gen_pci_of_match[] = {
};
MODULE_DEVICE_TABLE(of, gen_pci_of_match);
-static int gen_pci_calc_io_offset(struct device *dev,
- struct of_pci_range *range,
- struct resource *res,
- resource_size_t *offset)
-{
- static atomic_t wins = ATOMIC_INIT(0);
- int err, idx, max_win;
- unsigned int window;
-
- if (!PAGE_ALIGNED(range-cpu_addr))
- return -EINVAL;
-
- max_win = (IO_SPACE_LIMIT + 1) / SZ_64K;
- idx = atomic_inc_return(wins);
- if (idx max_win)
- return -ENOSPC;
-
- window = (idx - 1) * SZ_64K;
- err = pci_ioremap_io(window, range-cpu_addr);
- if (err)
- return err;
-
- of_pci_range_to_resource(range, dev-of_node, res);
- res-start = window;
- res-end = res-start + range-size - 1;
- *offset = window - range-pci_addr;
- return 0;
-}
-
-static int gen_pci_calc_mem_offset(struct device *dev,
- struct of_pci_range *range,
- struct resource *res,
- resource_size_t *offset)
-{
- of_pci_range_to_resource(range, dev-of_node, res);
- *offset = range-cpu_addr - range-pci_addr;
- return 0;
-}
-
-static void gen_pci_release_of_pci_ranges(struct gen_pci *pci)
-{
- struct pci_host_bridge_window *win;
-
- list_for_each_entry(win, pci-resources, list)
- release_resource(win-res);
-
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
Adding Michael Simek... On Thu, Jun 26, 2014 at 3:59 AM, Catalin Marinas wrote: > (sorry for replying to a months old thread) > > On Mon, Apr 07, 2014 at 06:58:24PM +0100, Bjorn Helgaas wrote: >> On Mon, Apr 7, 2014 at 5:36 AM, Arnd Bergmann wrote: >> >> > I think migrating other architectures to use the same code should be >> > a separate effort from adding a generic implementation that can be >> > used by arm64. It's probably a good idea to have patches to convert >> > arm32 and/or microblaze. >> >> Let me reiterate that I am 100% in favor of replacing arch-specific >> code with more generic implementations. >> >> However, I am not 100% in favor of doing it as separate efforts >> (although maybe I could be convinced). The reasons I hesitate are >> that (1) if only one architecture uses a new "generic" implementation, >> we really don't know whether it is generic enough, (2) until I see the >> patches to convert other architectures, I have to assume that I'm the >> one who will write them, and (3) as soon as we add the code to >> drivers/pci, it becomes partly my headache to maintain it, even if >> only one arch benefits from it. > > I agree and understand your point. > >> Please don't think I'm questioning anyone's intent or good will. It's >> just that I understand the business pressures, and I know how hard it >> can be to justify this sort of work to one's management, especially >> after the immediate problem has been solved. > > But, unfortunately, that's something we failed to address in reasonable > time (even though I was one of the proponents of the generic PCIe > implementation). This work is very likely to slip further into the late > part of this year and I am aware that several ARM partners are blocked > on the (upstream) availability of PCIe support for the arm64 kernel. > > Although a bit late, I'm raising this now and hopefully we'll come to a > conclusion soon. Delaying arm64 PCIe support even further is not a real > option, which leaves us with: > > 1. Someone else (with enough PCIe knowledge) volunteering to take over >soon or Well, I might have 2 months ago, but now I'm pretty booked up. > 2. Dropping Liviu's work and going for an arm64-specific implementation >(most likely based on the arm32 implementation, see below) 3. Keeping Liviu's patches leaving some of the architecture specific bits. I know Arnd and I both commented on it still needing more common pieces, but compared to option 2 that would be way better. Let's look at the patches in question: 3e71867 pci: Introduce pci_register_io_range() helper function. 6681dff pci: OF: Fix the conversion of IO ranges into IO resources. Both OF patches. I'll happily merge them. 2d5dd85 pci: Create pci_host_bridge before its associated bus in pci_create_root_bus. f6f2854 pci: Introduce a domain number for pci_host_bridge. 524a9f5 pci: Export find_pci_host_bridge() function. These don't seem to be too controversial. fb75718 pci: of: Parse and map the IRQ when adding the PCI device. 6 LOC. Hardly controversial. 920a685 pci: Add support for creating a generic host_bridge from device tree This function could be moved to drivers/of/of_pci.c if having it in drivers/pci is too much maintenance burden. However, nearly the same code is already being duplicated in every DT enabled ARM PCI host driver and will continue as more PCI hosts are added. So this isn't really a question of converting other architectures to common PCI host infrastructure, but converting DT based PCI hosts to common infrastructure. ARM is the only arch moving host drivers to drivers/pci ATM. Until other architectures start doing that, converting them is pointless. bcf5c10 Fix ioport_map() for !CONFIG_GENERIC_IOMAP cases. Seems like an independent fix that should be applied regardless. 7cfde80 arm64: Add architecture support for PCI What is here is really just a function of which option we pick. > First option is ideal but there is work to do as laid out by Arnd here: > > http://article.gmane.org/gmane.linux.kernel/1679304 I don't agree arm32 is harder than microblaze. Yes, converting ALL of arm would be, but that is not necessary. With Liviu's latest branch the hacks I previously needed are gone (thanks!), and this is all I need to get Versatile PCI working (under QEMU): diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h index 3d23418..22b7529 100644 --- a/arch/arm/include/asm/io.h +++ b/arch/arm/include/asm/io.h @@ -178,6 +178,7 @@ static inline void __iomem *__typesafe_io(unsigned long addr) /* PCI fixed i/o mapping */ #define PCI_IO_VIRT_BASE 0xfee0 +#define PCI_IOBASE PCI_IO_VIRT_BASE #if defined(CONFIG_PCI) void pci_ioremap_set_mem_type(int mem_type); Hum, so I guess now I've converted ARM... Here's a branch with my changes[1]. And BTW, it also has multi-platform support for Versatile as moving the PCI host to DT (and drivers/pci/host) is about the last remaining obstacle. Rob [1]
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Thu, Jun 26, 2014 at 03:11:38PM +0100, Catalin Marinas wrote: > On Thu, Jun 26, 2014 at 10:30:29AM +0100, Liviu Dudau wrote: > > On Thu, Jun 26, 2014 at 09:59:26AM +0100, Catalin Marinas wrote: > > > Although a bit late, I'm raising this now and hopefully we'll come to a > > > conclusion soon. Delaying arm64 PCIe support even further is not a real > > > option, which leaves us with: > > > > > > 1. Someone else (with enough PCIe knowledge) volunteering to take over > > >soon or > > > 2. Dropping Liviu's work and going for an arm64-specific implementation > > >(most likely based on the arm32 implementation, see below) > [...] > > > In conclusion, unless someone volunteers for the first option fairly > > > soon, we'll post the alternative patches for review and take it from > > > there. > > > > That would be a huge step backwards IMO and a huge dissapointment. If > > you go with the alternative patches from Will you will basically reset > > every partner's implementation that has been built on top of my > > patches (when they did so with the understanding that my series will be > > the one ARM will support and publish) *and* make anyone's attempt to > > create a generic implementation harder, as they will have to undo this > > code to remove the arch-specific parts. > > I fully agree and the alternative patchset is definitely _not_ my > preferred solution. You can read this email as a request for help to > complete the work (whether it comes from ARM, Linaro or other interested > parties). I don't mean taking over the whole patchset but potentially > helping with other arch conversion (microblaze, arm multi-platform). I feel it's also worth pointing out that I didn't write that code with the intention of getting it merged, nor as a competing solution to what Liviu was proposing at the time. It was merely a development tool to enable some of the SMMU and GICv3 work that Marc and I have been working on recently. Will -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Thu, Jun 26, 2014 at 10:30:29AM +0100, Liviu Dudau wrote: > On Thu, Jun 26, 2014 at 09:59:26AM +0100, Catalin Marinas wrote: > > Although a bit late, I'm raising this now and hopefully we'll come to a > > conclusion soon. Delaying arm64 PCIe support even further is not a real > > option, which leaves us with: > > > > 1. Someone else (with enough PCIe knowledge) volunteering to take over > >soon or > > 2. Dropping Liviu's work and going for an arm64-specific implementation > >(most likely based on the arm32 implementation, see below) [...] > > In conclusion, unless someone volunteers for the first option fairly > > soon, we'll post the alternative patches for review and take it from > > there. > > That would be a huge step backwards IMO and a huge dissapointment. If > you go with the alternative patches from Will you will basically reset > every partner's implementation that has been built on top of my > patches (when they did so with the understanding that my series will be > the one ARM will support and publish) *and* make anyone's attempt to > create a generic implementation harder, as they will have to undo this > code to remove the arch-specific parts. I fully agree and the alternative patchset is definitely _not_ my preferred solution. You can read this email as a request for help to complete the work (whether it comes from ARM, Linaro or other interested parties). I don't mean taking over the whole patchset but potentially helping with other arch conversion (microblaze, arm multi-platform). (however, if the generic PCIe work won't happen in reasonable time, we need to set some deadline rather than keeping the patchset out of tree indefinitely) -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Thu, Jun 26, 2014 at 09:59:26AM +0100, Catalin Marinas wrote: > (sorry for replying to a months old thread) > > On Mon, Apr 07, 2014 at 06:58:24PM +0100, Bjorn Helgaas wrote: > > On Mon, Apr 7, 2014 at 5:36 AM, Arnd Bergmann wrote: > > > > > I think migrating other architectures to use the same code should be > > > a separate effort from adding a generic implementation that can be > > > used by arm64. It's probably a good idea to have patches to convert > > > arm32 and/or microblaze. > > > > Let me reiterate that I am 100% in favor of replacing arch-specific > > code with more generic implementations. > > > > However, I am not 100% in favor of doing it as separate efforts > > (although maybe I could be convinced). The reasons I hesitate are > > that (1) if only one architecture uses a new "generic" implementation, > > we really don't know whether it is generic enough, (2) until I see the > > patches to convert other architectures, I have to assume that I'm the > > one who will write them, and (3) as soon as we add the code to > > drivers/pci, it becomes partly my headache to maintain it, even if > > only one arch benefits from it. > > I agree and understand your point. > > > Please don't think I'm questioning anyone's intent or good will. It's > > just that I understand the business pressures, and I know how hard it > > can be to justify this sort of work to one's management, especially > > after the immediate problem has been solved. > > But, unfortunately, that's something we failed to address in reasonable > time (even though I was one of the proponents of the generic PCIe > implementation). This work is very likely to slip further into the late > part of this year and I am aware that several ARM partners are blocked > on the (upstream) availability of PCIe support for the arm64 kernel. > > Although a bit late, I'm raising this now and hopefully we'll come to a > conclusion soon. Delaying arm64 PCIe support even further is not a real > option, which leaves us with: > > 1. Someone else (with enough PCIe knowledge) volunteering to take over >soon or > 2. Dropping Liviu's work and going for an arm64-specific implementation >(most likely based on the arm32 implementation, see below) > > First option is ideal but there is work to do as laid out by Arnd here: > > http://article.gmane.org/gmane.linux.kernel/1679304 > > The latest patches from Liviu are here (they only target arm64 and there > are additional comments to be addressed from the above thread): > > http://linux-arm.org/git?p=linux-ld.git;a=shortlog;h=refs/heads/for-upstream/pci-next > > The main reason for the second option is timing. We could temporarily > move Liviu's code under arch/arm64 with the hope that we generalise it > later. However, the risk is even higher that once the code is in > mainline, the generic implementation won't happen. In which case, I > don't see much point in departing from the arm32 PCI API, making bios32 > clone the best second option. > > For the alternative implementation above, we already have a heavily cut > down version of the arm32 PCI support but only tested in a virtual > environment so far: > > https://git.kernel.org/cgit/linux/kernel/git/will/linux.git/log/?h=pci/bios32 > > In conclusion, unless someone volunteers for the first option fairly > soon, we'll post the alternative patches for review and take it from > there. That would be a huge step backwards IMO and a huge dissapointment. If you go with the alternative patches from Will you will basically reset every partner's implementation that has been built on top of my patches (when they did so with the understanding that my series will be the one ARM will support and publish) *and* make anyone's attempt to create a generic implementation harder, as they will have to undo this code to remove the arch-specific parts. While I have part of a personal blame to carry as I have dragged some of the work for too long, there are things that I could not have done differently due to internal pressure inside the project I work on. It is my intent to resume work on it as soon as possible, but life is such at the moment that I have to dedicate my time to other things. Best regards, Liviu > > Thanks. > > -- > Catalin -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/¯ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
(sorry for replying to a months old thread) On Mon, Apr 07, 2014 at 06:58:24PM +0100, Bjorn Helgaas wrote: > On Mon, Apr 7, 2014 at 5:36 AM, Arnd Bergmann wrote: > > > I think migrating other architectures to use the same code should be > > a separate effort from adding a generic implementation that can be > > used by arm64. It's probably a good idea to have patches to convert > > arm32 and/or microblaze. > > Let me reiterate that I am 100% in favor of replacing arch-specific > code with more generic implementations. > > However, I am not 100% in favor of doing it as separate efforts > (although maybe I could be convinced). The reasons I hesitate are > that (1) if only one architecture uses a new "generic" implementation, > we really don't know whether it is generic enough, (2) until I see the > patches to convert other architectures, I have to assume that I'm the > one who will write them, and (3) as soon as we add the code to > drivers/pci, it becomes partly my headache to maintain it, even if > only one arch benefits from it. I agree and understand your point. > Please don't think I'm questioning anyone's intent or good will. It's > just that I understand the business pressures, and I know how hard it > can be to justify this sort of work to one's management, especially > after the immediate problem has been solved. But, unfortunately, that's something we failed to address in reasonable time (even though I was one of the proponents of the generic PCIe implementation). This work is very likely to slip further into the late part of this year and I am aware that several ARM partners are blocked on the (upstream) availability of PCIe support for the arm64 kernel. Although a bit late, I'm raising this now and hopefully we'll come to a conclusion soon. Delaying arm64 PCIe support even further is not a real option, which leaves us with: 1. Someone else (with enough PCIe knowledge) volunteering to take over soon or 2. Dropping Liviu's work and going for an arm64-specific implementation (most likely based on the arm32 implementation, see below) First option is ideal but there is work to do as laid out by Arnd here: http://article.gmane.org/gmane.linux.kernel/1679304 The latest patches from Liviu are here (they only target arm64 and there are additional comments to be addressed from the above thread): http://linux-arm.org/git?p=linux-ld.git;a=shortlog;h=refs/heads/for-upstream/pci-next The main reason for the second option is timing. We could temporarily move Liviu's code under arch/arm64 with the hope that we generalise it later. However, the risk is even higher that once the code is in mainline, the generic implementation won't happen. In which case, I don't see much point in departing from the arm32 PCI API, making bios32 clone the best second option. For the alternative implementation above, we already have a heavily cut down version of the arm32 PCI support but only tested in a virtual environment so far: https://git.kernel.org/cgit/linux/kernel/git/will/linux.git/log/?h=pci/bios32 In conclusion, unless someone volunteers for the first option fairly soon, we'll post the alternative patches for review and take it from there. Thanks. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
(sorry for replying to a months old thread) On Mon, Apr 07, 2014 at 06:58:24PM +0100, Bjorn Helgaas wrote: On Mon, Apr 7, 2014 at 5:36 AM, Arnd Bergmann a...@arndb.de wrote: I think migrating other architectures to use the same code should be a separate effort from adding a generic implementation that can be used by arm64. It's probably a good idea to have patches to convert arm32 and/or microblaze. Let me reiterate that I am 100% in favor of replacing arch-specific code with more generic implementations. However, I am not 100% in favor of doing it as separate efforts (although maybe I could be convinced). The reasons I hesitate are that (1) if only one architecture uses a new generic implementation, we really don't know whether it is generic enough, (2) until I see the patches to convert other architectures, I have to assume that I'm the one who will write them, and (3) as soon as we add the code to drivers/pci, it becomes partly my headache to maintain it, even if only one arch benefits from it. I agree and understand your point. Please don't think I'm questioning anyone's intent or good will. It's just that I understand the business pressures, and I know how hard it can be to justify this sort of work to one's management, especially after the immediate problem has been solved. But, unfortunately, that's something we failed to address in reasonable time (even though I was one of the proponents of the generic PCIe implementation). This work is very likely to slip further into the late part of this year and I am aware that several ARM partners are blocked on the (upstream) availability of PCIe support for the arm64 kernel. Although a bit late, I'm raising this now and hopefully we'll come to a conclusion soon. Delaying arm64 PCIe support even further is not a real option, which leaves us with: 1. Someone else (with enough PCIe knowledge) volunteering to take over soon or 2. Dropping Liviu's work and going for an arm64-specific implementation (most likely based on the arm32 implementation, see below) First option is ideal but there is work to do as laid out by Arnd here: http://article.gmane.org/gmane.linux.kernel/1679304 The latest patches from Liviu are here (they only target arm64 and there are additional comments to be addressed from the above thread): http://linux-arm.org/git?p=linux-ld.git;a=shortlog;h=refs/heads/for-upstream/pci-next The main reason for the second option is timing. We could temporarily move Liviu's code under arch/arm64 with the hope that we generalise it later. However, the risk is even higher that once the code is in mainline, the generic implementation won't happen. In which case, I don't see much point in departing from the arm32 PCI API, making bios32 clone the best second option. For the alternative implementation above, we already have a heavily cut down version of the arm32 PCI support but only tested in a virtual environment so far: https://git.kernel.org/cgit/linux/kernel/git/will/linux.git/log/?h=pci/bios32 In conclusion, unless someone volunteers for the first option fairly soon, we'll post the alternative patches for review and take it from there. Thanks. -- Catalin -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Thu, Jun 26, 2014 at 09:59:26AM +0100, Catalin Marinas wrote: (sorry for replying to a months old thread) On Mon, Apr 07, 2014 at 06:58:24PM +0100, Bjorn Helgaas wrote: On Mon, Apr 7, 2014 at 5:36 AM, Arnd Bergmann a...@arndb.de wrote: I think migrating other architectures to use the same code should be a separate effort from adding a generic implementation that can be used by arm64. It's probably a good idea to have patches to convert arm32 and/or microblaze. Let me reiterate that I am 100% in favor of replacing arch-specific code with more generic implementations. However, I am not 100% in favor of doing it as separate efforts (although maybe I could be convinced). The reasons I hesitate are that (1) if only one architecture uses a new generic implementation, we really don't know whether it is generic enough, (2) until I see the patches to convert other architectures, I have to assume that I'm the one who will write them, and (3) as soon as we add the code to drivers/pci, it becomes partly my headache to maintain it, even if only one arch benefits from it. I agree and understand your point. Please don't think I'm questioning anyone's intent or good will. It's just that I understand the business pressures, and I know how hard it can be to justify this sort of work to one's management, especially after the immediate problem has been solved. But, unfortunately, that's something we failed to address in reasonable time (even though I was one of the proponents of the generic PCIe implementation). This work is very likely to slip further into the late part of this year and I am aware that several ARM partners are blocked on the (upstream) availability of PCIe support for the arm64 kernel. Although a bit late, I'm raising this now and hopefully we'll come to a conclusion soon. Delaying arm64 PCIe support even further is not a real option, which leaves us with: 1. Someone else (with enough PCIe knowledge) volunteering to take over soon or 2. Dropping Liviu's work and going for an arm64-specific implementation (most likely based on the arm32 implementation, see below) First option is ideal but there is work to do as laid out by Arnd here: http://article.gmane.org/gmane.linux.kernel/1679304 The latest patches from Liviu are here (they only target arm64 and there are additional comments to be addressed from the above thread): http://linux-arm.org/git?p=linux-ld.git;a=shortlog;h=refs/heads/for-upstream/pci-next The main reason for the second option is timing. We could temporarily move Liviu's code under arch/arm64 with the hope that we generalise it later. However, the risk is even higher that once the code is in mainline, the generic implementation won't happen. In which case, I don't see much point in departing from the arm32 PCI API, making bios32 clone the best second option. For the alternative implementation above, we already have a heavily cut down version of the arm32 PCI support but only tested in a virtual environment so far: https://git.kernel.org/cgit/linux/kernel/git/will/linux.git/log/?h=pci/bios32 In conclusion, unless someone volunteers for the first option fairly soon, we'll post the alternative patches for review and take it from there. That would be a huge step backwards IMO and a huge dissapointment. If you go with the alternative patches from Will you will basically reset every partner's implementation that has been built on top of my patches (when they did so with the understanding that my series will be the one ARM will support and publish) *and* make anyone's attempt to create a generic implementation harder, as they will have to undo this code to remove the arch-specific parts. While I have part of a personal blame to carry as I have dragged some of the work for too long, there are things that I could not have done differently due to internal pressure inside the project I work on. It is my intent to resume work on it as soon as possible, but life is such at the moment that I have to dedicate my time to other things. Best regards, Liviu Thanks. -- Catalin -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/¯ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Thu, Jun 26, 2014 at 10:30:29AM +0100, Liviu Dudau wrote: On Thu, Jun 26, 2014 at 09:59:26AM +0100, Catalin Marinas wrote: Although a bit late, I'm raising this now and hopefully we'll come to a conclusion soon. Delaying arm64 PCIe support even further is not a real option, which leaves us with: 1. Someone else (with enough PCIe knowledge) volunteering to take over soon or 2. Dropping Liviu's work and going for an arm64-specific implementation (most likely based on the arm32 implementation, see below) [...] In conclusion, unless someone volunteers for the first option fairly soon, we'll post the alternative patches for review and take it from there. That would be a huge step backwards IMO and a huge dissapointment. If you go with the alternative patches from Will you will basically reset every partner's implementation that has been built on top of my patches (when they did so with the understanding that my series will be the one ARM will support and publish) *and* make anyone's attempt to create a generic implementation harder, as they will have to undo this code to remove the arch-specific parts. I fully agree and the alternative patchset is definitely _not_ my preferred solution. You can read this email as a request for help to complete the work (whether it comes from ARM, Linaro or other interested parties). I don't mean taking over the whole patchset but potentially helping with other arch conversion (microblaze, arm multi-platform). (however, if the generic PCIe work won't happen in reasonable time, we need to set some deadline rather than keeping the patchset out of tree indefinitely) -- Catalin -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Thu, Jun 26, 2014 at 03:11:38PM +0100, Catalin Marinas wrote: On Thu, Jun 26, 2014 at 10:30:29AM +0100, Liviu Dudau wrote: On Thu, Jun 26, 2014 at 09:59:26AM +0100, Catalin Marinas wrote: Although a bit late, I'm raising this now and hopefully we'll come to a conclusion soon. Delaying arm64 PCIe support even further is not a real option, which leaves us with: 1. Someone else (with enough PCIe knowledge) volunteering to take over soon or 2. Dropping Liviu's work and going for an arm64-specific implementation (most likely based on the arm32 implementation, see below) [...] In conclusion, unless someone volunteers for the first option fairly soon, we'll post the alternative patches for review and take it from there. That would be a huge step backwards IMO and a huge dissapointment. If you go with the alternative patches from Will you will basically reset every partner's implementation that has been built on top of my patches (when they did so with the understanding that my series will be the one ARM will support and publish) *and* make anyone's attempt to create a generic implementation harder, as they will have to undo this code to remove the arch-specific parts. I fully agree and the alternative patchset is definitely _not_ my preferred solution. You can read this email as a request for help to complete the work (whether it comes from ARM, Linaro or other interested parties). I don't mean taking over the whole patchset but potentially helping with other arch conversion (microblaze, arm multi-platform). I feel it's also worth pointing out that I didn't write that code with the intention of getting it merged, nor as a competing solution to what Liviu was proposing at the time. It was merely a development tool to enable some of the SMMU and GICv3 work that Marc and I have been working on recently. Will -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
Adding Michael Simek... On Thu, Jun 26, 2014 at 3:59 AM, Catalin Marinas catalin.mari...@arm.com wrote: (sorry for replying to a months old thread) On Mon, Apr 07, 2014 at 06:58:24PM +0100, Bjorn Helgaas wrote: On Mon, Apr 7, 2014 at 5:36 AM, Arnd Bergmann a...@arndb.de wrote: I think migrating other architectures to use the same code should be a separate effort from adding a generic implementation that can be used by arm64. It's probably a good idea to have patches to convert arm32 and/or microblaze. Let me reiterate that I am 100% in favor of replacing arch-specific code with more generic implementations. However, I am not 100% in favor of doing it as separate efforts (although maybe I could be convinced). The reasons I hesitate are that (1) if only one architecture uses a new generic implementation, we really don't know whether it is generic enough, (2) until I see the patches to convert other architectures, I have to assume that I'm the one who will write them, and (3) as soon as we add the code to drivers/pci, it becomes partly my headache to maintain it, even if only one arch benefits from it. I agree and understand your point. Please don't think I'm questioning anyone's intent or good will. It's just that I understand the business pressures, and I know how hard it can be to justify this sort of work to one's management, especially after the immediate problem has been solved. But, unfortunately, that's something we failed to address in reasonable time (even though I was one of the proponents of the generic PCIe implementation). This work is very likely to slip further into the late part of this year and I am aware that several ARM partners are blocked on the (upstream) availability of PCIe support for the arm64 kernel. Although a bit late, I'm raising this now and hopefully we'll come to a conclusion soon. Delaying arm64 PCIe support even further is not a real option, which leaves us with: 1. Someone else (with enough PCIe knowledge) volunteering to take over soon or Well, I might have 2 months ago, but now I'm pretty booked up. 2. Dropping Liviu's work and going for an arm64-specific implementation (most likely based on the arm32 implementation, see below) 3. Keeping Liviu's patches leaving some of the architecture specific bits. I know Arnd and I both commented on it still needing more common pieces, but compared to option 2 that would be way better. Let's look at the patches in question: 3e71867 pci: Introduce pci_register_io_range() helper function. 6681dff pci: OF: Fix the conversion of IO ranges into IO resources. Both OF patches. I'll happily merge them. 2d5dd85 pci: Create pci_host_bridge before its associated bus in pci_create_root_bus. f6f2854 pci: Introduce a domain number for pci_host_bridge. 524a9f5 pci: Export find_pci_host_bridge() function. These don't seem to be too controversial. fb75718 pci: of: Parse and map the IRQ when adding the PCI device. 6 LOC. Hardly controversial. 920a685 pci: Add support for creating a generic host_bridge from device tree This function could be moved to drivers/of/of_pci.c if having it in drivers/pci is too much maintenance burden. However, nearly the same code is already being duplicated in every DT enabled ARM PCI host driver and will continue as more PCI hosts are added. So this isn't really a question of converting other architectures to common PCI host infrastructure, but converting DT based PCI hosts to common infrastructure. ARM is the only arch moving host drivers to drivers/pci ATM. Until other architectures start doing that, converting them is pointless. bcf5c10 Fix ioport_map() for !CONFIG_GENERIC_IOMAP cases. Seems like an independent fix that should be applied regardless. 7cfde80 arm64: Add architecture support for PCI What is here is really just a function of which option we pick. First option is ideal but there is work to do as laid out by Arnd here: http://article.gmane.org/gmane.linux.kernel/1679304 I don't agree arm32 is harder than microblaze. Yes, converting ALL of arm would be, but that is not necessary. With Liviu's latest branch the hacks I previously needed are gone (thanks!), and this is all I need to get Versatile PCI working (under QEMU): diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h index 3d23418..22b7529 100644 --- a/arch/arm/include/asm/io.h +++ b/arch/arm/include/asm/io.h @@ -178,6 +178,7 @@ static inline void __iomem *__typesafe_io(unsigned long addr) /* PCI fixed i/o mapping */ #define PCI_IO_VIRT_BASE 0xfee0 +#define PCI_IOBASE PCI_IO_VIRT_BASE #if defined(CONFIG_PCI) void pci_ioremap_set_mem_type(int mem_type); Hum, so I guess now I've converted ARM... Here's a branch with my changes[1]. And BTW, it also has multi-platform support for Versatile as moving the PCI host to DT (and drivers/pci/host) is about the last remaining obstacle. Rob [1]
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Tue, Apr 8, 2014 at 4:22 AM, Arnd Bergmann wrote: > On Tuesday 08 April 2014 10:50:39 Liviu Dudau wrote: >> On Mon, Apr 07, 2014 at 06:58:24PM +0100, Bjorn Helgaas wrote: >> > On Mon, Apr 7, 2014 at 5:36 AM, Arnd Bergmann wrote: >> > >> > > I think migrating other architectures to use the same code should be >> > > a separate effort from adding a generic implementation that can be >> > > used by arm64. It's probably a good idea to have patches to convert >> > > arm32 and/or microblaze. >> > >> > Let me reiterate that I am 100% in favor of replacing arch-specific >> > code with more generic implementations. >> > >> > However, I am not 100% in favor of doing it as separate efforts >> > (although maybe I could be convinced). The reasons I hesitate are >> > that (1) if only one architecture uses a new "generic" implementation, >> > we really don't know whether it is generic enough, (2) until I see the >> > patches to convert other architectures, I have to assume that I'm the >> > one who will write them, and (3) as soon as we add the code to >> > drivers/pci, it becomes partly my headache to maintain it, even if >> > only one arch benefits from it. > > Fair enough. > > My approach to the asm-generic infrastruction has mostly been to ensure > that whoever adds a new architecture has to make things easier for the > next person. That's a good rule. But if we add a generic implementation used only by one architecture, the overall complexity has increased (we added new unshared code), so the next person has to look at N+1 existing implementations. If we even convert one existing arch, that seems like an improvement: we have N implementations with one being used by at least two arches. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Tue, Apr 8, 2014 at 4:11 AM, Arnd Bergmann wrote: > There is a wide range of implementations across architectures: > > a) no access to I/O ports at all (tile, s390, ...) > b) access to I/O ports only through special instructions (x86, ...) > c) all MMIO is mapped virtual contiguous to PCI_IOBASE or _IO_BASE >(most ppc64, arm32 with MMU, arm64, ...) > d) only one PCI host can have an I/O space (mips, microblaze, ...) > e) each host controller can have its own method (ppc64 with indirect pio) > f) PIO token equals virtual address plus offset (some legacy ARM platforms, >probably some other architectures), or physical address (sparc) > g) PIO token encodes address space number plus offset (ia64) > > a) and b) are trivially handled by any implementation that falls > back to 'return -EINVAL'. > I believe that c) is the most appropriate solution and we should be > able to adopt it by most of the architectures that have an MMU and > make it the default implementation. > d) seems like a good fallback for noMMU architectures: While we do > need to support I/O spaces, we need to support multiple PCI domains, > and we need to support noMMU, the combination of all three should > be extremely rare, and I'd leave it up to the architecture to support > that if there is a real use case, rather than trying to put that into > common code. > Anything that currently requires e), f) or g) I think should keep > doing that and not try to use the generic implementation. Thanks for the nice summary. That's way more than I had figured out myself. I don't know whether it'd be worth it, especially for something that's so close to obsolete, but it seems like it should be *possible* to generalize and unify this somewhat. I would argue that g) (which I wrote, so I know it better than the others) could fairly easily subsume c), d), and f), since it maps an ioport number to a virtual address for an MMIO access, but it doesn't assume that all the MMIO spaces are contiguous. b), e), and maybe a) could be handled with an exception, e.g., inside inb(), look up the struct io_space (e.g., similar to what ia64 does in __ia64_mk_io_addr()), and if that struct contains a non-zero ops pointer, use that instead of doing the MMIO access. The ops pointer functions could use the x86 INB instruction or do the indirect PIO thing or whatever. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Tuesday 08 April 2014 10:50:39 Liviu Dudau wrote: > On Mon, Apr 07, 2014 at 06:58:24PM +0100, Bjorn Helgaas wrote: > > On Mon, Apr 7, 2014 at 5:36 AM, Arnd Bergmann wrote: > > > > > I think migrating other architectures to use the same code should be > > > a separate effort from adding a generic implementation that can be > > > used by arm64. It's probably a good idea to have patches to convert > > > arm32 and/or microblaze. > > > > Let me reiterate that I am 100% in favor of replacing arch-specific > > code with more generic implementations. > > > > However, I am not 100% in favor of doing it as separate efforts > > (although maybe I could be convinced). The reasons I hesitate are > > that (1) if only one architecture uses a new "generic" implementation, > > we really don't know whether it is generic enough, (2) until I see the > > patches to convert other architectures, I have to assume that I'm the > > one who will write them, and (3) as soon as we add the code to > > drivers/pci, it becomes partly my headache to maintain it, even if > > only one arch benefits from it. Fair enough. My approach to the asm-generic infrastruction has mostly been to ensure that whoever adds a new architecture has to make things easier for the next person. For the PCI code it's clearly your call to pick whatever works best for you. > > Please don't think I'm questioning anyone's intent or good will. It's > > just that I understand the business pressures, and I know how hard it > > can be to justify this sort of work to one's management, especially > > after the immediate problem has been solved. > > I understand your concern. I guess the only way to prove my good intentions > is to shut up and show the code. I'd suggest looking at architectures in this order then: * microblaze (this one is easy and wants to share code with us) * arm32-multiplatform (obviously interesting, but not as easy as microblaze) * powerpc64 (they are keen on sharing, code is similar to what you have) * mips (this is really platform specific, some want to share drivers with arm32, others should keep their current code. Note that platform selection on mips is compile-time only, they don't have to do it all the same way) * powerpc32 (their code is currently different, might not be worth it) My preference would be to have only the first two done initially and leave the other ones up to architecture maintainers, but Bjorn should say how much he wants to see get done. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Tuesday 08 April 2014 10:49:33 Liviu Dudau wrote:
> On Tue, Apr 08, 2014 at 12:21:51AM +0100, Bjorn Helgaas wrote:
> > On Fri, Mar 14, 2014 at 9:34 AM, Liviu Dudau wrote:
> > > Some architectures do not share x86 simple view of the PCI I/O space
> > > and instead use a range of addresses that map to bus addresses. For
> > > some architectures these ranges will be expressed by OF bindings
> > > in a device tree file.
> > >
> > > Introduce a pci_register_io_range() helper function that can be used
> > > by the architecture code to keep track of the I/O ranges described by the
> > > PCI bindings. If the PCI_IOBASE macro is not defined that signals
> > > lack of support for PCI and we return an error.
> > >
> > > Signed-off-by: Liviu Dudau
> > > Acked-by: Grant Likely
> > > Tested-by: Tanmay Inamdar
> > > ---
> > > drivers/of/address.c | 9 +
> > > include/linux/of_address.h | 1 +
> > > 2 files changed, 10 insertions(+)
> > >
> > > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > > index 1a54f1f..be958ed 100644
> > > --- a/drivers/of/address.c
> > > +++ b/drivers/of/address.c
> > > @@ -619,6 +619,15 @@ const __be32 *of_get_address(struct device_node
> > > *dev, int index, u64 *size,
> > > }
> > > EXPORT_SYMBOL(of_get_address);
> > >
> > > +int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> > > +{
> > > +#ifndef PCI_IOBASE
> > > + return -EINVAL;
> > > +#else
> > > + return 0;
> > > +#endif
> > > +}
> >
> > This isn't PCI code, so I'm fine with it in that sense, but I'm not
> > sure the idea of a PCI_IOBASE #define is really what we need. It's
> > not really determined by the processor architecture, it's determined
> > by the platform. And a single address isn't enough in general,
> > either, because if there are multiple host bridges, there's no reason
> > the apertures that generate PCI I/O transactions need to be contiguous
> > on the CPU side.
>
> It should not be only platform's choice if the architecture doesn't support
> it.
> To my mind PCI_IOBASE means "I support MMIO operations and this is the
> start of the virtual address where my I/O ranges are mapped." It's the
> same as ppc's _IO_BASE. And pci_address_to_pio() will take care to give
> you the correct io_offset in the presence of multiple host bridges,
> while keeping the io resource in the range [0 .. host_bridge_io_range_size -
> 1]
There is a wide range of implementations across architectures:
a) no access to I/O ports at all (tile, s390, ...)
b) access to I/O ports only through special instructions (x86, ...)
c) all MMIO is mapped virtual contiguous to PCI_IOBASE or _IO_BASE
(most ppc64, arm32 with MMU, arm64, ...)
d) only one PCI host can have an I/O space (mips, microblaze, ...)
e) each host controller can have its own method (ppc64 with indirect pio)
f) PIO token equals virtual address plus offset (some legacy ARM platforms,
probably some other architectures), or physical address (sparc)
g) PIO token encodes address space number plus offset (ia64)
a) and b) are trivially handled by any implementation that falls
back to 'return -EINVAL'.
I believe that c) is the most appropriate solution and we should be
able to adopt it by most of the architectures that have an MMU and
make it the default implementation.
d) seems like a good fallback for noMMU architectures: While we do
need to support I/O spaces, we need to support multiple PCI domains,
and we need to support noMMU, the combination of all three should
be extremely rare, and I'd leave it up to the architecture to support
that if there is a real use case, rather than trying to put that into
common code.
Anything that currently requires e), f) or g) I think should keep
doing that and not try to use the generic implementation.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Mon, Apr 07, 2014 at 06:58:24PM +0100, Bjorn Helgaas wrote: > On Mon, Apr 7, 2014 at 5:36 AM, Arnd Bergmann wrote: > > > I think migrating other architectures to use the same code should be > > a separate effort from adding a generic implementation that can be > > used by arm64. It's probably a good idea to have patches to convert > > arm32 and/or microblaze. > > Let me reiterate that I am 100% in favor of replacing arch-specific > code with more generic implementations. > > However, I am not 100% in favor of doing it as separate efforts > (although maybe I could be convinced). The reasons I hesitate are > that (1) if only one architecture uses a new "generic" implementation, > we really don't know whether it is generic enough, (2) until I see the > patches to convert other architectures, I have to assume that I'm the > one who will write them, and (3) as soon as we add the code to > drivers/pci, it becomes partly my headache to maintain it, even if > only one arch benefits from it. > > Please don't think I'm questioning anyone's intent or good will. It's > just that I understand the business pressures, and I know how hard it > can be to justify this sort of work to one's management, especially > after the immediate problem has been solved. I understand your concern. I guess the only way to prove my good intentions is to shut up and show the code. Liviu > > Bjorn > -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/¯ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Tue, Apr 08, 2014 at 12:21:51AM +0100, Bjorn Helgaas wrote:
> On Fri, Mar 14, 2014 at 9:34 AM, Liviu Dudau wrote:
> > Some architectures do not share x86 simple view of the PCI I/O space
> > and instead use a range of addresses that map to bus addresses. For
> > some architectures these ranges will be expressed by OF bindings
> > in a device tree file.
> >
> > Introduce a pci_register_io_range() helper function that can be used
> > by the architecture code to keep track of the I/O ranges described by the
> > PCI bindings. If the PCI_IOBASE macro is not defined that signals
> > lack of support for PCI and we return an error.
> >
> > Signed-off-by: Liviu Dudau
> > Acked-by: Grant Likely
> > Tested-by: Tanmay Inamdar
> > ---
> > drivers/of/address.c | 9 +
> > include/linux/of_address.h | 1 +
> > 2 files changed, 10 insertions(+)
> >
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index 1a54f1f..be958ed 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -619,6 +619,15 @@ const __be32 *of_get_address(struct device_node *dev,
> > int index, u64 *size,
> > }
> > EXPORT_SYMBOL(of_get_address);
> >
> > +int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> > +{
> > +#ifndef PCI_IOBASE
> > + return -EINVAL;
> > +#else
> > + return 0;
> > +#endif
> > +}
>
> This isn't PCI code, so I'm fine with it in that sense, but I'm not
> sure the idea of a PCI_IOBASE #define is really what we need. It's
> not really determined by the processor architecture, it's determined
> by the platform. And a single address isn't enough in general,
> either, because if there are multiple host bridges, there's no reason
> the apertures that generate PCI I/O transactions need to be contiguous
> on the CPU side.
It should not be only platform's choice if the architecture doesn't support it.
To my mind PCI_IOBASE means "I support MMIO operations and this is the
start of the virtual address where my I/O ranges are mapped." It's the
same as ppc's _IO_BASE. And pci_address_to_pio() will take care to give
you the correct io_offset in the presence of multiple host bridges,
while keeping the io resource in the range [0 .. host_bridge_io_range_size - 1]
>
> That's just a long way of saying that if we ever came up with a more
> generic way to handle I/O port spaces, PCI_IOBASE might go away. And
> I guess part of that rework could be changing this use of it along
> with the others.
And I have a patch series that #defines PCI_IOBASE only in those architectures
that support MMIO, where this macro makes sense. Also notice that the
arm64 series has a patch that I'm going to roll into this one where ioport_map()
gets fixed to include PCI_IOBASE when !CONFIG_GENERIC_MAP.
Best regards,
Liviu
>
> > unsigned long __weak pci_address_to_pio(phys_addr_t address)
> > {
> > if (address > IO_SPACE_LIMIT)
> > diff --git a/include/linux/of_address.h b/include/linux/of_address.h
> > index 5f6ed6b..40c418d 100644
> > --- a/include/linux/of_address.h
> > +++ b/include/linux/of_address.h
> > @@ -56,6 +56,7 @@ extern void __iomem *of_iomap(struct device_node *device,
> > int index);
> > extern const __be32 *of_get_address(struct device_node *dev, int index,
> >u64 *size, unsigned int *flags);
> >
> > +extern int pci_register_io_range(phys_addr_t addr, resource_size_t size);
> > extern unsigned long pci_address_to_pio(phys_addr_t addr);
> >
> > extern int of_pci_range_parser_init(struct of_pci_range_parser *parser,
> > --
> > 1.9.0
> >
>
--
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---
¯\_(ツ)_/¯
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Monday 07 April 2014 17:21:51 Bjorn Helgaas wrote:
> On Fri, Mar 14, 2014 at 9:34 AM, Liviu Dudau wrote:
> > Some architectures do not share x86 simple view of the PCI I/O space
> > and instead use a range of addresses that map to bus addresses. For
> > some architectures these ranges will be expressed by OF bindings
> > in a device tree file.
> >
> > Introduce a pci_register_io_range() helper function that can be used
> > by the architecture code to keep track of the I/O ranges described by the
> > PCI bindings. If the PCI_IOBASE macro is not defined that signals
> > lack of support for PCI and we return an error.
> >
> > Signed-off-by: Liviu Dudau
> > Acked-by: Grant Likely
> > Tested-by: Tanmay Inamdar
> > ---
> > drivers/of/address.c | 9 +
> > include/linux/of_address.h | 1 +
> > 2 files changed, 10 insertions(+)
> >
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index 1a54f1f..be958ed 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -619,6 +619,15 @@ const __be32 *of_get_address(struct device_node *dev,
> > int index, u64 *size,
> > }
> > EXPORT_SYMBOL(of_get_address);
> >
> > +int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> > +{
> > +#ifndef PCI_IOBASE
> > + return -EINVAL;
> > +#else
> > + return 0;
> > +#endif
> > +}
>
> This isn't PCI code, so I'm fine with it in that sense, but I'm not
> sure the idea of a PCI_IOBASE #define is really what we need. It's
> not really determined by the processor architecture, it's determined
> by the platform. And a single address isn't enough in general,
> either, because if there are multiple host bridges, there's no reason
> the apertures that generate PCI I/O transactions need to be contiguous
> on the CPU side.
>
> That's just a long way of saying that if we ever came up with a more
> generic way to handle I/O port spaces, PCI_IOBASE might go away. And
> I guess part of that rework could be changing this use of it along
> with the others.
I'd rather not add a generic implementation of this at all, but
keep it all within the host resource scanning code.
If we do add a generic implementation, my preference would be
to use the version introduced for arm64, with a fallback of
returning -EINVAL if the architecture doesn't implement it.
There is no way ever that returning '0' makes sense here: Either
the architecture supports memory mapped I/O spaces and then we
should be able to find an appropriate io_offset for it, or it
doesn't support memory mapped I/O spaces and we should never
even call this function.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Monday 07 April 2014 17:21:51 Bjorn Helgaas wrote:
On Fri, Mar 14, 2014 at 9:34 AM, Liviu Dudau liviu.du...@arm.com wrote:
Some architectures do not share x86 simple view of the PCI I/O space
and instead use a range of addresses that map to bus addresses. For
some architectures these ranges will be expressed by OF bindings
in a device tree file.
Introduce a pci_register_io_range() helper function that can be used
by the architecture code to keep track of the I/O ranges described by the
PCI bindings. If the PCI_IOBASE macro is not defined that signals
lack of support for PCI and we return an error.
Signed-off-by: Liviu Dudau liviu.du...@arm.com
Acked-by: Grant Likely grant.lik...@linaro.org
Tested-by: Tanmay Inamdar tinam...@apm.com
---
drivers/of/address.c | 9 +
include/linux/of_address.h | 1 +
2 files changed, 10 insertions(+)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 1a54f1f..be958ed 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -619,6 +619,15 @@ const __be32 *of_get_address(struct device_node *dev,
int index, u64 *size,
}
EXPORT_SYMBOL(of_get_address);
+int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
+{
+#ifndef PCI_IOBASE
+ return -EINVAL;
+#else
+ return 0;
+#endif
+}
This isn't PCI code, so I'm fine with it in that sense, but I'm not
sure the idea of a PCI_IOBASE #define is really what we need. It's
not really determined by the processor architecture, it's determined
by the platform. And a single address isn't enough in general,
either, because if there are multiple host bridges, there's no reason
the apertures that generate PCI I/O transactions need to be contiguous
on the CPU side.
That's just a long way of saying that if we ever came up with a more
generic way to handle I/O port spaces, PCI_IOBASE might go away. And
I guess part of that rework could be changing this use of it along
with the others.
I'd rather not add a generic implementation of this at all, but
keep it all within the host resource scanning code.
If we do add a generic implementation, my preference would be
to use the version introduced for arm64, with a fallback of
returning -EINVAL if the architecture doesn't implement it.
There is no way ever that returning '0' makes sense here: Either
the architecture supports memory mapped I/O spaces and then we
should be able to find an appropriate io_offset for it, or it
doesn't support memory mapped I/O spaces and we should never
even call this function.
Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Tue, Apr 08, 2014 at 12:21:51AM +0100, Bjorn Helgaas wrote:
On Fri, Mar 14, 2014 at 9:34 AM, Liviu Dudau liviu.du...@arm.com wrote:
Some architectures do not share x86 simple view of the PCI I/O space
and instead use a range of addresses that map to bus addresses. For
some architectures these ranges will be expressed by OF bindings
in a device tree file.
Introduce a pci_register_io_range() helper function that can be used
by the architecture code to keep track of the I/O ranges described by the
PCI bindings. If the PCI_IOBASE macro is not defined that signals
lack of support for PCI and we return an error.
Signed-off-by: Liviu Dudau liviu.du...@arm.com
Acked-by: Grant Likely grant.lik...@linaro.org
Tested-by: Tanmay Inamdar tinam...@apm.com
---
drivers/of/address.c | 9 +
include/linux/of_address.h | 1 +
2 files changed, 10 insertions(+)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 1a54f1f..be958ed 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -619,6 +619,15 @@ const __be32 *of_get_address(struct device_node *dev,
int index, u64 *size,
}
EXPORT_SYMBOL(of_get_address);
+int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
+{
+#ifndef PCI_IOBASE
+ return -EINVAL;
+#else
+ return 0;
+#endif
+}
This isn't PCI code, so I'm fine with it in that sense, but I'm not
sure the idea of a PCI_IOBASE #define is really what we need. It's
not really determined by the processor architecture, it's determined
by the platform. And a single address isn't enough in general,
either, because if there are multiple host bridges, there's no reason
the apertures that generate PCI I/O transactions need to be contiguous
on the CPU side.
It should not be only platform's choice if the architecture doesn't support it.
To my mind PCI_IOBASE means I support MMIO operations and this is the
start of the virtual address where my I/O ranges are mapped. It's the
same as ppc's _IO_BASE. And pci_address_to_pio() will take care to give
you the correct io_offset in the presence of multiple host bridges,
while keeping the io resource in the range [0 .. host_bridge_io_range_size - 1]
That's just a long way of saying that if we ever came up with a more
generic way to handle I/O port spaces, PCI_IOBASE might go away. And
I guess part of that rework could be changing this use of it along
with the others.
And I have a patch series that #defines PCI_IOBASE only in those architectures
that support MMIO, where this macro makes sense. Also notice that the
arm64 series has a patch that I'm going to roll into this one where ioport_map()
gets fixed to include PCI_IOBASE when !CONFIG_GENERIC_MAP.
Best regards,
Liviu
unsigned long __weak pci_address_to_pio(phys_addr_t address)
{
if (address IO_SPACE_LIMIT)
diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index 5f6ed6b..40c418d 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h
@@ -56,6 +56,7 @@ extern void __iomem *of_iomap(struct device_node *device,
int index);
extern const __be32 *of_get_address(struct device_node *dev, int index,
u64 *size, unsigned int *flags);
+extern int pci_register_io_range(phys_addr_t addr, resource_size_t size);
extern unsigned long pci_address_to_pio(phys_addr_t addr);
extern int of_pci_range_parser_init(struct of_pci_range_parser *parser,
--
1.9.0
--
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---
¯\_(ツ)_/¯
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Mon, Apr 07, 2014 at 06:58:24PM +0100, Bjorn Helgaas wrote: On Mon, Apr 7, 2014 at 5:36 AM, Arnd Bergmann a...@arndb.de wrote: I think migrating other architectures to use the same code should be a separate effort from adding a generic implementation that can be used by arm64. It's probably a good idea to have patches to convert arm32 and/or microblaze. Let me reiterate that I am 100% in favor of replacing arch-specific code with more generic implementations. However, I am not 100% in favor of doing it as separate efforts (although maybe I could be convinced). The reasons I hesitate are that (1) if only one architecture uses a new generic implementation, we really don't know whether it is generic enough, (2) until I see the patches to convert other architectures, I have to assume that I'm the one who will write them, and (3) as soon as we add the code to drivers/pci, it becomes partly my headache to maintain it, even if only one arch benefits from it. Please don't think I'm questioning anyone's intent or good will. It's just that I understand the business pressures, and I know how hard it can be to justify this sort of work to one's management, especially after the immediate problem has been solved. I understand your concern. I guess the only way to prove my good intentions is to shut up and show the code. Liviu Bjorn -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/¯ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Tuesday 08 April 2014 10:49:33 Liviu Dudau wrote:
On Tue, Apr 08, 2014 at 12:21:51AM +0100, Bjorn Helgaas wrote:
On Fri, Mar 14, 2014 at 9:34 AM, Liviu Dudau liviu.du...@arm.com wrote:
Some architectures do not share x86 simple view of the PCI I/O space
and instead use a range of addresses that map to bus addresses. For
some architectures these ranges will be expressed by OF bindings
in a device tree file.
Introduce a pci_register_io_range() helper function that can be used
by the architecture code to keep track of the I/O ranges described by the
PCI bindings. If the PCI_IOBASE macro is not defined that signals
lack of support for PCI and we return an error.
Signed-off-by: Liviu Dudau liviu.du...@arm.com
Acked-by: Grant Likely grant.lik...@linaro.org
Tested-by: Tanmay Inamdar tinam...@apm.com
---
drivers/of/address.c | 9 +
include/linux/of_address.h | 1 +
2 files changed, 10 insertions(+)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 1a54f1f..be958ed 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -619,6 +619,15 @@ const __be32 *of_get_address(struct device_node
*dev, int index, u64 *size,
}
EXPORT_SYMBOL(of_get_address);
+int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
+{
+#ifndef PCI_IOBASE
+ return -EINVAL;
+#else
+ return 0;
+#endif
+}
This isn't PCI code, so I'm fine with it in that sense, but I'm not
sure the idea of a PCI_IOBASE #define is really what we need. It's
not really determined by the processor architecture, it's determined
by the platform. And a single address isn't enough in general,
either, because if there are multiple host bridges, there's no reason
the apertures that generate PCI I/O transactions need to be contiguous
on the CPU side.
It should not be only platform's choice if the architecture doesn't support
it.
To my mind PCI_IOBASE means I support MMIO operations and this is the
start of the virtual address where my I/O ranges are mapped. It's the
same as ppc's _IO_BASE. And pci_address_to_pio() will take care to give
you the correct io_offset in the presence of multiple host bridges,
while keeping the io resource in the range [0 .. host_bridge_io_range_size -
1]
There is a wide range of implementations across architectures:
a) no access to I/O ports at all (tile, s390, ...)
b) access to I/O ports only through special instructions (x86, ...)
c) all MMIO is mapped virtual contiguous to PCI_IOBASE or _IO_BASE
(most ppc64, arm32 with MMU, arm64, ...)
d) only one PCI host can have an I/O space (mips, microblaze, ...)
e) each host controller can have its own method (ppc64 with indirect pio)
f) PIO token equals virtual address plus offset (some legacy ARM platforms,
probably some other architectures), or physical address (sparc)
g) PIO token encodes address space number plus offset (ia64)
a) and b) are trivially handled by any implementation that falls
back to 'return -EINVAL'.
I believe that c) is the most appropriate solution and we should be
able to adopt it by most of the architectures that have an MMU and
make it the default implementation.
d) seems like a good fallback for noMMU architectures: While we do
need to support I/O spaces, we need to support multiple PCI domains,
and we need to support noMMU, the combination of all three should
be extremely rare, and I'd leave it up to the architecture to support
that if there is a real use case, rather than trying to put that into
common code.
Anything that currently requires e), f) or g) I think should keep
doing that and not try to use the generic implementation.
Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Tuesday 08 April 2014 10:50:39 Liviu Dudau wrote: On Mon, Apr 07, 2014 at 06:58:24PM +0100, Bjorn Helgaas wrote: On Mon, Apr 7, 2014 at 5:36 AM, Arnd Bergmann a...@arndb.de wrote: I think migrating other architectures to use the same code should be a separate effort from adding a generic implementation that can be used by arm64. It's probably a good idea to have patches to convert arm32 and/or microblaze. Let me reiterate that I am 100% in favor of replacing arch-specific code with more generic implementations. However, I am not 100% in favor of doing it as separate efforts (although maybe I could be convinced). The reasons I hesitate are that (1) if only one architecture uses a new generic implementation, we really don't know whether it is generic enough, (2) until I see the patches to convert other architectures, I have to assume that I'm the one who will write them, and (3) as soon as we add the code to drivers/pci, it becomes partly my headache to maintain it, even if only one arch benefits from it. Fair enough. My approach to the asm-generic infrastruction has mostly been to ensure that whoever adds a new architecture has to make things easier for the next person. For the PCI code it's clearly your call to pick whatever works best for you. Please don't think I'm questioning anyone's intent or good will. It's just that I understand the business pressures, and I know how hard it can be to justify this sort of work to one's management, especially after the immediate problem has been solved. I understand your concern. I guess the only way to prove my good intentions is to shut up and show the code. I'd suggest looking at architectures in this order then: * microblaze (this one is easy and wants to share code with us) * arm32-multiplatform (obviously interesting, but not as easy as microblaze) * powerpc64 (they are keen on sharing, code is similar to what you have) * mips (this is really platform specific, some want to share drivers with arm32, others should keep their current code. Note that platform selection on mips is compile-time only, they don't have to do it all the same way) * powerpc32 (their code is currently different, might not be worth it) My preference would be to have only the first two done initially and leave the other ones up to architecture maintainers, but Bjorn should say how much he wants to see get done. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Tue, Apr 8, 2014 at 4:11 AM, Arnd Bergmann a...@arndb.de wrote: There is a wide range of implementations across architectures: a) no access to I/O ports at all (tile, s390, ...) b) access to I/O ports only through special instructions (x86, ...) c) all MMIO is mapped virtual contiguous to PCI_IOBASE or _IO_BASE (most ppc64, arm32 with MMU, arm64, ...) d) only one PCI host can have an I/O space (mips, microblaze, ...) e) each host controller can have its own method (ppc64 with indirect pio) f) PIO token equals virtual address plus offset (some legacy ARM platforms, probably some other architectures), or physical address (sparc) g) PIO token encodes address space number plus offset (ia64) a) and b) are trivially handled by any implementation that falls back to 'return -EINVAL'. I believe that c) is the most appropriate solution and we should be able to adopt it by most of the architectures that have an MMU and make it the default implementation. d) seems like a good fallback for noMMU architectures: While we do need to support I/O spaces, we need to support multiple PCI domains, and we need to support noMMU, the combination of all three should be extremely rare, and I'd leave it up to the architecture to support that if there is a real use case, rather than trying to put that into common code. Anything that currently requires e), f) or g) I think should keep doing that and not try to use the generic implementation. Thanks for the nice summary. That's way more than I had figured out myself. I don't know whether it'd be worth it, especially for something that's so close to obsolete, but it seems like it should be *possible* to generalize and unify this somewhat. I would argue that g) (which I wrote, so I know it better than the others) could fairly easily subsume c), d), and f), since it maps an ioport number to a virtual address for an MMIO access, but it doesn't assume that all the MMIO spaces are contiguous. b), e), and maybe a) could be handled with an exception, e.g., inside inb(), look up the struct io_space (e.g., similar to what ia64 does in __ia64_mk_io_addr()), and if that struct contains a non-zero ops pointer, use that instead of doing the MMIO access. The ops pointer functions could use the x86 INB instruction or do the indirect PIO thing or whatever. Bjorn -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Tue, Apr 8, 2014 at 4:22 AM, Arnd Bergmann a...@arndb.de wrote: On Tuesday 08 April 2014 10:50:39 Liviu Dudau wrote: On Mon, Apr 07, 2014 at 06:58:24PM +0100, Bjorn Helgaas wrote: On Mon, Apr 7, 2014 at 5:36 AM, Arnd Bergmann a...@arndb.de wrote: I think migrating other architectures to use the same code should be a separate effort from adding a generic implementation that can be used by arm64. It's probably a good idea to have patches to convert arm32 and/or microblaze. Let me reiterate that I am 100% in favor of replacing arch-specific code with more generic implementations. However, I am not 100% in favor of doing it as separate efforts (although maybe I could be convinced). The reasons I hesitate are that (1) if only one architecture uses a new generic implementation, we really don't know whether it is generic enough, (2) until I see the patches to convert other architectures, I have to assume that I'm the one who will write them, and (3) as soon as we add the code to drivers/pci, it becomes partly my headache to maintain it, even if only one arch benefits from it. Fair enough. My approach to the asm-generic infrastruction has mostly been to ensure that whoever adds a new architecture has to make things easier for the next person. That's a good rule. But if we add a generic implementation used only by one architecture, the overall complexity has increased (we added new unshared code), so the next person has to look at N+1 existing implementations. If we even convert one existing arch, that seems like an improvement: we have N implementations with one being used by at least two arches. Bjorn -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Fri, Mar 14, 2014 at 9:34 AM, Liviu Dudau wrote:
> Some architectures do not share x86 simple view of the PCI I/O space
> and instead use a range of addresses that map to bus addresses. For
> some architectures these ranges will be expressed by OF bindings
> in a device tree file.
>
> Introduce a pci_register_io_range() helper function that can be used
> by the architecture code to keep track of the I/O ranges described by the
> PCI bindings. If the PCI_IOBASE macro is not defined that signals
> lack of support for PCI and we return an error.
>
> Signed-off-by: Liviu Dudau
> Acked-by: Grant Likely
> Tested-by: Tanmay Inamdar
> ---
> drivers/of/address.c | 9 +
> include/linux/of_address.h | 1 +
> 2 files changed, 10 insertions(+)
>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 1a54f1f..be958ed 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -619,6 +619,15 @@ const __be32 *of_get_address(struct device_node *dev,
> int index, u64 *size,
> }
> EXPORT_SYMBOL(of_get_address);
>
> +int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> +{
> +#ifndef PCI_IOBASE
> + return -EINVAL;
> +#else
> + return 0;
> +#endif
> +}
This isn't PCI code, so I'm fine with it in that sense, but I'm not
sure the idea of a PCI_IOBASE #define is really what we need. It's
not really determined by the processor architecture, it's determined
by the platform. And a single address isn't enough in general,
either, because if there are multiple host bridges, there's no reason
the apertures that generate PCI I/O transactions need to be contiguous
on the CPU side.
That's just a long way of saying that if we ever came up with a more
generic way to handle I/O port spaces, PCI_IOBASE might go away. And
I guess part of that rework could be changing this use of it along
with the others.
> unsigned long __weak pci_address_to_pio(phys_addr_t address)
> {
> if (address > IO_SPACE_LIMIT)
> diff --git a/include/linux/of_address.h b/include/linux/of_address.h
> index 5f6ed6b..40c418d 100644
> --- a/include/linux/of_address.h
> +++ b/include/linux/of_address.h
> @@ -56,6 +56,7 @@ extern void __iomem *of_iomap(struct device_node *device,
> int index);
> extern const __be32 *of_get_address(struct device_node *dev, int index,
>u64 *size, unsigned int *flags);
>
> +extern int pci_register_io_range(phys_addr_t addr, resource_size_t size);
> extern unsigned long pci_address_to_pio(phys_addr_t addr);
>
> extern int of_pci_range_parser_init(struct of_pci_range_parser *parser,
> --
> 1.9.0
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Mon, Apr 7, 2014 at 5:36 AM, Arnd Bergmann wrote: > I think migrating other architectures to use the same code should be > a separate effort from adding a generic implementation that can be > used by arm64. It's probably a good idea to have patches to convert > arm32 and/or microblaze. Let me reiterate that I am 100% in favor of replacing arch-specific code with more generic implementations. However, I am not 100% in favor of doing it as separate efforts (although maybe I could be convinced). The reasons I hesitate are that (1) if only one architecture uses a new "generic" implementation, we really don't know whether it is generic enough, (2) until I see the patches to convert other architectures, I have to assume that I'm the one who will write them, and (3) as soon as we add the code to drivers/pci, it becomes partly my headache to maintain it, even if only one arch benefits from it. Please don't think I'm questioning anyone's intent or good will. It's just that I understand the business pressures, and I know how hard it can be to justify this sort of work to one's management, especially after the immediate problem has been solved. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Mon, Apr 07, 2014 at 12:36:15PM +0100, Arnd Bergmann wrote: > On Monday 07 April 2014 09:31:20 Liviu Dudau wrote: > > On Sat, Apr 05, 2014 at 01:19:53AM +0100, Bjorn Helgaas wrote: > > > > > Host bridges on x86 could have MMIO apertures that turn CPU memory > > > accesses > > > into PCI port accesses. We could implement any number of I/O port spaces > > > this way, by making the kernel inb()/outb()/etc. interfaces smart enough > > > to > > > use the memory-mapped space instead of (or in addition to) the > > > INB/OUTB/etc. instructions. > > PowerPC actually has this already, as CONFIG_PPC_INDIRECT_PIO meaning that > access to PIO registers is bus specific, and there is also > CONFIG_PPC_INDIRECT_MMIO > for the case where MMIO access is not native. > > > Right, sorry for my ignorance then: how does *currently* the device driver > > do > > the I/O transfer transparent of the implementation mechanism? Or they have > > intimate knowledge of wether the device is behind a host bridge and can do > > MMIO > > or is on an ISA or CF bus and then it needs INB/OUTB ? And if we make > > inb/outb > > smarter, does that mean that we need to change the drivers? > > The idea of that would be to not change drivers. > > My preference here would be to only have a generic function for those > architectures that have the simple MMIO access all the time. > > > > ia64 does this (see arch/ia64/include/asm/io.h for a little description) > > > and I think maybe one or two other arches have something similar. > > > > > > > Introduce a pci_register_io_range() helper function that can be used > > > > by the architecture code to keep track of the I/O ranges described by > > > > the > > > > PCI bindings. If the PCI_IOBASE macro is not defined that signals > > > > lack of support for PCI and we return an error. > > > > > > I don't quite see how you intend to use this, because this series doesn't > > > include any non-stub implementation of pci_register_io_range(). > > > > > > Is this anything like the ia64 strategy I mentioned above? If so, it > > > would > > > be really nice to unify some of this stuff. > > > > After discussions with Arnd and Catalin I know have a new series that moves > > some of the code from arm64 series into this one. I am putting it through > > testing right know as I am going to have to depend on another series that > > makes PCI_IOBASE defined only for architectures that do MMIO in order to > > choose the correct default implementation for these functions. My hope is > > that I will be able to send the series this week. > > I think migrating other architectures to use the same code should be > a separate effort from adding a generic implementation that can be > used by arm64. It's probably a good idea to have patches to convert > arm32 and/or microblaze. Agree. My updated series only moves the arm64 code into framework to make the arm64 part a noop. Liviu > > Arnd > > -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/¯ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Monday 07 April 2014 09:31:20 Liviu Dudau wrote: > On Sat, Apr 05, 2014 at 01:19:53AM +0100, Bjorn Helgaas wrote: > > > Host bridges on x86 could have MMIO apertures that turn CPU memory accesses > > into PCI port accesses. We could implement any number of I/O port spaces > > this way, by making the kernel inb()/outb()/etc. interfaces smart enough to > > use the memory-mapped space instead of (or in addition to) the > > INB/OUTB/etc. instructions. PowerPC actually has this already, as CONFIG_PPC_INDIRECT_PIO meaning that access to PIO registers is bus specific, and there is also CONFIG_PPC_INDIRECT_MMIO for the case where MMIO access is not native. > Right, sorry for my ignorance then: how does *currently* the device driver do > the I/O transfer transparent of the implementation mechanism? Or they have > intimate knowledge of wether the device is behind a host bridge and can do > MMIO > or is on an ISA or CF bus and then it needs INB/OUTB ? And if we make inb/outb > smarter, does that mean that we need to change the drivers? The idea of that would be to not change drivers. My preference here would be to only have a generic function for those architectures that have the simple MMIO access all the time. > > ia64 does this (see arch/ia64/include/asm/io.h for a little description) > > and I think maybe one or two other arches have something similar. > > > > > Introduce a pci_register_io_range() helper function that can be used > > > by the architecture code to keep track of the I/O ranges described by the > > > PCI bindings. If the PCI_IOBASE macro is not defined that signals > > > lack of support for PCI and we return an error. > > > > I don't quite see how you intend to use this, because this series doesn't > > include any non-stub implementation of pci_register_io_range(). > > > > Is this anything like the ia64 strategy I mentioned above? If so, it would > > be really nice to unify some of this stuff. > > After discussions with Arnd and Catalin I know have a new series that moves > some of the code from arm64 series into this one. I am putting it through > testing right know as I am going to have to depend on another series that > makes PCI_IOBASE defined only for architectures that do MMIO in order to > choose the correct default implementation for these functions. My hope is > that I will be able to send the series this week. I think migrating other architectures to use the same code should be a separate effort from adding a generic implementation that can be used by arm64. It's probably a good idea to have patches to convert arm32 and/or microblaze. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Monday 07 April 2014 19:13:28 Benjamin Herrenschmidt wrote: > On Mon, 2014-04-07 at 09:35 +0100, Liviu Dudau wrote: > > Thanks for the summary, is really useful as I was recently looking > > into code in that > > area. One thing I was trying to understand is why ppc needs _IO_BASE > > at all rather > > than using the generic PCI_IOBASE? > > Perhaps because our code predates it ? I haven't looked much into > the semantics of PCI_IOBASE yet... Yes, I'm pretty sure that's all there is to it. PCI_IOBASE just happened to be an identifier we picked for asm-generic, but the use on PowerPC is much older than the generic file. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Mon, 2014-04-07 at 09:35 +0100, Liviu Dudau wrote: > Thanks for the summary, is really useful as I was recently looking > into code in that > area. One thing I was trying to understand is why ppc needs _IO_BASE > at all rather > than using the generic PCI_IOBASE? Perhaps because our code predates it ? :-) I haven't looked much into the semantics of PCI_IOBASE yet... Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Sun, Apr 06, 2014 at 10:49:53AM +0100, Benjamin Herrenschmidt wrote: > On Fri, 2014-04-04 at 18:19 -0600, Bjorn Helgaas wrote: > > > Introduce a pci_register_io_range() helper function that can be used > > > by the architecture code to keep track of the I/O ranges described by the > > > PCI bindings. If the PCI_IOBASE macro is not defined that signals > > > lack of support for PCI and we return an error. > > > > I don't quite see how you intend to use this, because this series doesn't > > include any non-stub implementation of pci_register_io_range(). > > > > Is this anything like the ia64 strategy I mentioned above? If so, it would > > be really nice to unify some of this stuff. > > We also use two different strategies on ppc32 and ppc64 > > - On ppc32, inb/outb turn into an MMIO access to _IO_BASE + port > > That _IO_BASE is a variable which is initialized to the ioremapped address > of the IO space MMIO aperture of the first bridge we discover. Then port > numbers are "fixed up" on all other bridges so that the addition _IO_BASE + > port > fits the ioremapped address of the IO space on that bridge. A bit messy... > and breaks > whenever drivers copy port numbers into variables of the wrong type such as > shorts. > > - On ppc64, we have more virtual space, so instead we reserve a range > of address space (fixed) for IO space, it's always the same. Bridges IO spaces > are then mapped into that range, so we always have a positive offset from > _IO_BASE > which makes things a bit more robust and less "surprising" than ppc32. > Additionally, > the first 64k are reserved. They are only mapped if we see an ISA bridge > (which some > older machines have). Otherwise it's left unmapped, so crappy drivers trying > to > hard code x86 IO ports will blow up immediately which I deem better than > silently > whacking the wrong hardware. In addition, we have a mechanism we use on > powernv to > re-route accesses to that first 64k to the power8 built-in LPC bus which can > have some legacy IOs on it such as a UART or a RTC. > > Cheers, > Ben. > Hi Benjamin, Thanks for the summary, is really useful as I was recently looking into code in that area. One thing I was trying to understand is why ppc needs _IO_BASE at all rather than using the generic PCI_IOBASE? Best regards, Liviu -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/¯ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Sat, Apr 05, 2014 at 01:19:53AM +0100, Bjorn Helgaas wrote:
> On Fri, Mar 14, 2014 at 03:34:27PM +, Liviu Dudau wrote:
> > Some architectures do not share x86 simple view of the PCI I/O space
> > and instead use a range of addresses that map to bus addresses. For
> > some architectures these ranges will be expressed by OF bindings
> > in a device tree file.
>
> It's true that the current Linux "x86 view of PCI I/O space" is pretty
> simple and limited. But I don't think that's a fundamental x86 limitation
> (other than the fact that the actual INB/OUTB/etc. CPU instructions
> themselves are limited to a single 64K I/O port space).
Hi Bjorn,
Thanks for reviewing this series.
I might've taken a too dim view of x86 world. I tend to split the existing
architectures into the ones that have special I/O instructions and the ones
that map a region of memory into CPU space and do I/O transactions there as
simple read/writes.
>
> Host bridges on x86 could have MMIO apertures that turn CPU memory accesses
> into PCI port accesses. We could implement any number of I/O port spaces
> this way, by making the kernel inb()/outb()/etc. interfaces smart enough to
> use the memory-mapped space instead of (or in addition to) the
> INB/OUTB/etc. instructions.
Right, sorry for my ignorance then: how does *currently* the device driver do
the I/O transfer transparent of the implementation mechanism? Or they have
intimate knowledge of wether the device is behind a host bridge and can do MMIO
or is on an ISA or CF bus and then it needs INB/OUTB ? And if we make inb/outb
smarter, does that mean that we need to change the drivers?
>
> ia64 does this (see arch/ia64/include/asm/io.h for a little description)
> and I think maybe one or two other arches have something similar.
>
> > Introduce a pci_register_io_range() helper function that can be used
> > by the architecture code to keep track of the I/O ranges described by the
> > PCI bindings. If the PCI_IOBASE macro is not defined that signals
> > lack of support for PCI and we return an error.
>
> I don't quite see how you intend to use this, because this series doesn't
> include any non-stub implementation of pci_register_io_range().
>
> Is this anything like the ia64 strategy I mentioned above? If so, it would
> be really nice to unify some of this stuff.
After discussions with Arnd and Catalin I know have a new series that moves
some of the code from arm64 series into this one. I am putting it through
testing right know as I am going to have to depend on another series that
makes PCI_IOBASE defined only for architectures that do MMIO in order to
choose the correct default implementation for these functions. My hope is
that I will be able to send the series this week.
Best regards,
Liviu
>
> > Signed-off-by: Liviu Dudau
> > Acked-by: Grant Likely
> > Tested-by: Tanmay Inamdar
> > ---
> > drivers/of/address.c | 9 +
> > include/linux/of_address.h | 1 +
> > 2 files changed, 10 insertions(+)
> >
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index 1a54f1f..be958ed 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -619,6 +619,15 @@ const __be32 *of_get_address(struct device_node *dev,
> > int index, u64 *size,
> > }
> > EXPORT_SYMBOL(of_get_address);
> >
> > +int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> > +{
> > +#ifndef PCI_IOBASE
> > + return -EINVAL;
> > +#else
> > + return 0;
> > +#endif
> > +}
> > +
> > unsigned long __weak pci_address_to_pio(phys_addr_t address)
> > {
> > if (address > IO_SPACE_LIMIT)
> > diff --git a/include/linux/of_address.h b/include/linux/of_address.h
> > index 5f6ed6b..40c418d 100644
> > --- a/include/linux/of_address.h
> > +++ b/include/linux/of_address.h
> > @@ -56,6 +56,7 @@ extern void __iomem *of_iomap(struct device_node *device,
> > int index);
> > extern const __be32 *of_get_address(struct device_node *dev, int index,
> >u64 *size, unsigned int *flags);
> >
> > +extern int pci_register_io_range(phys_addr_t addr, resource_size_t size);
> > extern unsigned long pci_address_to_pio(phys_addr_t addr);
> >
> > extern int of_pci_range_parser_init(struct of_pci_range_parser *parser,
> > --
> > 1.9.0
> >
>
--
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---
¯\_(ツ)_/¯
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Sat, Apr 05, 2014 at 01:19:53AM +0100, Bjorn Helgaas wrote:
On Fri, Mar 14, 2014 at 03:34:27PM +, Liviu Dudau wrote:
Some architectures do not share x86 simple view of the PCI I/O space
and instead use a range of addresses that map to bus addresses. For
some architectures these ranges will be expressed by OF bindings
in a device tree file.
It's true that the current Linux x86 view of PCI I/O space is pretty
simple and limited. But I don't think that's a fundamental x86 limitation
(other than the fact that the actual INB/OUTB/etc. CPU instructions
themselves are limited to a single 64K I/O port space).
Hi Bjorn,
Thanks for reviewing this series.
I might've taken a too dim view of x86 world. I tend to split the existing
architectures into the ones that have special I/O instructions and the ones
that map a region of memory into CPU space and do I/O transactions there as
simple read/writes.
Host bridges on x86 could have MMIO apertures that turn CPU memory accesses
into PCI port accesses. We could implement any number of I/O port spaces
this way, by making the kernel inb()/outb()/etc. interfaces smart enough to
use the memory-mapped space instead of (or in addition to) the
INB/OUTB/etc. instructions.
Right, sorry for my ignorance then: how does *currently* the device driver do
the I/O transfer transparent of the implementation mechanism? Or they have
intimate knowledge of wether the device is behind a host bridge and can do MMIO
or is on an ISA or CF bus and then it needs INB/OUTB ? And if we make inb/outb
smarter, does that mean that we need to change the drivers?
ia64 does this (see arch/ia64/include/asm/io.h for a little description)
and I think maybe one or two other arches have something similar.
Introduce a pci_register_io_range() helper function that can be used
by the architecture code to keep track of the I/O ranges described by the
PCI bindings. If the PCI_IOBASE macro is not defined that signals
lack of support for PCI and we return an error.
I don't quite see how you intend to use this, because this series doesn't
include any non-stub implementation of pci_register_io_range().
Is this anything like the ia64 strategy I mentioned above? If so, it would
be really nice to unify some of this stuff.
After discussions with Arnd and Catalin I know have a new series that moves
some of the code from arm64 series into this one. I am putting it through
testing right know as I am going to have to depend on another series that
makes PCI_IOBASE defined only for architectures that do MMIO in order to
choose the correct default implementation for these functions. My hope is
that I will be able to send the series this week.
Best regards,
Liviu
Signed-off-by: Liviu Dudau liviu.du...@arm.com
Acked-by: Grant Likely grant.lik...@linaro.org
Tested-by: Tanmay Inamdar tinam...@apm.com
---
drivers/of/address.c | 9 +
include/linux/of_address.h | 1 +
2 files changed, 10 insertions(+)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 1a54f1f..be958ed 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -619,6 +619,15 @@ const __be32 *of_get_address(struct device_node *dev,
int index, u64 *size,
}
EXPORT_SYMBOL(of_get_address);
+int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
+{
+#ifndef PCI_IOBASE
+ return -EINVAL;
+#else
+ return 0;
+#endif
+}
+
unsigned long __weak pci_address_to_pio(phys_addr_t address)
{
if (address IO_SPACE_LIMIT)
diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index 5f6ed6b..40c418d 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h
@@ -56,6 +56,7 @@ extern void __iomem *of_iomap(struct device_node *device,
int index);
extern const __be32 *of_get_address(struct device_node *dev, int index,
u64 *size, unsigned int *flags);
+extern int pci_register_io_range(phys_addr_t addr, resource_size_t size);
extern unsigned long pci_address_to_pio(phys_addr_t addr);
extern int of_pci_range_parser_init(struct of_pci_range_parser *parser,
--
1.9.0
--
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---
¯\_(ツ)_/¯
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Sun, Apr 06, 2014 at 10:49:53AM +0100, Benjamin Herrenschmidt wrote: On Fri, 2014-04-04 at 18:19 -0600, Bjorn Helgaas wrote: Introduce a pci_register_io_range() helper function that can be used by the architecture code to keep track of the I/O ranges described by the PCI bindings. If the PCI_IOBASE macro is not defined that signals lack of support for PCI and we return an error. I don't quite see how you intend to use this, because this series doesn't include any non-stub implementation of pci_register_io_range(). Is this anything like the ia64 strategy I mentioned above? If so, it would be really nice to unify some of this stuff. We also use two different strategies on ppc32 and ppc64 - On ppc32, inb/outb turn into an MMIO access to _IO_BASE + port That _IO_BASE is a variable which is initialized to the ioremapped address of the IO space MMIO aperture of the first bridge we discover. Then port numbers are fixed up on all other bridges so that the addition _IO_BASE + port fits the ioremapped address of the IO space on that bridge. A bit messy... and breaks whenever drivers copy port numbers into variables of the wrong type such as shorts. - On ppc64, we have more virtual space, so instead we reserve a range of address space (fixed) for IO space, it's always the same. Bridges IO spaces are then mapped into that range, so we always have a positive offset from _IO_BASE which makes things a bit more robust and less surprising than ppc32. Additionally, the first 64k are reserved. They are only mapped if we see an ISA bridge (which some older machines have). Otherwise it's left unmapped, so crappy drivers trying to hard code x86 IO ports will blow up immediately which I deem better than silently whacking the wrong hardware. In addition, we have a mechanism we use on powernv to re-route accesses to that first 64k to the power8 built-in LPC bus which can have some legacy IOs on it such as a UART or a RTC. Cheers, Ben. Hi Benjamin, Thanks for the summary, is really useful as I was recently looking into code in that area. One thing I was trying to understand is why ppc needs _IO_BASE at all rather than using the generic PCI_IOBASE? Best regards, Liviu -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/¯ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Mon, 2014-04-07 at 09:35 +0100, Liviu Dudau wrote: Thanks for the summary, is really useful as I was recently looking into code in that area. One thing I was trying to understand is why ppc needs _IO_BASE at all rather than using the generic PCI_IOBASE? Perhaps because our code predates it ? :-) I haven't looked much into the semantics of PCI_IOBASE yet... Cheers, Ben. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Monday 07 April 2014 19:13:28 Benjamin Herrenschmidt wrote: On Mon, 2014-04-07 at 09:35 +0100, Liviu Dudau wrote: Thanks for the summary, is really useful as I was recently looking into code in that area. One thing I was trying to understand is why ppc needs _IO_BASE at all rather than using the generic PCI_IOBASE? Perhaps because our code predates it ? I haven't looked much into the semantics of PCI_IOBASE yet... Yes, I'm pretty sure that's all there is to it. PCI_IOBASE just happened to be an identifier we picked for asm-generic, but the use on PowerPC is much older than the generic file. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Monday 07 April 2014 09:31:20 Liviu Dudau wrote: On Sat, Apr 05, 2014 at 01:19:53AM +0100, Bjorn Helgaas wrote: Host bridges on x86 could have MMIO apertures that turn CPU memory accesses into PCI port accesses. We could implement any number of I/O port spaces this way, by making the kernel inb()/outb()/etc. interfaces smart enough to use the memory-mapped space instead of (or in addition to) the INB/OUTB/etc. instructions. PowerPC actually has this already, as CONFIG_PPC_INDIRECT_PIO meaning that access to PIO registers is bus specific, and there is also CONFIG_PPC_INDIRECT_MMIO for the case where MMIO access is not native. Right, sorry for my ignorance then: how does *currently* the device driver do the I/O transfer transparent of the implementation mechanism? Or they have intimate knowledge of wether the device is behind a host bridge and can do MMIO or is on an ISA or CF bus and then it needs INB/OUTB ? And if we make inb/outb smarter, does that mean that we need to change the drivers? The idea of that would be to not change drivers. My preference here would be to only have a generic function for those architectures that have the simple MMIO access all the time. ia64 does this (see arch/ia64/include/asm/io.h for a little description) and I think maybe one or two other arches have something similar. Introduce a pci_register_io_range() helper function that can be used by the architecture code to keep track of the I/O ranges described by the PCI bindings. If the PCI_IOBASE macro is not defined that signals lack of support for PCI and we return an error. I don't quite see how you intend to use this, because this series doesn't include any non-stub implementation of pci_register_io_range(). Is this anything like the ia64 strategy I mentioned above? If so, it would be really nice to unify some of this stuff. After discussions with Arnd and Catalin I know have a new series that moves some of the code from arm64 series into this one. I am putting it through testing right know as I am going to have to depend on another series that makes PCI_IOBASE defined only for architectures that do MMIO in order to choose the correct default implementation for these functions. My hope is that I will be able to send the series this week. I think migrating other architectures to use the same code should be a separate effort from adding a generic implementation that can be used by arm64. It's probably a good idea to have patches to convert arm32 and/or microblaze. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Mon, Apr 07, 2014 at 12:36:15PM +0100, Arnd Bergmann wrote: On Monday 07 April 2014 09:31:20 Liviu Dudau wrote: On Sat, Apr 05, 2014 at 01:19:53AM +0100, Bjorn Helgaas wrote: Host bridges on x86 could have MMIO apertures that turn CPU memory accesses into PCI port accesses. We could implement any number of I/O port spaces this way, by making the kernel inb()/outb()/etc. interfaces smart enough to use the memory-mapped space instead of (or in addition to) the INB/OUTB/etc. instructions. PowerPC actually has this already, as CONFIG_PPC_INDIRECT_PIO meaning that access to PIO registers is bus specific, and there is also CONFIG_PPC_INDIRECT_MMIO for the case where MMIO access is not native. Right, sorry for my ignorance then: how does *currently* the device driver do the I/O transfer transparent of the implementation mechanism? Or they have intimate knowledge of wether the device is behind a host bridge and can do MMIO or is on an ISA or CF bus and then it needs INB/OUTB ? And if we make inb/outb smarter, does that mean that we need to change the drivers? The idea of that would be to not change drivers. My preference here would be to only have a generic function for those architectures that have the simple MMIO access all the time. ia64 does this (see arch/ia64/include/asm/io.h for a little description) and I think maybe one or two other arches have something similar. Introduce a pci_register_io_range() helper function that can be used by the architecture code to keep track of the I/O ranges described by the PCI bindings. If the PCI_IOBASE macro is not defined that signals lack of support for PCI and we return an error. I don't quite see how you intend to use this, because this series doesn't include any non-stub implementation of pci_register_io_range(). Is this anything like the ia64 strategy I mentioned above? If so, it would be really nice to unify some of this stuff. After discussions with Arnd and Catalin I know have a new series that moves some of the code from arm64 series into this one. I am putting it through testing right know as I am going to have to depend on another series that makes PCI_IOBASE defined only for architectures that do MMIO in order to choose the correct default implementation for these functions. My hope is that I will be able to send the series this week. I think migrating other architectures to use the same code should be a separate effort from adding a generic implementation that can be used by arm64. It's probably a good idea to have patches to convert arm32 and/or microblaze. Agree. My updated series only moves the arm64 code into framework to make the arm64 part a noop. Liviu Arnd -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/¯ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Mon, Apr 7, 2014 at 5:36 AM, Arnd Bergmann a...@arndb.de wrote: I think migrating other architectures to use the same code should be a separate effort from adding a generic implementation that can be used by arm64. It's probably a good idea to have patches to convert arm32 and/or microblaze. Let me reiterate that I am 100% in favor of replacing arch-specific code with more generic implementations. However, I am not 100% in favor of doing it as separate efforts (although maybe I could be convinced). The reasons I hesitate are that (1) if only one architecture uses a new generic implementation, we really don't know whether it is generic enough, (2) until I see the patches to convert other architectures, I have to assume that I'm the one who will write them, and (3) as soon as we add the code to drivers/pci, it becomes partly my headache to maintain it, even if only one arch benefits from it. Please don't think I'm questioning anyone's intent or good will. It's just that I understand the business pressures, and I know how hard it can be to justify this sort of work to one's management, especially after the immediate problem has been solved. Bjorn -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Fri, Mar 14, 2014 at 9:34 AM, Liviu Dudau liviu.du...@arm.com wrote:
Some architectures do not share x86 simple view of the PCI I/O space
and instead use a range of addresses that map to bus addresses. For
some architectures these ranges will be expressed by OF bindings
in a device tree file.
Introduce a pci_register_io_range() helper function that can be used
by the architecture code to keep track of the I/O ranges described by the
PCI bindings. If the PCI_IOBASE macro is not defined that signals
lack of support for PCI and we return an error.
Signed-off-by: Liviu Dudau liviu.du...@arm.com
Acked-by: Grant Likely grant.lik...@linaro.org
Tested-by: Tanmay Inamdar tinam...@apm.com
---
drivers/of/address.c | 9 +
include/linux/of_address.h | 1 +
2 files changed, 10 insertions(+)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 1a54f1f..be958ed 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -619,6 +619,15 @@ const __be32 *of_get_address(struct device_node *dev,
int index, u64 *size,
}
EXPORT_SYMBOL(of_get_address);
+int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
+{
+#ifndef PCI_IOBASE
+ return -EINVAL;
+#else
+ return 0;
+#endif
+}
This isn't PCI code, so I'm fine with it in that sense, but I'm not
sure the idea of a PCI_IOBASE #define is really what we need. It's
not really determined by the processor architecture, it's determined
by the platform. And a single address isn't enough in general,
either, because if there are multiple host bridges, there's no reason
the apertures that generate PCI I/O transactions need to be contiguous
on the CPU side.
That's just a long way of saying that if we ever came up with a more
generic way to handle I/O port spaces, PCI_IOBASE might go away. And
I guess part of that rework could be changing this use of it along
with the others.
unsigned long __weak pci_address_to_pio(phys_addr_t address)
{
if (address IO_SPACE_LIMIT)
diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index 5f6ed6b..40c418d 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h
@@ -56,6 +56,7 @@ extern void __iomem *of_iomap(struct device_node *device,
int index);
extern const __be32 *of_get_address(struct device_node *dev, int index,
u64 *size, unsigned int *flags);
+extern int pci_register_io_range(phys_addr_t addr, resource_size_t size);
extern unsigned long pci_address_to_pio(phys_addr_t addr);
extern int of_pci_range_parser_init(struct of_pci_range_parser *parser,
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Fri, 2014-04-04 at 18:19 -0600, Bjorn Helgaas wrote: > > Introduce a pci_register_io_range() helper function that can be used > > by the architecture code to keep track of the I/O ranges described by the > > PCI bindings. If the PCI_IOBASE macro is not defined that signals > > lack of support for PCI and we return an error. > > I don't quite see how you intend to use this, because this series doesn't > include any non-stub implementation of pci_register_io_range(). > > Is this anything like the ia64 strategy I mentioned above? If so, it would > be really nice to unify some of this stuff. We also use two different strategies on ppc32 and ppc64 - On ppc32, inb/outb turn into an MMIO access to _IO_BASE + port That _IO_BASE is a variable which is initialized to the ioremapped address of the IO space MMIO aperture of the first bridge we discover. Then port numbers are "fixed up" on all other bridges so that the addition _IO_BASE + port fits the ioremapped address of the IO space on that bridge. A bit messy... and breaks whenever drivers copy port numbers into variables of the wrong type such as shorts. - On ppc64, we have more virtual space, so instead we reserve a range of address space (fixed) for IO space, it's always the same. Bridges IO spaces are then mapped into that range, so we always have a positive offset from _IO_BASE which makes things a bit more robust and less "surprising" than ppc32. Additionally, the first 64k are reserved. They are only mapped if we see an ISA bridge (which some older machines have). Otherwise it's left unmapped, so crappy drivers trying to hard code x86 IO ports will blow up immediately which I deem better than silently whacking the wrong hardware. In addition, we have a mechanism we use on powernv to re-route accesses to that first 64k to the power8 built-in LPC bus which can have some legacy IOs on it such as a UART or a RTC. Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Fri, 2014-04-04 at 18:19 -0600, Bjorn Helgaas wrote: Introduce a pci_register_io_range() helper function that can be used by the architecture code to keep track of the I/O ranges described by the PCI bindings. If the PCI_IOBASE macro is not defined that signals lack of support for PCI and we return an error. I don't quite see how you intend to use this, because this series doesn't include any non-stub implementation of pci_register_io_range(). Is this anything like the ia64 strategy I mentioned above? If so, it would be really nice to unify some of this stuff. We also use two different strategies on ppc32 and ppc64 - On ppc32, inb/outb turn into an MMIO access to _IO_BASE + port That _IO_BASE is a variable which is initialized to the ioremapped address of the IO space MMIO aperture of the first bridge we discover. Then port numbers are fixed up on all other bridges so that the addition _IO_BASE + port fits the ioremapped address of the IO space on that bridge. A bit messy... and breaks whenever drivers copy port numbers into variables of the wrong type such as shorts. - On ppc64, we have more virtual space, so instead we reserve a range of address space (fixed) for IO space, it's always the same. Bridges IO spaces are then mapped into that range, so we always have a positive offset from _IO_BASE which makes things a bit more robust and less surprising than ppc32. Additionally, the first 64k are reserved. They are only mapped if we see an ISA bridge (which some older machines have). Otherwise it's left unmapped, so crappy drivers trying to hard code x86 IO ports will blow up immediately which I deem better than silently whacking the wrong hardware. In addition, we have a mechanism we use on powernv to re-route accesses to that first 64k to the power8 built-in LPC bus which can have some legacy IOs on it such as a UART or a RTC. Cheers, Ben. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Fri, Mar 14, 2014 at 03:34:27PM +, Liviu Dudau wrote:
> Some architectures do not share x86 simple view of the PCI I/O space
> and instead use a range of addresses that map to bus addresses. For
> some architectures these ranges will be expressed by OF bindings
> in a device tree file.
It's true that the current Linux "x86 view of PCI I/O space" is pretty
simple and limited. But I don't think that's a fundamental x86 limitation
(other than the fact that the actual INB/OUTB/etc. CPU instructions
themselves are limited to a single 64K I/O port space).
Host bridges on x86 could have MMIO apertures that turn CPU memory accesses
into PCI port accesses. We could implement any number of I/O port spaces
this way, by making the kernel inb()/outb()/etc. interfaces smart enough to
use the memory-mapped space instead of (or in addition to) the
INB/OUTB/etc. instructions.
ia64 does this (see arch/ia64/include/asm/io.h for a little description)
and I think maybe one or two other arches have something similar.
> Introduce a pci_register_io_range() helper function that can be used
> by the architecture code to keep track of the I/O ranges described by the
> PCI bindings. If the PCI_IOBASE macro is not defined that signals
> lack of support for PCI and we return an error.
I don't quite see how you intend to use this, because this series doesn't
include any non-stub implementation of pci_register_io_range().
Is this anything like the ia64 strategy I mentioned above? If so, it would
be really nice to unify some of this stuff.
> Signed-off-by: Liviu Dudau
> Acked-by: Grant Likely
> Tested-by: Tanmay Inamdar
> ---
> drivers/of/address.c | 9 +
> include/linux/of_address.h | 1 +
> 2 files changed, 10 insertions(+)
>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 1a54f1f..be958ed 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -619,6 +619,15 @@ const __be32 *of_get_address(struct device_node *dev,
> int index, u64 *size,
> }
> EXPORT_SYMBOL(of_get_address);
>
> +int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> +{
> +#ifndef PCI_IOBASE
> + return -EINVAL;
> +#else
> + return 0;
> +#endif
> +}
> +
> unsigned long __weak pci_address_to_pio(phys_addr_t address)
> {
> if (address > IO_SPACE_LIMIT)
> diff --git a/include/linux/of_address.h b/include/linux/of_address.h
> index 5f6ed6b..40c418d 100644
> --- a/include/linux/of_address.h
> +++ b/include/linux/of_address.h
> @@ -56,6 +56,7 @@ extern void __iomem *of_iomap(struct device_node *device,
> int index);
> extern const __be32 *of_get_address(struct device_node *dev, int index,
> u64 *size, unsigned int *flags);
>
> +extern int pci_register_io_range(phys_addr_t addr, resource_size_t size);
> extern unsigned long pci_address_to_pio(phys_addr_t addr);
>
> extern int of_pci_range_parser_init(struct of_pci_range_parser *parser,
> --
> 1.9.0
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
On Fri, Mar 14, 2014 at 03:34:27PM +, Liviu Dudau wrote:
Some architectures do not share x86 simple view of the PCI I/O space
and instead use a range of addresses that map to bus addresses. For
some architectures these ranges will be expressed by OF bindings
in a device tree file.
It's true that the current Linux x86 view of PCI I/O space is pretty
simple and limited. But I don't think that's a fundamental x86 limitation
(other than the fact that the actual INB/OUTB/etc. CPU instructions
themselves are limited to a single 64K I/O port space).
Host bridges on x86 could have MMIO apertures that turn CPU memory accesses
into PCI port accesses. We could implement any number of I/O port spaces
this way, by making the kernel inb()/outb()/etc. interfaces smart enough to
use the memory-mapped space instead of (or in addition to) the
INB/OUTB/etc. instructions.
ia64 does this (see arch/ia64/include/asm/io.h for a little description)
and I think maybe one or two other arches have something similar.
Introduce a pci_register_io_range() helper function that can be used
by the architecture code to keep track of the I/O ranges described by the
PCI bindings. If the PCI_IOBASE macro is not defined that signals
lack of support for PCI and we return an error.
I don't quite see how you intend to use this, because this series doesn't
include any non-stub implementation of pci_register_io_range().
Is this anything like the ia64 strategy I mentioned above? If so, it would
be really nice to unify some of this stuff.
Signed-off-by: Liviu Dudau liviu.du...@arm.com
Acked-by: Grant Likely grant.lik...@linaro.org
Tested-by: Tanmay Inamdar tinam...@apm.com
---
drivers/of/address.c | 9 +
include/linux/of_address.h | 1 +
2 files changed, 10 insertions(+)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 1a54f1f..be958ed 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -619,6 +619,15 @@ const __be32 *of_get_address(struct device_node *dev,
int index, u64 *size,
}
EXPORT_SYMBOL(of_get_address);
+int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
+{
+#ifndef PCI_IOBASE
+ return -EINVAL;
+#else
+ return 0;
+#endif
+}
+
unsigned long __weak pci_address_to_pio(phys_addr_t address)
{
if (address IO_SPACE_LIMIT)
diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index 5f6ed6b..40c418d 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h
@@ -56,6 +56,7 @@ extern void __iomem *of_iomap(struct device_node *device,
int index);
extern const __be32 *of_get_address(struct device_node *dev, int index,
u64 *size, unsigned int *flags);
+extern int pci_register_io_range(phys_addr_t addr, resource_size_t size);
extern unsigned long pci_address_to_pio(phys_addr_t addr);
extern int of_pci_range_parser_init(struct of_pci_range_parser *parser,
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
Some architectures do not share x86 simple view of the PCI I/O space
and instead use a range of addresses that map to bus addresses. For
some architectures these ranges will be expressed by OF bindings
in a device tree file.
Introduce a pci_register_io_range() helper function that can be used
by the architecture code to keep track of the I/O ranges described by the
PCI bindings. If the PCI_IOBASE macro is not defined that signals
lack of support for PCI and we return an error.
Signed-off-by: Liviu Dudau
Acked-by: Grant Likely
Tested-by: Tanmay Inamdar
---
drivers/of/address.c | 9 +
include/linux/of_address.h | 1 +
2 files changed, 10 insertions(+)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 1a54f1f..be958ed 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -619,6 +619,15 @@ const __be32 *of_get_address(struct device_node *dev, int
index, u64 *size,
}
EXPORT_SYMBOL(of_get_address);
+int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
+{
+#ifndef PCI_IOBASE
+ return -EINVAL;
+#else
+ return 0;
+#endif
+}
+
unsigned long __weak pci_address_to_pio(phys_addr_t address)
{
if (address > IO_SPACE_LIMIT)
diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index 5f6ed6b..40c418d 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h
@@ -56,6 +56,7 @@ extern void __iomem *of_iomap(struct device_node *device, int
index);
extern const __be32 *of_get_address(struct device_node *dev, int index,
u64 *size, unsigned int *flags);
+extern int pci_register_io_range(phys_addr_t addr, resource_size_t size);
extern unsigned long pci_address_to_pio(phys_addr_t addr);
extern int of_pci_range_parser_init(struct of_pci_range_parser *parser,
--
1.9.0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v7 1/6] pci: Introduce pci_register_io_range() helper function.
Some architectures do not share x86 simple view of the PCI I/O space
and instead use a range of addresses that map to bus addresses. For
some architectures these ranges will be expressed by OF bindings
in a device tree file.
Introduce a pci_register_io_range() helper function that can be used
by the architecture code to keep track of the I/O ranges described by the
PCI bindings. If the PCI_IOBASE macro is not defined that signals
lack of support for PCI and we return an error.
Signed-off-by: Liviu Dudau liviu.du...@arm.com
Acked-by: Grant Likely grant.lik...@linaro.org
Tested-by: Tanmay Inamdar tinam...@apm.com
---
drivers/of/address.c | 9 +
include/linux/of_address.h | 1 +
2 files changed, 10 insertions(+)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 1a54f1f..be958ed 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -619,6 +619,15 @@ const __be32 *of_get_address(struct device_node *dev, int
index, u64 *size,
}
EXPORT_SYMBOL(of_get_address);
+int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
+{
+#ifndef PCI_IOBASE
+ return -EINVAL;
+#else
+ return 0;
+#endif
+}
+
unsigned long __weak pci_address_to_pio(phys_addr_t address)
{
if (address IO_SPACE_LIMIT)
diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index 5f6ed6b..40c418d 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h
@@ -56,6 +56,7 @@ extern void __iomem *of_iomap(struct device_node *device, int
index);
extern const __be32 *of_get_address(struct device_node *dev, int index,
u64 *size, unsigned int *flags);
+extern int pci_register_io_range(phys_addr_t addr, resource_size_t size);
extern unsigned long pci_address_to_pio(phys_addr_t addr);
extern int of_pci_range_parser_init(struct of_pci_range_parser *parser,
--
1.9.0
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
