diff mbox series

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

Message ID 20200308214442.v2.32.I29adaf758a9ea81f51ffbecf89b53983968a0d2a@changeid
State New
Headers show
Series dm: Add programmatic generation of ACPI tables (part A) | expand

Commit Message

Simon Glass March 9, 2020, 3:44 a.m. UTC
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(-)

Comments

Wolfgang Wallner March 18, 2020, 4:17 p.m. UTC | #1
Hi Simon,

"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?

>     ops = irq_get_ops(dev);
> 
>     irq->dev = dev;
> @@ -176,6 +174,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/include/acpi_device.h b/include/acpi_device.h
> index acd26c0f54..50ba9b66aa 100644
> --- a/include/acpi_device.h
> +++ b/include/acpi_device.h
> @@ -545,6 +545,33 @@ int acpi_dp_write(struct acpi_ctx *ctx, struct 
> acpi_dp *table);
>  int acpi_device_write_gpio_desc(struct acpi_ctx *ctx,
>              const struct gpio_desc *desc);
> 
> +/**
> + * acpi_device_write_interrupt_irq() - Write an interrupt to ACPI
> + *
> + * This creates an interrupt descriptor for an interrupt, including
> information
> + * ACPI needs to use it.
> + *
> + * @req_irq: Interrupt to write
> + * @return 0 if OK, -ve on error
> + */
> +int acpi_device_write_interrupt_irq(struct acpi_ctx *ctx,
> +                const struct irq *req_irq);
> +
> +/**
> + * 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.

> + * reads the first GPIO with the name @prop.
> + *
> + * If an interrupt is found, that is written to ACPI. If not, but an 
GPIO is
> + * found, that is written.
> + *
> + * @return 0 if OK, -ve if neither an interrupt nor a GPIO could 
befound, or
> + * some other error occurred
> + */
> +int acpi_device_write_interrupt_or_gpio(struct acpi_ctx *ctx,
> +               struct udevice *dev, const char *prop);
> +
>  /**
>   * acpi_device_write_i2c_dev() - Write an I2C device to ACPI, including
>   * information ACPI needs to use it.
> diff --git a/include/irq.h b/include/irq.h
> index d4948e6dc4..8527e4dd79 100644
> --- a/include/irq.h
> +++ b/include/irq.h
> @@ -8,6 +8,7 @@
>  #ifndef __irq_H
>  #define __irq_H
> 
> +struct acpi_irq;
>  struct ofnode_phandle_args;
> 
>  /*
> @@ -26,10 +27,12 @@ enum irq_dev_t {
>   *
>   * @dev: IRQ device that handles this irq
>   * @id: ID to identify this irq with the device
> + * @flags: Flags associated with this interrupt (IRQ_TYPE_...)
>   */
>  struct irq {
>     struct udevice *dev;
>     ulong id;
> +   ulong flags;
>  };
> 
>  /**
> @@ -121,10 +124,36 @@ struct irq_ops {
>      * @return 0 if OK, or a negative error code.
>      */
>     int (*free)(struct irq *irq);
> +
> +#if CONFIG_IS_ENABLED(ACPIGEN)
> +   /**
> +    * get_acpi() - Get the ACPI info for an irq
> +    *
> +    * This converts a irq to an ACPI structure for adding to the ACPI
> +    * tables.
> +    *
> +    * @irq:   irq to convert
> +    * @acpi_irq:   Output ACPI interrupt information
> +    * @return ACPI pin number or -ve on error
> +    */
> +   int (*get_acpi)(const struct irq *irq, struct acpi_irq *acpi_irq);
> +#endif
>  };
> 
>  #define irq_get_ops(dev)   ((struct irq_ops *)(dev)->driver->ops)
> 
> +/**
> + * irq_is_valid() - Check if an IRQ is valid
> + *
> + * @irq:   IRQ description containing device and ID, e.g. previously
> + *      returned by irq_get_by_index()
> + * @return true if valid, false if not
> + */
> +static inline bool irq_is_valid(const struct irq *irq)
> +{
> +   return irq->dev != NULL;
> +}
> +
>  /**
>   * irq_route_pmc_gpio_gpe() - Get the GPIO for an event
>   *
> @@ -225,4 +254,16 @@ int irq_free(struct irq *irq);
>   */
>  int irq_first_device_type(enum irq_dev_t type, struct udevice **devp);
> 
> +/**
> + * irq_get_acpi() - Get the ACPI info for an irq
> + *
> + * This converts a irq to an ACPI structure for adding to the ACPI
> + * tables.
> + *
> + * @irq:   irq to convert
> + * @acpi_irq:   Output ACPI interrupt information
> + * @return ACPI pin number or -ve on error
> + */
> +int irq_get_acpi(const struct irq *irq, struct acpi_irq *acpi_irq);
> +
>  #endif
> diff --git a/lib/acpi/acpi_device.c b/lib/acpi/acpi_device.c
> index adc32f1216..aa5edfe807 100644
> --- a/lib/acpi/acpi_device.c
> +++ b/lib/acpi/acpi_device.c
> @@ -154,6 +154,58 @@ int acpi_device_status(const struct udevice *dev)
>     return ACPI_STATUS_DEVICE_ALL_ON;
>  }
> 
> +/* ACPI 6.1 section 6.4.3.6: Extended Interrupt Descriptor */
> +static int acpi_device_write_interrupt(struct acpi_ctx *ctx,
> +                   const struct acpi_irq *irq)
> +{
> +   void *desc_length;
> +   u8 flags;
> +
> +   if (!irq || !irq->pin)
> +      return -ENOENT;
> +
> +   /* This is supported by GpioInt() but not Interrupt() */
> +   if (irq->polarity == ACPI_IRQ_ACTIVE_BOTH)
> +      return -EINVAL;
> +
> +   /* Byte 0: Descriptor Type */
> +   acpigen_emit_byte(ctx, ACPI_DESCRIPTOR_INTERRUPT);
> +
> +   /* Byte 1-2: Length (filled in later) */
> +   desc_length = acpi_device_write_zero_len(ctx);
> +
> +   /*
> +    * Byte 3: Flags
> +    *  [7:5]: Reserved
> +    *    [4]: Wake     (0=NO_WAKE   1=WAKE)
> +    *    [3]: Sharing  (0=EXCLUSIVE 1=SHARED)
> +    *    [2]: Polarity (0=HIGH      1=LOW)
> +    *    [1]: Mode     (0=LEVEL     1=EDGE)
> +    *    [0]: Resource (0=PRODUCER  1=CONSUMER)
> +    */
> +   flags = 1 << 0; /* ResourceConsumer */
> +   if (irq->mode == ACPI_IRQ_EDGE_TRIGGERED)
> +      flags |= 1 << 1;
> +   if (irq->polarity == ACPI_IRQ_ACTIVE_LOW)
> +      flags |= 1 << 2;
> +   if (irq->shared == ACPI_IRQ_SHARED)
> +      flags |= 1 << 3;
> +   if (irq->wake == ACPI_IRQ_WAKE)
> +      flags |= 1 << 4;
> +   acpigen_emit_byte(ctx, flags);
> +
> +   /* Byte 4: Interrupt Table Entry Count */
> +   acpigen_emit_byte(ctx, 1);
> +
> +   /* Byte 5-8: Interrupt Number */
> +   acpigen_emit_dword(ctx, irq->pin);
> +
> +   /* Fill in Descriptor Length (account for len word) */
> +   acpi_device_fill_len(ctx, desc_length);
> +
> +   return 0;
> +}
> +
>  /* ACPI 6.1 section 6.4.3.8.1 - GPIO Interrupt or I/O */
>  int acpi_device_write_gpio(struct acpi_ctx *ctx, const struct 
> acpi_gpio *gpio)
>  {
> @@ -304,6 +356,48 @@ int acpi_device_write_gpio_desc(struct acpi_ctx 
*ctx,
>     return 0;
>  }
> 
> +int acpi_device_write_interrupt_irq(struct acpi_ctx *ctx,
> +                const struct irq *req_irq)
> +{
> +   struct acpi_irq irq;
> +   int ret;
> +
> +   ret = irq_get_acpi(req_irq, &irq);
> +   if (ret)
> +      return log_msg_ret("get", ret);
> +   ret = acpi_device_write_interrupt(ctx, &irq);
> +   if (ret)
> +      return log_msg_ret("write", ret);
> +
> +   return 0;
> +}
> +
> +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?

> +
> +   return 0;
> +}
> +
>  /* ACPI 6.1 section 6.4.3.8.2.1 - I2cSerialBus() */
>  static void acpi_device_write_i2c(struct acpi_ctx *ctx,
>                const struct acpi_i2c *i2c)
> -- 
> 2.25.1.481.gfbce0eb801-goog
> 

regards, Wolfgang
Wolfgang Wallner March 18, 2020, 4:20 p.m. UTC | #2
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?

>     ops = irq_get_ops(dev);
> 
>     irq->dev = dev;
> @@ -176,6 +174,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/include/acpi_device.h b/include/acpi_device.h
> index acd26c0f54..50ba9b66aa 100644
> --- a/include/acpi_device.h
> +++ b/include/acpi_device.h
> @@ -545,6 +545,33 @@ int acpi_dp_write(struct acpi_ctx *ctx, struct 
> acpi_dp *table);
>  int acpi_device_write_gpio_desc(struct acpi_ctx *ctx,
>              const struct gpio_desc *desc);
> 
> +/**
> + * acpi_device_write_interrupt_irq() - Write an interrupt to ACPI
> + *
> + * This creates an interrupt descriptor for an interrupt, including
> information
> + * ACPI needs to use it.
> + *
> + * @req_irq: Interrupt to write
> + * @return 0 if OK, -ve on error
> + */
> +int acpi_device_write_interrupt_irq(struct acpi_ctx *ctx,
> +                const struct irq *req_irq);
> +
> +/**
> + * 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.

> + * reads the first GPIO with the name @prop.
> + *
> + * If an interrupt is found, that is written to ACPI. If not, but an 
GPIO is
> + * found, that is written.
> + *
> + * @return 0 if OK, -ve if neither an interrupt nor a GPIO could 
befound, or
> + * some other error occurred
> + */
> +int acpi_device_write_interrupt_or_gpio(struct acpi_ctx *ctx,
> +               struct udevice *dev, const char *prop);
> +
>  /**
>   * acpi_device_write_i2c_dev() - Write an I2C device to ACPI, including
>   * information ACPI needs to use it.
> diff --git a/include/irq.h b/include/irq.h
> index d4948e6dc4..8527e4dd79 100644
> --- a/include/irq.h
> +++ b/include/irq.h
> @@ -8,6 +8,7 @@
>  #ifndef __irq_H
>  #define __irq_H
> 
> +struct acpi_irq;
>  struct ofnode_phandle_args;
> 
>  /*
> @@ -26,10 +27,12 @@ enum irq_dev_t {
>   *
>   * @dev: IRQ device that handles this irq
>   * @id: ID to identify this irq with the device
> + * @flags: Flags associated with this interrupt (IRQ_TYPE_...)
>   */
>  struct irq {
>     struct udevice *dev;
>     ulong id;
> +   ulong flags;
>  };
> 
>  /**
> @@ -121,10 +124,36 @@ struct irq_ops {
>      * @return 0 if OK, or a negative error code.
>      */
>     int (*free)(struct irq *irq);
> +
> +#if CONFIG_IS_ENABLED(ACPIGEN)
> +   /**
> +    * get_acpi() - Get the ACPI info for an irq
> +    *
> +    * This converts a irq to an ACPI structure for adding to the ACPI
> +    * tables.
> +    *
> +    * @irq:   irq to convert
> +    * @acpi_irq:   Output ACPI interrupt information
> +    * @return ACPI pin number or -ve on error
> +    */
> +   int (*get_acpi)(const struct irq *irq, struct acpi_irq *acpi_irq);
> +#endif
>  };
> 
>  #define irq_get_ops(dev)   ((struct irq_ops *)(dev)->driver->ops)
> 
> +/**
> + * irq_is_valid() - Check if an IRQ is valid
> + *
> + * @irq:   IRQ description containing device and ID, e.g. previously
> + *      returned by irq_get_by_index()
> + * @return true if valid, false if not
> + */
> +static inline bool irq_is_valid(const struct irq *irq)
> +{
> +   return irq->dev != NULL;
> +}
> +
>  /**
>   * irq_route_pmc_gpio_gpe() - Get the GPIO for an event
>   *
> @@ -225,4 +254,16 @@ int irq_free(struct irq *irq);
>   */
>  int irq_first_device_type(enum irq_dev_t type, struct udevice **devp);
> 
> +/**
> + * irq_get_acpi() - Get the ACPI info for an irq
> + *
> + * This converts a irq to an ACPI structure for adding to the ACPI
> + * tables.
> + *
> + * @irq:   irq to convert
> + * @acpi_irq:   Output ACPI interrupt information
> + * @return ACPI pin number or -ve on error
> + */
> +int irq_get_acpi(const struct irq *irq, struct acpi_irq *acpi_irq);
> +
>  #endif
> diff --git a/lib/acpi/acpi_device.c b/lib/acpi/acpi_device.c
> index adc32f1216..aa5edfe807 100644
> --- a/lib/acpi/acpi_device.c
> +++ b/lib/acpi/acpi_device.c
> @@ -154,6 +154,58 @@ int acpi_device_status(const struct udevice *dev)
>     return ACPI_STATUS_DEVICE_ALL_ON;
>  }
> 
> +/* ACPI 6.1 section 6.4.3.6: Extended Interrupt Descriptor */
> +static int acpi_device_write_interrupt(struct acpi_ctx *ctx,
> +                   const struct acpi_irq *irq)
> +{
> +   void *desc_length;
> +   u8 flags;
> +
> +   if (!irq || !irq->pin)
> +      return -ENOENT;
> +
> +   /* This is supported by GpioInt() but not Interrupt() */
> +   if (irq->polarity == ACPI_IRQ_ACTIVE_BOTH)
> +      return -EINVAL;
> +
> +   /* Byte 0: Descriptor Type */
> +   acpigen_emit_byte(ctx, ACPI_DESCRIPTOR_INTERRUPT);
> +
> +   /* Byte 1-2: Length (filled in later) */
> +   desc_length = acpi_device_write_zero_len(ctx);
> +
> +   /*
> +    * Byte 3: Flags
> +    *  [7:5]: Reserved
> +    *    [4]: Wake     (0=NO_WAKE   1=WAKE)
> +    *    [3]: Sharing  (0=EXCLUSIVE 1=SHARED)
> +    *    [2]: Polarity (0=HIGH      1=LOW)
> +    *    [1]: Mode     (0=LEVEL     1=EDGE)
> +    *    [0]: Resource (0=PRODUCER  1=CONSUMER)
> +    */
> +   flags = 1 << 0; /* ResourceConsumer */
> +   if (irq->mode == ACPI_IRQ_EDGE_TRIGGERED)
> +      flags |= 1 << 1;
> +   if (irq->polarity == ACPI_IRQ_ACTIVE_LOW)
> +      flags |= 1 << 2;
> +   if (irq->shared == ACPI_IRQ_SHARED)
> +      flags |= 1 << 3;
> +   if (irq->wake == ACPI_IRQ_WAKE)
> +      flags |= 1 << 4;
> +   acpigen_emit_byte(ctx, flags);
> +
> +   /* Byte 4: Interrupt Table Entry Count */
> +   acpigen_emit_byte(ctx, 1);
> +
> +   /* Byte 5-8: Interrupt Number */
> +   acpigen_emit_dword(ctx, irq->pin);
> +
> +   /* Fill in Descriptor Length (account for len word) */
> +   acpi_device_fill_len(ctx, desc_length);
> +
> +   return 0;
> +}
> +
>  /* ACPI 6.1 section 6.4.3.8.1 - GPIO Interrupt or I/O */
>  int acpi_device_write_gpio(struct acpi_ctx *ctx, const struct 
> acpi_gpio *gpio)
>  {
> @@ -304,6 +356,48 @@ int acpi_device_write_gpio_desc(struct acpi_ctx 
*ctx,
>     return 0;
>  }
> 
> +int acpi_device_write_interrupt_irq(struct acpi_ctx *ctx,
> +                const struct irq *req_irq)
> +{
> +   struct acpi_irq irq;
> +   int ret;
> +
> +   ret = irq_get_acpi(req_irq, &irq);
> +   if (ret)
> +      return log_msg_ret("get", ret);
> +   ret = acpi_device_write_interrupt(ctx, &irq);
> +   if (ret)
> +      return log_msg_ret("write", ret);
> +
> +   return 0;
> +}
> +
> +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?

> +
> +   return 0;
> +}
> +
>  /* ACPI 6.1 section 6.4.3.8.2.1 - I2cSerialBus() */
>  static void acpi_device_write_i2c(struct acpi_ctx *ctx,
>                const struct acpi_i2c *i2c)
> -- 
> 2.25.1.481.gfbce0eb801-goog
>
Simon Glass March 19, 2020, 4:18 p.m. UTC | #3
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
diff mbox series

Patch

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;
 	ops = irq_get_ops(dev);
 
 	irq->dev = dev;
@@ -176,6 +174,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/include/acpi_device.h b/include/acpi_device.h
index acd26c0f54..50ba9b66aa 100644
--- a/include/acpi_device.h
+++ b/include/acpi_device.h
@@ -545,6 +545,33 @@  int acpi_dp_write(struct acpi_ctx *ctx, struct acpi_dp *table);
 int acpi_device_write_gpio_desc(struct acpi_ctx *ctx,
 				const struct gpio_desc *desc);
 
+/**
+ * acpi_device_write_interrupt_irq() - Write an interrupt to ACPI
+ *
+ * This creates an interrupt descriptor for an interrupt, including information
+ * ACPI needs to use it.
+ *
+ * @req_irq: Interrupt to write
+ * @return 0 if OK, -ve on error
+ */
+int acpi_device_write_interrupt_irq(struct acpi_ctx *ctx,
+				    const struct irq *req_irq);
+
+/**
+ * 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
+ * reads the first GPIO with the name @prop.
+ *
+ * If an interrupt is found, that is written to ACPI. If not, but an GPIO is
+ * found, that is written.
+ *
+ * @return 0 if OK, -ve if neither an interrupt nor a GPIO could be found, or
+ * some other error occurred
+ */
+int acpi_device_write_interrupt_or_gpio(struct acpi_ctx *ctx,
+					struct udevice *dev, const char *prop);
+
 /**
  * acpi_device_write_i2c_dev() - Write an I2C device to ACPI, including
  * information ACPI needs to use it.
diff --git a/include/irq.h b/include/irq.h
index d4948e6dc4..8527e4dd79 100644
--- a/include/irq.h
+++ b/include/irq.h
@@ -8,6 +8,7 @@ 
 #ifndef __irq_H
 #define __irq_H
 
+struct acpi_irq;
 struct ofnode_phandle_args;
 
 /*
@@ -26,10 +27,12 @@  enum irq_dev_t {
  *
  * @dev: IRQ device that handles this irq
  * @id: ID to identify this irq with the device
+ * @flags: Flags associated with this interrupt (IRQ_TYPE_...)
  */
 struct irq {
 	struct udevice *dev;
 	ulong id;
+	ulong flags;
 };
 
 /**
@@ -121,10 +124,36 @@  struct irq_ops {
 	 * @return 0 if OK, or a negative error code.
 	 */
 	int (*free)(struct irq *irq);
+
+#if CONFIG_IS_ENABLED(ACPIGEN)
+	/**
+	 * get_acpi() - Get the ACPI info for an irq
+	 *
+	 * This converts a irq to an ACPI structure for adding to the ACPI
+	 * tables.
+	 *
+	 * @irq:	irq to convert
+	 * @acpi_irq:	Output ACPI interrupt information
+	 * @return ACPI pin number or -ve on error
+	 */
+	int (*get_acpi)(const struct irq *irq, struct acpi_irq *acpi_irq);
+#endif
 };
 
 #define irq_get_ops(dev)	((struct irq_ops *)(dev)->driver->ops)
 
+/**
+ * irq_is_valid() - Check if an IRQ is valid
+ *
+ * @irq:	IRQ description containing device and ID, e.g. previously
+ *		returned by irq_get_by_index()
+ * @return true if valid, false if not
+ */
+static inline bool irq_is_valid(const struct irq *irq)
+{
+	return irq->dev != NULL;
+}
+
 /**
  * irq_route_pmc_gpio_gpe() - Get the GPIO for an event
  *
@@ -225,4 +254,16 @@  int irq_free(struct irq *irq);
  */
 int irq_first_device_type(enum irq_dev_t type, struct udevice **devp);
 
+/**
+ * irq_get_acpi() - Get the ACPI info for an irq
+ *
+ * This converts a irq to an ACPI structure for adding to the ACPI
+ * tables.
+ *
+ * @irq:	irq to convert
+ * @acpi_irq:	Output ACPI interrupt information
+ * @return ACPI pin number or -ve on error
+ */
+int irq_get_acpi(const struct irq *irq, struct acpi_irq *acpi_irq);
+
 #endif
diff --git a/lib/acpi/acpi_device.c b/lib/acpi/acpi_device.c
index adc32f1216..aa5edfe807 100644
--- a/lib/acpi/acpi_device.c
+++ b/lib/acpi/acpi_device.c
@@ -154,6 +154,58 @@  int acpi_device_status(const struct udevice *dev)
 	return ACPI_STATUS_DEVICE_ALL_ON;
 }
 
+/* ACPI 6.1 section 6.4.3.6: Extended Interrupt Descriptor */
+static int acpi_device_write_interrupt(struct acpi_ctx *ctx,
+				       const struct acpi_irq *irq)
+{
+	void *desc_length;
+	u8 flags;
+
+	if (!irq || !irq->pin)
+		return -ENOENT;
+
+	/* This is supported by GpioInt() but not Interrupt() */
+	if (irq->polarity == ACPI_IRQ_ACTIVE_BOTH)
+		return -EINVAL;
+
+	/* Byte 0: Descriptor Type */
+	acpigen_emit_byte(ctx, ACPI_DESCRIPTOR_INTERRUPT);
+
+	/* Byte 1-2: Length (filled in later) */
+	desc_length = acpi_device_write_zero_len(ctx);
+
+	/*
+	 * Byte 3: Flags
+	 *  [7:5]: Reserved
+	 *    [4]: Wake     (0=NO_WAKE   1=WAKE)
+	 *    [3]: Sharing  (0=EXCLUSIVE 1=SHARED)
+	 *    [2]: Polarity (0=HIGH      1=LOW)
+	 *    [1]: Mode     (0=LEVEL     1=EDGE)
+	 *    [0]: Resource (0=PRODUCER  1=CONSUMER)
+	 */
+	flags = 1 << 0; /* ResourceConsumer */
+	if (irq->mode == ACPI_IRQ_EDGE_TRIGGERED)
+		flags |= 1 << 1;
+	if (irq->polarity == ACPI_IRQ_ACTIVE_LOW)
+		flags |= 1 << 2;
+	if (irq->shared == ACPI_IRQ_SHARED)
+		flags |= 1 << 3;
+	if (irq->wake == ACPI_IRQ_WAKE)
+		flags |= 1 << 4;
+	acpigen_emit_byte(ctx, flags);
+
+	/* Byte 4: Interrupt Table Entry Count */
+	acpigen_emit_byte(ctx, 1);
+
+	/* Byte 5-8: Interrupt Number */
+	acpigen_emit_dword(ctx, irq->pin);
+
+	/* Fill in Descriptor Length (account for len word) */
+	acpi_device_fill_len(ctx, desc_length);
+
+	return 0;
+}
+
 /* ACPI 6.1 section 6.4.3.8.1 - GPIO Interrupt or I/O */
 int acpi_device_write_gpio(struct acpi_ctx *ctx, const struct acpi_gpio *gpio)
 {
@@ -304,6 +356,48 @@  int acpi_device_write_gpio_desc(struct acpi_ctx *ctx,
 	return 0;
 }
 
+int acpi_device_write_interrupt_irq(struct acpi_ctx *ctx,
+				    const struct irq *req_irq)
+{
+	struct acpi_irq irq;
+	int ret;
+
+	ret = irq_get_acpi(req_irq, &irq);
+	if (ret)
+		return log_msg_ret("get", ret);
+	ret = acpi_device_write_interrupt(ctx, &irq);
+	if (ret)
+		return log_msg_ret("write", ret);
+
+	return 0;
+}
+
+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);
+	}
+
+	return 0;
+}
+
 /* ACPI 6.1 section 6.4.3.8.2.1 - I2cSerialBus() */
 static void acpi_device_write_i2c(struct acpi_ctx *ctx,
 				  const struct acpi_i2c *i2c)