| Message ID | 20200510203409.203520-9-sjg@chromium.org |
|---|---|
| State | New |
| Headers | show |
| Series | dm: Add programmatic generation of ACPI tables (part B) | expand |
Hi Simon, -----"Simon Glass" <sjg at chromium.org> schrieb: ----- > Betreff: [PATCH v2 11/35] acpi: Support generation of I2C descriptor > > Add a function to write a GPIO descriptor to the generated ACPI code. > > Signed-off-by: Simon Glass <sjg at chromium.org> > --- > > Changes in v2: > - Fix memset of I2C descriptor > > Changes in v1: None > > drivers/i2c/sandbox_i2c.c | 11 ++++ > drivers/rtc/sandbox_rtc.c | 13 +++++ > include/acpi/acpi_device.h | 36 +++++++++++++ > lib/acpi/acpi_device.c | 103 +++++++++++++++++++++++++++++++++++++ > test/dm/acpigen.c | 32 ++++++++++++ > 5 files changed, 195 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 70c151d150..cf1ac695a7 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 > + > +/** > + * 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 423b91cfd2..0136a0bdc9 100644 > --- a/lib/acpi/acpi_device.c > +++ b/lib/acpi/acpi_device.c > @@ -381,3 +381,106 @@ 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 > + * > + * @dev: I2C device to convert > + * @i2c: Place to put the new structure > + * @scope: Scope of the I2C device (this is the controller path) >From the declaration I would assume that "scope" is internally copied, but in the code it is only referenced. I would propose to add something like the following to its description: "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." > + * @return 0 (always) dev_get_parent_platdata() could return NULL. Should we check this and return and error? > + */ > +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)); Nit: memset(i2c, 0, sizeof(*i2c)); This is only a style question. But it seems 0 is used for memset in existing U-Boot code much more often then '\0' (~120 grep results vs ~1100 grep results). > + 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", 100000); Nit: I2C_SPEED_STANDARD_RATE instead of 100000? > + 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 6aefa6845d..de9996ab35 100644 > --- a/test/dm/acpigen.c > +++ b/test/dm/acpigen.c > @@ -266,3 +266,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); > -- > 2.26.2.645.ge9eca65c58-goog The remarks above are just this: remarks. I would also be fine with merging the code as it is: Reviewed-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>
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 70c151d150..cf1ac695a7 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 + +/** + * 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 423b91cfd2..0136a0bdc9 100644 --- a/lib/acpi/acpi_device.c +++ b/lib/acpi/acpi_device.c @@ -381,3 +381,106 @@ 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 + * + * @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", 100000); + 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 6aefa6845d..de9996ab35 100644 --- a/test/dm/acpigen.c +++ b/test/dm/acpigen.c @@ -266,3 +266,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);
Add a function to write a GPIO descriptor to the generated ACPI code. Signed-off-by: Simon Glass <sjg at chromium.org> --- Changes in v2: - Fix memset of I2C descriptor Changes in v1: None drivers/i2c/sandbox_i2c.c | 11 ++++ drivers/rtc/sandbox_rtc.c | 13 +++++ include/acpi/acpi_device.h | 36 +++++++++++++ lib/acpi/acpi_device.c | 103 +++++++++++++++++++++++++++++++++++++ test/dm/acpigen.c | 32 ++++++++++++ 5 files changed, 195 insertions(+)