* [PATCH v2 0/3] hwmon: (lm75) Add regulator support
@ 2020-09-28 15:39 Alban Bedel
2020-09-28 15:39 ` [PATCH v2 1/3] dt-bindings: hwmon: Convert lm75 bindings to yaml Alban Bedel
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Alban Bedel @ 2020-09-28 15:39 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.
v2: - Fixed the DT example while converting to YAML
- Removed the unneeded maxItems from the binding documentation
- Use dummy regulator insted of explicitly handling missing regulator
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 | 64 +++++++++++++++++++
drivers/hwmon/lm75.c | 23 ++++++-
3 files changed, 85 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] 8+ messages in thread* [PATCH v2 1/3] dt-bindings: hwmon: Convert lm75 bindings to yaml 2020-09-28 15:39 [PATCH v2 0/3] hwmon: (lm75) Add regulator support Alban Bedel @ 2020-09-28 15:39 ` Alban Bedel 2020-09-29 20:07 ` Rob Herring 2020-09-28 15:39 ` [PATCH v2 2/3] dt-bindings: hwmon: Add the +vs supply to the lm75 bindings Alban Bedel 2020-09-28 15:39 ` [PATCH v2 3/3] hwmon: (lm75) Add regulator support Alban Bedel 2 siblings, 1 reply; 8+ messages in thread From: Alban Bedel @ 2020-09-28 15:39 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> --- v2: Fix the example to pass `make dt_binding_check` --- .../devicetree/bindings/hwmon/lm75.txt | 39 ------------ .../devicetree/bindings/hwmon/lm75.yaml | 60 +++++++++++++++++++ 2 files changed, 60 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..203829c6ba6f --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/lm75.yaml @@ -0,0 +1,60 @@ +# 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: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + sensor@48 { + compatible = "st,stlm75"; + reg = <0x48>; + }; + }; -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/3] dt-bindings: hwmon: Convert lm75 bindings to yaml 2020-09-28 15:39 ` [PATCH v2 1/3] dt-bindings: hwmon: Convert lm75 bindings to yaml Alban Bedel @ 2020-09-29 20:07 ` Rob Herring 0 siblings, 0 replies; 8+ messages in thread From: Rob Herring @ 2020-09-29 20:07 UTC (permalink / raw) To: Alban Bedel Cc: linux-hwmon, Jean Delvare, Guenter Roeck, Liam Girdwood, Mark Brown, devicetree, linux-kernel On Mon, Sep 28, 2020 at 05:39:21PM +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> > --- > v2: Fix the example to pass `make dt_binding_check` > --- > .../devicetree/bindings/hwmon/lm75.txt | 39 ------------ > .../devicetree/bindings/hwmon/lm75.yaml | 60 +++++++++++++++++++ > 2 files changed, 60 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..203829c6ba6f > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/lm75.yaml > @@ -0,0 +1,60 @@ > +# 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 additionalProperties: false > + > +examples: > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + sensor@48 { > + compatible = "st,stlm75"; > + reg = <0x48>; > + }; > + }; > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/3] dt-bindings: hwmon: Add the +vs supply to the lm75 bindings 2020-09-28 15:39 [PATCH v2 0/3] hwmon: (lm75) Add regulator support Alban Bedel 2020-09-28 15:39 ` [PATCH v2 1/3] dt-bindings: hwmon: Convert lm75 bindings to yaml Alban Bedel @ 2020-09-28 15:39 ` Alban Bedel 2020-09-29 20:08 ` Rob Herring 2020-09-28 15:39 ` [PATCH v2 3/3] hwmon: (lm75) Add regulator support Alban Bedel 2 siblings, 1 reply; 8+ messages in thread From: Alban Bedel @ 2020-09-28 15:39 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> --- v2: Removed the unneeded `maxItems` attribute --- Documentation/devicetree/bindings/hwmon/lm75.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/hwmon/lm75.yaml b/Documentation/devicetree/bindings/hwmon/lm75.yaml index 203829c6ba6f..1c1ce4cf1616 100644 --- a/Documentation/devicetree/bindings/hwmon/lm75.yaml +++ b/Documentation/devicetree/bindings/hwmon/lm75.yaml @@ -43,6 +43,9 @@ properties: reg: maxItems: 1 + vs-supply: + description: phandle to the regulator that provides the +VS supply + required: - compatible - reg @@ -56,5 +59,6 @@ examples: sensor@48 { compatible = "st,stlm75"; reg = <0x48>; + vs-supply = <&vs>; }; }; -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/3] dt-bindings: hwmon: Add the +vs supply to the lm75 bindings 2020-09-28 15:39 ` [PATCH v2 2/3] dt-bindings: hwmon: Add the +vs supply to the lm75 bindings Alban Bedel @ 2020-09-29 20:08 ` Rob Herring 0 siblings, 0 replies; 8+ messages in thread From: Rob Herring @ 2020-09-29 20:08 UTC (permalink / raw) To: Alban Bedel Cc: Liam Girdwood, Guenter Roeck, devicetree, linux-kernel, Rob Herring, Jean Delvare, Mark Brown, linux-hwmon On Mon, 28 Sep 2020 17:39:22 +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> > --- > v2: Removed the unneeded `maxItems` attribute > --- > Documentation/devicetree/bindings/hwmon/lm75.yaml | 4 ++++ > 1 file changed, 4 insertions(+) > Acked-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 3/3] hwmon: (lm75) Add regulator support 2020-09-28 15:39 [PATCH v2 0/3] hwmon: (lm75) Add regulator support Alban Bedel 2020-09-28 15:39 ` [PATCH v2 1/3] dt-bindings: hwmon: Convert lm75 bindings to yaml Alban Bedel 2020-09-28 15:39 ` [PATCH v2 2/3] dt-bindings: hwmon: Add the +vs supply to the lm75 bindings Alban Bedel @ 2020-09-28 15:39 ` Alban Bedel 2020-09-28 16:39 ` Guenter Roeck 2 siblings, 1 reply; 8+ messages in thread From: Alban Bedel @ 2020-09-28 15:39 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> --- v2: Rely on dummy regulators instead of explicitly handling missing regulator --- drivers/hwmon/lm75.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c index ba0be48aeadd..e394df648c26 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,7 @@ static void lm75_remove(void *data) struct i2c_client *client = lm75->client; i2c_smbus_write_byte_data(client, LM75_REG_CONF, lm75->orig_conf); + regulator_disable(lm75->vs); } static int @@ -567,6 +570,10 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id) data->client = client; data->kind = kind; + data->vs = devm_regulator_get(dev, "vs"); + if (IS_ERR(data->vs)) + 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 +588,19 @@ 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 */ + 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 +608,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 +623,10 @@ 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: + regulator_disable(data->vs); + return err; } static const struct i2c_device_id lm75_ids[] = { -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 3/3] hwmon: (lm75) Add regulator support 2020-09-28 15:39 ` [PATCH v2 3/3] hwmon: (lm75) Add regulator support Alban Bedel @ 2020-09-28 16:39 ` Guenter Roeck 2020-09-29 14:05 ` Bedel, Alban 0 siblings, 1 reply; 8+ messages in thread From: Guenter Roeck @ 2020-09-28 16:39 UTC (permalink / raw) To: Alban Bedel Cc: linux-hwmon, Jean Delvare, Rob Herring, Liam Girdwood, Mark Brown, devicetree, linux-kernel On Mon, Sep 28, 2020 at 05:39:23PM +0200, 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> > --- > v2: Rely on dummy regulators instead of explicitly handling missing > regulator > --- > drivers/hwmon/lm75.c | 23 +++++++++++++++++++++-- > 1 file changed, 21 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c > index ba0be48aeadd..e394df648c26 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,7 @@ static void lm75_remove(void *data) > struct i2c_client *client = lm75->client; > > i2c_smbus_write_byte_data(client, LM75_REG_CONF, lm75->orig_conf); > + regulator_disable(lm75->vs); > } > > static int > @@ -567,6 +570,10 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id) > data->client = client; > data->kind = kind; > > + data->vs = devm_regulator_get(dev, "vs"); > + if (IS_ERR(data->vs)) > + 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 +588,19 @@ 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 */ > + 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; The point of using devm_add_action_or_reset() was specifically to avoid having to have the cleanup gotos. On top of that, the lm75_remove() function was specifically intended to clean up configuration data, not to do anything else. While hijacking lm75_remove() to also disable the regulator is technically correct, it makes the code more difficult to understand, and it creates a potential source for subsequently introduced bugs. Right now I am not inclined to accept this code as-is. Please provide arguments for handling the cleanup this way instead of using devm_add_action_or_reset(). Thanks, Guenter ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 3/3] hwmon: (lm75) Add regulator support 2020-09-28 16:39 ` Guenter Roeck @ 2020-09-29 14:05 ` Bedel, Alban 0 siblings, 0 replies; 8+ messages in thread From: Bedel, Alban @ 2020-09-29 14:05 UTC (permalink / raw) To: linux Cc: lgirdwood, linux-hwmon, broonie, devicetree, linux-kernel, jdelvare, robh+dt [-- Attachment #1: Type: text/plain, Size: 3354 bytes --] On Mon, 2020-09-28 at 09:39 -0700, Guenter Roeck wrote: > On Mon, Sep 28, 2020 at 05:39:23PM +0200, 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> > > --- > > v2: Rely on dummy regulators instead of explicitly handling missing > > regulator > > --- > > drivers/hwmon/lm75.c | 23 +++++++++++++++++++++-- > > 1 file changed, 21 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c > > index ba0be48aeadd..e394df648c26 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,7 @@ static void lm75_remove(void *data) > > struct i2c_client *client = lm75->client; > > > > i2c_smbus_write_byte_data(client, LM75_REG_CONF, lm75- > > >orig_conf); > > + regulator_disable(lm75->vs); > > } > > > > static int > > @@ -567,6 +570,10 @@ lm75_probe(struct i2c_client *client, const > > struct i2c_device_id *id) > > data->client = client; > > data->kind = kind; > > > > + data->vs = devm_regulator_get(dev, "vs"); > > + if (IS_ERR(data->vs)) > > + 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 +588,19 @@ 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 */ > > + 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; > > The point of using devm_add_action_or_reset() was specifically to avoid having > to have the cleanup gotos. On top of that, the lm75_remove() function was > specifically intended to clean up configuration data, not to do anything else. > While hijacking lm75_remove() to also disable the regulator is technically > correct, it makes the code more difficult to understand, and it creates a > potential source for subsequently introduced bugs. Right now I am not inclined > to accept this code as-is. Please provide arguments for handling the cleanup > this way instead of using devm_add_action_or_reset(). I wrote it that way to keep the memory usage lower, but I see your point and will update the patch accordingly. Alban [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-09-29 20:08 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-28 15:39 [PATCH v2 0/3] hwmon: (lm75) Add regulator support Alban Bedel 2020-09-28 15:39 ` [PATCH v2 1/3] dt-bindings: hwmon: Convert lm75 bindings to yaml Alban Bedel 2020-09-29 20:07 ` Rob Herring 2020-09-28 15:39 ` [PATCH v2 2/3] dt-bindings: hwmon: Add the +vs supply to the lm75 bindings Alban Bedel 2020-09-29 20:08 ` Rob Herring 2020-09-28 15:39 ` [PATCH v2 3/3] hwmon: (lm75) Add regulator support Alban Bedel 2020-09-28 16:39 ` Guenter Roeck 2020-09-29 14:05 ` Bedel, Alban
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).