[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 7/9] acpi: replace opencoded notify codes with n
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCH 7/9] acpi: replace opencoded notify codes with named values |
Date: |
Tue, 9 Dec 2014 11:59:27 +0100 |
On Mon, 8 Dec 2014 22:54:03 +0200
"Michael S. Tsirkin" <address@hidden> wrote:
> On Mon, Dec 08, 2014 at 04:08:06PM +0000, Igor Mammedov wrote:
> > Signed-off-by: Igor Mammedov <address@hidden>
>
> I'm not sure this makes sense for these constants.
>
> Device Check seems more readable than ACPI_DEV_CHK.
> If your object here is readability, please do not
> abbreviate.
>
> Generally ability to match spec names exactly is
> what made me prefer code comments to enums for one-off constants.
> Looking up "Device Check" works in any spec version,
> I don't have to dig up the exact one, find table by name,
> break my eyes trying to locate the correct line in
> a huge table.
> Just text search, and the correct line is highlighted.
>
> Why look in another spec version you might ask?
> Well it's definitely helpful to understand legacy
> guest quirks.
>
>
sure, lets drop it.
> > ---
> > hw/i386/acpi-build.c | 4 ++--
> > include/hw/acpi/acpi_gen_utils.h | 6 ++++++
> > 2 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 0f6202d..a33d130 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -840,10 +840,10 @@ static void build_pci_bus_end(PCIBus *bus, void
> > *bus_state)
> > build_append_namestring(method, "BNUM");
> > build_append_namestring(method, "DVNT");
> > build_append_namestring(method, "PCIU");
> > - build_append_int(method, 1); /* Device Check */
> > + build_append_int(method, ACPI_DEV_CHK);
> > build_append_namestring(method, "DVNT");
> > build_append_namestring(method, "PCID");
> > - build_append_int(method, 3); /* Eject Request */
> > + build_append_int(method, ACPI_DEV_EJ);
> > }
> >
> > /* Notify about child bus events in any case */
> > diff --git a/include/hw/acpi/acpi_gen_utils.h
> > b/include/hw/acpi/acpi_gen_utils.h
> > index fd50625..ce76dc1 100644
> > --- a/include/hw/acpi/acpi_gen_utils.h
> > +++ b/include/hw/acpi/acpi_gen_utils.h
> > @@ -5,6 +5,12 @@
> > #include <glib.h>
> > #include "qemu/compiler.h"
> >
> > +/* ACPI 5.0: table "Device Object Notification Values" */
> > +enum {
> > + ACPI_DEV_CHK = 1,
> > + ACPI_DEV_EJ = 3,
> > +};
> > +
> > GArray *build_alloc_array(void);
> > void build_free_array(GArray *array);
> > void build_prepend_byte(GArray *array, uint8_t val);
> > --
> > 1.8.3.1
>
- Re: [Qemu-devel] [PATCH 1/9] pc: acpi: fix WindowsXP BSOD when memory hotplug is enabled, (continued)
[Qemu-devel] [PATCH 8/9] acpi: drop min-bytes in build_package(), Igor Mammedov, 2014/12/08
[Qemu-devel] [PATCH 9/9] pc: acpi-build: replace recursive PCI bus tree generation with loop based, Igor Mammedov, 2014/12/08
[Qemu-devel] [PATCH 7/9] acpi: replace opencoded notify codes with named values, Igor Mammedov, 2014/12/08
[Qemu-devel] [PATCH 6/9] acpi: add build_append_namestring() helper, Igor Mammedov, 2014/12/08
Re: [Qemu-devel] [PATCH 0/9] pc: acpi: various fixes and cleanups, Michael S. Tsirkin, 2014/12/08