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

[Delphine_CC_Chiu/WYHQ/Wiwynn]

> -----Original Message-----
> From: Guenter Roeck <groeck7@xxxxxxxxx> On Behalf Of Guenter Roeck
> Sent: Thursday, November 16, 2023 6:46 AM
> To: Delphine_CC_Chiu/WYHQ/Wiwynn <Delphine_CC_Chiu@xxxxxxxxxx>
> Cc: patrick@xxxxxxxxx; Jean Delvare <jdelvare@xxxxxxxx>; Jonathan Corbet
> <corbet@xxxxxxx>; 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 Wed, Nov 15, 2023 at 08:42:22AM +0000,
> Delphine_CC_Chiu/WYHQ/Wiwynn wrote:
> > > -----Original Message-----
> > > From: Guenter Roeck <groeck7@xxxxxxxxx> On Behalf Of Guenter Roeck
> > > Sent: Tuesday, November 7, 2023 11:30 AM
> > > 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 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.
> >
> > We still can’t get your point.
> > Could you tell us about your concern here?
> 
> I have repeated it several times. You are making assumptions about register
> values when the driver is loaded. Those asumptions are wrong since the state
> of the chip is unknown when the driver is loaded. Any entty (BIOS, ROMMON,
> i2cset, some operating system loaded earlier, or even some other driver or
> platform code) may have changed those values.
> 
> On top of that, as I also have pointed out, LTC4287 supports saving its
> configuration data in eeprom. That means that any chip configuration set
> during production or anytime later will be retained, meaning any assumption
> about chip configuration when the driver is loaded is even more wrong.

OK
We will check the register value first.
Then, we will check the property in dts.

> 
> Guenter