Hi Simon, -----"Simon Glass" <s...@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 <s...@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