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 = <c4286_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. Guenter