Message ID | 1449704528-289297-27-git-send-email-imammedo@redhat.com |
---|---|
State | New |
Headers | show |
On 12/10/2015 01:41 AM, Igor Mammedov wrote: > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > hw/acpi/memory_hotplug_acpi_table.c | 12 ++++++++++++ > hw/i386/acpi-dsdt-mem-hotplug.dsl | 8 -------- > 2 files changed, 12 insertions(+), 8 deletions(-) > > diff --git a/hw/acpi/memory_hotplug_acpi_table.c b/hw/acpi/memory_hotplug_acpi_table.c > index 25bbf5e..de51717 100644 > --- a/hw/acpi/memory_hotplug_acpi_table.c > +++ b/hw/acpi/memory_hotplug_acpi_table.c > @@ -29,11 +29,23 @@ void build_memory_hotplug_aml(Aml *ctx, uint32_t nr_mem, > { > Aml *pci_scope; > Aml *ctrl_dev; > + Aml *method; > + Aml *ifctx; > + Aml *a_zero = aml_int(0); > + Aml *a_slots_nr = aml_name(stringify(MEMORY_SLOTS_NUMBER)); > > /* scope for memory hotplug controller device node */ > pci_scope = aml_scope("_SB.PCI0"); > ctrl_dev = aml_scope(stringify(MEMORY_HOTPLUG_DEVICE)); > { > + /* MHPD._STA() method */ > + method = aml_method("_STA", 0, AML_NOTSERIALIZED); > + ifctx = aml_if(aml_equal(a_slots_nr, a_zero)); > + aml_append(ifctx, aml_return(a_zero)); Hi Igor, The indentation is a little off here. I understand *why* you are doing it, to emphasize that is an if statement. However this is not clear I think. What I would do is use the { } parentheses for all the method: method = aml_method("_STA", 0, AML_NOTSERIALIZED); { ifctx = aml_if(aml_equal(a_slots_nr, a_zero)) { aml_append(ifctx, aml_return(a_zero)); } aml_append(method, ifctx); /* present, functioning, decoding, not shown in UI */ aml_append(method, aml_return(aml_int(0xB))); } aml_append(ctrl_dev, method); As you can see the, the _STA method is looking good both as C code and is also easy to see the asl (compare with the original asl). This is of course only a suggestion, me trying to help :) I see no problem in the conversion itself. Thanks, Marcel > + aml_append(method, ifctx); > + /* present, functioning, decoding, not shown in UI */ > + aml_append(method, aml_return(aml_int(0xB))); > + aml_append(ctrl_dev, method); > } > aml_append(pci_scope, ctrl_dev); > aml_append(ctx, pci_scope); > diff --git a/hw/i386/acpi-dsdt-mem-hotplug.dsl b/hw/i386/acpi-dsdt-mem-hotplug.dsl > index c2bb6a1..b4eacc9 100644 > --- a/hw/i386/acpi-dsdt-mem-hotplug.dsl > +++ b/hw/i386/acpi-dsdt-mem-hotplug.dsl > @@ -35,14 +35,6 @@ > External(MEMORY_SLOT_OST_EVENT, FieldUnitObj) // _OST event code, write only > External(MEMORY_SLOT_OST_STATUS, FieldU835163 nitObj) // _OST status code, write only > > - Method(_STA, 0) { > - If (LEqual(MEMORY_SLOTS_NUMBER, Zero)) { > - Return(0x0) > - } > - /* present, functioning, decoding, not shown in UI */ > - Return(0xB) > - } > - > Mutex (MEMORY_SLOT_LOCK, 0) > > Method(MEMORY_SLOT_SCAN_METHOD, 0) { >
On Wed, 16 Dec 2015 14:08:17 +0200 Marcel Apfelbaum <marcel.apfelbaum@gmail.com> wrote: > On 12/10/2015 01:41 AM, Igor Mammedov wrote: > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > hw/acpi/memory_hotplug_acpi_table.c | 12 ++++++++++++ > > hw/i386/acpi-dsdt-mem-hotplug.dsl | 8 -------- > > 2 files changed, 12 insertions(+), 8 deletions(-) > > > > diff --git a/hw/acpi/memory_hotplug_acpi_table.c > > b/hw/acpi/memory_hotplug_acpi_table.c index 25bbf5e..de51717 100644 > > --- a/hw/acpi/memory_hotplug_acpi_table.c > > +++ b/hw/acpi/memory_hotplug_acpi_table.c > > @@ -29,11 +29,23 @@ void build_memory_hotplug_aml(Aml *ctx, > > uint32_t nr_mem, { > > Aml *pci_scope; > > Aml *ctrl_dev; > > + Aml *method; > > + Aml *ifctx; > > + Aml *a_zero = aml_int(0); > > + Aml *a_slots_nr = aml_name(stringify(MEMORY_SLOTS_NUMBER)); > > > > /* scope for memory hotplug controller device node */ > > pci_scope = aml_scope("_SB.PCI0"); > > ctrl_dev = aml_scope(stringify(MEMORY_HOTPLUG_DEVICE)); > > { > > + /* MHPD._STA() method */ > > + method = aml_method("_STA", 0, AML_NOTSERIALIZED); > > + ifctx = aml_if(aml_equal(a_slots_nr, a_zero)); > > + aml_append(ifctx, aml_return(a_zero)); > > Hi Igor, > > The indentation is a little off here. I understand *why* > you are doing it, to emphasize that is an if statement. > However this is not clear I think. > > What I would do is use the { } parentheses for all the method: > > method = aml_method("_STA", 0, AML_NOTSERIALIZED); > { > ifctx = aml_if(aml_equal(a_slots_nr, a_zero)) > { > aml_append(ifctx, aml_return(a_zero)); > } > aml_append(method, ifctx); > /* present, functioning, decoding, not shown in UI */ > aml_append(method, aml_return(aml_int(0xB))); > } > aml_append(ctrl_dev, method); > > > As you can see the, the _STA method is looking good > both as C code and is also easy to see the asl (compare with the > original asl). sure, I'll add { } parentheses around if body as you suggest. > > > This is of course only a suggestion, me trying to help :) > I see no problem in the conversion itself. > > Thanks, > Marcel > > > + aml_append(method, ifctx); > > + /* present, functioning, decoding, not shown in UI */ > > + aml_append(method, aml_return(aml_int(0xB))); > > + aml_append(ctrl_dev, method); > > } > > aml_append(pci_scope, ctrl_dev); > > aml_append(ctx, pci_scope); > > diff --git a/hw/i386/acpi-dsdt-mem-hotplug.dsl > > b/hw/i386/acpi-dsdt-mem-hotplug.dsl index c2bb6a1..b4eacc9 100644 > > --- a/hw/i386/acpi-dsdt-mem-hotplug.dsl > > +++ b/hw/i386/acpi-dsdt-mem-hotplug.dsl > > @@ -35,14 +35,6 @@ > > External(MEMORY_SLOT_OST_EVENT, FieldUnitObj) // _OST > > event code, write only External(MEMORY_SLOT_OST_STATUS, FieldU835163 > > nitObj) // _OST status code, write only > > > > - Method(_STA, 0) { > > - If (LEqual(MEMORY_SLOTS_NUMBER, Zero)) { > > - Return(0x0) > > - } > > - /* present, functioning, decoding, not shown in UI > > */ > > - Return(0xB) > > - } > > - > > Mutex (MEMORY_SLOT_LOCK, 0) > > > > Method(MEMORY_SLOT_SCAN_METHOD, 0) { > > >
diff --git a/hw/acpi/memory_hotplug_acpi_table.c b/hw/acpi/memory_hotplug_acpi_table.c index 25bbf5e..de51717 100644 --- a/hw/acpi/memory_hotplug_acpi_table.c +++ b/hw/acpi/memory_hotplug_acpi_table.c @@ -29,11 +29,23 @@ void build_memory_hotplug_aml(Aml *ctx, uint32_t nr_mem, { Aml *pci_scope; Aml *ctrl_dev; + Aml *method; + Aml *ifctx; + Aml *a_zero = aml_int(0); + Aml *a_slots_nr = aml_name(stringify(MEMORY_SLOTS_NUMBER)); /* scope for memory hotplug controller device node */ pci_scope = aml_scope("_SB.PCI0"); ctrl_dev = aml_scope(stringify(MEMORY_HOTPLUG_DEVICE)); { + /* MHPD._STA() method */ + method = aml_method("_STA", 0, AML_NOTSERIALIZED); + ifctx = aml_if(aml_equal(a_slots_nr, a_zero)); + aml_append(ifctx, aml_return(a_zero)); + aml_append(method, ifctx); + /* present, functioning, decoding, not shown in UI */ + aml_append(method, aml_return(aml_int(0xB))); + aml_append(ctrl_dev, method); } aml_append(pci_scope, ctrl_dev); aml_append(ctx, pci_scope); diff --git a/hw/i386/acpi-dsdt-mem-hotplug.dsl b/hw/i386/acpi-dsdt-mem-hotplug.dsl index c2bb6a1..b4eacc9 100644 --- a/hw/i386/acpi-dsdt-mem-hotplug.dsl +++ b/hw/i386/acpi-dsdt-mem-hotplug.dsl @@ -35,14 +35,6 @@ External(MEMORY_SLOT_OST_EVENT, FieldUnitObj) // _OST event code, write only External(MEMORY_SLOT_OST_STATUS, FieldUnitObj) // _OST status code, write only - Method(_STA, 0) { - If (LEqual(MEMORY_SLOTS_NUMBER, Zero)) { - Return(0x0) - } - /* present, functioning, decoding, not shown in UI */ - Return(0xB) - } - Mutex (MEMORY_SLOT_LOCK, 0) Method(MEMORY_SLOT_SCAN_METHOD, 0) {
Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- hw/acpi/memory_hotplug_acpi_table.c | 12 ++++++++++++ hw/i386/acpi-dsdt-mem-hotplug.dsl | 8 -------- 2 files changed, 12 insertions(+), 8 deletions(-)