Re: [PATCH 3/3] hwmon: (pmbus/fs1412) Add TDK FS1412 driver

[Guenter Roeck <linux@roeck-us.net>]

On Tue, Oct 20, 2026 at 12:31:11AM -0700, Abdurrahman Hussain wrote:
> On Mon Oct 19, 2026 at 1:25 PM PDT, Guenter Roeck wrote:
> > This implementation is wrong. Support for relative mode should instead
> > be added to the PMBus core. Or, rather, this driver should be tested with
> > https://patchwork.kernel.org/project/linux-hwmon/patch/20240315151855.377627-2-lars.petter.mostad@appear.net/
> > in place (with appropriate fixes for that patch if necessary), and the
> > patches should be applied together.
> 
> Thanks! That is exactly what I was looking for, but my search for
> fs1412 on lore didn't turn it up anything.
> 
> v2 will carry Lars' patch (Cc'd), rebased, with the driver reduced to
> the MFR_ID check and hiding OT_WARN_LIMIT. v2 also adds the FS1525, a
> 25A part from the same family that is relative-only as well, so the
> core change has two users. Fixes on top of Lars' version:
> 
>  - check VOUT_MODE for 0xff before testing the relative bit; as posted,
>    a chip that does not support VOUT_MODE (reads 0xff) is marked
>    relative and gets a bogus exponent
>  - signed division, and clamp the user value before scaling it
>  - fail probe if VOUT_COMMAND reads 0 rather than divide by it later
>  - only update the cached VOUT_COMMAND after the write succeeded
> 
> We have a board with four FS1412 and one FS1525, so I could test both
> parts with these patches on 7.2.10. Since your concerns in the 2024
> thread were about how these parts actually behave, here is what they

Not that I recall that ;-)

> do:
> 
> VOUT_MODE is read-only and relative-only. Both datasheets mark it
> read-only. FS1412 returns 0x98 (exponent -8), FS1525 returns 0x96
> (exponent -10). Writing 0x18 to VOUT_MODE on all four FS1412 is NACKed,
> STATUS_CML reports invalid data (0x40), and VOUT_MODE stays 0x98.
> 
> Which commands are relative. The FS1525 command table marks
> VOUT_MARGIN_HIGH/LOW, VOUT_OV_FAULT_LIMIT, VOUT_OV_WARN_LIMIT,
> VOUT_UV_WARN_LIMIT, VOUT_UV_FAULT_LIMIT and POWER_GOOD_ON as "L16, 1.3
> relative format". VOUT_COMMAND, VOUT_MAX and READ_VOUT are absolute.
> That matches the list in Lars' patch; POWER_GOOD_ON is relative too,
> but the core does not use it. The FS1412 datasheet says the same for
> the limits it describes. Raw defaults, as a fraction of VOUT_COMMAND:
> 
>                      FS1412 (0x98)   FS1525 (0x96)
>   VOUT_COMMAND       0x009a  0.602V  0x0266  0.600V
>   VOUT_OV_FAULT      0x0133  1.199   0x04cc  1.199
>   VOUT_OV_WARN       0x0126  1.148   0x1000  4.000
>   VOUT_UV_WARN       0x00d9  0.848   0x0366  0.850
>   VOUT_UV_FAULT      0x00cc  0.797   0x0333  0.800
>   POWER_GOOD_ON      0x00e6  0.898   0x0365  0.849
>   VOUT_MARGIN_HIGH   0x0119  1.098   0x0466  1.100
>   VOUT_MARGIN_LOW    0x00e6  0.898   0x0399  0.899
> 
> With the patches the FS1412 reports in2_crit/max/min/lcrit as
> 721/690/509/478 mV against an in2_input of ~560-580 mV. Writing
> in2_max and in2_min stores 0x012e / 0x00d0 and reads back within one
> 1/256 step. On the FS1525, writing 600 to in2_min stores 0x0402 and
> writing 509 restores the original 0x0366. I did not exercise the
> regulator path; the driver does not register a regulator.
> 
> Paging. Neither part implements PAGE. A PAGE read is NACKed and sets
> STATUS_CML INVALID_CMD (0x80), so on these parts relative mode can only
> be per chip. The patch tracks it per page, which also covers a
> multi-page chip that sets the bit on every page.
> 
> Vendors getting it wrong. You were right to worry. Some things I ran
> into:
> 
>  - The OV fault limit is rounded internally to one of four settings
>    (105-120% in 5% steps on the FS1525), so only some values written
>    to it take effect.
>  - The parts are only partly consistent with their own command tables:
>    OT_WARN_LIMIT and READ_IOUT are not listed for the FS1412, but both
>    answer without a CML error, and OT_WARNING sets at room temperature.
>  - The factory programming varies between parts on the same board: one
>    FS1412 has VOUT_MAX 0xa500 and VOUT_MARGIN_HIGH 0xa400, the others
>    0x0300 and 0x0119; another has VOUT_MARGIN_LOW 0x0000. The core does
>    not touch these for hwmon.
>  - The VOUT domain differs between the two parts. FS1412 reports
>    READ_VOUT at the feedback pin, so it is in the same domain as
>    VOUT_COMMAND and the converted limits. TDK told us that the FS1525
>    reports READ_VOUT as the actual output regardless of an external
>    divider. On our board that is 1.094V against a VOUT_COMMAND of 0.600V
>    (a divider for a 1.1V rail).
> 
>    I checked what the FS1525 compares READ_VOUT against. Setting
>    VOUT_OV_WARN_LIMIT to 1.5 sets VOUT_OV_WARNING; setting
>    VOUT_UV_WARN_LIMIT to 1.5 does not. So the part compares the
>    output-domain READ_VOUT against limit * VOUT_COMMAND (0.9V), which
>    is exactly the value the core reports. That is presumably why this
>    board's OV warn limit defaults to 400%.
> 
> One question before v2. The FS1525 over-voltage fault is the exception:
> the datasheet says it is an analog comparison at the FB pin, at the
> VOUT_OV_FAULT_LIMIT fraction of VOUT_COMMAND. So on a board with an
> external divider in2_crit is in feedback-pin volts while in2_input is
> the rail. On our board's FS1525 that is in2_crit 718 mV against
> in2_input 1093 mV, with no alarm, and the actual trip point is about
> 1.31V on the rail. The chip has no way to report the divider. Which
> would you prefer?
> 
>  a) Hide VOUT_OV_FAULT_LIMIT on the FS1525, so there is no in2_crit.
>     Nothing misleading, but boards without a divider lose a correct
>     in2_crit too.
>  b) Scale in2_crit by READ_VOUT / VOUT_COMMAND measured at probe.
>     Right while the rail regulates, wrong if it is off, ramping or
>     margined at probe time.
>  c) Describe the divider in DT and scale in2_crit by it. The FS1525
>     would then need its own binding instead of a trivial-devices entry.
> 
> I lean towards a).
> 
Either a) or c), your call. We already have a vout-voltage-divider
property, but it looks like that won't work here (it sounds like only
the limit is affected ?), so something new/similar would be required.

Thanks,
Guenter