Staging/max20826 support - #3431
Conversation
f9f13ed to
0539a3f
Compare
LLM reviewThis series adds PMBus hwmon driver support for MAX20826/MAX20855B/MAX20908/MAX20912/MAX20916 multiphase regulators, along with three PMBus core patches (regulator enable fix, phase count increase, read_block_data hook). run: 29251533245
|
0539a3f to
fe3685a
Compare
LLM reviewThis series adds PMBus hwmon/regulator driver support for MAX20826/MAX20855B/MAX20908/MAX20912/MAX20916 multiphase regulators, plus three pmbus_core fixes (regulator enable return value, phase count to 16, read_block_data hook). run: 29264461327
|
4e21503 to
97e15ef
Compare
fe3685a to
000f1c8
Compare
LLM reviewThis series adds PMBus hwmon driver support for MAX20826/MAX20855B/MAX20908/MAX20912/MAX20916 run: 29316844925
|
000f1c8 to
0be7b8a
Compare
LLM reviewThis series adds PMBus hwmon driver support for MAX20826/MAX20855B/MAX20908/MAX20912/MAX20916 run: 29320436295
|
0be7b8a to
2a92a56
Compare
LLM reviewThis series adds PMBus hwmon driver support for MAX20826/MAX20855B/MAX20908/MAX20912/MAX20916 run: 29338694846
|
2a92a56 to
6f837e7
Compare
| /* | ||
| * OPERATION | ||
| */ | ||
| #define PB_OPERATION_CONTROL_V_SRC GENMASK(5, 4) |
There was a problem hiding this comment.
as a total stranger to pmbus, I wanted to find where this is defined. Now that I found it, feel free to use the citation in your commit message if you want:
12.1.3. OPERATION Command Bits [5:4]
https://pmbus.org/wp-content/uploads/2022/01/PMBus-Specification-Rev-1-3-1-Part-II-20150313.pdf
LLM reviewThis series adds PMBus hwmon/regulator driver support for MAX20826/MAX20855B/MAX20908/MAX20912/MAX20916 run: 29344289845
|
sipraga
left a comment
There was a problem hiding this comment.
quite a weak review from me as I'm not at all familiar with pmbus, but having said that, it looks very good to me
721dbd5 to
4010bbc
Compare
4010bbc to
25c3559
Compare
| return ret; | ||
|
|
||
| /* Let's see if RAIL_B is present */ | ||
| rail = max20826_select_rail(st, RAIL_B, true); |
There was a problem hiding this comment.
this line changes page to RAIL B
if no bvren declared, driver can still probe, but it's on rail B rather than rail A
around line 920 or before 'ret = max20826_setup_vout_format(st);'
should we reselect RAIL A?
There was a problem hiding this comment.
should we revert to the older code style
- check bvren gpio
- if(bvren)
select rail b
do rail B code stuff
There was a problem hiding this comment.
Hmm yeah seems right! Will fix this
There was a problem hiding this comment.
So, in the end I ignored this because of two things:
- bvren is an optional GPIO. RAIL_B enable pin can be something always on with no control from Linux;
- max20826_setup_vout_format() already selects the proper RAIL it needs so being left in RAIL_B should not be an issue
There was a problem hiding this comment.
I guess that's what it means to be gpio_optional, ok I get it. :)
| if (ret < 0) | ||
| goto out_restore_oper; | ||
|
|
||
| n_phases = field_get(st->chip_info->phase_num_mask, status_mon[1]); |
There was a problem hiding this comment.
missing defintion of this small letters 'field_get'
innersource version has:
#define field_get(_mask, _reg) (_reg & _mask) >> __builtin_ctz(_mask);
because the actual FIELD_GET wanted a constant for the mask field
(not sure if there's a cleaner way to do this, and am surprised this built with no error)
There was a problem hiding this comment.
not an issue! upstream kernel already defines field_get() for non constant masks
| for (unsigned int page = 0; page < info->pages; page++) { | ||
| if (!st->chip_info->select_vrm) { | ||
| /* only vr13 in this case */ | ||
| st->info.vrm_version[page] = vr13; |
There was a problem hiding this comment.
max209xx devices are VR12,
There was a problem hiding this comment.
Will double check this one
8411d1b to
a17a53a
Compare
6f837e7 to
f6bd462
Compare
|
Changes in v2: |
f6bd462 to
c425249
Compare
|
Changes in v2:
|
Increase the number of phases to 16 as a new upcoming device supports such a number. While at it, add a new mask for controlling the source of the output voltage. Signed-off-by: Nuno Sá <nuno.sa@analog.com>
This is in preparation for adding support to a device which needs to use it's own read_block implementation. The MAX20826 family is one such device. When not in PMBus page mode, each of its two rails sits at a different I2C address, so selecting the rail cannot be done through pmbus_set_page() and has to be handled by the driver. Signed-off-by: Nuno Sá <nuno.sa@analog.com>
The MAX20826 IC and similar provide a high-density, flexible and scalable dual-loop solution for high current cores for AI applications. These are dual loop solutions multiphase voltage regulators. Between Rails A and B, MAX20855B and MAX20908 supports up to 8 phases total configurable from 8+0 to 4+4 phases, MAX20912 supports up to 12 phases from 12+0 to 6+6, and MAX20826 and MAX20916 supports up to 16 phases from 16+0 to 8+8. The output voltage of each rail is controlled either through PMBus or through the high-speed processor voltage-control interface (Intel SVID, AMD SVI3, Nvidia PWMVID or AVSBus, depending on the device). Which one is in use is part of the configuration the device powers up with and is read back from it, hence adi,rail-a-high-speed and adi,rail-b-high-speed only tell whether the high-speed interface or PMBus controls the output voltage of the respective rail. Co-developed-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
The MAX20826 IC and similar provide a high-density, flexible and scalable dual-loop solution for high current cores for AI applications. These are dual loop solutions multiphase voltage regulators. Between Rails A and B, MAX20855B and MAX20908 supports up to 8 phases total configurable from 8+0 to 4+4 phases, MAX20912 supports up to 12 phases from 12+0 to 6+6, and MAX20826 and MAX20916 supports up to 16 phases from 16+0 to 8+8. Co-developed-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
|
@gastmaier looks like our mirrors are somehow not updating... |
PR Description
Just opening for the llm review. If someone else wants to add some reviewing, also welcome to do so.
PR Type
PR Checklist