diff mbox

[v4,2/8] ARM: ACPI: Add GPIO controller in ACPI DSDT table

Message ID 1449473992-11560-3-git-send-email-zhaoshenglong@huawei.com
State New
Headers show

Commit Message

Shannon Zhao Dec. 7, 2015, 7:39 a.m. UTC 
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(+)

Comments

Igor Mammedov Dec. 10, 2015, 1:22 p.m. UTC | #1 
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.

> +    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);
>
Wei Huang Dec. 10, 2015, 7:16 p.m. UTC | #2 
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);
>>  
>
Igor Mammedov Dec. 10, 2015, 8:13 p.m. UTC | #3 
On Thu, 10 Dec 2015 13:16:22 -0600
Wei Huang <wei@redhat.com> wrote:

> 
> 
> 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.
I've also tried and only saw it on lkml in mentioned patchset,
which wasn't warmly met.
Given that and lack of spec where it's allocated perhaps we should wait
on it till ARM puts in in some their spec formally.

> 
> > 
> >> +    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);
> >>  
> >
Shannon Zhao Dec. 11, 2015, 1:35 a.m. UTC | #4 
Hi,

On 2015/12/11 4:13, Igor Mammedov wrote:
> On Thu, 10 Dec 2015 13:16:22 -0600
> Wei Huang <wei@redhat.com> wrote:
> 
>> > 
>> > 
>> > 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.
> I've also tried and only saw it on lkml in mentioned patchset,
> which wasn't warmly met.
> Given that and lack of spec where it's allocated perhaps we should wait
> on it till ARM puts in in some their spec formally.
> 
We discussed this at previous patchset with Peter Maydell and Al Stone.
I think ARM should publish a list of ACPI ID for its own devices. Al
also confirmed the _HID of PL061 is ARMH0061.

>> > 
>>> > > 
>>>> > >> +    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.
>>> > > 
sure

>>> > > 
>>>> > >> +    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);
>>>> > >>  
>>> > >
diff mbox

Patch

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)
+{
+    Aml *dev = aml_device("GPO0");
+    aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0061")));
+    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));
+    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);