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;
}
Arnd
