Re: [PATCH v3 1/1] hw/adc/npcm7xx_adc.c: convert NPCM7XX_ADC object props to class props

["Daniel P. Berrangé" <berrange@redhat.com>]

On Fri, Jul 03, 2026 at 03:32:17PM +0100, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
> ---
>  hw/adc/npcm7xx_adc.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/adc/npcm7xx_adc.c b/hw/adc/npcm7xx_adc.c
> index 3584c27c75..27d42f87b1 100644
> --- a/hw/adc/npcm7xx_adc.c
> +++ b/hw/adc/npcm7xx_adc.c
> @@ -229,7 +229,6 @@ static void npcm7xx_adc_init(Object *obj)
>  {
>      NPCM7xxADCState *s = NPCM7XX_ADC(obj);
>      SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> -    int i;
>  
>      sysbus_init_irq(sbd, &s->irq);
>  
> @@ -240,12 +239,6 @@ static void npcm7xx_adc_init(Object *obj)
>      sysbus_init_mmio(sbd, &s->iomem);
>      s->clock = qdev_init_clock_in(DEVICE(s), "clock", NULL, NULL, 0);
>  
> -    for (i = 0; i < NPCM7XX_ADC_NUM_INPUTS; ++i) {
> -        object_property_add_uint32_ptr(obj, "adci[*]",
> -                &s->adci[i], OBJ_PROP_FLAG_READWRITE);
> -    }
> -    object_property_add_uint32_ptr(obj, "vref",
> -            &s->vref, OBJ_PROP_FLAG_WRITE);
>      npcm7xx_adc_calibrate(s);
>  }
>  
> @@ -275,6 +268,7 @@ static void npcm7xx_adc_class_init(ObjectClass *klass, const void *data)
>  {
>      ResettableClass *rc = RESETTABLE_CLASS(klass);
>      DeviceClass *dc = DEVICE_CLASS(klass);
> +    int i;
>  
>      dc->desc = "NPCM7xx ADC Module";
>      dc->vmsd = &vmstate_npcm7xx_adc;
> @@ -282,6 +276,17 @@ static void npcm7xx_adc_class_init(ObjectClass *klass, const void *data)
>      rc->phases.hold = npcm7xx_adc_hold_reset;
>  
>      device_class_set_props(dc, npcm7xx_timer_properties);
> +
> +    for (i = 0; i < NPCM7XX_ADC_NUM_INPUTS; ++i) {
> +        g_autofree char *adciprop = g_strdup_printf("adci[%u]", i);
> +
> +        object_class_property_add_uint32_ptr(klass, adciprop,
> +                offsetof(NPCM7xxADCState, adci[i]),
> +                OBJ_PROP_FLAG_READWRITE);
> +    }
> +    object_class_property_add_uint32_ptr(klass, "vref",
> +                offsetof(NPCM7xxADCState, vref),
> +                OBJ_PROP_FLAG_WRITE);

Are you sure this actually works ?

With the old code:

        object_property_add_uint32_ptr(obj, "adci[*]",
                &s->adci[i], OBJ_PROP_FLAG_READWRITE);

we're passing the absolute address of a struct field.

With the new code:

        object_class_property_add_uint32_ptr(klass, adciprop,
                offsetof(NPCM7xxADCState, adci[i]),
                OBJ_PROP_FLAG_READWRITE);

we're passing the relative offset from the start of the struct.

I agree this makes sense as the semantics we'd want/need, but it
doesn't seem to be what's actually implementefd

Both object_property_add_uint32_ptr and object_class_property_add_uint32_ptr
use the same property_get_uint32_ptr & property_set_uint32_ptr
callbacks.

IOW, afaict object_class_property_add_uint32_ptr is expecting to get
an absolute address, not a relative offset.

This seems to make object_class_property_add_uint32_ptr largely
useless unless trying to set global variables, instead of instance
level fields.


With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|