From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wolfgang Wallner Date: Wed, 18 Mar 2020 17:20:16 +0100 Subject: Antwort: [PATCH v2 32/39] irq: Add a method to convert an interrupt to ACPI In-Reply-To: <20200308214442.v2.32.I29adaf758a9ea81f51ffbecf89b53983968a0d2a@changeid> References: <20200309034504.149659-1-sjg@chromium.org> <20200308214442.v2.32.I29adaf758a9ea81f51ffbecf89b53983968a0d2a@changeid> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Simon, I'm resending this mail, as my email client has broken the formating in the first attempt, sorry. "Simon Glass" schrieb am 09.03.2020 04:44:56: > Von: "Simon Glass" > An: "U-Boot Mailing List" , > Kopie: "Bin Meng" , "Wolfgang Wallner" > , "Andy Shevchenko" > , "Simon Glass" > 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 > --- > > 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 >