POWERPC: Support ISA legacy addresses in of_address_to_resource()
PATCH 1/1 POWERPC: Support ISA legacy addresses in of_address_to_resource()
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.
--- arch/powerpc/kernel/prom_parse.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-)
arch/powerpc/kernel/prom_parse.c +9 -1
@@ -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;
Signature · 1 line
1.5.4.4
Quoted 7 lines · 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.
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.
Quoted 26 lines
Signed-off-by Nate Case <[email protected]>--- arch/powerpc/kernel/prom_parse.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-)arch/powerpc/kernel/prom_parse.c +9
-1@@ -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;
Quoted 7 lines · Benjamin Herrenschmidt
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 <[email protected]>
Quoted 8 lines
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.