Re: [PATCH V3 2/4] ARM64 LPC: LPC driver implementation on Hip06
From: Arnd Bergmann <arnd@arndb.de>
Date: 2016-09-22 12:15:56
Also in:
linux-arm-kernel, linux-pci, linux-serial, lkml
Subsystem:
open firmware and flattened device tree, the rest · Maintainers:
Rob Herring, Saravana Kannan, Linus Torvalds
On Thursday, September 22, 2016 11:55:45 AM CEST Gabriele Paoloni wrote:
quoted hunk ↗ jump to hunk
quoted
quoted
I think extending of_empty_ranges_quirk() may be a reasonablesolution.quoted
What do you think Arnd?I don't really like that idea, that quirk is meant to work around broken DTs, but we can just make the DT valid and implement the code properly.Ok I understand your point where it is not right to use of_empty_ranges_quirk() As a quirk is used to work around broken HW or broken FW (as in this case) rather than to fix code What about the following? I think adding the check you suggested next to of_empty_ranges_quirk() is adding the case we need in the right point (thus avoiding any duplication)--- a/drivers/of/address.c +++ b/drivers/of/address.c@@ -457,6 +457,15 @@ static struct of_bus *of_match_bus(struct device_node *np) return NULL; } +static inline int of_isa_indirect_io(struct device_node *np) +{ + /* + * check if the current node is an isa bus and if indirectio operation + * are registered + */ + return (of_bus_isa_match(np) && arm64_extio_ops); +} + static int of_empty_ranges_quirk(struct device_node *np) { if (IS_ENABLED(CONFIG_PPC)) {@@ -503,7 +512,7 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus, * This code is only enabled on powerpc. --gcl */ ranges = of_get_property(parent, rprop, &rlen); - if (ranges == NULL && !of_empty_ranges_quirk(parent)) { + if (ranges == NULL && !of_empty_ranges_quirk(parent) && !of_isa_indirect_io(parent)) { pr_debug("OF: no ranges; cannot translate\n"); return 1; }
I don't see what effect that would have. What do you want to achieve with this? I think all we need from this function is to return '1' if we hit an ISA I/O window, and that should happen for the two interesting cases, either no 'ranges' at all, or no translation for the range in question, so that __of_translate_address can return OF_BAD_ADDR, and we can enter the special case handling in the caller, that handles it like
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 02b2903fe9d2..a18d96843fae 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c@@ -685,17 +685,24 @@ static int __of_address_to_resource(struct device_node *dev, if ((flags & (IORESOURCE_IO | IORESOURCE_MEM)) == 0) return -EINVAL; taddr = of_translate_address(dev, addrp); - if (taddr == OF_BAD_ADDR) - return -EINVAL; memset(r, 0, sizeof(struct resource)); + if (flags & IORESOURCE_IO) { unsigned long port; - port = pci_address_to_pio(taddr); + + if (taddr == OF_BAD_ADDR) + port = arch_of_address_to_pio(dev, addrp) + else + port = pci_address_to_pio(taddr); + if (port == (unsigned long)-1) return -EINVAL; r->start = port; r->end = port + size - 1; } else { + if (taddr == OF_BAD_ADDR) + return -EINVAL; + r->start = taddr; r->end = taddr + size - 1; }
Arnd