From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:53992) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1USPoR-0003UI-6A for qemu-devel@nongnu.org; Wed, 17 Apr 2013 06:42:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1USPoL-0005NC-7l for qemu-devel@nongnu.org; Wed, 17 Apr 2013 06:42:03 -0400 Received: from mail-ve0-f181.google.com ([209.85.128.181]:54381) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1USPoL-0005N3-17 for qemu-devel@nongnu.org; Wed, 17 Apr 2013 06:41:57 -0400 Received: by mail-ve0-f181.google.com with SMTP id pa12so1296526veb.12 for ; Wed, 17 Apr 2013 03:41:56 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1366183380-24333-2-git-send-email-lig.fnst@cn.fujitsu.com> References: <1366183380-24333-1-git-send-email-lig.fnst@cn.fujitsu.com> <1366183380-24333-2-git-send-email-lig.fnst@cn.fujitsu.com> Date: Wed, 17 Apr 2013 12:41:56 +0200 Message-ID: From: Mark Marshall Content-Type: multipart/alternative; boundary=047d7bacc5d685617704da8c22ca Subject: Re: [Qemu-devel] [SeaBIOS] [RFC][PATCH 2/2] hw: add Embedded Controller chip emulation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: liguang Cc: seabios@seabios.org, qemu-devel@nongnu.org --047d7bacc5d685617704da8c22ca Content-Type: text/plain; charset=ISO-8859-1 Hi. At least one major bug (noted below), have you tested all of this yet? MM On 17 April 2013 09:23, liguang wrote: > this work implemented Embedded Controller chip emulation > which was defined at ACPI SEPC v5 chapter 12: > "ACPI Embedded Controller Interface Specification" > > commonly Embedded Controller will emulate keyboard, > mouse, handle ACPI defined operations and some > low-speed devices like SMbus. > > Signed-off-by: liguang > --- > hw/ec.c | 113 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > hw/ec.h | 20 +++++++++++ > 2 files changed, 133 insertions(+), 0 deletions(-) > create mode 100644 hw/ec.c > create mode 100644 hw/ec.h > > diff --git a/hw/ec.c b/hw/ec.c > new file mode 100644 > index 0000000..69c92cf > --- /dev/null > +++ b/hw/ec.c > @@ -0,0 +1,113 @@ > +#include "ec.h" > +#include "hw/hw.h" > +#include "hw/isa/isa.h" > +#include "sysemu/sysemu.h" > + > +#define TYPE_EC_DEV > +#define EC_DEV(obj) \ > + OBJECT_CHECK(ECState, (obj), TYPE_EC_DEV) > + > +static char ec_acpi_space[EC_ACPI_SPACE_SIZE] = {0}; > + > +typedef struct ECState { > + ISADevice dev; > + char cmd; > + char status; > + char data; > + char irq; > + char buf; > + MemoryRegion io; > +} ECState; > + > + > +static void io62_write(void *opaque, hwaddr addr, uint64_t val, unsigned > size) > +{ > + ECState *s = opaque; > + char tmp = val & 0xff; > + > + if (s->status & EC_ACPI_CMD) { > + s->buf = tmp; > + s->status &= ~EC_ACPI_CMD; > + } else { > + if (tmp < EC_ACPI_SPACE_SIZE) { > + ec_acpi_space[s->buf] = tmp; > + } > + } > +} > + > +static uint64_t io62_read(void *opaque, hwaddr addr, unsigned size) > +{ > + return s->data; > +} > + > +static void io66_write(void *opaque, hwaddr addr, uint64_t val, unsigned > size) > +{ > + ECState *s = opaque; > + > + s->status = EC_ACPI_CMD | EC_ACPI_IBF; > + > + switch (val & 0xff) { > + case EC_ACPI_CMD_READ: > + case EC_ACPI_CMD_WRITE: > + case EC_ACPI_CMD_BURST_EN: > + s->statu |= EC_ACPI_BST; > + case EC_ACPI_CMD_BURST_DN: > + s->statu &= ~EC_ACPI_BST; > + case EC_ACPI_CMD_QUERY: > + s->cmd = val & 0xff; > + case default: > + break; > + } > +} > You've missed 'break' here a few times. > + > +static uint64_t io66_read(void *opaque, hwaddr addr, unsigned size) > +{ > + ECState *s = opaque; > + > + return s->status; > +} > + > +static const MemoryRegionOps io62_io_ops = { > + .write = io62_write, > + .read = io62_read, > + .endianness = DEVICE_NATIVE_ENDIAN, > + .impl = { > + .min_access_size = 1, > + .max_access_size = 1, > + }, > +}; > + > +static const MemoryRegionOps io66_io_ops = { > + .write = io66_write, > + .read = io66_read, > + .endianness = DEVICE_NATIVE_ENDIAN, > + .impl = { > + .min_access_size = 1, > + .max_access_size = 1, > + }, > +}; > + > +static void ec_realizefn(DeviceState *dev, Error **err) > +{ > + ISADevice *isadev = ISA_DEVICE(dev); > + ECState *s = EC_DEV(dev); > + > + isa_init_irq(isadev, &s->irq, 0xb); > + > + memory_region_init_io(&s->io, &io62_io_ops, NULL, "ec-acpi-data", 1); > + isa_register_ioport(isadev, &s->io, 0x62); > + > + memory_region_init_io(&s->io, &io66_io_ops, NULL, "ec-acpi-cmd", 1); > + isa_register_ioport(isadev, &s->io, 0x66); > + > + s->status = 0; > + s->data = 0; > +} > + > +static void ec_class_initfn(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + > + dc->realize = ec_realizefn; > + dc->no_user = 1; > +} > diff --git a/hw/ec.h b/hw/ec.h > new file mode 100644 > index 0000000..110ce04 > --- /dev/null > +++ b/hw/ec.h > @@ -0,0 +1,20 @@ > +#inndef __EC_H > +#define __EC_H > + > +#define EC_ACPI_SPACE_SIZE 0x80 > + > +#define EC_ACPI_CMD_PORT 0x66 > +#define EC_ACPI_DATA_PORT 0x62 > + > +#define EC_ACPI_OBF 0x1 > +#define EC_ACPI_IBF 0x2 > +#define EC_ACPI_CMD 0x8 > +#define EC_ACPI_BST 0x10 > + > +#define EC_ACPI_CMD_READ 0x80 > +#define EC_ACPI_CMD_WRITE 0x81 > +#define EC_ACPI_BURST_EN 0x82 > +#define EC_ACPI_BURST_DN 0x83 > +#define EC_ACPI_CMD_QUERY 0x84 > + > +#endif > -- > 1.7.2.5 > > > _______________________________________________ > SeaBIOS mailing list > SeaBIOS@seabios.org > http://www.seabios.org/mailman/listinfo/seabios > --047d7bacc5d685617704da8c22ca Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable
Hi.

At least one major bug (noted b= elow), have you tested all of this yet?

MM
<= div class=3D"gmail_extra">

On 17 April 20= 13 09:23, liguang <lig.fnst@cn.fujitsu.com> wrote:
this work implemented Embedded Controller ch= ip emulation
which was defined at ACPI SEPC v5 chapter 12:
"ACPI Embedded Controller Interface Specification"

commonly Embedded Controller will emulate keyboard,
mouse, handle ACPI defined operations and some
low-speed devices like SMbus.

Signed-off-by: liguang <lig.f= nst@cn.fujitsu.com>
---
=A0hw/ec.c | =A0113 +++++++++++++++++++++++++++++++++++++++++++++++++++++++= ++++++++
=A0hw/ec.h | =A0 20 +++++++++++
=A02 files changed, 133 insertions(+), 0 deletions(-)
=A0create mode 100644 hw/ec.c
=A0create mode 100644 hw/ec.h

diff --git a/hw/ec.c b/hw/ec.c
new file mode 100644
index 0000000..69c92cf
--- /dev/null
+++ b/hw/ec.c
@@ -0,0 +1,113 @@
+#include "ec.h"
+#include "hw/hw.h"
+#include "hw/isa/isa.h"
+#include "sysemu/sysemu.h"
+
+#define TYPE_EC_DEV
+#define EC_DEV(obj) \
+ =A0 =A0OBJECT_CHECK(ECState, (obj), TYPE_EC_DEV)
+
+static char ec_acpi_space[EC_ACPI_SPACE_SIZE] =3D {0};
+
+typedef struct ECState {
+ =A0 =A0ISADevice dev;
+ =A0 =A0char cmd;
+ =A0 =A0char status;
+ =A0 =A0char data;
+ =A0 =A0char irq;
+ =A0 =A0char buf;
+ =A0 =A0MemoryRegion io;
+} ECState;
+
+
+static void io62_write(void *opaque, hwaddr addr, uint64_t val, unsigned s= ize)
+{
+ =A0 =A0ECState *s =3D opaque;
+ =A0 =A0char tmp =3D val & 0xff;
+
+ =A0 =A0if (s->status & EC_ACPI_CMD) {
+ =A0 =A0 =A0 =A0s->buf =3D tmp;
+ =A0 =A0 =A0 =A0s->status &=3D ~EC_ACPI_CMD;
+ =A0 =A0} else {
+ =A0 =A0 =A0 =A0if (tmp < EC_ACPI_SPACE_SIZE) {
+ =A0 =A0 =A0 =A0 =A0 =A0ec_acpi_space[s->buf] =3D tmp;
+ =A0 =A0 =A0 =A0}
+ =A0 =A0}
+}
+
+static uint64_t io62_read(void *opaque, hwaddr addr, unsigned size)
+{
+ =A0 =A0return s->data;
+}
+
+static void io66_write(void *opaque, hwaddr addr, uint64_t val, unsigned s= ize)
+{
+ =A0 =A0ECState *s =3D opaque;
+
+ =A0 =A0s->status =3D EC_ACPI_CMD | EC_ACPI_IBF;
+
+ =A0 =A0switch (val & 0xff) {
+ =A0 =A0case EC_ACPI_CMD_READ:
+ =A0 =A0case EC_ACPI_CMD_WRITE:
+ =A0 =A0case EC_ACPI_CMD_BURST_EN:
+ =A0 =A0 =A0 =A0s->statu |=3D EC_ACPI_BST;
+ =A0 =A0case EC_ACPI_CMD_BURST_DN:
+ =A0 =A0 =A0 =A0s->statu &=3D ~EC_ACPI_BST;
+ =A0 =A0case EC_ACPI_CMD_QUERY:
+ =A0 =A0 =A0 =A0s->cmd =3D val & 0xff;
+ =A0 =A0case default:
+ =A0 =A0 =A0 =A0break;
+ =A0 =A0}
+}

You've missed 'break' here a few ti= mes.
=A0
+
+static uint64_t io66_read(void *opaque, hwaddr addr, unsigned size)
+{
+ =A0 =A0ECState *s =3D opaque;
+
+ =A0 =A0return s->status;
+}
+
+static const MemoryRegionOps io62_io_ops =3D {
+ =A0 =A0.write =3D io62_write,
+ =A0 =A0.read =3D io62_read,
+ =A0 =A0.endianness =3D DEVICE_NATIVE_ENDIAN,
+ =A0 =A0.impl =3D {
+ =A0 =A0 =A0 =A0.min_access_size =3D 1,
+ =A0 =A0 =A0 =A0.max_access_size =3D 1,
+ =A0 =A0},
+};
+
+static const MemoryRegionOps io66_io_ops =3D {
+ =A0 =A0.write =3D io66_write,
+ =A0 =A0.read =3D io66_read,
+ =A0 =A0.endianness =3D DEVICE_NATIVE_ENDIAN,
+ =A0 =A0.impl =3D {
+ =A0 =A0 =A0 =A0.min_access_size =3D 1,
+ =A0 =A0 =A0 =A0.max_access_size =3D 1,
+ =A0 =A0},
+};
+
+static void ec_realizefn(DeviceState *dev, Error **err)
+{
+ =A0 =A0ISADevice *isadev =3D ISA_DEVICE(dev);
+ =A0 =A0ECState *s =3D EC_DEV(dev);
+
+ =A0 =A0isa_init_irq(isadev, &s->irq, 0xb);
+
+ =A0 =A0memory_region_init_io(&s->io, &io62_io_ops, NULL, "= ;ec-acpi-data", 1);
+ =A0 =A0isa_register_ioport(isadev, &s->io, 0x62);
+
+ =A0 =A0memory_region_init_io(&s->io, &io66_io_ops, NULL, "= ;ec-acpi-cmd", 1);
+ =A0 =A0isa_register_ioport(isadev, &s->io, 0x66);
+
+ =A0 =A0s->status =3D 0;
+ =A0 =A0s->data =3D 0;
+}
+
+static void ec_class_initfn(ObjectClass *klass, void *data)
+{
+ =A0 =A0DeviceClass *dc =3D DEVICE_CLASS(klass);
+
+ =A0 =A0dc->realize =3D ec_realizefn;
+ =A0 =A0dc->no_user =3D 1;
+}
diff --git a/hw/ec.h b/hw/ec.h
new file mode 100644
index 0000000..110ce04
--- /dev/null
+++ b/hw/ec.h
@@ -0,0 +1,20 @@
+#inndef __EC_H
+#define __EC_H
+
+#define EC_ACPI_SPACE_SIZE 0x80
+
+#define EC_ACPI_CMD_PORT 0x66
+#define EC_ACPI_DATA_PORT 0x62
+
+#define EC_ACPI_OBF 0x1
+#define EC_ACPI_IBF 0x2
+#define EC_ACPI_CMD 0x8
+#define EC_ACPI_BST 0x10
+
+#define EC_ACPI_CMD_READ 0x80
+#define EC_ACPI_CMD_WRITE 0x81
+#define EC_ACPI_BURST_EN 0x82
+#define EC_ACPI_BURST_DN 0x83
+#define EC_ACPI_CMD_QUERY 0x84
+
+#endif
--
1.7.2.5


_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

--047d7bacc5d685617704da8c22ca--