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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.
--
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



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux