[PATCH v2 32/39] irq: Add a method to convert an interrupt to ACPI

[Simon Glass]

Hi Wolfgang,

On Wed, 18 Mar 2020 at 10:20, Wolfgang Wallner
<wolfgang.wallner at br-automation.com> wrote:
>
> Hi Simon,
>
> I'm resending this mail, as my email client has broken the formating
> in the first attempt, sorry.
>
>
> "Simon Glass" <sjg at chromium.org> schrieb am 09.03.2020 04:44:56:
>
> > Von: "Simon Glass" <sjg at chromium.org>
> > An: "U-Boot Mailing List" <u-boot at lists.denx.de>,
> > Kopie: "Bin Meng" <bmeng.cn at gmail.com>, "Wolfgang Wallner"
> > <wolfgang.wallner at br-automation.com>, "Andy Shevchenko"
> > <andriy.shevchenko at linux.intel.com>, "Simon Glass" <sjg at chromium.org>
> > Datum: 09.03.2020 04:46
> > Betreff: [PATCH v2 32/39] irq: Add a method to convert an interrupt to
> ACPI
> >
> > When generating ACPI tables we need to convert IRQs in U-Boot to the
> ACPI
> > structures required by ACPI. This is a SoC-specific conversion and
> cannot
> > be handled by generic code, so add a new IRQ method to do the
> conversion.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> > Changes in v2: None
> >
> >  drivers/misc/irq-uclass.c | 18 +++++++-
> >  include/acpi_device.h     | 27 +++++++++++
> >  include/irq.h             | 41 +++++++++++++++++
> >  lib/acpi/acpi_device.c    | 94 +++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 178 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/misc/irq-uclass.c b/drivers/misc/irq-uclass.c
> > index 61aa10e465..b4a8b7b429 100644
> > --- a/drivers/misc/irq-uclass.c
> > +++ b/drivers/misc/irq-uclass.c
> > @@ -153,8 +153,6 @@ int irq_request(struct udevice *dev, struct irq
> *irq)
> >     const struct irq_ops *ops;
> >
> >     log_debug("(dev=%p, irq=%p)\n", dev, irq);
> > -   if (!irq)
> > -      return 0;
>
> Why is this code dropped?

We don't support passing NULL to this function. It would be invalid
and there is no point in adding code to check for it.

[..]

> > +/**
> > + * acpi_device_write_interrupt_or_gpio() - Write interrupt or GPIO to
> ACPI
> > + *
> > + * This reads the an interrupt from the device tree, if available. If
> not it
>
> typo: "the an"
>
> The description of what this function should do is rather vague.
> At least I'm not sure how it is meant to work.

OK I will beef it up.

[..]

> > +int acpi_device_write_interrupt_or_gpio(struct acpi_ctx *ctx,
> > +               struct udevice *dev, const char *prop)
> > +{
> > +   struct irq req_irq;
> > +   int ret;
> > +
> > +   ret = irq_get_by_index(dev, 0, &req_irq);
> > +   if (!ret) {
> > +      ret = acpi_device_write_interrupt_irq(ctx, &req_irq);
> > +      if (ret)
> > +         return log_msg_ret("irq", ret);
> > +   } else {
> > +      struct gpio_desc req_gpio;
> > +
> > +      ret = gpio_request_by_name(dev, prop, 0, &req_gpio,
> > +                  GPIOD_IS_IN);
> > +      if (ret)
> > +         return log_msg_ret("no gpio", ret);
> > +      ret = acpi_device_write_gpio_desc(ctx, &req_gpio);
> > +      if (ret)
> > +         return log_msg_ret("gpio", ret);
> > +   }
>
> Both code paths set the index value hardcoded to 0.
> Why is that? Would other indices not make sense?

We only support a single interrupt / GPIO at present. We could expand
this but there is no use case for that yet.

Regards,
Simon