On Fri, Jul 03, 2026 at 03:32:17PM +0100, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <[email protected]>
> ---
> 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 :|
