| Message ID | 20230322172136.48010-1-venture@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RESEND,v2] hw/i2c: Enable an id for the pca954x devices | expand |
On Wed, Mar 22, 2023 at 10:21:36AM -0700, Patrick Venture wrote: > This allows the devices to be more readily found and specified. > Without setting the name 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 name > 'pca954x[77]', for an instance attached at that address. This looks good to me. Acked-by: Corey Minyard <cminyard@mvista.com> if you are taking this in through another tree. Or do you want me to take this? -corey > > Signed-off-by: Patrick Venture <venture@google.com> > Reviewed-by: Hao Wu <wuhaotsh@google.com> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > v2: s/id/name/g to use name as the identifier field. left 'id' in subject for email chain. > --- > 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 3945de795c..76e69bebc5 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 *name; > } 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->name) { > + d->id = g_strdup(s->name); > + } 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("nane", Pca954xState, name), > + 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.40.0.rc1.284.g88254d51c5-goog >
On 22/3/23 22:19, Corey Minyard wrote: > On Wed, Mar 22, 2023 at 10:21:36AM -0700, Patrick Venture wrote: >> This allows the devices to be more readily found and specified. >> Without setting the name 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 name >> 'pca954x[77]', for an instance attached at that address. > > This looks good to me. > > Acked-by: Corey Minyard <cminyard@mvista.com> > > if you are taking this in through another tree. Or do you want me to > take this? Since I have to send a MIPS PR, I'll take this one; to alleviate you and the CI minutes.
On Wed, Mar 22, 2023 at 2:40 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > On 22/3/23 22:19, Corey Minyard wrote: > > On Wed, Mar 22, 2023 at 10:21:36AM -0700, Patrick Venture wrote: > >> This allows the devices to be more readily found and specified. > >> Without setting the name 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 name > >> 'pca954x[77]', for an instance attached at that address. > > > > This looks good to me. > > > > Acked-by: Corey Minyard <cminyard@mvista.com> > > > > if you are taking this in through another tree. Or do you want me to > > take this? > > Since I have to send a MIPS PR, I'll take this one; > to alleviate you and the CI minutes. > I don't see this patch yet, did it got lost in the shuffle? thanks, Patrick
Hi Patrick, On 31/5/23 18:34, Patrick Venture wrote: > > > On Wed, Mar 22, 2023 at 2:40 PM Philippe Mathieu-Daudé > <philmd@linaro.org <mailto:philmd@linaro.org>> wrote: > > On 22/3/23 22:19, Corey Minyard wrote: > > On Wed, Mar 22, 2023 at 10:21:36AM -0700, Patrick Venture wrote: > >> This allows the devices to be more readily found and specified. > >> Without setting the name 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 name > >> 'pca954x[77]', for an instance attached at that address. > > > > This looks good to me. > > > > Acked-by: Corey Minyard <cminyard@mvista.com > <mailto:cminyard@mvista.com>> > > > > if you are taking this in through another tree. Or do you want me to > > take this? > > Since I have to send a MIPS PR, I'll take this one; > to alleviate you and the CI minutes. > > > I don't see this patch yet, did it got lost in the shuffle? I quickly tried to test the patch before sending the PR and it was not working, so I dropped it; but since it was a busy day I neglected to post an update on the list. I apologize for that. Revisiting the patch, the problem is trivial, a simple typo: +static Property pca954x_props[] = { + DEFINE_PROP_STRING("nane", Pca954xState, name), ^^^^ + DEFINE_PROP_END_OF_LIST() +}; I'm queuing this patch again with s/nane/name/. Regards, Phil.
diff --git a/hw/i2c/i2c_mux_pca954x.c b/hw/i2c/i2c_mux_pca954x.c index 3945de795c..76e69bebc5 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 *name; } 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->name) { + d->id = g_strdup(s->name); + } 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("nane", Pca954xState, name), + 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[] = {