linux-kernel - Re: [PATCH v4 2/2] hwmon: pmbus: Add ltc4286 driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 9 Nov 2023 11:50:19 -0800
From:   Guenter Roeck <linux@...ck-us.net>
To:     Delphine CC Chiu <Delphine_CC_Chiu@...ynn.com>, patrick@...cx.xyz,
        Jean Delvare <jdelvare@...e.com>,
        Jonathan Corbet <corbet@....net>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>, linux-i2c@...r.kernel.org,
        linux-hwmon@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org
Subject: Re: [PATCH v4 2/2] hwmon: pmbus: Add ltc4286 driver

On 11/9/23 09:27, Guenter Roeck wrote:
> On 11/8/23 17:49, Delphine CC Chiu wrote:

>> +        info->m[PSC_POWER] = temp_setting;
>> +    } else {
>> +        info->m[PSC_POWER] = rsense;
> 
> I told you before, the default range needs to be set. The range may have been
> changed by the BIOS/ROMMON, or someone could have changed it manually with
> i2cset or some other application, or some other operating system was loaded
> earlier which did its own setting.
> 
> I do understand by now that you don't want to do that, but I won't accept
> the driver without it, sorry.
> 

As a follow-up on this, it turns out that LTC4287 supports an external EEPROM to
store its configuration as well as "STORE_USER_ALL" and "RESTORE_USER_ALL"
PMBus commands. It is therefore very well possible and even likely that the
voltage range (together with other configurable parameters) was set during
system production.

With that in mind, it seems to me that not only the default (property not present)
case has to be handled, but that a boolean property to reflect the voltage range
is insufficient. I think what is needed instead is a property which specifies
the requested range and which, if not present, explicitly means "use whatever
is currently programmed into the chip".

Thanks,
Guenter