Re: [PATCH 3/3] ir38064: Expose a regulator

[Guenter Roeck <linux@xxxxxxxxxxxx>]

On 12/13/21 2:19 AM, arthur@xxxxxxxxxxxx wrote:
> From: Patrick Rudolph <patrick.rudolph@xxxxxxxxxxxxx>
>
> Signed-off-by: Patrick Rudolph <patrick.rudolph@xxxxxxxxxxxxx>
> ---
>   drivers/hwmon/pmbus/Kconfig   |  6 ++++++
>   drivers/hwmon/pmbus/ir38064.c | 17 +++++++++++++++++
>   2 files changed, 23 insertions(+)
>
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index d2b178a902f0..fdfe1cd54799 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -131,6 +131,12 @@ config SENSORS_IR38064
>   	  This driver can also be built as a module. If so, the module will
>   	  be called ir38064.
>   
> +config SENSORS_IR38064_REGULATOR
> +	bool "Regulator support for IR38064 and compatibles"
> +	depends on SENSORS_IR38064 && REGULATOR
> +	help
> +	  Uses the IR38064 or compatible as regulator.
> +
>   config SENSORS_IRPS5401
>   	tristate "Infineon IRPS5401"
>   	help
> diff --git a/drivers/hwmon/pmbus/ir38064.c b/drivers/hwmon/pmbus/ir38064.c
> index 429577b027b4..61cbe5063688 100644
> --- a/drivers/hwmon/pmbus/ir38064.c
> +++ b/drivers/hwmon/pmbus/ir38064.c
> @@ -17,8 +17,15 @@
>   #include <linux/kernel.h>
>   #include <linux/module.h>
>   #include <linux/of_device.h>
> +#include <linux/regulator/driver.h>
>   #include "pmbus.h"
>   
> +#if IS_ENABLED(CONFIG_SENSORS_IR38064_REGULATOR)
> +static const struct regulator_desc ir38064_reg_desc[] = {
> +	PMBUS_REGULATOR("vout", 0),
> +};
> +#endif /* CONFIG_SENSORS_IR38064_REGULATOR */
> +
>   static struct pmbus_driver_info ir38064_info = {
>   	.pages = 1,
>   	.format[PSC_VOLTAGE_IN] = linear,
> @@ -33,9 +40,19 @@ static struct pmbus_driver_info ir38064_info = {
>   	    | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP
>   	    | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
>   	    | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT
> +#if IS_ENABLED(CONFIG_SENSORS_IR38064_REGULATOR)
> +#ifdef PMBUS_HAVE_VOUT_COMMAND
> +		| PMBUS_HAVE_VOUT_COMMAND
> +#endif
> +#endif

Please drop. PMBUS_HAVE_VOUT_COMMAND is not defined in the upstream kernel.
Yes, code introducing it was suggested two years ago, and I rejected it.
The reasons are still valid. If you have out-of tree code which enables
setting voltages values and voltage margins with hwmon sysfs attributes,
please keep it out-of-tree.

Thanks,
Guenter

>   	    | PMBUS_HAVE_POUT,
> +#if IS_ENABLED(CONFIG_SENSORS_IR38064_REGULATOR)
> +	.num_regulators = 1,
> +	.reg_desc = ir38064_reg_desc,
> +#endif
>   };
>   
> +
>   static int ir38064_probe(struct i2c_client *client)
>   {
>   	return pmbus_do_probe(client, &ir38064_info);