[PATCH v2 04/35] irq: Add a method to convert an interrupt to ACPI

[Simon Glass]

Hi Wolfgang,

On Wed, 13 May 2020 at 07:01, Wolfgang Wallner
<wolfgang.wallner at br-automation.com> wrote:
>
> Hi Simon,
>
> -----"Simon Glass" <sjg at chromium.org> schrieb: -----
> >Betreff: [PATCH v2 04/35] 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
> >Changes in v1:
> >- Fix 'the an' typo
> >- Move header definitions into this patch
> >
> > drivers/misc/irq-uclass.c  | 18 ++++++++++--
> > drivers/misc/irq_sandbox.c | 16 +++++++++++
> > include/acpi/acpi_device.h | 59
> >++++++++++++++++++++++++++++++++++++++
> > include/irq.h              | 43 +++++++++++++++++++++++++++
> > test/dm/irq.c              | 22 ++++++++++++++
> > 5 files changed, 156 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/misc/irq-uclass.c b/drivers/misc/irq-uclass.c
> >index 16dc0be75c..98bc79eaba 100644
> >--- a/drivers/misc/irq-uclass.c
> >+++ b/drivers/misc/irq-uclass.c
> >@@ -154,8 +154,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 are these lines dropped?

Because we are not allowed to pass a NULL irq to this function. That
check should not have been there.

>
> As far as I understand the code they can be dropped, I just fail
> to see how that is related to the ACPI changes.
>
> >       ops = irq_get_ops(dev);
> >
> >       irq->dev = dev;
> >@@ -177,6 +175,22 @@ int irq_first_device_type(enum irq_dev_t type,
> >struct udevice **devp)
> >       return 0;
> > }


> >
> >+#if CONFIG_IS_ENABLED(ACPIGEN)
> >+int irq_get_acpi(const struct irq *irq, struct acpi_irq *acpi_irq)
> >+{
> >+      struct irq_ops *ops;
> >+
> >+      if (!irq_is_valid(irq))
> >+              return -EINVAL;
> >+
> >+      ops = irq_get_ops(irq->dev);
> >+      if (!ops->get_acpi)
> >+              return -ENOSYS;
> >+
> >+      return ops->get_acpi(irq, acpi_irq);
> >+}
> >+#endif
> >+
> > UCLASS_DRIVER(irq) = {
> >       .id             = UCLASS_IRQ,
> >       .name           = "irq",
> >diff --git a/drivers/misc/irq_sandbox.c b/drivers/misc/irq_sandbox.c
> >index 54bc47c8d8..a2511b32fc 100644
> >--- a/drivers/misc/irq_sandbox.c
> >+++ b/drivers/misc/irq_sandbox.c
> >@@ -8,6 +8,7 @@
> > #include <common.h>
> > #include <dm.h>
> > #include <irq.h>
> >+#include <acpi/acpi_device.h>
> > #include <asm/test.h>
> >
> > /**
> >@@ -73,6 +74,18 @@ static int sandbox_irq_of_xlate(struct irq *irq,
> >       return 0;
> > }
> >
> >+static __maybe_unused int sandbox_get_acpi(const struct irq *irq,
> >+                                         struct acpi_irq *acpi_irq)
> >+{
> >+      acpi_irq->pin = irq->id;
> >+      acpi_irq->mode = ACPI_IRQ_LEVEL_TRIGGERED;
> >+      acpi_irq->polarity = ACPI_IRQ_ACTIVE_HIGH;
> >+      acpi_irq->shared = ACPI_IRQ_SHARED;
> >+      acpi_irq->wake = ACPI_IRQ_WAKE;
> >+
> >+      return 0;
> >+}
> >+
> > static const struct irq_ops sandbox_irq_ops = {
> >       .route_pmc_gpio_gpe     = sandbox_route_pmc_gpio_gpe,
> >       .set_polarity           = sandbox_set_polarity,
> >@@ -80,6 +93,9 @@ static const struct irq_ops sandbox_irq_ops = {
> >       .restore_polarities     = sandbox_restore_polarities,
> >       .read_and_clear         = sandbox_irq_read_and_clear,
> >       .of_xlate               = sandbox_irq_of_xlate,
> >+#if CONFIG_IS_ENABLED(ACPIGEN)
> >+      .get_acpi               = sandbox_get_acpi,
> >+#endif
> > };
> >
> > static const struct udevice_id sandbox_irq_ids[] = {
> >diff --git a/include/acpi/acpi_device.h b/include/acpi/acpi_device.h
> >index 09c227489a..24895de0da 100644
> >--- a/include/acpi/acpi_device.h
> >+++ b/include/acpi/acpi_device.h
> >@@ -13,6 +13,12 @@
> >
> > struct udevice;
> >
> >+/* ACPI descriptor values for common descriptors: SERIAL_BUS means
> >I2C */
>
> I don't understand the comment above. Why does SERIAL_BUS mean I2C?
> It could also mean SPI, or am I missing something?

Just that descriptor 14 is used for serial bus (UART, SPI, I2C) and in
this code it is used for I2C. I didn't want to call it I2C since that
would be inaccurate, hence the comment.

>
> >+#define ACPI_DESCRIPTOR_LARGE         BIT(7)
> >+#define ACPI_DESCRIPTOR_INTERRUPT     (ACPI_DESCRIPTOR_LARGE | 9)
> >+#define ACPI_DESCRIPTOR_GPIO          (ACPI_DESCRIPTOR_LARGE | 12)
> >+#define ACPI_DESCRIPTOR_SERIAL_BUS    (ACPI_DESCRIPTOR_LARGE | 14)
> >+

Regards,
Simon