Hi Simon, On Tue, Feb 4, 2020 at 8:19 AM Simon Glass <s...@chromium.org> wrote: > > Hi Bin, > > On Mon, 3 Feb 2020 at 10:05, Bin Meng <bmeng...@gmail.com> wrote: > > > > Hi Simon, > > > > On Sun, Dec 22, 2019 at 2:16 AM Simon Glass <s...@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 <s...@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 <s...@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..
If that's the way how other DM codes do, I am okay with that. > > [..] > > > > +static int sandbox_irq_of_xlate(struct irq *irq, > > > + struct ofnode_phandle_args *args) > > > +{ > > > + irq->id = args->args[0]; > > > + > > > + return 0; > > > +} > > > > This > > ...? I must have been working too long yesterday ... Regards, Bin