[PATCH] POWERPC: Support ISA legacy addresses in of_address_to_resource()

[PATCH] POWERPC: Support ISA legacy addresses in of_address_to_resource()

[Nate Case]

When mapping an open firmware device tree node to a resource,
check if the device is on the "isa" legacy bus.  In this case,
pci_address_to_pio() should not be used since that function is only
for addresses above the 64KB reserved region.

This was necessary to get IPMI working on a board that accesses
the IPMI controller via the legacy I/O region.

Signed-off-by: Nate Case <ncase@xes-inc.com>
---
 arch/powerpc/kernel/prom_parse.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/kernel/prom_parse.c b/arch/powerpc/kernel/prom_parse.c
index 90eb3a3..28d354d 100644
--- a/arch/powerpc/kernel/prom_parse.c
+++ b/arch/powerpc/kernel/prom_parse.c
@@ -622,7 +622,15 @@ static int __of_address_to_resource(struct device_node *dev, const u32 *addrp,
 	memset(r, 0, sizeof(struct resource));
 	if (flags & IORESOURCE_IO) {
 		unsigned long port;
-		port = pci_address_to_pio(taddr);
+		struct device_node *parent;
+
+		parent = of_get_parent(dev);
+		if (of_bus_isa_match(parent))
+			port = (unsigned long) taddr;
+		else
+			port = pci_address_to_pio(taddr);
+		of_node_put(parent);
+
 		if (port == (unsigned long)-1)
 			return -EINVAL;
 		r->start = port;
-- 
1.5.4.4

Re: [PATCH] POWERPC: Support ISA legacy addresses in of_address_to_resource()

[Benjamin Herrenschmidt]

On Thu, 2008-05-08 at 18:26 -0500, Nate Case wrote:
> When mapping an open firmware device tree node to a resource,
> check if the device is on the "isa" legacy bus.  In this case,
> pci_address_to_pio() should not be used since that function is only
> for addresses above the 64KB reserved region.
> 
> This was necessary to get IPMI working on a board that accesses
> the IPMI controller via the legacy I/O region.

I don't understand that fix. Can you tell us more about the exact
failure mode ? Especially, show us the device-tree bits and the
addresses returned.

I suspect the bug is in your device-tree that is forwarding IO addresses
all the way without converting them to memory or something like that.

> Signed-off-by: Nate Case <ncase@xes-inc.com>
> ---
>  arch/powerpc/kernel/prom_parse.c |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/prom_parse.c b/arch/powerpc/kernel/prom_parse.c
> index 90eb3a3..28d354d 100644
> --- a/arch/powerpc/kernel/prom_parse.c
> +++ b/arch/powerpc/kernel/prom_parse.c
> @@ -622,7 +622,15 @@ static int __of_address_to_resource(struct device_node *dev, const u32 *addrp,
>  	memset(r, 0, sizeof(struct resource));
>  	if (flags & IORESOURCE_IO) {
>  		unsigned long port;
> -		port = pci_address_to_pio(taddr);
> +		struct device_node *parent;
> +
> +		parent = of_get_parent(dev);
> +		if (of_bus_isa_match(parent))
> +			port = (unsigned long) taddr;
> +		else
> +			port = pci_address_to_pio(taddr);
> +		of_node_put(parent);
> +
>  		if (port == (unsigned long)-1)
>  			return -EINVAL;
>  		r->start = port;

Re: [PATCH] POWERPC: Support ISA legacy addresses in of_address_to_resource()

[Nate Case]

On Fri, 2008-05-09 at 14:31 +1000, Benjamin Herrenschmidt wrote:
> I don't understand that fix. Can you tell us more about the exact
> failure mode ? Especially, show us the device-tree bits and the
> addresses returned.
> 
> I suspect the bug is in your device-tree that is forwarding IO
> addresses
> all the way without converting them to memory or something like that.

Your suspicion is correct in a way -- this allows IO port addresses to
make their way to 'struct resource' so that drivers can use inb(),
outb(), etc.  After some thought, I'm not sure if a device tree using
I/O port addresses for a legacy node is considered a bug or not.

Here is the failure case my patch fixes:

        isa@0,e00f0000 {
                linux,phandle = <0x3ff93688>;
                ranges = <0x1 0x0 0x0 0x0 0x1000>;
                reg = <0x0 0xe00f0000 0x0 0x10000>;
                class-code = <0x601ff>;
                device-id = <0xa008>;
                vendor-id = <0x1959>;
                #address-cells = <0x2>;
                #size-cells = <0x1>;
                device_type = "isa";

                ipmi@ca2 {
                        linux,phandle = <0x3ff93a00>;
                        reg = <0x1 0xca2 0x100>;
                        reg-spacing = <0x1>;
                        reg-size = <0x1>;
                        compatible = "ipmi-kcs";
                        device_type = "ipmi";
                };
        };

The address returned for ipmi@ for this case is 0x10ca2 instead of
0xca2.

The legacy "isa" bus node and the device nodes give the raw port
addresses (0xca2 in this case).  In order for this to work, it has to be
assumed that ISA_IO_BASE is mapped properly on the platform so that the
port I/O functions will work with the raw port addresses.

The problem I encountered was that it would call pci_address_to_pio() on
the reg address of the ipmi node just because it had the IORESOURCE_IO
flag.  This would result in the 'struct resource' getting the bogus
address of 0x10ca2 rather than 0xca2.

In my case, my real fix I have settled on is using the following instead
of using the I/O method:

        isa@0,e00f0000 {
                linux,phandle = <0x3ff93688>;
                ranges = <0x0 0x0 0x0 0xfc800000 0x1000>;
                reg = <0x0 0xe00f0000 0x0 0x10000>;
                class-code = <0x601ff>;
                device-id = <0xa008>;
                vendor-id = <0x1959>;
                #address-cells = <0x2>;
                #size-cells = <0x1>;
                device_type = "isa";

                ipmi@ca2 {
                        linux,phandle = <0x3ff93a00>;
                        reg = <0x0 0xca2 0x100>;
                        reg-spacing = <0x1>;
                        reg-size = <0x1>;
                        compatible = "ipmi-kcs";
                        device_type = "ipmi";
                };
        };

That is, the typical MMIO case so that 'struct resource' ends up with
0xfc800ca2.  This works fine without the patch of course.  If legacy I/O
port addresses are not permitted in device trees this way and we don't
want to support it, then you can ignore my patch.  However, it does seem
a little strange to me that __of_address_to_resource() assumes anything
with IORESOURCE_IO is a PCI device.

- Nate Case <ncase@xes-inc.com>

Re: [PATCH] POWERPC: Support ISA legacy addresses in of_address_to_resource()

[Benjamin Herrenschmidt]

> m to memory or something like that.
> 
> Your suspicion is correct in a way -- this allows IO port addresses to
> make their way to 'struct resource' so that drivers can use inb(),
> outb(), etc.  After some thought, I'm not sure if a device tree using
> I/O port addresses for a legacy node is considered a bug or not.
> 
> Here is the failure case
 
.../...

Your isa node is at the root of the device-tree and not behind some kind
of PCI bridge ?

In that case, I suspect that you do need a range property that converts
ISA IO addresses to toplevel memory addresses.

That is, you keep the "1" at the beginning of the reg property of ISA
devices that use ISA IO ports, but you make the bridge range property
convert from IO to memory.

Now, that might expose different issues with of_address_to_resource()
but then we can fix them.

One thing we can do for example, is to make it find legacy ISA based on
the base of the ISA bridge and turn them back into IO ports.

Ben.