On Fri, Oct 21, 2016 at 10:49:11AM +0200, Jerome Brunet wrote:

On Thu, 2016-10-20 at 17:33 +0100, Marc Zyngier wrote:

quoted

On 19/10/16 16:21, Jerome Brunet wrote:

quoted

+struct meson_gpio_irq_chip_data {
+	void __iomem *base;
+	int index;
+};
+
+static irq_hw_number_t meson_parent_hwirqs[] = {
+	64, 65, 66, 67, 68, 69, 70, 71,
+};

If that a guarantee that these numbers will always represent the
parent interrupt?

At the moment, the 3 supported SoC use these parent interrupts, but we
have absolutely no idea (or guarantee) that is will remain the same, or
even contiguous, in the upcoming SoC (like the GXM or GXL)

I reckon, it is likely that manufacturer will keep on using these
parent irqs for a while but I would prefer not make an assumption about
it in the driver.

If a SoC get a different set of interrupts I would have added a new
table like this and passed it to the appropriate params :

static irq_hw_number_t meson_new_parent_hwirqs[] = {
	143, 144, 150, 151, 152, 173, 178, 179,
};

quoted

It feels a bit odd not to get that information directly from
the device tree, in the form of a device specific property. Something
like:

	upstream-interrupts = <64 65 66 ... >;

I wondered about putting this information in DT or in the driver for a
while. Maybe DT would be a more suitable place holder for these data
(parent irq and number of provided hwirq) but I was under the
understanding that we should now put these information in the driver
and use the compatible property to get the appropriate parameters.

I'd love to get the view of the DT guys on this.

Please describe inter-device relationships in DT when you are aware of
them. The SoC-specific compatible string is more of a future-proofing
thing / last restort for things we realise too late.

To be clear, we should *also* have an soc-specific compatible string,
but for differences we already know about, we should use DT properties.

quoted

quoted

+static const struct meson_gpio_irq_params meson8b_params = {
+	.nhwirq??= 119,
+	.source??= meson_parent_hwirqs,
+	.nsource = ARRAY_SIZE(meson_parent_hwirqs),
+};
+
+static const struct meson_gpio_irq_params meson_gxbb_params = {
+	.nhwirq??= 133,
+	.source??= meson_parent_hwirqs,
+	.nsource = ARRAY_SIZE(meson_parent_hwirqs),
+};

Same thing. How big is the variability of these structures? Are we
going to see more of those? or is that now set into stone?

The number of pad mapped to the controller seems to change with every
SoC version. The parent irqs have not changed so far, but as explained
above, there is no guarantee it will keep on being this way.

So i'd say probably more of those ...

quoted

+Mark: what's the policy to describe this kind of things?

Generally, I'd prefer that we describe this in DT rather than
accumulating a set of string -> number mappings in the driver.

Thanks,
Mark.