Re: [PATCH v2 1/6] hw/i2c: add dummy device

[Timur Maksimov <gessingate@gmail.com>]

Hi, thanks for the review. 

It was my decision to use such API, I thought it would be okay for a
test device. I don't think there is a way to expose integer array as a
single QOM property.

Actually, we can go without this model. I suppose I will rewrite tests
so they don't use it.

On Wed, 2026-10-28 at 11:17 +0100, Markus Armbruster wrote:
> Cédric Le Goater <clg@kaod.org> writes:
> 
> > + Corey for I2C feed back.
> > 
> > + Markus for QOM
> > 
> > The brute-force solution to expose the contents of the device array
> > with 128 properties has a slight AI smell. I find it ugly but I am
> > human :)
> 
> Timur, have you read docs/devel/code-provenance.rst section "Use of
> AI-generated content"?
> 
> >          That said I don't know if there are better alternatives.
> > My approach would have been to use a string property :
> > 
> >       object_property_add_str(obj, "tx",
> >                               i2c_dummy_device_get_tx, NULL);
> >       object_property_add_str(obj, "rx",
> >                               i2c_dummy_device_get_rx,
> >                               i2c_dummy_device_set_rx);
> >  Any better ideas ?
> 
> I believe you're talking about this function:
> 
> [...]
> 
> > > +static void i2c_dummy_device_init(Object *obj)
> > > +{
> > > +    I2CDummyDeviceState *s = I2C_DUMMY_DEVICE(obj);
> > > 
> > > +    for (int i = 0; i < I2C_DUMMY_ARRAY_SIZE; i++) {
> > > +        g_autofree char *name = g_strdup_printf("tx[%d]", i);
> > > +
> > > +        object_property_add_uint8_ptr(
> > > +            obj, name, &s->tx[i],
> > > +            OBJ_PROP_FLAG_READWRITE);
> > > +    }
> > > +
> > > +    for (int i = 0; i < I2C_DUMMY_ARRAY_SIZE; i++) {
> > > +        g_autofree char *name = g_strdup_printf("rx[%d]", i);
> > > +
> > > +        object_property_add_uint8_ptr(
> > > +            obj, name, &s->rx[i],
> > > +            OBJ_PROP_FLAG_READWRITE);
> > > +    }
> > > +
> > > +    object_property_add_uint8_ptr(
> > > +            obj, "tx-pos", &s->tx_pos,
> > > +            OBJ_PROP_FLAG_READWRITE);
> > > +    object_property_add_uint8_ptr(
> > > +            obj, "rx-pos", &s->rx_pos,
> > > +            OBJ_PROP_FLAG_READWRITE);
> > 
> > alignment could be improved.
> 
> Yes, this style is unusual in QEMU, and should therefore be avoided.
> 
> > > +}
> 
> The function adds properties "tx-pos", "tx[0]", ... "tx[63]", and
> same
> for rx.
> 
> The commit message tells us this is a "Simple I2C device with QOM
> properties to access transmit and receive buffers [...] for testing
> or
> prototyping."  It doesn't tell us how it could be used, and I can't
> see
> any documentation, either.  So let's start right there: please show
> us
> how this device could be used.  This should help me understand what
> the
> properties are for.  Only then can I advise on proper QOM usage.
> 
> [...]