Re: [PATCH v1 3/3] ARM64 LPC: update binding doc

From: Rolland Chau
Date: Sun Jan 10 2016 - 04:31:20 EST


On 2016/1/5 20:19, Arnd Bergmann wrote:
On Tuesday 05 January 2016 19:59:49 Rongrong Zou wrote:
å 2016/1/5 0:34, Arnd Bergmann åé:
On Tuesday 05 January 2016 00:04:19 Rongrong Zou wrote:
å 2016/1/4 19:13, Arnd Bergmann åé:
On Sunday 03 January 2016 20:24:14 Rongrong Zou wrote:
å 2015/12/31 23:00, Rongrong Zou åé:
Ranges property can set empty, but this means 1:1 translation. the I/O
port range is translated to MMIO address 0x00000001 00000000 to
0x00000001 00000004, it looks wrong else. I wonder if anyone get legacy
I/O port resource from dts.

As I said, nothing should really require the ranges property here, unless
you have a valid IORESOURCE_MEM translation. The code that requires
the ranges to be present is wrong.


I think the openfirmware(DT) do not support for those unmapped I/O ports, because I
must get resource by calling of_address_to_resource(), which have to call
pci_address_to_pio() when resource type is IORESOURCE_IO. I'm sorry I have no
better idea for this now. Maybe liviu can give me some opinions.

I think on x86 it works (or used to work, few people use open firmware on
x86 these days, and it may be broken), and the pci_address_to_pio() call
behaves differently when PCI_IOBASE is set. x86 never maps I/O ports into
memory mapped I/O addresses, they have their own way of accessing them
just like your platform.

/**
* of_address_to_resource - Translate device tree address and return as resource
*
* Note that if your address is a PIO address, the conversion will fail if
* the physical address can't be internally converted to an IO token with
* pci_address_to_pio(), that is because it's either called to early or it
* can't be matched to any host bridge IO space
*/
int of_address_to_resource(struct device_node *dev, int index,
struct resource *r)

The problem here seems to be that the code assumes that either the I/O ports
are always mapped or they are never mapped (no PCI_IOBASE). We need to extend
it because now we can have the combination of the two.


Arnd , please take a look at the new solution when you have a moment,thanks.
I modify the "drivers/of/address.c" to address the problem above.

There are some assumptions:

1) ISA(LPC) Like bus must be scanned before PCI.
2) a property "unmapped-size" is introduced to ISA bus DT config, it means the address
range of the bus. if bus-A have a port range(0-99), bus-B have a port range(0-199), then
the cpu port range (0-99) is reserved for bus-A and cpu port range is reserved for bus-B.
3) It it
---
drivers/of/address.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 70 insertions(+), 6 deletions(-)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index 9582c57..d3c71e5 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -682,8 +682,16 @@ struct io_range {

static LIST_HEAD(io_range_list);
static DEFINE_SPINLOCK(io_range_lock);
+phy_addr_t pci_pio_start = 0;
+EXPORT_SYMBOL(pci_pio_start)
#endif

+int isa_register_unmapped_io_range(resource_size_t size)
+{
+ pci_pio_start += size;
+}
+
+
/*
* Record the PCI IO range (expressed as CPU physical address + size).
* Return a negative value if an error has occured, zero otherwise
@@ -694,7 +702,7 @@ int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)

#ifdef PCI_IOBASE
struct io_range *range;
- resource_size_t allocated_size = 0;
+ resource_size_t allocated_size = pci_pio_start;

/* check if the range hasn't been previously recorded */
spin_lock(&io_range_lock);
@@ -743,7 +751,7 @@ phys_addr_t pci_pio_to_address(unsigned long pio)

#ifdef PCI_IOBASE
struct io_range *range;
- resource_size_t allocated_size = 0;
+ resource_size_t allocated_size = pci_pio_start;

if (pio > IO_SPACE_LIMIT)
return address;
@@ -766,7 +774,7 @@ unsigned long __weak pci_address_to_pio(phys_addr_t address)
{
#ifdef PCI_IOBASE
struct io_range *res;
- resource_size_t offset = 0;
+ resource_size_t offset = pci_pio_start;
unsigned long addr = -1;

spin_lock(&io_range_lock);
@@ -788,21 +796,77 @@ unsigned long __weak pci_address_to_pio(phys_addr_t address)
#endif
}

+static u64 of_get_unmapped_pio(struct device_node *dev, const __be32 *inaddr)
+{
+ struct device_node *np, *parent = NULL;
+ struct of_bus * bus, *pbus;
+ struct property *p_property;
+ int rlen;
+ u32 size = 0;
+ u32 port_base = 0;
+
+ /*IF NOT I/O port*/
+ if(*(in_addr) != 0x01)
+ return OF_BAD_ADDR;
+ else
+ *in_addr = 0;
+
+ parent = of_get_parent(dev);
+ if(parent == NULL)
+ return result;
+ bus = of_match_bus(parent);
+ if(!strcmp(bus->name, "isa")) {
+ p_property = of_find_property(parent, "ranges", &rlen);
+ /*no ranges property, this is a non-bridged isa bus, and the port on it is unmapped*/
+ if(p_property == NULL) {
+ for_each_node_by_name(np, "isa") {
+ if((np != parent) &&
+ !of_find_property(parent, "ranges", &rlen) &&
+ !of_property_read_u32(parent, "unmapped-size", &size)) {
+ port_base += size;
+ }
+ else if (np == parent) {
+ of_bus_default_translate(in_addr + 1, port_base, 1);
+ break;
+ }
+ else {
+ continue;
+ }
+ }
+ return of_read_number(in_addr, 2);
+
+ }
+
+ }
+ return OF_BAD_ADDR;
+}
+
static int __of_address_to_resource(struct device_node *dev,
const __be32 *addrp, u64 size, unsigned int flags,
const char *name, struct resource *r)
{
u64 taddr;
+ u8 addr_type = 0;

if ((flags & (IORESOURCE_IO | IORESOURCE_MEM)) == 0)
return -EINVAL;
taddr = of_translate_address(dev, addrp);
- if (taddr == OF_BAD_ADDR)
- return -EINVAL;
+ if (taddr == OF_BAD_ADDR) {
+
+ taddr == of_get_unmapped_pio(dev, addrp);
+ if(taddr == OF_BAD_ADDR)
+ return -EINVAL
+ else /*we get unmapped I/O port successfully*/
+ addr_type = 1;
+ }
+
memset(r, 0, sizeof(struct resource));
if (flags & IORESOURCE_IO) {
unsigned long port;
- port = pci_address_to_pio(taddr);
+ if(!addr_type)
+ port = pci_address_to_pio(taddr);
+ else
+ port = (unsigned long)taddr;
if (port == (unsigned long)-1)
return -EINVAL;
r->start = port;
--


In bus device probe code
static int lpc_probe(struct platform_device *pdev)
{
...
if(ret = of_property_read_u32((pdev->dev).of_node, "unmapped-size", &io_ranges))
return ret;

lpc_dev->bus_size = io_ranges;
isa_register_io_range(io_ranges);
ret = of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
...

return 0;
}


For ipmi driver, I can get I/O port resource by DMI rather than dts.

No, the ipmi driver uses the resource that belongs to the platform
device already, you can't rely on DMI data to be present there.

Ipmi has a lot of way to be discovered(ACPI, DMI, hardcoded, hot-add,
openfirmware and a few other), I think we just use one of them, not all of them.
It depend on vendor's hardware solution actually.

I don't think we should mix multiple methods here: if the bus is described
in DT, all its children should be there as well. Otherwise you get into problems
e.g. if you have multiple instances of the LPC bus and the Linux I/O addresses
for one or more of them have an offset to the bus specific addresses.

The bus probe code decides what the Linux I/O port numbers are, but DMI
and other methods have no idea of the mapping. As long as there is only
one instance, using the first 0x1000 addresses with a 1:1 mapping saves
us a bit of trouble, but I'd be worried about relying on that assumption
too much.

Arnd