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};
+ g_assert_cmphex(sizeof(ic_device_id), <=, sizeof(s->ic_device_id));
+
+ isl_pmbus_vr_exit_reset(obj);
+
+ s->ic_device_id_len = sizeof(ic_device_id);
+ memcpy(s->ic_device_id, ic_device_id, sizeof(ic_device_id));
+}
+
static void isl_pmbus_vr_add_props(Object *obj, uint64_t *flags, uint8_t pages)
{
PMBusDevice *pmdev = PMBUS_DEVICE(obj);
@@ -257,6 +269,21 @@ static void raa229004_class_init(ObjectClass *klass, void *data)
isl_pmbus_vr_class_init(klass, data, 2);
}
+static void isl69259_class_init(ObjectClass *klass, void *data)
+{
+ ResettableClass *rc = RESETTABLE_CLASS(klass);
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ dc->desc = "Renesas ISL69259 Digital Multiphase Voltage Regulator";
+ rc->phases.exit = isl69259_exit_reset;
+ isl_pmbus_vr_class_init(klass, data, 2);
+}
+
+static const TypeInfo isl69259_info = {
+ .name = TYPE_ISL69259,
+ .parent = TYPE_ISL69260,
+ .class_init = isl69259_class_init,
+};
+
static const TypeInfo isl69260_info = {
.name = TYPE_ISL69260,
.parent = TYPE_PMBUS_DEVICE,
@@ -283,6 +310,7 @@ static const TypeInfo raa228000_info = {
static void isl_pmbus_vr_register_types(void)
{
+ type_register_static(&isl69259_info);
type_register_static(&isl69260_info);
type_register_static(&raa228000_info);
type_register_static(&raa229004_info);
--
2.37.0On Wed, 29 Jun 2022 at 21:52, Peter Delevoryas <me@pjd.dev> 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};
> + g_assert_cmphex(sizeof(ic_device_id), <=, sizeof(s->ic_device_id));
> +
This generates an error from the checkpatch script:
Checking 0010-hw-sensor-Add-Renesas-ISL69259-device-model.patch...
ERROR: Use g_assert or g_assert_not_reached
#27: FILE: hw/sensor/isl_pmbus_vr.c:126:
+ g_assert_cmphex(sizeof(ic_device_id), <=, sizeof(s->ic_device_id));
otherwise, LGTM.
Titus
On Thu, Jun 30, 2022 at 12:16:05PM -0700, Titus Rwantare wrote:
> On Wed, 29 Jun 2022 at 21:52, Peter Delevoryas <me@pjd.dev> 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};
> > + g_assert_cmphex(sizeof(ic_device_id), <=, sizeof(s->ic_device_id));
> > +
>
> This generates an error from the checkpatch script:
> Checking 0010-hw-sensor-Add-Renesas-ISL69259-device-model.patch...
> ERROR: Use g_assert or g_assert_not_reached
> #27: FILE: hw/sensor/isl_pmbus_vr.c:126:
> + g_assert_cmphex(sizeof(ic_device_id), <=, sizeof(s->ic_device_id));
Argghhh I should have caught this, thanks. I'll replace it with g_assert. I
didn't realize there was some kind of portability issue with using
g_assert_cmphex in non-test code.
>
> otherwise, LGTM.
That's great! Thanks for the review. I'll let you and Cedric sort
out if we want to make IC_DEVICE_ID a class property or keep it
in exit_reset as everything else class-specific is right now.
I'll still resubmit the patches as a separate series though with
the g_assert fix and your reviewed-by tags.
>
>
> Titus
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.
> + g_assert_cmphex(sizeof(ic_device_id), <=, sizeof(s->ic_device_id));
> +
> + isl_pmbus_vr_exit_reset(obj);
> +
> + s->ic_device_id_len = sizeof(ic_device_id);
> + memcpy(s->ic_device_id, ic_device_id, sizeof(ic_device_id));
> +}
> +
> static void isl_pmbus_vr_add_props(Object *obj, uint64_t *flags, uint8_t pages)
> {
> PMBusDevice *pmdev = PMBUS_DEVICE(obj);
> @@ -257,6 +269,21 @@ static void raa229004_class_init(ObjectClass *klass, void *data)
> isl_pmbus_vr_class_init(klass, data, 2);
> }
>
> +static void isl69259_class_init(ObjectClass *klass, void *data)
> +{
> + ResettableClass *rc = RESETTABLE_CLASS(klass);
> + DeviceClass *dc = DEVICE_CLASS(klass);
> + dc->desc = "Renesas ISL69259 Digital Multiphase Voltage Regulator";
> + rc->phases.exit = isl69259_exit_reset;
> + isl_pmbus_vr_class_init(klass, data, 2);
> +}
> +
> +static const TypeInfo isl69259_info = {
> + .name = TYPE_ISL69259,
> + .parent = TYPE_ISL69260,
> + .class_init = isl69259_class_init,
> +};
> +
> static const TypeInfo isl69260_info = {
> .name = TYPE_ISL69260,
> .parent = TYPE_PMBUS_DEVICE,
> @@ -283,6 +310,7 @@ static const TypeInfo raa228000_info = {
>
> static void isl_pmbus_vr_register_types(void)
> {
> + type_register_static(&isl69259_info);
> type_register_static(&isl69260_info);
> type_register_static(&raa228000_info);
> type_register_static(&raa229004_info);
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
On 6/30/22 21:16, Titus Rwantare wrote:
> 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.
Sure. Fine for me.
Thanks,
C.
© 2016 - 2026 Red Hat, Inc.