Re: [PATCH v2 2/2] hwmon: pmbus: Add ltc4286 driver

[Guenter Roeck]

On 11/6/23 19:08, Delphine_CC_Chiu/WYHQ/Wiwynn wrote:
> > -----Original Message-----
> > From: Guenter Roeck <groeck7@xxxxxxxxx> On Behalf Of Guenter Roeck
> > Sent: Tuesday, October 31, 2023 9:47 PM
> > To: Delphine_CC_Chiu/WYHQ/Wiwynn <Delphine_CC_Chiu@xxxxxxxxxx>;
> > patrick@xxxxxxxxx; Jean Delvare <jdelvare@xxxxxxxx>; Jonathan Corbet
> > <corbet@xxxxxxx>
> > Cc: Rob Herring <robh+dt@xxxxxxxxxx>; Krzysztof Kozlowski
> > <krzysztof.kozlowski+dt@xxxxxxxxxx>; Conor Dooley <conor+dt@xxxxxxxxxx>;
> > linux-i2c@xxxxxxxxxxxxxxx; linux-hwmon@xxxxxxxxxxxxxxx;
> > devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> > linux-doc@xxxxxxxxxxxxxxx
> > Subject: Re: [PATCH v2 2/2] hwmon: pmbus: Add ltc4286 driver
> >
> >    Security Reminder: Please be aware that this email is sent by an external
> > sender.
> >
> > On 10/30/23 23:46, Delphine_CC_Chiu/WYHQ/Wiwynn wrote:
> > [ ... ]
> > > > > +
> > > > > +     ret = of_property_read_u32(client->dev.of_node,
> > > > > +                                "shunt-resistor-micro-ohms",
> > > > &rsense);
> > > > > +     if (ret < 0)
> > > > > +             return ret;
> > > > > +
> > > > > +     if (rsense == 0)
> > > > > +             return -EINVAL;
> > > > > +
> > > > > +     info = &ltc4286_info;
> > > > > +
> > > > > +     /* Default of VRANGE_SELECT = 1, 102.4V */
> > > > > +     if (device_property_read_bool(&client->dev,
> > > > "adi,vrange-select-25p6")) {
> > > >
> > > > What if the adi,vrange-select-25p6 property is not provided, but the
> > > > chip is programmed for this range ?
> > > The binding document tells programmers how to fill the dts.
> > > Thus, programmers must fill this property if their system is 25.6 volts voltage
> > range.
> > >
> >
> > Sure, but there is no else case, meaning VRANGE_SELECT is unmodified in that
> > case. There is no guarantee that the chip is in its power-on state.
>
> The else case is in v2 ltc4286.c line 133
> It means that the voltage range for programmer is 102.4 volts which is default value,
> so driver doesn't need to do any change for VRANGE_SELECT bit.

There is no guarantee that the value wasn't changed before the driver was loaded.

Guenter

> Additionally, we have checked the behavior of VRANGE_SELECT bit with vendor.
> Below is the reply from vendor:
> [Our question]
> If we change the VRANGE_SELECT bit value to 0, the value would return to default after we restart this device?
> Or VRANGE_SELECT bit value would never change util someone change its value?
> [ADI reply]
> chip reset will go back to default. Thanks.
>
> Thus, we must overwrite this bit if the user fill adi,vrange-select-25p6 here whenever driver probes
>
> > Guenter