[PATCH v2 0/1] hw/adc: convert object props to class props

[PATCH v2 0/1] hw/adc: convert object props to class props

[Mark Cave-Ayland]

Since the use of object props is effectively deprecated, here is an attempt
to convert all use of object props in hw/adc to class props. The eventual
aim is to continue working through the codebase, removing all remaining uses
of object props.

The series is lightly tested: it passes "make check", GitLab CI and some
simple local tests. I'm mostly interested for feedback on the conversion
strategy, and to get a feel for the best way to merge this series since once
the basic conversion patterns are in place, the same patterns can be applied
elsewhere and it would be good to minimise the merge window for such changes.

Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>


v2:
- Drop support for "foo[*]" notation, and generate the array property name
  directly as suggested by Daniel


Mark Cave-Ayland (1):
  hw/adc/npcm7xx_adc.c: convert NPCM7XX_ADC object props to class props

 hw/adc/npcm7xx_adc.c | 77 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 70 insertions(+), 7 deletions(-)

-- 
2.43.0

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

[Mark Cave-Ayland]

Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
---
 hw/adc/npcm7xx_adc.c | 77 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 70 insertions(+), 7 deletions(-)

diff --git a/hw/adc/npcm7xx_adc.c b/hw/adc/npcm7xx_adc.c
index 3584c27c75..833e01a913 100644
--- a/hw/adc/npcm7xx_adc.c
+++ b/hw/adc/npcm7xx_adc.c
@@ -20,6 +20,8 @@
 #include "hw/core/qdev-properties.h"
 #include "hw/core/registerfields.h"
 #include "migration/vmstate.h"
+#include "qapi/error.h"
+#include "qapi/visitor.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
 #include "qemu/timer.h"
@@ -229,7 +231,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,15 +241,63 @@ 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);
 }
 
+static void npcm7xx_adc_get_input(Object *obj, Visitor *v,
+                                  const char *name, void *opaque,
+                                  Error **errp)
+{
+    NPCM7xxADCState *s = NPCM7XX_ADC(obj);
+    uint32_t idx, val;
+    int res;
+
+    res = sscanf(name, "adci[%u]", &idx);
+    if (res == EOF || res != 1) {
+        error_setg(errp, "unable to parse index from %s", name);
+        return;
+    }
+
+    val = s->adci[idx];
+    visit_type_uint32(v, name, &val, errp);
+}
+
+static void npcm7xx_adc_set_input(Object *obj, Visitor *v,
+                                  const char *name, void *opaque,
+                                  Error **errp)
+{
+    NPCM7xxADCState *s = NPCM7XX_ADC(obj);
+    uint32_t idx, val;
+    int res;
+
+    res = sscanf(name, "adci[%u]", &idx);
+    if (res == EOF || res != 1) {
+        error_setg(errp, "unable to parse index from %s", name);
+        return;
+    }
+
+    val = s->adci[idx];
+    if (!visit_type_uint32(v, name, &val, errp)) {
+        return;
+    }
+
+    s->adci[idx] = val;
+}
+
+static void npcm7xx_adc_set_vref(Object *obj, Visitor *v,
+                                 const char *name, void *opaque,
+                                 Error **errp)
+{
+    NPCM7xxADCState *s = NPCM7XX_ADC(obj);
+    uint32_t vref = s->vref;
+
+    if (!visit_type_uint32(v, name, &vref, errp)) {
+        return;
+    }
+
+    s->vref = vref;
+}
+
 static const VMStateDescription vmstate_npcm7xx_adc = {
     .name = "npcm7xx-adc",
     .version_id = 0,
@@ -275,6 +324,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 +332,19 @@ 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(klass, adciprop, "uint32",
+                                  npcm7xx_adc_get_input,
+                                  npcm7xx_adc_set_input,
+                                  NULL, NULL);
+    }
+    object_class_property_add(klass, "vref", "uint32",
+                              NULL,
+                              npcm7xx_adc_set_vref,
+                              NULL, NULL);
 }
 
 static const TypeInfo npcm7xx_adc_info = {
-- 
2.43.0

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

[Daniel P. Berrangé]

On Thu, Jun 25, 2026 at 12:11:17PM +0100, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
> ---
>  hw/adc/npcm7xx_adc.c | 77 ++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 70 insertions(+), 7 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|

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

[Mark Cave-Ayland]

On 25/06/2026 12:15, Daniel P. Berrangé wrote:

> On Thu, Jun 25, 2026 at 12:11:17PM +0100, Mark Cave-Ayland wrote:
>> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
>> ---
>>   hw/adc/npcm7xx_adc.c | 77 ++++++++++++++++++++++++++++++++++++++++----
>>   1 file changed, 70 insertions(+), 7 deletions(-)
> 
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Thanks Daniel.

This is actually the 2nd conversion series I've sent, the first is for 
hw/acpi which I posted at 
https://patchew.org/QEMU/20260622112403.928954-1-mark.caveayland@nutanix.com/.

Given that such changes will cover a large amount of the codebase, do 
you think it makes sense to send all the series through the QOM tree? 
And would that require an Ack from all the relevant maintainers too?

My worry is that the conversion process and dropping of standard object 
properties will get dragged out over many months for what is effectively 
a global mechanical-ish conversion.


ATB,

Mark.

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

[Peter Maydell]

On Thu, 25 Jun 2026 at 12:12, Mark Cave-Ayland
<mark.caveayland@nutanix.com> wrote:
>
> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
> ---
>  hw/adc/npcm7xx_adc.c | 77 ++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 70 insertions(+), 7 deletions(-)
>
> diff --git a/hw/adc/npcm7xx_adc.c b/hw/adc/npcm7xx_adc.c
> index 3584c27c75..833e01a913 100644
> --- a/hw/adc/npcm7xx_adc.c
> +++ b/hw/adc/npcm7xx_adc.c
> @@ -20,6 +20,8 @@
>  #include "hw/core/qdev-properties.h"
>  #include "hw/core/registerfields.h"
>  #include "migration/vmstate.h"
> +#include "qapi/error.h"
> +#include "qapi/visitor.h"
>  #include "qemu/log.h"
>  #include "qemu/module.h"
>  #include "qemu/timer.h"
> @@ -229,7 +231,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,15 +241,63 @@ 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);
> -    }

In the old style we get to use "adci[*]", which avoids the
boilerplate of creating and managing the string for the
name with the substituted-in integer value:

        g_autofree char *adciprop = g_strdup_printf("adci[%u]", i);

Is there a way of keeping that with the new setup?

> -    object_property_add_uint32_ptr(obj, "vref",
> -            &s->vref, OBJ_PROP_FLAG_WRITE);
>      npcm7xx_adc_calibrate(s);
>  }
>
> +static void npcm7xx_adc_get_input(Object *obj, Visitor *v,
> +                                  const char *name, void *opaque,
> +                                  Error **errp)
> +{
> +    NPCM7xxADCState *s = NPCM7XX_ADC(obj);
> +    uint32_t idx, val;
> +    int res;
> +
> +    res = sscanf(name, "adci[%u]", &idx);
> +    if (res == EOF || res != 1) {
> +        error_setg(errp, "unable to parse index from %s", name);
> +        return;
> +    }
> +
> +    val = s->adci[idx];
> +    visit_type_uint32(v, name, &val, errp);
> +}
> +
> +static void npcm7xx_adc_set_input(Object *obj, Visitor *v,
> +                                  const char *name, void *opaque,
> +                                  Error **errp)
> +{
> +    NPCM7xxADCState *s = NPCM7XX_ADC(obj);
> +    uint32_t idx, val;
> +    int res;
> +
> +    res = sscanf(name, "adci[%u]", &idx);
> +    if (res == EOF || res != 1) {
> +        error_setg(errp, "unable to parse index from %s", name);
> +        return;
> +    }
> +
> +    val = s->adci[idx];
> +    if (!visit_type_uint32(v, name, &val, errp)) {
> +        return;
> +    }
> +
> +    s->adci[idx] = val;
> +}
> +
> +static void npcm7xx_adc_set_vref(Object *obj, Visitor *v,
> +                                 const char *name, void *opaque,
> +                                 Error **errp)
> +{
> +    NPCM7xxADCState *s = NPCM7XX_ADC(obj);
> +    uint32_t vref = s->vref;
> +
> +    if (!visit_type_uint32(v, name, &vref, errp)) {
> +        return;
> +    }
> +
> +    s->vref = vref;
> +}

In the old code, we say "I want a uint32 property", and we get
one. In the new code we have to have a lot of code here that's
manually calling sscanf and doing visitor pattern stuff.
Is there a way to say "give me a uint32 property" and have
the implementation be done in common for the new style ?

>  1 file changed, 70 insertions(+), 7 deletions(-)

This diffstat makes me want to say we haven't got the
APIs for doing this right yet.

thanks
-- PMM