Re: [RFC PATCH v3] irqchip: Add support for Tango interrupt controller

Marc Gonzalez [off-list ref] writes:

On 20/01/2016 17:38, M?ns Rullg?rd wrote:

quoted

Marc Gonzalez [off-list ref] writes:

quoted

On 20/01/2016 17:25, M?ns Rullg?rd wrote:

quoted

Marc Zyngier [off-list ref] writes:

quoted

On 20/01/16 16:10, M?ns Rullg?rd wrote:

quoted

Marc Zyngier [off-list ref] writes:

quoted

quoted

+	if (of_property_read_u32(node, "reg", &ctl))
+		panic("%s: failed to get reg base", node->name);
+
+	chip = kzalloc(sizeof(*chip), GFP_KERNEL);
+	chip->ctl = ctl;
+	chip->base = base;

As I said before, this assumes the outer DT node uses a ranges
property.  Normally reg properties work the same whether they specify an
offset within an outer "ranges" or have a full address directly.  It
would be easy enough to make this work with either, so I don't see any
reason not to.

Yup, that is a good point. I guess Marc can address this in the next
round, since we need a DT binding anyway.

I'd suggest using of_address_to_resource() on both nodes and subtracting
the start addresses returned.

For my own reference, Marc Zyngier suggested:
"you should use of_iomap to map the child nodes, and not mess with
the parent one."

That's going to get very messy since the generic irqchip code needs all
the registers as offsets from a common base address.

The two suggestions are over my head at the moment.

1 more quoted lines

Done.  If this isn't acceptable either, I'm out of ideas that don't end
up being far uglier than anything suggested so far.

-- 
M?ns Rullg?rd

On 20/01/2016 19:09, M?ns Rullg?rd wrote:

Marc Gonzalez [off-list ref] writes:

quoted

On 20/01/2016 17:38, M?ns Rullg?rd wrote:

quoted

Marc Gonzalez [off-list ref] writes:

quoted

On 20/01/2016 17:25, M?ns Rullg?rd wrote:

quoted

Marc Zyngier [off-list ref] writes:

quoted

On 20/01/16 16:10, M?ns Rullg?rd wrote:

quoted

Marc Zyngier [off-list ref] writes:

quoted

quoted

+	if (of_property_read_u32(node, "reg", &ctl))
+		panic("%s: failed to get reg base", node->name);
+
+	chip = kzalloc(sizeof(*chip), GFP_KERNEL);
+	chip->ctl = ctl;
+	chip->base = base;

As I said before, this assumes the outer DT node uses a ranges
property.  Normally reg properties work the same whether they specify an
offset within an outer "ranges" or have a full address directly.  It
would be easy enough to make this work with either, so I don't see any
reason not to.

Yup, that is a good point. I guess Marc can address this in the next
round, since we need a DT binding anyway.

I'd suggest using of_address_to_resource() on both nodes and subtracting
the start addresses returned.

For my own reference, Marc Zyngier suggested:
"you should use of_iomap to map the child nodes, and not mess with
the parent one."

That's going to get very messy since the generic irqchip code needs all
the registers as offsets from a common base address.

6 more quoted lines

With your latest patch, can I drop the ranges property?

Regards.