2013-09-12 11:23:44

by Wei Ni

[permalink] [raw]
Subject: [PATCH v4 0/3] Add power control for lm90

The device lm90 can be controlled by the vcc rail.
Add function to power on/off the vcc.

This series must base on Mark's new dummy regulator support.

This series is v4, previous version patches are:
[v3]: http://www.spinics.net/lists/lm-sensors/msg39503.html
[v2]: http://www.spinics.net/lists/arm-kernel/msg265373.html
[v1]: http://www.mail-archive.com/[email protected]/msg12034.html

Changes from v3:
1. change to use devm_regulator_get, as per discussion in previous patches.
2. just return errors when get error from regulator_get(), Mark's new dummy
regulator can cope with errors.
3. add vendor prefix for GMT
3. list manufacturer in the binding doc.

Changes from v2:
1. use devm_regulator_get_optional(), as suggested by Mark.
2. remove the lm90_power_control(), enable the regulator in the probe()
directly, so the codes are more clear, as suggested by Alexander and Guenter.
3. change the binding documentation, as per Stephen suggestion.
4. couple of changes, as per previous review.

Changes from v1:
1. if get regulator failed, we should continue to run probe function,
not return fail.
2. call regulator_put() in error handler and remove function.
3. add LM90 DT binding document.

Wei Ni (3):
hwmon: (lm90) Add power control
of: add vendor prefix for GMT
Documentation: dt: hwmon: add OF document for LM90

Documentation/devicetree/bindings/hwmon/lm90.txt | 44 ++++++++++++++++++++
.../devicetree/bindings/vendor-prefixes.txt | 1 +
drivers/hwmon/lm90.c | 20 +++++++++
3 files changed, 65 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/lm90.txt

--
1.7.9.5


2013-09-12 11:23:48

by Wei Ni

[permalink] [raw]
Subject: [PATCH v4 3/3] Documentation: dt: hwmon: add OF document for LM90

Add OF document for LM90 in Documentation/devicetree/.

Signed-off-by: Wei Ni <[email protected]>
---
Documentation/devicetree/bindings/hwmon/lm90.txt | 44 ++++++++++++++++++++++
1 file changed, 44 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/lm90.txt

diff --git a/Documentation/devicetree/bindings/hwmon/lm90.txt b/Documentation/devicetree/bindings/hwmon/lm90.txt
new file mode 100644
index 0000000..6888498
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/lm90.txt
@@ -0,0 +1,44 @@
+* LM90 series thermometer.
+
+Required node properties:
+- compatible: manufacturer and chip name, one of
+ "adi,adm1032"
+ "adi,adt7461"
+ "adi,adt7461a"
+ "gmt,g781"
+ "national,lm90"
+ "national,lm86"
+ "national,lm89"
+ "national,lm99"
+ "dallas,max6646"
+ "dallas,max6647"
+ "dallas,max6649"
+ "dallas,max6657"
+ "dallas,max6658"
+ "dallas,max6659"
+ "dallas,max6680"
+ "dallas,max6681"
+ "dallas,max6695"
+ "dallas,max6696"
+ "onnn,nct1008"
+ "winbond,w83l771"
+ "nxp,sa56004"
+
+- reg: I2C bus address of the device
+
+Optional properties:
+- vcc-supply: vcc regulator for the supply voltage.
+ If this is not set, assuming vcc is always powered.
+- interrupts: Contains a single interrupt specifier which describes the
+ LM90 "-ALERT" pin output.
+ See interrupt-controller/interrupts.txt for the format.
+
+Example LM90 node:
+
+temp-sensor {
+ compatible = "onnn,nct1008";
+ reg = <0x4c>;
+ vcc-supply = <&palmas_ldo6_reg>;
+ interrupt-parent = <&gpio>;
+ interrupts = <TEGRA_GPIO(O, 4) IRQ_TYPE_LEVEL_LOW>;
+}
--
1.7.9.5

2013-09-12 11:23:45

by Wei Ni

[permalink] [raw]
Subject: [PATCH v4 1/3] hwmon: (lm90) Add power control

The device lm90 can be controlled by the vcc rail.
Adding the regulator support to power on/off the vcc rail.
Enable the "vcc" regulator before accessing the device.

Signed-off-by: Wei Ni <[email protected]>
---
drivers/hwmon/lm90.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index cdff742..f3f66d5 100644
--- a/drivers/hwmon/lm90.c
+++ b/drivers/hwmon/lm90.c
@@ -89,6 +89,8 @@
#include <linux/err.h>
#include <linux/mutex.h>
#include <linux/sysfs.h>
+#include <linux/regulator/consumer.h>
+#include <linux/delay.h>

/*
* Addresses to scan
@@ -302,6 +304,7 @@ static const struct lm90_params lm90_params[] = {
struct lm90_data {
struct device *hwmon_dev;
struct mutex update_lock;
+ struct regulator *lm90_reg;
char valid; /* zero until following fields are valid */
unsigned long last_updated; /* in jiffies */
int kind;
@@ -1397,8 +1400,20 @@ static int lm90_probe(struct i2c_client *client,
struct device *dev = &client->dev;
struct i2c_adapter *adapter = to_i2c_adapter(dev->parent);
struct lm90_data *data;
+ struct regulator *reg;
int err;

+ reg = devm_regulator_get(dev, "vcc");
+ if (IS_ERR(reg))
+ return PTR_ERR(reg);
+
+ err = regulator_enable(reg);
+ if (err < 0) {
+ dev_err(&client->dev,
+ "Failed to enable regulator: %d\n", err);
+ return err;
+ }
+
data = devm_kzalloc(&client->dev, sizeof(struct lm90_data), GFP_KERNEL);
if (!data)
return -ENOMEM;
@@ -1406,6 +1421,8 @@ static int lm90_probe(struct i2c_client *client,
i2c_set_clientdata(client, data);
mutex_init(&data->update_lock);

+ data->lm90_reg = reg;
+
/* Set the device type */
data->kind = id->driver_data;
if (data->kind == adm1032) {
@@ -1473,6 +1490,8 @@ exit_remove_files:
lm90_remove_files(client, data);
exit_restore:
lm90_restore_conf(client, data);
+ regulator_disable(data->lm90_reg);
+
return err;
}

@@ -1483,6 +1502,7 @@ static int lm90_remove(struct i2c_client *client)
hwmon_device_unregister(data->hwmon_dev);
lm90_remove_files(client, data);
lm90_restore_conf(client, data);
+ regulator_disable(data->lm90_reg);

return 0;
}
--
1.7.9.5

2013-09-12 11:24:29

by Wei Ni

[permalink] [raw]
Subject: [PATCH v4 2/3] of: add vendor prefix for GMT

Adding Global Mixed-mode Technology Inc. to the list
of devicetree vendor prefixes.

Signed-off-by: Wei Ni <[email protected]>
---
.../devicetree/bindings/vendor-prefixes.txt | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 2956800..634c35f 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -28,6 +28,7 @@ est ESTeem Wireless Modems
fsl Freescale Semiconductor
GEFanuc GE Fanuc Intelligent Platforms Embedded Systems, Inc.
gef GE Fanuc Intelligent Platforms Embedded Systems, Inc.
+gmt Global Mixed-mode Technology Inc.
hisilicon Hisilicon Limited.
hp Hewlett Packard
ibm International Business Machines (IBM)
--
1.7.9.5

2013-09-12 11:52:10

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] hwmon: (lm90) Add power control

On Thu, Sep 12, 2013 at 07:24:19PM +0800, Wei Ni wrote:

> +#include <linux/delay.h>

You don't need this any more, otherwise

Reviewed-by: Mark Brown <[email protected]>


Attachments:
(No filename) (172.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-09-12 13:31:11

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] hwmon: (lm90) Add power control

On 09/12/2013 04:24 AM, Wei Ni wrote:
> The device lm90 can be controlled by the vcc rail.
> Adding the regulator support to power on/off the vcc rail.
> Enable the "vcc" regulator before accessing the device.
>
> Signed-off-by: Wei Ni <[email protected]>

I still believe that you should update all .dts files referencing this driver
to match the new requirement.

Guenter

> ---
> drivers/hwmon/lm90.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
> index cdff742..f3f66d5 100644
> --- a/drivers/hwmon/lm90.c
> +++ b/drivers/hwmon/lm90.c
> @@ -89,6 +89,8 @@
> #include <linux/err.h>
> #include <linux/mutex.h>
> #include <linux/sysfs.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/delay.h>
>
> /*
> * Addresses to scan
> @@ -302,6 +304,7 @@ static const struct lm90_params lm90_params[] = {
> struct lm90_data {
> struct device *hwmon_dev;
> struct mutex update_lock;
> + struct regulator *lm90_reg;
> char valid; /* zero until following fields are valid */
> unsigned long last_updated; /* in jiffies */
> int kind;
> @@ -1397,8 +1400,20 @@ static int lm90_probe(struct i2c_client *client,
> struct device *dev = &client->dev;
> struct i2c_adapter *adapter = to_i2c_adapter(dev->parent);
> struct lm90_data *data;
> + struct regulator *reg;
> int err;
>
> + reg = devm_regulator_get(dev, "vcc");
> + if (IS_ERR(reg))
> + return PTR_ERR(reg);
> +
> + err = regulator_enable(reg);
> + if (err < 0) {
> + dev_err(&client->dev,
> + "Failed to enable regulator: %d\n", err);
> + return err;
> + }
> +
> data = devm_kzalloc(&client->dev, sizeof(struct lm90_data), GFP_KERNEL);
> if (!data)
> return -ENOMEM;
> @@ -1406,6 +1421,8 @@ static int lm90_probe(struct i2c_client *client,
> i2c_set_clientdata(client, data);
> mutex_init(&data->update_lock);
>
> + data->lm90_reg = reg;
> +
> /* Set the device type */
> data->kind = id->driver_data;
> if (data->kind == adm1032) {
> @@ -1473,6 +1490,8 @@ exit_remove_files:
> lm90_remove_files(client, data);
> exit_restore:
> lm90_restore_conf(client, data);
> + regulator_disable(data->lm90_reg);
> +
> return err;
> }
>
> @@ -1483,6 +1502,7 @@ static int lm90_remove(struct i2c_client *client)
> hwmon_device_unregister(data->hwmon_dev);
> lm90_remove_files(client, data);
> lm90_restore_conf(client, data);
> + regulator_disable(data->lm90_reg);
>
> return 0;
> }
>

2013-09-12 14:58:26

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] of: add vendor prefix for GMT

On 09/12/2013 05:24 AM, Wei Ni wrote:
> Adding Global Mixed-mode Technology Inc. to the list
> of devicetree vendor prefixes.

> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt

> +gmt Global Mixed-mode Technology Inc.

There's some opportunity for confusion here re: the GMT timezone, but
GMT seems to be what the company abbreviates itself to on their website,
so I guess this is reasonable.

Acked-by: Stephen Warren <[email protected]>

2013-09-12 15:00:30

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] Documentation: dt: hwmon: add OF document for LM90

On 09/12/2013 05:24 AM, Wei Ni wrote:
> Add OF document for LM90 in Documentation/devicetree/.

> diff --git a/Documentation/devicetree/bindings/hwmon/lm90.txt b/Documentation/devicetree/bindings/hwmon/lm90.txt

> +Optional properties:
> +- vcc-supply: vcc regulator for the supply voltage.
> + If this is not set, assuming vcc is always powered.

I thought that was going to be a "Required Property".

2013-09-16 04:21:53

by Wei Ni

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] hwmon: (lm90) Add power control

On 09/12/2013 07:51 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Thu, Sep 12, 2013 at 07:24:19PM +0800, Wei Ni wrote:
>
>> +#include <linux/delay.h>
>
> You don't need this any more, otherwise

Oh, yes, I will remove it.

>
> Reviewed-by: Mark Brown <[email protected]>
>
> * Unknown Key
> * 0x7EA229BD
>

2013-09-16 04:28:42

by Wei Ni

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] Documentation: dt: hwmon: add OF document for LM90

On 09/12/2013 11:00 PM, Stephen Warren wrote:
> On 09/12/2013 05:24 AM, Wei Ni wrote:
>> Add OF document for LM90 in Documentation/devicetree/.
>
>> diff --git a/Documentation/devicetree/bindings/hwmon/lm90.txt b/Documentation/devicetree/bindings/hwmon/lm90.txt
>
>> +Optional properties:
>> +- vcc-supply: vcc regulator for the supply voltage.
>> + If this is not set, assuming vcc is always powered.
>
> I thought that was going to be a "Required Property".

There may have many platforms without regulator, they always power on
this device, so how can they set this "vcc-supply "property if we set it
as "Required Property".
Set to dummy-regulator? but it seems the regulator framework can't
handle it yet.

Wei.

>

2013-09-16 10:24:46

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] Documentation: dt: hwmon: add OF document for LM90

On Mon, Sep 16, 2013 at 12:28:43PM +0800, Wei Ni wrote:
> On 09/12/2013 11:00 PM, Stephen Warren wrote:

> >> +- vcc-supply: vcc regulator for the supply voltage.
> >> + If this is not set, assuming vcc is always powered.

> > I thought that was going to be a "Required Property".

> There may have many platforms without regulator, they always power on
> this device, so how can they set this "vcc-supply "property if we set it
> as "Required Property".
> Set to dummy-regulator? but it seems the regulator framework can't
> handle it yet.

To repeat once more: these platforms will physically have a regulator,
the device won't function without power and unfortunately the device
tree bindings only have optional and required properties.


Attachments:
(No filename) (753.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments