* [PATCH 0/3] hwmon: (lm75) Add regulator support
@ 2020-09-17 10:18 Alban Bedel
2020-09-17 10:18 ` [PATCH 1/3] dt-bindings: hwmon: Convert lm75 bindings to yaml Alban Bedel
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Alban Bedel @ 2020-09-17 10:18 UTC (permalink / raw)
To: linux-hwmon
Cc: Jean Delvare, Guenter Roeck, Rob Herring, Liam Girdwood,
Mark Brown, devicetree, linux-kernel, Alban Bedel
Hi everybody,
this small series add regulator support to the lm75 driver for boards
that don't always power such a sensor. While at it also convert the
DT bindings to yaml.
Alban Bedel (3):
dt-bindings: hwmon: Convert lm75 bindings to yaml
dt-bindings: hwmon: Add the +vs supply to the lm75 bindings
hwmon: (lm75) Add regulator support
.../devicetree/bindings/hwmon/lm75.txt | 39 ------------
.../devicetree/bindings/hwmon/lm75.yaml | 60 +++++++++++++++++++
drivers/hwmon/lm75.c | 31 +++++++++-
3 files changed, 89 insertions(+), 41 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/hwmon/lm75.txt
create mode 100644 Documentation/devicetree/bindings/hwmon/lm75.yaml
--
2.25.1
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1/3] dt-bindings: hwmon: Convert lm75 bindings to yaml 2020-09-17 10:18 [PATCH 0/3] hwmon: (lm75) Add regulator support Alban Bedel @ 2020-09-17 10:18 ` Alban Bedel 2020-09-18 17:28 ` Rob Herring 2020-09-17 10:18 ` [PATCH 2/3] dt-bindings: hwmon: Add the +vs supply to the lm75 bindings Alban Bedel 2020-09-17 10:18 ` [PATCH 3/3] hwmon: (lm75) Add regulator support Alban Bedel 2 siblings, 1 reply; 14+ messages in thread From: Alban Bedel @ 2020-09-17 10:18 UTC (permalink / raw) To: linux-hwmon Cc: Jean Delvare, Guenter Roeck, Rob Herring, Liam Girdwood, Mark Brown, devicetree, linux-kernel, Alban Bedel In order to automate the verification of DT nodes convert lm75.txt to lm75.yaml. Signed-off-by: Alban Bedel <alban.bedel@aerq.com> --- .../devicetree/bindings/hwmon/lm75.txt | 39 ------------- .../devicetree/bindings/hwmon/lm75.yaml | 55 +++++++++++++++++++ 2 files changed, 55 insertions(+), 39 deletions(-) delete mode 100644 Documentation/devicetree/bindings/hwmon/lm75.txt create mode 100644 Documentation/devicetree/bindings/hwmon/lm75.yaml diff --git a/Documentation/devicetree/bindings/hwmon/lm75.txt b/Documentation/devicetree/bindings/hwmon/lm75.txt deleted file mode 100644 index 273616702c51..000000000000 --- a/Documentation/devicetree/bindings/hwmon/lm75.txt +++ /dev/null @@ -1,39 +0,0 @@ -*LM75 hwmon sensor. - -Required properties: -- compatible: manufacturer and chip name, one of - "adi,adt75", - "dallas,ds1775", - "dallas,ds75", - "dallas,ds7505", - "gmt,g751", - "national,lm75", - "national,lm75a", - "national,lm75b", - "maxim,max6625", - "maxim,max6626", - "maxim,max31725", - "maxim,max31726", - "maxim,mcp980x", - "nxp,pct2075", - "st,stds75", - "st,stlm75", - "microchip,tcn75", - "ti,tmp100", - "ti,tmp101", - "ti,tmp105", - "ti,tmp112", - "ti,tmp175", - "ti,tmp275", - "ti,tmp75", - "ti,tmp75b", - "ti,tmp75c", - -- reg: I2C bus address of the device - -Example: - -sensor@48 { - compatible = "st,stlm75"; - reg = <0x48>; -}; diff --git a/Documentation/devicetree/bindings/hwmon/lm75.yaml b/Documentation/devicetree/bindings/hwmon/lm75.yaml new file mode 100644 index 000000000000..78d6291fcdcc --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/lm75.yaml @@ -0,0 +1,55 @@ +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/hwmon/lm75.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: LM75 hwmon sensor + +maintainers: + - Jean Delvare <jdelvare@suse.com> + - Guenter Roeck <linux@roeck-us.net> + +properties: + compatible: + enum: + - adi,adt75 + - dallas,ds1775 + - dallas,ds75 + - dallas,ds7505 + - gmt,g751 + - national,lm75 + - national,lm75a + - national,lm75b + - maxim,max6625 + - maxim,max6626 + - maxim,max31725 + - maxim,max31726 + - maxim,mcp980x + - nxp,pct2075 + - st,stds75 + - st,stlm75 + - microchip,tcn75 + - ti,tmp100 + - ti,tmp101 + - ti,tmp105 + - ti,tmp112 + - ti,tmp175 + - ti,tmp275 + - ti,tmp75 + - ti,tmp75b + - ti,tmp75c + + reg: + maxItems: 1 + +required: + - compatible + - reg + +examples: + - | + sensor@48 { + compatible = "st,stlm75"; + reg = <0x48>; + }; -- 2.25.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] dt-bindings: hwmon: Convert lm75 bindings to yaml 2020-09-17 10:18 ` [PATCH 1/3] dt-bindings: hwmon: Convert lm75 bindings to yaml Alban Bedel @ 2020-09-18 17:28 ` Rob Herring 0 siblings, 0 replies; 14+ messages in thread From: Rob Herring @ 2020-09-18 17:28 UTC (permalink / raw) To: Alban Bedel Cc: Guenter Roeck, Liam Girdwood, Mark Brown, Jean Delvare, linux-kernel, devicetree, linux-hwmon, Rob Herring On Thu, 17 Sep 2020 12:18:17 +0200, Alban Bedel wrote: > In order to automate the verification of DT nodes convert lm75.txt to > lm75.yaml. > > Signed-off-by: Alban Bedel <alban.bedel@aerq.com> > --- > .../devicetree/bindings/hwmon/lm75.txt | 39 ------------- > .../devicetree/bindings/hwmon/lm75.yaml | 55 +++++++++++++++++++ > 2 files changed, 55 insertions(+), 39 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/hwmon/lm75.txt > create mode 100644 Documentation/devicetree/bindings/hwmon/lm75.yaml > My bot found errors running 'make dt_binding_check' on your patch: Documentation/devicetree/bindings/hwmon/lm75.example.dts:21.17-30: Warning (reg_format): /example-0/sensor@48:reg: property has invalid length (4 bytes) (#address-cells == 1, #size-cells == 1) Documentation/devicetree/bindings/hwmon/lm75.example.dt.yaml: Warning (pci_device_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/hwmon/lm75.example.dt.yaml: Warning (pci_device_bus_num): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/hwmon/lm75.example.dt.yaml: Warning (simple_bus_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/hwmon/lm75.example.dt.yaml: Warning (i2c_bus_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/hwmon/lm75.example.dt.yaml: Warning (spi_bus_reg): Failed prerequisite 'reg_format' /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/hwmon/lm75.example.dt.yaml: example-0: sensor@48:reg:0: [72] is too short From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml See https://patchwork.ozlabs.org/patch/1366031 If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure dt-schema is up to date: pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade Please check and re-submit. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] dt-bindings: hwmon: Add the +vs supply to the lm75 bindings 2020-09-17 10:18 [PATCH 0/3] hwmon: (lm75) Add regulator support Alban Bedel 2020-09-17 10:18 ` [PATCH 1/3] dt-bindings: hwmon: Convert lm75 bindings to yaml Alban Bedel @ 2020-09-17 10:18 ` Alban Bedel 2020-09-23 20:16 ` Rob Herring 2020-09-17 10:18 ` [PATCH 3/3] hwmon: (lm75) Add regulator support Alban Bedel 2 siblings, 1 reply; 14+ messages in thread From: Alban Bedel @ 2020-09-17 10:18 UTC (permalink / raw) To: linux-hwmon Cc: Jean Delvare, Guenter Roeck, Rob Herring, Liam Girdwood, Mark Brown, devicetree, linux-kernel, Alban Bedel Some boards might have a regulator that control the +VS supply, add it to the bindings. Signed-off-by: Alban Bedel <alban.bedel@aerq.com> --- Documentation/devicetree/bindings/hwmon/lm75.yaml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Documentation/devicetree/bindings/hwmon/lm75.yaml b/Documentation/devicetree/bindings/hwmon/lm75.yaml index 78d6291fcdcc..1a4405b17924 100644 --- a/Documentation/devicetree/bindings/hwmon/lm75.yaml +++ b/Documentation/devicetree/bindings/hwmon/lm75.yaml @@ -43,6 +43,10 @@ properties: reg: maxItems: 1 + vs-supply: + maxItems: 1 + description: phandle to the regulator that provides the +VS supply + required: - compatible - reg @@ -52,4 +56,5 @@ examples: sensor@48 { compatible = "st,stlm75"; reg = <0x48>; + vs-supply = <&vs>; }; -- 2.25.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] dt-bindings: hwmon: Add the +vs supply to the lm75 bindings 2020-09-17 10:18 ` [PATCH 2/3] dt-bindings: hwmon: Add the +vs supply to the lm75 bindings Alban Bedel @ 2020-09-23 20:16 ` Rob Herring 0 siblings, 0 replies; 14+ messages in thread From: Rob Herring @ 2020-09-23 20:16 UTC (permalink / raw) To: Alban Bedel Cc: linux-hwmon, Jean Delvare, Guenter Roeck, Liam Girdwood, Mark Brown, devicetree, linux-kernel On Thu, Sep 17, 2020 at 12:18:18PM +0200, Alban Bedel wrote: > Some boards might have a regulator that control the +VS supply, add it > to the bindings. > > Signed-off-by: Alban Bedel <alban.bedel@aerq.com> > --- > Documentation/devicetree/bindings/hwmon/lm75.yaml | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/Documentation/devicetree/bindings/hwmon/lm75.yaml b/Documentation/devicetree/bindings/hwmon/lm75.yaml > index 78d6291fcdcc..1a4405b17924 100644 > --- a/Documentation/devicetree/bindings/hwmon/lm75.yaml > +++ b/Documentation/devicetree/bindings/hwmon/lm75.yaml > @@ -43,6 +43,10 @@ properties: > reg: > maxItems: 1 > > + vs-supply: > + maxItems: 1 *-supply is always 1, so drop. > + description: phandle to the regulator that provides the +VS supply > + > required: > - compatible > - reg > @@ -52,4 +56,5 @@ examples: > sensor@48 { > compatible = "st,stlm75"; > reg = <0x48>; > + vs-supply = <&vs>; > }; > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] hwmon: (lm75) Add regulator support 2020-09-17 10:18 [PATCH 0/3] hwmon: (lm75) Add regulator support Alban Bedel 2020-09-17 10:18 ` [PATCH 1/3] dt-bindings: hwmon: Convert lm75 bindings to yaml Alban Bedel 2020-09-17 10:18 ` [PATCH 2/3] dt-bindings: hwmon: Add the +vs supply to the lm75 bindings Alban Bedel @ 2020-09-17 10:18 ` Alban Bedel 2020-09-17 11:00 ` Mark Brown 2020-09-18 5:33 ` Guenter Roeck 2 siblings, 2 replies; 14+ messages in thread From: Alban Bedel @ 2020-09-17 10:18 UTC (permalink / raw) To: linux-hwmon Cc: Jean Delvare, Guenter Roeck, Rob Herring, Liam Girdwood, Mark Brown, devicetree, linux-kernel, Alban Bedel Add regulator support for boards where the sensor first need to be powered up before it can be used. Signed-off-by: Alban Bedel <alban.bedel@aerq.com> --- drivers/hwmon/lm75.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c index ba0be48aeadd..b673f8d2ef20 100644 --- a/drivers/hwmon/lm75.c +++ b/drivers/hwmon/lm75.c @@ -17,6 +17,7 @@ #include <linux/of.h> #include <linux/regmap.h> #include <linux/util_macros.h> +#include <linux/regulator/consumer.h> #include "lm75.h" /* @@ -101,6 +102,7 @@ static const unsigned short normal_i2c[] = { 0x48, 0x49, 0x4a, 0x4b, 0x4c, struct lm75_data { struct i2c_client *client; struct regmap *regmap; + struct regulator *vs; u8 orig_conf; u8 current_conf; u8 resolution; /* In bits, 9 to 16 */ @@ -540,6 +542,8 @@ static void lm75_remove(void *data) struct i2c_client *client = lm75->client; i2c_smbus_write_byte_data(client, LM75_REG_CONF, lm75->orig_conf); + if (lm75->vs) + regulator_disable(lm75->vs); } static int @@ -567,6 +571,14 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id) data->client = client; data->kind = kind; + data->vs = devm_regulator_get_optional(dev, "vs"); + if (IS_ERR(data->vs)) { + if (PTR_ERR(data->vs) == -ENODEV) + data->vs = NULL; + else + return PTR_ERR(data->vs); + } + data->regmap = devm_regmap_init_i2c(client, &lm75_regmap_config); if (IS_ERR(data->regmap)) return PTR_ERR(data->regmap); @@ -581,11 +593,21 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id) data->sample_time = data->params->default_sample_time; data->resolution = data->params->default_resolution; + /* Enable the power */ + if (data->vs) { + err = regulator_enable(data->vs); + if (err) { + dev_err(dev, "failed to enable regulator: %d\n", err); + return err; + } + } + /* Cache original configuration */ status = i2c_smbus_read_byte_data(client, LM75_REG_CONF); if (status < 0) { dev_dbg(dev, "Can't read config? %d\n", status); - return status; + err = status; + goto disable_regulator; } data->orig_conf = status; data->current_conf = status; @@ -593,7 +615,7 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id) err = lm75_write_config(data, data->params->set_mask, data->params->clr_mask); if (err) - return err; + goto disable_regulator; err = devm_add_action_or_reset(dev, lm75_remove, data); if (err) @@ -608,6 +630,11 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id) dev_info(dev, "%s: sensor '%s'\n", dev_name(hwmon_dev), client->name); return 0; + +disable_regulator: + if (data->vs) + regulator_disable(data->vs); + return err; } static const struct i2c_device_id lm75_ids[] = { -- 2.25.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] hwmon: (lm75) Add regulator support 2020-09-17 10:18 ` [PATCH 3/3] hwmon: (lm75) Add regulator support Alban Bedel @ 2020-09-17 11:00 ` Mark Brown 2020-09-18 2:04 ` Guenter Roeck 2020-09-18 5:33 ` Guenter Roeck 1 sibling, 1 reply; 14+ messages in thread From: Mark Brown @ 2020-09-17 11:00 UTC (permalink / raw) To: Alban Bedel Cc: linux-hwmon, Jean Delvare, Guenter Roeck, Rob Herring, Liam Girdwood, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 237 bytes --] On Thu, Sep 17, 2020 at 12:18:19PM +0200, Alban Bedel wrote: > + data->vs = devm_regulator_get_optional(dev, "vs"); > + if (IS_ERR(data->vs)) { Unless the device can work without power you should not be using regulator_get_optional(). [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] hwmon: (lm75) Add regulator support 2020-09-17 11:00 ` Mark Brown @ 2020-09-18 2:04 ` Guenter Roeck 2020-09-18 10:03 ` Mark Brown 0 siblings, 1 reply; 14+ messages in thread From: Guenter Roeck @ 2020-09-18 2:04 UTC (permalink / raw) To: Mark Brown, Alban Bedel Cc: linux-hwmon, Jean Delvare, Rob Herring, Liam Girdwood, devicetree, linux-kernel [-- Attachment #1.1: Type: text/plain, Size: 390 bytes --] On 9/17/20 4:00 AM, Mark Brown wrote: > On Thu, Sep 17, 2020 at 12:18:19PM +0200, Alban Bedel wrote: > >> + data->vs = devm_regulator_get_optional(dev, "vs"); >> + if (IS_ERR(data->vs)) { > > Unless the device can work without power you should not be using > regulator_get_optional(). > The driver works today without regulator, and needs to continue doing so. Guenter [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] hwmon: (lm75) Add regulator support 2020-09-18 2:04 ` Guenter Roeck @ 2020-09-18 10:03 ` Mark Brown 0 siblings, 0 replies; 14+ messages in thread From: Mark Brown @ 2020-09-18 10:03 UTC (permalink / raw) To: Guenter Roeck Cc: Alban Bedel, linux-hwmon, Jean Delvare, Rob Herring, Liam Girdwood, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 598 bytes --] On Thu, Sep 17, 2020 at 07:04:51PM -0700, Guenter Roeck wrote: > On 9/17/20 4:00 AM, Mark Brown wrote: > > On Thu, Sep 17, 2020 at 12:18:19PM +0200, Alban Bedel wrote: > >> + data->vs = devm_regulator_get_optional(dev, "vs"); > >> + if (IS_ERR(data->vs)) { > > Unless the device can work without power you should not be using > > regulator_get_optional(). > The driver works today without regulator, and needs to continue > doing so. And it will continue to do so if it uses the normal regulator API, it will as ever ensure that if no regulator is provided by the firmware a dummy is provided. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] hwmon: (lm75) Add regulator support 2020-09-17 10:18 ` [PATCH 3/3] hwmon: (lm75) Add regulator support Alban Bedel 2020-09-17 11:00 ` Mark Brown @ 2020-09-18 5:33 ` Guenter Roeck 2020-09-18 10:04 ` Mark Brown 2020-09-28 15:13 ` Bedel, Alban 1 sibling, 2 replies; 14+ messages in thread From: Guenter Roeck @ 2020-09-18 5:33 UTC (permalink / raw) To: Alban Bedel, linux-hwmon Cc: Jean Delvare, Rob Herring, Liam Girdwood, Mark Brown, devicetree, linux-kernel On 9/17/20 3:18 AM, Alban Bedel wrote: > Add regulator support for boards where the sensor first need to be > powered up before it can be used. > > Signed-off-by: Alban Bedel <alban.bedel@aerq.com> > --- > drivers/hwmon/lm75.c | 31 +++++++++++++++++++++++++++++-- > 1 file changed, 29 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c > index ba0be48aeadd..b673f8d2ef20 100644 > --- a/drivers/hwmon/lm75.c > +++ b/drivers/hwmon/lm75.c > @@ -17,6 +17,7 @@ > #include <linux/of.h> > #include <linux/regmap.h> > #include <linux/util_macros.h> > +#include <linux/regulator/consumer.h> > #include "lm75.h" > > /* > @@ -101,6 +102,7 @@ static const unsigned short normal_i2c[] = { 0x48, 0x49, 0x4a, 0x4b, 0x4c, > struct lm75_data { > struct i2c_client *client; > struct regmap *regmap; > + struct regulator *vs; > u8 orig_conf; > u8 current_conf; > u8 resolution; /* In bits, 9 to 16 */ > @@ -540,6 +542,8 @@ static void lm75_remove(void *data) > struct i2c_client *client = lm75->client; > > i2c_smbus_write_byte_data(client, LM75_REG_CONF, lm75->orig_conf); > + if (lm75->vs) > + regulator_disable(lm75->vs); > } > > static int > @@ -567,6 +571,14 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id) > data->client = client; > data->kind = kind; > > + data->vs = devm_regulator_get_optional(dev, "vs"); Looking into the regulator API, it may be better if you use devm_regulator_get(). AFAICS it returns a dummy regulator if there is none, and NULL if the regulator subsystem is disabled. So data->vs = devm_regulator_get(dev, "vs"); if (IS_ERR(data->vs)) return PTR_ERR(data->vs); should work and would be less messy. > + if (IS_ERR(data->vs)) { > + if (PTR_ERR(data->vs) == -ENODEV) > + data->vs = NULL; > + else > + return PTR_ERR(data->vs); > + } > + > data->regmap = devm_regmap_init_i2c(client, &lm75_regmap_config); > if (IS_ERR(data->regmap)) > return PTR_ERR(data->regmap); > @@ -581,11 +593,21 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id) > data->sample_time = data->params->default_sample_time; > data->resolution = data->params->default_resolution; > > + /* Enable the power */ > + if (data->vs) { > + err = regulator_enable(data->vs); > + if (err) { > + dev_err(dev, "failed to enable regulator: %d\n", err); > + return err; > + } > + } > + How about device removal ? Don't you have to call regulator_disable() there as well ? If so, it might be best to use devm_add_action_or_reset() to register a disable function. Thanks, Guenter > /* Cache original configuration */ > status = i2c_smbus_read_byte_data(client, LM75_REG_CONF); > if (status < 0) { > dev_dbg(dev, "Can't read config? %d\n", status); > - return status; > + err = status; > + goto disable_regulator; > } > data->orig_conf = status; > data->current_conf = status; > @@ -593,7 +615,7 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id) > err = lm75_write_config(data, data->params->set_mask, > data->params->clr_mask); > if (err) > - return err; > + goto disable_regulator; > > err = devm_add_action_or_reset(dev, lm75_remove, data); > if (err) > @@ -608,6 +630,11 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id) > dev_info(dev, "%s: sensor '%s'\n", dev_name(hwmon_dev), client->name); > > return 0; > + > +disable_regulator: > + if (data->vs) > + regulator_disable(data->vs); > + return err; > } > > static const struct i2c_device_id lm75_ids[] = { > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] hwmon: (lm75) Add regulator support 2020-09-18 5:33 ` Guenter Roeck @ 2020-09-18 10:04 ` Mark Brown 2020-09-28 15:13 ` Bedel, Alban 1 sibling, 0 replies; 14+ messages in thread From: Mark Brown @ 2020-09-18 10:04 UTC (permalink / raw) To: Guenter Roeck Cc: Alban Bedel, linux-hwmon, Jean Delvare, Rob Herring, Liam Girdwood, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 464 bytes --] On Thu, Sep 17, 2020 at 10:33:37PM -0700, Guenter Roeck wrote: > On 9/17/20 3:18 AM, Alban Bedel wrote: > > + err = regulator_enable(data->vs); > How about device removal ? Don't you have to call regulator_disable() > there as well ? If so, it might be best to use devm_add_action_or_reset() > to register a disable function. Yes, disables should be balanced (and any attempt to unregister the regulator with references still held should result in a warning). [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] hwmon: (lm75) Add regulator support 2020-09-18 5:33 ` Guenter Roeck 2020-09-18 10:04 ` Mark Brown @ 2020-09-28 15:13 ` Bedel, Alban 2020-09-28 16:29 ` Guenter Roeck 1 sibling, 1 reply; 14+ messages in thread From: Bedel, Alban @ 2020-09-28 15:13 UTC (permalink / raw) To: linux, linux-hwmon Cc: lgirdwood, broonie, devicetree, linux-kernel, jdelvare, robh+dt [-- Attachment #1: Type: text/plain, Size: 3059 bytes --] On Thu, 2020-09-17 at 22:33 -0700, Guenter Roeck wrote: > On 9/17/20 3:18 AM, Alban Bedel wrote: > > Add regulator support for boards where the sensor first need to be > > powered up before it can be used. > > > > Signed-off-by: Alban Bedel <alban.bedel@aerq.com> > > --- > > drivers/hwmon/lm75.c | 31 +++++++++++++++++++++++++++++-- > > 1 file changed, 29 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c > > index ba0be48aeadd..b673f8d2ef20 100644 > > --- a/drivers/hwmon/lm75.c > > +++ b/drivers/hwmon/lm75.c > > @@ -17,6 +17,7 @@ > > #include <linux/of.h> > > #include <linux/regmap.h> > > #include <linux/util_macros.h> > > +#include <linux/regulator/consumer.h> > > #include "lm75.h" > > > > /* > > @@ -101,6 +102,7 @@ static const unsigned short normal_i2c[] = { > > 0x48, 0x49, 0x4a, 0x4b, 0x4c, > > struct lm75_data { > > struct i2c_client *client; > > struct regmap *regmap; > > + struct regulator *vs; > > u8 orig_conf; > > u8 current_conf; > > u8 resolution; /* In bits, 9 to 16 */ > > @@ -540,6 +542,8 @@ static void lm75_remove(void *data) > > struct i2c_client *client = lm75->client; > > > > i2c_smbus_write_byte_data(client, LM75_REG_CONF, lm75->orig_conf); > > + if (lm75->vs) > > + regulator_disable(lm75->vs); > > } > > > > static int > > @@ -567,6 +571,14 @@ lm75_probe(struct i2c_client *client, const > > struct i2c_device_id *id) > > data->client = client; > > data->kind = kind; > > > > + data->vs = devm_regulator_get_optional(dev, "vs"); > > Looking into the regulator API, it may be better if you use > devm_regulator_get(). > AFAICS it returns a dummy regulator if there is none, and NULL if the > regulator subsystem is disabled. So > data->vs = devm_regulator_get(dev, "vs"); > if (IS_ERR(data->vs)) > return PTR_ERR(data->vs); > should work and would be less messy. Ok, I'll change that in the next version. > > + if (IS_ERR(data->vs)) { > > + if (PTR_ERR(data->vs) == -ENODEV) > > + data->vs = NULL; > > + else > > + return PTR_ERR(data->vs); > > + } > > + > > data->regmap = devm_regmap_init_i2c(client, > > &lm75_regmap_config); > > if (IS_ERR(data->regmap)) > > return PTR_ERR(data->regmap); > > @@ -581,11 +593,21 @@ lm75_probe(struct i2c_client *client, const > > struct i2c_device_id *id) > > data->sample_time = data->params->default_sample_time; > > data->resolution = data->params->default_resolution; > > > > + /* Enable the power */ > > + if (data->vs) { > > + err = regulator_enable(data->vs); > > + if (err) { > > + dev_err(dev, "failed to enable regulator: > > %d\n", err); > > + return err; > > + } > > + } > > + > > How about device removal ? Don't you have to call regulator_disable() > there as well ? If so, it might be best to use > devm_add_action_or_reset() to register a disable function. This is handled in lm75_remove() where I added the regulator_disable() call. Alban [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] hwmon: (lm75) Add regulator support 2020-09-28 15:13 ` Bedel, Alban @ 2020-09-28 16:29 ` Guenter Roeck 2020-09-28 16:31 ` Guenter Roeck 0 siblings, 1 reply; 14+ messages in thread From: Guenter Roeck @ 2020-09-28 16:29 UTC (permalink / raw) To: Bedel, Alban Cc: linux-hwmon, lgirdwood, broonie, devicetree, linux-kernel, jdelvare, robh+dt On Mon, Sep 28, 2020 at 03:13:53PM +0000, Bedel, Alban wrote: > On Thu, 2020-09-17 at 22:33 -0700, Guenter Roeck wrote: > > On 9/17/20 3:18 AM, Alban Bedel wrote: > > > Add regulator support for boards where the sensor first need to be > > > powered up before it can be used. > > > > > > Signed-off-by: Alban Bedel <alban.bedel@aerq.com> > > > --- > > > drivers/hwmon/lm75.c | 31 +++++++++++++++++++++++++++++-- > > > 1 file changed, 29 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c > > > index ba0be48aeadd..b673f8d2ef20 100644 > > > --- a/drivers/hwmon/lm75.c > > > +++ b/drivers/hwmon/lm75.c > > > @@ -17,6 +17,7 @@ > > > #include <linux/of.h> > > > #include <linux/regmap.h> > > > #include <linux/util_macros.h> > > > +#include <linux/regulator/consumer.h> > > > #include "lm75.h" > > > > > > /* > > > @@ -101,6 +102,7 @@ static const unsigned short normal_i2c[] = { > > > 0x48, 0x49, 0x4a, 0x4b, 0x4c, > > > struct lm75_data { > > > struct i2c_client *client; > > > struct regmap *regmap; > > > + struct regulator *vs; > > > u8 orig_conf; > > > u8 current_conf; > > > u8 resolution; /* In bits, 9 to 16 */ > > > @@ -540,6 +542,8 @@ static void lm75_remove(void *data) > > > struct i2c_client *client = lm75->client; > > > > > > i2c_smbus_write_byte_data(client, LM75_REG_CONF, lm75->orig_conf); > > > + if (lm75->vs) > > > + regulator_disable(lm75->vs); > > > } > > > > > > static int > > > @@ -567,6 +571,14 @@ lm75_probe(struct i2c_client *client, const > > > struct i2c_device_id *id) > > > data->client = client; > > > data->kind = kind; > > > > > > + data->vs = devm_regulator_get_optional(dev, "vs"); > > > > Looking into the regulator API, it may be better if you use > > devm_regulator_get(). > > AFAICS it returns a dummy regulator if there is none, and NULL if the > > regulator subsystem is disabled. So > > data->vs = devm_regulator_get(dev, "vs"); > > if (IS_ERR(data->vs)) > > return PTR_ERR(data->vs); > > should work and would be less messy. > > Ok, I'll change that in the next version. > > > > + if (IS_ERR(data->vs)) { > > > + if (PTR_ERR(data->vs) == -ENODEV) > > > + data->vs = NULL; > > > + else > > > + return PTR_ERR(data->vs); > > > + } > > > + > > > data->regmap = devm_regmap_init_i2c(client, > > > &lm75_regmap_config); > > > if (IS_ERR(data->regmap)) > > > return PTR_ERR(data->regmap); > > > @@ -581,11 +593,21 @@ lm75_probe(struct i2c_client *client, const > > > struct i2c_device_id *id) > > > data->sample_time = data->params->default_sample_time; > > > data->resolution = data->params->default_resolution; > > > > > > + /* Enable the power */ > > > + if (data->vs) { > > > + err = regulator_enable(data->vs); > > > + if (err) { > > > + dev_err(dev, "failed to enable regulator: > > > %d\n", err); > > > + return err; > > > + } > > > + } > > > + > > > > How about device removal ? Don't you have to call regulator_disable() > > there as well ? If so, it might be best to use > > devm_add_action_or_reset() to register a disable function. > > This is handled in lm75_remove() where I added the regulator_disable() > call. lm75_remove() won't be called if the probe function fails. Guenter ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] hwmon: (lm75) Add regulator support 2020-09-28 16:29 ` Guenter Roeck @ 2020-09-28 16:31 ` Guenter Roeck 0 siblings, 0 replies; 14+ messages in thread From: Guenter Roeck @ 2020-09-28 16:31 UTC (permalink / raw) To: Bedel, Alban Cc: linux-hwmon, lgirdwood, broonie, devicetree, linux-kernel, jdelvare, robh+dt On Mon, Sep 28, 2020 at 09:29:49AM -0700, Guenter Roeck wrote: > > > > This is handled in lm75_remove() where I added the regulator_disable() > > call. > > lm75_remove() won't be called if the probe function fails. > Sorry, I am confused; please ignore this noise. Guenter ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-09-28 16:31 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-17 10:18 [PATCH 0/3] hwmon: (lm75) Add regulator support Alban Bedel 2020-09-17 10:18 ` [PATCH 1/3] dt-bindings: hwmon: Convert lm75 bindings to yaml Alban Bedel 2020-09-18 17:28 ` Rob Herring 2020-09-17 10:18 ` [PATCH 2/3] dt-bindings: hwmon: Add the +vs supply to the lm75 bindings Alban Bedel 2020-09-23 20:16 ` Rob Herring 2020-09-17 10:18 ` [PATCH 3/3] hwmon: (lm75) Add regulator support Alban Bedel 2020-09-17 11:00 ` Mark Brown 2020-09-18 2:04 ` Guenter Roeck 2020-09-18 10:03 ` Mark Brown 2020-09-18 5:33 ` Guenter Roeck 2020-09-18 10:04 ` Mark Brown 2020-09-28 15:13 ` Bedel, Alban 2020-09-28 16:29 ` Guenter Roeck 2020-09-28 16:31 ` Guenter Roeck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).