Re: [PATCH] hw/acpi: Add aml_gpio_io() wrapper for GPIO IO Connection

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

On Thu, Nov 27, 2025 at 08:36:02PM +0800, AlanoSong@163.com wrote:
> According to ACPI 5.0 section 19.5.54, I add aml_gpio_io()
> wrapper for GPIO IO Connection purpose.
> 
> Signed-off-by: Alano Song <AlanoSong@163.com>
> ---
>  hw/acpi/aml-build.c         | 19 +++++++++++++++++++
>  include/hw/acpi/aml-build.h | 17 +++++++++++++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 2d5826a8f1..b4dd0bc665 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -962,6 +962,25 @@ Aml *aml_gpio_int(AmlConsumerAndProducer con_and_pro,
>                                 vendor_data_len);
>  }
>  
> +/*
> + * ACPI 5.0: 19.5.54
> + * GpioIo(GPIO Connection IO Resource Descriptor Macro)
> + */
> +Aml *aml_gpio_io(AmlConsumerAndProducer con_and_pro,
> +                 AmlIoRestriction io_restriction, AmlShared shared,
> +                 AmlPinConfig pin_config, uint16_t debounce_timeout,
> +                 const uint32_t pin_list[], uint32_t pin_count,
> +                 const char *resource_source_name,
> +                 const uint8_t *vendor_data, uint16_t vendor_data_len)
> +{
> +    uint8_t flags = io_restriction | shared << 3;
> +
> +    return aml_gpio_connection(AML_IO_CONNECTION, con_and_pro, flags,
> +                               pin_config, 0, debounce_timeout, pin_list,
> +                               pin_count, resource_source_name, vendor_data,
> +                               vendor_data_len);
> +}
> +
>  /*
>   * ACPI 1.0b: 6.4.3.4 32-Bit Fixed Location Memory Range Descriptor
>   * (Type 1, Large Item Name 0x6)
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index f38e129719..f5c0c5886b 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -167,6 +167,17 @@ typedef enum {
>      AML_ACTIVE_LOW = 1,
>  } AmlActiveHighAndLow;
>  
> +/*
> + * ACPI 5.0: Table 5-133 Predefined ACPI Names

what does that have to do with anything?

> + * _IOR field definition
> + */
> +typedef enum {
> +    AML_IO_RESTRICTION_NONE = 0,
> +    AML_IO_RESTRICTION_INPUT_ONLY = 1,
> +    AML_IO_RESTRICTION_OUTPUT_ONLY = 2,
> +    AML_IO_RESTRICTION_NONE_AND_PRESERVE = 3,

these numbers are not from that table.

> +} AmlIoRestriction;
> +

the actual table is here:

Table 6-189 GPIO Connection Descriptor Definition



and there are no names there to not match this enum.

So a better way to do it is different. Pass in u8,
and at the calling site, you add text matching spec verbatim:
	0x1 /* This pin or pins can only be used for Input, and the pin configuration must be preserved while not in use.  */

only use an enum if same value used multiple times.
in that cases, add this comment at the enum values.

but it is important to use spec text verbatim so people can
easily find the relevant spec part.



>  /*
>   * ACPI 5.0: Table 6-187 Extended Interrupt Descriptor Definition
>   * _SHR field definition
> @@ -331,6 +342,12 @@ Aml *aml_gpio_int(AmlConsumerAndProducer con_and_pro,
>                    const uint32_t pin_list[], uint32_t pin_count,
>                    const char *resource_source_name,
>                    const uint8_t *vendor_data, uint16_t vendor_data_len);
> +Aml *aml_gpio_io(AmlConsumerAndProducer con_and_pro,
> +                 AmlIoRestriction io_restriction, AmlShared shared,
> +                 AmlPinConfig pin_config, uint16_t debounce_timeout,
> +                 const uint32_t pin_list[], uint32_t pin_count,
> +                 const char *resource_source_name,
> +                 const uint8_t *vendor_data, uint16_t vendor_data_len);
>  Aml *aml_memory32_fixed(uint32_t addr, uint32_t size,
>                          AmlReadAndWrite read_and_write);
>  Aml *aml_interrupt(AmlConsumerAndProducer con_and_pro,


fyi I am not merging unused code. pls make this patch a part
of a patchset using this.

> -- 
> 2.43.0