[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH RESEND] hw/i2c: Enable an id for the pca954x devices
From: |
Corey Minyard |
Subject: |
Re: [PATCH RESEND] hw/i2c: Enable an id for the pca954x devices |
Date: |
Wed, 22 Mar 2023 13:35:40 -0500 |
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
> > >
> >