Re: [PATCH 2/6] hwmon: (pmbus/max20830): add support for enable GPIO

[Guenter Roeck]

On 6/29/26 19:46, Alexis Czezar Torreno wrote:
> Add support for the GPIO controlled EN pin. The EN pin is asserted high
> for device to operate.
>
> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@xxxxxxxxxx>
> ---
>   drivers/hwmon/pmbus/max20830.c | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/drivers/hwmon/pmbus/max20830.c b/drivers/hwmon/pmbus/max20830.c
> index cb2c23672166d641852199ca07eb716924f4f286..cb3a39d747edee3aefb0fb4051ef957436b3c15b 100644
> --- a/drivers/hwmon/pmbus/max20830.c
> +++ b/drivers/hwmon/pmbus/max20830.c
> @@ -6,6 +6,7 @@
>    */
>   
>   #include <linux/errno.h>
> +#include <linux/gpio/consumer.h>
>   #include <linux/i2c.h>
>   #include <linux/mod_devicetable.h>
>   #include <linux/module.h>
> @@ -29,8 +30,14 @@ static struct pmbus_driver_info max20830_info = {
>   static int max20830_probe(struct i2c_client *client)
>   {
>   	u8 buf[I2C_SMBUS_BLOCK_MAX + 1] = {};
> +	struct gpio_desc *enable_gpio;
>   	int ret;
>   
> +	enable_gpio = devm_gpiod_get_optional(&client->dev, "enable", GPIOD_OUT_HIGH);
> +	if (IS_ERR(enable_gpio))
> +		return dev_err_probe(&client->dev, PTR_ERR(enable_gpio),
> +				     "Failed to get enable GPIO\n");
> +

The above code gets the gpio reference, and then it doesn't do anything
with it. What exactly is the point of this exercise ? Where is the
chip actually enabled ?

Do you have an actual customer with such a set-up or is this
"just in case" ? Have you tested this code to ensure that the chip
is actually enabled in this setup ?

If there is indeed a use case where a customer indeed connects the
enable pin to a gpio output, wouldn't that same customer also want
to connect the "pgood" output to a gpio pin ? And what about
the LDOIN pin ? Shouldn't that be connected to a power supply ?

Guenter