From: Guenter Roeck <linux@roeck-us.net>
To: Lars Petter Mostad <larspm@gmail.com>
Cc: jdelvare@suse.com, linux-hwmon@vger.kernel.org,
Lars Petter Mostad <lars.petter.mostad@appear.net>
Subject: Re: [PATCH 0/1] hwmon: (pmbus_core) Relative encoding of vout related commands
Date: Sat, 16 Mar 2024 08:19:49 -0700 [thread overview]
Message-ID: <41213d44-a72a-40e3-a858-4a66a3107139@roeck-us.net> (raw)
In-Reply-To: <CAC-Dm24yn_aeTVjRofst+NGtdzhDDhtouJSxY_bw3yCwZPb1Jg@mail.gmail.com>
On 3/16/24 06:33, Lars Petter Mostad wrote:
> On Fri, Mar 15, 2024 at 7:05 PM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On 3/15/24 08:18, Lars Petter Mostad wrote:
>>> PMBus 1.3 allows for relative encoding of voltages in certain output
>>> voltage related commands. I'm starting this thread to ask if adding support
>>> for this, either by me or someone else, is of interest. I have made an
>>> initial attempt to add support that is adequate for my needs. I have
>>> included the patch for discussion. I don't expect it to be accepted as is.
>>>
>>> I'm working on the software for an ARM64 based board that uses the
>>> TDK FS1412, which uses relative encoding. This is the only chip that uses
>>> this feature that I have access to. I only have access to this chip on this
>>> board, so I'm only able to test it on this architecture. I have to use the
>>> kernel sources from NXPs LLDP project, which ATTOW is based on kernel
>>> 6.1.55. I'm not able to test on a newer kernel at the moment. The board is
>>> fairly expensive, so I don't dare test setting voltages. I have only tested
>>> monitoring voltages.
>>>
>>> In relative mode, certain output voltage related commands (e.g.
>>> VOUT_MARGIN_HIGH) are encoded relative to the nominal value (VOUT_COMMAND).
>>> If e.g. the high margin is to be 10% above the nominal value,
>>> VOUT_MARGIN_HIGH is set to 1.1. This factor is encoded the same way as
>>> VOUT_COMMAND (i.e. as indicated by VOUT_MODE).
>>>
>>> I have been a little unsure about how to best organize the changes.
>>> Decoding the relative values requires access to the current value of
>>> VOUT_COMMAND. I decided to handle it the same way exponent is handled.
>>> I placed the relevant information next to exponent in pmbus_data.
>>> vout_mode_relative is true if relative encoding is used, vout_command
>>> stores what is believed to be the current VOUT_COMMAND. As exponent,
>>> vout_mode_relative is set during decoding of VOUT_MODE in
>>> pmbus_identify_common.
>>>
>>> I decided to assume that VOUT_COMMAND is not changed outside the driver,
>>> as this is also done for VOUT_MODE. VOUT_COMMAND is read and saved during
>>> pmbus_init_common. It is also saved to vout_command during
>>> pmbus_regulator_set_voltage.
>>>
>>> pmbus_regulator_set_voltage usually clamps the requested voltage to be
>>> between VOUT_MARGIN_LOW and VOUT_MARGIN_HIGH. This does not make sense in
>>> relative mode, as the margins move with the requested voltage. This part is
>>> skipped in relative mode.
>>>
>>> pmbus_reg2data does the actual relative decoding. The pmbus_reg2data_*
>>> functions return the register value multiplied by 1000, which in relative
>>> mode means the permilleage of the nominal value. If we are in relative mode,
>>> and the current register is one of the registers that can use relative
>>> decoding, the permilleage value is multiplied with the stored vout_command.
>>> Currently, the vout_command in pmbus_data is stored decoded, i.e. 1000x and
>>> rounded to an integer. Better precision could be achieved if the
>>> multiplication was done inside each pmbus_reg2data_*, before the rounding
>>> to integer, but this would be a bit messier.
>>>
>>> I welcome comments on wether this is of interest, and what is needed to
>>> get it into an acceptable state!
>>>
>>
>> My major concern is unintended side effects. In drivers/hwmon/pmbus/tps546d24.c
>> we "solve" the problem by explicitly setting absolute mode. Would that help
>> for the time being ?
>>
>> I'd prefer to get some experience with actual chips (meaning evaluation boards)
>> before trying to provide a supposedly generic solution which retains relative mode.
>> I ordered a couple of evaluation boards for TPS546D24 variants. The evaluation
>> board for FS1412 is a bit too expensive for my liking, so I don't really want
>> to order that one. Do you know of any other chips supporting this functionality ?
>>
>> Thanks,
>> Guenter
>>
>
> I did try the solution used for the TI chip by using i2cset to write VOUT_MODE
> before binding the driver, but the command only returns "Error: Write failed"
> for this address. I guess this chip is relative mode only.
>
Yes. I looked into the datasheet. It says the VOUT_MODE command is read-only
and that it only supports relative mode.
NCP3286 is another chip which, as it looks like, only supports relative mode.
Unfortunately there is no evaluation board available for that chip, but
it does mean that we'll have to add support for it.
I'll bite the bullet and buy the FS1412 evaluation board. I really don't want
to apply a change like this without testing myself with multiple chips.
This is one of the PMBus functionalities where it is likely that chip vendors
get something wrong, and playing with VOUT can be critical.
The PMBus standard suggests that relative mode should apply to the entire chip,
not just to a single page. This will be one thing to confirm. Of course that will
be difficult to confirm with only two chips to test, especially since tps546d24 is
single-page and fs1412 is always in relative mode.
If anyone reading this knows of another chip supporting VOUT relative mode,
please let me know.
Thanks,
Guenter
prev parent reply other threads:[~2024-03-16 15:19 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-15 15:18 [PATCH 0/1] hwmon: (pmbus_core) Relative encoding of vout related commands Lars Petter Mostad
2024-03-15 15:18 ` [PATCH 1/1] hwmon: (pmbus_core) Support relative " Lars Petter Mostad
2024-03-15 18:05 ` [PATCH 0/1] hwmon: (pmbus_core) Relative " Guenter Roeck
2024-03-16 13:33 ` Lars Petter Mostad
2024-03-16 15:19 ` Guenter Roeck [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=41213d44-a72a-40e3-a858-4a66a3107139@roeck-us.net \
--to=linux@roeck-us.net \
--cc=jdelvare@suse.com \
--cc=lars.petter.mostad@appear.net \
--cc=larspm@gmail.com \
--cc=linux-hwmon@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).