Re: [Qemu-devel] [PATCH v4 2/8] ARM: ACPI: Add GPIO controller in ACPI DSDT table

[Wei Huang <wei@redhat.com>]

On 12/10/2015 07:22 AM, Igor Mammedov wrote:
> On Mon, 7 Dec 2015 15:39:46 +0800
> Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> 
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> Add GPIO controller in ACPI DSDT table. It can be used for gpio event.
>>
>> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> Tested-by: Wei Huang <wei@redhat.com>
>> ---
>>  hw/arm/virt-acpi-build.c | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>>
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 3c2c5d6..bf6b934 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -323,6 +323,23 @@ static void acpi_dsdt_add_pci(Aml *scope, const
>> MemMapEntry *memmap, int irq, aml_append(scope, dev);
>>  }
>>  
>> +static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry
>> *gpio_memmap,
>> +                                           int gpio_irq)
> s/int/uint32_t/
> 
>> +{
>> +    Aml *dev = aml_device("GPO0");
>> +    aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0061")));
> What spec "ARMH0061" comes from?
> Probably it should be mentioned in comment.

I tried to find the official source for this definition several weeks
ago, but failed. Later, in private, Al Stone confirmed with me that
ARMH0061 is the right one. Maybe ARM or Linaro should push this
definition into ACPI spec or somewhere else in a formal way.

> 
>> +    aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
>> +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
>> +
>> +    Aml *crs = aml_resource_template();
>> +    aml_append(crs, aml_memory32_fixed(gpio_memmap->base,
>> gpio_memmap->size,
>> +                                       AML_READ_WRITE));
>> +    aml_append(crs, aml_interrupt(AML_CONSUMER, AML_LEVEL,
>> AML_ACTIVE_HIGH,
>> +                                  AML_EXCLUSIVE, gpio_irq));
> that conflicts with
> https://github.com/imammedo/qemu/commit/acb34e533bc31fdf3eb6230c93654b0b0ae4e76e
> perhaps you could include it in your series and redo this hunk
> to take array instead of int.
> 
> 
>> +    aml_append(dev, aml_name_decl("_CRS", crs));
>> +    aml_append(scope, dev);
>> +}
>> +
>>  /* RSDP */
>>  static GArray *
>>  build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
>> @@ -539,6 +556,8 @@ build_dsdt(GArray *table_data, GArray *linker,
>> VirtGuestInfo *guest_info) (irqmap[VIRT_MMIO] + ARM_SPI_BASE),
>> NUM_VIRTIO_TRANSPORTS); acpi_dsdt_add_pci(scope, memmap,
>> (irqmap[VIRT_PCIE] + ARM_SPI_BASE), guest_info->use_highmem);
>> +    acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
>> +                       (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
>>  
>>      aml_append(dsdt, scope);
>>  
>