Re: [PATCH v3 10/14] hw/sensor: Add Renesas ISL69259 device model

[Titus Rwantare]

On Wed, 29 Jun 2022 at 23:30, Cédric Le Goater <clg@kaod.org> wrote:
>
> On 6/30/22 06:51, Peter Delevoryas wrote:
> > From: Peter Delevoryas <pdel@fb.com>
> >
> > This adds the ISL69259, using all the same functionality as the existing
> > ISL69260 but overriding the IC_DEVICE_ID.
> >
> > Signed-off-by: Peter Delevoryas <pdel@fb.com>
> > ---
> >   hw/sensor/isl_pmbus_vr.c | 28 ++++++++++++++++++++++++++++
> >   1 file changed, 28 insertions(+)
> >
> > diff --git a/hw/sensor/isl_pmbus_vr.c b/hw/sensor/isl_pmbus_vr.c
> > index 799ea9d89e..853d70536f 100644
> > --- a/hw/sensor/isl_pmbus_vr.c
> > +++ b/hw/sensor/isl_pmbus_vr.c
> > @@ -119,6 +119,18 @@ static void raa228000_exit_reset(Object *obj)
> >       pmdev->pages[0].read_temperature_3 = 0;
> >   }
> >
> > +static void isl69259_exit_reset(Object *obj)
> > +{
> > +    ISLState *s = ISL69260(obj);
> > +    static const uint8_t ic_device_id[] = {0x04, 0x00, 0x81, 0xD2, 0x49, 
> > 0x3c};
>
> This looks like an ISLClass attribute to me. In which case, you wouldn't need 
> the
> reset handler nor the 'ic_device_id_len' field.
>
> Thanks,
>
> C.

I asked for this because, so far, I've been doing all the register
defaults in reset handlers, including read-only registers.
I don't mind either way, but it seemed preferable to have the devices
consistent.

Titus