All of lore.kernel.org
 help / color / mirror / Atom feed
* [lm-sensors] [PATCH 2/2] hwmon: (ltc2978) Add support for LTC2974 and LTC3883
@ 2013-02-05 15:56 Guenter Roeck
  2013-02-19 12:28 ` Jean Delvare
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Guenter Roeck @ 2013-02-05 15:56 UTC (permalink / raw)
  To: lm-sensors

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 Documentation/hwmon/ltc2978   |   26 +++++---
 drivers/hwmon/pmbus/Kconfig   |    4 +-
 drivers/hwmon/pmbus/ltc2978.c |  136 +++++++++++++++++++++++++++++++++++++----
 3 files changed, 143 insertions(+), 23 deletions(-)

diff --git a/Documentation/hwmon/ltc2978 b/Documentation/hwmon/ltc2978
index 8a5e791..8db0b61 100644
--- a/Documentation/hwmon/ltc2978
+++ b/Documentation/hwmon/ltc2978
@@ -2,6 +2,10 @@ Kernel driver ltc2978
 ========== 
 Supported chips:
+  * Linear Technology LTC2974
+    Prefix: 'ltc2974'
+    Addresses scanned: -
+    Datasheet: http://cds.linear.com/docs/en/datasheet/2974f.pdf
   * Linear Technology LTC2978
     Prefix: 'ltc2978'
     Addresses scanned: -
@@ -10,6 +14,10 @@ Supported chips:
     Prefix: 'ltc3880'
     Addresses scanned: -
     Datasheet: http://cds.linear.com/docs/en/datasheet/3880fa.pdf
+  * Linear Technology LTC3883
+    Prefix: 'ltc3883'
+    Addresses scanned: -
+    Datasheet: http://cds.linear.com/docs/en/datasheet/3883f.pdf
 
 Author: Guenter Roeck <guenter.roeck@ericsson.com>
 
@@ -17,9 +25,9 @@ Author: Guenter Roeck <guenter.roeck@ericsson.com>
 Description
 -----------
 
-The LTC2978 is an octal power supply monitor, supervisor, sequencer and
-margin controller. The LTC3880 is a dual, PolyPhase DC/DC synchronous
-step-down switching regulator controller.
+LTC2974 is a quad digital power supply manager. LTC2978 is an octal power supply
+monitor. LTC3880 is a dual output poly-phase step-down DC/DC controller. LTC3883
+is a single phase step-down DC/DC controller.
 
 
 Usage Notes
@@ -48,7 +56,7 @@ in1_min_alarm		Input voltage low alarm.
 in1_max_alarm		Input voltage high alarm.
 in1_lcrit_alarm		Input voltage critical low alarm.
 in1_crit_alarm		Input voltage critical high alarm.
-in1_lowest		Lowest input voltage. LTC2978 only.
+in1_lowest		Lowest input voltage. LTC2974 and LTC2978 only.
 in1_highest		Highest input voltage.
 in1_reset_history	Reset history. Writing into this attribute will reset
 			history for all attributes.
@@ -63,7 +71,7 @@ in[2-9]_min_alarm	Output voltage low alarm.
 in[2-9]_max_alarm	Output voltage high alarm.
 in[2-9]_lcrit_alarm	Output voltage critical low alarm.
 in[2-9]_crit_alarm	Output voltage critical high alarm.
-in[2-9]_lowest		Lowest output voltage. LTC2978 only.
+in[2-9]_lowest		Lowest output voltage. LTC2974 and LTC2978 only.
 in[2-9]_highest		Lowest output voltage.
 in[2-9]_reset_history	Reset history. Writing into this attribute will reset
 			history for all attributes.
@@ -82,20 +90,20 @@ temp[1-3]_min_alarm	Chip temperature low alarm.
 temp[1-3]_max_alarm	Chip temperature high alarm.
 temp[1-3]_lcrit_alarm	Chip temperature critical low alarm.
 temp[1-3]_crit_alarm	Chip temperature critical high alarm.
-temp[1-3]_lowest	Lowest measured temperature. LTC2978 only.
+temp[1-3]_lowest	Lowest measured temperature. LTC2974 and LTC2978 only.
 temp[1-3]_highest	Highest measured temperature.
 temp[1-3]_reset_history	Reset history. Writing into this attribute will reset
 			history for all attributes.
 
-power[1-2]_label	"pout[1-2]". LTC3880 only.
+power[1-2]_label	"pout[1-2]". LTC2974, LTC3880, LTC3883 only.
 power[1-2]_input	Measured power.
 
-curr1_label		"iin". LTC3880 only.
+curr1_label		"iin". LTC3880 and LTC3883 only.
 curr1_input		Measured input current.
 curr1_max		Maximum input current.
 curr1_max_alarm		Input current high alarm.
 
-curr[2-3]_label		"iout[1-2]". LTC3880 only.
+curr[2-3]_label		"iout[1-2]". LTC3880 and LTC3883 only.
 curr[2-3]_input		Measured input current.
 curr[2-3]_max		Maximum input current.
 curr[2-3]_crit		Critical input current.
diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index 4f9eb0a..b1adad6 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -48,11 +48,11 @@ config SENSORS_LM25066
 	  be called lm25066.
 
 config SENSORS_LTC2978
-	tristate "Linear Technologies LTC2978 and LTC3880"
+	tristate "Linear Technologies LTC2974, LTC2978, LTC3880, and LTC3883"
 	default n
 	help
 	  If you say yes here you get hardware monitoring support for Linear
-	  Technology LTC2978 and LTC3880.
+	  Technology LTC2974, LTC2978, LTC3880, and LTC3883.
 
 	  This driver can also be built as a module. If so, the module will
 	  be called ltc2978.
diff --git a/drivers/hwmon/pmbus/ltc2978.c b/drivers/hwmon/pmbus/ltc2978.c
index 9652a2c..755ab35 100644
--- a/drivers/hwmon/pmbus/ltc2978.c
+++ b/drivers/hwmon/pmbus/ltc2978.c
@@ -1,5 +1,5 @@
 /*
- * Hardware monitoring driver for LTC2978 and LTC3880
+ * Hardware monitoring driver for LTC2974, LTC2978, LTC3880, and LTC3883
  *
  * Copyright (c) 2011 Ericsson AB.
  *
@@ -26,28 +26,42 @@
 #include <linux/i2c.h>
 #include "pmbus.h"
 
-enum chips { ltc2978, ltc3880 };
+enum chips { ltc2974, ltc2978, ltc3880, ltc3883 };
 
-/* LTC2978 and LTC3880 */
+/* Common for all chips */
 #define LTC2978_MFR_VOUT_PEAK		0xdd
 #define LTC2978_MFR_VIN_PEAK		0xde
 #define LTC2978_MFR_TEMPERATURE_PEAK	0xdf
 #define LTC2978_MFR_SPECIAL_ID		0xe7
 
-/* LTC2978 only */
+/* LTC2974 and LTC2978 */
 #define LTC2978_MFR_VOUT_MIN		0xfb
 #define LTC2978_MFR_VIN_MIN		0xfc
 #define LTC2978_MFR_TEMPERATURE_MIN	0xfd
 
-/* LTC3880 only */
+/* LTC3880 and LTC3883 */
 #define LTC3880_MFR_IOUT_PEAK		0xd7
 #define LTC3880_MFR_CLEAR_PEAKS		0xe3
 #define LTC3880_MFR_TEMPERATURE2_PEAK	0xf4
 
+/* LTC3883 only */
+#define LTC3883_MFR_IIN_PEAK		0xe1
+
+/* LTC2974 only */
+#define LTC2974_MFR_IOUT_PEAK		0xd7
+#define LTC2974_MFR_IOUT_MIN		0xd8
+
+#define LTC2974_ID			0x0212
 #define LTC2978_ID_REV1			0x0121
 #define LTC2978_ID_REV2			0x0122
 #define LTC3880_ID			0x4000
 #define LTC3880_ID_MASK			0xff00
+#define LTC3883_ID			0x4300
+
+#define LTC2974_NUM_PAGES		4
+#define LTC2978_NUM_PAGES		8
+#define LTC3880_NUM_PAGES		2
+#define LTC3883_NUM_PAGES		1
 
 /*
  * LTC2978 clears peak data whenever the CLEAR_FAULTS command is executed, which
@@ -61,7 +75,9 @@ struct ltc2978_data {
 	int vin_min, vin_max;
 	int temp_min, temp_max;
 	int vout_min[8], vout_max[8];
-	int iout_max[2];
+	int iin_max;
+	int iout_max[4];
+	int iout_min[4];
 	int temp2_max[2];
 	struct pmbus_driver_info info;
 };
@@ -184,6 +200,38 @@ static int ltc2978_read_word_data(struct i2c_client *client, int page, int reg)
 	return ret;
 }
 
+static int ltc2974_read_word_data(struct i2c_client *client, int page, int reg)
+{
+	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
+	struct ltc2978_data *data = to_ltc2978_data(info);
+	int ret;
+
+	switch (reg) {
+	case PMBUS_VIRT_READ_IOUT_MAX:
+		ret = pmbus_read_word_data(client, page, LTC2974_MFR_IOUT_PEAK);
+		if (ret >= 0) {
+			if (lin11_to_val(ret)
+			    > lin11_to_val(data->iout_max[page]))
+				data->iout_max[page] = ret;
+			ret = data->iout_max[page];
+		}
+		break;
+	case PMBUS_VIRT_READ_IOUT_MIN:
+		ret = pmbus_read_word_data(client, page, LTC2974_MFR_IOUT_MIN);
+		if (ret >= 0) {
+			if (lin11_to_val(ret)
+			    < lin11_to_val(data->iout_min[page]))
+				data->iout_min[page] = ret;
+			ret = data->iout_min[page];
+		}
+		break;
+	default:
+		ret = ltc2978_read_word_data(client, page, reg);
+		break;
+	}
+	return ret;
+}
+
 static int ltc3880_read_word_data(struct i2c_client *client, int page, int reg)
 {
 	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
@@ -226,6 +274,29 @@ static int ltc3880_read_word_data(struct i2c_client *client, int page, int reg)
 	return ret;
 }
 
+static int ltc3883_read_word_data(struct i2c_client *client, int page, int reg)
+{
+	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
+	struct ltc2978_data *data = to_ltc2978_data(info);
+	int ret;
+
+	switch (reg) {
+	case PMBUS_VIRT_READ_IIN_MAX:
+		ret = pmbus_read_word_data(client, page, LTC3883_MFR_IIN_PEAK);
+		if (ret >= 0) {
+			if (lin11_to_val(ret)
+			    > lin11_to_val(data->iin_max))
+				data->iin_max = ret;
+			ret = data->iin_max;
+		}
+		break;
+	default:
+		ret = ltc3880_read_word_data(client, page, reg);
+		break;
+	}
+	return ret;
+}
+
 static int ltc2978_clear_peaks(struct i2c_client *client, int page,
 			       enum chips id)
 {
@@ -247,8 +318,17 @@ static int ltc2978_write_word_data(struct i2c_client *client, int page,
 	int ret;
 
 	switch (reg) {
+	case PMBUS_VIRT_RESET_IIN_HISTORY:
+		if (data->id != ltc3883) {
+			ret = -ENODATA;
+			break;
+		}
+		data->iin_max = 0xffff;
+		ret = ltc2978_clear_peaks(client, page, data->id);
+		break;
 	case PMBUS_VIRT_RESET_IOUT_HISTORY:
-		data->iout_max[page] = 0x7fff;
+		data->iout_min[page] = 0xffff;
+		data->iout_max[page] = 0;
 		ret = ltc2978_clear_peaks(client, page, data->id);
 		break;
 	case PMBUS_VIRT_RESET_TEMP2_HISTORY:
@@ -278,8 +358,10 @@ static int ltc2978_write_word_data(struct i2c_client *client, int page,
 }
 
 static const struct i2c_device_id ltc2978_id[] = {
+	{"ltc2974", ltc2974},
 	{"ltc2978", ltc2978},
 	{"ltc3880", ltc3880},
+	{"ltc3883", ltc3883},
 	{}
 };
 MODULE_DEVICE_TABLE(i2c, ltc2978_id);
@@ -304,10 +386,14 @@ static int ltc2978_probe(struct i2c_client *client,
 	if (chip_id < 0)
 		return chip_id;
 
-	if (chip_id = LTC2978_ID_REV1 || chip_id = LTC2978_ID_REV2) {
+	if (chip_id = LTC2974_ID) {
+		data->id = ltc2974;
+	} else if (chip_id = LTC2978_ID_REV1 || chip_id = LTC2978_ID_REV2) {
 		data->id = ltc2978;
 	} else if ((chip_id & LTC3880_ID_MASK) = LTC3880_ID) {
 		data->id = ltc3880;
+	} else if ((chip_id & LTC3880_ID_MASK) = LTC3883_ID) {
+		data->id = ltc3883;
 	} else {
 		dev_err(&client->dev, "Unsupported chip ID 0x%x\n", chip_id);
 		return -ENODEV;
@@ -327,13 +413,29 @@ static int ltc2978_probe(struct i2c_client *client,
 	data->temp_max = 0x7fff;
 
 	switch (id->driver_data) {
+	case ltc2974:
+		info->read_word_data = ltc2974_read_word_data;
+		info->pages = LTC2974_NUM_PAGES;
+		info->func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IOUT
+		  | PMBUS_HAVE_STATUS_INPUT
+		  | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
+		  | PMBUS_HAVE_POUT | PMBUS_HAVE_TEMP2
+		  | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
+		for (i = 1; i < info->pages; i++) {
+			info->func[i] = PMBUS_HAVE_VOUT
+			  | PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_POUT
+			  | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP
+			  | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT;
+			data->vout_min[i] = 0xffff;
+		}
+		break;
 	case ltc2978:
 		info->read_word_data = ltc2978_read_word_data;
-		info->pages = 8;
+		info->pages = LTC2978_NUM_PAGES;
 		info->func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_STATUS_INPUT
 		  | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
 		  | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
-		for (i = 1; i < 8; i++) {
+		for (i = 1; i < info->pages; i++) {
 			info->func[i] = PMBUS_HAVE_VOUT
 			  | PMBUS_HAVE_STATUS_VOUT;
 			data->vout_min[i] = 0xffff;
@@ -341,7 +443,7 @@ static int ltc2978_probe(struct i2c_client *client,
 		break;
 	case ltc3880:
 		info->read_word_data = ltc3880_read_word_data;
-		info->pages = 2;
+		info->pages = LTC3880_NUM_PAGES;
 		info->func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN
 		  | PMBUS_HAVE_STATUS_INPUT
 		  | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
@@ -354,6 +456,16 @@ static int ltc2978_probe(struct i2c_client *client,
 		  | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
 		data->vout_min[1] = 0xffff;
 		break;
+	case ltc3883:
+		info->read_word_data = ltc3883_read_word_data;
+		info->pages = LTC3883_NUM_PAGES;
+		info->func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN
+		  | PMBUS_HAVE_STATUS_INPUT
+		  | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
+		  | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT
+		  | PMBUS_HAVE_POUT | PMBUS_HAVE_TEMP
+		  | PMBUS_HAVE_TEMP2 | PMBUS_HAVE_STATUS_TEMP;
+		break;
 	default:
 		return -ENODEV;
 	}
@@ -374,5 +486,5 @@ static struct i2c_driver ltc2978_driver = {
 module_i2c_driver(ltc2978_driver);
 
 MODULE_AUTHOR("Guenter Roeck");
-MODULE_DESCRIPTION("PMBus driver for LTC2978 and LTC3880");
+MODULE_DESCRIPTION("PMBus driver for LTC2974, LTC2978, LTC3880, and LTC3883");
 MODULE_LICENSE("GPL");
-- 
1.7.9.7


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [lm-sensors] [PATCH 2/2] hwmon: (ltc2978) Add support for LTC2974 and LTC3883
  2013-02-05 15:56 [lm-sensors] [PATCH 2/2] hwmon: (ltc2978) Add support for LTC2974 and LTC3883 Guenter Roeck
@ 2013-02-19 12:28 ` Jean Delvare
  2013-02-20 16:42 ` Guenter Roeck
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Jean Delvare @ 2013-02-19 12:28 UTC (permalink / raw)
  To: lm-sensors

Hi Guenter,

On Tue,  5 Feb 2013 07:56:55 -0800, Guenter Roeck wrote:
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  Documentation/hwmon/ltc2978   |   26 +++++---
>  drivers/hwmon/pmbus/Kconfig   |    4 +-
>  drivers/hwmon/pmbus/ltc2978.c |  136 +++++++++++++++++++++++++++++++++++++----
>  3 files changed, 143 insertions(+), 23 deletions(-)

Not being familiar with pmbus, my review may be incomplete, but I did
my best.

> 
> diff --git a/Documentation/hwmon/ltc2978 b/Documentation/hwmon/ltc2978
> index 8a5e791..8db0b61 100644
> --- a/Documentation/hwmon/ltc2978
> +++ b/Documentation/hwmon/ltc2978
> @@ -2,6 +2,10 @@ Kernel driver ltc2978
>  ==========>  
>  Supported chips:
> +  * Linear Technology LTC2974
> +    Prefix: 'ltc2974'
> +    Addresses scanned: -
> +    Datasheet: http://cds.linear.com/docs/en/datasheet/2974f.pdf
>    * Linear Technology LTC2978
>      Prefix: 'ltc2978'
>      Addresses scanned: -
> @@ -10,6 +14,10 @@ Supported chips:
>      Prefix: 'ltc3880'
>      Addresses scanned: -
>      Datasheet: http://cds.linear.com/docs/en/datasheet/3880fa.pdf
> +  * Linear Technology LTC3883
> +    Prefix: 'ltc3883'
> +    Addresses scanned: -
> +    Datasheet: http://cds.linear.com/docs/en/datasheet/3883f.pdf
>  
>  Author: Guenter Roeck <guenter.roeck@ericsson.com>

You might want to update this e-mail address. This might be better done
as a tree-wide patch though.

>  
> @@ -17,9 +25,9 @@ Author: Guenter Roeck <guenter.roeck@ericsson.com>
>  Description
>  -----------
>  
> -The LTC2978 is an octal power supply monitor, supervisor, sequencer and
> -margin controller. The LTC3880 is a dual, PolyPhase DC/DC synchronous
> -step-down switching regulator controller.
> +LTC2974 is a quad digital power supply manager. LTC2978 is an octal power supply
> +monitor. LTC3880 is a dual output poly-phase step-down DC/DC controller. LTC3883
> +is a single phase step-down DC/DC controller.
>  
>  
>  Usage Notes
> @@ -48,7 +56,7 @@ in1_min_alarm		Input voltage low alarm.
>  in1_max_alarm		Input voltage high alarm.
>  in1_lcrit_alarm		Input voltage critical low alarm.
>  in1_crit_alarm		Input voltage critical high alarm.
> -in1_lowest		Lowest input voltage. LTC2978 only.
> +in1_lowest		Lowest input voltage. LTC2974 and LTC2978 only.
>  in1_highest		Highest input voltage.
>  in1_reset_history	Reset history. Writing into this attribute will reset
>  			history for all attributes.
> @@ -63,7 +71,7 @@ in[2-9]_min_alarm	Output voltage low alarm.

in[2-9]_label says "Channels 3 to 9 on LTC2978 only" but I suppose
channels 3 to 5 are also available on LTC2974?

>  in[2-9]_max_alarm	Output voltage high alarm.
>  in[2-9]_lcrit_alarm	Output voltage critical low alarm.
>  in[2-9]_crit_alarm	Output voltage critical high alarm.
> -in[2-9]_lowest		Lowest output voltage. LTC2978 only.
> +in[2-9]_lowest		Lowest output voltage. LTC2974 and LTC2978 only.
>  in[2-9]_highest		Lowest output voltage.
>  in[2-9]_reset_history	Reset history. Writing into this attribute will reset
>  			history for all attributes.
> @@ -82,20 +90,20 @@ temp[1-3]_min_alarm	Chip temperature low alarm.

temp[1-3]_input has a detailed description of the input mappings for
the LTC2978 and LTC3880, it would seem desirable to have a similar
description for the two new chips LTC2974 and LTC3883.

>  temp[1-3]_max_alarm	Chip temperature high alarm.
>  temp[1-3]_lcrit_alarm	Chip temperature critical low alarm.
>  temp[1-3]_crit_alarm	Chip temperature critical high alarm.
> -temp[1-3]_lowest	Lowest measured temperature. LTC2978 only.
> +temp[1-3]_lowest	Lowest measured temperature. LTC2974 and LTC2978 only.
>  temp[1-3]_highest	Highest measured temperature.
>  temp[1-3]_reset_history	Reset history. Writing into this attribute will reset
>  			history for all attributes.
>  
> -power[1-2]_label	"pout[1-2]". LTC3880 only.
> +power[1-2]_label	"pout[1-2]". LTC2974, LTC3880, LTC3883 only.

I am under the impression that LTC2974 has pout[3-4] as well?

>  power[1-2]_input	Measured power.
>  
> -curr1_label		"iin". LTC3880 only.
> +curr1_label		"iin". LTC3880 and LTC3883 only.
>  curr1_input		Measured input current.
>  curr1_max		Maximum input current.
>  curr1_max_alarm		Input current high alarm.
>  
> -curr[2-3]_label		"iout[1-2]". LTC3880 only.
> +curr[2-3]_label		"iout[1-2]". LTC3880 and LTC3883 only.

What about LTC2974? It has PMBUS_HAVE_IOUT on all 4 pages, doesn't this
translate to curr[2-5]_* files? This would correlate with the size
increase of data->iout_min/max from 2 to 4 elements.

>  curr[2-3]_input		Measured input current.
>  curr[2-3]_max		Maximum input current.
>  curr[2-3]_crit		Critical input current.

> diff --git a/drivers/hwmon/pmbus/ltc2978.c b/drivers/hwmon/pmbus/ltc2978.c
> index 9652a2c..755ab35 100644
> --- a/drivers/hwmon/pmbus/ltc2978.c
> +++ b/drivers/hwmon/pmbus/ltc2978.c
> @@ -1,5 +1,5 @@
>  /*
> - * Hardware monitoring driver for LTC2978 and LTC3880
> + * Hardware monitoring driver for LTC2974, LTC2978, LTC3880, and LTC3883
>   *
>   * Copyright (c) 2011 Ericsson AB.

Feel free to add your name and copyright here, with the new year.

>   *
> @@ -26,28 +26,42 @@
>  #include <linux/i2c.h>
>  #include "pmbus.h"
>  
> -enum chips { ltc2978, ltc3880 };
> +enum chips { ltc2974, ltc2978, ltc3880, ltc3883 };
>  
> -/* LTC2978 and LTC3880 */
> +/* Common for all chips */
>  #define LTC2978_MFR_VOUT_PEAK		0xdd
>  #define LTC2978_MFR_VIN_PEAK		0xde
>  #define LTC2978_MFR_TEMPERATURE_PEAK	0xdf
>  #define LTC2978_MFR_SPECIAL_ID		0xe7
>  
> -/* LTC2978 only */
> +/* LTC2974 and LTC2978 */
>  #define LTC2978_MFR_VOUT_MIN		0xfb
>  #define LTC2978_MFR_VIN_MIN		0xfc
>  #define LTC2978_MFR_TEMPERATURE_MIN	0xfd
>  
> -/* LTC3880 only */
> +/* LTC3880 and LTC3883 */
>  #define LTC3880_MFR_IOUT_PEAK		0xd7
>  #define LTC3880_MFR_CLEAR_PEAKS		0xe3
>  #define LTC3880_MFR_TEMPERATURE2_PEAK	0xf4
>  
> +/* LTC3883 only */
> +#define LTC3883_MFR_IIN_PEAK		0xe1
> +
> +/* LTC2974 only */
> +#define LTC2974_MFR_IOUT_PEAK		0xd7
> +#define LTC2974_MFR_IOUT_MIN		0xd8

As you have two sub-families (LTC2974/8 vs. LTC3880/3) it would be
clearer to group "LTC2974 only" register definitions with "LTC2974 and
LTC2978" register definitions (i.e. move this one block two blocks up.) 

> +
> +#define LTC2974_ID			0x0212
>  #define LTC2978_ID_REV1			0x0121
>  #define LTC2978_ID_REV2			0x0122
>  #define LTC3880_ID			0x4000
>  #define LTC3880_ID_MASK			0xff00
> +#define LTC3883_ID			0x4300
> +
> +#define LTC2974_NUM_PAGES		4
> +#define LTC2978_NUM_PAGES		8
> +#define LTC3880_NUM_PAGES		2
> +#define LTC3883_NUM_PAGES		1
>  
>  /*
>   * LTC2978 clears peak data whenever the CLEAR_FAULTS command is executed, which
> @@ -61,7 +75,9 @@ struct ltc2978_data {
>  	int vin_min, vin_max;
>  	int temp_min, temp_max;
>  	int vout_min[8], vout_max[8];
> -	int iout_max[2];
> +	int iin_max;
> +	int iout_max[4];
> +	int iout_min[4];

Nitpicking:

	int iout_min[4], iout_max[4];

would match the existing coding style.

>  	int temp2_max[2];
>  	struct pmbus_driver_info info;
>  };
> @@ -184,6 +200,38 @@ static int ltc2978_read_word_data(struct i2c_client *client, int page, int reg)
>  	return ret;
>  }
>  
> +static int ltc2974_read_word_data(struct i2c_client *client, int page, int reg)
> +{
> +	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
> +	struct ltc2978_data *data = to_ltc2978_data(info);
> +	int ret;
> +
> +	switch (reg) {
> +	case PMBUS_VIRT_READ_IOUT_MAX:
> +		ret = pmbus_read_word_data(client, page, LTC2974_MFR_IOUT_PEAK);
> +		if (ret >= 0) {
> +			if (lin11_to_val(ret)
> +			    > lin11_to_val(data->iout_max[page]))
> +				data->iout_max[page] = ret;
> +			ret = data->iout_max[page];
> +		}
> +		break;
> +	case PMBUS_VIRT_READ_IOUT_MIN:
> +		ret = pmbus_read_word_data(client, page, LTC2974_MFR_IOUT_MIN);
> +		if (ret >= 0) {
> +			if (lin11_to_val(ret)
> +			    < lin11_to_val(data->iout_min[page]))
> +				data->iout_min[page] = ret;
> +			ret = data->iout_min[page];
> +		}
> +		break;
> +	default:
> +		ret = ltc2978_read_word_data(client, page, reg);
> +		break;
> +	}
> +	return ret;
> +}
> +
>  static int ltc3880_read_word_data(struct i2c_client *client, int page, int reg)
>  {
>  	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
> @@ -226,6 +274,29 @@ static int ltc3880_read_word_data(struct i2c_client *client, int page, int reg)
>  	return ret;
>  }
>  
> +static int ltc3883_read_word_data(struct i2c_client *client, int page, int reg)
> +{
> +	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
> +	struct ltc2978_data *data = to_ltc2978_data(info);
> +	int ret;
> +
> +	switch (reg) {
> +	case PMBUS_VIRT_READ_IIN_MAX:
> +		ret = pmbus_read_word_data(client, page, LTC3883_MFR_IIN_PEAK);
> +		if (ret >= 0) {
> +			if (lin11_to_val(ret)
> +			    > lin11_to_val(data->iin_max))
> +				data->iin_max = ret;
> +			ret = data->iin_max;
> +		}
> +		break;
> +	default:
> +		ret = ltc3880_read_word_data(client, page, reg);
> +		break;
> +	}
> +	return ret;
> +}
> +
>  static int ltc2978_clear_peaks(struct i2c_client *client, int page,
>  			       enum chips id)
>  {

In function ltc2978_clear_peaks() the test on "id" looks wrong:

	if (id = ltc2978)
		ret = pmbus_write_byte(client, page, PMBUS_CLEAR_FAULTS);
	else
		ret = pmbus_write_byte(client, 0, LTC3880_MFR_CLEAR_PEAKS);

LTC2974 doesn't have LTC3880_MFR_CLEAR_PEAKS, the address is used for a
different register, still you'll write to this register for that chip.
No good.

In order to avoid this from happening again when adding support for new
chips, I'd suggest reverting the test to:

	if (id = ltc3880 || id = ltc3883)
		ret = pmbus_write_byte(client, 0, LTC3880_MFR_CLEAR_PEAKS);
	else
		ret = pmbus_write_byte(client, page, PMBUS_CLEAR_FAULTS);


> @@ -247,8 +318,17 @@ static int ltc2978_write_word_data(struct i2c_client *client, int page,
>  	int ret;
>  
>  	switch (reg) {
> +	case PMBUS_VIRT_RESET_IIN_HISTORY:
> +		if (data->id != ltc3883) {
> +			ret = -ENODATA;
> +			break;
> +		}
> +		data->iin_max = 0xffff;

This initial value is inconsistent with the iout_max one below (both the
original one and the new one.) If I understand the L11 format
correctly, 0xffff corresponds to the closest negative value to 0 (-1 *
2^(-15)), right? If negative values are supported I'd expect you to put
the most negative value (largest in absolute value) possible here,
which would be 0x7c00 (exponent = 15, sign = 1, mantissa = 0.) If
negative values aren't supported then 0 would do?

> +		ret = ltc2978_clear_peaks(client, page, data->id);
> +		break;
>  	case PMBUS_VIRT_RESET_IOUT_HISTORY:
> -		data->iout_max[page] = 0x7fff;
> +		data->iout_min[page] = 0xffff;
> +		data->iout_max[page] = 0;

This change looks suspicious, can you explain it? It seems to be
unrelated to the support of new chips. If it is a bug fix, it should be
split to a separate patch.

I noticed that the driver has inconsistent initial values for iin_max,
iout_min and temp2_max, between the probe function and the history
reset code. In fact iin_max, iout_min and temp2_max have no explicit
initial value in the probe path, so they default to 0, while history
reset sets explicit values. This was the case for iout_max too before
the change above. This looks wrong.

Several L11-formatted initial values look wrong to me as well. I
suggest sorting this out first, both the consistency and the values
themselves, and only after this is done, adding support for the new
chips. As I understand it, for L11-formatted registers, min should be
set to 0x7bff and max should be set to 0x7c00.

>  		ret = ltc2978_clear_peaks(client, page, data->id);
>  		break;
>  	case PMBUS_VIRT_RESET_TEMP2_HISTORY:
> @@ -278,8 +358,10 @@ static int ltc2978_write_word_data(struct i2c_client *client, int page,
>  }
>  
>  static const struct i2c_device_id ltc2978_id[] = {
> +	{"ltc2974", ltc2974},
>  	{"ltc2978", ltc2978},
>  	{"ltc3880", ltc3880},
> +	{"ltc3883", ltc3883},
>  	{}
>  };
>  MODULE_DEVICE_TABLE(i2c, ltc2978_id);
> @@ -304,10 +386,14 @@ static int ltc2978_probe(struct i2c_client *client,
>  	if (chip_id < 0)
>  		return chip_id;
>  
> -	if (chip_id = LTC2978_ID_REV1 || chip_id = LTC2978_ID_REV2) {
> +	if (chip_id = LTC2974_ID) {
> +		data->id = ltc2974;
> +	} else if (chip_id = LTC2978_ID_REV1 || chip_id = LTC2978_ID_REV2) {
>  		data->id = ltc2978;
>  	} else if ((chip_id & LTC3880_ID_MASK) = LTC3880_ID) {
>  		data->id = ltc3880;
> +	} else if ((chip_id & LTC3880_ID_MASK) = LTC3883_ID) {
> +		data->id = ltc3883;
>  	} else {
>  		dev_err(&client->dev, "Unsupported chip ID 0x%x\n", chip_id);
>  		return -ENODEV;
> @@ -327,13 +413,29 @@ static int ltc2978_probe(struct i2c_client *client,
>  	data->temp_max = 0x7fff;
>  
>  	switch (id->driver_data) {
> +	case ltc2974:
> +		info->read_word_data = ltc2974_read_word_data;
> +		info->pages = LTC2974_NUM_PAGES;
> +		info->func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IOUT

No PMBUS_HAVE_STATUS_IOUT here?

> +		  | PMBUS_HAVE_STATUS_INPUT
> +		  | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
> +		  | PMBUS_HAVE_POUT | PMBUS_HAVE_TEMP2
> +		  | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
> +		for (i = 1; i < info->pages; i++) {
> +			info->func[i] = PMBUS_HAVE_VOUT
> +			  | PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_POUT
> +			  | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP
> +			  | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT;
> +			data->vout_min[i] = 0xffff;
> +		}
> +		break;
>  	case ltc2978:
>  		info->read_word_data = ltc2978_read_word_data;
> -		info->pages = 8;
> +		info->pages = LTC2978_NUM_PAGES;
>  		info->func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_STATUS_INPUT
>  		  | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
>  		  | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
> -		for (i = 1; i < 8; i++) {
> +		for (i = 1; i < info->pages; i++) {
>  			info->func[i] = PMBUS_HAVE_VOUT
>  			  | PMBUS_HAVE_STATUS_VOUT;
>  			data->vout_min[i] = 0xffff;
> @@ -341,7 +443,7 @@ static int ltc2978_probe(struct i2c_client *client,
>  		break;
>  	case ltc3880:
>  		info->read_word_data = ltc3880_read_word_data;
> -		info->pages = 2;
> +		info->pages = LTC3880_NUM_PAGES;
>  		info->func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN
>  		  | PMBUS_HAVE_STATUS_INPUT
>  		  | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
> @@ -354,6 +456,16 @@ static int ltc2978_probe(struct i2c_client *client,
>  		  | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
>  		data->vout_min[1] = 0xffff;
>  		break;
> +	case ltc3883:
> +		info->read_word_data = ltc3883_read_word_data;
> +		info->pages = LTC3883_NUM_PAGES;
> +		info->func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN
> +		  | PMBUS_HAVE_STATUS_INPUT
> +		  | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
> +		  | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT
> +		  | PMBUS_HAVE_POUT | PMBUS_HAVE_TEMP
> +		  | PMBUS_HAVE_TEMP2 | PMBUS_HAVE_STATUS_TEMP;
> +		break;
>  	default:
>  		return -ENODEV;
>  	}
> @@ -374,5 +486,5 @@ static struct i2c_driver ltc2978_driver = {
>  module_i2c_driver(ltc2978_driver);
>  
>  MODULE_AUTHOR("Guenter Roeck");
> -MODULE_DESCRIPTION("PMBus driver for LTC2978 and LTC3880");
> +MODULE_DESCRIPTION("PMBus driver for LTC2974, LTC2978, LTC3880, and LTC3883");
>  MODULE_LICENSE("GPL");


-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [lm-sensors] [PATCH 2/2] hwmon: (ltc2978) Add support for LTC2974 and LTC3883
  2013-02-05 15:56 [lm-sensors] [PATCH 2/2] hwmon: (ltc2978) Add support for LTC2974 and LTC3883 Guenter Roeck
  2013-02-19 12:28 ` Jean Delvare
@ 2013-02-20 16:42 ` Guenter Roeck
  2013-02-20 20:47 ` Jean Delvare
  2015-08-07  8:44 ` [lm-sensors] [PATCH 2/2] hwmon: (ltc2978) Add support for LTC3882 Guenter Roeck
  3 siblings, 0 replies; 5+ messages in thread
From: Guenter Roeck @ 2013-02-20 16:42 UTC (permalink / raw)
  To: lm-sensors

On Tue, Feb 19, 2013 at 01:28:28PM +0100, Jean Delvare wrote:
> Hi Guenter,
> 
> On Tue,  5 Feb 2013 07:56:55 -0800, Guenter Roeck wrote:
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > ---
> >  Documentation/hwmon/ltc2978   |   26 +++++---
> >  drivers/hwmon/pmbus/Kconfig   |    4 +-
> >  drivers/hwmon/pmbus/ltc2978.c |  136 +++++++++++++++++++++++++++++++++++++----
> >  3 files changed, 143 insertions(+), 23 deletions(-)
> 
> Not being familiar with pmbus, my review may be incomplete, but I did
> my best.
> 
Hi Jean,

Excellent review anyway, as always ...

> > 
> > diff --git a/Documentation/hwmon/ltc2978 b/Documentation/hwmon/ltc2978
> > index 8a5e791..8db0b61 100644
> > --- a/Documentation/hwmon/ltc2978
> > +++ b/Documentation/hwmon/ltc2978
> > @@ -2,6 +2,10 @@ Kernel driver ltc2978
> >  ==========> >  
> >  Supported chips:
> > +  * Linear Technology LTC2974
> > +    Prefix: 'ltc2974'
> > +    Addresses scanned: -
> > +    Datasheet: http://cds.linear.com/docs/en/datasheet/2974f.pdf
> >    * Linear Technology LTC2978
> >      Prefix: 'ltc2978'
> >      Addresses scanned: -
> > @@ -10,6 +14,10 @@ Supported chips:
> >      Prefix: 'ltc3880'
> >      Addresses scanned: -
> >      Datasheet: http://cds.linear.com/docs/en/datasheet/3880fa.pdf
> > +  * Linear Technology LTC3883
> > +    Prefix: 'ltc3883'
> > +    Addresses scanned: -
> > +    Datasheet: http://cds.linear.com/docs/en/datasheet/3883f.pdf
> >  
> >  Author: Guenter Roeck <guenter.roeck@ericsson.com>
> 
> You might want to update this e-mail address. This might be better done
> as a tree-wide patch though.
> 
> >  
> > @@ -17,9 +25,9 @@ Author: Guenter Roeck <guenter.roeck@ericsson.com>
> >  Description
> >  -----------
> >  
> > -The LTC2978 is an octal power supply monitor, supervisor, sequencer and
> > -margin controller. The LTC3880 is a dual, PolyPhase DC/DC synchronous
> > -step-down switching regulator controller.
> > +LTC2974 is a quad digital power supply manager. LTC2978 is an octal power supply
> > +monitor. LTC3880 is a dual output poly-phase step-down DC/DC controller. LTC3883
> > +is a single phase step-down DC/DC controller.
> >  
> >  
> >  Usage Notes
> > @@ -48,7 +56,7 @@ in1_min_alarm		Input voltage low alarm.
> >  in1_max_alarm		Input voltage high alarm.
> >  in1_lcrit_alarm		Input voltage critical low alarm.
> >  in1_crit_alarm		Input voltage critical high alarm.
> > -in1_lowest		Lowest input voltage. LTC2978 only.
> > +in1_lowest		Lowest input voltage. LTC2974 and LTC2978 only.
> >  in1_highest		Highest input voltage.
> >  in1_reset_history	Reset history. Writing into this attribute will reset
> >  			history for all attributes.
> > @@ -63,7 +71,7 @@ in[2-9]_min_alarm	Output voltage low alarm.
> 
> in[2-9]_label says "Channels 3 to 9 on LTC2978 only" but I suppose
> channels 3 to 5 are also available on LTC2974?
> 
> >  in[2-9]_max_alarm	Output voltage high alarm.
> >  in[2-9]_lcrit_alarm	Output voltage critical low alarm.
> >  in[2-9]_crit_alarm	Output voltage critical high alarm.
> > -in[2-9]_lowest		Lowest output voltage. LTC2978 only.
> > +in[2-9]_lowest		Lowest output voltage. LTC2974 and LTC2978 only.
> >  in[2-9]_highest		Lowest output voltage.
> >  in[2-9]_reset_history	Reset history. Writing into this attribute will reset
> >  			history for all attributes.
> > @@ -82,20 +90,20 @@ temp[1-3]_min_alarm	Chip temperature low alarm.
> 
> temp[1-3]_input has a detailed description of the input mappings for
> the LTC2978 and LTC3880, it would seem desirable to have a similar
> description for the two new chips LTC2974 and LTC3883.
> 
> >  temp[1-3]_max_alarm	Chip temperature high alarm.
> >  temp[1-3]_lcrit_alarm	Chip temperature critical low alarm.
> >  temp[1-3]_crit_alarm	Chip temperature critical high alarm.
> > -temp[1-3]_lowest	Lowest measured temperature. LTC2978 only.
> > +temp[1-3]_lowest	Lowest measured temperature. LTC2974 and LTC2978 only.
> >  temp[1-3]_highest	Highest measured temperature.
> >  temp[1-3]_reset_history	Reset history. Writing into this attribute will reset
> >  			history for all attributes.
> >  
> > -power[1-2]_label	"pout[1-2]". LTC3880 only.
> > +power[1-2]_label	"pout[1-2]". LTC2974, LTC3880, LTC3883 only.
> 
> I am under the impression that LTC2974 has pout[3-4] as well?
> 
> >  power[1-2]_input	Measured power.
> >  
> > -curr1_label		"iin". LTC3880 only.
> > +curr1_label		"iin". LTC3880 and LTC3883 only.
> >  curr1_input		Measured input current.
> >  curr1_max		Maximum input current.
> >  curr1_max_alarm		Input current high alarm.
> >  
> > -curr[2-3]_label		"iout[1-2]". LTC3880 only.
> > +curr[2-3]_label		"iout[1-2]". LTC3880 and LTC3883 only.
> 
> What about LTC2974? It has PMBUS_HAVE_IOUT on all 4 pages, doesn't this
> translate to curr[2-5]_* files? This would correlate with the size
> increase of data->iout_min/max from 2 to 4 elements.
> 

I have been wondering if I should create per device attribute lists. Things are
getting confusing. Do you think that would make sense / be better ?

Other comments are all valid ... I pulled the code from 3.9 and will have another
look. Especially regarding the peak values - yes, I noticed at some point that
something is wrong with the initialization, but then forgot to follow up on it.
Oh well :(.

Thanks,
Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [lm-sensors] [PATCH 2/2] hwmon: (ltc2978) Add support for LTC2974 and LTC3883
  2013-02-05 15:56 [lm-sensors] [PATCH 2/2] hwmon: (ltc2978) Add support for LTC2974 and LTC3883 Guenter Roeck
  2013-02-19 12:28 ` Jean Delvare
  2013-02-20 16:42 ` Guenter Roeck
@ 2013-02-20 20:47 ` Jean Delvare
  2015-08-07  8:44 ` [lm-sensors] [PATCH 2/2] hwmon: (ltc2978) Add support for LTC3882 Guenter Roeck
  3 siblings, 0 replies; 5+ messages in thread
From: Jean Delvare @ 2013-02-20 20:47 UTC (permalink / raw)
  To: lm-sensors

On Wed, 20 Feb 2013 08:42:31 -0800, Guenter Roeck wrote:
> On Tue, Feb 19, 2013 at 01:28:28PM +0100, Jean Delvare wrote:
> > On Tue,  5 Feb 2013 07:56:55 -0800, Guenter Roeck wrote:
> > > @@ -48,7 +56,7 @@ in1_min_alarm		Input voltage low alarm.
> > >  in1_max_alarm		Input voltage high alarm.
> > >  in1_lcrit_alarm		Input voltage critical low alarm.
> > >  in1_crit_alarm		Input voltage critical high alarm.
> > > -in1_lowest		Lowest input voltage. LTC2978 only.
> > > +in1_lowest		Lowest input voltage. LTC2974 and LTC2978 only.
> > >  in1_highest		Highest input voltage.
> > >  in1_reset_history	Reset history. Writing into this attribute will reset
> > >  			history for all attributes.
> > > @@ -63,7 +71,7 @@ in[2-9]_min_alarm	Output voltage low alarm.
> > 
> > in[2-9]_label says "Channels 3 to 9 on LTC2978 only" but I suppose
> > channels 3 to 5 are also available on LTC2974?
> > 
> > >  in[2-9]_max_alarm	Output voltage high alarm.
> > >  in[2-9]_lcrit_alarm	Output voltage critical low alarm.
> > >  in[2-9]_crit_alarm	Output voltage critical high alarm.
> > > -in[2-9]_lowest		Lowest output voltage. LTC2978 only.
> > > +in[2-9]_lowest		Lowest output voltage. LTC2974 and LTC2978 only.
> > >  in[2-9]_highest		Lowest output voltage.
> > >  in[2-9]_reset_history	Reset history. Writing into this attribute will reset
> > >  			history for all attributes.
> > > @@ -82,20 +90,20 @@ temp[1-3]_min_alarm	Chip temperature low alarm.
> > 
> > temp[1-3]_input has a detailed description of the input mappings for
> > the LTC2978 and LTC3880, it would seem desirable to have a similar
> > description for the two new chips LTC2974 and LTC3883.
> > 
> > >  temp[1-3]_max_alarm	Chip temperature high alarm.
> > >  temp[1-3]_lcrit_alarm	Chip temperature critical low alarm.
> > >  temp[1-3]_crit_alarm	Chip temperature critical high alarm.
> > > -temp[1-3]_lowest	Lowest measured temperature. LTC2978 only.
> > > +temp[1-3]_lowest	Lowest measured temperature. LTC2974 and LTC2978 only.
> > >  temp[1-3]_highest	Highest measured temperature.
> > >  temp[1-3]_reset_history	Reset history. Writing into this attribute will reset
> > >  			history for all attributes.
> > >  
> > > -power[1-2]_label	"pout[1-2]". LTC3880 only.
> > > +power[1-2]_label	"pout[1-2]". LTC2974, LTC3880, LTC3883 only.
> > 
> > I am under the impression that LTC2974 has pout[3-4] as well?
> > 
> > >  power[1-2]_input	Measured power.
> > >  
> > > -curr1_label		"iin". LTC3880 only.
> > > +curr1_label		"iin". LTC3880 and LTC3883 only.
> > >  curr1_input		Measured input current.
> > >  curr1_max		Maximum input current.
> > >  curr1_max_alarm		Input current high alarm.
> > >  
> > > -curr[2-3]_label		"iout[1-2]". LTC3880 only.
> > > +curr[2-3]_label		"iout[1-2]". LTC3880 and LTC3883 only.
> > 
> > What about LTC2974? It has PMBUS_HAVE_IOUT on all 4 pages, doesn't this
> > translate to curr[2-5]_* files? This would correlate with the size
> > increase of data->iout_min/max from 2 to 4 elements.
> 
> I have been wondering if I should create per device attribute lists. Things are
> getting confusing. Do you think that would make sense / be better ?

There are pros and cons each way, as you have already found out I'm
sure. I do not have a strong opinion. Maybe take the middle road, and
make one section for LTC297x and one for LTC388x?

> Other comments are all valid ... I pulled the code from 3.9 and will have another
> look. Especially regarding the peak values - yes, I noticed at some point that
> something is wrong with the initialization, but then forgot to follow up on it.
> Oh well :(.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [lm-sensors] [PATCH 2/2] hwmon: (ltc2978) Add support for LTC3882
  2013-02-05 15:56 [lm-sensors] [PATCH 2/2] hwmon: (ltc2978) Add support for LTC2974 and LTC3883 Guenter Roeck
                   ` (2 preceding siblings ...)
  2013-02-20 20:47 ` Jean Delvare
@ 2015-08-07  8:44 ` Guenter Roeck
  3 siblings, 0 replies; 5+ messages in thread
From: Guenter Roeck @ 2015-08-07  8:44 UTC (permalink / raw)
  To: lm-sensors

LTC3882 is mostly compatible with LTC3880. Major differences are that it
does not measure the input current, and it no longer supports LTC's legacy
mechanism to identify the chip.

Cc: Ananda Babu Nettam <anandab@juniper.net>
Cc: Amit U Jain <amjain@juniper.net>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 .../devicetree/bindings/hwmon/ltc2978.txt          |  3 +-
 Documentation/hwmon/ltc2978                        | 26 ++++++-----
 drivers/hwmon/pmbus/ltc2978.c                      | 50 +++++++++++++++++++---
 3 files changed, 62 insertions(+), 17 deletions(-)

diff --git a/Documentation/devicetree/bindings/hwmon/ltc2978.txt b/Documentation/devicetree/bindings/hwmon/ltc2978.txt
index ed2f09dc2483..230389f6c984 100644
--- a/Documentation/devicetree/bindings/hwmon/ltc2978.txt
+++ b/Documentation/devicetree/bindings/hwmon/ltc2978.txt
@@ -6,6 +6,7 @@ Required properties:
   * "lltc,ltc2977"
   * "lltc,ltc2978"
   * "lltc,ltc3880"
+  * "lltc,ltc3882"
   * "lltc,ltc3883"
   * "lltc,ltm4676"
 - reg: I2C slave address
@@ -20,7 +21,7 @@ Valid names of regulators depend on number of supplies supported per device:
   * ltc2974 : vout0 - vout3
   * ltc2977 : vout0 - vout7
   * ltc2978 : vout0 - vout7
-  * ltc3880 : vout0 - vout1
+  * ltc3880, ltc3882 : vout0 - vout1
   * ltc3883 : vout0
   * ltm4676 : vout0 - vout1
 
diff --git a/Documentation/hwmon/ltc2978 b/Documentation/hwmon/ltc2978
index 686c078bb0e0..521ee8a1135c 100644
--- a/Documentation/hwmon/ltc2978
+++ b/Documentation/hwmon/ltc2978
@@ -19,6 +19,10 @@ Supported chips:
     Prefix: 'ltc3880'
     Addresses scanned: -
     Datasheet: http://www.linear.com/product/ltc3880
+  * Linear Technology LTC3882
+    Prefix: 'ltc3882'
+    Addresses scanned: -
+    Datasheet: http://www.linear.com/product/ltc3882
   * Linear Technology LTC3883
     Prefix: 'ltc3883'
     Addresses scanned: -
@@ -34,11 +38,12 @@ Author: Guenter Roeck <linux@roeck-us.net>
 Description
 -----------
 
-LTC2974 is a quad digital power supply manager. LTC2978 is an octal power supply
-monitor. LTC2977 is a pin compatible replacement for LTC2978. LTC3880 is a dual
-output poly-phase step-down DC/DC controller. LTC3883 is a single phase
-step-down DC/DC controller. LTM4676 is a dual 13A or single 26A uModule
-regulator.
+LTC2974 is a quad digital power supply managers.
+LTC2978 is an octal power supply monitor.
+LTC2977 is a pin compatible replacement for LTC2978.
+LTC3880 and LTC3882 are dual output poly-phase step-down DC/DC controllers.
+LTC3883 is a single phase step-down DC/DC controller.
+LTM4676 is a dual 13A or single 26A uModule regulator.
 
 
 Usage Notes
@@ -80,7 +85,7 @@ in[N]_label		"vout[1-8]".
 			LTC2974: N=2-5
 			LTC2977: N=2-9
 			LTC2978: N=2-9
-			LTC3880, LTM4676: N=2-3
+			LTC3880, LTC3882, LTM4676: N=2-3
 			LTC3883: N=2
 in[N]_input		Measured output voltage.
 in[N]_min		Minimum output voltage.
@@ -100,8 +105,9 @@ temp[N]_input		Measured temperature.
 			and temp5 reports the chip temperature.
 			On LTC2977 and LTC2978, only one temperature measurement
 			is supported and reports the chip temperature.
-			On LTC3880 and LTM4676, temp1 and temp2 report external
-			temperatures, and temp3 reports the chip temperature.
+			On LTC3880, LTC3882, and LTM4676, temp1 and temp2
+			report external temperatures, and temp3 reports
+			the chip temperature.
 			On LTC3883, temp1 reports an external temperature,
 			and temp2 reports the chip temperature.
 temp[N]_min		Mimimum temperature. LTC2974, LCT2977, and LTC2978 only.
@@ -128,7 +134,7 @@ power[N]_label		"pout[1-4]".
 			LTC2974: N=1-4
 			LTC2977: Not supported
 			LTC2978: Not supported
-			LTC3880, LTM4676: N=1-2
+			LTC3880, LTC3882, LTM4676: N=1-2
 			LTC3883: N=2
 power[N]_input		Measured output power.
 
@@ -143,7 +149,7 @@ curr[N]_label		"iout[1-4]".
 			LTC2974: N=1-4
 			LTC2977: not supported
 			LTC2978: not supported
-			LTC3880, LTM4676: N=2-3
+			LTC3880, LTC3882, LTM4676: N=2-3
 			LTC3883: N=2
 curr[N]_input		Measured output current.
 curr[N]_max		Maximum output current.
diff --git a/drivers/hwmon/pmbus/ltc2978.c b/drivers/hwmon/pmbus/ltc2978.c
index a27debafaad9..b312f4390fe9 100644
--- a/drivers/hwmon/pmbus/ltc2978.c
+++ b/drivers/hwmon/pmbus/ltc2978.c
@@ -25,13 +25,13 @@
 #include <linux/regulator/driver.h>
 #include "pmbus.h"
 
-enum chips { ltc2974, ltc2977, ltc2978, ltc3880, ltc3883, ltm4676 };
+enum chips { ltc2974, ltc2977, ltc2978, ltc3880, ltc3882, ltc3883, ltm4676 };
 
 /* Common for all chips */
 #define LTC2978_MFR_VOUT_PEAK		0xdd
 #define LTC2978_MFR_VIN_PEAK		0xde
 #define LTC2978_MFR_TEMPERATURE_PEAK	0xdf
-#define LTC2978_MFR_SPECIAL_ID		0xe7
+#define LTC2978_MFR_SPECIAL_ID		0xe7	/* Not on LTC3882 */
 
 /* LTC2974, LCT2977, and LTC2978 */
 #define LTC2978_MFR_VOUT_MIN		0xfb
@@ -42,7 +42,7 @@ enum chips { ltc2974, ltc2977, ltc2978, ltc3880, ltc3883, ltm4676 };
 #define LTC2974_MFR_IOUT_PEAK		0xd7
 #define LTC2974_MFR_IOUT_MIN		0xd8
 
-/* LTC3880, LTC3883, and LTM4676 */
+/* LTC3880, LTC3882, LTC3883, and LTM4676 */
 #define LTC3880_MFR_IOUT_PEAK		0xd7
 #define LTC3880_MFR_CLEAR_PEAKS		0xe3
 #define LTC3880_MFR_TEMPERATURE2_PEAK	0xf4
@@ -313,7 +313,7 @@ static int ltc2978_clear_peaks(struct i2c_client *client, int page,
 {
 	int ret;
 
-	if (id = ltc3880 || id = ltc3883 || id = ltm4676)
+	if (id = ltc3880 || id = ltc3882 || id = ltc3883 || id = ltm4676)
 		ret = pmbus_write_byte(client, 0, LTC3880_MFR_CLEAR_PEAKS);
 	else
 		ret = pmbus_write_byte(client, page, PMBUS_CLEAR_FAULTS);
@@ -369,6 +369,7 @@ static const struct i2c_device_id ltc2978_id[] = {
 	{"ltc2977", ltc2977},
 	{"ltc2978", ltc2978},
 	{"ltc3880", ltc3880},
+	{"ltc3882", ltc3882},
 	{"ltc3883", ltc3883},
 	{"ltm4676", ltm4676},
 	{}
@@ -393,8 +394,30 @@ static int ltc2978_get_id(struct i2c_client *client)
 	int chip_id;
 
 	chip_id = i2c_smbus_read_word_data(client, LTC2978_MFR_SPECIAL_ID);
-	if (chip_id < 0)
-		return chip_id;
+	if (chip_id < 0) {
+		const struct i2c_device_id *id;
+		u8 buf[I2C_SMBUS_BLOCK_MAX];
+		int ret;
+
+		if (!i2c_check_functionality(client->adapter,
+					     I2C_FUNC_SMBUS_READ_BLOCK_DATA))
+			return -ENODEV;
+
+		ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, buf);
+		if (ret < 0)
+			return ret;
+		if (ret < 3 || strncmp(buf, "LTC", 3))
+			return -ENODEV;
+
+		ret = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, buf);
+		if (ret < 0)
+			return ret;
+		for (id = &ltc2978_id[0]; strlen(id->name); id++) {
+			if (!strncasecmp(id->name, buf, strlen(id->name)))
+				return (int)id->driver_data;
+		}
+		return -ENODEV;
+	}
 
 	if (chip_id = LTC2974_ID_REV1 || chip_id = LTC2974_ID_REV2)
 		return ltc2974;
@@ -497,6 +520,20 @@ static int ltc2978_probe(struct i2c_client *client,
 		  | PMBUS_HAVE_POUT
 		  | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
 		break;
+	case ltc3882:
+		info->read_word_data = ltc3880_read_word_data;
+		info->pages = LTC3880_NUM_PAGES;
+		info->func[0] = PMBUS_HAVE_VIN
+		  | PMBUS_HAVE_STATUS_INPUT
+		  | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
+		  | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT
+		  | PMBUS_HAVE_POUT | PMBUS_HAVE_TEMP
+		  | PMBUS_HAVE_TEMP2 | PMBUS_HAVE_STATUS_TEMP;
+		info->func[1] = PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
+		  | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT
+		  | PMBUS_HAVE_POUT
+		  | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
+		break;
 	case ltc3883:
 		info->read_word_data = ltc3883_read_word_data;
 		info->pages = LTC3883_NUM_PAGES;
@@ -529,6 +566,7 @@ static const struct of_device_id ltc2978_of_match[] = {
 	{ .compatible = "lltc,ltc2977" },
 	{ .compatible = "lltc,ltc2978" },
 	{ .compatible = "lltc,ltc3880" },
+	{ .compatible = "lltc,ltc3882" },
 	{ .compatible = "lltc,ltc3883" },
 	{ .compatible = "lltc,ltm4676" },
 	{ }
-- 
2.1.4


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-08-07  8:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-05 15:56 [lm-sensors] [PATCH 2/2] hwmon: (ltc2978) Add support for LTC2974 and LTC3883 Guenter Roeck
2013-02-19 12:28 ` Jean Delvare
2013-02-20 16:42 ` Guenter Roeck
2013-02-20 20:47 ` Jean Delvare
2015-08-07  8:44 ` [lm-sensors] [PATCH 2/2] hwmon: (ltc2978) Add support for LTC3882 Guenter Roeck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.