Antwort: [PATCH v2 20/35] acpi: Support writing a GPIO

Hi Simon,

-----"Simon Glass" sjg@chromium.org schrieb: -----

Betreff: [PATCH v2 20/35] acpi: Support writing a GPIO

Allowing writing out a reference to a GPIO within the ACPI output. This can be used by ACPI code to access a GPIO at runtime.

Signed-off-by: Simon Glass sjg@chromium.org

Changes in v2: None Changes in v1: None

include/acpi/acpi_dp.h | 18 ++++++++++++++++++ lib/acpi/acpi_dp.c | 21 +++++++++++++++++++++ test/dm/acpi_dp.c | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+)

diff --git a/include/acpi/acpi_dp.h b/include/acpi/acpi_dp.h index 840be855c5..f42602405a 100644 --- a/include/acpi/acpi_dp.h +++ b/include/acpi/acpi_dp.h @@ -202,6 +202,24 @@ struct acpi_dp *acpi_dp_add_integer_array(struct acpi_dp *dp, const char *name, struct acpi_dp *acpi_dp_add_child(struct acpi_dp *dp, const char *name, struct acpi_dp *child);

+/**

• • acpi_dp_add_gpio() - Add a GPIO to a list of Device Properties
• • A new node is added to the end of the property list of @dp, with the
• • GPIO properties added to the the new node
• • @dp: Table to add this property to
• • @name: Name of property
• • @ref: Reference to device with a _CRS containing GpioIO or GpioInt
• • @index: Index of the GPIO resource in _CRS starting from zero
• • @pin: Pin in the GPIO resource, typically zero
• • @active_low: true if pin is active low
• • @return pointer to new node, or NULL if out of memory
• */

+struct acpi_dp *acpi_dp_add_gpio(struct acpi_dp *dp, const char *name,

• 		 const char *ref, int index, int pin,
  

• 		 bool active_low);
  

/**

• acpi_dp_write() - Write Device Property hierarchy and clean up resources

diff --git a/lib/acpi/acpi_dp.c b/lib/acpi/acpi_dp.c index 479cb6743c..4ba5d555a0 100644 --- a/lib/acpi/acpi_dp.c +++ b/lib/acpi/acpi_dp.c @@ -322,3 +322,24 @@ struct acpi_dp *acpi_dp_add_integer_array(struct acpi_dp *dp, const char *name,

return dp_array; }

+struct acpi_dp *acpi_dp_add_gpio(struct acpi_dp *dp, const char *name,

• 		 const char *ref, int index, int pin,
  

• 		 bool active_low)
  

Boolean parameters can be hard to read. Could we instead of a boolean parameter use the type enum acpi_irq_polarity which is defined in acpi_device.h?

+{

• struct acpi_dp *gpio;
• assert(dp);
• gpio = acpi_dp_new_table(name);
• if (!gpio)

• return NULL;
  

• acpi_dp_add_reference(gpio, NULL, ref);
• acpi_dp_add_integer(gpio, NULL, index);
• acpi_dp_add_integer(gpio, NULL, pin);
• acpi_dp_add_integer(gpio, NULL, active_low);

Is it allowed to pass NULL for the parameter 'name' of these functions?

The descriptions of acpi_dp_add_reference() and acpi_dp_add_integer() don't mention that name may be NULL, and both of these functions pass it on internally to acpi_dp_new() which has an explicit check for assert(name) ... ?

Also, acpi_dp_add_reference() and acpi_dp_add_integer() could return an error (NULL), which is not checked for here.

• acpi_dp_add_array(dp, gpio);
• return gpio;

+} diff --git a/test/dm/acpi_dp.c b/test/dm/acpi_dp.c index c11394786f..d6a054a98b 100644 --- a/test/dm/acpi_dp.c +++ b/test/dm/acpi_dp.c @@ -403,3 +403,42 @@ static int dm_test_acpi_dp_child(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_acpi_dp_child, 0);

+/* Test emitting a GPIO */ +static int dm_test_acpi_dp_gpio(struct unit_test_state *uts) +{

• struct acpi_ctx *ctx;
• struct acpi_dp *dp;
• u8 *ptr, *pptr;
• ut_assertok(alloc_context(&ctx));
• dp = acpi_dp_new_table("FRED");
• ut_assertnonnull(dp);
• /* Try a few different parameters */
• ut_assertnonnull(acpi_dp_add_gpio(dp, "reset", TEST_REF, 0x23, 0x24,

• 			  false));
  

• ut_assertnonnull(acpi_dp_add_gpio(dp, "allow", TEST_REF, 0, 0, true));
• ptr = acpigen_get_current(ctx);
• ut_assertok(acpi_dp_write(ctx, dp));
• ut_asserteq(0x6e, acpigen_get_current(ctx) - ptr);
• pptr = ptr + 0x2c; //0x3a;
• ut_asserteq_str("reset", (char *)pptr);
• ut_asserteq_strn(EXPECT_REF, (char *)pptr + 0xe);
• ut_asserteq(0x23, pptr[0x1b]);
• ut_asserteq(0x24, pptr[0x1d]);
• ut_asserteq(ZERO_OP, pptr[0x1e]);
• pptr = ptr + 0x51;
• ut_asserteq_str("allow", (char *)pptr);
• ut_asserteq_strn(EXPECT_REF, (char *)pptr + 0xe);
• ut_asserteq(ZERO_OP, pptr[0x1a]);
• ut_asserteq(ZERO_OP, pptr[0x1b]);
• ut_asserteq(ONE_OP, pptr[0x1c]);
• return 0;

+}

+DM_TEST(dm_test_acpi_dp_gpio, 0);

2.26.2.645.ge9eca65c58-goog

regards, Wolfgang