[Qemu-devel] [PATCH] isa: configure serial+parallel by index.

[Qemu-devel] [PATCH] isa: configure serial+parallel by index.

[Gerd Hoffmann]

This patch adds a 'index' property to the isa-parallel and isa-serial
devices.  This can be used to create devices with the default isa irqs
and ioports by simply specifying the index, i.e.

   -device isa-serial,index=1

instead of

   -device isa-serial,iobase=0x2f8,irq=3

for ttyS1 aka com2.  Likewise for parallel ports.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/parallel.c |   23 +++++++++++++++++------
 hw/serial.c   |   26 +++++++++++++++++++-------
 2 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/hw/parallel.c b/hw/parallel.c
index 92eecb1..79fa8f6 100644
--- a/hw/parallel.c
+++ b/hw/parallel.c
@@ -80,6 +80,7 @@ struct ParallelState {
 
 typedef struct ISAParallelState {
     ISADevice dev;
+    uint32_t index;
     uint32_t iobase;
     uint32_t isairq;
     ParallelState state;
@@ -445,11 +446,14 @@ static void parallel_reset(void *opaque)
     s->last_read_offset = ~0U;
 }
 
+static const int isa_parallel_io[MAX_PARALLEL_PORTS] = { 0x378, 0x278, 0x3bc };
+
 static int parallel_isa_initfn(ISADevice *dev)
 {
+    static int index;
     ISAParallelState *isa = DO_UPCAST(ISAParallelState, dev, dev);
     ParallelState *s = &isa->state;
-    int base = isa->iobase;
+    int base;
     uint8_t dummy;
 
     if (!s->chr) {
@@ -457,6 +461,15 @@ static int parallel_isa_initfn(ISADevice *dev)
         exit(1);
     }
 
+    if (isa->index == -1)
+        isa->index = index;
+    if (isa->index >= MAX_PARALLEL_PORTS)
+        return -1;
+    if (isa->iobase == -1)
+        isa->iobase = isa_parallel_io[isa->index];
+    index++;
+
+    base = isa->iobase;
     isa_init_irq(dev, &s->irq, isa->isairq);
     parallel_reset(s);
     qemu_register_reset(parallel_reset, s);
@@ -483,15 +496,12 @@ static int parallel_isa_initfn(ISADevice *dev)
     return 0;
 }
 
-static const int isa_parallel_io[MAX_PARALLEL_PORTS] = { 0x378, 0x278, 0x3bc };
-
 ParallelState *parallel_init(int index, CharDriverState *chr)
 {
     ISADevice *dev;
 
     dev = isa_create("isa-parallel");
-    qdev_prop_set_uint32(&dev->qdev, "iobase", isa_parallel_io[index]);
-    qdev_prop_set_uint32(&dev->qdev, "irq", 7);
+    qdev_prop_set_uint32(&dev->qdev, "index", index);
     qdev_prop_set_chr(&dev->qdev, "chardev", chr);
     if (qdev_init(&dev->qdev) < 0)
         return NULL;
@@ -579,7 +589,8 @@ static ISADeviceInfo parallel_isa_info = {
     .qdev.size  = sizeof(ISAParallelState),
     .init       = parallel_isa_initfn,
     .qdev.props = (Property[]) {
-        DEFINE_PROP_HEX32("iobase", ISAParallelState, iobase,  0x378),
+        DEFINE_PROP_HEX32("index",  ISAParallelState, index,   -1),
+        DEFINE_PROP_HEX32("iobase", ISAParallelState, iobase,  -1),
         DEFINE_PROP_UINT32("irq",   ISAParallelState, isairq,  7),
         DEFINE_PROP_CHR("chardev",  ISAParallelState, state.chr),
         DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/serial.c b/hw/serial.c
index eb14f11..869063c 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -148,6 +148,7 @@ struct SerialState {
 
 typedef struct ISASerialState {
     ISADevice dev;
+    uint32_t index;
     uint32_t iobase;
     uint32_t isairq;
     SerialState state;
@@ -733,11 +734,25 @@ static void serial_init_core(SerialState *s)
                           serial_event, s);
 }
 
+static const int isa_serial_io[MAX_SERIAL_PORTS] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 };
+static const int isa_serial_irq[MAX_SERIAL_PORTS] = { 4, 3, 4, 3 };
+
 static int serial_isa_initfn(ISADevice *dev)
 {
+    static int index;
     ISASerialState *isa = DO_UPCAST(ISASerialState, dev, dev);
     SerialState *s = &isa->state;
 
+    if (isa->index == -1)
+        isa->index = index;
+    if (isa->index >= MAX_SERIAL_PORTS)
+        return -1;
+    if (isa->iobase == -1)
+        isa->iobase = isa_serial_io[isa->index];
+    if (isa->isairq == -1)
+        isa->isairq = isa_serial_irq[isa->index];
+    index++;
+
     s->baudbase = 115200;
     isa_init_irq(dev, &s->irq, isa->isairq);
     serial_init_core(s);
@@ -748,16 +763,12 @@ static int serial_isa_initfn(ISADevice *dev)
     return 0;
 }
 
-static const int isa_serial_io[MAX_SERIAL_PORTS] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 };
-static const int isa_serial_irq[MAX_SERIAL_PORTS] = { 4, 3, 4, 3 };
-
 SerialState *serial_isa_init(int index, CharDriverState *chr)
 {
     ISADevice *dev;
 
     dev = isa_create("isa-serial");
-    qdev_prop_set_uint32(&dev->qdev, "iobase", isa_serial_io[index]);
-    qdev_prop_set_uint32(&dev->qdev, "irq", isa_serial_irq[index]);
+    qdev_prop_set_uint32(&dev->qdev, "index", index);
     qdev_prop_set_chr(&dev->qdev, "chardev", chr);
     if (qdev_init(&dev->qdev) < 0)
         return NULL;
@@ -886,8 +897,9 @@ static ISADeviceInfo serial_isa_info = {
     .qdev.size  = sizeof(ISASerialState),
     .init       = serial_isa_initfn,
     .qdev.props = (Property[]) {
-        DEFINE_PROP_HEX32("iobase", ISASerialState, iobase,  0x3f8),
-        DEFINE_PROP_UINT32("irq",   ISASerialState, isairq,  4),
+        DEFINE_PROP_HEX32("index",  ISASerialState, index,   -1),
+        DEFINE_PROP_HEX32("iobase", ISASerialState, iobase,  -1),
+        DEFINE_PROP_UINT32("irq",   ISASerialState, isairq,  -1),
         DEFINE_PROP_CHR("chardev",  ISASerialState, state.chr),
         DEFINE_PROP_END_OF_LIST(),
     },
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH] isa: configure serial+parallel by index.

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

> This patch adds a 'index' property to the isa-parallel and isa-serial
> devices.  This can be used to create devices with the default isa irqs
> and ioports by simply specifying the index, i.e.
>
>    -device isa-serial,index=1
>
> instead of
>
>    -device isa-serial,iobase=0x2f8,irq=3
>
> for ttyS1 aka com2.  Likewise for parallel ports.

Not mentioned here, only in the code:

* index defaults to 0 for the first device to initialize, 1 for the
  second, and so forth.

* It is okay to overwrite the defaults provided by index, e.g.

    -device isa-serial,index=1,irq=4

Looks fine to me.  A similar solution could do for default mac address.

Re: [Qemu-devel] [PATCH] isa: configure serial+parallel by index.

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> Gerd Hoffmann <kraxel@redhat.com> writes:
>
>> This patch adds a 'index' property to the isa-parallel and isa-serial
>> devices.  This can be used to create devices with the default isa irqs
>> and ioports by simply specifying the index, i.e.
>>
>>    -device isa-serial,index=1
>>
>> instead of
>>
>>    -device isa-serial,iobase=0x2f8,irq=3
>>
>> for ttyS1 aka com2.  Likewise for parallel ports.
>
> Not mentioned here, only in the code:
>
> * index defaults to 0 for the first device to initialize, 1 for the
>   second, and so forth.
>
> * It is okay to overwrite the defaults provided by index, e.g.
>
>     -device isa-serial,index=1,irq=4
>
> Looks fine to me.  A similar solution could do for default mac address.

One little thing I missed on first reading: making an index property
hexadecimal is weird.  I'd really expect decimal there.

Re: [Qemu-devel] [PATCH] isa: configure serial+parallel by index.

[Gerd Hoffmann]

>> Looks fine to me.  A similar solution could do for default mac address.
>
> One little thing I missed on first reading: making an index property
> hexadecimal is weird.  I'd really expect decimal there.

Indeed.  Wasn't intentional, cut+paste bug.  Will fix.

cheers,
   Gerd

[Qemu-devel] [PATCH] isa: configure serial+parallel by index.

[Gerd Hoffmann]

This patch adds a 'index' property to the isa-parallel and isa-serial
devices.  This can be used to create devices with the default isa irqs
and ioports by simply specifying the index, i.e.

   -device isa-serial,index=1

instead of

   -device isa-serial,iobase=0x2f8,irq=3

for ttyS1 aka com2.  Likewise for parallel ports.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/parallel.c |   23 +++++++++++++++++------
 hw/serial.c   |   26 +++++++++++++++++++-------
 2 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/hw/parallel.c b/hw/parallel.c
index 92eecb1..263b5bf 100644
--- a/hw/parallel.c
+++ b/hw/parallel.c
@@ -80,6 +80,7 @@ struct ParallelState {
 
 typedef struct ISAParallelState {
     ISADevice dev;
+    uint32_t index;
     uint32_t iobase;
     uint32_t isairq;
     ParallelState state;
@@ -445,11 +446,14 @@ static void parallel_reset(void *opaque)
     s->last_read_offset = ~0U;
 }
 
+static const int isa_parallel_io[MAX_PARALLEL_PORTS] = { 0x378, 0x278, 0x3bc };
+
 static int parallel_isa_initfn(ISADevice *dev)
 {
+    static int index;
     ISAParallelState *isa = DO_UPCAST(ISAParallelState, dev, dev);
     ParallelState *s = &isa->state;
-    int base = isa->iobase;
+    int base;
     uint8_t dummy;
 
     if (!s->chr) {
@@ -457,6 +461,15 @@ static int parallel_isa_initfn(ISADevice *dev)
         exit(1);
     }
 
+    if (isa->index == -1)
+        isa->index = index;
+    if (isa->index >= MAX_PARALLEL_PORTS)
+        return -1;
+    if (isa->iobase == -1)
+        isa->iobase = isa_parallel_io[isa->index];
+    index++;
+
+    base = isa->iobase;
     isa_init_irq(dev, &s->irq, isa->isairq);
     parallel_reset(s);
     qemu_register_reset(parallel_reset, s);
@@ -483,15 +496,12 @@ static int parallel_isa_initfn(ISADevice *dev)
     return 0;
 }
 
-static const int isa_parallel_io[MAX_PARALLEL_PORTS] = { 0x378, 0x278, 0x3bc };
-
 ParallelState *parallel_init(int index, CharDriverState *chr)
 {
     ISADevice *dev;
 
     dev = isa_create("isa-parallel");
-    qdev_prop_set_uint32(&dev->qdev, "iobase", isa_parallel_io[index]);
-    qdev_prop_set_uint32(&dev->qdev, "irq", 7);
+    qdev_prop_set_uint32(&dev->qdev, "index", index);
     qdev_prop_set_chr(&dev->qdev, "chardev", chr);
     if (qdev_init(&dev->qdev) < 0)
         return NULL;
@@ -579,7 +589,8 @@ static ISADeviceInfo parallel_isa_info = {
     .qdev.size  = sizeof(ISAParallelState),
     .init       = parallel_isa_initfn,
     .qdev.props = (Property[]) {
-        DEFINE_PROP_HEX32("iobase", ISAParallelState, iobase,  0x378),
+        DEFINE_PROP_UINT32("index", ISAParallelState, index,   -1),
+        DEFINE_PROP_HEX32("iobase", ISAParallelState, iobase,  -1),
         DEFINE_PROP_UINT32("irq",   ISAParallelState, isairq,  7),
         DEFINE_PROP_CHR("chardev",  ISAParallelState, state.chr),
         DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/serial.c b/hw/serial.c
index eb14f11..381b027 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -148,6 +148,7 @@ struct SerialState {
 
 typedef struct ISASerialState {
     ISADevice dev;
+    uint32_t index;
     uint32_t iobase;
     uint32_t isairq;
     SerialState state;
@@ -733,11 +734,25 @@ static void serial_init_core(SerialState *s)
                           serial_event, s);
 }
 
+static const int isa_serial_io[MAX_SERIAL_PORTS] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 };
+static const int isa_serial_irq[MAX_SERIAL_PORTS] = { 4, 3, 4, 3 };
+
 static int serial_isa_initfn(ISADevice *dev)
 {
+    static int index;
     ISASerialState *isa = DO_UPCAST(ISASerialState, dev, dev);
     SerialState *s = &isa->state;
 
+    if (isa->index == -1)
+        isa->index = index;
+    if (isa->index >= MAX_SERIAL_PORTS)
+        return -1;
+    if (isa->iobase == -1)
+        isa->iobase = isa_serial_io[isa->index];
+    if (isa->isairq == -1)
+        isa->isairq = isa_serial_irq[isa->index];
+    index++;
+
     s->baudbase = 115200;
     isa_init_irq(dev, &s->irq, isa->isairq);
     serial_init_core(s);
@@ -748,16 +763,12 @@ static int serial_isa_initfn(ISADevice *dev)
     return 0;
 }
 
-static const int isa_serial_io[MAX_SERIAL_PORTS] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 };
-static const int isa_serial_irq[MAX_SERIAL_PORTS] = { 4, 3, 4, 3 };
-
 SerialState *serial_isa_init(int index, CharDriverState *chr)
 {
     ISADevice *dev;
 
     dev = isa_create("isa-serial");
-    qdev_prop_set_uint32(&dev->qdev, "iobase", isa_serial_io[index]);
-    qdev_prop_set_uint32(&dev->qdev, "irq", isa_serial_irq[index]);
+    qdev_prop_set_uint32(&dev->qdev, "index", index);
     qdev_prop_set_chr(&dev->qdev, "chardev", chr);
     if (qdev_init(&dev->qdev) < 0)
         return NULL;
@@ -886,8 +897,9 @@ static ISADeviceInfo serial_isa_info = {
     .qdev.size  = sizeof(ISASerialState),
     .init       = serial_isa_initfn,
     .qdev.props = (Property[]) {
-        DEFINE_PROP_HEX32("iobase", ISASerialState, iobase,  0x3f8),
-        DEFINE_PROP_UINT32("irq",   ISASerialState, isairq,  4),
+        DEFINE_PROP_UINT32("index", ISASerialState, index,   -1),
+        DEFINE_PROP_HEX32("iobase", ISASerialState, iobase,  -1),
+        DEFINE_PROP_UINT32("irq",   ISASerialState, isairq,  -1),
         DEFINE_PROP_CHR("chardev",  ISASerialState, state.chr),
         DEFINE_PROP_END_OF_LIST(),
     },
-- 
1.6.2.5

[Qemu-devel] [PATCH] isa: configure serial+parallel by index.

[Gerd Hoffmann]

This patch adds a 'index' property to the isa-parallel and isa-serial
devices.  This can be used to create devices with the default isa irqs
and ioports by simply specifying the index, i.e.

   -device isa-serial,index=1

instead of

   -device isa-serial,iobase=0x2f8,irq=3

for ttyS1 aka com2.  Likewise for parallel ports.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/parallel.c |    2 +-
 hw/serial.c   |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/parallel.c b/hw/parallel.c
index 5ae8348..12693d4 100644
--- a/hw/parallel.c
+++ b/hw/parallel.c
@@ -587,7 +587,7 @@ static ISADeviceInfo parallel_isa_info = {
     .qdev.size  = sizeof(ISAParallelState),
     .init       = parallel_isa_initfn,
     .qdev.props = (Property[]) {
-        DEFINE_PROP_HEX32("index",  ISAParallelState, index,   -1),
+        DEFINE_PROP_UINT32("index", ISAParallelState, index,   -1),
         DEFINE_PROP_HEX32("iobase", ISAParallelState, iobase,  -1),
         DEFINE_PROP_UINT32("irq",   ISAParallelState, isairq,  7),
         DEFINE_PROP_CHR("chardev",  ISAParallelState, state.chr),
diff --git a/hw/serial.c b/hw/serial.c
index 0063260..e7538ac 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -900,7 +900,7 @@ static ISADeviceInfo serial_isa_info = {
     .qdev.size  = sizeof(ISASerialState),
     .init       = serial_isa_initfn,
     .qdev.props = (Property[]) {
-        DEFINE_PROP_HEX32("index",  ISASerialState, index,   -1),
+        DEFINE_PROP_UINT32("index", ISASerialState, index,   -1),
         DEFINE_PROP_HEX32("iobase", ISASerialState, iobase,  -1),
         DEFINE_PROP_UINT32("irq",   ISASerialState, isairq,  -1),
         DEFINE_PROP_CHR("chardev",  ISASerialState, state.chr),
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH] isa: configure serial+parallel by index.

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

> This patch adds a 'index' property to the isa-parallel and isa-serial
> devices.  This can be used to create devices with the default isa irqs
> and ioports by simply specifying the index, i.e.
>
>    -device isa-serial,index=1
>
> instead of
>
>    -device isa-serial,iobase=0x2f8,irq=3
>
> for ttyS1 aka com2.  Likewise for parallel ports.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/parallel.c |    2 +-
>  hw/serial.c   |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/parallel.c b/hw/parallel.c
> index 5ae8348..12693d4 100644
> --- a/hw/parallel.c
> +++ b/hw/parallel.c
> @@ -587,7 +587,7 @@ static ISADeviceInfo parallel_isa_info = {
>      .qdev.size  = sizeof(ISAParallelState),
>      .init       = parallel_isa_initfn,
>      .qdev.props = (Property[]) {
> -        DEFINE_PROP_HEX32("index",  ISAParallelState, index,   -1),
> +        DEFINE_PROP_UINT32("index", ISAParallelState, index,   -1),
>          DEFINE_PROP_HEX32("iobase", ISAParallelState, iobase,  -1),
>          DEFINE_PROP_UINT32("irq",   ISAParallelState, isairq,  7),
>          DEFINE_PROP_CHR("chardev",  ISAParallelState, state.chr),
> diff --git a/hw/serial.c b/hw/serial.c
> index 0063260..e7538ac 100644
> --- a/hw/serial.c
> +++ b/hw/serial.c
> @@ -900,7 +900,7 @@ static ISADeviceInfo serial_isa_info = {
>      .qdev.size  = sizeof(ISASerialState),
>      .init       = serial_isa_initfn,
>      .qdev.props = (Property[]) {
> -        DEFINE_PROP_HEX32("index",  ISASerialState, index,   -1),
> +        DEFINE_PROP_UINT32("index", ISASerialState, index,   -1),
>          DEFINE_PROP_HEX32("iobase", ISASerialState, iobase,  -1),
>          DEFINE_PROP_UINT32("irq",   ISASerialState, isairq,  -1),
>          DEFINE_PROP_CHR("chardev",  ISASerialState, state.chr),

The commit message is bogus.  The patch is a minor fix of commit
e8ee28fb, and the message is a copy of that commit's message.

Re: [Qemu-devel] [PATCH] isa: configure serial+parallel by index.

[Anthony Liguori]

Markus Armbruster wrote:
> The commit message is bogus.  The patch is a minor fix of commit
> e8ee28fb, and the message is a copy of that commit's message.
>   

Thanks, I've updated the commit message appropriately.

Regards,

Anthony Liguori