hwmon: (pmbus/max20830): add enable-gpio property and add support for max20830c/max20840c#3357
hwmon: (pmbus/max20830): add enable-gpio property and add support for max20830c/max20840c#3357actorreno wants to merge 4 commits into
Conversation
actorreno
requested review from
CiprianRegus,
amiclaus,
btogorean,
buha,
chegbeli,
danmois,
dbogdan,
machschmitt,
mhennerich,
nunojsa,
rbolboac and
stefpopa
as code owners
Adding a missing entry for the MAX20830 EN (enable) pin. Without this pin device can still be enabled using PMBUS commands Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
Add support for the GPIO controlled EN pin. The EN pin is asserted high for device to operate. If not specified, EN pin can be tied high for device controlled software enable. Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
…port Add compatible strings for variants of MAX20830 which are MAX20830C and MAX20840C. These devices have the same register functionality with MAX20830 but with a longer IC_DEVICE_ID. Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
Add support for MAX20830C and MAX20840 step-down DC-DC switching regulator with PMBus interface. MAX20830C is a different packaging for MAX20830, and MAX20840C supports 40A regulation compared to MAX20830 of only 30A. Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
actorreno
changed the title
| enum: | ||
| - adi,max20830 | ||
| - adi,max20830c | ||
| - adi,max20840c |
There was a problem hiding this comment.
Can the new devices somehow fallback to the adi,max20830 so that we can use fallback compatible? If not, you might be asked to justify it and typically DT maintainers want that in the commit message.
There was a problem hiding this comment.
ah yes, they're so similar that falling back wouldn't change behavior
| {"max20830"}, | ||
| {"max20830", max20830}, | ||
| {"max20830c", max20830c}, | ||
| {"max20840c", max20840c}, |
There was a problem hiding this comment.
I know this is something common in pmbus but I would try it as in IIO? per device struct with a const char * identifier.
There was a problem hiding this comment.
ill try it out, i think it could reduce a lot of the code complexity
| /* MAX20840C variant detected */ | ||
| } else if (!strncmp(buf, "MAX20830", 8)) { | ||
| /* MAX20830 variant detected */ | ||
| } else { |
There was a problem hiding this comment.
Then no need for the above. Just one comparison
| */ | ||
| ret = i2c_smbus_read_i2c_block_data(client, PMBUS_IC_DEVICE_ID, | ||
| MAX20830_IC_DEVICE_ID_LENGTH + 1, | ||
| MAX20830_IC_DEVICE_ID_MAX_LENGTH + 1, |
There was a problem hiding this comment.
Does this actually work for the devices that only have MAX20830_IC_DEVICE_ID_MIN_LENGTH? Anyways, with a per chip struct this becomes again trivial to just use the amount of bytes any device is really needs to be read.
There was a problem hiding this comment.
just to answer so far yes, the i2c read here just returns FF beyond what the chip replies,
but yeah if edited as suggested above, this should be trivial
2 participants
PR Description
Adds a missing entry for enable pin in the driver.
This PR adds support for the Analog Devices MAX20830C and MAX20840C
step-down DC-DC switching regulator with PMBus interface.
This PR is based on top of this: #3183
PR Type
PR Checklist