On Fri, Feb 07, 2014 at 01:51:31PM +0100, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov
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 DWordPrefix0x0C
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 ScopeOp0x10
> +#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
