On Tue, Mar 21, 2023 at 6:41 PM Corey Minyard <cminy...@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 <vent...@google.com>
> > Reviewed-by: Hao Wu <wuhao...@google.com>
> > Reviewed-by: Philippe Mathieu-Daudé <phi...@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
> >
>
