Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Sat, 21 Mar 2020 15:09:43 -0700
From:   Guenter Roeck <linux@roeck-us.net>
To:     Grant Peltier <grantpeltier93@gmail.com>
Cc:     linux-hwmon@vger.kernel.org, linux-kernel@vger.kernel.org,
        adam.vaughn.xh@renesas.com
Subject: Re: [PATCH v3 1/2] hwmon: (pmbus) add support for 2nd Gen Renesas
 digital multiphase
Message-ID: <20200321220943.GA22233@roeck-us.net>
References: <cover.1584720563.git.grantpeltier93@gmail.com>
 <62c000adf0108aeb65d3f275f28eb26b690384aa.1584720563.git.grantpeltier93@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <62c000adf0108aeb65d3f275f28eb26b690384aa.1584720563.git.grantpeltier93@gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Mar 20, 2020 at 11:16:21AM -0500, Grant Peltier wrote:
> Extend the isl68137 driver to provide support for 2nd generation Renesas
> digital multiphase voltage regulators.
> 
> Signed-off-by: Grant Peltier <grantpeltier93@gmail.com>

Applied.

Thanks,
Guenter

> ---
>  drivers/hwmon/pmbus/Kconfig    |   6 +-
>  drivers/hwmon/pmbus/isl68137.c | 110 ++++++++++++++++++++++++++++-----
>  2 files changed, 98 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index a9ea06204767..1e3e5a61ed9c 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -92,10 +92,10 @@ config SENSORS_IRPS5401
>  	  be called irps5401.
>  
>  config SENSORS_ISL68137
> -	tristate "Intersil ISL68137"
> +	tristate "Renesas Digital Multiphase Voltage Regulators"
>  	help
> -	  If you say yes here you get hardware monitoring support for Intersil
> -	  ISL68137.
> +	  If you say yes here you get hardware monitoring support for Renesas
> +	  digital multiphase voltage regulators.
>  
>  	  This driver can also be built as a module. If so, the module will
>  	  be called isl68137.
> diff --git a/drivers/hwmon/pmbus/isl68137.c b/drivers/hwmon/pmbus/isl68137.c
> index 515596c92fe1..47f6cce1da58 100644
> --- a/drivers/hwmon/pmbus/isl68137.c
> +++ b/drivers/hwmon/pmbus/isl68137.c
> @@ -1,8 +1,9 @@
>  // SPDX-License-Identifier: GPL-2.0+
>  /*
> - * Hardware monitoring driver for Intersil ISL68137
> + * Hardware monitoring driver for Renesas Digital Multiphase Voltage Regulators
>   *
>   * Copyright (c) 2017 Google Inc
> + * Copyright (c) 2020 Renesas Electronics America
>   *
>   */
>  
> @@ -14,9 +15,19 @@
>  #include <linux/module.h>
>  #include <linux/string.h>
>  #include <linux/sysfs.h>
> +
>  #include "pmbus.h"
>  
>  #define ISL68137_VOUT_AVS	0x30
> +#define RAA_DMPVR2_READ_VMON	0xc8
> +
> +enum versions {
> +	isl68137,
> +	raa_dmpvr2_1rail,
> +	raa_dmpvr2_2rail,
> +	raa_dmpvr2_3rail,
> +	raa_dmpvr2_hv,
> +};
>  
>  static ssize_t isl68137_avs_enable_show_page(struct i2c_client *client,
>  					     int page,
> @@ -98,13 +109,30 @@ static const struct attribute_group enable_group = {
>  	.attrs = enable_attrs,
>  };
>  
> -static const struct attribute_group *attribute_groups[] = {
> +static const struct attribute_group *isl68137_attribute_groups[] = {
>  	&enable_group,
>  	NULL,
>  };
>  
> -static struct pmbus_driver_info isl68137_info = {
> -	.pages = 2,
> +static int raa_dmpvr2_read_word_data(struct i2c_client *client, int page,
> +				     int reg)
> +{
> +	int ret;
> +
> +	switch (reg) {
> +	case PMBUS_VIRT_READ_VMON:
> +		ret = pmbus_read_word_data(client, page, RAA_DMPVR2_READ_VMON);
> +		break;
> +	default:
> +		ret = -ENODATA;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static struct pmbus_driver_info raa_dmpvr_info = {
> +	.pages = 3,
>  	.format[PSC_VOLTAGE_IN] = direct,
>  	.format[PSC_VOLTAGE_OUT] = direct,
>  	.format[PSC_CURRENT_IN] = direct,
> @@ -113,7 +141,7 @@ static struct pmbus_driver_info isl68137_info = {
>  	.format[PSC_TEMPERATURE] = direct,
>  	.m[PSC_VOLTAGE_IN] = 1,
>  	.b[PSC_VOLTAGE_IN] = 0,
> -	.R[PSC_VOLTAGE_IN] = 3,
> +	.R[PSC_VOLTAGE_IN] = 2,
>  	.m[PSC_VOLTAGE_OUT] = 1,
>  	.b[PSC_VOLTAGE_OUT] = 0,
>  	.R[PSC_VOLTAGE_OUT] = 3,
> @@ -133,24 +161,76 @@ static struct pmbus_driver_info isl68137_info = {
>  	    | PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2
>  	    | PMBUS_HAVE_TEMP3 | PMBUS_HAVE_STATUS_TEMP
>  	    | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
> -	    | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_POUT,
> -	.func[1] = PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
> -	    | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_POUT,
> -	.groups = attribute_groups,
> +	    | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_POUT
> +		| PMBUS_HAVE_VMON,
> +	.func[1] = PMBUS_HAVE_IIN | PMBUS_HAVE_PIN | PMBUS_HAVE_STATUS_INPUT
> +	    | PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP3 | PMBUS_HAVE_STATUS_TEMP
> +	    | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_IOUT
> +	    | PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_POUT,
> +	.func[2] = PMBUS_HAVE_IIN | PMBUS_HAVE_PIN | PMBUS_HAVE_STATUS_INPUT
> +	    | PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP3 | PMBUS_HAVE_STATUS_TEMP
> +	    | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_IOUT
> +	    | PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_POUT,
>  };
>  
>  static int isl68137_probe(struct i2c_client *client,
>  			  const struct i2c_device_id *id)
>  {
> -	return pmbus_do_probe(client, id, &isl68137_info);
> +	struct pmbus_driver_info *info;
> +
> +	info = devm_kzalloc(&client->dev, sizeof(*info), GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +	memcpy(info, &raa_dmpvr_info, sizeof(*info));
> +
> +	switch (id->driver_data) {
> +	case isl68137:
> +		info->pages = 2;
> +		info->R[PSC_VOLTAGE_IN] = 3;
> +		info->func[0] &= ~PMBUS_HAVE_VMON;
> +		info->func[1] = PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
> +		    | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT
> +		    | PMBUS_HAVE_POUT;
> +		info->groups = isl68137_attribute_groups;
> +		break;
> +	case raa_dmpvr2_1rail:
> +		info->pages = 1;
> +		info->read_word_data = raa_dmpvr2_read_word_data;
> +		break;
> +	case raa_dmpvr2_2rail:
> +		info->pages = 2;
> +		info->read_word_data = raa_dmpvr2_read_word_data;
> +		break;
> +	case raa_dmpvr2_3rail:
> +		info->read_word_data = raa_dmpvr2_read_word_data;
> +		break;
> +	case raa_dmpvr2_hv:
> +		info->pages = 1;
> +		info->R[PSC_VOLTAGE_IN] = 1;
> +		info->m[PSC_VOLTAGE_OUT] = 2;
> +		info->R[PSC_VOLTAGE_OUT] = 2;
> +		info->m[PSC_CURRENT_IN] = 2;
> +		info->m[PSC_POWER] = 2;
> +		info->R[PSC_POWER] = -1;
> +		info->read_word_data = raa_dmpvr2_read_word_data;
> +		break;
> +	default:
> +		return -ENODEV;
> +	}
> +
> +	return pmbus_do_probe(client, id, info);
>  }
>  
> -static const struct i2c_device_id isl68137_id[] = {
> -	{"isl68137", 0},
> +static const struct i2c_device_id raa_dmpvr_id[] = {
> +	{"isl68137", isl68137},
> +	{"raa_dmpvr2_1rail", raa_dmpvr2_1rail},
> +	{"raa_dmpvr2_2rail", raa_dmpvr2_2rail},
> +	{"raa_dmpvr2_3rail", raa_dmpvr2_3rail},
> +	{"raa_dmpvr2_hv", raa_dmpvr2_hv},
>  	{}
>  };
>  
> -MODULE_DEVICE_TABLE(i2c, isl68137_id);
> +MODULE_DEVICE_TABLE(i2c, raa_dmpvr_id);
>  
>  /* This is the driver that will be inserted */
>  static struct i2c_driver isl68137_driver = {
> @@ -159,11 +239,11 @@ static struct i2c_driver isl68137_driver = {
>  		   },
>  	.probe = isl68137_probe,
>  	.remove = pmbus_do_remove,
> -	.id_table = isl68137_id,
> +	.id_table = raa_dmpvr_id,
>  };
>  
>  module_i2c_driver(isl68137_driver);
>  
>  MODULE_AUTHOR("Maxim Sloyko <maxims@google.com>");
> -MODULE_DESCRIPTION("PMBus driver for Intersil ISL68137");
> +MODULE_DESCRIPTION("PMBus driver for Renesas digital multiphase voltage regulators");
>  MODULE_LICENSE("GPL");