[PATCH] hwmon: (ltc4151) Make shunt-resistor configurable

[PATCH] hwmon: (ltc4151) Make shunt-resistor configurable

[Daniel Golle]

Allow to specify the resistance of the attached shunt via DT by
adding the shunt-resistor property. Fall-back to the previous
default (1 mOhm) if unset.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 drivers/hwmon/ltc4151.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/ltc4151.c b/drivers/hwmon/ltc4151.c
index c86a184..f856e44 100644
--- a/drivers/hwmon/ltc4151.c
+++ b/drivers/hwmon/ltc4151.c
@@ -30,6 +30,8 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/init.h>
 #include <linux/err.h>
 #include <linux/slab.h>
@@ -52,6 +54,7 @@ struct ltc4151_data {
 	struct mutex update_lock;
 	bool valid;
 	unsigned long last_updated; /* in jiffies */
+	unsigned int shunt; /* in micro ohms */
 
 	/* Registers */
 	u8 regs[6];
@@ -111,9 +114,9 @@ static int ltc4151_get_value(struct ltc4151_data *data, u8 reg)
 	case LTC4151_SENSE_H:
 		/*
 		 * 20uV resolution. Convert to current as measured with
-		 * an 1 mOhm sense resistor, in mA.
+		 * a given sense resistor, in mA.
 		 */
-		val = val * 20;
+		val = val * 20 * 1000 / data->shunt;
 		break;
 	case LTC4151_VIN_H:
 		/* 25 mV per increment */
@@ -176,6 +179,7 @@ static int ltc4151_probe(struct i2c_client *client,
 	struct device *dev = &client->dev;
 	struct ltc4151_data *data;
 	struct device *hwmon_dev;
+	u32 shunt;
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
 		return -ENODEV;
@@ -184,6 +188,14 @@ static int ltc4151_probe(struct i2c_client *client,
 	if (!data)
 		return -ENOMEM;
 
+#ifdef CONFIG_OF
+	if (!of_property_read_u32(client->dev.of_node, "shunt-resistor", &shunt)
+		&& shunt > 0 )
+		data->shunt = shunt;
+	else
+#endif
+		data->shunt = 1000; /* 1 mOhm if not set */
+
 	data->client = client;
 	mutex_init(&data->update_lock);
 
@@ -199,10 +211,20 @@ static const struct i2c_device_id ltc4151_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, ltc4151_id);
 
+#ifdef CONFIG_OF
+static const struct of_device_id ltc4151_match[] = {
+	{ .compatible = "lltc,ltc4151" },
+	{},
+};
+#endif
+
 /* This is the driver that will be inserted */
 static struct i2c_driver ltc4151_driver = {
+	.class		= I2C_CLASS_HWMON,
 	.driver = {
 		.name	= "ltc4151",
+		.owner  = THIS_MODULE,
+		.of_match_table = of_match_ptr(ltc4151_match),
 	},
 	.probe		= ltc4151_probe,
 	.id_table	= ltc4151_id,
-- 
2.9.0

Re: [PATCH] hwmon: (ltc4151) Make shunt-resistor configurable

[Guenter Roeck]

On 07/24/2016 01:26 PM, Daniel Golle wrote:
> Allow to specify the resistance of the attached shunt via DT by
> adding the shunt-resistor property. Fall-back to the previous
> default (1 mOhm) if unset.
>
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
>   drivers/hwmon/ltc4151.c | 26 ++++++++++++++++++++++++--
>   1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwmon/ltc4151.c b/drivers/hwmon/ltc4151.c
> index c86a184..f856e44 100644
> --- a/drivers/hwmon/ltc4151.c
> +++ b/drivers/hwmon/ltc4151.c
> @@ -30,6 +30,8 @@
>
>   #include <linux/kernel.h>
>   #include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>

I don't immediately see why this include file would be necessary.

>   #include <linux/init.h>
>   #include <linux/err.h>
>   #include <linux/slab.h>
> @@ -52,6 +54,7 @@ struct ltc4151_data {
>   	struct mutex update_lock;
>   	bool valid;
>   	unsigned long last_updated; /* in jiffies */
> +	unsigned int shunt; /* in micro ohms */
>
>   	/* Registers */
>   	u8 regs[6];
> @@ -111,9 +114,9 @@ static int ltc4151_get_value(struct ltc4151_data *data, u8 reg)
>   	case LTC4151_SENSE_H:
>   		/*
>   		 * 20uV resolution. Convert to current as measured with
> -		 * an 1 mOhm sense resistor, in mA.
> +		 * a given sense resistor, in mA.
>   		 */
> -		val = val * 20;
> +		val = val * 20 * 1000 / data->shunt;
>   		break;
>   	case LTC4151_VIN_H:
>   		/* 25 mV per increment */
> @@ -176,6 +179,7 @@ static int ltc4151_probe(struct i2c_client *client,
>   	struct device *dev = &client->dev;
>   	struct ltc4151_data *data;
>   	struct device *hwmon_dev;
> +	u32 shunt;
>
>   	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
>   		return -ENODEV;
> @@ -184,6 +188,14 @@ static int ltc4151_probe(struct i2c_client *client,
>   	if (!data)
>   		return -ENOMEM;
>
> +#ifdef CONFIG_OF

#ifdef is unnecessary. Worse, it results in

drivers/hwmon/ltc4151.c: In function ‘ltc4151_probe’:
drivers/hwmon/ltc4151.c:182:6: warning: unused variable ‘shunt’

if CONFIG_OF is not defined.

> +	if (!of_property_read_u32(client->dev.of_node, "shunt-resistor", &shunt)
> +		&& shunt > 0 )

checkpatch error

> +		data->shunt = shunt;

shunt == 0 is silently accepted and replaced with 1000. This is quite unusual.
Any reason ? If not, return -EINVAL might be more appropriate for shunt == 0.

> +	else
> +#endif
> +		data->shunt = 1000; /* 1 mOhm if not set */
> +
>   	data->client = client;
>   	mutex_init(&data->update_lock);
>
> @@ -199,10 +211,20 @@ static const struct i2c_device_id ltc4151_id[] = {
>   };
>   MODULE_DEVICE_TABLE(i2c, ltc4151_id);
>
> +#ifdef CONFIG_OF

#ifdef is unnecessary.

> +static const struct of_device_id ltc4151_match[] = {
> +	{ .compatible = "lltc,ltc4151" },

This as well as the property need to be documented.

> +	{},
> +};
> +#endif
> +
>   /* This is the driver that will be inserted */
>   static struct i2c_driver ltc4151_driver = {
> +	.class		= I2C_CLASS_HWMON,

This is an unrelated change, and appears to be quite pointless,
since the driver does not have a detect function.
Any reason for adding it ? If so, please explain, and submit
as separate patch.

>   	.driver = {
>   		.name	= "ltc4151",
> +		.owner  = THIS_MODULE,

Is this now necessary again ? If so, please explain, and submit
as separate patch.

> +		.of_match_table = of_match_ptr(ltc4151_match),
>   	},
>   	.probe		= ltc4151_probe,
>   	.id_table	= ltc4151_id,
>

Re: [PATCH] hwmon: (ltc4151) Make shunt-resistor configurable

[Guenter Roeck]

On 07/24/2016 01:26 PM, Daniel Golle wrote:
> Allow to specify the resistance of the attached shunt via DT by
> adding the shunt-resistor property. Fall-back to the previous
> default (1 mOhm) if unset.
>
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
>   drivers/hwmon/ltc4151.c | 26 ++++++++++++++++++++++++--
>   1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwmon/ltc4151.c b/drivers/hwmon/ltc4151.c
> index c86a184..f856e44 100644
> --- a/drivers/hwmon/ltc4151.c
> +++ b/drivers/hwmon/ltc4151.c
> @@ -30,6 +30,8 @@
>
>   #include <linux/kernel.h>
>   #include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>

I don't immediately see why this include file would be necessary.

>   #include <linux/init.h>
>   #include <linux/err.h>
>   #include <linux/slab.h>
> @@ -52,6 +54,7 @@ struct ltc4151_data {
>   	struct mutex update_lock;
>   	bool valid;
>   	unsigned long last_updated; /* in jiffies */
> +	unsigned int shunt; /* in micro ohms */
>
>   	/* Registers */
>   	u8 regs[6];
> @@ -111,9 +114,9 @@ static int ltc4151_get_value(struct ltc4151_data *data, u8 reg)
>   	case LTC4151_SENSE_H:
>   		/*
>   		 * 20uV resolution. Convert to current as measured with
> -		 * an 1 mOhm sense resistor, in mA.
> +		 * a given sense resistor, in mA.
>   		 */
> -		val = val * 20;
> +		val = val * 20 * 1000 / data->shunt;
>   		break;
>   	case LTC4151_VIN_H:
>   		/* 25 mV per increment */
> @@ -176,6 +179,7 @@ static int ltc4151_probe(struct i2c_client *client,
>   	struct device *dev = &client->dev;
>   	struct ltc4151_data *data;
>   	struct device *hwmon_dev;
> +	u32 shunt;
>
>   	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
>   		return -ENODEV;
> @@ -184,6 +188,14 @@ static int ltc4151_probe(struct i2c_client *client,
>   	if (!data)
>   		return -ENOMEM;
>
> +#ifdef CONFIG_OF

#ifdef is unnecessary. Worse, it results in

drivers/hwmon/ltc4151.c: In function ‘ltc4151_probe’:
drivers/hwmon/ltc4151.c:182:6: warning: unused variable ‘shunt’

if CONFIG_OF is not defined.

> +	if (!of_property_read_u32(client->dev.of_node, "shunt-resistor", &shunt)
> +		&& shunt > 0 )

checkpatch error

> +		data->shunt = shunt;

shunt == 0 is silently accepted and replaced with 1000. This is quite unusual.
Any reason ? If not, return -EINVAL might be more appropriate for shunt == 0.

> +	else
> +#endif
> +		data->shunt = 1000; /* 1 mOhm if not set */
> +
>   	data->client = client;
>   	mutex_init(&data->update_lock);
>
> @@ -199,10 +211,20 @@ static const struct i2c_device_id ltc4151_id[] = {
>   };
>   MODULE_DEVICE_TABLE(i2c, ltc4151_id);
>
> +#ifdef CONFIG_OF

#ifdef is unnecessary.

> +static const struct of_device_id ltc4151_match[] = {
> +	{ .compatible = "lltc,ltc4151" },

This as well as the property need to be documented.

> +	{},
> +};
> +#endif
> +
>   /* This is the driver that will be inserted */
>   static struct i2c_driver ltc4151_driver = {
> +	.class		= I2C_CLASS_HWMON,

This is an unrelated change, and appears to be quite pointless,
since the driver does not have a detect function.
Any reason for adding it ? If so, please explain, and submit
as separate patch.

>   	.driver = {
>   		.name	= "ltc4151",
> +		.owner  = THIS_MODULE,

Is this now necessary again ? If so, please explain, and submit
as separate patch.

> +		.of_match_table = of_match_ptr(ltc4151_match),
>   	},
>   	.probe		= ltc4151_probe,
>   	.id_table	= ltc4151_id,
>

--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2 v2] hwmon: (ltc4151) Make shunt-resistor configurable

[Daniel Golle]

Allow to specify the resistance of the attached shunt via DT by
adding the shunt-resistor property. Fall-back to the previous
default (1 mOhm) if unset.

Signed-off-by: Daniel Golle <daniel-g5gK2j5usbvCyp4qypjU+w@public.gmane.org>
---
v2: removed unneeded things and throw -EINVAL for shunt==0

 drivers/hwmon/ltc4151.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/ltc4151.c b/drivers/hwmon/ltc4151.c
index c86a184..419c6f7 100644
--- a/drivers/hwmon/ltc4151.c
+++ b/drivers/hwmon/ltc4151.c
@@ -30,6 +30,7 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/init.h>
 #include <linux/err.h>
 #include <linux/slab.h>
@@ -52,6 +53,7 @@ struct ltc4151_data {
 	struct mutex update_lock;
 	bool valid;
 	unsigned long last_updated; /* in jiffies */
+	unsigned int shunt; /* in micro ohms */
 
 	/* Registers */
 	u8 regs[6];
@@ -111,9 +113,9 @@ static int ltc4151_get_value(struct ltc4151_data *data, u8 reg)
 	case LTC4151_SENSE_H:
 		/*
 		 * 20uV resolution. Convert to current as measured with
-		 * an 1 mOhm sense resistor, in mA.
+		 * a given sense resistor, in mA.
 		 */
-		val = val * 20;
+		val = val * 20 * 1000 / data->shunt;
 		break;
 	case LTC4151_VIN_H:
 		/* 25 mV per increment */
@@ -176,6 +178,7 @@ static int ltc4151_probe(struct i2c_client *client,
 	struct device *dev = &client->dev;
 	struct ltc4151_data *data;
 	struct device *hwmon_dev;
+	u32 shunt;
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
 		return -ENODEV;
@@ -184,6 +187,14 @@ static int ltc4151_probe(struct i2c_client *client,
 	if (!data)
 		return -ENOMEM;
 
+	if (of_property_read_u32(client->dev.of_node, "shunt-resistor", &shunt))
+		shunt = 1000; /* 1 mOhm if not set via DT */
+
+	if (shunt == 0)
+		return -EINVAL;
+
+	data->shunt = shunt;
+
 	data->client = client;
 	mutex_init(&data->update_lock);
 
@@ -199,10 +210,16 @@ static const struct i2c_device_id ltc4151_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, ltc4151_id);
 
+static const struct of_device_id ltc4151_match[] = {
+	{ .compatible = "lltc,ltc4151" },
+	{},
+};
+
 /* This is the driver that will be inserted */
 static struct i2c_driver ltc4151_driver = {
 	.driver = {
 		.name	= "ltc4151",
+		.of_match_table = of_match_ptr(ltc4151_match),
 	},
 	.probe		= ltc4151_probe,
 	.id_table	= ltc4151_id,
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] hwmon: (ltc4151) Add devicetree binding for ltc4151

[Daniel Golle]

Signed-off-by: Daniel Golle <daniel-g5gK2j5usbvCyp4qypjU+w@public.gmane.org>
---
 Documentation/devicetree/bindings/hwmon/ltc4151.txt | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/ltc4151.txt

diff --git a/Documentation/devicetree/bindings/hwmon/ltc4151.txt b/Documentation/devicetree/bindings/hwmon/ltc4151.txt
new file mode 100644
index 0000000..aec69ed
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/ltc4151.txt
@@ -0,0 +1,18 @@
+ltc4151 properties
+
+Required properties:
+- compatible: Must be "lltc,ltc4151"
+- reg: I2C address
+
+Optional properties:
+
+- shunt-resistor
+	Shunt resistor value in micro-Ohm
+
+Example:
+
+ltc4151@6e {
+	compatible = "lltc,ltc4151";
+	reg = <0x6e>;
+	shunt-resistor = <1500>;
+};
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2 v2] hwmon: (ltc4151) Make shunt-resistor configurable

[Daniel Golle]

Allow to specify the resistance of the attached shunt via DT by
adding the shunt-resistor property. Fall-back to the previous
default (1 mOhm) if unset.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2: removed unneeded things and throw -EINVAL for shunt==0
(resent to linux-hwmon which by accident wasn't among the receipients)

 drivers/hwmon/ltc4151.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/ltc4151.c b/drivers/hwmon/ltc4151.c
index c86a184..419c6f7 100644
--- a/drivers/hwmon/ltc4151.c
+++ b/drivers/hwmon/ltc4151.c
@@ -30,6 +30,7 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/init.h>
 #include <linux/err.h>
 #include <linux/slab.h>
@@ -52,6 +53,7 @@ struct ltc4151_data {
 	struct mutex update_lock;
 	bool valid;
 	unsigned long last_updated; /* in jiffies */
+	unsigned int shunt; /* in micro ohms */
 
 	/* Registers */
 	u8 regs[6];
@@ -111,9 +113,9 @@ static int ltc4151_get_value(struct ltc4151_data *data, u8 reg)
 	case LTC4151_SENSE_H:
 		/*
 		 * 20uV resolution. Convert to current as measured with
-		 * an 1 mOhm sense resistor, in mA.
+		 * a given sense resistor, in mA.
 		 */
-		val = val * 20;
+		val = val * 20 * 1000 / data->shunt;
 		break;
 	case LTC4151_VIN_H:
 		/* 25 mV per increment */
@@ -176,6 +178,7 @@ static int ltc4151_probe(struct i2c_client *client,
 	struct device *dev = &client->dev;
 	struct ltc4151_data *data;
 	struct device *hwmon_dev;
+	u32 shunt;
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
 		return -ENODEV;
@@ -184,6 +187,14 @@ static int ltc4151_probe(struct i2c_client *client,
 	if (!data)
 		return -ENOMEM;
 
+	if (of_property_read_u32(client->dev.of_node, "shunt-resistor", &shunt))
+		shunt = 1000; /* 1 mOhm if not set via DT */
+
+	if (shunt == 0)
+		return -EINVAL;
+
+	data->shunt = shunt;
+
 	data->client = client;
 	mutex_init(&data->update_lock);
 
@@ -199,10 +210,16 @@ static const struct i2c_device_id ltc4151_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, ltc4151_id);
 
+static const struct of_device_id ltc4151_match[] = {
+	{ .compatible = "lltc,ltc4151" },
+	{},
+};
+
 /* This is the driver that will be inserted */
 static struct i2c_driver ltc4151_driver = {
 	.driver = {
 		.name	= "ltc4151",
+		.of_match_table = of_match_ptr(ltc4151_match),
 	},
 	.probe		= ltc4151_probe,
 	.id_table	= ltc4151_id,
-- 
2.9.0

[PATCH 2/2] hwmon: (ltc4151) Add devicetree binding for ltc4151

[Daniel Golle]

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
(resent to linux-hwmon which by accident wasn't among the receipients)
 Documentation/devicetree/bindings/hwmon/ltc4151.txt | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/ltc4151.txt

diff --git a/Documentation/devicetree/bindings/hwmon/ltc4151.txt b/Documentation/devicetree/bindings/hwmon/ltc4151.txt
new file mode 100644
index 0000000..aec69ed
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/ltc4151.txt
@@ -0,0 +1,18 @@
+ltc4151 properties
+
+Required properties:
+- compatible: Must be "lltc,ltc4151"
+- reg: I2C address
+
+Optional properties:
+
+- shunt-resistor
+	Shunt resistor value in micro-Ohm
+
+Example:
+
+ltc4151@6e {
+	compatible = "lltc,ltc4151";
+	reg = <0x6e>;
+	shunt-resistor = <1500>;
+};
-- 
2.9.0

Re: [PATCH 2/2] hwmon: (ltc4151) Add devicetree binding for ltc4151

[Rob Herring]

On Mon, Jul 25, 2016 at 01:56:20PM +0200, Daniel Golle wrote:
> Signed-off-by: Daniel Golle <daniel-g5gK2j5usbvCyp4qypjU+w@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/hwmon/ltc4151.txt | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/ltc4151.txt
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/ltc4151.txt b/Documentation/devicetree/bindings/hwmon/ltc4151.txt
> new file mode 100644
> index 0000000..aec69ed
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/ltc4151.txt
> @@ -0,0 +1,18 @@
> +ltc4151 properties

Add a brief description of what the chip does please.

> +
> +Required properties:
> +- compatible: Must be "lltc,ltc4151"
> +- reg: I2C address
> +
> +Optional properties:
> +
> +- shunt-resistor
> +	Shunt resistor value in micro-Ohm

Add a unit suffix (-micro-ohms).

> +
> +Example:
> +
> +ltc4151@6e {
> +	compatible = "lltc,ltc4151";
> +	reg = <0x6e>;
> +	shunt-resistor = <1500>;
> +};
> -- 
> 2.9.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2 v2] hwmon: (ltc4151) Make shunt-resistor configurable

[Guenter Roeck]

Daniel,

On 07/25/2016 04:56 AM, Daniel Golle wrote:
> Allow to specify the resistance of the attached shunt via DT by
> adding the shunt-resistor property. Fall-back to the previous
> default (1 mOhm) if unset.
>
> Signed-off-by: Daniel Golle <daniel-g5gK2j5usbvCyp4qypjU+w@public.gmane.org>

Please resend with the property name changed as asked for by Rob.

Thanks,
Guenter

> ---
> v2: removed unneeded things and throw -EINVAL for shunt==0
>
>   drivers/hwmon/ltc4151.c | 21 +++++++++++++++++++--
>   1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwmon/ltc4151.c b/drivers/hwmon/ltc4151.c
> index c86a184..419c6f7 100644
> --- a/drivers/hwmon/ltc4151.c
> +++ b/drivers/hwmon/ltc4151.c
> @@ -30,6 +30,7 @@
>
>   #include <linux/kernel.h>
>   #include <linux/module.h>
> +#include <linux/of.h>
>   #include <linux/init.h>
>   #include <linux/err.h>
>   #include <linux/slab.h>
> @@ -52,6 +53,7 @@ struct ltc4151_data {
>   	struct mutex update_lock;
>   	bool valid;
>   	unsigned long last_updated; /* in jiffies */
> +	unsigned int shunt; /* in micro ohms */
>
>   	/* Registers */
>   	u8 regs[6];
> @@ -111,9 +113,9 @@ static int ltc4151_get_value(struct ltc4151_data *data, u8 reg)
>   	case LTC4151_SENSE_H:
>   		/*
>   		 * 20uV resolution. Convert to current as measured with
> -		 * an 1 mOhm sense resistor, in mA.
> +		 * a given sense resistor, in mA.
>   		 */
> -		val = val * 20;
> +		val = val * 20 * 1000 / data->shunt;
>   		break;
>   	case LTC4151_VIN_H:
>   		/* 25 mV per increment */
> @@ -176,6 +178,7 @@ static int ltc4151_probe(struct i2c_client *client,
>   	struct device *dev = &client->dev;
>   	struct ltc4151_data *data;
>   	struct device *hwmon_dev;
> +	u32 shunt;
>
>   	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
>   		return -ENODEV;
> @@ -184,6 +187,14 @@ static int ltc4151_probe(struct i2c_client *client,
>   	if (!data)
>   		return -ENOMEM;
>
> +	if (of_property_read_u32(client->dev.of_node, "shunt-resistor", &shunt))
> +		shunt = 1000; /* 1 mOhm if not set via DT */
> +
> +	if (shunt == 0)
> +		return -EINVAL;
> +
> +	data->shunt = shunt;
> +
>   	data->client = client;
>   	mutex_init(&data->update_lock);
>
> @@ -199,10 +210,16 @@ static const struct i2c_device_id ltc4151_id[] = {
>   };
>   MODULE_DEVICE_TABLE(i2c, ltc4151_id);
>
> +static const struct of_device_id ltc4151_match[] = {
> +	{ .compatible = "lltc,ltc4151" },
> +	{},
> +};
> +
>   /* This is the driver that will be inserted */
>   static struct i2c_driver ltc4151_driver = {
>   	.driver = {
>   		.name	= "ltc4151",
> +		.of_match_table = of_match_ptr(ltc4151_match),
>   	},
>   	.probe		= ltc4151_probe,
>   	.id_table	= ltc4151_id,
>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2 v2] hwmon: (ltc4151) Make shunt-resistor configurable

[Daniel Golle]

Hi Guenter,

On Sat, Jul 30, 2016 at 09:00:14AM -0700, Guenter Roeck wrote:
> Daniel,
> 
> On 07/25/2016 04:56 AM, Daniel Golle wrote:
> > Allow to specify the resistance of the attached shunt via DT by
> > adding the shunt-resistor property. Fall-back to the previous
> > default (1 mOhm) if unset.
> > 
> > Signed-off-by: Daniel Golle <daniel-g5gK2j5usbvCyp4qypjU+w@public.gmane.org>
> 
> Please resend with the property name changed as asked for by Rob.

Ok, will do. I originally copy-pasted the property name from ina209,
so I reckon it's supposed to be changed there as well...?

Cheers

Daniel

> 
> Thanks,
> Guenter
> 
> > ---
> > v2: removed unneeded things and throw -EINVAL for shunt==0
> > 
> >   drivers/hwmon/ltc4151.c | 21 +++++++++++++++++++--
> >   1 file changed, 19 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/hwmon/ltc4151.c b/drivers/hwmon/ltc4151.c
> > index c86a184..419c6f7 100644
> > --- a/drivers/hwmon/ltc4151.c
> > +++ b/drivers/hwmon/ltc4151.c
> > @@ -30,6 +30,7 @@
> > 
> >   #include <linux/kernel.h>
> >   #include <linux/module.h>
> > +#include <linux/of.h>
> >   #include <linux/init.h>
> >   #include <linux/err.h>
> >   #include <linux/slab.h>
> > @@ -52,6 +53,7 @@ struct ltc4151_data {
> >   	struct mutex update_lock;
> >   	bool valid;
> >   	unsigned long last_updated; /* in jiffies */
> > +	unsigned int shunt; /* in micro ohms */
> > 
> >   	/* Registers */
> >   	u8 regs[6];
> > @@ -111,9 +113,9 @@ static int ltc4151_get_value(struct ltc4151_data *data, u8 reg)
> >   	case LTC4151_SENSE_H:
> >   		/*
> >   		 * 20uV resolution. Convert to current as measured with
> > -		 * an 1 mOhm sense resistor, in mA.
> > +		 * a given sense resistor, in mA.
> >   		 */
> > -		val = val * 20;
> > +		val = val * 20 * 1000 / data->shunt;
> >   		break;
> >   	case LTC4151_VIN_H:
> >   		/* 25 mV per increment */
> > @@ -176,6 +178,7 @@ static int ltc4151_probe(struct i2c_client *client,
> >   	struct device *dev = &client->dev;
> >   	struct ltc4151_data *data;
> >   	struct device *hwmon_dev;
> > +	u32 shunt;
> > 
> >   	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> >   		return -ENODEV;
> > @@ -184,6 +187,14 @@ static int ltc4151_probe(struct i2c_client *client,
> >   	if (!data)
> >   		return -ENOMEM;
> > 
> > +	if (of_property_read_u32(client->dev.of_node, "shunt-resistor", &shunt))
> > +		shunt = 1000; /* 1 mOhm if not set via DT */
> > +
> > +	if (shunt == 0)
> > +		return -EINVAL;
> > +
> > +	data->shunt = shunt;
> > +
> >   	data->client = client;
> >   	mutex_init(&data->update_lock);
> > 
> > @@ -199,10 +210,16 @@ static const struct i2c_device_id ltc4151_id[] = {
> >   };
> >   MODULE_DEVICE_TABLE(i2c, ltc4151_id);
> > 
> > +static const struct of_device_id ltc4151_match[] = {
> > +	{ .compatible = "lltc,ltc4151" },
> > +	{},
> > +};
> > +
> >   /* This is the driver that will be inserted */
> >   static struct i2c_driver ltc4151_driver = {
> >   	.driver = {
> >   		.name	= "ltc4151",
> > +		.of_match_table = of_match_ptr(ltc4151_match),
> >   	},
> >   	.probe		= ltc4151_probe,
> >   	.id_table	= ltc4151_id,
> > 
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2 v2] hwmon: (ltc4151) Make shunt-resistor configurable

[Guenter Roeck]

On 07/30/2016 09:11 AM, Daniel Golle wrote:
> Hi Guenter,
>
> On Sat, Jul 30, 2016 at 09:00:14AM -0700, Guenter Roeck wrote:
>> Daniel,
>>
>> On 07/25/2016 04:56 AM, Daniel Golle wrote:
>>> Allow to specify the resistance of the attached shunt via DT by
>>> adding the shunt-resistor property. Fall-back to the previous
>>> default (1 mOhm) if unset.
>>>
>>> Signed-off-by: Daniel Golle <daniel-g5gK2j5usbvCyp4qypjU+w@public.gmane.org>
>>
>> Please resend with the property name changed as asked for by Rob.
>
> Ok, will do. I originally copy-pasted the property name from ina209,
> so I reckon it's supposed to be changed there as well...?
>

One can not simply change an existing property. We would have to deprecate
the old one, define a replacement, and have the driver support both. For
ina2xx, it would have to be done in the iio driver as well.

Sure, if you feel up to it, feel free to prepare a set of patches to do that.

Note that Documentation/devicetree/bindings/property-units.txt documents
that -micro-ohms should be used.

Thanks,
Guenter

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/2] hwmon: (ltc4151) Make shunt-resistor configurable

[Daniel Golle]

Allow to specify the resistance of the attached shunt via DT by
adding the shunt-resistor property. Fall-back to the previous
default (1 mOhm) if unset.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2: add -micro-ohms unit suffix

 drivers/hwmon/ltc4151.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/ltc4151.c b/drivers/hwmon/ltc4151.c
index c86a184..d36e5e1 100644
--- a/drivers/hwmon/ltc4151.c
+++ b/drivers/hwmon/ltc4151.c
@@ -30,6 +30,7 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/init.h>
 #include <linux/err.h>
 #include <linux/slab.h>
@@ -52,6 +53,7 @@ struct ltc4151_data {
 	struct mutex update_lock;
 	bool valid;
 	unsigned long last_updated; /* in jiffies */
+	unsigned int shunt; /* in micro ohms */
 
 	/* Registers */
 	u8 regs[6];
@@ -111,9 +113,9 @@ static int ltc4151_get_value(struct ltc4151_data *data, u8 reg)
 	case LTC4151_SENSE_H:
 		/*
 		 * 20uV resolution. Convert to current as measured with
-		 * an 1 mOhm sense resistor, in mA.
+		 * a given sense resistor, in mA.
 		 */
-		val = val * 20;
+		val = val * 20 * 1000 / data->shunt;
 		break;
 	case LTC4151_VIN_H:
 		/* 25 mV per increment */
@@ -176,6 +178,7 @@ static int ltc4151_probe(struct i2c_client *client,
 	struct device *dev = &client->dev;
 	struct ltc4151_data *data;
 	struct device *hwmon_dev;
+	u32 shunt;
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
 		return -ENODEV;
@@ -184,6 +187,14 @@ static int ltc4151_probe(struct i2c_client *client,
 	if (!data)
 		return -ENOMEM;
 
+	if (of_property_read_u32(client->dev.of_node, "shunt-resistor-micro-ohms", &shunt))
+		shunt = 1000; /* 1 mOhm if not set via DT */
+
+	if (shunt == 0)
+		return -EINVAL;
+
+	data->shunt = shunt;
+
 	data->client = client;
 	mutex_init(&data->update_lock);
 
@@ -199,10 +210,16 @@ static const struct i2c_device_id ltc4151_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, ltc4151_id);
 
+static const struct of_device_id ltc4151_match[] = {
+	{ .compatible = "lltc,ltc4151" },
+	{},
+};
+
 /* This is the driver that will be inserted */
 static struct i2c_driver ltc4151_driver = {
 	.driver = {
 		.name	= "ltc4151",
+		.of_match_table = of_match_ptr(ltc4151_match),
 	},
 	.probe		= ltc4151_probe,
 	.id_table	= ltc4151_id,
-- 
2.9.2

[PATCH 2/2] hwmon: (ltc4151) Add devicetree binding for ltc4151

[Daniel Golle]

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2: add description and -micro-ohms unit suffix to property

 Documentation/devicetree/bindings/hwmon/ltc4151.txt | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/ltc4151.txt

diff --git a/Documentation/devicetree/bindings/hwmon/ltc4151.txt b/Documentation/devicetree/bindings/hwmon/ltc4151.txt
new file mode 100644
index 0000000..d008a5e
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/ltc4151.txt
@@ -0,0 +1,18 @@
+LTC4151 High Voltage I2C Current and Voltage Monitor
+
+Required properties:
+- compatible: Must be "lltc,ltc4151"
+- reg: I2C address
+
+Optional properties:
+- shunt-resistor-micro-ohms
+	Shunt resistor value in micro-Ohms
+	Defaults to <1000> if unset.
+
+Example:
+
+ltc4151@6e {
+	compatible = "lltc,ltc4151";
+	reg = <0x6e>;
+	shunt-resistor-micro-ohms = <1500>;
+};
-- 
2.9.2