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

Reply via email to