[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 50/74] pc: acpi: move FDC0 device from DSDT to S
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH 50/74] pc: acpi: move FDC0 device from DSDT to SSDT |
Date: |
Sat, 19 Dec 2015 21:44:55 +0200 |
On Thu, Dec 10, 2015 at 12:41:44AM +0100, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <address@hidden>
> ---
> hw/i386/acpi-build.c | 38 ++++++++++++++++++++++++++++++++------
> hw/i386/acpi-dsdt-isa.dsl | 18 ------------------
> 2 files changed, 32 insertions(+), 24 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index e5ec6af..2f0f2e1 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1173,17 +1173,34 @@ static void build_hpet_aml(Aml *table)
>
> static Aml *
> build_eisa_device_aml(const char *name, const char *hid, Aml *crs,
> - bool have_sta)
> + bool have_sta, const char *present_field)
At this point, the number of arguments is getting ridiculous.
I think you should give it up, and write a function
per device type, even if there's a minor code duplication
as a result.
As a bonus, we will get reasonably readable function
names, e.g. build_floppy_controller()
> {
> Aml *dev;
> Aml *method;
> - Aml *a_device_present = aml_int(0x0f);
>
> dev = aml_device("%s", name);
> aml_append(dev, aml_name_decl("_HID", aml_eisaid(hid)));
> if (have_sta) {
> + Aml *if_ctx;
> + Aml *else_ctx;
> + Aml *a_zero = aml_int(0);
> + Aml *a_is_present = aml_local(0);
> + Aml *a_device_present = aml_int(0x0f);
> + Aml *a_device_not_present = aml_int(0x00);
> +
What does a_ prefix mean here?
> method = aml_method("_STA", 0, AML_NOTSERIALIZED);
> - aml_append(method, aml_return(a_device_present));
> + if (!present_field) {
> + aml_append(method, aml_return(a_device_present));
> + } else {
> + aml_append(method,
> + aml_store(aml_name("%s", present_field), a_is_present));
> + if_ctx = aml_if(aml_equal(a_is_present, a_zero));
> + aml_append(if_ctx, aml_return(a_device_not_present));
> + aml_append(method, if_ctx);
> + else_ctx = aml_else();
> + aml_append(else_ctx, aml_return(a_device_present));
> + aml_append(method, else_ctx);
> + }
> aml_append(dev, method);
> }
> aml_append(dev, aml_name_decl("_CRS", crs));
> @@ -1201,19 +1218,28 @@ static void build_isa_devices_aml(Aml *table)
> aml_append(crs, aml_irq_no_flags(8));
> aml_append(crs, aml_io(AML_DECODE16, 0x0072, 0x0072, 0x02, 0x06));
> aml_append(scope, build_eisa_device_aml(
> - "RTC", "PNP0B00", crs, false));
> + "RTC", "PNP0B00", crs, false, NULL));
>
> crs = aml_resource_template();
> aml_append(crs, aml_io(AML_DECODE16, 0x0060, 0x0060, 0x01, 0x01));
> aml_append(crs, aml_io(AML_DECODE16, 0x0064, 0x0064, 0x01, 0x01));
> aml_append(crs, aml_irq_no_flags(1));
> aml_append(scope, build_eisa_device_aml(
> - "KBD", "PNP0303", crs, true));
> + "KBD", "PNP0303", crs, true, NULL));
>
> crs = aml_resource_template();
> aml_append(crs, aml_irq_no_flags(12));
> aml_append(scope, build_eisa_device_aml(
> - "MOU", "PNP0F13", crs, true));
> + "MOU", "PNP0F13", crs, true, NULL));
> +
> + crs = aml_resource_template();
> + aml_append(crs, aml_io(AML_DECODE16, 0x03F2, 0x03F2, 0x00, 0x04));
> + aml_append(crs, aml_io(AML_DECODE16, 0x03F7, 0x03F7, 0x00, 0x01));
> + aml_append(crs, aml_irq_no_flags(6));
> + aml_append(crs,
> + aml_dma(AML_COMPATIBILITY, AML_NOTBUSMASTER, AML_TRANSFER8, 2));
> + aml_append(scope, build_eisa_device_aml(
> + "FDC0", "PNP0700", crs, true, "FDEN"));
>
> aml_append(table, scope);
> }
> diff --git a/hw/i386/acpi-dsdt-isa.dsl b/hw/i386/acpi-dsdt-isa.dsl
> index 8936271..64dd4ac 100644
> --- a/hw/i386/acpi-dsdt-isa.dsl
> +++ b/hw/i386/acpi-dsdt-isa.dsl
> @@ -16,24 +16,6 @@
> /* Common legacy ISA style devices. */
> Scope(\_SB.PCI0.ISA) {
>
> - Device(FDC0) {
> - Name(_HID, EisaId("PNP0700"))
> - Method(_STA, 0, NotSerialized) {
> - Store(FDEN, Local0)
> - If (LEqual(Local0, 0)) {
> - Return (0x00)
> - } Else {
> - Return (0x0F)
> - }
> - }
All these guys have in common is the _STA method.
So write a function generating one, and be done with it.
> - Name(_CRS, ResourceTemplate() {
> - IO(Decode16, 0x03F2, 0x03F2, 0x00, 0x04)
> - IO(Decode16, 0x03F7, 0x03F7, 0x00, 0x01)
> - IRQNoFlags() { 6 }
> - DMA(Compatibility, NotBusMaster, Transfer8) { 2 }
> - })
> - }
> -
> Device(LPT) {
> Name(_HID, EisaId("PNP0400"))
> Method(_STA, 0, NotSerialized) {
> --
> 1.8.3.1
>
- Re: [Qemu-devel] [PATCH 24/74] acpi: extend aml_interrupt() to support multiple irqs, (continued)
[Qemu-devel] [PATCH 32/74] pc: acpi: memhp: move MHPD.MEJ0 method into SSDT, Igor Mammedov, 2015/12/09
[Qemu-devel] [PATCH 31/74] pc: acpi: memhp: move MHPD.MOST method into SSDT, Igor Mammedov, 2015/12/09
[Qemu-devel] [PATCH 36/74] pc: acpi: memhp: move \_GPE._E03 into SSDT, Igor Mammedov, 2015/12/09
[Qemu-devel] [PATCH 37/74] pc: acpi: memhp: drop not needed stringify(MEMORY_foo) usage, Igor Mammedov, 2015/12/09
[Qemu-devel] [PATCH 39/74] pc: acpi: cpuhp: move CPEJ() method to SSDT, Igor Mammedov, 2015/12/09
[Qemu-devel] [PATCH 43/74] pc: acpi: cpuhp: move \_GPE._E02() into SSDT, Igor Mammedov, 2015/12/09
[Qemu-devel] [PATCH 50/74] pc: acpi: move FDC0 device from DSDT to SSDT, Igor Mammedov, 2015/12/09
- Re: [Qemu-devel] [PATCH 50/74] pc: acpi: move FDC0 device from DSDT to SSDT,
Michael S. Tsirkin <=
[Qemu-devel] [PATCH 55/74] pc: acpi: pci: move link devices into SSDT, Igor Mammedov, 2015/12/09
[Qemu-devel] [PATCH 42/74] pc: acpi: cpuhp: move PRSC() method into SSDT, Igor Mammedov, 2015/12/09
[Qemu-devel] [PATCH 52/74] pc: acpi: move COM devices from DSDT to SSDT, Igor Mammedov, 2015/12/09
[Qemu-devel] [PATCH 61/74] pc: acpi: q35: move GSI links to SSDT, Igor Mammedov, 2015/12/09
[Qemu-devel] [PATCH 57/74] pc: acpi: piix4: move IQST() into SSDT, Igor Mammedov, 2015/12/09
[Qemu-devel] [PATCH 65/74] pc: acpi: q35: move ISA bridge into SSDT, Igor Mammedov, 2015/12/09
[Qemu-devel] [PATCH 49/74] pc: acpi: move MOU device from DSDT to SSDT, Igor Mammedov, 2015/12/09
[Qemu-devel] [PATCH 54/74] pc: acpi: move remaining GPE handlers into SSDT, Igor Mammedov, 2015/12/09