On Mon, Aug 14, 2017 at 04:48:07PM -0500, Rob Herring wrote:
> Add a check for nodes with interrupts property that they have a valid
> parent, the parent has #interrupt-cells property, and the size is a
> valid multiple of #interrupt-cells.
>
> This may not handle every possible case and doesn't deal with
> translation thru interrupt-map properties, but should be enough for
> modern dts files.
>
> Signed-off-by: Rob Herring <robh@xxxxxxxxxx>
> ---
> checks.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 58 insertions(+)
>
> diff --git a/checks.c b/checks.c
> index c0450e118043..0d452bf8e674 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -1070,6 +1070,63 @@ static void check_gpio_cells_property(struct check *c,
> }
> WARNING(gpio_cells_property, check_gpio_cells_property, NULL);
>
> +static void check_interrupts_property(struct check *c,
> + struct dt_info *dti,
> + struct node *node)
> +{
> + struct node *root = dti->dt;
> + struct node *irq_node = NULL, *parent = node;
> + struct property *irq_prop, *prop = NULL;
> + int irq_cells, phandle;
> +
> + irq_prop = get_property(node, "interrupts");
> + if (!irq_prop)
> + return;
> +
> + while (parent && !prop) {
> + if (parent != node) {
So, it's kind of academic, but is it actually disallowed for an
interrupt-controller node to itself have interrupts which are
implicityly routed to itself?
> + prop = get_property(parent, "interrupt-controller");
> + if (prop) {
> + irq_node = parent;
> + break;
> + }
> + }
> +
> + prop = get_property(parent, "interrupt-parent");
> + if (prop) {
> + phandle = propval_cell(prop);
> + irq_node = get_node_by_phandle(root, phandle);
> + if (!irq_node) {
> + FAIL(c, dti, "Bad interrupt-parent phandle for %s",
> + node->fullpath);
> + return;
> + }
> + break;
> + }
As noted you also need a check for interrupt-map.
> +
> + parent = parent->parent;
> + }
> +
> + if (!irq_node) {
> + FAIL(c, dti, "Missing interrupt-parent for %s", node->fullpath);
> + return;
> + }
> +
> + prop = get_property(irq_node, "#interrupt-cells");
> + if (!prop) {
> + FAIL(c, dti, "Missing #interrupt-cells in interrupt-parent %s",
> + irq_node->fullpath);
> + return;
> + }
> +
> + irq_cells = propval_cell(prop);
So this is unsafe if #interrupt-cells is misformatted in the interrupt
controller. There's already a test for that (using
WARNING_IF_NOT_CELL), so you just need to make this check dependent on
that one.
> + if (irq_prop->val.len % (irq_cells * sizeof(cell_t))) {
> + FAIL(c, dti, "interrupts size is (%d), expected multiple of %d in %s",
> + irq_prop->val.len, (int)(irq_cells * sizeof(cell_t)), node->fullpath);
> + }
> +}
> +WARNING(interrupts_property, check_interrupts_property, &phandle_references);
> +
> static struct check *check_table[] = {
> &duplicate_node_names, &duplicate_property_names,
> &node_name_chars, &node_name_format, &property_name_chars,
> @@ -1103,6 +1160,7 @@ static struct check *check_table[] = {
>
> &provider_cells_property,
> &gpio_cells_property,
> + &interrupts_property,
>
> &always_fail,
> };
With both these patches testcases to make sure the checks actually
trip on a bad example would be good.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature
