Patchwork [05/14] hw/stellaris: Removed gpio_out init array.

login
register
mail settings
Submitter Peter A. G. Crosthwaite
Date Oct. 5, 2012, 12:08 a.m.
Message ID <1349395739-26502-6-git-send-email-peter.crosthwaite@xilinx.com>
Download mbox | patch
Permalink /patch/189383/
State New
Headers show

Comments

Peter A. G. Crosthwaite - Oct. 5, 2012, 12:08 a.m.
From: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>

stellaris_init() defines arrays of qemu_irq to decides what each of the GPIO
pins are connected to. This is ok for inputs (as an input can only have one
source) but is flawed for outputs as an output can connect to any number of
sinks. Removed the gpio_out array completely and just replaced its setters with
direct calls to qdev_connect_gpio_out().

Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
 hw/stellaris.c |   26 ++++++++++++--------------
 1 files changed, 12 insertions(+), 14 deletions(-)
Peter Maydell - Oct. 5, 2012, 12:31 p.m.
On 5 October 2012 01:08, Peter Crosthwaite
<peter.crosthwaite@petalogix.com> wrote:
> From: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
>
> stellaris_init() defines arrays of qemu_irq to decides what each of the GPIO
> pins are connected to. This is ok for inputs (as an input can only have one
> source) but is flawed for outputs as an output can connect to any number of
> sinks. Removed the gpio_out array completely and just replaced its setters with
> direct calls to qdev_connect_gpio_out().

You can only connect an output to one sink. (If you want to wire it to
multiple places you need to use a qemu_irq_split() somewhere.) What
bug is this patch fixing?

-- PMM

> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
> ---
>  hw/stellaris.c |   26 ++++++++++++--------------
>  1 files changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/hw/stellaris.c b/hw/stellaris.c
> index 01050d1..a7b68f4 100644
> --- a/hw/stellaris.c
> +++ b/hw/stellaris.c
> @@ -1244,7 +1244,6 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model,
>      qemu_irq *pic;
>      DeviceState *gpio_dev[7];
>      qemu_irq gpio_in[7][8];
> -    qemu_irq gpio_out[7][8];
>      qemu_irq adc;
>      int sram_size;
>      int flash_size;
> @@ -1284,8 +1283,9 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model,
>                                                 pic[gpio_irq[i]]);
>              for (j = 0; j < 8; j++) {
>                  gpio_in[i][j] = qdev_get_gpio_in(gpio_dev[i], j);
> -                gpio_out[i][j] = NULL;
>              }
> +        } else {
> +            gpio_dev[i] = NULL;
>          }
>      }
>
> @@ -1308,20 +1308,27 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model,
>          if (board->peripherals & BP_OLED_SSI) {
>              DeviceState *mux;
>              void *bus;
> +            qemu_irq select_pin;
>
>              bus = qdev_get_child_bus(dev, "ssi");
>              mux = ssi_create_slave(bus, "evb6965-ssi");
> -            gpio_out[GPIO_D][0] = qdev_get_gpio_in(mux, 0);
> +            select_pin = qdev_get_gpio_in(mux, 0);
> +            if (gpio_dev[GPIO_D]) {
> +                qdev_connect_gpio_out(gpio_dev[GPIO_D], 0, select_pin);
> +            }
>
>              bus = qdev_get_child_bus(mux, "ssi0");
>              ssi_create_slave(bus, "ssi-sd");
>
>              bus = qdev_get_child_bus(mux, "ssi1");
>              dev = ssi_create_slave(bus, "ssd0323");
> -            gpio_out[GPIO_C][7] = qdev_get_gpio_in(dev, 0);
> +            if (gpio_dev[GPIO_C]) {
> +                qdev_connect_gpio_out(gpio_dev[GPIO_C], 7,
> +                                        qdev_get_gpio_in(dev, 0));
> +            }
>
>              /* Make sure the select pin is high.  */
> -            qemu_irq_raise(gpio_out[GPIO_D][0]);
> +            qemu_irq_raise(select_pin);
>          }
>      }
>      if (board->dc4 & (1 << 28)) {
> @@ -1347,15 +1354,6 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model,
>
>          stellaris_gamepad_init(5, gpad_irq, gpad_keycode);
>      }
> -    for (i = 0; i < 7; i++) {
> -        if (board->dc4 & (1 << i)) {
> -            for (j = 0; j < 8; j++) {
> -                if (gpio_out[i][j]) {
> -                    qdev_connect_gpio_out(gpio_dev[i], j, gpio_out[i][j]);
> -                }
> -            }
> -        }
> -    }
>  }
>
>  /* FIXME: Figure out how to generate these from stellaris_boards.  */
> --
> 1.7.0.4
>
>
Peter A. G. Crosthwaite - Oct. 5, 2012, 2:17 p.m.
On Fri, Oct 5, 2012 at 10:31 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 5 October 2012 01:08, Peter Crosthwaite
> <peter.crosthwaite@petalogix.com> wrote:
>> From: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
>>
>> stellaris_init() defines arrays of qemu_irq to decides what each of the GPIO
>> pins are connected to. This is ok for inputs (as an input can only have one
>> source) but is flawed for outputs as an output can connect to any number of
>> sinks. Removed the gpio_out array completely and just replaced its setters with
>> direct calls to qdev_connect_gpio_out().
>
> You can only connect an output to one sink. (If you want to wire it to
> multiple places you need to use a qemu_irq_split() somewhere.) What
> bug is this patch fixing?
>

qemu_irq_split() is the answer, this patch is deleted in v9.

Regards,
Peter

> -- PMM
>
>> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
>> ---
>>  hw/stellaris.c |   26 ++++++++++++--------------
>>  1 files changed, 12 insertions(+), 14 deletions(-)
>>
>> diff --git a/hw/stellaris.c b/hw/stellaris.c
>> index 01050d1..a7b68f4 100644
>> --- a/hw/stellaris.c
>> +++ b/hw/stellaris.c
>> @@ -1244,7 +1244,6 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model,
>>      qemu_irq *pic;
>>      DeviceState *gpio_dev[7];
>>      qemu_irq gpio_in[7][8];
>> -    qemu_irq gpio_out[7][8];
>>      qemu_irq adc;
>>      int sram_size;

Patch

diff --git a/hw/stellaris.c b/hw/stellaris.c
index 01050d1..a7b68f4 100644
--- a/hw/stellaris.c
+++ b/hw/stellaris.c
@@ -1244,7 +1244,6 @@  static void stellaris_init(const char *kernel_filename, const char *cpu_model,
     qemu_irq *pic;
     DeviceState *gpio_dev[7];
     qemu_irq gpio_in[7][8];
-    qemu_irq gpio_out[7][8];
     qemu_irq adc;
     int sram_size;
     int flash_size;
@@ -1284,8 +1283,9 @@  static void stellaris_init(const char *kernel_filename, const char *cpu_model,
                                                pic[gpio_irq[i]]);
             for (j = 0; j < 8; j++) {
                 gpio_in[i][j] = qdev_get_gpio_in(gpio_dev[i], j);
-                gpio_out[i][j] = NULL;
             }
+        } else {
+            gpio_dev[i] = NULL;
         }
     }
 
@@ -1308,20 +1308,27 @@  static void stellaris_init(const char *kernel_filename, const char *cpu_model,
         if (board->peripherals & BP_OLED_SSI) {
             DeviceState *mux;
             void *bus;
+            qemu_irq select_pin;
 
             bus = qdev_get_child_bus(dev, "ssi");
             mux = ssi_create_slave(bus, "evb6965-ssi");
-            gpio_out[GPIO_D][0] = qdev_get_gpio_in(mux, 0);
+            select_pin = qdev_get_gpio_in(mux, 0);
+            if (gpio_dev[GPIO_D]) {
+                qdev_connect_gpio_out(gpio_dev[GPIO_D], 0, select_pin);
+            }
 
             bus = qdev_get_child_bus(mux, "ssi0");
             ssi_create_slave(bus, "ssi-sd");
 
             bus = qdev_get_child_bus(mux, "ssi1");
             dev = ssi_create_slave(bus, "ssd0323");
-            gpio_out[GPIO_C][7] = qdev_get_gpio_in(dev, 0);
+            if (gpio_dev[GPIO_C]) {
+                qdev_connect_gpio_out(gpio_dev[GPIO_C], 7,
+                                        qdev_get_gpio_in(dev, 0));
+            }
 
             /* Make sure the select pin is high.  */
-            qemu_irq_raise(gpio_out[GPIO_D][0]);
+            qemu_irq_raise(select_pin);
         }
     }
     if (board->dc4 & (1 << 28)) {
@@ -1347,15 +1354,6 @@  static void stellaris_init(const char *kernel_filename, const char *cpu_model,
 
         stellaris_gamepad_init(5, gpad_irq, gpad_keycode);
     }
-    for (i = 0; i < 7; i++) {
-        if (board->dc4 & (1 << i)) {
-            for (j = 0; j < 8; j++) {
-                if (gpio_out[i][j]) {
-                    qdev_connect_gpio_out(gpio_dev[i], j, gpio_out[i][j]);
-                }
-            }
-        }
-    }
 }
 
 /* FIXME: Figure out how to generate these from stellaris_boards.  */