Hi Wolfgang,
On Wed, 18 Mar 2020 at 10:20, Wolfgang Wallner
<wolfgang.wall...@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" <s...@chromium.org> schrieb am 09.03.2020 04:44:56:
>
> > Von: "Simon Glass" <s...@chromium.org>
> > An: "U-Boot Mailing List" <u-boot@lists.denx.de>,
> > Kopie: "Bin Meng" <bmeng...@gmail.com>, "Wolfgang Wallner"
> > <wolfgang.wall...@br-automation.com>, "Andy Shevchenko"
> > <andriy.shevche...@linux.intel.com>, "Simon Glass" <s...@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 <s...@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
