* [PATCH v6 1/9] hw/i2c/core: Add i2c_try_create_slave() and i2c_realize_and_unref()
2020-06-23 7:27 [PATCH v6 0/9] hw/misc/pca9552: Trace GPIO change events Philippe Mathieu-Daudé
@ 2020-06-23 7:27 ` Philippe Mathieu-Daudé
2020-06-23 11:07 ` Philippe Mathieu-Daudé
2020-06-26 18:23 ` Peter Maydell
2020-06-23 7:27 ` [PATCH v6 2/9] hw/misc/pca9552: Rename 'nr_leds' as 'pin_count' Philippe Mathieu-Daudé
` (8 subsequent siblings)
9 siblings, 2 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-23 7:27 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Corey Minyard, Andrew Jeffery,
Philippe Mathieu-Daudé, Markus Armbruster, qemu-arm,
Cédric Le Goater, Joel Stanley
Extract i2c_try_create_slave() and i2c_realize_and_unref()
from i2c_create_slave().
We can now set properties on a I2CSlave before it is realized.
This is in line with the recent qdev/QOM changes merged
in commit 6675a653d2e.
Reviewed-by: Corey Minyard <cminyard@mvista.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Cc: Markus Armbruster <armbru@redhat.com>
---
include/hw/i2c/i2c.h | 2 ++
hw/i2c/core.c | 18 ++++++++++++++++--
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h
index 4117211565..d6e3d85faf 100644
--- a/include/hw/i2c/i2c.h
+++ b/include/hw/i2c/i2c.h
@@ -80,6 +80,8 @@ int i2c_send(I2CBus *bus, uint8_t data);
uint8_t i2c_recv(I2CBus *bus);
DeviceState *i2c_create_slave(I2CBus *bus, const char *name, uint8_t addr);
+DeviceState *i2c_try_create_slave(const char *name, uint8_t addr);
+bool i2c_realize_and_unref(DeviceState *dev, I2CBus *bus, Error **errp);
/* lm832x.c */
void lm832x_key_event(DeviceState *dev, int key, int state);
diff --git a/hw/i2c/core.c b/hw/i2c/core.c
index 1aac457a2a..acf34a12d6 100644
--- a/hw/i2c/core.c
+++ b/hw/i2c/core.c
@@ -267,13 +267,27 @@ const VMStateDescription vmstate_i2c_slave = {
}
};
-DeviceState *i2c_create_slave(I2CBus *bus, const char *name, uint8_t addr)
+DeviceState *i2c_try_create_slave(const char *name, uint8_t addr)
{
DeviceState *dev;
dev = qdev_new(name);
qdev_prop_set_uint8(dev, "address", addr);
- qdev_realize_and_unref(dev, &bus->qbus, &error_fatal);
+ return dev;
+}
+
+bool i2c_realize_and_unref(DeviceState *dev, I2CBus *bus, Error **errp)
+{
+ return qdev_realize_and_unref(dev, &bus->qbus, errp);
+}
+
+DeviceState *i2c_create_slave(I2CBus *bus, const char *name, uint8_t addr)
+{
+ DeviceState *dev;
+
+ dev = i2c_try_create_slave(name, addr);
+ i2c_realize_and_unref(dev, bus, &error_fatal);
+
return dev;
}
--
2.21.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v6 1/9] hw/i2c/core: Add i2c_try_create_slave() and i2c_realize_and_unref()
2020-06-23 7:27 ` [PATCH v6 1/9] hw/i2c/core: Add i2c_try_create_slave() and i2c_realize_and_unref() Philippe Mathieu-Daudé
@ 2020-06-23 11:07 ` Philippe Mathieu-Daudé
2020-06-26 18:23 ` Peter Maydell
1 sibling, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-23 11:07 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Corey Minyard, Andrew Jeffery, Markus Armbruster,
qemu-arm, Cédric Le Goater, Joel Stanley
On 6/23/20 9:27 AM, Philippe Mathieu-Daudé wrote:
> Extract i2c_try_create_slave() and i2c_realize_and_unref()
> from i2c_create_slave().
> We can now set properties on a I2CSlave before it is realized.
>
> This is in line with the recent qdev/QOM changes merged
> in commit 6675a653d2e.
>
> Reviewed-by: Corey Minyard <cminyard@mvista.com>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
Collecting Markus review on v5:
Reviewed-by: Markus Armbruster <armbru@redhat.com>
https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg07060.html
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Cc: Markus Armbruster <armbru@redhat.com>
> ---
> include/hw/i2c/i2c.h | 2 ++
> hw/i2c/core.c | 18 ++++++++++++++++--
> 2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h
> index 4117211565..d6e3d85faf 100644
> --- a/include/hw/i2c/i2c.h
> +++ b/include/hw/i2c/i2c.h
> @@ -80,6 +80,8 @@ int i2c_send(I2CBus *bus, uint8_t data);
> uint8_t i2c_recv(I2CBus *bus);
>
> DeviceState *i2c_create_slave(I2CBus *bus, const char *name, uint8_t addr);
> +DeviceState *i2c_try_create_slave(const char *name, uint8_t addr);
> +bool i2c_realize_and_unref(DeviceState *dev, I2CBus *bus, Error **errp);
>
> /* lm832x.c */
> void lm832x_key_event(DeviceState *dev, int key, int state);
> diff --git a/hw/i2c/core.c b/hw/i2c/core.c
> index 1aac457a2a..acf34a12d6 100644
> --- a/hw/i2c/core.c
> +++ b/hw/i2c/core.c
> @@ -267,13 +267,27 @@ const VMStateDescription vmstate_i2c_slave = {
> }
> };
>
> -DeviceState *i2c_create_slave(I2CBus *bus, const char *name, uint8_t addr)
> +DeviceState *i2c_try_create_slave(const char *name, uint8_t addr)
> {
> DeviceState *dev;
>
> dev = qdev_new(name);
> qdev_prop_set_uint8(dev, "address", addr);
> - qdev_realize_and_unref(dev, &bus->qbus, &error_fatal);
> + return dev;
> +}
> +
> +bool i2c_realize_and_unref(DeviceState *dev, I2CBus *bus, Error **errp)
> +{
> + return qdev_realize_and_unref(dev, &bus->qbus, errp);
> +}
> +
> +DeviceState *i2c_create_slave(I2CBus *bus, const char *name, uint8_t addr)
> +{
> + DeviceState *dev;
> +
> + dev = i2c_try_create_slave(name, addr);
> + i2c_realize_and_unref(dev, bus, &error_fatal);
> +
> return dev;
> }
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v6 1/9] hw/i2c/core: Add i2c_try_create_slave() and i2c_realize_and_unref()
2020-06-23 7:27 ` [PATCH v6 1/9] hw/i2c/core: Add i2c_try_create_slave() and i2c_realize_and_unref() Philippe Mathieu-Daudé
2020-06-23 11:07 ` Philippe Mathieu-Daudé
@ 2020-06-26 18:23 ` Peter Maydell
2020-06-26 21:28 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 18+ messages in thread
From: Peter Maydell @ 2020-06-26 18:23 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Corey Minyard, Andrew Jeffery, QEMU Developers, Markus Armbruster,
qemu-arm, Joel Stanley, Cédric Le Goater
On Tue, 23 Jun 2020 at 08:27, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Extract i2c_try_create_slave() and i2c_realize_and_unref()
> from i2c_create_slave().
> We can now set properties on a I2CSlave before it is realized.
>
> This is in line with the recent qdev/QOM changes merged
> in commit 6675a653d2e.
>
> Reviewed-by: Corey Minyard <cminyard@mvista.com>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Couple of things I belatedly noticed on this patch, which I don't
think are important enough for me to drop it from my pullreq,
but which I think it would be nice to address in a followup:
> diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h
> index 4117211565..d6e3d85faf 100644
> --- a/include/hw/i2c/i2c.h
> +++ b/include/hw/i2c/i2c.h
> @@ -80,6 +80,8 @@ int i2c_send(I2CBus *bus, uint8_t data);
> uint8_t i2c_recv(I2CBus *bus);
>
> DeviceState *i2c_create_slave(I2CBus *bus, const char *name, uint8_t addr);
> +DeviceState *i2c_try_create_slave(const char *name, uint8_t addr);
> +bool i2c_realize_and_unref(DeviceState *dev, I2CBus *bus, Error **errp);
Can we have doc-comments for new global-scope functions, please ?
> --- a/hw/i2c/core.c
> +++ b/hw/i2c/core.c
> @@ -267,13 +267,27 @@ const VMStateDescription vmstate_i2c_slave = {
> }
> };
>
> -DeviceState *i2c_create_slave(I2CBus *bus, const char *name, uint8_t addr)
> +DeviceState *i2c_try_create_slave(const char *name, uint8_t addr)
> {
> DeviceState *dev;
>
> dev = qdev_new(name);
> qdev_prop_set_uint8(dev, "address", addr);
> - qdev_realize_and_unref(dev, &bus->qbus, &error_fatal);
> + return dev;
> +}
> +
> +bool i2c_realize_and_unref(DeviceState *dev, I2CBus *bus, Error **errp)
> +{
> + return qdev_realize_and_unref(dev, &bus->qbus, errp);
> +}
> +
> +DeviceState *i2c_create_slave(I2CBus *bus, const char *name, uint8_t addr)
> +{
> + DeviceState *dev;
> +
> + dev = i2c_try_create_slave(name, addr);
> + i2c_realize_and_unref(dev, bus, &error_fatal);
> +
We now have a _try_ function which isn't "same behaviour as
the non-try function, but give me back an error status rather
than just killing QEMU". That seems confusing -- is there a
better name we can use ?
thanks
-- PMM
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v6 1/9] hw/i2c/core: Add i2c_try_create_slave() and i2c_realize_and_unref()
2020-06-26 18:23 ` Peter Maydell
@ 2020-06-26 21:28 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-26 21:28 UTC (permalink / raw)
To: Peter Maydell
Cc: Corey Minyard, Andrew Jeffery, QEMU Developers, Markus Armbruster,
qemu-arm, Joel Stanley, Cédric Le Goater
On Fri, Jun 26, 2020 at 8:27 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> On Tue, 23 Jun 2020 at 08:27, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> > Extract i2c_try_create_slave() and i2c_realize_and_unref()
> > from i2c_create_slave().
> > We can now set properties on a I2CSlave before it is realized.
> >
> > This is in line with the recent qdev/QOM changes merged
> > in commit 6675a653d2e.
> >
> > Reviewed-by: Corey Minyard <cminyard@mvista.com>
> > Reviewed-by: Cédric Le Goater <clg@kaod.org>
> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> Couple of things I belatedly noticed on this patch, which I don't
> think are important enough for me to drop it from my pullreq,
> but which I think it would be nice to address in a followup:
>
> > diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h
> > index 4117211565..d6e3d85faf 100644
> > --- a/include/hw/i2c/i2c.h
> > +++ b/include/hw/i2c/i2c.h
> > @@ -80,6 +80,8 @@ int i2c_send(I2CBus *bus, uint8_t data);
> > uint8_t i2c_recv(I2CBus *bus);
> >
> > DeviceState *i2c_create_slave(I2CBus *bus, const char *name, uint8_t addr);
> > +DeviceState *i2c_try_create_slave(const char *name, uint8_t addr);
> > +bool i2c_realize_and_unref(DeviceState *dev, I2CBus *bus, Error **errp);
>
> Can we have doc-comments for new global-scope functions, please ?
Sure.
>
> > --- a/hw/i2c/core.c
> > +++ b/hw/i2c/core.c
> > @@ -267,13 +267,27 @@ const VMStateDescription vmstate_i2c_slave = {
> > }
> > };
> >
> > -DeviceState *i2c_create_slave(I2CBus *bus, const char *name, uint8_t addr)
> > +DeviceState *i2c_try_create_slave(const char *name, uint8_t addr)
> > {
> > DeviceState *dev;
> >
> > dev = qdev_new(name);
> > qdev_prop_set_uint8(dev, "address", addr);
> > - qdev_realize_and_unref(dev, &bus->qbus, &error_fatal);
> > + return dev;
> > +}
> > +
> > +bool i2c_realize_and_unref(DeviceState *dev, I2CBus *bus, Error **errp)
> > +{
> > + return qdev_realize_and_unref(dev, &bus->qbus, errp);
> > +}
> > +
> > +DeviceState *i2c_create_slave(I2CBus *bus, const char *name, uint8_t addr)
> > +{
> > + DeviceState *dev;
> > +
> > + dev = i2c_try_create_slave(name, addr);
> > + i2c_realize_and_unref(dev, bus, &error_fatal);
> > +
>
> We now have a _try_ function which isn't "same behaviour as
> the non-try function, but give me back an error status rather
> than just killing QEMU". That seems confusing -- is there a
> better name we can use ?
Yes, Markus asked me to change that, but as it could be done on top,
he didn't block the series.
I'll address that next week.
> thanks
> -- PMM
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v6 2/9] hw/misc/pca9552: Rename 'nr_leds' as 'pin_count'
2020-06-23 7:27 [PATCH v6 0/9] hw/misc/pca9552: Trace GPIO change events Philippe Mathieu-Daudé
2020-06-23 7:27 ` [PATCH v6 1/9] hw/i2c/core: Add i2c_try_create_slave() and i2c_realize_and_unref() Philippe Mathieu-Daudé
@ 2020-06-23 7:27 ` Philippe Mathieu-Daudé
2020-06-23 7:27 ` [PATCH v6 3/9] hw/misc/pca9552: Rename generic code as pca955x Philippe Mathieu-Daudé
` (7 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-23 7:27 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Corey Minyard, Andrew Jeffery,
Philippe Mathieu-Daudé, qemu-arm, Cédric Le Goater,
Joel Stanley
The PCA9552 device does not expose LEDs, but simple pins
to connnect LEDs to. To be clearer with the device model,
rename 'nr_leds' as 'pin_count'.
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/misc/pca9552.h | 2 +-
hw/misc/pca9552.c | 10 +++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/hw/misc/pca9552.h b/include/hw/misc/pca9552.h
index ebb43c63fe..bc5ed31087 100644
--- a/include/hw/misc/pca9552.h
+++ b/include/hw/misc/pca9552.h
@@ -26,7 +26,7 @@ typedef struct PCA9552State {
uint8_t regs[PCA9552_NR_REGS];
uint8_t max_reg;
- uint8_t nr_leds;
+ uint8_t pin_count;
} PCA9552State;
#endif
diff --git a/hw/misc/pca9552.c b/hw/misc/pca9552.c
index cac729e35a..81da757a7e 100644
--- a/hw/misc/pca9552.c
+++ b/hw/misc/pca9552.c
@@ -37,7 +37,7 @@ static void pca9552_update_pin_input(PCA9552State *s)
{
int i;
- for (i = 0; i < s->nr_leds; i++) {
+ for (i = 0; i < s->pin_count; i++) {
uint8_t input_reg = PCA9552_INPUT0 + (i / 8);
uint8_t input_shift = (i % 8);
uint8_t config = pca9552_pin_get_config(s, i);
@@ -185,7 +185,7 @@ static void pca9552_get_led(Object *obj, Visitor *v, const char *name,
error_setg(errp, "%s: error reading %s", __func__, name);
return;
}
- if (led < 0 || led > s->nr_leds) {
+ if (led < 0 || led > s->pin_count) {
error_setg(errp, "%s invalid led %s", __func__, name);
return;
}
@@ -228,7 +228,7 @@ static void pca9552_set_led(Object *obj, Visitor *v, const char *name,
error_setg(errp, "%s: error reading %s", __func__, name);
return;
}
- if (led < 0 || led > s->nr_leds) {
+ if (led < 0 || led > s->pin_count) {
error_setg(errp, "%s invalid led %s", __func__, name);
return;
}
@@ -291,9 +291,9 @@ static void pca9552_initfn(Object *obj)
* PCA955X device
*/
s->max_reg = PCA9552_LS3;
- s->nr_leds = 16;
+ s->pin_count = 16;
- for (led = 0; led < s->nr_leds; led++) {
+ for (led = 0; led < s->pin_count; led++) {
char *name;
name = g_strdup_printf("led%d", led);
--
2.21.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v6 3/9] hw/misc/pca9552: Rename generic code as pca955x
2020-06-23 7:27 [PATCH v6 0/9] hw/misc/pca9552: Trace GPIO change events Philippe Mathieu-Daudé
2020-06-23 7:27 ` [PATCH v6 1/9] hw/i2c/core: Add i2c_try_create_slave() and i2c_realize_and_unref() Philippe Mathieu-Daudé
2020-06-23 7:27 ` [PATCH v6 2/9] hw/misc/pca9552: Rename 'nr_leds' as 'pin_count' Philippe Mathieu-Daudé
@ 2020-06-23 7:27 ` Philippe Mathieu-Daudé
2020-06-23 7:27 ` [PATCH v6 4/9] hw/misc/pca9552: Add generic PCA955xClass, parent of TYPE_PCA9552 Philippe Mathieu-Daudé
` (6 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-23 7:27 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Corey Minyard, Andrew Jeffery,
Philippe Mathieu-Daudé, qemu-arm, Cédric Le Goater,
Joel Stanley
Various code from the PCA9552 device model is generic to the
PCA955X family. We'll split the generic code in a base class
in the next commit. To ease review, first do a dumb renaming.
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/misc/pca9552.h | 10 ++---
hw/misc/pca9552.c | 80 +++++++++++++++++++--------------------
2 files changed, 45 insertions(+), 45 deletions(-)
diff --git a/include/hw/misc/pca9552.h b/include/hw/misc/pca9552.h
index bc5ed31087..db527595a3 100644
--- a/include/hw/misc/pca9552.h
+++ b/include/hw/misc/pca9552.h
@@ -12,11 +12,11 @@
#include "hw/i2c/i2c.h"
#define TYPE_PCA9552 "pca9552"
-#define PCA9552(obj) OBJECT_CHECK(PCA9552State, (obj), TYPE_PCA9552)
+#define PCA955X(obj) OBJECT_CHECK(PCA955xState, (obj), TYPE_PCA9552)
-#define PCA9552_NR_REGS 10
+#define PCA955X_NR_REGS 10
-typedef struct PCA9552State {
+typedef struct PCA955xState {
/*< private >*/
I2CSlave i2c;
/*< public >*/
@@ -24,9 +24,9 @@ typedef struct PCA9552State {
uint8_t len;
uint8_t pointer;
- uint8_t regs[PCA9552_NR_REGS];
+ uint8_t regs[PCA955X_NR_REGS];
uint8_t max_reg;
uint8_t pin_count;
-} PCA9552State;
+} PCA955xState;
#endif
diff --git a/hw/misc/pca9552.c b/hw/misc/pca9552.c
index 81da757a7e..5681ff3b22 100644
--- a/hw/misc/pca9552.c
+++ b/hw/misc/pca9552.c
@@ -25,7 +25,7 @@
static const char *led_state[] = {"on", "off", "pwm0", "pwm1"};
-static uint8_t pca9552_pin_get_config(PCA9552State *s, int pin)
+static uint8_t pca955x_pin_get_config(PCA955xState *s, int pin)
{
uint8_t reg = PCA9552_LS0 + (pin / 4);
uint8_t shift = (pin % 4) << 1;
@@ -33,14 +33,14 @@ static uint8_t pca9552_pin_get_config(PCA9552State *s, int pin)
return extract32(s->regs[reg], shift, 2);
}
-static void pca9552_update_pin_input(PCA9552State *s)
+static void pca955x_update_pin_input(PCA955xState *s)
{
int i;
for (i = 0; i < s->pin_count; i++) {
uint8_t input_reg = PCA9552_INPUT0 + (i / 8);
uint8_t input_shift = (i % 8);
- uint8_t config = pca9552_pin_get_config(s, i);
+ uint8_t config = pca955x_pin_get_config(s, i);
switch (config) {
case PCA9552_LED_ON:
@@ -58,7 +58,7 @@ static void pca9552_update_pin_input(PCA9552State *s)
}
}
-static uint8_t pca9552_read(PCA9552State *s, uint8_t reg)
+static uint8_t pca955x_read(PCA955xState *s, uint8_t reg)
{
switch (reg) {
case PCA9552_INPUT0:
@@ -79,7 +79,7 @@ static uint8_t pca9552_read(PCA9552State *s, uint8_t reg)
}
}
-static void pca9552_write(PCA9552State *s, uint8_t reg, uint8_t data)
+static void pca955x_write(PCA955xState *s, uint8_t reg, uint8_t data)
{
switch (reg) {
case PCA9552_PSC0:
@@ -94,7 +94,7 @@ static void pca9552_write(PCA9552State *s, uint8_t reg, uint8_t data)
case PCA9552_LS2:
case PCA9552_LS3:
s->regs[reg] = data;
- pca9552_update_pin_input(s);
+ pca955x_update_pin_input(s);
break;
case PCA9552_INPUT0:
@@ -110,7 +110,7 @@ static void pca9552_write(PCA9552State *s, uint8_t reg, uint8_t data)
* after each byte is sent to or received by the device. The index
* rollovers to 0 when the maximum register address is reached.
*/
-static void pca9552_autoinc(PCA9552State *s)
+static void pca955x_autoinc(PCA955xState *s)
{
if (s->pointer != 0xFF && s->pointer & PCA9552_AUTOINC) {
uint8_t reg = s->pointer & 0xf;
@@ -120,12 +120,12 @@ static void pca9552_autoinc(PCA9552State *s)
}
}
-static uint8_t pca9552_recv(I2CSlave *i2c)
+static uint8_t pca955x_recv(I2CSlave *i2c)
{
- PCA9552State *s = PCA9552(i2c);
+ PCA955xState *s = PCA955X(i2c);
uint8_t ret;
- ret = pca9552_read(s, s->pointer & 0xf);
+ ret = pca955x_read(s, s->pointer & 0xf);
/*
* From the Specs:
@@ -143,40 +143,40 @@ static uint8_t pca9552_recv(I2CSlave *i2c)
__func__);
}
- pca9552_autoinc(s);
+ pca955x_autoinc(s);
return ret;
}
-static int pca9552_send(I2CSlave *i2c, uint8_t data)
+static int pca955x_send(I2CSlave *i2c, uint8_t data)
{
- PCA9552State *s = PCA9552(i2c);
+ PCA955xState *s = PCA955X(i2c);
/* First byte sent by is the register address */
if (s->len == 0) {
s->pointer = data;
s->len++;
} else {
- pca9552_write(s, s->pointer & 0xf, data);
+ pca955x_write(s, s->pointer & 0xf, data);
- pca9552_autoinc(s);
+ pca955x_autoinc(s);
}
return 0;
}
-static int pca9552_event(I2CSlave *i2c, enum i2c_event event)
+static int pca955x_event(I2CSlave *i2c, enum i2c_event event)
{
- PCA9552State *s = PCA9552(i2c);
+ PCA955xState *s = PCA955X(i2c);
s->len = 0;
return 0;
}
-static void pca9552_get_led(Object *obj, Visitor *v, const char *name,
+static void pca955x_get_led(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
- PCA9552State *s = PCA9552(obj);
+ PCA955xState *s = PCA955X(obj);
int led, rc, reg;
uint8_t state;
@@ -195,7 +195,7 @@ static void pca9552_get_led(Object *obj, Visitor *v, const char *name,
* reading the INPUTx reg
*/
reg = PCA9552_LS0 + led / 4;
- state = (pca9552_read(s, reg) >> (led % 8)) & 0x3;
+ state = (pca955x_read(s, reg) >> (led % 8)) & 0x3;
visit_type_str(v, name, (char **)&led_state[state], errp);
}
@@ -209,10 +209,10 @@ static inline uint8_t pca955x_ledsel(uint8_t oldval, int led_num, int state)
((state & 0x3) << (led_num << 1));
}
-static void pca9552_set_led(Object *obj, Visitor *v, const char *name,
+static void pca955x_set_led(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
- PCA9552State *s = PCA9552(obj);
+ PCA955xState *s = PCA955X(obj);
Error *local_err = NULL;
int led, rc, reg, val;
uint8_t state;
@@ -244,9 +244,9 @@ static void pca9552_set_led(Object *obj, Visitor *v, const char *name,
}
reg = PCA9552_LS0 + led / 4;
- val = pca9552_read(s, reg);
+ val = pca955x_read(s, reg);
val = pca955x_ledsel(val, led % 4, state);
- pca9552_write(s, reg, val);
+ pca955x_write(s, reg, val);
}
static const VMStateDescription pca9552_vmstate = {
@@ -254,17 +254,17 @@ static const VMStateDescription pca9552_vmstate = {
.version_id = 0,
.minimum_version_id = 0,
.fields = (VMStateField[]) {
- VMSTATE_UINT8(len, PCA9552State),
- VMSTATE_UINT8(pointer, PCA9552State),
- VMSTATE_UINT8_ARRAY(regs, PCA9552State, PCA9552_NR_REGS),
- VMSTATE_I2C_SLAVE(i2c, PCA9552State),
+ VMSTATE_UINT8(len, PCA955xState),
+ VMSTATE_UINT8(pointer, PCA955xState),
+ VMSTATE_UINT8_ARRAY(regs, PCA955xState, PCA955X_NR_REGS),
+ VMSTATE_I2C_SLAVE(i2c, PCA955xState),
VMSTATE_END_OF_LIST()
}
};
static void pca9552_reset(DeviceState *dev)
{
- PCA9552State *s = PCA9552(dev);
+ PCA955xState *s = PCA955X(dev);
s->regs[PCA9552_PSC0] = 0xFF;
s->regs[PCA9552_PWM0] = 0x80;
@@ -275,15 +275,15 @@ static void pca9552_reset(DeviceState *dev)
s->regs[PCA9552_LS2] = 0x55;
s->regs[PCA9552_LS3] = 0x55;
- pca9552_update_pin_input(s);
+ pca955x_update_pin_input(s);
s->pointer = 0xFF;
s->len = 0;
}
-static void pca9552_initfn(Object *obj)
+static void pca955x_initfn(Object *obj)
{
- PCA9552State *s = PCA9552(obj);
+ PCA955xState *s = PCA955X(obj);
int led;
/* If support for the other PCA955X devices are implemented, these
@@ -297,7 +297,7 @@ static void pca9552_initfn(Object *obj)
char *name;
name = g_strdup_printf("led%d", led);
- object_property_add(obj, name, "bool", pca9552_get_led, pca9552_set_led,
+ object_property_add(obj, name, "bool", pca955x_get_led, pca955x_set_led,
NULL, NULL);
g_free(name);
}
@@ -308,9 +308,9 @@ static void pca9552_class_init(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
- k->event = pca9552_event;
- k->recv = pca9552_recv;
- k->send = pca9552_send;
+ k->event = pca955x_event;
+ k->recv = pca955x_recv;
+ k->send = pca955x_send;
dc->reset = pca9552_reset;
dc->vmsd = &pca9552_vmstate;
}
@@ -318,14 +318,14 @@ static void pca9552_class_init(ObjectClass *klass, void *data)
static const TypeInfo pca9552_info = {
.name = TYPE_PCA9552,
.parent = TYPE_I2C_SLAVE,
- .instance_init = pca9552_initfn,
- .instance_size = sizeof(PCA9552State),
+ .instance_init = pca955x_initfn,
+ .instance_size = sizeof(PCA955xState),
.class_init = pca9552_class_init,
};
-static void pca9552_register_types(void)
+static void pca955x_register_types(void)
{
type_register_static(&pca9552_info);
}
-type_init(pca9552_register_types)
+type_init(pca955x_register_types)
--
2.21.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v6 4/9] hw/misc/pca9552: Add generic PCA955xClass, parent of TYPE_PCA9552
2020-06-23 7:27 [PATCH v6 0/9] hw/misc/pca9552: Trace GPIO change events Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2020-06-23 7:27 ` [PATCH v6 3/9] hw/misc/pca9552: Rename generic code as pca955x Philippe Mathieu-Daudé
@ 2020-06-23 7:27 ` Philippe Mathieu-Daudé
2020-06-23 7:27 ` [PATCH v6 5/9] hw/misc/pca9552: Add a 'description' property for debugging purpose Philippe Mathieu-Daudé
` (5 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-23 7:27 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Corey Minyard, Andrew Jeffery,
Philippe Mathieu-Daudé, qemu-arm, Cédric Le Goater,
Joel Stanley
Extract the code common to the PCA955x family in PCA955xClass,
keeping the PCA9552 specific parts into pca9552_class_init().
Remove the 'TODO' comment added in commit 5141d4158cf.
Suggested-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/misc/pca9552.h | 6 ++--
hw/misc/pca9552.c | 66 ++++++++++++++++++++++++++++-----------
2 files changed, 51 insertions(+), 21 deletions(-)
diff --git a/include/hw/misc/pca9552.h b/include/hw/misc/pca9552.h
index db527595a3..90843b03b8 100644
--- a/include/hw/misc/pca9552.h
+++ b/include/hw/misc/pca9552.h
@@ -12,9 +12,11 @@
#include "hw/i2c/i2c.h"
#define TYPE_PCA9552 "pca9552"
-#define PCA955X(obj) OBJECT_CHECK(PCA955xState, (obj), TYPE_PCA9552)
+#define TYPE_PCA955X "pca955x"
+#define PCA955X(obj) OBJECT_CHECK(PCA955xState, (obj), TYPE_PCA955X)
#define PCA955X_NR_REGS 10
+#define PCA955X_PIN_COUNT_MAX 16
typedef struct PCA955xState {
/*< private >*/
@@ -25,8 +27,6 @@ typedef struct PCA955xState {
uint8_t pointer;
uint8_t regs[PCA955X_NR_REGS];
- uint8_t max_reg;
- uint8_t pin_count;
} PCA955xState;
#endif
diff --git a/hw/misc/pca9552.c b/hw/misc/pca9552.c
index 5681ff3b22..4de57dbe2e 100644
--- a/hw/misc/pca9552.c
+++ b/hw/misc/pca9552.c
@@ -4,6 +4,7 @@
* https://www.nxp.com/docs/en/application-note/AN264.pdf
*
* Copyright (c) 2017-2018, IBM Corporation.
+ * Copyright (c) 2020 Philippe Mathieu-Daudé
*
* This work is licensed under the terms of the GNU GPL, version 2 or
* later. See the COPYING file in the top-level directory.
@@ -18,6 +19,20 @@
#include "qapi/error.h"
#include "qapi/visitor.h"
+typedef struct PCA955xClass {
+ /*< private >*/
+ I2CSlaveClass parent_class;
+ /*< public >*/
+
+ uint8_t pin_count;
+ uint8_t max_reg;
+} PCA955xClass;
+
+#define PCA955X_CLASS(klass) \
+ OBJECT_CLASS_CHECK(PCA955xClass, (klass), TYPE_PCA955X)
+#define PCA955X_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(PCA955xClass, (obj), TYPE_PCA955X)
+
#define PCA9552_LED_ON 0x0
#define PCA9552_LED_OFF 0x1
#define PCA9552_LED_PWM0 0x2
@@ -35,9 +50,10 @@ static uint8_t pca955x_pin_get_config(PCA955xState *s, int pin)
static void pca955x_update_pin_input(PCA955xState *s)
{
+ PCA955xClass *k = PCA955X_GET_CLASS(s);
int i;
- for (i = 0; i < s->pin_count; i++) {
+ for (i = 0; i < k->pin_count; i++) {
uint8_t input_reg = PCA9552_INPUT0 + (i / 8);
uint8_t input_shift = (i % 8);
uint8_t config = pca955x_pin_get_config(s, i);
@@ -112,10 +128,12 @@ static void pca955x_write(PCA955xState *s, uint8_t reg, uint8_t data)
*/
static void pca955x_autoinc(PCA955xState *s)
{
+ PCA955xClass *k = PCA955X_GET_CLASS(s);
+
if (s->pointer != 0xFF && s->pointer & PCA9552_AUTOINC) {
uint8_t reg = s->pointer & 0xf;
- reg = (reg + 1) % (s->max_reg + 1);
+ reg = (reg + 1) % (k->max_reg + 1);
s->pointer = reg | PCA9552_AUTOINC;
}
}
@@ -176,6 +194,7 @@ static int pca955x_event(I2CSlave *i2c, enum i2c_event event)
static void pca955x_get_led(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
+ PCA955xClass *k = PCA955X_GET_CLASS(obj);
PCA955xState *s = PCA955X(obj);
int led, rc, reg;
uint8_t state;
@@ -185,7 +204,7 @@ static void pca955x_get_led(Object *obj, Visitor *v, const char *name,
error_setg(errp, "%s: error reading %s", __func__, name);
return;
}
- if (led < 0 || led > s->pin_count) {
+ if (led < 0 || led > k->pin_count) {
error_setg(errp, "%s invalid led %s", __func__, name);
return;
}
@@ -212,6 +231,7 @@ static inline uint8_t pca955x_ledsel(uint8_t oldval, int led_num, int state)
static void pca955x_set_led(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
+ PCA955xClass *k = PCA955X_GET_CLASS(obj);
PCA955xState *s = PCA955X(obj);
Error *local_err = NULL;
int led, rc, reg, val;
@@ -228,7 +248,7 @@ static void pca955x_set_led(Object *obj, Visitor *v, const char *name,
error_setg(errp, "%s: error reading %s", __func__, name);
return;
}
- if (led < 0 || led > s->pin_count) {
+ if (led < 0 || led > k->pin_count) {
error_setg(errp, "%s invalid led %s", __func__, name);
return;
}
@@ -283,17 +303,11 @@ static void pca9552_reset(DeviceState *dev)
static void pca955x_initfn(Object *obj)
{
- PCA955xState *s = PCA955X(obj);
+ PCA955xClass *k = PCA955X_GET_CLASS(obj);
int led;
- /* If support for the other PCA955X devices are implemented, these
- * constant values might be part of class structure describing the
- * PCA955X device
- */
- s->max_reg = PCA9552_LS3;
- s->pin_count = 16;
-
- for (led = 0; led < s->pin_count; led++) {
+ assert(k->pin_count <= PCA955X_PIN_COUNT_MAX);
+ for (led = 0; led < k->pin_count; led++) {
char *name;
name = g_strdup_printf("led%d", led);
@@ -303,28 +317,44 @@ static void pca955x_initfn(Object *obj)
}
}
-static void pca9552_class_init(ObjectClass *klass, void *data)
+static void pca955x_class_init(ObjectClass *klass, void *data)
{
- DeviceClass *dc = DEVICE_CLASS(klass);
I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
k->event = pca955x_event;
k->recv = pca955x_recv;
k->send = pca955x_send;
+}
+
+static const TypeInfo pca955x_info = {
+ .name = TYPE_PCA955X,
+ .parent = TYPE_I2C_SLAVE,
+ .instance_init = pca955x_initfn,
+ .instance_size = sizeof(PCA955xState),
+ .class_init = pca955x_class_init,
+ .abstract = true,
+};
+
+static void pca9552_class_init(ObjectClass *oc, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(oc);
+ PCA955xClass *pc = PCA955X_CLASS(oc);
+
dc->reset = pca9552_reset;
dc->vmsd = &pca9552_vmstate;
+ pc->max_reg = PCA9552_LS3;
+ pc->pin_count = 16;
}
static const TypeInfo pca9552_info = {
.name = TYPE_PCA9552,
- .parent = TYPE_I2C_SLAVE,
- .instance_init = pca955x_initfn,
- .instance_size = sizeof(PCA955xState),
+ .parent = TYPE_PCA955X,
.class_init = pca9552_class_init,
};
static void pca955x_register_types(void)
{
+ type_register_static(&pca955x_info);
type_register_static(&pca9552_info);
}
--
2.21.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v6 5/9] hw/misc/pca9552: Add a 'description' property for debugging purpose
2020-06-23 7:27 [PATCH v6 0/9] hw/misc/pca9552: Trace GPIO change events Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2020-06-23 7:27 ` [PATCH v6 4/9] hw/misc/pca9552: Add generic PCA955xClass, parent of TYPE_PCA9552 Philippe Mathieu-Daudé
@ 2020-06-23 7:27 ` Philippe Mathieu-Daudé
2020-06-23 7:27 ` [PATCH v6 6/9] hw/misc/pca9552: Trace GPIO High/Low events Philippe Mathieu-Daudé
` (4 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-23 7:27 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Corey Minyard, Andrew Jeffery,
Philippe Mathieu-Daudé, qemu-arm, Cédric Le Goater,
Joel Stanley
Add a description field to distinguish between multiple devices.
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/misc/pca9552.h | 1 +
hw/misc/pca9552.c | 18 ++++++++++++++++++
2 files changed, 19 insertions(+)
diff --git a/include/hw/misc/pca9552.h b/include/hw/misc/pca9552.h
index 90843b03b8..bf1a589137 100644
--- a/include/hw/misc/pca9552.h
+++ b/include/hw/misc/pca9552.h
@@ -27,6 +27,7 @@ typedef struct PCA955xState {
uint8_t pointer;
uint8_t regs[PCA955X_NR_REGS];
+ char *description; /* For debugging purpose only */
} PCA955xState;
#endif
diff --git a/hw/misc/pca9552.c b/hw/misc/pca9552.c
index 4de57dbe2e..2cc52b0205 100644
--- a/hw/misc/pca9552.c
+++ b/hw/misc/pca9552.c
@@ -13,6 +13,7 @@
#include "qemu/osdep.h"
#include "qemu/log.h"
#include "qemu/module.h"
+#include "hw/qdev-properties.h"
#include "hw/misc/pca9552.h"
#include "hw/misc/pca9552_regs.h"
#include "migration/vmstate.h"
@@ -317,13 +318,30 @@ static void pca955x_initfn(Object *obj)
}
}
+static void pca955x_realize(DeviceState *dev, Error **errp)
+{
+ PCA955xState *s = PCA955X(dev);
+
+ if (!s->description) {
+ s->description = g_strdup("pca-unspecified");
+ }
+}
+
+static Property pca955x_properties[] = {
+ DEFINE_PROP_STRING("description", PCA955xState, description),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
static void pca955x_class_init(ObjectClass *klass, void *data)
{
+ DeviceClass *dc = DEVICE_CLASS(klass);
I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
k->event = pca955x_event;
k->recv = pca955x_recv;
k->send = pca955x_send;
+ dc->realize = pca955x_realize;
+ device_class_set_props(dc, pca955x_properties);
}
static const TypeInfo pca955x_info = {
--
2.21.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v6 6/9] hw/misc/pca9552: Trace GPIO High/Low events
2020-06-23 7:27 [PATCH v6 0/9] hw/misc/pca9552: Trace GPIO change events Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2020-06-23 7:27 ` [PATCH v6 5/9] hw/misc/pca9552: Add a 'description' property for debugging purpose Philippe Mathieu-Daudé
@ 2020-06-23 7:27 ` Philippe Mathieu-Daudé
2020-06-23 7:27 ` [PATCH v6 7/9] hw/arm/aspeed: Describe each PCA9552 device Philippe Mathieu-Daudé
` (3 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-23 7:27 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Corey Minyard, Andrew Jeffery,
Philippe Mathieu-Daudé, qemu-arm, Cédric Le Goater,
Joel Stanley
Add a trivial representation of the PCA9552 GPIOs.
Example booting obmc-phosphor-image:
$ qemu-system-arm -M witherspoon-bmc -trace pca955x_gpio_status
1592689902.327837:pca955x_gpio_status pca-unspecified GPIOs 0-15 [*...............]
1592689902.329934:pca955x_gpio_status pca-unspecified GPIOs 0-15 [**..............]
1592689902.330717:pca955x_gpio_status pca-unspecified GPIOs 0-15 [***.............]
1592689902.331431:pca955x_gpio_status pca-unspecified GPIOs 0-15 [****............]
1592689902.332163:pca955x_gpio_status pca-unspecified GPIOs 0-15 [****.........*..]
1592689902.332888:pca955x_gpio_status pca-unspecified GPIOs 0-15 [****.........**.]
1592689902.333629:pca955x_gpio_status pca-unspecified GPIOs 0-15 [****.........***]
1592690032.793289:pca955x_gpio_status pca-unspecified GPIOs 0-15 [****.........*.*]
1592690033.303163:pca955x_gpio_status pca-unspecified GPIOs 0-15 [****.........***]
1592690033.812962:pca955x_gpio_status pca-unspecified GPIOs 0-15 [****.........*.*]
1592690034.323234:pca955x_gpio_status pca-unspecified GPIOs 0-15 [****.........***]
1592690034.832922:pca955x_gpio_status pca-unspecified GPIOs 0-15 [****.........*.*]
We notice the GPIO #14 (front-power LED) starts to blink.
This LED is described in the witherspoon device-tree [*]:
front-power {
retain-state-shutdown;
default-state = "keep";
gpios = <&pca0 14 GPIO_ACTIVE_LOW>;
};
[*] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts?id=b1f9be9392f0#n140
Suggested-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/misc/pca9552.c | 39 +++++++++++++++++++++++++++++++++++++++
hw/misc/trace-events | 3 +++
2 files changed, 42 insertions(+)
diff --git a/hw/misc/pca9552.c b/hw/misc/pca9552.c
index 2cc52b0205..41f8ad213d 100644
--- a/hw/misc/pca9552.c
+++ b/hw/misc/pca9552.c
@@ -13,12 +13,14 @@
#include "qemu/osdep.h"
#include "qemu/log.h"
#include "qemu/module.h"
+#include "qemu/bitops.h"
#include "hw/qdev-properties.h"
#include "hw/misc/pca9552.h"
#include "hw/misc/pca9552_regs.h"
#include "migration/vmstate.h"
#include "qapi/error.h"
#include "qapi/visitor.h"
+#include "trace.h"
typedef struct PCA955xClass {
/*< private >*/
@@ -49,6 +51,39 @@ static uint8_t pca955x_pin_get_config(PCA955xState *s, int pin)
return extract32(s->regs[reg], shift, 2);
}
+/* Return INPUT status (bit #N belongs to GPIO #N) */
+static uint16_t pca955x_pins_get_status(PCA955xState *s)
+{
+ return (s->regs[PCA9552_INPUT1] << 8) | s->regs[PCA9552_INPUT0];
+}
+
+static void pca955x_display_pins_status(PCA955xState *s,
+ uint16_t previous_pins_status)
+{
+ PCA955xClass *k = PCA955X_GET_CLASS(s);
+ uint16_t pins_status, pins_changed;
+ int i;
+
+ pins_status = pca955x_pins_get_status(s);
+ pins_changed = previous_pins_status ^ pins_status;
+ if (!pins_changed) {
+ return;
+ }
+ if (trace_event_get_state_backends(TRACE_PCA955X_GPIO_STATUS)) {
+ char *buf = g_newa(char, k->pin_count + 1);
+
+ for (i = 0; i < k->pin_count; i++) {
+ if (extract32(pins_status, i, 1)) {
+ buf[i] = '*';
+ } else {
+ buf[i] = '.';
+ }
+ }
+ buf[i] = '\0';
+ trace_pca955x_gpio_status(s->description, buf);
+ }
+}
+
static void pca955x_update_pin_input(PCA955xState *s)
{
PCA955xClass *k = PCA955X_GET_CLASS(s);
@@ -98,6 +133,8 @@ static uint8_t pca955x_read(PCA955xState *s, uint8_t reg)
static void pca955x_write(PCA955xState *s, uint8_t reg, uint8_t data)
{
+ uint16_t pins_status;
+
switch (reg) {
case PCA9552_PSC0:
case PCA9552_PWM0:
@@ -110,8 +147,10 @@ static void pca955x_write(PCA955xState *s, uint8_t reg, uint8_t data)
case PCA9552_LS1:
case PCA9552_LS2:
case PCA9552_LS3:
+ pins_status = pca955x_pins_get_status(s);
s->regs[reg] = data;
pca955x_update_pin_input(s);
+ pca955x_display_pins_status(s, pins_status);
break;
case PCA9552_INPUT0:
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 5561746866..9282c60dd9 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -206,3 +206,6 @@ via1_rtc_cmd_pram_sect_write(int sector, int offset, int addr, int value) "secto
# grlib_ahb_apb_pnp.c
grlib_ahb_pnp_read(uint64_t addr, uint32_t value) "AHB PnP read addr:0x%03"PRIx64" data:0x%08x"
grlib_apb_pnp_read(uint64_t addr, uint32_t value) "APB PnP read addr:0x%03"PRIx64" data:0x%08x"
+
+# pca9552.c
+pca955x_gpio_status(const char *description, const char *buf) "%s GPIOs 0-15 [%s]"
--
2.21.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v6 7/9] hw/arm/aspeed: Describe each PCA9552 device
2020-06-23 7:27 [PATCH v6 0/9] hw/misc/pca9552: Trace GPIO change events Philippe Mathieu-Daudé
` (5 preceding siblings ...)
2020-06-23 7:27 ` [PATCH v6 6/9] hw/misc/pca9552: Trace GPIO High/Low events Philippe Mathieu-Daudé
@ 2020-06-23 7:27 ` Philippe Mathieu-Daudé
2020-06-23 11:08 ` Philippe Mathieu-Daudé
2020-06-23 16:30 ` Corey Minyard
2020-06-23 7:27 ` [PATCH v6 8/9] hw/misc/pca9552: Trace GPIO change events Philippe Mathieu-Daudé
` (2 subsequent siblings)
9 siblings, 2 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-23 7:27 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Corey Minyard, Andrew Jeffery,
Philippe Mathieu-Daudé, Markus Armbruster, qemu-arm,
Cédric Le Goater, Joel Stanley
We have 2 distinct PCA9552 devices. Set their description
to distinguish them when looking at the trace events.
Description name taken from:
https://github.com/open-power/witherspoon-xml/blob/master/witherspoon.xml
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Corey Minyard <cminyard@mvista.com>
---
hw/arm/aspeed.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index ccf127b328..307dba5065 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -508,12 +508,15 @@ static void witherspoon_bmc_i2c_init(AspeedMachineState *bmc)
{
AspeedSoCState *soc = &bmc->soc;
uint8_t *eeprom_buf = g_malloc0(8 * 1024);
+ DeviceState *dev;
/* Bus 3: TODO bmp280@77 */
/* Bus 3: TODO max31785@52 */
/* Bus 3: TODO dps310@76 */
- i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 3), TYPE_PCA9552,
- 0x60);
+ dev = i2c_try_create_slave(TYPE_PCA9552, 0x60);
+ qdev_prop_set_string(dev, "description", "pca1");
+ i2c_realize_and_unref(dev, aspeed_i2c_get_bus(DEVICE(&soc->i2c), 3),
+ &error_fatal);
i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 4), "tmp423", 0x4c);
i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 5), "tmp423", 0x4c);
@@ -528,8 +531,10 @@ static void witherspoon_bmc_i2c_init(AspeedMachineState *bmc)
smbus_eeprom_init_one(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 11), 0x51,
eeprom_buf);
- i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 11), TYPE_PCA9552,
- 0x60);
+ dev = i2c_try_create_slave(TYPE_PCA9552, 0x60);
+ qdev_prop_set_string(dev, "description", "pca0");
+ i2c_realize_and_unref(dev, aspeed_i2c_get_bus(DEVICE(&soc->i2c), 11),
+ &error_fatal);
/* Bus 11: TODO ucd90160@64 */
}
--
2.21.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v6 7/9] hw/arm/aspeed: Describe each PCA9552 device
2020-06-23 7:27 ` [PATCH v6 7/9] hw/arm/aspeed: Describe each PCA9552 device Philippe Mathieu-Daudé
@ 2020-06-23 11:08 ` Philippe Mathieu-Daudé
2020-06-23 16:30 ` Corey Minyard
1 sibling, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-23 11:08 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Corey Minyard, Andrew Jeffery, Markus Armbruster,
qemu-arm, Cédric Le Goater, Joel Stanley
On 6/23/20 9:27 AM, Philippe Mathieu-Daudé wrote:
> We have 2 distinct PCA9552 devices. Set their description
> to distinguish them when looking at the trace events.
>
> Description name taken from:
> https://github.com/open-power/witherspoon-xml/blob/master/witherspoon.xml
>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Collecting Markus review on v5:
Reviewed-by: Markus Armbruster <armbru@redhat.com>
https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg07078.html
> ---
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Corey Minyard <cminyard@mvista.com>
> ---
> hw/arm/aspeed.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index ccf127b328..307dba5065 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -508,12 +508,15 @@ static void witherspoon_bmc_i2c_init(AspeedMachineState *bmc)
> {
> AspeedSoCState *soc = &bmc->soc;
> uint8_t *eeprom_buf = g_malloc0(8 * 1024);
> + DeviceState *dev;
>
> /* Bus 3: TODO bmp280@77 */
> /* Bus 3: TODO max31785@52 */
> /* Bus 3: TODO dps310@76 */
> - i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 3), TYPE_PCA9552,
> - 0x60);
> + dev = i2c_try_create_slave(TYPE_PCA9552, 0x60);
> + qdev_prop_set_string(dev, "description", "pca1");
> + i2c_realize_and_unref(dev, aspeed_i2c_get_bus(DEVICE(&soc->i2c), 3),
> + &error_fatal);
>
> i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 4), "tmp423", 0x4c);
> i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 5), "tmp423", 0x4c);
> @@ -528,8 +531,10 @@ static void witherspoon_bmc_i2c_init(AspeedMachineState *bmc)
>
> smbus_eeprom_init_one(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 11), 0x51,
> eeprom_buf);
> - i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 11), TYPE_PCA9552,
> - 0x60);
> + dev = i2c_try_create_slave(TYPE_PCA9552, 0x60);
> + qdev_prop_set_string(dev, "description", "pca0");
> + i2c_realize_and_unref(dev, aspeed_i2c_get_bus(DEVICE(&soc->i2c), 11),
> + &error_fatal);
> /* Bus 11: TODO ucd90160@64 */
> }
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v6 7/9] hw/arm/aspeed: Describe each PCA9552 device
2020-06-23 7:27 ` [PATCH v6 7/9] hw/arm/aspeed: Describe each PCA9552 device Philippe Mathieu-Daudé
2020-06-23 11:08 ` Philippe Mathieu-Daudé
@ 2020-06-23 16:30 ` Corey Minyard
1 sibling, 0 replies; 18+ messages in thread
From: Corey Minyard @ 2020-06-23 16:30 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Andrew Jeffery, qemu-devel, Markus Armbruster,
qemu-arm, Joel Stanley, Cédric Le Goater
On Tue, Jun 23, 2020 at 09:27:21AM +0200, Philippe Mathieu-Daudé wrote:
> We have 2 distinct PCA9552 devices. Set their description
> to distinguish them when looking at the trace events.
>
> Description name taken from:
> https://github.com/open-power/witherspoon-xml/blob/master/witherspoon.xml
I forgot to respond to this earlier.
Reviewed-by: Corey Minyard <cminyard@mvista.com>
>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Corey Minyard <cminyard@mvista.com>
> ---
> hw/arm/aspeed.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index ccf127b328..307dba5065 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -508,12 +508,15 @@ static void witherspoon_bmc_i2c_init(AspeedMachineState *bmc)
> {
> AspeedSoCState *soc = &bmc->soc;
> uint8_t *eeprom_buf = g_malloc0(8 * 1024);
> + DeviceState *dev;
>
> /* Bus 3: TODO bmp280@77 */
> /* Bus 3: TODO max31785@52 */
> /* Bus 3: TODO dps310@76 */
> - i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 3), TYPE_PCA9552,
> - 0x60);
> + dev = i2c_try_create_slave(TYPE_PCA9552, 0x60);
> + qdev_prop_set_string(dev, "description", "pca1");
> + i2c_realize_and_unref(dev, aspeed_i2c_get_bus(DEVICE(&soc->i2c), 3),
> + &error_fatal);
>
> i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 4), "tmp423", 0x4c);
> i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 5), "tmp423", 0x4c);
> @@ -528,8 +531,10 @@ static void witherspoon_bmc_i2c_init(AspeedMachineState *bmc)
>
> smbus_eeprom_init_one(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 11), 0x51,
> eeprom_buf);
> - i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 11), TYPE_PCA9552,
> - 0x60);
> + dev = i2c_try_create_slave(TYPE_PCA9552, 0x60);
> + qdev_prop_set_string(dev, "description", "pca0");
> + i2c_realize_and_unref(dev, aspeed_i2c_get_bus(DEVICE(&soc->i2c), 11),
> + &error_fatal);
> /* Bus 11: TODO ucd90160@64 */
> }
>
> --
> 2.21.3
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v6 8/9] hw/misc/pca9552: Trace GPIO change events
2020-06-23 7:27 [PATCH v6 0/9] hw/misc/pca9552: Trace GPIO change events Philippe Mathieu-Daudé
` (6 preceding siblings ...)
2020-06-23 7:27 ` [PATCH v6 7/9] hw/arm/aspeed: Describe each PCA9552 device Philippe Mathieu-Daudé
@ 2020-06-23 7:27 ` Philippe Mathieu-Daudé
2020-06-23 7:27 ` [PATCH v6 9/9] hw/misc/pca9552: Model qdev output GPIOs Philippe Mathieu-Daudé
2020-06-23 8:14 ` [PATCH v6 0/9] hw/misc/pca9552: Trace GPIO change events Cédric Le Goater
9 siblings, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-23 7:27 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Corey Minyard, Andrew Jeffery,
Philippe Mathieu-Daudé, qemu-arm, Cédric Le Goater,
Joel Stanley
Emit a trace event when a GPIO change its state.
Example booting obmc-phosphor-image:
$ qemu-system-arm -M witherspoon-bmc -trace pca955x_gpio_change
1592690552.687372:pca955x_gpio_change pca1 GPIO id:0 status: 0 -> 1
1592690552.690169:pca955x_gpio_change pca1 GPIO id:1 status: 0 -> 1
1592690552.691673:pca955x_gpio_change pca1 GPIO id:2 status: 0 -> 1
1592690552.696886:pca955x_gpio_change pca1 GPIO id:3 status: 0 -> 1
1592690552.698614:pca955x_gpio_change pca1 GPIO id:13 status: 0 -> 1
1592690552.699833:pca955x_gpio_change pca1 GPIO id:14 status: 0 -> 1
1592690552.700842:pca955x_gpio_change pca1 GPIO id:15 status: 0 -> 1
1592690683.841921:pca955x_gpio_change pca1 GPIO id:14 status: 1 -> 0
1592690683.861660:pca955x_gpio_change pca1 GPIO id:14 status: 0 -> 1
1592690684.371460:pca955x_gpio_change pca1 GPIO id:14 status: 1 -> 0
1592690684.882115:pca955x_gpio_change pca1 GPIO id:14 status: 0 -> 1
1592690685.391411:pca955x_gpio_change pca1 GPIO id:14 status: 1 -> 0
1592690685.901391:pca955x_gpio_change pca1 GPIO id:14 status: 0 -> 1
1592690686.411678:pca955x_gpio_change pca1 GPIO id:14 status: 1 -> 0
1592690686.921279:pca955x_gpio_change pca1 GPIO id:14 status: 0 -> 1
We notice the GPIO #14 (front-power LED) starts to blink.
This LED is described in the witherspoon device-tree [*]:
front-power {
retain-state-shutdown;
default-state = "keep";
gpios = <&pca0 14 GPIO_ACTIVE_LOW>;
};
[*] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts?id=b1f9be9392f0#n140
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/misc/pca9552.c | 15 +++++++++++++++
hw/misc/trace-events | 1 +
2 files changed, 16 insertions(+)
diff --git a/hw/misc/pca9552.c b/hw/misc/pca9552.c
index 41f8ad213d..1c3ad57432 100644
--- a/hw/misc/pca9552.c
+++ b/hw/misc/pca9552.c
@@ -82,6 +82,21 @@ static void pca955x_display_pins_status(PCA955xState *s,
buf[i] = '\0';
trace_pca955x_gpio_status(s->description, buf);
}
+ if (trace_event_get_state_backends(TRACE_PCA955X_GPIO_CHANGE)) {
+ for (i = 0; i < k->pin_count; i++) {
+ if (extract32(pins_changed, i, 1)) {
+ unsigned new_state = extract32(pins_status, i, 1);
+
+ /*
+ * We display the state using the PCA logic ("active-high").
+ * This is not the state of the LED, which signal might be
+ * wired "active-low" on the board.
+ */
+ trace_pca955x_gpio_change(s->description, i,
+ !new_state, new_state);
+ }
+ }
+ }
}
static void pca955x_update_pin_input(PCA955xState *s)
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 9282c60dd9..7ccf683dd1 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -209,3 +209,4 @@ grlib_apb_pnp_read(uint64_t addr, uint32_t value) "APB PnP read addr:0x%03"PRIx6
# pca9552.c
pca955x_gpio_status(const char *description, const char *buf) "%s GPIOs 0-15 [%s]"
+pca955x_gpio_change(const char *description, unsigned id, unsigned prev_state, unsigned current_state) "%s GPIO id:%u status: %u -> %u"
--
2.21.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v6 9/9] hw/misc/pca9552: Model qdev output GPIOs
2020-06-23 7:27 [PATCH v6 0/9] hw/misc/pca9552: Trace GPIO change events Philippe Mathieu-Daudé
` (7 preceding siblings ...)
2020-06-23 7:27 ` [PATCH v6 8/9] hw/misc/pca9552: Trace GPIO change events Philippe Mathieu-Daudé
@ 2020-06-23 7:27 ` Philippe Mathieu-Daudé
2020-06-23 8:14 ` [PATCH v6 0/9] hw/misc/pca9552: Trace GPIO change events Cédric Le Goater
9 siblings, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-23 7:27 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Corey Minyard, Andrew Jeffery,
Philippe Mathieu-Daudé, qemu-arm, Cédric Le Goater,
Joel Stanley
The PCA9552 has 16 GPIOs which can be used as input,
output or PWM mode. QEMU models the output GPIO with
the qemu_irq type. Let the device expose the 16 GPIOs
to allow us to later connect LEDs to these outputs.
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/misc/pca9552.h | 1 +
hw/misc/pca9552.c | 6 ++++++
2 files changed, 7 insertions(+)
diff --git a/include/hw/misc/pca9552.h b/include/hw/misc/pca9552.h
index bf1a589137..600356fbf9 100644
--- a/include/hw/misc/pca9552.h
+++ b/include/hw/misc/pca9552.h
@@ -27,6 +27,7 @@ typedef struct PCA955xState {
uint8_t pointer;
uint8_t regs[PCA955X_NR_REGS];
+ qemu_irq gpio[PCA955X_PIN_COUNT_MAX];
char *description; /* For debugging purpose only */
} PCA955xState;
diff --git a/hw/misc/pca9552.c b/hw/misc/pca9552.c
index 1c3ad57432..80caa9ec8f 100644
--- a/hw/misc/pca9552.c
+++ b/hw/misc/pca9552.c
@@ -17,6 +17,7 @@
#include "hw/qdev-properties.h"
#include "hw/misc/pca9552.h"
#include "hw/misc/pca9552_regs.h"
+#include "hw/irq.h"
#include "migration/vmstate.h"
#include "qapi/error.h"
#include "qapi/visitor.h"
@@ -111,9 +112,11 @@ static void pca955x_update_pin_input(PCA955xState *s)
switch (config) {
case PCA9552_LED_ON:
+ qemu_set_irq(s->gpio[i], 1);
s->regs[input_reg] |= 1 << input_shift;
break;
case PCA9552_LED_OFF:
+ qemu_set_irq(s->gpio[i], 0);
s->regs[input_reg] &= ~(1 << input_shift);
break;
case PCA9552_LED_PWM0:
@@ -374,11 +377,14 @@ static void pca955x_initfn(Object *obj)
static void pca955x_realize(DeviceState *dev, Error **errp)
{
+ PCA955xClass *k = PCA955X_GET_CLASS(dev);
PCA955xState *s = PCA955X(dev);
if (!s->description) {
s->description = g_strdup("pca-unspecified");
}
+
+ qdev_init_gpio_out(dev, s->gpio, k->pin_count);
}
static Property pca955x_properties[] = {
--
2.21.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v6 0/9] hw/misc/pca9552: Trace GPIO change events
2020-06-23 7:27 [PATCH v6 0/9] hw/misc/pca9552: Trace GPIO change events Philippe Mathieu-Daudé
` (8 preceding siblings ...)
2020-06-23 7:27 ` [PATCH v6 9/9] hw/misc/pca9552: Model qdev output GPIOs Philippe Mathieu-Daudé
@ 2020-06-23 8:14 ` Cédric Le Goater
2020-06-23 11:09 ` Philippe Mathieu-Daudé
9 siblings, 1 reply; 18+ messages in thread
From: Cédric Le Goater @ 2020-06-23 8:14 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Andrew Jeffery, Corey Minyard, qemu-arm, Joel Stanley,
Peter Maydell
On 6/23/20 9:27 AM, Philippe Mathieu-Daudé wrote:
> This series add trace events to better display GPIO changes.
> We'll continue in the following series by connecting LEDs to
> these GPIOs.
>
> This helps me to work on a generic LED device, see:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg711917.html
Tested-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
>
> Since v5, addressed Cédric review comment:
> - Move pin_count check from realize() to instance_init()
>
> Since v4: Addressed Cédric review comments
> - Extract PCA955xClass
> - Add/use pca955x_pins_get_status() method instead of keeping
> cached value in PCA955xState
>
> Example when booting an obmc-phosphor-image, we can see the LED #14
> (front-power LED) starting to blink.
>
> - ASCII LED bar view:
>
> $ qemu-system-arm -M witherspoon-bmc -trace pca955x_gpio_status
> 1592689902.327837:pca955x_gpio_status pca-unspecified GPIOs 0-15 [*...............]
> 1592689902.329934:pca955x_gpio_status pca-unspecified GPIOs 0-15 [**..............]
> 1592689902.330717:pca955x_gpio_status pca-unspecified GPIOs 0-15 [***.............]
> 1592689902.331431:pca955x_gpio_status pca-unspecified GPIOs 0-15 [****............]
> 1592689902.332163:pca955x_gpio_status pca-unspecified GPIOs 0-15 [****.........*..]
> 1592689902.332888:pca955x_gpio_status pca-unspecified GPIOs 0-15 [****.........**.]
> 1592689902.333629:pca955x_gpio_status pca-unspecified GPIOs 0-15 [****.........***]
> 1592690032.793289:pca955x_gpio_status pca-unspecified GPIOs 0-15 [****.........*.*]
> 1592690033.303163:pca955x_gpio_status pca-unspecified GPIOs 0-15 [****.........***]
> 1592690033.812962:pca955x_gpio_status pca-unspecified GPIOs 0-15 [****.........*.*]
> 1592690034.323234:pca955x_gpio_status pca-unspecified GPIOs 0-15 [****.........***]
> 1592690034.832922:pca955x_gpio_status pca-unspecified GPIOs 0-15 [****.........*.*]
>
> - Only display GPIOs which status changes:
>
> $ qemu-system-arm -M witherspoon-bmc -trace pca955x_gpio_change
> 1592690552.687372:pca955x_gpio_change pca1 GPIO id:0 status: 0 -> 1
> 1592690552.690169:pca955x_gpio_change pca1 GPIO id:1 status: 0 -> 1
> 1592690552.691673:pca955x_gpio_change pca1 GPIO id:2 status: 0 -> 1
> 1592690552.696886:pca955x_gpio_change pca1 GPIO id:3 status: 0 -> 1
> 1592690552.698614:pca955x_gpio_change pca1 GPIO id:13 status: 0 -> 1
> 1592690552.699833:pca955x_gpio_change pca1 GPIO id:14 status: 0 -> 1
> 1592690552.700842:pca955x_gpio_change pca1 GPIO id:15 status: 0 -> 1
> 1592690683.841921:pca955x_gpio_change pca1 GPIO id:14 status: 1 -> 0
> 1592690683.861660:pca955x_gpio_change pca1 GPIO id:14 status: 0 -> 1
> 1592690684.371460:pca955x_gpio_change pca1 GPIO id:14 status: 1 -> 0
> 1592690684.882115:pca955x_gpio_change pca1 GPIO id:14 status: 0 -> 1
> 1592690685.391411:pca955x_gpio_change pca1 GPIO id:14 status: 1 -> 0
> 1592690685.901391:pca955x_gpio_change pca1 GPIO id:14 status: 0 -> 1
>
> For information about how to test the obmc-phosphor-image, see:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg712911.html
>
> $ git backport-diff -u v5
> Key:
> [----] : patches are identical
> [####] : number of functional differences between upstream/downstream patch
> [down] : patch is downstream-only
> The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
>
> 001/9:[----] [--] 'hw/i2c/core: Add i2c_try_create_slave() and i2c_realize_and_unref()'
> 002/9:[----] [--] 'hw/misc/pca9552: Rename 'nr_leds' as 'pin_count''
> 003/9:[----] [--] 'hw/misc/pca9552: Rename generic code as pca955x'
> 004/9:[0010] [FC] 'hw/misc/pca9552: Add generic PCA955xClass, parent of TYPE_PCA9552'
> 005/9:[0007] [FC] 'hw/misc/pca9552: Add a 'description' property for debugging purpose'
> 006/9:[----] [--] 'hw/misc/pca9552: Trace GPIO High/Low events'
> 007/9:[----] [-C] 'hw/arm/aspeed: Describe each PCA9552 device'
> 008/9:[----] [--] 'hw/misc/pca9552: Trace GPIO change events'
> 009/9:[0003] [FC] 'hw/misc/pca9552: Model qdev output GPIOs'
>
> Based-on: <20200623072132.2868-1-f4bug@amsat.org>
>
> Philippe Mathieu-Daudé (9):
> hw/i2c/core: Add i2c_try_create_slave() and i2c_realize_and_unref()
> hw/misc/pca9552: Rename 'nr_leds' as 'pin_count'
> hw/misc/pca9552: Rename generic code as pca955x
> hw/misc/pca9552: Add generic PCA955xClass, parent of TYPE_PCA9552
> hw/misc/pca9552: Add a 'description' property for debugging purpose
> hw/misc/pca9552: Trace GPIO High/Low events
> hw/arm/aspeed: Describe each PCA9552 device
> hw/misc/pca9552: Trace GPIO change events
> hw/misc/pca9552: Model qdev output GPIOs
>
> include/hw/i2c/i2c.h | 2 +
> include/hw/misc/pca9552.h | 16 +--
> hw/arm/aspeed.c | 13 ++-
> hw/i2c/core.c | 18 +++-
> hw/misc/pca9552.c | 216 ++++++++++++++++++++++++++++----------
> hw/misc/trace-events | 4 +
> 6 files changed, 202 insertions(+), 67 deletions(-)
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v6 0/9] hw/misc/pca9552: Trace GPIO change events
2020-06-23 8:14 ` [PATCH v6 0/9] hw/misc/pca9552: Trace GPIO change events Cédric Le Goater
@ 2020-06-23 11:09 ` Philippe Mathieu-Daudé
2020-06-25 15:53 ` Peter Maydell
0 siblings, 1 reply; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-23 11:09 UTC (permalink / raw)
To: Cédric Le Goater, qemu-devel
Cc: Andrew Jeffery, Corey Minyard, qemu-arm, Joel Stanley,
Peter Maydell
On 6/23/20 10:14 AM, Cédric Le Goater wrote:
> On 6/23/20 9:27 AM, Philippe Mathieu-Daudé wrote:
>> This series add trace events to better display GPIO changes.
>> We'll continue in the following series by connecting LEDs to
>> these GPIOs.
>>
>> This helps me to work on a generic LED device, see:
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg711917.html
>
>
> Tested-by: Cédric Le Goater <clg@kaod.org>
Thanks :)
Note to the maintainer, this series is now fully reviewed/tested.
>>
>> Based-on: <20200623072132.2868-1-f4bug@amsat.org>
>>
>> Philippe Mathieu-Daudé (9):
>> hw/i2c/core: Add i2c_try_create_slave() and i2c_realize_and_unref()
>> hw/misc/pca9552: Rename 'nr_leds' as 'pin_count'
>> hw/misc/pca9552: Rename generic code as pca955x
>> hw/misc/pca9552: Add generic PCA955xClass, parent of TYPE_PCA9552
>> hw/misc/pca9552: Add a 'description' property for debugging purpose
>> hw/misc/pca9552: Trace GPIO High/Low events
>> hw/arm/aspeed: Describe each PCA9552 device
>> hw/misc/pca9552: Trace GPIO change events
>> hw/misc/pca9552: Model qdev output GPIOs
>>
>> include/hw/i2c/i2c.h | 2 +
>> include/hw/misc/pca9552.h | 16 +--
>> hw/arm/aspeed.c | 13 ++-
>> hw/i2c/core.c | 18 +++-
>> hw/misc/pca9552.c | 216 ++++++++++++++++++++++++++++----------
>> hw/misc/trace-events | 4 +
>> 6 files changed, 202 insertions(+), 67 deletions(-)
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v6 0/9] hw/misc/pca9552: Trace GPIO change events
2020-06-23 11:09 ` Philippe Mathieu-Daudé
@ 2020-06-25 15:53 ` Peter Maydell
0 siblings, 0 replies; 18+ messages in thread
From: Peter Maydell @ 2020-06-25 15:53 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Corey Minyard, Andrew Jeffery, QEMU Developers, qemu-arm,
Joel Stanley, Cédric Le Goater
On Tue, 23 Jun 2020 at 12:09, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 6/23/20 10:14 AM, Cédric Le Goater wrote:
> > On 6/23/20 9:27 AM, Philippe Mathieu-Daudé wrote:
> >> This series add trace events to better display GPIO changes.
> >> We'll continue in the following series by connecting LEDs to
> >> these GPIOs.
> >>
> >> This helps me to work on a generic LED device, see:
> >> https://www.mail-archive.com/qemu-devel@nongnu.org/msg711917.html
> >
> >
> > Tested-by: Cédric Le Goater <clg@kaod.org>
>
> Thanks :)
>
> Note to the maintainer, this series is now fully reviewed/tested.
Applied to target-arm.next, thanks.
-- PMM
^ permalink raw reply [flat|nested] 18+ messages in thread