Hi Moritz, On Tue, Oct 15, 2024 at 2:59 AM Moritz Fischer <mori...@google.com> wrote: > > Hi Patrick > > On Mon, Oct 14, 2024 at 6:16 AM Patrick Rudolph > <patrick.rudo...@9elements.com> wrote: > > > > Support reading the "interrupts" property from the devicetree in case > > the "interrupts-extended" property isn't found. As the "interrupts" > > property is commonly used, this allows to parse all existing FDT and > > makes irq_get_by_index() more useful. > > > > The "interrupts" property doesn't contain a phandle as "interrupts-extended" > > does, so implement a new method to locate the interrupt-parent called > > irq_get_interrupt_parent(). > > > > TEST: Read the interrupts from the GIC node for ACPI MADT generation. > > > > Signed-off-by: Patrick Rudolph <patrick.rudo...@9elements.com> > > Reviewed-by: Simon Glass <s...@chromium.org> > > --- > > arch/sandbox/dts/test.dts | 3 ++ > > drivers/misc/irq-uclass.c | 66 ++++++++++++++++++++++++++++++++++++++- > > include/irq.h | 14 +++++++++ > > test/dm/irq.c | 15 +++++++++ > > 4 files changed, 97 insertions(+), 1 deletion(-) > > > > diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts > > index 8412506c17..9599a8db9c 100644 > > --- a/arch/sandbox/dts/test.dts > > +++ b/arch/sandbox/dts/test.dts > > @@ -522,6 +522,9 @@ > > }; > > > > f-test { > > + #interrupt-cells = <2>; > > + interrupt-parent = <&irq>; > > + interrupts = <4 0>; > > compatible = "denx,u-boot-fdt-test"; > > }; > > > > diff --git a/drivers/misc/irq-uclass.c b/drivers/misc/irq-uclass.c > > index 79eb7c200d..068f67fe02 100644 > > --- a/drivers/misc/irq-uclass.c > > +++ b/drivers/misc/irq-uclass.c > > @@ -62,6 +62,40 @@ int irq_read_and_clear(struct irq *irq) > > return ops->read_and_clear(irq); > > } > > > > +int irq_get_interrupt_parent(const struct udevice *dev, > > + struct udevice **interrupt_parent) > > +{ > > + struct ofnode_phandle_args phandle_args; > > + struct udevice *irq = NULL; > > + ofnode node; > > + int ret; > > + > > + if (!dev || !interrupt_parent) > > + return -EINVAL; > > + > > + *interrupt_parent = NULL; > > + > > + node = dev_ofnode(dev); > > + if (!ofnode_valid(node)) > > + return -EINVAL; > > + > > + while (ofnode_valid(node)) { > > + ret = ofnode_parse_phandle_with_args(node, > > "interrupt-parent", > > + NULL, 0, 0, > > &phandle_args); > > + if (!ret && !device_get_global_by_ofnode(phandle_args.node, > > &irq)) > > + break; > > + node = ofnode_get_parent(node); > > + } > > + > > + if (!irq) { > > + log_err("Cannot find an interrupt parent for device %s\n", > > dev->name); > > + return -ENODEV; > > + } > > + *interrupt_parent = irq; > > + > > + return 0; > > +} > > + > > #if CONFIG_IS_ENABLED(OF_PLATDATA) > > int irq_get_by_phandle(struct udevice *dev, const struct phandle_2_arg > > *cells, > > struct irq *irq) > > @@ -142,10 +176,40 @@ err: > > int irq_get_by_index(struct udevice *dev, int index, struct irq *irq) > > { > > struct ofnode_phandle_args args; > > - int ret; > > + struct udevice *interrupt_parent; > > + int ret, size, i; > > + const __be32 *list; > > + u32 count; > > > > ret = dev_read_phandle_with_args(dev, "interrupts-extended", > > "#interrupt-cells", 0, index, > > &args); > > + if (ret) { > > + list = dev_read_prop(dev, "interrupts", &size); > > + if (!list) > > + return -ENOENT; > > + > > + ret = irq_get_interrupt_parent(dev, &interrupt_parent); > > + if (ret) > > + return -ENODEV; > > + args.node = dev_ofnode(interrupt_parent); > > + > > + if (dev_read_u32(dev, "#interrupt-cells", &count)) { > > + log_err("%s: could not get #interrupt-cells for > > %s\n", > > + __func__, dev->name); > > + return -ENOENT; > > + } > > Don't you want to read this out of the interrupt-parent? No. It should be read from the node/dev containing the "interrupts" property. It encodes how many fields the interrupts property has and how many fields to read.
> > > + > > + if (index * count >= size / sizeof(*list)) > > + return -ENOENT; > > + if (count > OF_MAX_PHANDLE_ARGS) > > + count = OF_MAX_PHANDLE_ARGS; > > + args.args_count = count; > > + for (i = 0; i < count; i++) > > + args.args[i] = be32_to_cpup(&list[index * count + > > i]); > > + > > + return irq_get_by_index_tail(ret, dev_ofnode(dev), &args, > > + "interrupts", index, irq); > > + } > > > > return irq_get_by_index_tail(ret, dev_ofnode(dev), &args, > > "interrupts-extended", index > 0, irq); > > diff --git a/include/irq.h b/include/irq.h > > index 5638c10128..0fbc1a5f48 100644 > > --- a/include/irq.h > > +++ b/include/irq.h > > @@ -200,6 +200,20 @@ int irq_restore_polarities(struct udevice *dev); > > */ > > int irq_read_and_clear(struct irq *irq); > > > > +/** > > + * irq_get_interrupt_parent() - returns the interrupt parent > > + * > > + * Walks the devicetree and returns the interrupt parent's ofnode > > + * for the specified device. > > + * > > + * @dev: device > > + * @interrupt_parent: The interrupt parent's ofnode' > > + * Return: 0 success, or error value > > + * > > + */ > > +int irq_get_interrupt_parent(const struct udevice *dev, > > + struct udevice **interrupt_parent); > > + > > struct phandle_2_arg; > > /** > > * irq_get_by_phandle() - Get an irq by its phandle information > > (of-platadata) > > diff --git a/test/dm/irq.c b/test/dm/irq.c > > index 836f2d82e7..ca3e188065 100644 > > --- a/test/dm/irq.c > > +++ b/test/dm/irq.c > > @@ -76,6 +76,21 @@ static int dm_test_request(struct unit_test_state *uts) > > } > > DM_TEST(dm_test_request, UTF_SCAN_PDATA | UTF_SCAN_FDT); > > > > +/* Test of irq_get_by_index() */ > > +static int dm_test_irq_get_by_index(struct unit_test_state *uts) > > +{ > > + struct udevice *dev; > > + struct irq irq; > > + > > + ut_assertok(uclass_get_device_by_name(UCLASS_TEST_FDT, "f-test", > > + &dev)); > > + ut_assertok(irq_get_by_index(dev, 0, &irq)); > > + ut_asserteq(4, irq.id); > > + > > + return 0; > > +} > > +DM_TEST(dm_test_irq_get_by_index, UTF_SCAN_PDATA | UTF_SCAN_FDT); > > + > > /* Test of irq_get_acpi() */ > > static int dm_test_irq_get_acpi(struct unit_test_state *uts) > > { > > -- > > 2.46.2 > > > > Cheers, > Moritz