[PATCH v2 08/13] dm: irq: Add support for requesting interrupts

[Simon Glass]

Hi Bin,

On Mon, 3 Feb 2020 at 10:05, Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Simon,
>
> On Sun, Dec 22, 2019 at 2:16 AM Simon Glass <sjg at chromium.org> wrote:
> >
> > At present driver model supports the IRQ uclass but there is no way to
> > request a particular interrupt for a driver.
> >
> > Add a mechanism, similar to clock and reset, to read the interrupts
> > required by a device from the device tree and to request those interrupts.
> >
> > U-Boot itself does not have interrupt-driven handlers, so just provide a
> > means to read and clear an interrupt. This can be useful to handle
> > peripherals which must use an interrupt to determine when data is
> > available, for example.
> >
> > Bring over the basic binding file as well, from Linux v5.4. Note that the
> > older binding is not supported in U-Boot; the newer 'special form' must be
> > used.
> >
> > Add a simple test of the new functionality.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> > Changes in v2: None
> >
> >  arch/sandbox/dts/test.dts                     |   5 +-
> >  .../interrupt-controller/interrupts.txt       | 131 ++++++++++++++++++
> >  drivers/misc/irq-uclass.c                     | 116 ++++++++++++++++
> >  drivers/misc/irq_sandbox.c                    |  41 ++++++
> >  include/irq.h                                 | 115 +++++++++++++++
> >  test/dm/irq.c                                 |  31 +++++
> >  6 files changed, 438 insertions(+), 1 deletion(-)
> >  create mode 100644 doc/device-tree-bindings/interrupt-controller/interrupts.txt
> >

[..]
> > diff --git a/drivers/misc/irq-uclass.c b/drivers/misc/irq-uclass.c
> > index 38498a66a4..7d65192858 100644
> > --- a/drivers/misc/irq-uclass.c
> > +++ b/drivers/misc/irq-uclass.c
> > @@ -4,8 +4,11 @@
> >   * Written by Simon Glass <sjg at chromium.org>
> >   */
> >
> > +#define LOG_CATEGORY UCLASS_IRQ
> > +
> >  #include <common.h>
> >  #include <dm.h>
> > +#include <dt-structs.h>
> >  #include <irq.h>
> >  #include <dm/device-internal.h>
> >
> > @@ -49,6 +52,119 @@ int irq_restore_polarities(struct udevice *dev)
> >         return ops->restore_polarities(dev);
> >  }
> >
> > +int irq_read_and_clear(struct irq *irq)
> > +{
> > +       const struct irq_ops *ops = irq_get_ops(irq->dev);
> > +
> > +       if (!ops->read_and_clear)
> > +               return -ENOSYS;
> > +
> > +       return ops->read_and_clear(irq);
> > +}
> > +
> > +#if CONFIG_IS_ENABLED(OF_PLATDATA)
> > +int irq_get_by_index_platdata(struct udevice *dev, int index,
> > +                             struct phandle_1_arg *cells, struct irq *irq)
> > +{
> > +       int ret;
> > +
> > +       if (index != 0)
> > +               return -ENOSYS;
> > +       ret = uclass_get_device(UCLASS_IRQ, 0, &irq->dev);
> > +       if (ret)
> > +               return ret;
> > +       irq->id = cells[0].arg[0];
> > +
> > +       return 0;
> > +}
> > +#else
> > +static int irq_of_xlate_default(struct irq *irq,
> > +                               struct ofnode_phandle_args *args)
> > +{
> > +       log_debug("(irq=%p)\n", irq);
> > +
> > +       if (args->args_count > 1) {
> > +               log_debug("Invaild args_count: %d\n", args->args_count);
> > +               return -EINVAL;
> > +       }
> > +
> > +       if (args->args_count)
> > +               irq->id = args->args[0];
> > +       else
> > +               irq->id = 0;
> > +
> > +       return 0;
> > +}
> > +
> > +static int irq_get_by_index_tail(int ret, ofnode node,
>
> It's odd to pass the return value of some other functions to this
> function and check it here. Can we please remove ret, and check its
> value at where we get that?

This pattern is similar to how it is done in DM core. It routes all
return paths through a single function where you can do error
checking, etc. So I think this is better than the alternative. The
only strange thing here is that this function is only used one. I did
this since I suspected there would be others coming. But I could
inline if it you like..

[..]

> > +static int sandbox_irq_of_xlate(struct irq *irq,
> > +                               struct ofnode_phandle_args *args)
> > +{
> > +       irq->id = args->args[0];
> > +
> > +       return 0;
> > +}
>
> This

...?

Regards,
Simon