On Wed, May 11, 2016 at 02:46:04PM -0500, miny...@acm.org wrote: > From: Corey Minyard <cminy...@mvista.com> > > Signed-off-by: Corey Minyard <cminy...@mvista.com> > --- > hw/acpi/aml-build.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > include/hw/acpi/aml-build.h | 11 +++++++++++ > 2 files changed, 53 insertions(+) > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > index ab89ca6..7a3874b 100644 > --- a/hw/acpi/aml-build.c > +++ b/hw/acpi/aml-build.c > @@ -1563,3 +1563,45 @@ build_rsdt(GArray *table_data, GArray *linker, GArray > *table_offsets, > build_header(linker, table_data, > (void *)rsdt, "RSDT", rsdt_len, 1, oem_id, oem_table_id); > } > +
Pls add comment with earliest spec revision that includes this. > +static Aml *aml_serial_bus_device(AmlSerialBusType type, uint8_t flags, > + uint16_t type_flags, > + uint8_t revid, uint16_t data_length, > + uint16_t resource_source_length) > +{ > + Aml *var = aml_alloc(); > + uint16_t length = data_length + resource_source_length + 9; > + > + build_append_byte(var->buf, 0x8e); /* Serial Bus Connection Descriptor */ > + build_append_byte(var->buf, length & 0xff); > + build_append_byte(var->buf, length >> 8); > + build_append_byte(var->buf, 1); /* Revision */ > + build_append_byte(var->buf, 0); /* Resource source index */ > + build_append_byte(var->buf, type); /* Serial bus type */ > + build_append_byte(var->buf, flags); /* Serial bus type */ Fix comments to match spec verbatim please. > + build_append_byte(var->buf, type_flags & 0xff); > + build_append_byte(var->buf, type_flags >> 8); > + build_append_byte(var->buf, revid); > + build_append_byte(var->buf, data_length & 0xff); > + build_append_byte(var->buf, data_length >> 8); > + Please use build_append_int_noprefix. > + return var; > +} > + > +Aml *aml_i2c_serial_bus_device(uint8_t flags, uint32_t con_speed, > + uint16_t address, const char *resource_source) > +{ > + unsigned int resource_source_len = strlen(resource_source) + 1; > + Aml *var = aml_serial_bus_device(aml_serial_bus_type_i2c, flags, 0, 1, > + 6, resource_source_len); > + > + build_append_byte(var->buf, con_speed & 0xff); > + build_append_byte(var->buf, (con_speed >> 8) & 0xff); > + build_append_byte(var->buf, (con_speed >> 16) & 0xff); > + build_append_byte(var->buf, (con_speed >> 24) & 0xff); Please use build_append_int_noprefix. > + build_append_byte(var->buf, address & 0xff); > + build_append_byte(var->buf, address >> 8); Same. > + g_array_append_vals(var->buf, resource_source, resource_source_len); Is resource_source a name then? Then you should do aml_append(var, aml_name("%s", alias_object)); > + > + return var; > +} > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h > index 2c994b3..1eb3ebd 100644 > --- a/include/hw/acpi/aml-build.h > +++ b/include/hw/acpi/aml-build.h > @@ -206,6 +206,15 @@ struct AcpiBuildTables { > GArray *linker; > } AcpiBuildTables; > > +typedef enum { > + aml_serial_bus_type_i2c = 1, > + aml_serial_bus_type_spi = 2, > + aml_serial_bus_type_uart = 3 > +} AmlSerialBusType; > + > +#define AML_SERIAL_BUS_FLAG_MASTER_DEVICE (1 << 0) > +#define AML_SERIAL_BUS_FLAG_CONSUME_ONLY (1 << 1) > + Please drop enums and add numbers with comments matching text in ACPI spec in aml-build.c > /** > * init_aml_allocator: > * > @@ -327,6 +336,8 @@ Aml *aml_qword_memory(AmlDecode dec, AmlMinFixed > min_fixed, > Aml *aml_dma(AmlDmaType typ, AmlDmaBusMaster bm, AmlTransferSize sz, > uint8_t channel); > Aml *aml_sleep(uint64_t msec); > +Aml *aml_i2c_serial_bus_device(uint8_t flags, uint32_t con_speed, Drop these two parameters until they are actually useful. > + uint16_t address, const char > *resource_source); > > /* Block AML object primitives */ > Aml *aml_scope(const char *name_format, ...) GCC_FMT_ATTR(1, 2); > -- > 2.7.4