On 09/22/2016 08:14 PM, Arnd Bergmann wrote:
On Thursday, September 22, 2016 11:55:45 AM CEST Gabriele Paoloni wrote:
I think extending of_empty_ranges_quirk() may be a reasonable
solution.
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;
}
For this patch sketch, I have a question.
Do we call pci_address_to_pio in arch_of_address_to_pio to get the
corresponding logical IO port
for LPC??
If we don't, it seems the LPC specific IO address will conflict with PCI
host bridges' logical IO.
Supposed our LPC populated the IO range from 0x100 to 0x3FF( this is
normal for ISA similar
devices), after arch_of_address_to_pio(), the r->start will be set as
0x100, r->end will be set as
0x3FF. And if there is one PCI host bridge who request a IO window size
over 0x400 at the same
time, the corresponding r->start and r->end will be set as 0x0, 0x3FF
after of_address_to_resource
for this host bridge. Then the IO conflict happens.
cheers,
Zhichang
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
