Message ID | 1447680189-2128-5-git-send-email-shannon.zhao@linaro.org |
---|---|
State | New |
Headers | show |
On Mon, 16 Nov 2015 21:23:05 +0800 shannon.zhao@linaro.org wrote: > From: Shannon Zhao <shannon.zhao@linaro.org> Subj can be shortened to: acpi: Add GPIO Connection Descriptor > > 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/acpi/aml-build.c | 61 +++++++++++++++++++++++++++++++++++++++++++++ > include/hw/acpi/aml-build.h | 20 +++++++++++++++ > 2 files changed, 81 insertions(+) > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > index a00a0ab..60d4703 100644 > --- a/hw/acpi/aml-build.c > +++ b/hw/acpi/aml-build.c > @@ -565,6 +565,67 @@ Aml *aml_call4(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4) > } > > /* > + * ACPI 5.0: 6.4.3.8.1 GPIO Connection Descriptor > + * Type 1, Large Item Name 0xC > + */ > + > +static Aml *aml_gpio_connection(AmlGpioConnectionType type, > + AmlConsumerAndProducer con_and_pro, > + uint8_t flags, AmlPinConfig pin_cfg, > + int16_t output_drive, int16_t debounce_timeout, > + int32_t pin_num[], int32_t pin_count, Probably intFOO_t should be uintFOO_t. s/pin_num/pin_list/ I've used a bit more complicated to make list of FOO integers, like this: https://github.com/imammedo/qemu/commit/f6925e53aa2e0266a07dbb39ae17efbf13dba388 + Aml *irqs = aml_interrupt_list(); + aml_append_interrupt2list(irqs, uart_irq); aml_append(crs, aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH, - AML_EXCLUSIVE, uart_irq)); + AML_EXCLUSIVE, irqs)); but I like simpler array way you're using here, Michael do you have any objection to passing IRQ/PIN lists as arrays like this patch does? > + const char *name, const char *vendor_data) s/name/resource_src_name/ s/const char *vendor_data/const uint8_t *vendor_data/ > +{ > + Aml *var = aml_alloc(); > + uint16_t name_len = name ? (strlen(name) + 1) : 0; name doesn't seem to optional so case 'name == NULL' should be invalid, add assert(name) and drop condition > + uint16_t vendor_data_len = vendor_data ? (strlen(vendor_data) + 1) : 0; vendor_data is binary blob so you can't use strlen() on it. > + uint16_t length = 0x16 + name_len + vendor_data_len; s/0x16/const min_desc_len = 0x16/ > + uint16_t name_offset = 0x17 + pin_count * 2; and then use it here and below for calculating pin_table_offset > + uint16_t vendor_data_offset = name_offset + name_len; > + int i; > + > + build_append_byte(var->buf, 0x8C); /* GpioInt Resource Descriptor */ CpioInt is wrong, it should be "GPIO Connection Descriptor" > + build_append_int_noprefix(var->buf, length, 2); /* Length */ > + build_append_byte(var->buf, 1); /* Revision ID */ > + build_append_byte(var->buf, type); /* GPIO Connection Type */ > + /* General Flags (2 bytes) */ > + build_append_int_noprefix(var->buf, con_and_pro, 2); > + /* Interrupt and IO Flags (2 bytes) */ > + build_append_int_noprefix(var->buf, flags, 2); > + /* Pin Configuration 0 = Default 1 = Pull-up 2 = Pull-down 3 = No Pull */ > + build_append_byte(var->buf, pin_cfg); > + /* Output Drive Strength (2 bytes) */ > + build_append_int_noprefix(var->buf, output_drive, 2); > + /* Debounce Timeout (2 bytes) */ > + build_append_int_noprefix(var->buf, debounce_timeout, 2); > + /* Pin Table Offset (2 bytes) */ > + build_append_int_noprefix(var->buf, 0x17, 2); > + build_append_byte(var->buf, 0); /* Resource Source Index */ > + /* Resource Source Name Offset (2 bytes) */ > + build_append_int_noprefix(var->buf, name_offset, 2); > + /* Vendor Data Offset (2 bytes) */ > + build_append_int_noprefix(var->buf, vendor_data_offset, 2); > + /* Vendor Data Length (2 bytes) */ > + build_append_int_noprefix(var->buf, vendor_data_len, 2); > + /* Pin Number (2n bytes)*/ > + for (i = 0; i < pin_count; i++) { > + build_append_int_noprefix(var->buf, pin_num[i], 2); > + } > + /* Resource Source Name */ > + if (name != NULL) { name shouldn't be NULL ever, so drop it > + build_append_namestring(var->buf, "%s", name); > + build_append_byte(var->buf, '\0'); > + } > + /* Vendor-defined Data */ > + if (vendor_data != NULL) { > + build_append_namestring(var->buf, "%s", vendor_data); > + build_append_byte(var->buf, '\0'); > + } that's wrong, vendor_data is RawDataBuffer and not NameString following would do the right thing: g_array_append_vals(var->buf, vendor_data, vendor_data_len); > + > + return var; > +} > + > +/* > * 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 1b632dc..4e88882 100644 > --- a/include/hw/acpi/aml-build.h > +++ b/include/hw/acpi/aml-build.h > @@ -148,6 +148,26 @@ typedef enum { > AML_SHARED_AND_WAKE = 3, > } AmlShared; > > +/* > + * ACPI 5.0: Table 6-189 GPIO Connection Descriptor Definition > + * GPIO Connection Type > + */ > +typedef enum { > + AML_INTERRUPT_CONNECTION = 0, > + AML_IO_CONNECTION = 1, > +} AmlGpioConnectionType; > + > +/* > + * ACPI 5.0: Table 6-189 GPIO Connection Descriptor Definition > + * _PPI field definition > + */ > +typedef enum { > + AML_DEFAULT_CONFIG = 0, I'd suggest to use AML_PULL_DEFAULT as it's mentioned in spec (see GpioInt/GpioIO) > + AML_PULL_UP = 1, > + AML_PULL_DOWN = 2, > + AML_NO_PULL = 3, Likewise AML_PULL_NONE > +} AmlPinConfig; > + > typedef > struct AcpiBuildTables { > GArray *table_data;
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c index a00a0ab..60d4703 100644 --- a/hw/acpi/aml-build.c +++ b/hw/acpi/aml-build.c @@ -565,6 +565,67 @@ Aml *aml_call4(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4) } /* + * ACPI 5.0: 6.4.3.8.1 GPIO Connection Descriptor + * Type 1, Large Item Name 0xC + */ + +static Aml *aml_gpio_connection(AmlGpioConnectionType type, + AmlConsumerAndProducer con_and_pro, + uint8_t flags, AmlPinConfig pin_cfg, + int16_t output_drive, int16_t debounce_timeout, + int32_t pin_num[], int32_t pin_count, + const char *name, const char *vendor_data) +{ + Aml *var = aml_alloc(); + uint16_t name_len = name ? (strlen(name) + 1) : 0; + uint16_t vendor_data_len = vendor_data ? (strlen(vendor_data) + 1) : 0; + uint16_t length = 0x16 + name_len + vendor_data_len; + uint16_t name_offset = 0x17 + pin_count * 2; + uint16_t vendor_data_offset = name_offset + name_len; + int i; + + build_append_byte(var->buf, 0x8C); /* GpioInt Resource Descriptor */ + build_append_int_noprefix(var->buf, length, 2); /* Length */ + build_append_byte(var->buf, 1); /* Revision ID */ + build_append_byte(var->buf, type); /* GPIO Connection Type */ + /* General Flags (2 bytes) */ + build_append_int_noprefix(var->buf, con_and_pro, 2); + /* Interrupt and IO Flags (2 bytes) */ + build_append_int_noprefix(var->buf, flags, 2); + /* Pin Configuration 0 = Default 1 = Pull-up 2 = Pull-down 3 = No Pull */ + build_append_byte(var->buf, pin_cfg); + /* Output Drive Strength (2 bytes) */ + build_append_int_noprefix(var->buf, output_drive, 2); + /* Debounce Timeout (2 bytes) */ + build_append_int_noprefix(var->buf, debounce_timeout, 2); + /* Pin Table Offset (2 bytes) */ + build_append_int_noprefix(var->buf, 0x17, 2); + build_append_byte(var->buf, 0); /* Resource Source Index */ + /* Resource Source Name Offset (2 bytes) */ + build_append_int_noprefix(var->buf, name_offset, 2); + /* Vendor Data Offset (2 bytes) */ + build_append_int_noprefix(var->buf, vendor_data_offset, 2); + /* Vendor Data Length (2 bytes) */ + build_append_int_noprefix(var->buf, vendor_data_len, 2); + /* Pin Number (2n bytes)*/ + for (i = 0; i < pin_count; i++) { + build_append_int_noprefix(var->buf, pin_num[i], 2); + } + /* Resource Source Name */ + if (name != NULL) { + build_append_namestring(var->buf, "%s", name); + build_append_byte(var->buf, '\0'); + } + /* Vendor-defined Data */ + if (vendor_data != NULL) { + build_append_namestring(var->buf, "%s", vendor_data); + build_append_byte(var->buf, '\0'); + } + + return var; +} + +/* * 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 1b632dc..4e88882 100644 --- a/include/hw/acpi/aml-build.h +++ b/include/hw/acpi/aml-build.h @@ -148,6 +148,26 @@ typedef enum { AML_SHARED_AND_WAKE = 3, } AmlShared; +/* + * ACPI 5.0: Table 6-189 GPIO Connection Descriptor Definition + * GPIO Connection Type + */ +typedef enum { + AML_INTERRUPT_CONNECTION = 0, + AML_IO_CONNECTION = 1, +} AmlGpioConnectionType; + +/* + * 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;