* [PATCH v2 1/5] checks: Make interrupt_provider check dependent on interrupts_extended_is_cell @ 2021-10-11 19:12 Rob Herring [not found] ` <20211011191245.1009682-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 0 siblings, 1 reply; 12+ messages in thread From: Rob Herring @ 2021-10-11 19:12 UTC (permalink / raw) To: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA, David Gibson; +Cc: Andre Przywara If '#interrupt-cells' doesn't pass checks, no reason to run interrupt provider check. Cc: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> --- checks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checks.c b/checks.c index fb3fc9cda4b1..8153793a3e7d 100644 --- a/checks.c +++ b/checks.c @@ -1587,7 +1587,7 @@ static void check_interrupt_provider(struct check *c, FAIL(c, dti, node, "Missing #address-cells in interrupt provider"); } -WARNING(interrupt_provider, check_interrupt_provider, NULL); +WARNING(interrupt_provider, check_interrupt_provider, NULL, &interrupts_extended_is_cell); static void check_interrupts_property(struct check *c, struct dt_info *dti, -- 2.30.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
[parent not found: <20211011191245.1009682-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* [PATCH v2 2/5] checks: Add an interrupt-map check [not found] ` <20211011191245.1009682-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> @ 2021-10-11 19:12 ` Rob Herring [not found] ` <20211011191245.1009682-2-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2021-10-11 19:12 ` [PATCH v2 3/5] checks: Drop interrupt provider '#address-cells' check Rob Herring ` (3 subsequent siblings) 4 siblings, 1 reply; 12+ messages in thread From: Rob Herring @ 2021-10-11 19:12 UTC (permalink / raw) To: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA, David Gibson; +Cc: Andre Przywara Add a check for parsing 'interrupt-map' properties. The check primarily tests parsing 'interrupt-map' properties which depends on and the parent interrupt controller (or another map) node. Note that this does not require '#address-cells' in the interrupt-map parent, but treats missing '#address-cells' as 0 which is how the Linux kernel parses it. There's numerous cases that expect this behavior. Cc: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> --- v2: - Rework '#interrupt-cells' and '#address-cells' checks to rely on other checks (interrupt_provider and addr_size_cells) --- checks.c | 82 ++++++++++++++++++++++++++++++ tests/bad-interrupt-map-mask.dts | 20 ++++++++ tests/bad-interrupt-map-parent.dts | 17 +++++++ tests/bad-interrupt-map.dts | 19 +++++++ tests/run_tests.sh | 3 ++ 5 files changed, 141 insertions(+) create mode 100644 tests/bad-interrupt-map-mask.dts create mode 100644 tests/bad-interrupt-map-parent.dts create mode 100644 tests/bad-interrupt-map.dts diff --git a/checks.c b/checks.c index 8153793a3e7d..a72ae4cc0be9 100644 --- a/checks.c +++ b/checks.c @@ -1589,6 +1589,87 @@ static void check_interrupt_provider(struct check *c, } WARNING(interrupt_provider, check_interrupt_provider, NULL, &interrupts_extended_is_cell); +static void check_interrupt_map(struct check *c, + struct dt_info *dti, + struct node *node) +{ + struct node *root = dti->dt; + struct property *prop, *irq_map_prop; + size_t cellsize, cell, map_cells; + + irq_map_prop = get_property(node, "interrupt-map"); + if (!irq_map_prop) + return; + + if (node->addr_cells < 0) { + FAIL(c, dti, node, + "Missing '#address-cells' in interrupt-map provider"); + return; + } + cellsize = node_addr_cells(node); + cellsize += propval_cell(get_property(node, "#interrupt-cells")); + + prop = get_property(node, "interrupt-map-mask"); + if (prop && (prop->val.len != (cellsize * sizeof(cell_t)))) + FAIL_PROP(c, dti, node, prop, + "property size (%d) is invalid, expected %zu", + prop->val.len, cellsize * sizeof(cell_t)); + + if (!is_multiple_of(irq_map_prop->val.len, sizeof(cell_t))) { + FAIL_PROP(c, dti, node, irq_map_prop, + "property size (%d) is invalid, expected multiple of %zu", + irq_map_prop->val.len, sizeof(cell_t)); + return; + } + + map_cells = irq_map_prop->val.len / sizeof(cell_t); + for (cell = 0; cell < map_cells; ) { + struct node *provider_node; + struct property *cellprop; + int phandle; + size_t parent_cellsize; + + if ((cell + cellsize) >= map_cells) { + FAIL_PROP(c, dti, node, irq_map_prop, + "property size (%d) too small, expected > %zu", + irq_map_prop->val.len, (cell + cellsize) * sizeof(cell_t)); + break; + } + + phandle = propval_cell_n(irq_map_prop, cell + cellsize); + + if ((phandle == 0) || (phandle == -1)) { + FAIL_PROP(c, dti, node, irq_map_prop, + "Cell %zu is not a phandle(%d)", + cell + cellsize, phandle); + break; + } + provider_node = get_node_by_phandle(root, phandle); + if (!provider_node) { + FAIL_PROP(c, dti, node, irq_map_prop, + "Could not get phandle node for (cell %zu)", + cell); + break; + } + + cellprop = get_property(provider_node, "#interrupt-cells"); + if (cellprop) { + parent_cellsize = propval_cell(cellprop); + } else { + FAIL(c, dti, node, "Missing property '#interrupt-cells' in node %s or bad phandle (referred from interrupt-map[%zu])", + provider_node->fullpath, cell); + break; + } + + cellprop = get_property(provider_node, "#address-cells"); + if (cellprop) + parent_cellsize += propval_cell(cellprop); + + cell += cellsize + 1 + parent_cellsize; + } +} +WARNING(interrupt_map, check_interrupt_map, NULL, &phandle_references, &addr_size_cells, &interrupt_provider); + static void check_interrupts_property(struct check *c, struct dt_info *dti, struct node *node) @@ -1887,6 +1968,7 @@ static struct check *check_table[] = { &gpios_property, &interrupts_property, &interrupt_provider, + &interrupt_map, &alias_paths, diff --git a/tests/bad-interrupt-map-mask.dts b/tests/bad-interrupt-map-mask.dts new file mode 100644 index 000000000000..10eaffd62310 --- /dev/null +++ b/tests/bad-interrupt-map-mask.dts @@ -0,0 +1,20 @@ +/dts-v1/; + +/ { + interrupt-parent = <&intc>; + intc: interrupt-controller { + #interrupt-cells = <3>; + interrupt-controller; + }; + + node { + #address-cells = <0>; + #interrupt-cells = <1>; + interrupt-map = <1 &intc 1 2 3>; + interrupt-map-mask = <0 0>; + + child { + interrupts = <1>; + }; + }; +}; diff --git a/tests/bad-interrupt-map-parent.dts b/tests/bad-interrupt-map-parent.dts new file mode 100644 index 000000000000..fe88ce2fe76f --- /dev/null +++ b/tests/bad-interrupt-map-parent.dts @@ -0,0 +1,17 @@ +/dts-v1/; + +/ { + interrupt-parent = <&intc>; + intc: interrupt-controller { + }; + + node { + #address-cells = <0>; + #interrupt-cells = <1>; + interrupt-map = <1 &intc 1 2 3>; + + child { + interrupts = <1>; + }; + }; +}; diff --git a/tests/bad-interrupt-map.dts b/tests/bad-interrupt-map.dts new file mode 100644 index 000000000000..6df8f93ae00c --- /dev/null +++ b/tests/bad-interrupt-map.dts @@ -0,0 +1,19 @@ +/dts-v1/; + +/ { + interrupt-parent = <&intc>; + intc: interrupt-controller { + #interrupt-cells = <3>; + interrupt-controller; + }; + + node { + /* Missing #address-cells = <0>; */ + #interrupt-cells = <1>; + interrupt-map = <1 &intc 1 2 3>; + + child { + interrupts = <1>; + }; + }; +}; diff --git a/tests/run_tests.sh b/tests/run_tests.sh index 0e270feb3e47..d100d5aaa21f 100755 --- a/tests/run_tests.sh +++ b/tests/run_tests.sh @@ -717,6 +717,9 @@ dtc_tests () { run_sh_test "$SRCDIR/dtc-checkfails.sh" -n deprecated_gpio_property -- -Wdeprecated_gpio_property -I dts -O dtb "$SRCDIR/good-gpio.dts" check_tests "$SRCDIR/bad-interrupt-cells.dts" interrupts_property check_tests "$SRCDIR/bad-interrupt-controller.dts" interrupt_provider + check_tests "$SRCDIR/bad-interrupt-map.dts" interrupt_map + check_tests "$SRCDIR/bad-interrupt-map-parent.dts" interrupt_map + check_tests "$SRCDIR/bad-interrupt-map-mask.dts" interrupt_map run_sh_test "$SRCDIR/dtc-checkfails.sh" node_name_chars -- -I dtb -O dtb bad_node_char.dtb run_sh_test "$SRCDIR/dtc-checkfails.sh" node_name_format -- -I dtb -O dtb bad_node_format.dtb run_sh_test "$SRCDIR/dtc-checkfails.sh" property_name_chars -- -I dtb -O dtb bad_prop_char.dtb -- 2.30.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
[parent not found: <20211011191245.1009682-2-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH v2 2/5] checks: Add an interrupt-map check [not found] ` <20211011191245.1009682-2-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> @ 2021-10-14 3:44 ` David Gibson 0 siblings, 0 replies; 12+ messages in thread From: David Gibson @ 2021-10-14 3:44 UTC (permalink / raw) To: Rob Herring; +Cc: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA, Andre Przywara [-- Attachment #1: Type: text/plain, Size: 7649 bytes --] On Mon, Oct 11, 2021 at 02:12:42PM -0500, Rob Herring wrote: > Add a check for parsing 'interrupt-map' properties. The check primarily > tests parsing 'interrupt-map' properties which depends on and the parent > interrupt controller (or another map) node. > > Note that this does not require '#address-cells' in the interrupt-map > parent, but treats missing '#address-cells' as 0 which is how the Linux > kernel parses it. There's numerous cases that expect this behavior. > > Cc: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> > Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> LGTM, with one exception: > --- > v2: > - Rework '#interrupt-cells' and '#address-cells' checks to rely on > other checks (interrupt_provider and addr_size_cells) > --- > checks.c | 82 ++++++++++++++++++++++++++++++ > tests/bad-interrupt-map-mask.dts | 20 ++++++++ > tests/bad-interrupt-map-parent.dts | 17 +++++++ > tests/bad-interrupt-map.dts | 19 +++++++ > tests/run_tests.sh | 3 ++ > 5 files changed, 141 insertions(+) > create mode 100644 tests/bad-interrupt-map-mask.dts > create mode 100644 tests/bad-interrupt-map-parent.dts > create mode 100644 tests/bad-interrupt-map.dts > > diff --git a/checks.c b/checks.c > index 8153793a3e7d..a72ae4cc0be9 100644 > --- a/checks.c > +++ b/checks.c > @@ -1589,6 +1589,87 @@ static void check_interrupt_provider(struct check *c, > } > WARNING(interrupt_provider, check_interrupt_provider, NULL, &interrupts_extended_is_cell); > > +static void check_interrupt_map(struct check *c, > + struct dt_info *dti, > + struct node *node) > +{ > + struct node *root = dti->dt; > + struct property *prop, *irq_map_prop; > + size_t cellsize, cell, map_cells; > + > + irq_map_prop = get_property(node, "interrupt-map"); > + if (!irq_map_prop) > + return; > + > + if (node->addr_cells < 0) { > + FAIL(c, dti, node, > + "Missing '#address-cells' in interrupt-map provider"); > + return; > + } > + cellsize = node_addr_cells(node); > + cellsize += propval_cell(get_property(node, "#interrupt-cells")); > + > + prop = get_property(node, "interrupt-map-mask"); > + if (prop && (prop->val.len != (cellsize * sizeof(cell_t)))) > + FAIL_PROP(c, dti, node, prop, > + "property size (%d) is invalid, expected %zu", > + prop->val.len, cellsize * sizeof(cell_t)); > + > + if (!is_multiple_of(irq_map_prop->val.len, sizeof(cell_t))) { > + FAIL_PROP(c, dti, node, irq_map_prop, > + "property size (%d) is invalid, expected multiple of %zu", > + irq_map_prop->val.len, sizeof(cell_t)); > + return; > + } > + > + map_cells = irq_map_prop->val.len / sizeof(cell_t); > + for (cell = 0; cell < map_cells; ) { > + struct node *provider_node; > + struct property *cellprop; > + int phandle; > + size_t parent_cellsize; > + > + if ((cell + cellsize) >= map_cells) { > + FAIL_PROP(c, dti, node, irq_map_prop, > + "property size (%d) too small, expected > %zu", > + irq_map_prop->val.len, (cell + cellsize) * sizeof(cell_t)); > + break; > + } > + > + phandle = propval_cell_n(irq_map_prop, cell + cellsize); > + > + if ((phandle == 0) || (phandle == -1)) { > + FAIL_PROP(c, dti, node, irq_map_prop, > + "Cell %zu is not a phandle(%d)", > + cell + cellsize, phandle); AFAICT this will incorrectly fail when run on a dtsi which has an unresolved reference in an interrupt-map. > + break; > + } > + provider_node = get_node_by_phandle(root, phandle); > + if (!provider_node) { > + FAIL_PROP(c, dti, node, irq_map_prop, > + "Could not get phandle node for (cell %zu)", > + cell); > + break; > + } > + > + cellprop = get_property(provider_node, "#interrupt-cells"); > + if (cellprop) { > + parent_cellsize = propval_cell(cellprop); > + } else { > + FAIL(c, dti, node, "Missing property '#interrupt-cells' in node %s or bad phandle (referred from interrupt-map[%zu])", > + provider_node->fullpath, cell); > + break; > + } > + > + cellprop = get_property(provider_node, "#address-cells"); > + if (cellprop) > + parent_cellsize += propval_cell(cellprop); > + > + cell += cellsize + 1 + parent_cellsize; > + } > +} > +WARNING(interrupt_map, check_interrupt_map, NULL, &phandle_references, &addr_size_cells, &interrupt_provider); > + > static void check_interrupts_property(struct check *c, > struct dt_info *dti, > struct node *node) > @@ -1887,6 +1968,7 @@ static struct check *check_table[] = { > &gpios_property, > &interrupts_property, > &interrupt_provider, > + &interrupt_map, > > &alias_paths, > > diff --git a/tests/bad-interrupt-map-mask.dts b/tests/bad-interrupt-map-mask.dts > new file mode 100644 > index 000000000000..10eaffd62310 > --- /dev/null > +++ b/tests/bad-interrupt-map-mask.dts > @@ -0,0 +1,20 @@ > +/dts-v1/; > + > +/ { > + interrupt-parent = <&intc>; > + intc: interrupt-controller { > + #interrupt-cells = <3>; > + interrupt-controller; > + }; > + > + node { > + #address-cells = <0>; > + #interrupt-cells = <1>; > + interrupt-map = <1 &intc 1 2 3>; > + interrupt-map-mask = <0 0>; > + > + child { > + interrupts = <1>; > + }; > + }; > +}; > diff --git a/tests/bad-interrupt-map-parent.dts b/tests/bad-interrupt-map-parent.dts > new file mode 100644 > index 000000000000..fe88ce2fe76f > --- /dev/null > +++ b/tests/bad-interrupt-map-parent.dts > @@ -0,0 +1,17 @@ > +/dts-v1/; > + > +/ { > + interrupt-parent = <&intc>; > + intc: interrupt-controller { > + }; > + > + node { > + #address-cells = <0>; > + #interrupt-cells = <1>; > + interrupt-map = <1 &intc 1 2 3>; > + > + child { > + interrupts = <1>; > + }; > + }; > +}; > diff --git a/tests/bad-interrupt-map.dts b/tests/bad-interrupt-map.dts > new file mode 100644 > index 000000000000..6df8f93ae00c > --- /dev/null > +++ b/tests/bad-interrupt-map.dts > @@ -0,0 +1,19 @@ > +/dts-v1/; > + > +/ { > + interrupt-parent = <&intc>; > + intc: interrupt-controller { > + #interrupt-cells = <3>; > + interrupt-controller; > + }; > + > + node { > + /* Missing #address-cells = <0>; */ > + #interrupt-cells = <1>; > + interrupt-map = <1 &intc 1 2 3>; > + > + child { > + interrupts = <1>; > + }; > + }; > +}; > diff --git a/tests/run_tests.sh b/tests/run_tests.sh > index 0e270feb3e47..d100d5aaa21f 100755 > --- a/tests/run_tests.sh > +++ b/tests/run_tests.sh > @@ -717,6 +717,9 @@ dtc_tests () { > run_sh_test "$SRCDIR/dtc-checkfails.sh" -n deprecated_gpio_property -- -Wdeprecated_gpio_property -I dts -O dtb "$SRCDIR/good-gpio.dts" > check_tests "$SRCDIR/bad-interrupt-cells.dts" interrupts_property > check_tests "$SRCDIR/bad-interrupt-controller.dts" interrupt_provider > + check_tests "$SRCDIR/bad-interrupt-map.dts" interrupt_map > + check_tests "$SRCDIR/bad-interrupt-map-parent.dts" interrupt_map > + check_tests "$SRCDIR/bad-interrupt-map-mask.dts" interrupt_map > run_sh_test "$SRCDIR/dtc-checkfails.sh" node_name_chars -- -I dtb -O dtb bad_node_char.dtb > run_sh_test "$SRCDIR/dtc-checkfails.sh" node_name_format -- -I dtb -O dtb bad_node_format.dtb > run_sh_test "$SRCDIR/dtc-checkfails.sh" property_name_chars -- -I dtb -O dtb bad_prop_char.dtb -- 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 #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 3/5] checks: Drop interrupt provider '#address-cells' check [not found] ` <20211011191245.1009682-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2021-10-11 19:12 ` [PATCH v2 2/5] checks: Add an interrupt-map check Rob Herring @ 2021-10-11 19:12 ` Rob Herring [not found] ` <20211011191245.1009682-3-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2021-10-11 19:12 ` [PATCH v2 4/5] checks: Ensure '#interrupt-cells' only exists in interrupt providers Rob Herring ` (2 subsequent siblings) 4 siblings, 1 reply; 12+ messages in thread From: Rob Herring @ 2021-10-11 19:12 UTC (permalink / raw) To: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA, David Gibson; +Cc: Andre Przywara '#address-cells' is only needed when parsing 'interrupt-map' properties, so remove it from the common interrupt-provider test. Cc: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> Reviewed-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> --- checks.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/checks.c b/checks.c index a72ae4cc0be9..1a39bfd2cd94 100644 --- a/checks.c +++ b/checks.c @@ -1581,11 +1581,6 @@ static void check_interrupt_provider(struct check *c, if (!prop) FAIL(c, dti, node, "Missing #interrupt-cells in interrupt provider"); - - prop = get_property(node, "#address-cells"); - if (!prop) - FAIL(c, dti, node, - "Missing #address-cells in interrupt provider"); } WARNING(interrupt_provider, check_interrupt_provider, NULL, &interrupts_extended_is_cell); -- 2.30.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
[parent not found: <20211011191245.1009682-3-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH v2 3/5] checks: Drop interrupt provider '#address-cells' check [not found] ` <20211011191245.1009682-3-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> @ 2021-10-14 3:45 ` David Gibson 0 siblings, 0 replies; 12+ messages in thread From: David Gibson @ 2021-10-14 3:45 UTC (permalink / raw) To: Rob Herring; +Cc: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA, Andre Przywara [-- Attachment #1: Type: text/plain, Size: 1278 bytes --] On Mon, Oct 11, 2021 at 02:12:43PM -0500, Rob Herring wrote: > '#address-cells' is only needed when parsing 'interrupt-map' properties, so > remove it from the common interrupt-provider test. > > Cc: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> > Reviewed-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> > Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> Applied, thanks. > --- > checks.c | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/checks.c b/checks.c > index a72ae4cc0be9..1a39bfd2cd94 100644 > --- a/checks.c > +++ b/checks.c > @@ -1581,11 +1581,6 @@ static void check_interrupt_provider(struct check *c, > if (!prop) > FAIL(c, dti, node, > "Missing #interrupt-cells in interrupt provider"); > - > - prop = get_property(node, "#address-cells"); > - if (!prop) > - FAIL(c, dti, node, > - "Missing #address-cells in interrupt provider"); > } > WARNING(interrupt_provider, check_interrupt_provider, NULL, &interrupts_extended_is_cell); > -- 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 #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 4/5] checks: Ensure '#interrupt-cells' only exists in interrupt providers [not found] ` <20211011191245.1009682-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2021-10-11 19:12 ` [PATCH v2 2/5] checks: Add an interrupt-map check Rob Herring 2021-10-11 19:12 ` [PATCH v2 3/5] checks: Drop interrupt provider '#address-cells' check Rob Herring @ 2021-10-11 19:12 ` Rob Herring [not found] ` <20211011191245.1009682-4-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2021-10-11 19:12 ` [PATCH v2 5/5] checks: Add a sanity check for #.*-cells property value Rob Herring 2021-10-14 3:41 ` [PATCH v2 1/5] checks: Make interrupt_provider check dependent on interrupts_extended_is_cell David Gibson 4 siblings, 1 reply; 12+ messages in thread From: Rob Herring @ 2021-10-11 19:12 UTC (permalink / raw) To: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA, David Gibson; +Cc: Andre Przywara The interrupt provider check currently checks if an interrupt provider has #interrupt-cells, but not whether #interrupt-cells is present outside of interrupt-providers. Rework the check to cover the latter case. Cc: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> Reviewed-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> --- checks.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/checks.c b/checks.c index 1a39bfd2cd94..903083bfc423 100644 --- a/checks.c +++ b/checks.c @@ -1573,14 +1573,20 @@ static void check_interrupt_provider(struct check *c, struct node *node) { struct property *prop; + bool irq_provider = node_is_interrupt_provider(node); - if (!node_is_interrupt_provider(node)) + prop = get_property(node, "#interrupt-cells"); + if (irq_provider && !prop) { + FAIL(c, dti, node, + "Missing '#interrupt-cells' in interrupt provider"); return; + } - prop = get_property(node, "#interrupt-cells"); - if (!prop) + if (!irq_provider && prop) { FAIL(c, dti, node, - "Missing #interrupt-cells in interrupt provider"); + "'#interrupt-cells' found, but node is not an interrupt provider"); + return; + } } WARNING(interrupt_provider, check_interrupt_provider, NULL, &interrupts_extended_is_cell); -- 2.30.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
[parent not found: <20211011191245.1009682-4-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH v2 4/5] checks: Ensure '#interrupt-cells' only exists in interrupt providers [not found] ` <20211011191245.1009682-4-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> @ 2021-10-14 3:46 ` David Gibson 2021-10-15 18:43 ` Rob Herring 0 siblings, 1 reply; 12+ messages in thread From: David Gibson @ 2021-10-14 3:46 UTC (permalink / raw) To: Rob Herring; +Cc: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA, Andre Przywara [-- Attachment #1: Type: text/plain, Size: 1803 bytes --] On Mon, Oct 11, 2021 at 02:12:44PM -0500, Rob Herring wrote: > The interrupt provider check currently checks if an interrupt provider > has #interrupt-cells, but not whether #interrupt-cells is present > outside of interrupt-providers. Rework the check to cover the latter > case. > > Cc: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> > Reviewed-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> > Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> Applied, thanks. > --- > checks.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/checks.c b/checks.c > index 1a39bfd2cd94..903083bfc423 100644 > --- a/checks.c > +++ b/checks.c > @@ -1573,14 +1573,20 @@ static void check_interrupt_provider(struct check *c, > struct node *node) > { > struct property *prop; > + bool irq_provider = node_is_interrupt_provider(node); > > - if (!node_is_interrupt_provider(node)) > + prop = get_property(node, "#interrupt-cells"); > + if (irq_provider && !prop) { > + FAIL(c, dti, node, > + "Missing '#interrupt-cells' in interrupt provider"); > return; > + } > > - prop = get_property(node, "#interrupt-cells"); > - if (!prop) > + if (!irq_provider && prop) { > FAIL(c, dti, node, > - "Missing #interrupt-cells in interrupt provider"); > + "'#interrupt-cells' found, but node is not an interrupt provider"); > + return; > + } > } > WARNING(interrupt_provider, check_interrupt_provider, NULL, &interrupts_extended_is_cell); > -- 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 #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 4/5] checks: Ensure '#interrupt-cells' only exists in interrupt providers 2021-10-14 3:46 ` David Gibson @ 2021-10-15 18:43 ` Rob Herring [not found] ` <CAL_JsqLT3FKtaFxTmWU=Y5_D=EyP=L_Aouij2Qw7MKOJ+RY-HQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 12+ messages in thread From: Rob Herring @ 2021-10-15 18:43 UTC (permalink / raw) To: David Gibson; +Cc: Devicetree Compiler, Andre Przywara On Wed, Oct 13, 2021 at 10:51 PM David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote: > > On Mon, Oct 11, 2021 at 02:12:44PM -0500, Rob Herring wrote: > > The interrupt provider check currently checks if an interrupt provider > > has #interrupt-cells, but not whether #interrupt-cells is present > > outside of interrupt-providers. Rework the check to cover the latter > > case. > > > > Cc: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> > > Reviewed-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> > > Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> > > Applied, thanks. Thanks, but not seeing these commits... Rob ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <CAL_JsqLT3FKtaFxTmWU=Y5_D=EyP=L_Aouij2Qw7MKOJ+RY-HQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v2 4/5] checks: Ensure '#interrupt-cells' only exists in interrupt providers [not found] ` <CAL_JsqLT3FKtaFxTmWU=Y5_D=EyP=L_Aouij2Qw7MKOJ+RY-HQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2021-10-15 19:28 ` Rob Herring 0 siblings, 0 replies; 12+ messages in thread From: Rob Herring @ 2021-10-15 19:28 UTC (permalink / raw) To: David Gibson; +Cc: Devicetree Compiler, Andre Przywara On Fri, Oct 15, 2021 at 1:43 PM Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote: > > On Wed, Oct 13, 2021 at 10:51 PM David Gibson > <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote: > > > > On Mon, Oct 11, 2021 at 02:12:44PM -0500, Rob Herring wrote: > > > The interrupt provider check currently checks if an interrupt provider > > > has #interrupt-cells, but not whether #interrupt-cells is present > > > outside of interrupt-providers. Rework the check to cover the latter > > > case. > > > > > > Cc: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> > > > Reviewed-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> > > > Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> > > > > Applied, thanks. > > Thanks, but not seeing these commits... NM, failure on my end... Rob ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 5/5] checks: Add a sanity check for #.*-cells property value [not found] ` <20211011191245.1009682-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> ` (2 preceding siblings ...) 2021-10-11 19:12 ` [PATCH v2 4/5] checks: Ensure '#interrupt-cells' only exists in interrupt providers Rob Herring @ 2021-10-11 19:12 ` Rob Herring [not found] ` <20211011191245.1009682-5-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2021-10-14 3:41 ` [PATCH v2 1/5] checks: Make interrupt_provider check dependent on interrupts_extended_is_cell David Gibson 4 siblings, 1 reply; 12+ messages in thread From: Rob Herring @ 2021-10-11 19:12 UTC (permalink / raw) To: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA, David Gibson; +Cc: Andre Przywara While in theory any value of number of cells is allowed for a #.*-cells property, for all practical purposes the number of cells is never more than a few cells. Add a check that the value is less than 255. This will catch cases like this which will currently pass: #foo-cells = "bar"; Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> --- checks.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/checks.c b/checks.c index 903083bfc423..467442250eda 100644 --- a/checks.c +++ b/checks.c @@ -260,8 +260,14 @@ static void check_is_cell(struct check *c, struct dt_info *dti, if (!prop) return; /* Not present, assumed ok */ - if (prop->val.len != sizeof(cell_t)) + if (prop->val.len != sizeof(cell_t)) { FAIL_PROP(c, dti, node, prop, "property is not a single cell"); + return; + } + + /* Sanity test for reasonable number of cells */ + if (propval_cell(prop) > 255) + FAIL_PROP(c, dti, node, prop, "cell size out of range (>255)"); } #define WARNING_IF_NOT_CELL(nm, propname) \ WARNING(nm, check_is_cell, (propname)) -- 2.30.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
[parent not found: <20211011191245.1009682-5-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH v2 5/5] checks: Add a sanity check for #.*-cells property value [not found] ` <20211011191245.1009682-5-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> @ 2021-10-14 3:50 ` David Gibson 0 siblings, 0 replies; 12+ messages in thread From: David Gibson @ 2021-10-14 3:50 UTC (permalink / raw) To: Rob Herring; +Cc: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA, Andre Przywara [-- Attachment #1: Type: text/plain, Size: 1757 bytes --] On Mon, Oct 11, 2021 at 02:12:45PM -0500, Rob Herring wrote: > While in theory any value of number of cells is allowed for a #.*-cells > property, for all practical purposes the number of cells is never more than > a few cells. Add a check that the value is less than 255. This will catch > cases like this which will currently pass: > > #foo-cells = "bar"; > > Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> Hmm. So the original idea was that this helper could be used for any 1-cell property, not just for #foo-cells. In practice we're only using it for #foo-cells so far. The change would be ok if we updated the names to make it clearer that this is specifically for #foo-cells properties, not anything else. > --- > checks.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/checks.c b/checks.c > index 903083bfc423..467442250eda 100644 > --- a/checks.c > +++ b/checks.c > @@ -260,8 +260,14 @@ static void check_is_cell(struct check *c, struct dt_info *dti, > if (!prop) > return; /* Not present, assumed ok */ > > - if (prop->val.len != sizeof(cell_t)) > + if (prop->val.len != sizeof(cell_t)) { > FAIL_PROP(c, dti, node, prop, "property is not a single cell"); > + return; > + } > + > + /* Sanity test for reasonable number of cells */ > + if (propval_cell(prop) > 255) > + FAIL_PROP(c, dti, node, prop, "cell size out of range (>255)"); > } > #define WARNING_IF_NOT_CELL(nm, propname) \ > WARNING(nm, check_is_cell, (propname)) -- 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 #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/5] checks: Make interrupt_provider check dependent on interrupts_extended_is_cell [not found] ` <20211011191245.1009682-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> ` (3 preceding siblings ...) 2021-10-11 19:12 ` [PATCH v2 5/5] checks: Add a sanity check for #.*-cells property value Rob Herring @ 2021-10-14 3:41 ` David Gibson 4 siblings, 0 replies; 12+ messages in thread From: David Gibson @ 2021-10-14 3:41 UTC (permalink / raw) To: Rob Herring; +Cc: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA, Andre Przywara [-- Attachment #1: Type: text/plain, Size: 1146 bytes --] On Mon, Oct 11, 2021 at 02:12:41PM -0500, Rob Herring wrote: > If '#interrupt-cells' doesn't pass checks, no reason to run interrupt > provider check. > > Cc: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> > Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> Applied, thanks. > --- > checks.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/checks.c b/checks.c > index fb3fc9cda4b1..8153793a3e7d 100644 > --- a/checks.c > +++ b/checks.c > @@ -1587,7 +1587,7 @@ static void check_interrupt_provider(struct check *c, > FAIL(c, dti, node, > "Missing #address-cells in interrupt provider"); > } > -WARNING(interrupt_provider, check_interrupt_provider, NULL); > +WARNING(interrupt_provider, check_interrupt_provider, NULL, &interrupts_extended_is_cell); > > static void check_interrupts_property(struct check *c, > struct dt_info *dti, -- 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 #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-10-15 19:28 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-11 19:12 [PATCH v2 1/5] checks: Make interrupt_provider check dependent on interrupts_extended_is_cell Rob Herring [not found] ` <20211011191245.1009682-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2021-10-11 19:12 ` [PATCH v2 2/5] checks: Add an interrupt-map check Rob Herring [not found] ` <20211011191245.1009682-2-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2021-10-14 3:44 ` David Gibson 2021-10-11 19:12 ` [PATCH v2 3/5] checks: Drop interrupt provider '#address-cells' check Rob Herring [not found] ` <20211011191245.1009682-3-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2021-10-14 3:45 ` David Gibson 2021-10-11 19:12 ` [PATCH v2 4/5] checks: Ensure '#interrupt-cells' only exists in interrupt providers Rob Herring [not found] ` <20211011191245.1009682-4-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2021-10-14 3:46 ` David Gibson 2021-10-15 18:43 ` Rob Herring [not found] ` <CAL_JsqLT3FKtaFxTmWU=Y5_D=EyP=L_Aouij2Qw7MKOJ+RY-HQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2021-10-15 19:28 ` Rob Herring 2021-10-11 19:12 ` [PATCH v2 5/5] checks: Add a sanity check for #.*-cells property value Rob Herring [not found] ` <20211011191245.1009682-5-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2021-10-14 3:50 ` David Gibson 2021-10-14 3:41 ` [PATCH v2 1/5] checks: Make interrupt_provider check dependent on interrupts_extended_is_cell David Gibson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.