Re: [PATCH v4 04/10] of: irq: add helper to remap interrupts to another irqdomain

Lina Iyer
2019-03-22T17:43:58Z

I observed an issue with return value being set to 0, when a match is
not found in the irqdomain-map.

On Wed, Mar 13 2019 at 15:20 -0600, Lina Iyer wrote:
>From: Stephen Boyd <swboyd@chromium.org>
>
>Sometimes interrupts are routed from an interrupt controller to another
>in no specific order. Having these in the drivers makes it difficult to
>maintain when the same drivers supports multiple variants with different
>mapping. Also, specifying them in DT makes little sense with a bunch of
>numbers like -
> <0, 13>, <5, 32>,
>
>It makes more sense when we can have the parent handle along with
>interrupt specifiers for the incoming interrupt as well as that of the
>outgoing interrupt like -
> <22 0 &intc 36 0>,
> <24 0 &intc 37 0>,
> <26 0 &intc 38 0>,
>
>And the interrupt specifiers can be interpreted using these optional
>properties -
> irqdomain-map-mask = <0xff 0>;
> irqdomain-map-pass-thru = <0 0xff>;
>
>The irqdomain-map-mask reads the input interrupt specifier to parse the
>incoming interrupt port. The format of the output port is specified with
>the irqdomain-map-pass-thru property.
>
>Let's add a helper function to parse this from DT and match a struct
>irq_fwspec using the input interrupt specifier from the irqdomain-map
>and the valid bits specified in the irqdomain-map-mask and copy the
>output interrupt specifier from the map to irq_fwspec per the mask in
>irqdomain-map-pass-thru property for the matched interrupt.
>
>Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>Signed-off-by: Lina Iyer <ilina@codeaurora.org>
>---
>Changes in v4:
> - Fix commit text spelling and verbosity
>---
> drivers/of/irq.c | 125 +++++++++++++++++++++++++++++++++++++++++
> include/linux/of_irq.h | 1 +
> 2 files changed, 126 insertions(+)
>
>diff --git a/drivers/of/irq.c b/drivers/of/irq.c
>index e1f6f392a4c0..a1534f947ed4 100644
>--- a/drivers/of/irq.c
>+++ b/drivers/of/irq.c
>@@ -273,6 +273,131 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
> }
> EXPORT_SYMBOL_GPL(of_irq_parse_raw);
>
>+int of_irq_domain_map(const struct irq_fwspec *in, struct irq_fwspec *out)
>+{
>+ char *stem_name;
>+ char *cells_name, *map_name = NULL, *mask_name = NULL;
>+ char *pass_name = NULL;
>+ struct device_node *cur, *new = NULL;
>+ const __be32 *map, *mask, *pass;
>+ static const __be32 dummy_mask[] = { [0 ... MAX_PHANDLE_ARGS] = ~0 };
>+ static const __be32 dummy_pass[] = { [0 ... MAX_PHANDLE_ARGS] = 0 };
>+ __be32 initial_match_array[MAX_PHANDLE_ARGS];
>+ const __be32 *match_array = initial_match_array;
>+ int i, ret, map_len, match;
>+ u32 in_size, out_size;
>+
>+ stem_name = "";
>+ cells_name = "#interrupt-cells";
>+
>+ ret = -ENOMEM;
>+ map_name = kasprintf(GFP_KERNEL, "irqdomain%s-map", stem_name);
>+ if (!map_name)
>+ goto free;
>+
>+ mask_name = kasprintf(GFP_KERNEL, "irqdomain%s-map-mask", stem_name);
>+ if (!mask_name)
>+ goto free;
>+
>+ pass_name = kasprintf(GFP_KERNEL, "irqdomain%s-map-pass-thru", stem_name);
>+ if (!pass_name)
>+ goto free;
>+
>+ /* Get the #interrupt-cells property */
>+ cur = to_of_node(in->fwnode);
>+ ret = of_property_read_u32(cur, cells_name, &in_size);
>+ if (ret < 0)
>+ goto put;
>+
>+ /* Precalculate the match array - this simplifies match loop */
>+ for (i = 0; i < in_size; i++)
>+ initial_match_array[i] = cpu_to_be32(in->param[i]);
>+
>+ ret = -EINVAL;
This value...
>+ /* Get the irqdomain-map property */
>+ map = of_get_property(cur, map_name, &map_len);
>+ if (!map) {
>+ ret = 0;
>+ goto free;
>+ }
>+ map_len /= sizeof(u32);
>+
>+ /* Get the irqdomain-map-mask property (optional) */
>+ mask = of_get_property(cur, mask_name, NULL);
>+ if (!mask)
>+ mask = dummy_mask;
>+ /* Iterate through irqdomain-map property */
>+ match = 0;
>+ while (map_len > (in_size + 1) && !match) {
>+ /* Compare specifiers */
>+ match = 1;
>+ for (i = 0; i < in_size; i++, map_len--)
>+ match &= !((match_array[i] ^ *map++) & mask[i]);
>+
>+ of_node_put(new);
>+ new = of_find_node_by_phandle(be32_to_cpup(map));
>+ map++;
>+ map_len--;
>+
>+ /* Check if not found */
>+ if (!new)
>+ goto put;
>+
>+ if (!of_device_is_available(new))
>+ match = 0;
>+
>+ ret = of_property_read_u32(new, cells_name, &out_size);
Gets overwritten by this, and set to 0.
>+ if (ret)
>+ goto put;
>+
>+ /* Check for malformed properties */
>+ if (WARN_ON(out_size > MAX_PHANDLE_ARGS))
>+ goto put;
>+ if (map_len < out_size)
>+ goto put;
>+
>+ /* Move forward by new node's #interrupt-cells amount */
>+ map += out_size;
>+ map_len -= out_size;
>+ }
>+ if (match) {
If match is not found, then ret remains 0.

>+ /* Get the irqdomain-map-pass-thru property (optional) */
>+ pass = of_get_property(cur, pass_name, NULL);
>+ if (!pass)
>+ pass = dummy_pass;
>+
>+ /*
>+ * Successfully parsed a irqdomain-map translation; copy new
>+ * specifier into the out structure, keeping the
>+ * bits specified in irqdomain-map-pass-thru.
>+ */
>+ match_array = map - out_size;
>+ for (i = 0; i < out_size; i++) {
>+ __be32 val = *(map - out_size + i);
>+
>+ out->param[i] = in->param[i];
>+ if (i < in_size) {
>+ val &= ~pass[i];
>+ val |= cpu_to_be32(out->param[i]) & pass[i];
>+ }
>+
>+ out->param[i] = be32_to_cpu(val);
>+ }
>+ out->param_count = in_size = out_size;
>+ out->fwnode = of_node_to_fwnode(new);
>+ }
>+put:
>+ of_node_put(cur);
>+ of_node_put(new);
>+free:
>+ kfree(mask_name);
>+ kfree(map_name);
>+ kfree(pass_name);
>+
>+ return ret;
>+}
>+EXPORT_SYMBOL(of_irq_domain_map);
>+
> /**
> * of_irq_parse_one - Resolve an interrupt for a device
> * @device: the device whose interrupt is to be resolved
>diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
>index 1214cabb2247..86342502a62a 100644
>--- a/include/linux/of_irq.h
>+++ b/include/linux/of_irq.h
>@@ -32,6 +32,7 @@ static inline int of_irq_parse_oldworld(struct device_node *device, int index,
> }
> #endif /* CONFIG_PPC32 && CONFIG_PPC_PMAC */
>
>+extern int of_irq_domain_map(const struct irq_fwspec *in, struct irq_fwspec *out);
> extern int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq);
> extern int of_irq_parse_one(struct device_node *device, int index,
> struct of_phandle_args *out_irq);
>--
>The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>a Linux Foundation Collaborative Project
>
--8<---
diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 2653abfaf09d..e12f27403b3a 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -361,32 +361,36 @@ int of_irq_domain_map(const struct irq_fwspec *in, struct irq_fwspec *out)
map += out_size;
map_len -= out_size;
}
- if (match) {
- /* Get the irqdomain-map-pass-thru property (optional) */
- pass = of_get_property(cur, pass_name, NULL);
- if (!pass)
- pass = dummy_pass;

- /*
- * Successfully parsed a irqdomain-map translation; copy new
- * specifier into the out structure, keeping the
- * bits specified in irqdomain-map-pass-thru.
- */
- match_array = map - out_size;
- for (i = 0; i < out_size; i++) {
- __be32 val = *(map - out_size + i);
-
- out->param[i] = in->param[i];
- if (i < in_size) {
- val &= ~pass[i];
- val |= cpu_to_be32(out->param[i]) & pass[i];
- }
+ if (!match) {
+ ret = -EINVAL;
+ goto put;
+ }

- out->param[i] = be32_to_cpu(val);
+ /* Get the irqdomain-map-pass-thru property (optional) */
+ pass = of_get_property(cur, pass_name, NULL);
+ if (!pass)
+ pass = dummy_pass;
+
+ /*
+ * Successfully parsed a irqdomain-map translation; copy new
+ * specifier into the out structure, keeping the
+ * bits specified in irqdomain-map-pass-thru.
+ */
+ match_array = map - out_size;
+ for (i = 0; i < out_size; i++) {
+ __be32 val = *(map - out_size + i);
+
+ out->param[i] = in->param[i];
+ if (i < in_size) {
+ val &= ~pass[i];
+ val |= cpu_to_be32(out->param[i]) & pass[i];
}
- out->param_count = in_size = out_size;
- out->fwnode = of_node_to_fwnode(new);
+
+ out->param[i] = be32_to_cpu(val);
}
+ out->param_count = in_size = out_size;
+ out->fwnode = of_node_to_fwnode(new);
put:
of_node_put(cur);
of_node_put(new);