Hi Simon, On Sun, Jun 14, 2020 at 10:55 AM Simon Glass <s...@chromium.org> wrote: > > Add a function to write a GPIO descriptor to the generated ACPI code. > > Reviewed-by: Wolfgang Wallner <wolfgang.wall...@br-automation.com> > Signed-off-by: Simon Glass <s...@chromium.org> > --- > > Changes in v3: > - Update comment in acpi_device_set_i2c() to talk about scope parameter > > Changes in v2: > - Fix memset of I2C descriptor > > drivers/i2c/sandbox_i2c.c | 11 ++++ > drivers/rtc/sandbox_rtc.c | 13 +++++ > include/acpi/acpi_device.h | 36 +++++++++++++ > lib/acpi/acpi_device.c | 107 +++++++++++++++++++++++++++++++++++++ > test/dm/acpigen.c | 32 +++++++++++ > 5 files changed, 199 insertions(+) > > diff --git a/drivers/i2c/sandbox_i2c.c b/drivers/i2c/sandbox_i2c.c > index f4ae2397a0..125026da90 100644 > --- a/drivers/i2c/sandbox_i2c.c > +++ b/drivers/i2c/sandbox_i2c.c > @@ -11,6 +11,7 @@ > #include <i2c.h> > #include <log.h> > #include <asm/test.h> > +#include <dm/acpi.h> > #include <dm/lists.h> > #include <dm/device-internal.h> > > @@ -83,6 +84,15 @@ static int sandbox_i2c_xfer(struct udevice *bus, struct > i2c_msg *msg, > return ops->xfer(emul, msg, nmsgs); > } > > +static int sandbox_i2c_get_name(const struct udevice *dev, char *out_name) > +{ > + return acpi_copy_name(out_name, "SI2C"); > +} > + > +struct acpi_ops sandbox_i2c_acpi_ops = { > + .get_name = sandbox_i2c_get_name, > +}; > + > static const struct dm_i2c_ops sandbox_i2c_ops = { > .xfer = sandbox_i2c_xfer, > }; > @@ -98,4 +108,5 @@ U_BOOT_DRIVER(i2c_sandbox) = { > .of_match = sandbox_i2c_ids, > .ops = &sandbox_i2c_ops, > .priv_auto_alloc_size = sizeof(struct sandbox_i2c_priv), > + ACPI_OPS_PTR(&sandbox_i2c_acpi_ops) > }; > diff --git a/drivers/rtc/sandbox_rtc.c b/drivers/rtc/sandbox_rtc.c > index b08d758a74..f2906c3397 100644 > --- a/drivers/rtc/sandbox_rtc.c > +++ b/drivers/rtc/sandbox_rtc.c > @@ -9,6 +9,7 @@ > #include <i2c.h> > #include <rtc.h> > #include <asm/rtc.h> > +#include <dm/acpi.h> > > #define REG_COUNT 0x80 > > @@ -84,6 +85,17 @@ static int sandbox_rtc_write8(struct udevice *dev, > unsigned int reg, int val) > return dm_i2c_reg_write(dev, reg, val); > } > > +#if CONFIG_IS_ENABLED(ACPIGEN) > +static int sandbox_rtc_get_name(const struct udevice *dev, char *out_name) > +{ > + return acpi_copy_name(out_name, "RTCC"); > +} > + > +struct acpi_ops sandbox_rtc_acpi_ops = { > + .get_name = sandbox_rtc_get_name, > +}; > +#endif > + > static const struct rtc_ops sandbox_rtc_ops = { > .get = sandbox_rtc_get, > .set = sandbox_rtc_set, > @@ -102,4 +114,5 @@ U_BOOT_DRIVER(rtc_sandbox) = { > .id = UCLASS_RTC, > .of_match = sandbox_rtc_ids, > .ops = &sandbox_rtc_ops, > + ACPI_OPS_PTR(&sandbox_rtc_acpi_ops) > }; > diff --git a/include/acpi/acpi_device.h b/include/acpi/acpi_device.h > index 863bf7cf47..f738b03d58 100644 > --- a/include/acpi/acpi_device.h > +++ b/include/acpi/acpi_device.h > @@ -9,6 +9,7 @@ > #ifndef __ACPI_DEVICE_H > #define __ACPI_DEVICE_H > > +#include <i2c.h> > #include <linux/bitops.h> > > struct acpi_ctx; > @@ -183,6 +184,29 @@ struct acpi_gpio { > enum acpi_gpio_polarity polarity; > }; > > +/* ACPI Descriptors for Serial Bus interfaces */ > +#define ACPI_SERIAL_BUS_TYPE_I2C 1 > +#define ACPI_SERIAL_BUS_TYPE_SPI 2 > +#define ACPI_I2C_SERIAL_BUS_REVISION_ID 1 /* TODO: upgrade to > 2 */ > +#define ACPI_I2C_TYPE_SPECIFIC_REVISION_ID 1 > +#define ACPI_SPI_SERIAL_BUS_REVISION_ID 1 > +#define ACPI_SPI_TYPE_SPECIFIC_REVISION_ID 1
nits: the SPI macros should be introduced in the next patch > + > +/** > + * struct acpi_i2c - representation of an ACPI I2C device > + * > + * @address: 7-bit or 10-bit I2C address > + * @mode_10bit: Which address size is used > + * @speed: Bus speed in Hz > + * @resource: Resource name for the I2C controller > + */ > +struct acpi_i2c { > + u16 address; > + enum i2c_address_mode mode_10bit; > + enum i2c_speed_rate speed; > + const char *resource; > +}; > + > /** > * acpi_device_path() - Get the full path to an ACPI device > * > @@ -270,4 +294,16 @@ int acpi_device_write_gpio_desc(struct acpi_ctx *ctx, > int acpi_device_write_interrupt_or_gpio(struct acpi_ctx *ctx, > struct udevice *dev, const char > *prop); > > +/** > + * acpi_device_write_i2c_dev() - Write an I2C device to ACPI > + * > + * This creates a I2cSerialBus descriptor for an I2C device, including > + * information ACPI needs to use it. > + * > + * @ctx: ACPI context pointer > + * @dev: I2C device to write > + * @return 0 if OK, -ve on error > + */ > +int acpi_device_write_i2c_dev(struct acpi_ctx *ctx, const struct udevice > *dev); > + > #endif > diff --git a/lib/acpi/acpi_device.c b/lib/acpi/acpi_device.c > index bf16e8d9dd..40d1a82d27 100644 > --- a/lib/acpi/acpi_device.c > +++ b/lib/acpi/acpi_device.c > @@ -381,3 +381,110 @@ int acpi_device_write_interrupt_or_gpio(struct acpi_ctx > *ctx, > > return 0; > } > + > +/* ACPI 6.3 section 6.4.3.8.2.1 - I2cSerialBus() */ > +static void acpi_device_write_i2c(struct acpi_ctx *ctx, > + const struct acpi_i2c *i2c) > +{ > + void *desc_length, *type_length; > + > + /* Byte 0: Descriptor Type */ > + acpigen_emit_byte(ctx, ACPI_DESCRIPTOR_SERIAL_BUS); > + > + /* Byte 1+2: Length (filled in later) */ > + desc_length = acpi_device_write_zero_len(ctx); > + > + /* Byte 3: Revision ID */ > + acpigen_emit_byte(ctx, ACPI_I2C_SERIAL_BUS_REVISION_ID); > + > + /* Byte 4: Resource Source Index is Reserved */ > + acpigen_emit_byte(ctx, 0); > + > + /* Byte 5: Serial Bus Type is I2C */ > + acpigen_emit_byte(ctx, ACPI_SERIAL_BUS_TYPE_I2C); > + > + /* > + * Byte 6: Flags > + * [7:2]: 0 => Reserved > + * [1]: 1 => ResourceConsumer > + * [0]: 0 => ControllerInitiated > + */ > + acpigen_emit_byte(ctx, 1 << 1); > + > + /* > + * Byte 7-8: Type Specific Flags > + * [15:1]: 0 => Reserved > + * [0]: 0 => 7bit, 1 => 10bit > + */ > + acpigen_emit_word(ctx, i2c->mode_10bit); > + > + /* Byte 9: Type Specific Revision ID */ > + acpigen_emit_byte(ctx, ACPI_I2C_TYPE_SPECIFIC_REVISION_ID); > + > + /* Byte 10-11: I2C Type Data Length */ > + type_length = acpi_device_write_zero_len(ctx); > + > + /* Byte 12-15: I2C Bus Speed */ > + acpigen_emit_dword(ctx, i2c->speed); > + > + /* Byte 16-17: I2C Slave Address */ > + acpigen_emit_word(ctx, i2c->address); > + > + /* Fill in Type Data Length */ > + acpi_device_fill_len(ctx, type_length); > + > + /* Byte 18+: ResourceSource */ > + acpigen_emit_string(ctx, i2c->resource); > + > + /* Fill in I2C Descriptor Length */ > + acpi_device_fill_len(ctx, desc_length); > +} > + > +/** > + * acpi_device_set_i2c() - Set up an ACPI I2C struct from a device > + * > + * The value of @scope is not copied, but only referenced. This implies the > + * caller has to ensure it stays valid for the lifetime of @i2c. > + * > + * @dev: I2C device to convert > + * @i2c: Place to put the new structure > + * @scope: Scope of the I2C device (this is the controller path) > + * @return 0 (always) > + */ > +static int acpi_device_set_i2c(const struct udevice *dev, struct acpi_i2c > *i2c, > + const char *scope) > +{ > + struct dm_i2c_chip *chip = dev_get_parent_platdata(dev); > + struct udevice *bus = dev_get_parent(dev); > + > + memset(i2c, '\0', sizeof(*i2c)); > + i2c->address = chip->chip_addr; > + i2c->mode_10bit = 0; > + > + /* > + * i2c_bus->speed_hz is set if this device is probed, but if not we > + * must use the device tree > + */ > + i2c->speed = dev_read_u32_default(bus, "clock-frequency", > + I2C_SPEED_STANDARD_RATE); > + i2c->resource = scope; > + > + return 0; > +} > + > +int acpi_device_write_i2c_dev(struct acpi_ctx *ctx, const struct udevice > *dev) > +{ > + char scope[ACPI_PATH_MAX]; > + struct acpi_i2c i2c; > + int ret; > + > + ret = acpi_device_scope(dev, scope, sizeof(scope)); > + if (ret) > + return log_msg_ret("scope", ret); > + ret = acpi_device_set_i2c(dev, &i2c, scope); > + if (ret) > + return log_msg_ret("set", ret); > + acpi_device_write_i2c(ctx, &i2c); > + > + return 0; > +} > diff --git a/test/dm/acpigen.c b/test/dm/acpigen.c > index bd96c2cd57..53842be55a 100644 > --- a/test/dm/acpigen.c > +++ b/test/dm/acpigen.c > @@ -270,3 +270,35 @@ static int dm_test_acpi_interrupt_or_gpio(struct > unit_test_state *uts) > } > DM_TEST(dm_test_acpi_interrupt_or_gpio, > DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); > + > +/* Test emitting an I2C descriptor */ > +static int dm_test_acpi_i2c(struct unit_test_state *uts) > +{ > + struct acpi_ctx *ctx; > + struct udevice *dev; > + u8 *ptr; > + > + ut_assertok(alloc_context(&ctx)); > + > + ptr = acpigen_get_current(ctx); > + > + ut_assertok(uclass_get_device(UCLASS_RTC, 0, &dev)); > + ut_assertok(acpi_device_write_i2c_dev(ctx, dev)); > + ut_asserteq(28, acpigen_get_current(ctx) - ptr); > + ut_asserteq(ACPI_DESCRIPTOR_SERIAL_BUS, ptr[0]); > + ut_asserteq(25, get_unaligned((u16 *)(ptr + 1))); > + ut_asserteq(ACPI_I2C_SERIAL_BUS_REVISION_ID, ptr[3]); > + ut_asserteq(0, ptr[4]); > + ut_asserteq(ACPI_SERIAL_BUS_TYPE_I2C, ptr[5]); > + ut_asserteq(0, get_unaligned((u16 *)(ptr + 7))); > + ut_asserteq(ACPI_I2C_TYPE_SPECIFIC_REVISION_ID, ptr[9]); > + ut_asserteq(6, get_unaligned((u16 *)(ptr + 10))); > + ut_asserteq(100000, get_unaligned((u32 *)(ptr + 12))); > + ut_asserteq(0x43, get_unaligned((u16 *)(ptr + 16))); > + ut_asserteq_str("\\_SB.SI2C", (char *)ptr + 18); > + > + free_context(&ctx); > + > + return 0; > +} > +DM_TEST(dm_test_acpi_i2c, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); > -- Other than that, Reviewed-by: Bin Meng <bmeng...@gmail.com> Regards, Bin