[PATCH v8 22/37] drivers: misc: irq-uclass: Update irq_get_by_index

[Patrick Rudolph]

Hi Moritz,
On Tue, Oct 15, 2024 at 2:59 AM Moritz Fischer <moritzf at google.com> wrote:
>
> Hi Patrick
>
> On Mon, Oct 14, 2024 at 6:16 AM Patrick Rudolph
> <patrick.rudolph at 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.rudolph at 9elements.com>
> > Reviewed-by: Simon Glass <sjg at 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