On Fri, Feb 07, 2014 at 01:51:31PM +0100, Igor Mammedov wrote: > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
The reason I avoided doing this is that this conflicts with qemu coding style which only uses camel case for types. So as a minimum this needs a comment explaining that we are using the names from ACPI spec as-is, that's why we deviate from the coding style, to simplify matching against that. Something like below: > --- > hw/i386/acpi-build.c | 28 ++++++++++++++++++---------- > 1 files changed, 18 insertions(+), 10 deletions(-) > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 6a43a7d..1dbe5ce 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -224,6 +224,14 @@ static void acpi_get_pci_info(PcPciInfo *info) > #define ACPI_BUILD_TABLE_FILE "etc/acpi/tables" > #define ACPI_BUILD_RSDP_FILE "etc/acpi/rsdp" /* Constants from ACPI spec 5.0a: * ACPI Machine Language (AML) Specification */ We probably should add in spec link as well. > +#define BytePrefix 0x0A > +#define WordPrefix 0x0B > +#define DWordPrefix 0x0C Not sure about these ones. There's a single user, and naming is different from rest of operators which makes it a bit confusing. Maybe define near the user? > + > +#define NameOp 0x08 > +#define ScopeOp 0x10 > +#define DeviceOp 0x82 Hmm if we are doing this let's do this for all Ops. > + > static void > build_header(GArray *linker, GArray *table_data, > AcpiTableHeader *h, uint32_t sig, int len, uint8_t rev) > @@ -364,13 +372,13 @@ static void build_append_value(GArray *table, uint32_t > value, int size) > > switch (size) { > case 1: > - prefix = 0x0A; /* BytePrefix */ > + prefix = BytePrefix; > break; > case 2: > - prefix = 0x0B; /* WordPrefix */ > + prefix = WordPrefix; > break; > case 4: > - prefix = 0x0C; /* DWordPrefix */ > + prefix = DWordPrefix; > break; > default: > assert(0); > @@ -762,24 +770,24 @@ static void build_pci_bus_end(PCIBus *bus, void > *bus_state) > bool bus_hotplug_support = false; > > if (bus->parent_dev) { > - op = 0x82; /* DeviceOp */ > + op = DeviceOp; > build_append_nameseg(bus_table, "S%.02X_", > bus->parent_dev->devfn); > - build_append_byte(bus_table, 0x08); /* NameOp */ > + build_append_byte(bus_table, NameOp); > build_append_nameseg(bus_table, "_SUN"); > build_append_value(bus_table, PCI_SLOT(bus->parent_dev->devfn), 1); > - build_append_byte(bus_table, 0x08); /* NameOp */ > + build_append_byte(bus_table, NameOp); > build_append_nameseg(bus_table, "_ADR"); > build_append_value(bus_table, (PCI_SLOT(bus->parent_dev->devfn) << > 16) | > PCI_FUNC(bus->parent_dev->devfn), 4); > } else { > - op = 0x10; /* ScopeOp */; > + op = ScopeOp; > build_append_nameseg(bus_table, "PCI0"); > } > > bsel = object_property_get_qobject(OBJECT(bus), ACPI_PCIHP_PROP_BSEL, > NULL); > if (bsel) { > - build_append_byte(bus_table, 0x08); /* NameOp */ > + build_append_byte(bus_table, NameOp); > build_append_nameseg(bus_table, "BSEL"); > build_append_int(bus_table, qint_get_int(qobject_to_qint(bsel))); > } > @@ -962,7 +970,7 @@ build_ssdt(GArray *table_data, GArray *linker, > > { > GArray *sb_scope = build_alloc_array(); > - uint8_t op = 0x10; /* ScopeOp */ > + uint8_t op = ScopeOp; > > build_append_nameseg(sb_scope, "_SB_"); > > @@ -983,7 +991,7 @@ build_ssdt(GArray *table_data, GArray *linker, > build_append_notify_method(sb_scope, "NTFY", "CP%0.02X", acpi_cpus); > > /* build "Name(CPON, Package() { One, One, ..., Zero, Zero, ... })" > */ > - build_append_byte(sb_scope, 0x08); /* NameOp */ > + build_append_byte(sb_scope, NameOp); > build_append_nameseg(sb_scope, "CPON"); > > { > -- > 1.7.1