Hi Wolfgang,
On Wed, 13 May 2020 at 07:01, Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
Hi Simon,
-----"Simon Glass" sjg@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@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