Re: [PATCH V7 1/3] ACPI: Retry IRQ conversion if it failed previously

From: Agustin Vega-Frias
Date: Tue Nov 15 2016 - 12:43:55 EST

Hi Lorenzo,

On 2016-11-15 10:48, Lorenzo Pieralisi wrote:
On Sun, Nov 13, 2016 at 04:59:33PM -0500, Agustin Vega-Frias wrote:
This allows probe deferral to work properly when a dependent device
fails to get a valid IRQ because the IRQ domain was not registered
at the time the resources were added to the platform_device.

Signed-off-by: Agustin Vega-Frias <agustinv@xxxxxxxxxxxxxx>
---
drivers/acpi/resource.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++
drivers/base/platform.c | 9 +++++++-
include/linux/acpi.h | 7 ++++++
3 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 56241eb..4beda15 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -664,3 +664,62 @@ int acpi_dev_filter_resource_type(struct acpi_resource *ares,
return (type & types) ? 0 : 1;
}
EXPORT_SYMBOL_GPL(acpi_dev_filter_resource_type);
+
+struct acpi_irq_get_ctx {
+ unsigned int index;
+ struct resource *res;
+};
+
+static acpi_status acpi_irq_get_cb(struct acpi_resource *ares, void *context)
+{
+ struct acpi_irq_get_ctx *ctx = context;
+ struct acpi_resource_irq *irq;
+ struct acpi_resource_extended_irq *ext_irq;
+
+ switch (ares->type) {
+ case ACPI_RESOURCE_TYPE_IRQ:
+ irq = &ares->data.irq;
+ if (ctx->index < irq->interrupt_count) {
+ acpi_dev_resource_interrupt(ares, ctx->index, ctx->res);
+ return AE_CTRL_TERMINATE;
+ }
+ ctx->index -= irq->interrupt_count;

I do not understand this code, mind explaining what it is meant to do ?

In particular I do not understand the logic behind the index decrement,
I think I am missing something here.


ACPI IRQ resources can be encoded into two types of structures:

struct acpi_resource_irq,
struct acpi_resource_extended_irq.

In theory only the extended version can contain multiple IRQs, but the Linux
ACPI core accommodates non-compliant DSDT tables that have regular IRQ resources
contain multiple IRQs.

To better explain, suppose you have a device that handles two GSIs and one
other IRQ form a separate device:

Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive, 0x00, )
{ 130, 131 }

Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive, 0x00, "\\_SB.TCS0.QIC0", )
{ 4 }

These are encoded into two separate structures with their own interrupts array:

res0.interrupts[] = { 130, 131 }
res1.interrupts[] = { 4 }

However, from the perspective of a client driver these are indexed into a flat space:

[0] -> 130
[1] -> 131
[2] -> 4

Now say mapping of IRQ 4 failed during bus scan. When acpi_irq_get retries to map
it, the client code will pass index 2. acpi_walk_resources will call acpi_irq_get_cb
with the first IRQ resource. If the index is less than the number of IRQs, we know
this IRQ resource contains the IRQ we want so we call acpi_dev_resource_interrupt
to do the actual mapping and return AE_CTRL_TERMINATE so acpi_walk_resources does
not continue walking the resource buffer. On the other hand if the index is equal
or larger it means we need to skip this IRQ resource and look at the next one,
but we need to adjust the lookup index to that of the next IRQ resource.

Makes sense?

+ break;
+ case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
+ ext_irq = &ares->data.extended_irq;
+ if (ctx->index < ext_irq->interrupt_count) {
+ acpi_dev_resource_interrupt(ares, ctx->index, ctx->res);
+ return AE_CTRL_TERMINATE;
+ }
+ ctx->index -= ext_irq->interrupt_count;

Ditto.

The same logic is used for both types of resources because they are handled in
the same way by the ACPI core when it comes to indexing.

Thanks,
Agustin


Thanks,
Lorenzo

+ break;
+ }
+
+ return AE_OK;
+}
+
+/**
+ * acpi_irq_get - Look for the ACPI IRQ resource with the given index and
+ * use it to initialize the given Linux IRQ resource.
+ * @handle ACPI device handle
+ * @index ACPI IRQ resource index to lookup
+ * @res Linux IRQ resource to initialize
+ *
+ * Return: 0 on success
+ * -EINVAL if an error occurs
+ * -EPROBE_DEFER if the IRQ lookup/conversion failed
+ */
+int acpi_irq_get(acpi_handle handle, unsigned int index, struct resource *res)
+{
+ struct acpi_irq_get_ctx ctx = { index, res };
+ acpi_status status;
+
+ status = acpi_walk_resources(handle, METHOD_NAME__CRS,
+ acpi_irq_get_cb, &ctx);
+ if (ACPI_FAILURE(status))
+ return -EINVAL;
+ if (res->flags & IORESOURCE_DISABLED)
+ return -EPROBE_DEFER;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(acpi_irq_get);
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index c4af003..61423d2 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -102,6 +102,14 @@ int platform_get_irq(struct platform_device *dev, unsigned int num)
}

r = platform_get_resource(dev, IORESOURCE_IRQ, num);
+ if (r && r->flags & IORESOURCE_DISABLED && ACPI_COMPANION(&dev->dev)) {
+ int ret;
+
+ ret = acpi_irq_get(ACPI_HANDLE(&dev->dev), num, r);
+ if (ret)
+ return ret;
+ }
+
/*
* The resources may pass trigger flags to the irqs that need
* to be set up. It so happens that the trigger flags for
@@ -1450,4 +1458,3 @@ void __init early_platform_cleanup(void)
memset(&pd->dev.devres_head, 0, sizeof(pd->dev.devres_head));
}
}
-
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 689a8b9..325bdb9 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -406,6 +406,7 @@ bool acpi_dev_resource_ext_address_space(struct acpi_resource *ares,
unsigned int acpi_dev_get_irq_type(int triggering, int polarity);
bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
struct resource *res);
+int acpi_irq_get(acpi_handle handle, unsigned int index, struct resource *res);

void acpi_dev_free_resource_list(struct list_head *list);
int acpi_dev_get_resources(struct acpi_device *adev, struct list_head *list,
@@ -763,6 +764,12 @@ static inline int acpi_reconfig_notifier_unregister(struct notifier_block *nb)
return -EINVAL;
}

+static inline int acpi_irq_get(acpi_handle handle, unsigned int index,
+ struct resource *res)
+{
+ return -EINVAL;
+}
+
#endif /* !CONFIG_ACPI */

#ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
--
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.