Named GPIOs
v4 cover Named GPIOs
This series implements Named GPIOs on the qdev level. Patch 1 is the feature presentation.
Second patch I give a useful example of the feature's use (an SSI cleanup).
Changed since v3:
Addressed PMM v2 review
Changed since v2:
Re-ordered function args (PMM review)
Changed since v1:
use QLIST (AF review)
Dropped former P1 (too far out of scope)
Peter Crosthwaite (2):
qdev: Implement named GPIOs
ssi: Name the CS GPIO.
hw/arm/stellaris.c | 7 ++-- hw/arm/xilinx_zynq.c | 2 +- hw/core/qdev.c | 82 ++++++++++++++++++++++++++++++++----- hw/microblaze/petalogix_ml605_mmu.c | 2 +- hw/ssi/ssi.c | 4 +- include/hw/qdev-core.h | 24 +++++++++-- include/hw/ssi.h | 2 + qdev-monitor.c | 16 +++++--- qtest.c | 19 ++++++--- 9 files changed, 127 insertions(+), 31 deletions(-)
Signature · 1 line
1.9.2.1.g06c4abd
PATCH v4 1/2 qdev: Implement named GPIOs
Implement named GPIOs on the Device layer. Listifies the existing GPIOs stuff using string keys. Legacy un-named GPIOs are preserved by using a NULL name string - they are just a single matchable element in the name list.
Signed-off-by Peter Crosthwaite <[email protected]>
---
Changed since v3 (PMM review):
Added finalize() cleanup of all allocated mem Remove %s printf of NULL in info qtree Restricted Qtest to unnamed GPIOs only
Changed since v2:
Reordered fn args to be name-before-number (PMM review) changed since v1:
Use QLIST instead of RYO list implementation (AF review) Reorder struct fields for consistency
hw/core/qdev.c | 82 ++++++++++++++++++++++++++++++++++++++++++++------ include/hw/qdev-core.h | 24 ++++++++++++--- qdev-monitor.c | 16 +++++++--- qtest.c | 19 +++++++++--- 4 files changed, 117 insertions(+), 24 deletions(-)
hw/core/qdev.c +72 -10
@@ -312,30 +312,82 @@ BusState *qdev_get_parent_bus(DeviceState *dev)return dev->parent_bus; } +static NamedGPIOList *qdev_get_named_gpio_list (DeviceState *dev, +const char *name) +{ +NamedGPIOList *ngl; + + QLIST_FOREACH(ngl, &dev->gpios, node) { +/* NULL is a valid and matchable name, otherwise do a normal +* strcmp match. +*/ +if ((!ngl->name && !name) || + (name && ngl->name && !strcmp (name, ngl->name))) { +return ngl; + } + } + + ngl =g_malloc0 (sizeof (*ngl)); + ngl->name =g_strdup (name); +QLIST_INSERT_HEAD (&dev->gpios, ngl, node); +return ngl; +} + +void qdev_init_gpio_in_named (DeviceState *dev,qemu_irq_handler handler, +const char *name,int n) +{ +NamedGPIOList *gpio_list =qdev_get_named_gpio_list (dev, name); + + gpio_list->in =qemu_extend_irqs (gpio_list->in, gpio_list->num_in, handler, + dev, n); + gpio_list->num_in += n; +} +void qdev_init_gpio_in (DeviceState *dev,qemu_irq_handler handler,int n) { - dev->gpio_in =qemu_extend_irqs (dev->gpio_in, dev->num_gpio_in, handler, - dev, n); - dev->num_gpio_in += n; +qdev_init_gpio_in_named (dev, handler,NULL , n); +} + +void qdev_init_gpio_out_named (DeviceState *dev,qemu_irq *pins, +const char *name,int n) +{ +NamedGPIOList *gpio_list =qdev_get_named_gpio_list (dev, name); + +assert (gpio_list->num_out ==0 ); + gpio_list->num_out = n; + gpio_list->out = pins; }void qdev_init_gpio_out (DeviceState *dev,qemu_irq *pins,int n) { -assert (dev->num_gpio_out ==0 ); - dev->num_gpio_out = n; - dev->gpio_out = pins; +qdev_init_gpio_out_named (dev, pins,NULL , n); +} + +qemu_irq qdev_get_gpio_in_named (DeviceState *dev,const char *name,int n) +{ +NamedGPIOList *gpio_list =qdev_get_named_gpio_list (dev, name); + +assert (n >=0 && n < gpio_list->num_in); +return gpio_list->in[n]; }qemu_irq qdev_get_gpio_in (DeviceState *dev,int n) { -assert (n >=0 && n < dev->num_gpio_in); -return dev->gpio_in[n]; +return qdev_get_gpio_in_named (dev,NULL , n); +} + +void qdev_connect_gpio_out_named (DeviceState *dev,const char *name,int n, +qemu_irq pin) +{ +NamedGPIOList *gpio_list =qdev_get_named_gpio_list (dev, name); + +assert (n >=0 && n < gpio_list->num_out); + gpio_list->out[n] = pin; }void qdev_connect_gpio_out (DeviceState * dev,int n,qemu_irq pin) { -assert (n >=0 && n < dev->num_gpio_out); - dev->gpio_out[n] = pin; +qdev_connect_gpio_out_named (dev,NULL , n, pin); }BusState *qdev_get_child_bus (DeviceState *dev,const char *name)
@@ -844,6 +896,7 @@ static void device_initfn(Object *obj)object_property_add_link (OBJECT (dev),"parent_bus" ,TYPE_BUS , (Object **)&dev->parent_bus,NULL ,0 , &error_abort); +QLIST_INIT (&dev->gpios); }static void device_post_init (Object *obj)
@@ -854,10 +907,19 @@ static void device_post_init(Object *obj)/* Unlink device from bus and free the structure. */ static void device_finalize (Object *obj) { +NamedGPIOList *ngl, *next; +DeviceState *dev =DEVICE (obj);if (dev->opts) {qemu_opts_del (dev->opts); } + + QLIST_FOREACH_SAFE(ngl, &dev->gpios, node, next) { +QLIST_REMOVE (ngl, node); +qemu_free_irqs (ngl->in); +g_free (ngl->name); +g_free (ngl); + } }static void device_class_base_init (ObjectClass *class,void *data)
include/hw/qdev-core.h +20 -4
@@ -131,6 +131,17 @@ typedef struct DeviceClass {const char *bus_type; } DeviceClass; +typedef struct NamedGPIOList NamedGPIOList ; + +struct NamedGPIOList { +char *name; +qemu_irq *in; +int num_in; +qemu_irq *out; +int num_out; +QLIST_ENTRY (NamedGPIOList) node; +}; +/** * DeviceState: * @realized: Indicates whether the device has been fully constructed.
@@ -148,10 +159,7 @@ struct DeviceState {QemuOpts *opts;int hotplugged;BusState *parent_bus; -int num_gpio_out; -qemu_irq *gpio_out; -int num_gpio_in; -qemu_irq *gpio_in; + QLIST_HEAD(, NamedGPIOList) gpios; QLIST_HEAD(, BusState) child_bus;int num_child_bus;int instance_id_alias;
@@ -252,7 +260,11 @@ void qdev_machine_creation_done(void);bool qdev_machine_modified (void );qemu_irq qdev_get_gpio_in (DeviceState *dev,int n); +qemu_irq qdev_get_gpio_in_named (DeviceState *dev,const char *name,int n); +void qdev_connect_gpio_out (DeviceState *dev,int n,qemu_irq pin); +void qdev_connect_gpio_out_named (DeviceState *dev,const char *name,int n, +qemu_irq pin);BusState *qdev_get_child_bus (DeviceState *dev,const char *name);
@@ -262,6 +274,10 @@ BusState *qdev_get_child_bus(DeviceState *dev, const char *name);/* GPIO inputs also double as IRQ sinks. */ void qdev_init_gpio_in (DeviceState *dev,qemu_irq_handler handler,int n);void qdev_init_gpio_out (DeviceState *dev,qemu_irq *pins,int n); +void qdev_init_gpio_in_named (DeviceState *dev,qemu_irq_handler handler, +const char *name,int n); +void qdev_init_gpio_out_named (DeviceState *dev,qemu_irq *pins, +const char *name,int n);BusState *qdev_get_parent_bus (DeviceState *dev);
qdev-monitor.c +11 -5
@@ -613,14 +613,20 @@ static void qdev_print(Monitor *mon, DeviceState *dev, int indent) {ObjectClass *class;BusState *child; +NamedGPIOList *ngl; +qdev_printf ("dev: %s, id \"%s\"\n" ,object_get_typename (OBJECT (dev)), dev->id ? dev->id :"" ); indent +=2 ; -if (dev->num_gpio_in) { -qdev_printf ("gpio-in %d\n" , dev->num_gpio_in); - } -if (dev->num_gpio_out) { -qdev_printf ("gpio-out %d\n" , dev->num_gpio_out); + QLIST_FOREACH(ngl, &dev->gpios, node) { +if (ngl->num_in) { +qdev_printf ("gpio-in \"%s\" %d\n" , ngl->name ? ngl->name :"" , + ngl->num_in); + } +if (ngl->num_out) { +qdev_printf ("gpio-out \"%s\" %d\n" , ngl->name ? ngl->name :"" , + ngl->num_out); + } } class =object_get_class (OBJECT (dev)); do {
qtest.c +14 -5
@@ -233,7 +233,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)g_assert (command);if (strcmp (words[0 ],"irq_intercept_out" ) ==0 ||strcmp (words[0 ],"irq_intercept_in" ) ==0 ) { -DeviceState *dev; +DeviceState *dev; +NamedGPIOList *ngl;g_assert (words[1 ]); dev =DEVICE (object_resolve_path (words[1 ],NULL ));
@@ -253,10 +254,18 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)return ; } -if (words[0 ][14 ] =='o' ) { -qemu_irq_intercept_out (&dev->gpio_out, qtest_irq_handler, dev->num_gpio_out); - }else { -qemu_irq_intercept_in (dev->gpio_in, qtest_irq_handler, dev->num_gpio_in); + QLIST_FOREACH(ngl, &dev->gpios, node) { +/* We dont support intercept of named GPIOs yet */ +if (ngl->name) { +continue ; + } +if (words[0 ][14 ] =='o' ) { +qemu_irq_intercept_out (&ngl->out, qtest_irq_handler, + ngl->num_out); + }else { +qemu_irq_intercept_in (ngl->in, qtest_irq_handler, + ngl->num_in); + } } irq_intercept_dev = dev;qtest_send_prefix (chr);
Signature · 1 line
1.9.2.1.g06c4abd
Re: [Qemu-devel] [PATCH v4 1/2] qdev: Implement named GPIOs
Quoted 6 lines · Peter Crosthwaite
Implement named GPIOs on the Device layer. Listifies the existing GPIOs stuff using string keys. Legacy un-named GPIOs are preserved by using a NULL name string - they are just a single matchable element in the name list.
Signed-off-by Peter Crosthwaite <[email protected]>
Two very minor nits involving comments below, but otherwise
Reviewed-by Peter Maydell <[email protected]>
and I would be happy for these patches to be committed (through the QOM tree?)
Quoted 6 lines
hw/core/qdev.c +2
-0@@ -854,10 +907,19 @@ static void device_post_init(Object *obj)/* Unlink device from bus and free the structure. */ static void device_finalize (Object *obj) { +NamedGPIOList *ngl, *next; +
hw/core/qdev.c static void device_post_init(Object *obj) · line 921
Quoted 10 lines
hw/core/qdev.c +6
-0DeviceState *dev =DEVICE (obj);if (dev->opts) {qemu_opts_del (dev->opts); } + + QLIST_FOREACH_SAFE(ngl, &dev->gpios, node, next) { +QLIST_REMOVE (ngl, node); +qemu_free_irqs (ngl->in); +g_free (ngl->name); +g_free (ngl);
I think we could use a comment along the lines of
/* ngl->out irqs are owned by the other end and should not be freed here */
to stop somebody later on wondering whether it's a bug that we don't clean up that field in NamedGPIOList.
qtest.c static void qtest_process_command(CharDriverState *chr, gchar **words) · line 258
Quoted 10 lines
qtest.c +2
-4@@ -253,10 +254,18 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)return ; } -if (words[0 ][14 ] =='o' ) { -qemu_irq_intercept_out (&dev->gpio_out, qtest_irq_handler, dev->num_gpio_out); - }else { -qemu_irq_intercept_in (dev->gpio_in, qtest_irq_handler, dev->num_gpio_in); + QLIST_FOREACH(ngl, &dev->gpios, node) { +/* We dont support intercept of named GPIOs yet */
"don't".
thanks
-- PMM
PATCH v4 2/2 ssi: Name the CS GPIO.
In reply to Peter CrosthwaiteTo get it out of the default GPIO list. This allows child devices to use the un-named GPIO namespace without having to be SSI aware. That is, there is no more need for machines to know about the obscure policy where GPIO 0 is the SSI chip-select and GPIO 1..N are the concrete class GPIOs (defined locally as 0..N-1).
This is most notable is stellaris, which uses a device which has both SSI and concrete level GPIOs.
Signed-off-by Peter Crosthwaite <[email protected]>
Reviewed-by Peter Maydell <[email protected]>
---
hw/arm/stellaris.c | 7 ++++--- hw/arm/xilinx_zynq.c | 2 +- hw/microblaze/petalogix_ml605_mmu.c | 2 +- hw/ssi/ssi.c | 4 ++-- include/hw/ssi.h | 2 ++ 5 files changed, 10 insertions(+), 7 deletions(-)
hw/arm/stellaris.c +4 -3
@@ -1287,9 +1287,10 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model, sddev =ssi_create_slave (bus,"ssi-sd" ); ssddev =ssi_create_slave (bus,"ssd0323" ); - gpio_out[GPIO_D ][0 ] =qemu_irq_split (qdev_get_gpio_in (sddev,0 ), -qdev_get_gpio_in (ssddev,0 )); - gpio_out[GPIO_C ][7 ] =qdev_get_gpio_in (ssddev,1 ); + gpio_out[GPIO_D ][0 ] =qemu_irq_split ( +qdev_get_gpio_in_named (sddev,SSI_GPIO_CS ,0 ), +qdev_get_gpio_in_named (ssddev,SSI_GPIO_CS ,0 )); + gpio_out[GPIO_C ][7 ] =qdev_get_gpio_in (ssddev,0 );/* Make sure the select pin is high. */ qemu_irq_raise (gpio_out[GPIO_D ][0 ]);
hw/arm/xilinx_zynq.c +1 -1
@@ -94,7 +94,7 @@ static inline void zynq_init_spi_flashes(uint32_t base_addr, qemu_irq irq,for (j =0 ; j < num_ss; ++j) { flash_dev =ssi_create_slave (spi,"n25q128" ); - cs_line =qdev_get_gpio_in (flash_dev,0 ); + cs_line =qdev_get_gpio_in_named (flash_dev,SSI_GPIO_CS ,0 );sysbus_connect_irq (busdev, i * num_ss + j +1 , cs_line); } }
hw/microblaze/petalogix_ml605_mmu.c +1 -1
@@ -196,7 +196,7 @@ petalogix_ml605_init(QEMUMachineInitArgs *args)qemu_irq cs_line; dev =ssi_create_slave (spi,"n25q128" ); - cs_line =qdev_get_gpio_in (dev,0 ); + cs_line =qdev_get_gpio_in_named (dev,SSI_GPIO_CS ,0 );sysbus_connect_irq (busdev, i+1 , cs_line); } }
hw/ssi/ssi.c +2 -2
@@ -60,7 +60,7 @@ static int ssi_slave_init(DeviceState *dev)if (ssc->transfer_raw == ssi_transfer_raw_default && ssc->cs_polarity !=SSI_CS_NONE ) { -qdev_init_gpio_in (dev, ssi_cs_default,1 ); +qdev_init_gpio_in_named (dev, ssi_cs_default,SSI_GPIO_CS ,1 ); }return ssc->init (s);
@@ -156,7 +156,7 @@ static int ssi_auto_connect_slave(Object *child, void *opaque)return 0 ; } - cs_line =qdev_get_gpio_in (DEVICE (dev),0 ); + cs_line =qdev_get_gpio_in_named (DEVICE (dev),SSI_GPIO_CS ,0 );qdev_set_parent_bus (DEVICE (dev),BUS (arg->bus)); **arg->cs_linep = cs_line; (*arg->cs_linep)++;
include/hw/ssi.h +2 -0
@@ -23,6 +23,8 @@ typedef struct SSISlave SSISlave;#define SSI_SLAVE_GET_CLASS (obj) \ OBJECT_GET_CLASS(SSISlaveClass, (obj), TYPE_SSI_SLAVE) +#define SSI_GPIO_CS "ssi-gpio-cs" + typedef enum { SSI_CS_NONE = 0, SSI_CS_LOW,
Signature · 1 line
1.9.2.1.g06c4abd