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
