[PATCH v2 09/12] acpi: move aml builder code for parallel device

Gerd Hoffmann posted 12 patches 5 years, 10 months ago
Maintainers: Richard Henderson <rth@twiddle.net>, Eduardo Habkost <ehabkost@redhat.com>, Igor Mammedov <imammedo@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Max Reitz <mreitz@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Kevin Wolf <kwolf@redhat.com>, John Snow <jsnow@redhat.com>
There is a newer version of this series
[PATCH v2 09/12] acpi: move aml builder code for parallel device
Posted by Gerd Hoffmann 5 years, 10 months ago
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/char/parallel.c   | 25 +++++++++++++++++++++++++
 hw/i386/acpi-build.c | 23 -----------------------
 2 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/hw/char/parallel.c b/hw/char/parallel.c
index 8dd67d13759b..2bff1f17fda7 100644
--- a/hw/char/parallel.c
+++ b/hw/char/parallel.c
@@ -28,6 +28,7 @@
 #include "qemu/module.h"
 #include "chardev/char-parallel.h"
 #include "chardev/char-fe.h"
+#include "hw/acpi/aml-build.h"
 #include "hw/irq.h"
 #include "hw/isa/isa.h"
 #include "hw/qdev-properties.h"
@@ -568,6 +569,28 @@ static void parallel_isa_realizefn(DeviceState *dev, Error **errp)
                              s, "parallel");
 }
 
+static void parallel_isa_build_aml(ISADevice *isadev, Aml *scope)
+{
+    ISAParallelState *isa = ISA_PARALLEL(isadev);
+    Aml *dev;
+    Aml *crs;
+
+    if (isa->iobase != 0x0378) {
+        return;
+    }
+
+    crs = aml_resource_template();
+    aml_append(crs, aml_io(AML_DECODE16, 0x0378, 0x0378, 0x08, 0x08));
+    aml_append(crs, aml_irq_no_flags(7));
+
+    dev = aml_device("LPT");
+    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0400")));
+    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
+    aml_append(dev, aml_name_decl("_CRS", crs));
+
+    aml_append(scope, dev);
+}
+
 /* Memory mapped interface */
 static uint64_t parallel_mm_readfn(void *opaque, hwaddr addr, unsigned size)
 {
@@ -624,9 +647,11 @@ static Property parallel_isa_properties[] = {
 static void parallel_isa_class_initfn(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    ISADeviceClass *isa = ISA_DEVICE_CLASS(klass);
 
     dc->realize = parallel_isa_realizefn;
     dc->vmsd = &vmstate_parallel_isa;
+    isa->build_aml = parallel_isa_build_aml;
     device_class_set_props(dc, parallel_isa_properties);
     set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
 }
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 81805bf85f8d..0539620ddff5 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1167,28 +1167,6 @@ static Aml *build_mouse_device_aml(void)
     return dev;
 }
 
-static void build_lpt_device_aml(Aml *scope)
-{
-    Aml *dev;
-    Aml *crs;
-
-    if (!memory_region_present(get_system_io(), 0x0378)) {
-        return;
-    }
-
-    dev = aml_device("LPT");
-    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0400")));
-
-    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
-
-    crs = aml_resource_template();
-    aml_append(crs, aml_io(AML_DECODE16, 0x0378, 0x0378, 0x08, 0x08));
-    aml_append(crs, aml_irq_no_flags(7));
-    aml_append(dev, aml_name_decl("_CRS", crs));
-
-    aml_append(scope, dev);
-}
-
 static void build_isa_devices_aml(Aml *table)
 {
     ISADevice *fdc = pc_find_fdc0();
@@ -1202,7 +1180,6 @@ static void build_isa_devices_aml(Aml *table)
     if (fdc) {
         aml_append(scope, build_fdc_device_aml(fdc));
     }
-    build_lpt_device_aml(scope);
 
     if (ambiguous) {
         error_report("Multiple ISA busses, unable to define IPMI ACPI data");
-- 
2.18.2


Re: [PATCH v2 09/12] acpi: move aml builder code for parallel device
Posted by Igor Mammedov 5 years, 10 months ago
On Fri,  3 Apr 2020 10:04:59 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/char/parallel.c   | 25 +++++++++++++++++++++++++
>  hw/i386/acpi-build.c | 23 -----------------------
>  2 files changed, 25 insertions(+), 23 deletions(-)
> 
> diff --git a/hw/char/parallel.c b/hw/char/parallel.c
> index 8dd67d13759b..2bff1f17fda7 100644
> --- a/hw/char/parallel.c
> +++ b/hw/char/parallel.c
> @@ -28,6 +28,7 @@
>  #include "qemu/module.h"
>  #include "chardev/char-parallel.h"
>  #include "chardev/char-fe.h"
> +#include "hw/acpi/aml-build.h"
>  #include "hw/irq.h"
>  #include "hw/isa/isa.h"
>  #include "hw/qdev-properties.h"
> @@ -568,6 +569,28 @@ static void parallel_isa_realizefn(DeviceState *dev, Error **errp)
>                               s, "parallel");
>  }
>  
> +static void parallel_isa_build_aml(ISADevice *isadev, Aml *scope)
> +{
> +    ISAParallelState *isa = ISA_PARALLEL(isadev);
> +    Aml *dev;
> +    Aml *crs;
> +
> +    if (isa->iobase != 0x0378) {
> +        return;
> +    }
> +
> +    crs = aml_resource_template();
> +    aml_append(crs, aml_io(AML_DECODE16, 0x0378, 0x0378, 0x08, 0x08));
> +    aml_append(crs, aml_irq_no_flags(7));
> +
> +    dev = aml_device("LPT");
> +    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0400")));
> +    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
> +    aml_append(dev, aml_name_decl("_CRS", crs));
> +
> +    aml_append(scope, dev);
> +}
> +
>  /* Memory mapped interface */
>  static uint64_t parallel_mm_readfn(void *opaque, hwaddr addr, unsigned size)
>  {
> @@ -624,9 +647,11 @@ static Property parallel_isa_properties[] = {
>  static void parallel_isa_class_initfn(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> +    ISADeviceClass *isa = ISA_DEVICE_CLASS(klass);
>  
>      dc->realize = parallel_isa_realizefn;
>      dc->vmsd = &vmstate_parallel_isa;
> +    isa->build_aml = parallel_isa_build_aml;
>      device_class_set_props(dc, parallel_isa_properties);
>      set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
>  }
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 81805bf85f8d..0539620ddff5 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1167,28 +1167,6 @@ static Aml *build_mouse_device_aml(void)
>      return dev;
>  }
>  
> -static void build_lpt_device_aml(Aml *scope)
> -{
> -    Aml *dev;
> -    Aml *crs;
> -
> -    if (!memory_region_present(get_system_io(), 0x0378)) {
> -        return;
> -    }
> -
> -    dev = aml_device("LPT");
> -    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0400")));
> -
> -    aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
> -
> -    crs = aml_resource_template();
> -    aml_append(crs, aml_io(AML_DECODE16, 0x0378, 0x0378, 0x08, 0x08));
> -    aml_append(crs, aml_irq_no_flags(7));
perhaps fetch values from device instead of hard-coding them

> -    aml_append(dev, aml_name_decl("_CRS", crs));
> -
> -    aml_append(scope, dev);
> -}
> -
>  static void build_isa_devices_aml(Aml *table)
>  {
>      ISADevice *fdc = pc_find_fdc0();
> @@ -1202,7 +1180,6 @@ static void build_isa_devices_aml(Aml *table)
>      if (fdc) {
>          aml_append(scope, build_fdc_device_aml(fdc));
>      }
> -    build_lpt_device_aml(scope);
>  
>      if (ambiguous) {
>          error_report("Multiple ISA busses, unable to define IPMI ACPI data");


Re: [PATCH v2 09/12] acpi: move aml builder code for parallel device
Posted by Igor Mammedov 5 years, 10 months ago
On Fri, 3 Apr 2020 12:12:10 +0200
Igor Mammedov <imammedo@redhat.com> wrote:

> On Fri,  3 Apr 2020 10:04:59 +0200
> Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
[...]
> > +static void parallel_isa_build_aml(ISADevice *isadev, Aml *scope)
> > +{
> > +    ISAParallelState *isa = ISA_PARALLEL(isadev);
> > +    Aml *dev;
> > +    Aml *crs;
> > +
> > +    if (isa->iobase != 0x0378) {
> > +        return;
> > +    }
if device is present why should we skip adding it to DSDT?

[..]


Re: [PATCH v2 09/12] acpi: move aml builder code for parallel device
Posted by Gerd Hoffmann 5 years, 10 months ago
On Fri, Apr 03, 2020 at 12:16:01PM +0200, Igor Mammedov wrote:
> On Fri, 3 Apr 2020 12:12:10 +0200
> Igor Mammedov <imammedo@redhat.com> wrote:
> 
> > On Fri,  3 Apr 2020 10:04:59 +0200
> > Gerd Hoffmann <kraxel@redhat.com> wrote:
> > 
> [...]
> > > +static void parallel_isa_build_aml(ISADevice *isadev, Aml *scope)
> > > +{
> > > +    ISAParallelState *isa = ISA_PARALLEL(isadev);
> > > +    Aml *dev;
> > > +    Aml *crs;
> > > +
> > > +    if (isa->iobase != 0x0378) {
> > > +        return;
> > > +    }
> if device is present why should we skip adding it to DSDT?

Well, that is the current state of affairs, only the first parallel
ports shows up in the dsdt.  And given how rare parallel ports are these
days I didn't bother changing that ...

We can handle this simliar to serial lines though, incremental below.
Do you prefer that?

take care,
  Gerd

=================================== cut here =======================
From 617797cf42e56e18d5d62cb171af00c28589caba Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Mon, 6 Apr 2020 12:17:59 +0200
Subject: [PATCH] [fixup] parallel

---
 hw/char/parallel.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/char/parallel.c b/hw/char/parallel.c
index 2bff1f17fda7..7157d6816b77 100644
--- a/hw/char/parallel.c
+++ b/hw/char/parallel.c
@@ -572,10 +572,16 @@ static void parallel_isa_realizefn(DeviceState *dev, Error **errp)
 static void parallel_isa_build_aml(ISADevice *isadev, Aml *scope)
 {
     ISAParallelState *isa = ISA_PARALLEL(isadev);
+    int i, uid = 0;
     Aml *dev;
     Aml *crs;
 
-    if (isa->iobase != 0x0378) {
+    for (i = 0; i < ARRAY_SIZE(isa_parallel_io); i++) {
+        if (isa->iobase == isa_parallel_io[i]) {
+            uid = i + 1;
+        }
+    }
+    if (!uid) {
         return;
     }
 
@@ -583,8 +589,9 @@ static void parallel_isa_build_aml(ISADevice *isadev, Aml *scope)
     aml_append(crs, aml_io(AML_DECODE16, 0x0378, 0x0378, 0x08, 0x08));
     aml_append(crs, aml_irq_no_flags(7));
 
-    dev = aml_device("LPT");
+    dev = aml_device("LPT%d", uid);
     aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0400")));
+    aml_append(dev, aml_name_decl("_UID", aml_int(uid)));
     aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
     aml_append(dev, aml_name_decl("_CRS", crs));
 
-- 
2.18.2


Re: [PATCH v2 09/12] acpi: move aml builder code for parallel device
Posted by Igor Mammedov 5 years, 10 months ago
On Mon, 6 Apr 2020 12:26:52 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> On Fri, Apr 03, 2020 at 12:16:01PM +0200, Igor Mammedov wrote:
> > On Fri, 3 Apr 2020 12:12:10 +0200
> > Igor Mammedov <imammedo@redhat.com> wrote:
> >   
> > > On Fri,  3 Apr 2020 10:04:59 +0200
> > > Gerd Hoffmann <kraxel@redhat.com> wrote:
> > >   
> > [...]  
> > > > +static void parallel_isa_build_aml(ISADevice *isadev, Aml *scope)
> > > > +{
> > > > +    ISAParallelState *isa = ISA_PARALLEL(isadev);
> > > > +    Aml *dev;
> > > > +    Aml *crs;
> > > > +
> > > > +    if (isa->iobase != 0x0378) {
> > > > +        return;
> > > > +    }  
> > if device is present why should we skip adding it to DSDT?  
> 
> Well, that is the current state of affairs, only the first parallel
> ports shows up in the dsdt.  And given how rare parallel ports are these
> days I didn't bother changing that ...
> 
> We can handle this simliar to serial lines though, incremental below.
> Do you prefer that?

yep (with Paolo's comment addressed)

> 
> take care,
>   Gerd
> 
> =================================== cut here =======================
> From 617797cf42e56e18d5d62cb171af00c28589caba Mon Sep 17 00:00:00 2001
> From: Gerd Hoffmann <kraxel@redhat.com>
> Date: Mon, 6 Apr 2020 12:17:59 +0200
> Subject: [PATCH] [fixup] parallel
> 
> ---
>  hw/char/parallel.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/char/parallel.c b/hw/char/parallel.c
> index 2bff1f17fda7..7157d6816b77 100644
> --- a/hw/char/parallel.c
> +++ b/hw/char/parallel.c
> @@ -572,10 +572,16 @@ static void parallel_isa_realizefn(DeviceState *dev, Error **errp)
>  static void parallel_isa_build_aml(ISADevice *isadev, Aml *scope)
>  {
>      ISAParallelState *isa = ISA_PARALLEL(isadev);
> +    int i, uid = 0;
>      Aml *dev;
>      Aml *crs;
>  
> -    if (isa->iobase != 0x0378) {
> +    for (i = 0; i < ARRAY_SIZE(isa_parallel_io); i++) {
> +        if (isa->iobase == isa_parallel_io[i]) {
> +            uid = i + 1;
> +        }
> +    }
> +    if (!uid) {
>          return;
>      }
>  
> @@ -583,8 +589,9 @@ static void parallel_isa_build_aml(ISADevice *isadev, Aml *scope)
>      aml_append(crs, aml_io(AML_DECODE16, 0x0378, 0x0378, 0x08, 0x08));
>      aml_append(crs, aml_irq_no_flags(7));
>  
> -    dev = aml_device("LPT");
> +    dev = aml_device("LPT%d", uid);
>      aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0400")));
> +    aml_append(dev, aml_name_decl("_UID", aml_int(uid)));
>      aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
>      aml_append(dev, aml_name_decl("_CRS", crs));
>  


Re: [PATCH v2 09/12] acpi: move aml builder code for parallel device
Posted by Paolo Bonzini 5 years, 10 months ago
On 06/04/20 12:26, Gerd Hoffmann wrote:
> -    if (isa->iobase != 0x0378) {
> +    for (i = 0; i < ARRAY_SIZE(isa_parallel_io); i++) {
> +        if (isa->iobase == isa_parallel_io[i]) {
> +            uid = i + 1;
> +        }
> +    }
> +    if (!uid) {
>          return;
>      }
>  
> @@ -583,8 +589,9 @@ static void parallel_isa_build_aml(ISADevice *isadev, Aml *scope)
>      aml_append(crs, aml_io(AML_DECODE16, 0x0378, 0x0378, 0x08, 0x08));

FWIW this should be replaced with iso->iobase if you want to support
multiple parallel ports (we probably should since the patch has been
written already :)).

Paolo

>      aml_append(crs, aml_irq_no_flags(7));
>  
> -    dev = aml_device("LPT");
> +    dev = aml_device("LPT%d", uid);
>      aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0400")));
> +    aml_append(dev, aml_name_decl("_UID", aml_int(uid)));
>      aml_append(dev, aml_name_decl("_STA", aml_int(0xf)));
>      aml_append(dev, aml_name_decl("_CRS", crs));
>