Issue with the emulated PCI bridge implementation

Issue with the emulated PCI bridge implementation

[Thomas Petazzoni]

Hello Jason,

I am contacting you regarding a design issue I have with the emulated
PCI bridge implementation that we have written to support the Marvell
PCI controllers.

I'm sure you remember that we emulate a bridge, and trap the writes to
this bridge to create/remove MBus windows as needed. But for I/O
regions, we also use these writes to do the appropriate
pci_ioremap_io() calls.

However, it turns how that the writes to the bridge registers are done
with IRQ disabled, and that pci_ioremap_io() can sleep (since it
essentially does an ioremap). Therefore, with the appropriate kernel
hacking options enabled, you get the following warning at boot time:

pci 0000:00:09.0: PCI bridge to [bus 02]
pci 0000:00:09.0:   bridge window [io  0x10000-0x10fff]
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at /home/thomas/projets/linux-2.6/kernel/locking/lockdep.c:2740 lockdep_trace_alloc+0x11c/0x120()
DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags))
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.13.0-rc5-00002-gf197217 #450
[<c0014a9c>] (unwind_backtrace+0x0/0xf4) from [<c0011348>] (show_stack+0x10/0x14)
[<c0011348>] (show_stack+0x10/0x14) from [<c03b5318>] (dump_stack+0x98/0xb8)
[<c03b5318>] (dump_stack+0x98/0xb8) from [<c001cd94>] (warn_slowpath_common+0x6c/0x88)
[<c001cd94>] (warn_slowpath_common+0x6c/0x88) from [<c001cde0>] (warn_slowpath_fmt+0x30/0x40)
[<c001cde0>] (warn_slowpath_fmt+0x30/0x40) from [<c00533cc>] (lockdep_trace_alloc+0x11c/0x120)
[<c00533cc>] (lockdep_trace_alloc+0x11c/0x120) from [<c00885c8>] (__alloc_pages_nodemask+0x84/0x734)
[<c00885c8>] (__alloc_pages_nodemask+0x84/0x734) from [<c0088c88>] (__get_free_pages+0x10/0x24)
[<c0088c88>] (__get_free_pages+0x10/0x24) from [<c009dd14>] (__pte_alloc_kernel+0x18/0xb8)
[<c009dd14>] (__pte_alloc_kernel+0x18/0xb8) from [<c017b718>] (ioremap_page_range+0x194/0x1a8)
[<c017b718>] (ioremap_page_range+0x194/0x1a8) from [<c01abf68>] (mvebu_pcie_wr_conf+0x2c0/0x434)
[<c01abf68>] (mvebu_pcie_wr_conf+0x2c0/0x434) from [<c019b4dc>] (pci_bus_write_config_dword+0x5c/0x7c)
[<c019b4dc>] (pci_bus_write_config_dword+0x5c/0x7c) from [<c01a6b60>] (__pci_setup_bridge+0x1c0/0x250)
[<c01a6b60>] (__pci_setup_bridge+0x1c0/0x250) from [<c03b23c0>] (__pci_bus_assign_resources+0xfc/0x100)
[<c03b23c0>] (__pci_bus_assign_resources+0xfc/0x100) from [<c00131d0>] (pci_common_init_dev+0x23c/0x2d4)
[<c00131d0>] (pci_common_init_dev+0x23c/0x2d4) from [<c01ac4f4>] (mvebu_pcie_probe+0x3b4/0x61c)
[<c01ac4f4>] (mvebu_pcie_probe+0x3b4/0x61c) from [<c01dddc4>] (platform_drv_probe+0x18/0x48)
[<c01dddc4>] (platform_drv_probe+0x18/0x48) from [<c01dc564>] (really_probe+0x80/0x218)
[<c01dc564>] (really_probe+0x80/0x218) from [<c01dc7f0>] (__driver_attach+0xa0/0xa4)
[<c01dc7f0>] (__driver_attach+0xa0/0xa4) from [<c01dac34>] (bus_for_each_dev+0x60/0x94)
[<c01dac34>] (bus_for_each_dev+0x60/0x94) from [<c01dbdf8>] (bus_add_driver+0x148/0x1f0)
[<c01dbdf8>] (bus_add_driver+0x148/0x1f0) from [<c01dce28>] (driver_register+0x78/0xf8)
[<c01dce28>] (driver_register+0x78/0xf8) from [<c00088c8>] (do_one_initcall+0xf8/0x148)
[<c00088c8>] (do_one_initcall+0xf8/0x148) from [<c04fac48>] (kernel_init_freeable+0x13c/0x1dc)
[<c04fac48>] (kernel_init_freeable+0x13c/0x1dc) from [<c03b0924>] (kernel_init+0x8/0x120)
[<c03b0924>] (kernel_init+0x8/0x120) from [<c000e268>] (ret_from_fork+0x14/0x2c)
---[ end trace eebd66da7489756f ]---
pci 0000:00:09.0:   bridge window [mem 0xe0000000-0xe00fffff]

Do you have any idea to solve this?

For now, my only idea would be to do the pci_ioremap_io()
unconditionally for all PCIe interfaces when the PCIe host controller
driver is initialized. We know the maximum size of the I/O region for
each PCIe interface, and this size is small (64 KB). We can keep the
creation of the corresponding MBus window as something dynamic done by
the bridge.

Do you think that's a reasonable solution? Do you see other
possibilities?

Thanks!

Thomas Petazzoni
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Issue with the emulated PCI bridge implementation

[Thomas Petazzoni]

Hello Jason,

I am contacting you regarding a design issue I have with the emulated
PCI bridge implementation that we have written to support the Marvell
PCI controllers.

I'm sure you remember that we emulate a bridge, and trap the writes to
this bridge to create/remove MBus windows as needed. But for I/O
regions, we also use these writes to do the appropriate
pci_ioremap_io() calls.

However, it turns how that the writes to the bridge registers are done
with IRQ disabled, and that pci_ioremap_io() can sleep (since it
essentially does an ioremap). Therefore, with the appropriate kernel
hacking options enabled, you get the following warning at boot time:

pci 0000:00:09.0: PCI bridge to [bus 02]
pci 0000:00:09.0:   bridge window [io  0x10000-0x10fff]
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at /home/thomas/projets/linux-2.6/kernel/locking/lockdep.c:2740 lockdep_trace_alloc+0x11c/0x120()
DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags))
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.13.0-rc5-00002-gf197217 #450
[<c0014a9c>] (unwind_backtrace+0x0/0xf4) from [<c0011348>] (show_stack+0x10/0x14)
[<c0011348>] (show_stack+0x10/0x14) from [<c03b5318>] (dump_stack+0x98/0xb8)
[<c03b5318>] (dump_stack+0x98/0xb8) from [<c001cd94>] (warn_slowpath_common+0x6c/0x88)
[<c001cd94>] (warn_slowpath_common+0x6c/0x88) from [<c001cde0>] (warn_slowpath_fmt+0x30/0x40)
[<c001cde0>] (warn_slowpath_fmt+0x30/0x40) from [<c00533cc>] (lockdep_trace_alloc+0x11c/0x120)
[<c00533cc>] (lockdep_trace_alloc+0x11c/0x120) from [<c00885c8>] (__alloc_pages_nodemask+0x84/0x734)
[<c00885c8>] (__alloc_pages_nodemask+0x84/0x734) from [<c0088c88>] (__get_free_pages+0x10/0x24)
[<c0088c88>] (__get_free_pages+0x10/0x24) from [<c009dd14>] (__pte_alloc_kernel+0x18/0xb8)
[<c009dd14>] (__pte_alloc_kernel+0x18/0xb8) from [<c017b718>] (ioremap_page_range+0x194/0x1a8)
[<c017b718>] (ioremap_page_range+0x194/0x1a8) from [<c01abf68>] (mvebu_pcie_wr_conf+0x2c0/0x434)
[<c01abf68>] (mvebu_pcie_wr_conf+0x2c0/0x434) from [<c019b4dc>] (pci_bus_write_config_dword+0x5c/0x7c)
[<c019b4dc>] (pci_bus_write_config_dword+0x5c/0x7c) from [<c01a6b60>] (__pci_setup_bridge+0x1c0/0x250)
[<c01a6b60>] (__pci_setup_bridge+0x1c0/0x250) from [<c03b23c0>] (__pci_bus_assign_resources+0xfc/0x100)
[<c03b23c0>] (__pci_bus_assign_resources+0xfc/0x100) from [<c00131d0>] (pci_common_init_dev+0x23c/0x2d4)
[<c00131d0>] (pci_common_init_dev+0x23c/0x2d4) from [<c01ac4f4>] (mvebu_pcie_probe+0x3b4/0x61c)
[<c01ac4f4>] (mvebu_pcie_probe+0x3b4/0x61c) from [<c01dddc4>] (platform_drv_probe+0x18/0x48)
[<c01dddc4>] (platform_drv_probe+0x18/0x48) from [<c01dc564>] (really_probe+0x80/0x218)
[<c01dc564>] (really_probe+0x80/0x218) from [<c01dc7f0>] (__driver_attach+0xa0/0xa4)
[<c01dc7f0>] (__driver_attach+0xa0/0xa4) from [<c01dac34>] (bus_for_each_dev+0x60/0x94)
[<c01dac34>] (bus_for_each_dev+0x60/0x94) from [<c01dbdf8>] (bus_add_driver+0x148/0x1f0)
[<c01dbdf8>] (bus_add_driver+0x148/0x1f0) from [<c01dce28>] (driver_register+0x78/0xf8)
[<c01dce28>] (driver_register+0x78/0xf8) from [<c00088c8>] (do_one_initcall+0xf8/0x148)
[<c00088c8>] (do_one_initcall+0xf8/0x148) from [<c04fac48>] (kernel_init_freeable+0x13c/0x1dc)
[<c04fac48>] (kernel_init_freeable+0x13c/0x1dc) from [<c03b0924>] (kernel_init+0x8/0x120)
[<c03b0924>] (kernel_init+0x8/0x120) from [<c000e268>] (ret_from_fork+0x14/0x2c)
---[ end trace eebd66da7489756f ]---
pci 0000:00:09.0:   bridge window [mem 0xe0000000-0xe00fffff]

Do you have any idea to solve this?

For now, my only idea would be to do the pci_ioremap_io()
unconditionally for all PCIe interfaces when the PCIe host controller
driver is initialized. We know the maximum size of the I/O region for
each PCIe interface, and this size is small (64 KB). We can keep the
creation of the corresponding MBus window as something dynamic done by
the bridge.

Do you think that's a reasonable solution? Do you see other
possibilities?

Thanks!

Thomas Petazzoni
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: Issue with the emulated PCI bridge implementation

[Thomas Petazzoni]

Jason,

On Thu, 26 Dec 2013 16:05:34 +0100, Thomas Petazzoni wrote:

> For now, my only idea would be to do the pci_ioremap_io()
> unconditionally for all PCIe interfaces when the PCIe host controller
> driver is initialized. We know the maximum size of the I/O region for
> each PCIe interface, and this size is small (64 KB). We can keep the
> creation of the corresponding MBus window as something dynamic done by
> the bridge.

Here is an implementation of this idea, tested to work with an e1000e
card, with the driver modified to do a few read/write to the I/O
region. What do you think about it?

Thanks!

Thomas

>From c062af329fa07ddc6d0ca305b4c9633f7e1dca00 Mon Sep 17 00:00:00 2001
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Date: Thu, 26 Dec 2013 16:46:24 +0100
Subject: [PATCH] pci: mvebu: call pci_ioremap_io() at startup instead of
 dynamically

The mvebu PCI host controller driver uses an emulated PCI-to-PCI
bridge to leverage the core PCI kernel enumeration logic to
dynamically create and remove the MBus windows needed to access the
memory and I/O regions of each PCI interface.

In the context of this PCI-to-PCI bridge emulation, the driver
emulates all reads and writes to the PCI bridge registers. Upon a
write to the registers conguring the I/O base and limit, the driver
was creating the MBus window, and calling pci_ioremap_io() to setup
the mapping.

However, it turns out that accesses to these registers are made in an
IRQ disabled context, while pci_ioremap_io() is a potentially sleeping
function. Not only this is wrong, but it is causing so fairly loud
warnings at boot time when the appropriate kernel hacking options are
enabled.

This patch solves this by moving the pci_ioremap_io() call at the
startup of the driver. At this point, we don't know how many PCI
interfaces will be enabled, so we are simply remapping the entire PCI
I/O space to virtual addresses. This is reasonable since this I/O
space is limited to 1 MB in size, and also because the MBus windows
continue to be created in a dynamic fashion only when devices need
them.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/pci/host/pci-mvebu.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index 2aa7b77c..476dd72 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -330,8 +330,6 @@ static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
 	mvebu_mbus_add_window_remap_by_id(port->io_target, port->io_attr,
 					  port->iowin_base, port->iowin_size,
 					  iobase);
-
-	pci_ioremap_io(iobase, port->iowin_base);
 }
 
 static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)
@@ -969,6 +967,10 @@ static int mvebu_pcie_probe(struct platform_device *pdev)
 	}
 
 	pcie->nports = i;
+
+	for (i = 0; i < (IO_SPACE_LIMIT - SZ_64K); i += SZ_64K)
+		pci_ioremap_io(i, pcie->io.start + i);
+
 	mvebu_pcie_msi_enable(pcie);
 	mvebu_pcie_enable(pcie);
 
-- 
1.8.3.2

-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Issue with the emulated PCI bridge implementation

[Thomas Petazzoni]

Jason,

On Thu, 26 Dec 2013 16:05:34 +0100, Thomas Petazzoni wrote:

> For now, my only idea would be to do the pci_ioremap_io()
> unconditionally for all PCIe interfaces when the PCIe host controller
> driver is initialized. We know the maximum size of the I/O region for
> each PCIe interface, and this size is small (64 KB). We can keep the
> creation of the corresponding MBus window as something dynamic done by
> the bridge.

Here is an implementation of this idea, tested to work with an e1000e
card, with the driver modified to do a few read/write to the I/O
region. What do you think about it?

Thanks!

Thomas

>From c062af329fa07ddc6d0ca305b4c9633f7e1dca00 Mon Sep 17 00:00:00 2001
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Date: Thu, 26 Dec 2013 16:46:24 +0100
Subject: [PATCH] pci: mvebu: call pci_ioremap_io() at startup instead of
 dynamically

The mvebu PCI host controller driver uses an emulated PCI-to-PCI
bridge to leverage the core PCI kernel enumeration logic to
dynamically create and remove the MBus windows needed to access the
memory and I/O regions of each PCI interface.

In the context of this PCI-to-PCI bridge emulation, the driver
emulates all reads and writes to the PCI bridge registers. Upon a
write to the registers conguring the I/O base and limit, the driver
was creating the MBus window, and calling pci_ioremap_io() to setup
the mapping.

However, it turns out that accesses to these registers are made in an
IRQ disabled context, while pci_ioremap_io() is a potentially sleeping
function. Not only this is wrong, but it is causing so fairly loud
warnings at boot time when the appropriate kernel hacking options are
enabled.

This patch solves this by moving the pci_ioremap_io() call at the
startup of the driver. At this point, we don't know how many PCI
interfaces will be enabled, so we are simply remapping the entire PCI
I/O space to virtual addresses. This is reasonable since this I/O
space is limited to 1 MB in size, and also because the MBus windows
continue to be created in a dynamic fashion only when devices need
them.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/pci/host/pci-mvebu.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index 2aa7b77c..476dd72 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -330,8 +330,6 @@ static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
 	mvebu_mbus_add_window_remap_by_id(port->io_target, port->io_attr,
 					  port->iowin_base, port->iowin_size,
 					  iobase);
-
-	pci_ioremap_io(iobase, port->iowin_base);
 }
 
 static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)
@@ -969,6 +967,10 @@ static int mvebu_pcie_probe(struct platform_device *pdev)
 	}
 
 	pcie->nports = i;
+
+	for (i = 0; i < (IO_SPACE_LIMIT - SZ_64K); i += SZ_64K)
+		pci_ioremap_io(i, pcie->io.start + i);
+
 	mvebu_pcie_msi_enable(pcie);
 	mvebu_pcie_enable(pcie);
 
-- 
1.8.3.2

-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: Issue with the emulated PCI bridge implementation

[Bjorn Helgaas]

On Thu, Dec 26, 2013 at 8:52 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Jason,
>
> On Thu, 26 Dec 2013 16:05:34 +0100, Thomas Petazzoni wrote:
>
>> For now, my only idea would be to do the pci_ioremap_io()
>> unconditionally for all PCIe interfaces when the PCIe host controller
>> driver is initialized. We know the maximum size of the I/O region for
>> each PCIe interface, and this size is small (64 KB). We can keep the
>> creation of the corresponding MBus window as something dynamic done by
>> the bridge.
>
> Here is an implementation of this idea, tested to work with an e1000e
> card, with the driver modified to do a few read/write to the I/O
> region. What do you think about it?
>
> Thanks!
>
> Thomas
>
> From c062af329fa07ddc6d0ca305b4c9633f7e1dca00 Mon Sep 17 00:00:00 2001
> From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Date: Thu, 26 Dec 2013 16:46:24 +0100
> Subject: [PATCH] pci: mvebu: call pci_ioremap_io() at startup instead of
>  dynamically
>
> The mvebu PCI host controller driver uses an emulated PCI-to-PCI
> bridge to leverage the core PCI kernel enumeration logic to
> dynamically create and remove the MBus windows needed to access the
> memory and I/O regions of each PCI interface.
>
> In the context of this PCI-to-PCI bridge emulation, the driver
> emulates all reads and writes to the PCI bridge registers. Upon a
> write to the registers conguring the I/O base and limit, the driver
> was creating the MBus window, and calling pci_ioremap_io() to setup
> the mapping.
>
> However, it turns out that accesses to these registers are made in an
> IRQ disabled context, while pci_ioremap_io() is a potentially sleeping
> function. Not only this is wrong, but it is causing so fairly loud
> warnings at boot time when the appropriate kernel hacking options are
> enabled.
>
> This patch solves this by moving the pci_ioremap_io() call at the
> startup of the driver. At this point, we don't know how many PCI
> interfaces will be enabled, so we are simply remapping the entire PCI
> I/O space to virtual addresses. This is reasonable since this I/O
> space is limited to 1 MB in size, and also because the MBus windows
> continue to be created in a dynamic fashion only when devices need
> them.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Applied to my pci/host-mvebu branch for v3.14, thanks!

Bjorn

> ---
>  drivers/pci/host/pci-mvebu.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
> index 2aa7b77c..476dd72 100644
> --- a/drivers/pci/host/pci-mvebu.c
> +++ b/drivers/pci/host/pci-mvebu.c
> @@ -330,8 +330,6 @@ static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
>         mvebu_mbus_add_window_remap_by_id(port->io_target, port->io_attr,
>                                           port->iowin_base, port->iowin_size,
>                                           iobase);
> -
> -       pci_ioremap_io(iobase, port->iowin_base);
>  }
>
>  static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)
> @@ -969,6 +967,10 @@ static int mvebu_pcie_probe(struct platform_device *pdev)
>         }
>
>         pcie->nports = i;
> +
> +       for (i = 0; i < (IO_SPACE_LIMIT - SZ_64K); i += SZ_64K)
> +               pci_ioremap_io(i, pcie->io.start + i);
> +
>         mvebu_pcie_msi_enable(pcie);
>         mvebu_pcie_enable(pcie);
>
> --
> 1.8.3.2
>
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

Issue with the emulated PCI bridge implementation

[Bjorn Helgaas]

On Thu, Dec 26, 2013 at 8:52 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Jason,
>
> On Thu, 26 Dec 2013 16:05:34 +0100, Thomas Petazzoni wrote:
>
>> For now, my only idea would be to do the pci_ioremap_io()
>> unconditionally for all PCIe interfaces when the PCIe host controller
>> driver is initialized. We know the maximum size of the I/O region for
>> each PCIe interface, and this size is small (64 KB). We can keep the
>> creation of the corresponding MBus window as something dynamic done by
>> the bridge.
>
> Here is an implementation of this idea, tested to work with an e1000e
> card, with the driver modified to do a few read/write to the I/O
> region. What do you think about it?
>
> Thanks!
>
> Thomas
>
> From c062af329fa07ddc6d0ca305b4c9633f7e1dca00 Mon Sep 17 00:00:00 2001
> From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Date: Thu, 26 Dec 2013 16:46:24 +0100
> Subject: [PATCH] pci: mvebu: call pci_ioremap_io() at startup instead of
>  dynamically
>
> The mvebu PCI host controller driver uses an emulated PCI-to-PCI
> bridge to leverage the core PCI kernel enumeration logic to
> dynamically create and remove the MBus windows needed to access the
> memory and I/O regions of each PCI interface.
>
> In the context of this PCI-to-PCI bridge emulation, the driver
> emulates all reads and writes to the PCI bridge registers. Upon a
> write to the registers conguring the I/O base and limit, the driver
> was creating the MBus window, and calling pci_ioremap_io() to setup
> the mapping.
>
> However, it turns out that accesses to these registers are made in an
> IRQ disabled context, while pci_ioremap_io() is a potentially sleeping
> function. Not only this is wrong, but it is causing so fairly loud
> warnings at boot time when the appropriate kernel hacking options are
> enabled.
>
> This patch solves this by moving the pci_ioremap_io() call at the
> startup of the driver. At this point, we don't know how many PCI
> interfaces will be enabled, so we are simply remapping the entire PCI
> I/O space to virtual addresses. This is reasonable since this I/O
> space is limited to 1 MB in size, and also because the MBus windows
> continue to be created in a dynamic fashion only when devices need
> them.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Applied to my pci/host-mvebu branch for v3.14, thanks!

Bjorn

> ---
>  drivers/pci/host/pci-mvebu.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
> index 2aa7b77c..476dd72 100644
> --- a/drivers/pci/host/pci-mvebu.c
> +++ b/drivers/pci/host/pci-mvebu.c
> @@ -330,8 +330,6 @@ static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
>         mvebu_mbus_add_window_remap_by_id(port->io_target, port->io_attr,
>                                           port->iowin_base, port->iowin_size,
>                                           iobase);
> -
> -       pci_ioremap_io(iobase, port->iowin_base);
>  }
>
>  static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)
> @@ -969,6 +967,10 @@ static int mvebu_pcie_probe(struct platform_device *pdev)
>         }
>
>         pcie->nports = i;
> +
> +       for (i = 0; i < (IO_SPACE_LIMIT - SZ_64K); i += SZ_64K)
> +               pci_ioremap_io(i, pcie->io.start + i);
> +
>         mvebu_pcie_msi_enable(pcie);
>         mvebu_pcie_enable(pcie);
>
> --
> 1.8.3.2
>
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

Re: Issue with the emulated PCI bridge implementation

[Jason Gunthorpe]

On Thu, Dec 26, 2013 at 04:52:41PM +0100, Thomas Petazzoni wrote:

> Here is an implementation of this idea, tested to work with an e1000e
> card, with the driver modified to do a few read/write to the I/O
> region. What do you think about it?

This seems reasonable, the only down side is that a stray read to an
unused portion of the Linux IO mapping will lock the machine instead
of getting a page fault - however I don't see that as a blocker.

Regards,
Jason

Issue with the emulated PCI bridge implementation

[Jason Gunthorpe]

On Thu, Dec 26, 2013 at 04:52:41PM +0100, Thomas Petazzoni wrote:

> Here is an implementation of this idea, tested to work with an e1000e
> card, with the driver modified to do a few read/write to the I/O
> region. What do you think about it?

This seems reasonable, the only down side is that a stray read to an
unused portion of the Linux IO mapping will lock the machine instead
of getting a page fault - however I don't see that as a blocker.

Regards,
Jason

Re: Issue with the emulated PCI bridge implementation

[Arnd Bergmann]

On Friday 03 January 2014, Jason Gunthorpe wrote:
> On Thu, Dec 26, 2013 at 04:52:41PM +0100, Thomas Petazzoni wrote:
> 
> > Here is an implementation of this idea, tested to work with an e1000e
> > card, with the driver modified to do a few read/write to the I/O
> > region. What do you think about it?
> 
> This seems reasonable, the only down side is that a stray read to an
> unused portion of the Linux IO mapping will lock the machine instead
> of getting a page fault - however I don't see that as a blocker.
> 

I've scratched my head a bit over this patch, and I couldn't find anything
wrong with it in the end, as long as we don't have any other device
in the system that also wants its share of the I/O space (e.g. a
PCMCIA port on the SRAM interface), but then we'd probably need other
changes as well.

However the part that made me wonder is that an e1000e with a PCI bridge
actually /should/not/ need to allocate an I/O space window with a precious
mbus resource, since AFAIK this adapter does not have an I/O space BARs.

Thomas, can you send the 'lspci -v' output of the system with this card
to confirm that the I/O space is actually needed? If not, we should probably
review the core PCI code to see why the bridge code tries to add this window.

	Arnd

Issue with the emulated PCI bridge implementation

[Arnd Bergmann]

On Friday 03 January 2014, Jason Gunthorpe wrote:
> On Thu, Dec 26, 2013 at 04:52:41PM +0100, Thomas Petazzoni wrote:
> 
> > Here is an implementation of this idea, tested to work with an e1000e
> > card, with the driver modified to do a few read/write to the I/O
> > region. What do you think about it?
> 
> This seems reasonable, the only down side is that a stray read to an
> unused portion of the Linux IO mapping will lock the machine instead
> of getting a page fault - however I don't see that as a blocker.
> 

I've scratched my head a bit over this patch, and I couldn't find anything
wrong with it in the end, as long as we don't have any other device
in the system that also wants its share of the I/O space (e.g. a
PCMCIA port on the SRAM interface), but then we'd probably need other
changes as well.

However the part that made me wonder is that an e1000e with a PCI bridge
actually /should/not/ need to allocate an I/O space window with a precious
mbus resource, since AFAIK this adapter does not have an I/O space BARs.

Thomas, can you send the 'lspci -v' output of the system with this card
to confirm that the I/O space is actually needed? If not, we should probably
review the core PCI code to see why the bridge code tries to add this window.

	Arnd

Re: Issue with the emulated PCI bridge implementation

[Jason Gunthorpe]

On Fri, Jan 03, 2014 at 01:22:31PM +0100, Arnd Bergmann wrote:

> However the part that made me wonder is that an e1000e with a PCI bridge
> actually /should/not/ need to allocate an I/O space window with a precious
> mbus resource, since AFAIK this adapter does not have an I/O space BARs.

IIRC the e1000 still has a legacy IO port BAR.. (guessing it is used
for PXE boot on x86?)

My patch set to allow the DT to turn off IO port allocation is already
in mainline - drop the IO ranges from the DT and no IO resources are
consumed at all.

Though, that is an interesting point, a small refinement would be to
not allocate the pci io map if IO is turned off as well..

Jason

Issue with the emulated PCI bridge implementation

[Jason Gunthorpe]

On Fri, Jan 03, 2014 at 01:22:31PM +0100, Arnd Bergmann wrote:

> However the part that made me wonder is that an e1000e with a PCI bridge
> actually /should/not/ need to allocate an I/O space window with a precious
> mbus resource, since AFAIK this adapter does not have an I/O space BARs.

IIRC the e1000 still has a legacy IO port BAR.. (guessing it is used
for PXE boot on x86?)

My patch set to allow the DT to turn off IO port allocation is already
in mainline - drop the IO ranges from the DT and no IO resources are
consumed at all.

Though, that is an interesting point, a small refinement would be to
not allocate the pci io map if IO is turned off as well..

Jason

Re: Issue with the emulated PCI bridge implementation

[Thomas Petazzoni]

Dear Arnd Bergmann,

On Fri, 3 Jan 2014 13:22:31 +0100, Arnd Bergmann wrote:

> However the part that made me wonder is that an e1000e with a PCI bridge
> actually /should/not/ need to allocate an I/O space window with a precious
> mbus resource, since AFAIK this adapter does not have an I/O space BARs.
> 
> Thomas, can you send the 'lspci -v' output of the system with this card
> to confirm that the I/O space is actually needed? If not, we should probably
> review the core PCI code to see why the bridge code tries to add this window.

I don't have the hardware next to me, but IIRC the card does have an
I/O space. It is not used by the mainline kernel driver, so I have a
hacky patch that modifies the e1000e driver to make it use the I/O
space and do a few I/O reads and writes. I've used that since the
beginning to test I/O space functionality of the pci-mvebu driver.

Digging in the LAKML archive, I found a lspci -v output about the
e1000e, and it has an I/O space:

03:00.0 Ethernet controller: Intel Corporation 82572EI Gigabit Ethernet Controller (Copper) (rev 06)
	Subsystem: Intel Corporation PRO/1000 PT Server Adapter
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr+ Stepping- SERR+ FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0, Cache Line Size: 64 bytes
	Interrupt: pin A routed to IRQ 105
	Region 0: Memory at c1000000 (32-bit, non-prefetchable) [size=128K]
	Region 1: Memory at c1020000 (32-bit, non-prefetchable) [size=128K]
	Region 2: I/O ports at c0000000 [disabled] [size=32]
	[virtual] Expansion ROM at c1100000 [disabled] [size=128K]
	Capabilities: [c8] Power Management version 2
		Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold+)
		Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=1 PME-
	Capabilities: [d0] MSI: Enable- Count=1/1 Maskable- 64bit+
		Address: 0000000000000000  Data: 0000
	Capabilities: [e0] Express (v1) Endpoint, MSI 00
		DevCap:	MaxPayload 256 bytes, PhantFunc 0, Latency L0s <512ns, L1 <64us
			ExtTag- AttnBtn- AttnInd- PwrInd- RBE- FLReset-
		DevCtl:	Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
			RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
			MaxPayload 128 bytes, MaxReadReq 512 bytes
		DevSta:	CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr+ TransPend-
		LnkCap:	Port #0, Speed 2.5GT/s, Width x1, ASPM L0s, Latency L0 <4us, L1 <64us
			ClockPM- Surprise- LLActRep- BwNot-
		LnkCtl:	ASPM Disabled; RCB 64 bytes Disabled- Retrain- CommClk-
			ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
		LnkSta:	Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
	Capabilities: [100 v1] Advanced Error Reporting
		UESta:	DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
		UEMsk:	DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
		UESvrt:	DLP+ SDES- TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol-
		CESta:	RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr-
		CEMsk:	RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr-
		AERCap:	First Error Pointer: 00, GenCap- CGenEn- ChkCap- ChkEn-
	Capabilities: [140 v1] Device Serial Number 00-1b-21-ff-ff-c1-c4-fe
	Kernel driver in use: e1000e

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Issue with the emulated PCI bridge implementation

[Thomas Petazzoni]

Dear Arnd Bergmann,

On Fri, 3 Jan 2014 13:22:31 +0100, Arnd Bergmann wrote:

> However the part that made me wonder is that an e1000e with a PCI bridge
> actually /should/not/ need to allocate an I/O space window with a precious
> mbus resource, since AFAIK this adapter does not have an I/O space BARs.
> 
> Thomas, can you send the 'lspci -v' output of the system with this card
> to confirm that the I/O space is actually needed? If not, we should probably
> review the core PCI code to see why the bridge code tries to add this window.

I don't have the hardware next to me, but IIRC the card does have an
I/O space. It is not used by the mainline kernel driver, so I have a
hacky patch that modifies the e1000e driver to make it use the I/O
space and do a few I/O reads and writes. I've used that since the
beginning to test I/O space functionality of the pci-mvebu driver.

Digging in the LAKML archive, I found a lspci -v output about the
e1000e, and it has an I/O space:

03:00.0 Ethernet controller: Intel Corporation 82572EI Gigabit Ethernet Controller (Copper) (rev 06)
	Subsystem: Intel Corporation PRO/1000 PT Server Adapter
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr+ Stepping- SERR+ FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0, Cache Line Size: 64 bytes
	Interrupt: pin A routed to IRQ 105
	Region 0: Memory at c1000000 (32-bit, non-prefetchable) [size=128K]
	Region 1: Memory at c1020000 (32-bit, non-prefetchable) [size=128K]
	Region 2: I/O ports at c0000000 [disabled] [size=32]
	[virtual] Expansion ROM@c1100000 [disabled] [size=128K]
	Capabilities: [c8] Power Management version 2
		Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold+)
		Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=1 PME-
	Capabilities: [d0] MSI: Enable- Count=1/1 Maskable- 64bit+
		Address: 0000000000000000  Data: 0000
	Capabilities: [e0] Express (v1) Endpoint, MSI 00
		DevCap:	MaxPayload 256 bytes, PhantFunc 0, Latency L0s <512ns, L1 <64us
			ExtTag- AttnBtn- AttnInd- PwrInd- RBE- FLReset-
		DevCtl:	Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
			RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
			MaxPayload 128 bytes, MaxReadReq 512 bytes
		DevSta:	CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr+ TransPend-
		LnkCap:	Port #0, Speed 2.5GT/s, Width x1, ASPM L0s, Latency L0 <4us, L1 <64us
			ClockPM- Surprise- LLActRep- BwNot-
		LnkCtl:	ASPM Disabled; RCB 64 bytes Disabled- Retrain- CommClk-
			ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
		LnkSta:	Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
	Capabilities: [100 v1] Advanced Error Reporting
		UESta:	DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
		UEMsk:	DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
		UESvrt:	DLP+ SDES- TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol-
		CESta:	RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr-
		CEMsk:	RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr-
		AERCap:	First Error Pointer: 00, GenCap- CGenEn- ChkCap- ChkEn-
	Capabilities: [140 v1] Device Serial Number 00-1b-21-ff-ff-c1-c4-fe
	Kernel driver in use: e1000e

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: Issue with the emulated PCI bridge implementation

[Arnd Bergmann]

On Friday 03 January 2014 11:44:24 Jason Gunthorpe wrote:
> On Fri, Jan 03, 2014 at 01:22:31PM +0100, Arnd Bergmann wrote:
> 
> > However the part that made me wonder is that an e1000e with a PCI bridge
> > actually /should/not/ need to allocate an I/O space window with a precious
> > mbus resource, since AFAIK this adapter does not have an I/O space BARs.
> 
> IIRC the e1000 still has a legacy IO port BAR.. (guessing it is used
> for PXE boot on x86?)

Ok, that would certainly explain it. I actually found a reference to the
I/O BAR in the e1000 driver now.

> My patch set to allow the DT to turn off IO port allocation is already
> in mainline - drop the IO ranges from the DT and no IO resources are
> consumed at all.

Ah, nice! I haven't followed that, but it sounds like a very good
solution.

> Though, that is an interesting point, a small refinement would be to
> not allocate the pci io map if IO is turned off as well..

Do you mean not call pci_ioremap_io() and pci_add_resource_offset(), or
something else? Either way, it sounds like the correct thing to do.
I wonder if the PCI core has any problems when there is no IO resource,
but it's probably fine.

	Arnd

Issue with the emulated PCI bridge implementation

[Arnd Bergmann]

On Friday 03 January 2014 11:44:24 Jason Gunthorpe wrote:
> On Fri, Jan 03, 2014 at 01:22:31PM +0100, Arnd Bergmann wrote:
> 
> > However the part that made me wonder is that an e1000e with a PCI bridge
> > actually /should/not/ need to allocate an I/O space window with a precious
> > mbus resource, since AFAIK this adapter does not have an I/O space BARs.
> 
> IIRC the e1000 still has a legacy IO port BAR.. (guessing it is used
> for PXE boot on x86?)

Ok, that would certainly explain it. I actually found a reference to the
I/O BAR in the e1000 driver now.

> My patch set to allow the DT to turn off IO port allocation is already
> in mainline - drop the IO ranges from the DT and no IO resources are
> consumed at all.

Ah, nice! I haven't followed that, but it sounds like a very good
solution.

> Though, that is an interesting point, a small refinement would be to
> not allocate the pci io map if IO is turned off as well..

Do you mean not call pci_ioremap_io() and pci_add_resource_offset(), or
something else? Either way, it sounds like the correct thing to do.
I wonder if the PCI core has any problems when there is no IO resource,
but it's probably fine.

	Arnd

Re: Issue with the emulated PCI bridge implementation

[Arnd Bergmann]

On Friday 03 January 2014 20:01:06 Thomas Petazzoni wrote:
> I don't have the hardware next to me, but IIRC the card does have an
> I/O space. It is not used by the mainline kernel driver, so I have a
> hacky patch that modifies the e1000e driver to make it use the I/O
> space and do a few I/O reads and writes. I've used that since the
> beginning to test I/O space functionality of the pci-mvebu driver.
> 
> Digging in the LAKML archive, I found a lspci -v output about the
> e1000e, and it has an I/O space:

Ok, thanks for the confirmation!

Do you mind posting your hack here? It may be useful for others as
well, such as the xgene developers that seem to be doing funny things
with their I/O space.

	Arnd

Issue with the emulated PCI bridge implementation

[Arnd Bergmann]

On Friday 03 January 2014 20:01:06 Thomas Petazzoni wrote:
> I don't have the hardware next to me, but IIRC the card does have an
> I/O space. It is not used by the mainline kernel driver, so I have a
> hacky patch that modifies the e1000e driver to make it use the I/O
> space and do a few I/O reads and writes. I've used that since the
> beginning to test I/O space functionality of the pci-mvebu driver.
> 
> Digging in the LAKML archive, I found a lspci -v output about the
> e1000e, and it has an I/O space:

Ok, thanks for the confirmation!

Do you mind posting your hack here? It may be useful for others as
well, such as the xgene developers that seem to be doing funny things
with their I/O space.

	Arnd

Re: Issue with the emulated PCI bridge implementation

[Thomas Petazzoni]

Dear Arnd Bergmann,

On Fri, 03 Jan 2014 20:04:57 +0100, Arnd Bergmann wrote:

> > Digging in the LAKML archive, I found a lspci -v output about the
> > e1000e, and it has an I/O space:
> 
> Ok, thanks for the confirmation!
> 
> Do you mind posting your hack here? It may be useful for others as
> well, such as the xgene developers that seem to be doing funny things
> with their I/O space.

The current hack is the following very ugly piece of code... It runs
some command and reads back some registers, and you should see some
well-known values (which can be found in the e1000 datasheet, I don't
remember them right now).

I'm not sure why I do the pci_request_selected_regions_exclusive() call
*after* the actual in/out. Probably a mistake, but as I said, this is
ugly stuff :)

Thomas

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index fbf75fd..739ca10 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -6045,7 +6045,7 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (aspm_disable_flag)
 		e1000e_disable_aspm(pdev, aspm_disable_flag);
 
-	err = pci_enable_device_mem(pdev);
+	err = pci_enable_device(pdev);
 	if (err)
 		return err;
 
@@ -6073,6 +6073,34 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (err)
 		goto err_pci_reg;
 
+	dev_info(&pdev->dev, "==> e1000e: I/O start 0x%x, len 0x%x\n",
+		 pci_resource_start(pdev, 2),
+		 pci_resource_len(pdev, 2));
+
+
+	{
+		unsigned long ioport_base = pci_resource_start(pdev, 2);
+		dev_info(&pdev->dev, "1. 0x%x 0x%x 0x%x 0x%x\n",
+			 inl(ioport_base),
+			 inl(ioport_base + 4),
+			 inl(ioport_base + 8),
+			 inl(ioport_base + 12));
+		outl(0x38, ioport_base);
+		dev_info(&pdev->dev, "2. 0x%x 0x%x 0x%x 0x%x\n",
+			 inl(ioport_base),
+			 inl(ioport_base + 4),
+			 inl(ioport_base + 8),
+			 inl(ioport_base + 12));
+	}
+
+	err = pci_request_selected_regions_exclusive(pdev,
+						     pci_select_bars(pdev, IORESOURCE_IO),
+						     e1000e_driver_name);
+	if (err) {
+		dev_err(&pdev->dev, "Cannot get I/O region\n");
+		goto err_pci_reg;
+	}
+
 	/* AER (Advanced Error Reporting) hooks */
 	pci_enable_pcie_error_reporting(pdev);
 
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Issue with the emulated PCI bridge implementation

[Thomas Petazzoni]

Dear Arnd Bergmann,

On Fri, 03 Jan 2014 20:04:57 +0100, Arnd Bergmann wrote:

> > Digging in the LAKML archive, I found a lspci -v output about the
> > e1000e, and it has an I/O space:
> 
> Ok, thanks for the confirmation!
> 
> Do you mind posting your hack here? It may be useful for others as
> well, such as the xgene developers that seem to be doing funny things
> with their I/O space.

The current hack is the following very ugly piece of code... It runs
some command and reads back some registers, and you should see some
well-known values (which can be found in the e1000 datasheet, I don't
remember them right now).

I'm not sure why I do the pci_request_selected_regions_exclusive() call
*after* the actual in/out. Probably a mistake, but as I said, this is
ugly stuff :)

Thomas

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index fbf75fd..739ca10 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -6045,7 +6045,7 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (aspm_disable_flag)
 		e1000e_disable_aspm(pdev, aspm_disable_flag);
 
-	err = pci_enable_device_mem(pdev);
+	err = pci_enable_device(pdev);
 	if (err)
 		return err;
 
@@ -6073,6 +6073,34 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (err)
 		goto err_pci_reg;
 
+	dev_info(&pdev->dev, "==> e1000e: I/O start 0x%x, len 0x%x\n",
+		 pci_resource_start(pdev, 2),
+		 pci_resource_len(pdev, 2));
+
+
+	{
+		unsigned long ioport_base = pci_resource_start(pdev, 2);
+		dev_info(&pdev->dev, "1. 0x%x 0x%x 0x%x 0x%x\n",
+			 inl(ioport_base),
+			 inl(ioport_base + 4),
+			 inl(ioport_base + 8),
+			 inl(ioport_base + 12));
+		outl(0x38, ioport_base);
+		dev_info(&pdev->dev, "2. 0x%x 0x%x 0x%x 0x%x\n",
+			 inl(ioport_base),
+			 inl(ioport_base + 4),
+			 inl(ioport_base + 8),
+			 inl(ioport_base + 12));
+	}
+
+	err = pci_request_selected_regions_exclusive(pdev,
+						     pci_select_bars(pdev, IORESOURCE_IO),
+						     e1000e_driver_name);
+	if (err) {
+		dev_err(&pdev->dev, "Cannot get I/O region\n");
+		goto err_pci_reg;
+	}
+
 	/* AER (Advanced Error Reporting) hooks */
 	pci_enable_pcie_error_reporting(pdev);
 
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: Issue with the emulated PCI bridge implementation

[Arnd Bergmann]

On Friday 03 January 2014 20:10:24 Thomas Petazzoni wrote:
> Dear Arnd Bergmann,
> 
> On Fri, 03 Jan 2014 20:04:57 +0100, Arnd Bergmann wrote:
> 
> > > Digging in the LAKML archive, I found a lspci -v output about the
> > > e1000e, and it has an I/O space:
> > 
> > Ok, thanks for the confirmation!
> > 
> > Do you mind posting your hack here? It may be useful for others as
> > well, such as the xgene developers that seem to be doing funny things
> > with their I/O space.
> 
> The current hack is the following very ugly piece of code... It runs
> some command and reads back some registers, and you should see some
> well-known values (which can be found in the e1000 datasheet, I don't
> remember them right now).

Totally enough for the purpose, thanks!

	Arnd

Issue with the emulated PCI bridge implementation

[Arnd Bergmann]

On Friday 03 January 2014 20:10:24 Thomas Petazzoni wrote:
> Dear Arnd Bergmann,
> 
> On Fri, 03 Jan 2014 20:04:57 +0100, Arnd Bergmann wrote:
> 
> > > Digging in the LAKML archive, I found a lspci -v output about the
> > > e1000e, and it has an I/O space:
> > 
> > Ok, thanks for the confirmation!
> > 
> > Do you mind posting your hack here? It may be useful for others as
> > well, such as the xgene developers that seem to be doing funny things
> > with their I/O space.
> 
> The current hack is the following very ugly piece of code... It runs
> some command and reads back some registers, and you should see some
> well-known values (which can be found in the e1000 datasheet, I don't
> remember them right now).

Totally enough for the purpose, thanks!

	Arnd