[v2,10/35] acpi: Support generation of a GPIO/irq for a device

Message ID 20200510203409.203520-8-sjg@chromium.org
State Accepted
Commit 4ebc940b39b6a43de9d1fa74653321cd6fdb4d3a
Headers show
Series
  • dm: Add programmatic generation of ACPI tables (part B)
Related show

Commit Message

Simon Glass May 10, 2020, 8:33 p.m.
Some devices use interrupts but some use GPIOs. Since these are fully
specified in the device tree we can automatically produce the correct ACPI
descriptor for a device.

Add a function to handle this.

Signed-off-by: Simon Glass <sjg at chromium.org>
---

Changes in v2: None
Changes in v1: None

 include/acpi/acpi_device.h | 15 ++++++++++++++
 lib/acpi/acpi_device.c     | 26 ++++++++++++++++++++++++
 test/dm/acpigen.c          | 41 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+)

Comments

Wolfgang Wallner May 19, 2020, 9:32 a.m. | #1
Hi Simon,

-----"Simon Glass" <sjg at chromium.org> schrieb: -----
> Betreff: [PATCH v2 10/35] acpi: Support generation of a GPIO/irq for a device
> 
> Some devices use interrupts but some use GPIOs. Since these are fully
> specified in the device tree we can automatically produce the correct ACPI
> descriptor for a device.
> 
> Add a function to handle this.
> 
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
> 
> Changes in v2: None
> Changes in v1: None
> 
>  include/acpi/acpi_device.h | 15 ++++++++++++++
>  lib/acpi/acpi_device.c     | 26 ++++++++++++++++++++++++
>  test/dm/acpigen.c          | 41 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 82 insertions(+)
> 
> diff --git a/include/acpi/acpi_device.h b/include/acpi/acpi_device.h
> index 5f229d79cf..70c151d150 100644
> --- a/include/acpi/acpi_device.h
> +++ b/include/acpi/acpi_device.h
> @@ -255,4 +255,19 @@ int acpi_device_write_gpio(struct acpi_ctx *ctx, const struct acpi_gpio *gpio);
>  int acpi_device_write_gpio_desc(struct acpi_ctx *ctx,
>  				const struct gpio_desc *desc);
>  
> +/**
> + * acpi_device_write_interrupt_or_gpio() - Write interrupt or GPIO to ACPI
> + *
> + * This reads an interrupt from the device tree "interrupts-extended" property,
> + * if available. If not it reads the first GPIO with the name @prop.
> + *
> + * If an interrupt is found, an ACPI interrupt descriptor is written to the ACPI
> + * output. If not, but an GPIO is found, a GPIO descriptor is written.

Nit: I think it should be "a GPIO is found"

> + *
> + * @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);
> +
>  #endif
> diff --git a/lib/acpi/acpi_device.c b/lib/acpi/acpi_device.c
> index 4c5bfdb9a2..423b91cfd2 100644
> --- a/lib/acpi/acpi_device.c
> +++ b/lib/acpi/acpi_device.c
> @@ -355,3 +355,29 @@ int acpi_device_write_gpio_desc(struct acpi_ctx *ctx,
>  
>  	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;
> +}

Note to other reviewers:
The function above was already discussed in another thread at [1].
Summary: the index value 0 is hardcoded, as only a single interrupt / GPIO
is supported at present.

[1] https://lists.denx.de/pipermail/u-boot/2020-March/403557.html

> diff --git a/test/dm/acpigen.c b/test/dm/acpigen.c
> index 73fe6c9f4d..6aefa6845d 100644
> --- a/test/dm/acpigen.c
> +++ b/test/dm/acpigen.c
> @@ -16,6 +16,7 @@
>  #include <asm/unaligned.h>
>  #include <dm/acpi.h>
>  #include <dm/test.h>
> +#include <dm/uclass-internal.h>
>  #include <test/ut.h>
>  
>  #define TEST_STRING	"frogmore"
> @@ -225,3 +226,43 @@ static int dm_test_acpi_gpio_irq(struct unit_test_state *uts)
>  	return 0;
>  }
>  DM_TEST(dm_test_acpi_gpio_irq, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
> +
> +/* Test emitting either a GPIO or interrupt descriptor */
> +static int dm_test_acpi_interrupt_or_gpio(struct unit_test_state *uts)
> +{
> +	struct acpi_ctx *ctx;
> +	struct udevice *dev;
> +	u8 *ptr;
> +
> +	ut_assertok(alloc_context(&ctx));
> +
> +	ptr = acpigen_get_current(ctx);
> +
> +	/* This should produce an interrupt, even though it also has a GPIO */
> +	ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 0, &dev));
> +	ut_asserteq_str("a-test", dev->name);
> +	ut_assertok(acpi_device_write_interrupt_or_gpio(ctx, dev,
> +							"test2-gpios"));
> +	ut_asserteq(ACPI_DESCRIPTOR_INTERRUPT, ptr[0]);
> +
> +	/* This has no interrupt so should produce a GPIO */
> +	ptr = ctx->current;
> +	ut_assertok(uclass_find_first_device(UCLASS_PANEL_BACKLIGHT, &dev));
> +	ut_assertok(acpi_device_write_interrupt_or_gpio(ctx, dev,
> +							"enable-gpios"));
> +	ut_asserteq(ACPI_DESCRIPTOR_GPIO, ptr[0]);
> +
> +	/* This one has neither */
> +	ptr = acpigen_get_current(ctx);
> +	ut_assertok(uclass_get_device_by_seq(UCLASS_TEST_FDT, 3, &dev));
> +	ut_asserteq_str("b-test", dev->name);
> +	ut_asserteq(-ENOENT,
> +		    acpi_device_write_interrupt_or_gpio(ctx, dev,
> +							"enable-gpios"));
> +
> +	free_context(&ctx);
> +
> +	return 0;
> +}
> +DM_TEST(dm_test_acpi_interrupt_or_gpio,
> +	DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
> -- 
> 2.26.2.645.ge9eca65c58-goog
> 
 
Reviewed-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>

Patch

diff --git a/include/acpi/acpi_device.h b/include/acpi/acpi_device.h
index 5f229d79cf..70c151d150 100644
--- a/include/acpi/acpi_device.h
+++ b/include/acpi/acpi_device.h
@@ -255,4 +255,19 @@  int acpi_device_write_gpio(struct acpi_ctx *ctx, const struct acpi_gpio *gpio);
 int acpi_device_write_gpio_desc(struct acpi_ctx *ctx,
 				const struct gpio_desc *desc);
 
+/**
+ * acpi_device_write_interrupt_or_gpio() - Write interrupt or GPIO to ACPI
+ *
+ * This reads an interrupt from the device tree "interrupts-extended" property,
+ * if available. If not it reads the first GPIO with the name @prop.
+ *
+ * If an interrupt is found, an ACPI interrupt descriptor is written to the ACPI
+ * output. If not, but an GPIO is found, a GPIO descriptor 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);
+
 #endif
diff --git a/lib/acpi/acpi_device.c b/lib/acpi/acpi_device.c
index 4c5bfdb9a2..423b91cfd2 100644
--- a/lib/acpi/acpi_device.c
+++ b/lib/acpi/acpi_device.c
@@ -355,3 +355,29 @@  int acpi_device_write_gpio_desc(struct acpi_ctx *ctx,
 
 	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;
+}
diff --git a/test/dm/acpigen.c b/test/dm/acpigen.c
index 73fe6c9f4d..6aefa6845d 100644
--- a/test/dm/acpigen.c
+++ b/test/dm/acpigen.c
@@ -16,6 +16,7 @@ 
 #include <asm/unaligned.h>
 #include <dm/acpi.h>
 #include <dm/test.h>
+#include <dm/uclass-internal.h>
 #include <test/ut.h>
 
 #define TEST_STRING	"frogmore"
@@ -225,3 +226,43 @@  static int dm_test_acpi_gpio_irq(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_acpi_gpio_irq, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+
+/* Test emitting either a GPIO or interrupt descriptor */
+static int dm_test_acpi_interrupt_or_gpio(struct unit_test_state *uts)
+{
+	struct acpi_ctx *ctx;
+	struct udevice *dev;
+	u8 *ptr;
+
+	ut_assertok(alloc_context(&ctx));
+
+	ptr = acpigen_get_current(ctx);
+
+	/* This should produce an interrupt, even though it also has a GPIO */
+	ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 0, &dev));
+	ut_asserteq_str("a-test", dev->name);
+	ut_assertok(acpi_device_write_interrupt_or_gpio(ctx, dev,
+							"test2-gpios"));
+	ut_asserteq(ACPI_DESCRIPTOR_INTERRUPT, ptr[0]);
+
+	/* This has no interrupt so should produce a GPIO */
+	ptr = ctx->current;
+	ut_assertok(uclass_find_first_device(UCLASS_PANEL_BACKLIGHT, &dev));
+	ut_assertok(acpi_device_write_interrupt_or_gpio(ctx, dev,
+							"enable-gpios"));
+	ut_asserteq(ACPI_DESCRIPTOR_GPIO, ptr[0]);
+
+	/* This one has neither */
+	ptr = acpigen_get_current(ctx);
+	ut_assertok(uclass_get_device_by_seq(UCLASS_TEST_FDT, 3, &dev));
+	ut_asserteq_str("b-test", dev->name);
+	ut_asserteq(-ENOENT,
+		    acpi_device_write_interrupt_or_gpio(ctx, dev,
+							"enable-gpios"));
+
+	free_context(&ctx);
+
+	return 0;
+}
+DM_TEST(dm_test_acpi_interrupt_or_gpio,
+	DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);