Re: [Qemu-devel] [PATCH 4/7] hw/acpi/aml-build: Add aml_gpio_int() term

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, May 12, 2015 at 12:24:13PM +0800, shannon.zhao@linaro.org wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  hw/acpi/aml-build.c         | 60 +++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/acpi/aml-build.h | 16 ++++++++++++
>  2 files changed, 76 insertions(+)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 643e885..40d7fa0 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -525,6 +525,66 @@ Aml *aml_call4(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4)
>  }
>  
>  /*
> + * ACPI 5.0: 19.5.53
> + * GpioInt(GPIO Interrupt Connection Resource Descriptor Macro)

GpioInt is an ASL thing, it has no place in QEMU as we
don't use ASL.

What you should point at, and model API after, is
6.4.3.8.1 GPIO Connection Descriptor

In fact, why not describe the layout with a struct?

That's much nicer than building it up byte by byte.
Also, this way offsets will be self-documenting
using sizeof.

> + */
> +Aml *aml_gpio_int(AmlLevelAndEdge level_and_edge,
> +                  AmlActiveHighAndLow high_and_low,
> +                  AmlExclusiveAndShared exclusive_and_shared,
> +                  AmlWakeCap wake_capable, AmlPinConfig pin_cfg,
> +                  int32_t pin_num, const char *name)
> +{
> +    Aml *var = aml_alloc();
> +    uint8_t flags = level_and_edge | (high_and_low << 1)
> +                    | (exclusive_and_shared << 3) | (wake_capable << 4);
> +
> +    build_append_byte(var->buf, 0x8C); /* GpioInt Resource Descriptor */
> +    /* Length */
> +    build_append_byte(var->buf, 0x1B); /* bits[7:0] */
> +    build_append_byte(var->buf, 0); /* bits[15:8] */
> +    build_append_byte(var->buf, 1); /* Revision ID */
> +    /* GPIO Connection Type 0x00 = Interrupt Connection */
> +    build_append_byte(var->buf, 0);
> +    /* General Flags */
> +    build_append_byte(var->buf, 1); /* bits[7:0] */
> +    build_append_byte(var->buf, 0); /* bits[15:8] */
> +    /* Interrupt and IO Flags */
> +    build_append_byte(var->buf, flags); /* bits[7:0] */
> +    build_append_byte(var->buf, 0); /* bits[15:8] */
> +    /* Pin Configuration 0 = Default 1 = Pull-up 2 = Pull-down 3 = No Pull */
> +    build_append_byte(var->buf, pin_cfg);
> +    /* Output Drive Strength */
> +    build_append_byte(var->buf, 0); /* bits[7:0] */
> +    build_append_byte(var->buf, 0); /* bits[15:8] */
> +    /* Debounce timeout */
> +    build_append_byte(var->buf, 0); /* bits[7:0] */
> +    build_append_byte(var->buf, 0); /* bits[15:8] */
> +
> +    /* Pin Table Offset */
> +    build_append_byte(var->buf, 0x17); /* bits[7:0] */
> +    build_append_byte(var->buf, 0); /* bits[15:8] */
> +    /* Resource Source Index */
> +    build_append_byte(var->buf, 0);
> +    /* Resource Source Name */
> +    build_append_byte(var->buf, 0x19); /* bits[7:0] */
> +    build_append_byte(var->buf, 0); /* bits[15:8] */
> +    /* Vendor Data Offse */

Offset?

> +    build_append_byte(var->buf, 0x1E); /* bits[7:0] */
> +    build_append_byte(var->buf, 0); /* bits[15:8] */
> +    /* Vendor Data Length */
> +    build_append_byte(var->buf, 0); /* bits[7:0] */
> +    build_append_byte(var->buf, 0); /* bits[15:8] */
> +    /* Pin Numbe */

Number?

> +    build_append_byte(var->buf, pin_num & 0xff); /* bits[7:0] */
> +    build_append_byte(var->buf, (pin_num >> 8) & 0xff); /* bits[15:8] */
> +    /* Resource Source */
> +    build_append_namestring(var->buf, "%s", name);
> +    build_append_byte(var->buf, '\0');
> +
> +    return var;
> +}
> +
> +/*
>   * ACPI 1.0: 6.4.3.4 Memory32Fixed (Memory Resource Descriptor Macro)
>   */
>  Aml *aml_memory32_fixed(uint32_t addr, uint32_t size,
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 39b44e4..b95a8e8 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -150,6 +150,17 @@ typedef enum {
>      aml_wake_capable = 1,
>  } AmlWakeCap;
>  
> +/*
> + * ACPI 5.0: Table 6-189 GPIO Connection Descriptor Definition
> + * _PPI field definition
> + */
> +typedef enum {
> +    aml_default_config = 0,
> +    aml_pull_up = 1,
> +    aml_pull_down = 2,
> +    aml_no_pull = 3,
> +} AmlPinConfig;
> +
>  typedef
>  struct AcpiBuildTables {
>      GArray *table_data;
> @@ -208,6 +219,11 @@ Aml *aml_call1(const char *method, Aml *arg1);
>  Aml *aml_call2(const char *method, Aml *arg1, Aml *arg2);
>  Aml *aml_call3(const char *method, Aml *arg1, Aml *arg2, Aml *arg3);
>  Aml *aml_call4(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4);
> +Aml *aml_gpio_int(AmlLevelAndEdge level_and_edge,
> +                  AmlActiveHighAndLow high_and_low,
> +                  AmlExclusiveAndShared exclusive_and_shared,
> +                  AmlWakeCap wake_capable, AmlPinConfig pin_cfg,
> +                  int32_t pin_num, const char *name);
>  Aml *aml_memory32_fixed(uint32_t addr, uint32_t size,
>                          AmlReadAndWrite read_and_write);
>  Aml *aml_interrupt(AmlConsumerAndProducer con_and_pro,
> -- 
> 2.1.0
>