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

[Shannon Zhao]

On 2015/6/1 2:21, Michael S. Tsirkin wrote:
> On Sun, May 31, 2015 at 08:13:37PM +0200, Michael S. Tsirkin wrote:
>> On Tue, May 12, 2015 at 12:24:13PM +0800, address@hidden wrote:
>>> From: Shannon Zhao <address@hidden>
>>>
>>> Signed-off-by: Shannon Zhao <address@hidden>
>>> Signed-off-by: Shannon Zhao <address@hidden>
>>> ---
>>>  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
> 
> You can make a wrapper for GpioInt if you like, but
> it should be layered on top of GPIO Connection Descriptor.
> 
Ok, I see. Thanks.

> 
>> 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.
> 
> Or at least, use build_append_int_noprefix for multibyte fields.
> 
>>> + */
>>> +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
>>>
> 
> .
> 

-- 
Shannon