* [PATCH RESEND] hw/i2c: Enable an id for the pca954x devices
@ 2023-03-21 18:27 Patrick Venture
2023-03-22 1:41 ` Corey Minyard
0 siblings, 1 reply; 6+ messages in thread
From: Patrick Venture @ 2023-03-21 18:27 UTC (permalink / raw)
To: cminyard, peter.maydell
Cc: qemu-devel, Patrick Venture, Hao Wu, Philippe Mathieu-Daudé
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="y", Size: 2449 bytes --]
This allows the devices to be more readily found and specified.
Without setting the id field, they can only be found by device type
name, which doesn't let you specify the second of the same device type
behind a bus.
Tested: Verified that by default the device was findable with the id
'pca954x[77]', for an instance attached at that address.
Signed-off-by: Patrick Venture <venture@google.com>
Reviewed-by: Hao Wu <wuhaotsh@google.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/i2c/i2c_mux_pca954x.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/hw/i2c/i2c_mux_pca954x.c b/hw/i2c/i2c_mux_pca954x.c
index a9517b612a..4f8c2d6ae1 100644
--- a/hw/i2c/i2c_mux_pca954x.c
+++ b/hw/i2c/i2c_mux_pca954x.c
@@ -20,6 +20,7 @@
#include "hw/i2c/i2c_mux_pca954x.h"
#include "hw/i2c/smbus_slave.h"
#include "hw/qdev-core.h"
+#include "hw/qdev-properties.h"
#include "hw/sysbus.h"
#include "qemu/log.h"
#include "qemu/module.h"
@@ -43,6 +44,8 @@ typedef struct Pca954xState {
bool enabled[PCA9548_CHANNEL_COUNT];
I2CBus *bus[PCA9548_CHANNEL_COUNT];
+
+ char *id;
} Pca954xState;
/*
@@ -181,6 +184,17 @@ static void pca9548_class_init(ObjectClass *klass, void *data)
s->nchans = PCA9548_CHANNEL_COUNT;
}
+static void pca954x_realize(DeviceState *dev, Error **errp)
+{
+ Pca954xState *s = PCA954X(dev);
+ DeviceState *d = DEVICE(s);
+ if (s->id) {
+ d->id = g_strdup(s->id);
+ } else {
+ d->id = g_strdup_printf("pca954x[%x]", s->parent.i2c.address);
+ }
+}
+
static void pca954x_init(Object *obj)
{
Pca954xState *s = PCA954X(obj);
@@ -197,6 +211,11 @@ static void pca954x_init(Object *obj)
}
}
+static Property pca954x_props[] = {
+ DEFINE_PROP_STRING("id", Pca954xState, id),
+ DEFINE_PROP_END_OF_LIST()
+};
+
static void pca954x_class_init(ObjectClass *klass, void *data)
{
I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass);
@@ -209,9 +228,12 @@ static void pca954x_class_init(ObjectClass *klass, void *data)
rc->phases.enter = pca954x_enter_reset;
dc->desc = "Pca954x i2c-mux";
+ dc->realize = pca954x_realize;
k->write_data = pca954x_write_data;
k->receive_byte = pca954x_read_byte;
+
+ device_class_set_props(dc, pca954x_props);
}
static const TypeInfo pca954x_info[] = {
--
2.35.1.894.gb6a874cedc-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH RESEND] hw/i2c: Enable an id for the pca954x devices
2023-03-21 18:27 [PATCH RESEND] hw/i2c: Enable an id for the pca954x devices Patrick Venture
@ 2023-03-22 1:41 ` Corey Minyard
2023-03-22 17:03 ` Patrick Venture
0 siblings, 1 reply; 6+ messages in thread
From: Corey Minyard @ 2023-03-22 1:41 UTC (permalink / raw)
To: Patrick Venture
Cc: peter.maydell, qemu-devel, Hao Wu, Philippe Mathieu-Daudé
[-- Attachment #1: Type: text/plain, Size: 3049 bytes --]
On Tue, Mar 21, 2023 at 11:27:44AM -0700, Patrick Venture wrote:
> This allows the devices to be more readily found and specified.
> Without setting the id field, they can only be found by device type
> name, which doesn't let you specify the second of the same device type
> behind a bus.
So basically, this helps you find a specific device if you have more
than one of them. Yeah, probably a good idea in this case.
>
> Tested: Verified that by default the device was findable with the id
> 'pca954x[77]', for an instance attached at that address.
>
> Signed-off-by: Patrick Venture <venture@google.com>
> Reviewed-by: Hao Wu <wuhaotsh@google.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/i2c/i2c_mux_pca954x.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/hw/i2c/i2c_mux_pca954x.c b/hw/i2c/i2c_mux_pca954x.c
> index a9517b612a..4f8c2d6ae1 100644
> --- a/hw/i2c/i2c_mux_pca954x.c
> +++ b/hw/i2c/i2c_mux_pca954x.c
> @@ -20,6 +20,7 @@
> #include "hw/i2c/i2c_mux_pca954x.h"
> #include "hw/i2c/smbus_slave.h"
> #include "hw/qdev-core.h"
> +#include "hw/qdev-properties.h"
> #include "hw/sysbus.h"
> #include "qemu/log.h"
> #include "qemu/module.h"
> @@ -43,6 +44,8 @@ typedef struct Pca954xState {
>
> bool enabled[PCA9548_CHANNEL_COUNT];
> I2CBus *bus[PCA9548_CHANNEL_COUNT];
> +
> + char *id;
> } Pca954xState;
>
> /*
> @@ -181,6 +184,17 @@ static void pca9548_class_init(ObjectClass *klass, void *data)
> s->nchans = PCA9548_CHANNEL_COUNT;
> }
>
> +static void pca954x_realize(DeviceState *dev, Error **errp)
> +{
> + Pca954xState *s = PCA954X(dev);
> + DeviceState *d = DEVICE(s);
> + if (s->id) {
> + d->id = g_strdup(s->id);
> + } else {
> + d->id = g_strdup_printf("pca954x[%x]", s->parent.i2c.address);
> + }
> +}
> +
> static void pca954x_init(Object *obj)
> {
> Pca954xState *s = PCA954X(obj);
> @@ -197,6 +211,11 @@ static void pca954x_init(Object *obj)
> }
> }
>
> +static Property pca954x_props[] = {
> + DEFINE_PROP_STRING("id", Pca954xState, id),
> + DEFINE_PROP_END_OF_LIST()
> +};
There is already an "id=" thing in qemu for doing links between front
ends and back ends. That's probably not the best name to use. Several
devices, like network devices, use "name" for identification in the
monitor.
-corey
> +
> static void pca954x_class_init(ObjectClass *klass, void *data)
> {
> I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass);
> @@ -209,9 +228,12 @@ static void pca954x_class_init(ObjectClass *klass, void *data)
> rc->phases.enter = pca954x_enter_reset;
>
> dc->desc = "Pca954x i2c-mux";
> + dc->realize = pca954x_realize;
>
> k->write_data = pca954x_write_data;
> k->receive_byte = pca954x_read_byte;
> +
> + device_class_set_props(dc, pca954x_props);
> }
>
> static const TypeInfo pca954x_info[] = {
> --
> 2.35.1.894.gb6a874cedc-goog
>
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3426 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH RESEND] hw/i2c: Enable an id for the pca954x devices
2023-03-22 1:41 ` Corey Minyard
@ 2023-03-22 17:03 ` Patrick Venture
2023-03-22 18:35 ` Corey Minyard
0 siblings, 1 reply; 6+ messages in thread
From: Patrick Venture @ 2023-03-22 17:03 UTC (permalink / raw)
To: cminyard; +Cc: peter.maydell, qemu-devel, Hao Wu, Philippe Mathieu-Daudé
[-- Attachment #1: Type: text/plain, Size: 3448 bytes --]
On Tue, Mar 21, 2023 at 6:41 PM Corey Minyard <cminyard@mvista.com> wrote:
> On Tue, Mar 21, 2023 at 11:27:44AM -0700, Patrick Venture wrote:
> > This allows the devices to be more readily found and specified.
> > Without setting the id field, they can only be found by device type
> > name, which doesn't let you specify the second of the same device type
> > behind a bus.
>
> So basically, this helps you find a specific device if you have more
> than one of them. Yeah, probably a good idea in this case.
>
> >
> > Tested: Verified that by default the device was findable with the id
> > 'pca954x[77]', for an instance attached at that address.
> >
> > Signed-off-by: Patrick Venture <venture@google.com>
> > Reviewed-by: Hao Wu <wuhaotsh@google.com>
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> > hw/i2c/i2c_mux_pca954x.c | 22 ++++++++++++++++++++++
> > 1 file changed, 22 insertions(+)
> >
> > diff --git a/hw/i2c/i2c_mux_pca954x.c b/hw/i2c/i2c_mux_pca954x.c
> > index a9517b612a..4f8c2d6ae1 100644
> > --- a/hw/i2c/i2c_mux_pca954x.c
> > +++ b/hw/i2c/i2c_mux_pca954x.c
> > @@ -20,6 +20,7 @@
> > #include "hw/i2c/i2c_mux_pca954x.h"
> > #include "hw/i2c/smbus_slave.h"
> > #include "hw/qdev-core.h"
> > +#include "hw/qdev-properties.h"
> > #include "hw/sysbus.h"
> > #include "qemu/log.h"
> > #include "qemu/module.h"
> > @@ -43,6 +44,8 @@ typedef struct Pca954xState {
> >
> > bool enabled[PCA9548_CHANNEL_COUNT];
> > I2CBus *bus[PCA9548_CHANNEL_COUNT];
> > +
> > + char *id;
> > } Pca954xState;
> >
> > /*
> > @@ -181,6 +184,17 @@ static void pca9548_class_init(ObjectClass *klass,
> void *data)
> > s->nchans = PCA9548_CHANNEL_COUNT;
> > }
> >
> > +static void pca954x_realize(DeviceState *dev, Error **errp)
> > +{
> > + Pca954xState *s = PCA954X(dev);
> > + DeviceState *d = DEVICE(s);
> > + if (s->id) {
> > + d->id = g_strdup(s->id);
> > + } else {
> > + d->id = g_strdup_printf("pca954x[%x]", s->parent.i2c.address);
> > + }
> > +}
> > +
> > static void pca954x_init(Object *obj)
> > {
> > Pca954xState *s = PCA954X(obj);
> > @@ -197,6 +211,11 @@ static void pca954x_init(Object *obj)
> > }
> > }
> >
> > +static Property pca954x_props[] = {
> > + DEFINE_PROP_STRING("id", Pca954xState, id),
> > + DEFINE_PROP_END_OF_LIST()
> > +};
>
> There is already an "id=" thing in qemu for doing links between front
> ends and back ends. That's probably not the best name to use. Several
> devices, like network devices, use "name" for identification in the
> monitor.
>
So I should change it to name? I'm ok with that. I think bus would be
slightly confusing. "id" was short and easy. Will send a v2.
>
> -corey
>
> > +
> > static void pca954x_class_init(ObjectClass *klass, void *data)
> > {
> > I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass);
> > @@ -209,9 +228,12 @@ static void pca954x_class_init(ObjectClass *klass,
> void *data)
> > rc->phases.enter = pca954x_enter_reset;
> >
> > dc->desc = "Pca954x i2c-mux";
> > + dc->realize = pca954x_realize;
> >
> > k->write_data = pca954x_write_data;
> > k->receive_byte = pca954x_read_byte;
> > +
> > + device_class_set_props(dc, pca954x_props);
> > }
> >
> > static const TypeInfo pca954x_info[] = {
> > --
> > 2.35.1.894.gb6a874cedc-goog
> >
>
[-- Attachment #2: Type: text/html, Size: 4821 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH RESEND] hw/i2c: Enable an id for the pca954x devices
2023-03-22 17:03 ` Patrick Venture
@ 2023-03-22 18:35 ` Corey Minyard
0 siblings, 0 replies; 6+ messages in thread
From: Corey Minyard @ 2023-03-22 18:35 UTC (permalink / raw)
To: Patrick Venture
Cc: cminyard, peter.maydell, qemu-devel, Hao Wu,
Philippe Mathieu-Daudé
On Wed, Mar 22, 2023 at 10:03:27AM -0700, Patrick Venture wrote:
> On Tue, Mar 21, 2023 at 6:41 PM Corey Minyard <cminyard@mvista.com> wrote:
>
> > On Tue, Mar 21, 2023 at 11:27:44AM -0700, Patrick Venture wrote:
> > > This allows the devices to be more readily found and specified.
> > > Without setting the id field, they can only be found by device type
> > > name, which doesn't let you specify the second of the same device type
> > > behind a bus.
> >
> > So basically, this helps you find a specific device if you have more
> > than one of them. Yeah, probably a good idea in this case.
> >
> > >
> > > Tested: Verified that by default the device was findable with the id
> > > 'pca954x[77]', for an instance attached at that address.
> > >
> > > Signed-off-by: Patrick Venture <venture@google.com>
> > > Reviewed-by: Hao Wu <wuhaotsh@google.com>
> > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > ---
> > > hw/i2c/i2c_mux_pca954x.c | 22 ++++++++++++++++++++++
> > > 1 file changed, 22 insertions(+)
> > >
> > > diff --git a/hw/i2c/i2c_mux_pca954x.c b/hw/i2c/i2c_mux_pca954x.c
> > > index a9517b612a..4f8c2d6ae1 100644
> > > --- a/hw/i2c/i2c_mux_pca954x.c
> > > +++ b/hw/i2c/i2c_mux_pca954x.c
> > > @@ -20,6 +20,7 @@
> > > #include "hw/i2c/i2c_mux_pca954x.h"
> > > #include "hw/i2c/smbus_slave.h"
> > > #include "hw/qdev-core.h"
> > > +#include "hw/qdev-properties.h"
> > > #include "hw/sysbus.h"
> > > #include "qemu/log.h"
> > > #include "qemu/module.h"
> > > @@ -43,6 +44,8 @@ typedef struct Pca954xState {
> > >
> > > bool enabled[PCA9548_CHANNEL_COUNT];
> > > I2CBus *bus[PCA9548_CHANNEL_COUNT];
> > > +
> > > + char *id;
> > > } Pca954xState;
> > >
> > > /*
> > > @@ -181,6 +184,17 @@ static void pca9548_class_init(ObjectClass *klass,
> > void *data)
> > > s->nchans = PCA9548_CHANNEL_COUNT;
> > > }
> > >
> > > +static void pca954x_realize(DeviceState *dev, Error **errp)
> > > +{
> > > + Pca954xState *s = PCA954X(dev);
> > > + DeviceState *d = DEVICE(s);
> > > + if (s->id) {
> > > + d->id = g_strdup(s->id);
> > > + } else {
> > > + d->id = g_strdup_printf("pca954x[%x]", s->parent.i2c.address);
> > > + }
> > > +}
> > > +
> > > static void pca954x_init(Object *obj)
> > > {
> > > Pca954xState *s = PCA954X(obj);
> > > @@ -197,6 +211,11 @@ static void pca954x_init(Object *obj)
> > > }
> > > }
> > >
> > > +static Property pca954x_props[] = {
> > > + DEFINE_PROP_STRING("id", Pca954xState, id),
> > > + DEFINE_PROP_END_OF_LIST()
> > > +};
> >
> > There is already an "id=" thing in qemu for doing links between front
> > ends and back ends. That's probably not the best name to use. Several
> > devices, like network devices, use "name" for identification in the
> > monitor.
> >
>
> So I should change it to name? I'm ok with that. I think bus would be
> slightly confusing. "id" was short and easy. Will send a v2.
Yes, "name" would be more consistent with other usage.
-corey
>
>
> >
> > -corey
> >
> > > +
> > > static void pca954x_class_init(ObjectClass *klass, void *data)
> > > {
> > > I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass);
> > > @@ -209,9 +228,12 @@ static void pca954x_class_init(ObjectClass *klass,
> > void *data)
> > > rc->phases.enter = pca954x_enter_reset;
> > >
> > > dc->desc = "Pca954x i2c-mux";
> > > + dc->realize = pca954x_realize;
> > >
> > > k->write_data = pca954x_write_data;
> > > k->receive_byte = pca954x_read_byte;
> > > +
> > > + device_class_set_props(dc, pca954x_props);
> > > }
> > >
> > > static const TypeInfo pca954x_info[] = {
> > > --
> > > 2.35.1.894.gb6a874cedc-goog
> > >
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH RESEND] hw/i2c: Enable an id for the pca954x devices
@ 2022-10-11 15:46 Patrick Venture
2022-10-18 10:36 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 6+ messages in thread
From: Patrick Venture @ 2022-10-11 15:46 UTC (permalink / raw)
To: f4bug, peter.maydell; +Cc: qemu-devel, Patrick Venture, Hao Wu
This allows the devices to be more readily found and specified.
Without setting the id field, they can only be found by device type
name, which doesn't let you specify the second of the same device type
behind a bus.
Tested: Verified that by default the device was findable with the id
'pca954x[77]', for an instance attached at that address.
Signed-off-by: Patrick Venture <venture@google.com>
Reviewed-by: Hao Wu <wuhaotsh@google.com>
---
hw/i2c/i2c_mux_pca954x.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/hw/i2c/i2c_mux_pca954x.c b/hw/i2c/i2c_mux_pca954x.c
index a9517b612a..4f8c2d6ae1 100644
--- a/hw/i2c/i2c_mux_pca954x.c
+++ b/hw/i2c/i2c_mux_pca954x.c
@@ -20,6 +20,7 @@
#include "hw/i2c/i2c_mux_pca954x.h"
#include "hw/i2c/smbus_slave.h"
#include "hw/qdev-core.h"
+#include "hw/qdev-properties.h"
#include "hw/sysbus.h"
#include "qemu/log.h"
#include "qemu/module.h"
@@ -43,6 +44,8 @@ typedef struct Pca954xState {
bool enabled[PCA9548_CHANNEL_COUNT];
I2CBus *bus[PCA9548_CHANNEL_COUNT];
+
+ char *id;
} Pca954xState;
/*
@@ -181,6 +184,17 @@ static void pca9548_class_init(ObjectClass *klass, void *data)
s->nchans = PCA9548_CHANNEL_COUNT;
}
+static void pca954x_realize(DeviceState *dev, Error **errp)
+{
+ Pca954xState *s = PCA954X(dev);
+ DeviceState *d = DEVICE(s);
+ if (s->id) {
+ d->id = g_strdup(s->id);
+ } else {
+ d->id = g_strdup_printf("pca954x[%x]", s->parent.i2c.address);
+ }
+}
+
static void pca954x_init(Object *obj)
{
Pca954xState *s = PCA954X(obj);
@@ -197,6 +211,11 @@ static void pca954x_init(Object *obj)
}
}
+static Property pca954x_props[] = {
+ DEFINE_PROP_STRING("id", Pca954xState, id),
+ DEFINE_PROP_END_OF_LIST()
+};
+
static void pca954x_class_init(ObjectClass *klass, void *data)
{
I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass);
@@ -209,9 +228,12 @@ static void pca954x_class_init(ObjectClass *klass, void *data)
rc->phases.enter = pca954x_enter_reset;
dc->desc = "Pca954x i2c-mux";
+ dc->realize = pca954x_realize;
k->write_data = pca954x_write_data;
k->receive_byte = pca954x_read_byte;
+
+ device_class_set_props(dc, pca954x_props);
}
static const TypeInfo pca954x_info[] = {
--
2.35.1.894.gb6a874cedc-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH RESEND] hw/i2c: Enable an id for the pca954x devices
2022-10-11 15:46 Patrick Venture
@ 2022-10-18 10:36 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-10-18 10:36 UTC (permalink / raw)
To: Patrick Venture, peter.maydell; +Cc: qemu-devel, Hao Wu
On 11/10/22 17:46, Patrick Venture wrote:
> This allows the devices to be more readily found and specified.
> Without setting the id field, they can only be found by device type
> name, which doesn't let you specify the second of the same device type
> behind a bus.
>
> Tested: Verified that by default the device was findable with the id
> 'pca954x[77]', for an instance attached at that address.
>
> Signed-off-by: Patrick Venture <venture@google.com>
> Reviewed-by: Hao Wu <wuhaotsh@google.com>
> ---
> hw/i2c/i2c_mux_pca954x.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-03-22 18:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-21 18:27 [PATCH RESEND] hw/i2c: Enable an id for the pca954x devices Patrick Venture
2023-03-22 1:41 ` Corey Minyard
2023-03-22 17:03 ` Patrick Venture
2023-03-22 18:35 ` Corey Minyard
-- strict thread matches above, loose matches on Subject: below --
2022-10-11 15:46 Patrick Venture
2022-10-18 10:36 ` Philippe Mathieu-Daudé
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).