2024-03-28 16:22:50

by Nuno Sa via B4 Relay

[permalink] [raw]
Subject: [PATCH v4 0/6] iio: temperature: ltc2983: small improvements

The v4 introduces an new dev_errp_probe() helper to deal with cases
where we want to return error pointers. The refactor in the IIO ltc2983
is an heavy user of the pattern and was the main motivation for this.

Also added two new patches so we have three users of the new
dev_errp_probe() helper.

---
Changes in v4:
- Link to v3: https://lore.kernel.org/r/[email protected]
- Patch 1
* New patch
- Patch 2
* Use dev_errp_probe() instead of local variant
- Patch 5
* New patch
- Patch 6
* New patch

---
Nuno Sa (6):
printk: add new dev_errp_probe() helper
iio: temperature: ltc2983: convert to dev_err_probe()
dt-bindings: iio: temperature: ltc2983: document power supply
iio: temperature: ltc2983: support vdd regulator
iio: backend: make use dev_errp_probe()
iio: common: scmi_iio: convert to dev_err_probe()

.../bindings/iio/temperature/adi,ltc2983.yaml | 4 +
drivers/iio/common/scmi_sensors/scmi_iio.c | 45 ++--
drivers/iio/industrialio-backend.c | 8 +-
drivers/iio/temperature/ltc2983.c | 260 ++++++++++-----------
include/linux/dev_printk.h | 5 +
5 files changed, 151 insertions(+), 171 deletions(-)
---
base-commit: 27eea4778db8268cd6dc80a5b853c599bd3099f1
change-id: 20240227-ltc2983-misc-improv-d9c4a3819b1f
--

Thanks!
- Nuno Sá




2024-03-28 16:22:52

by Nuno Sa via B4 Relay

[permalink] [raw]
Subject: [PATCH v4 1/6] printk: add new dev_errp_probe() helper

From: Nuno Sa <[email protected]>

This is similar to dev_err_probe() but for cases where an ERR_PTR() is
to be returned simplifying patterns like:

dev_err_probe(dev, ret, ...);
return ERR_PTR(ret)

Signed-off-by: Nuno Sa <[email protected]>
---
include/linux/dev_printk.h | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/include/linux/dev_printk.h b/include/linux/dev_printk.h
index 6bfe70decc9f..64484d092a77 100644
--- a/include/linux/dev_printk.h
+++ b/include/linux/dev_printk.h
@@ -276,4 +276,9 @@ do { \

__printf(3, 4) int dev_err_probe(const struct device *dev, int err, const char *fmt, ...);

+/* Simple helper for dev_err_probe() when ERR_PTR() is to be returned. */
+#define dev_errp_probe(dev, ___err, fmt, ...) ({ \
+ ERR_PTR(dev_err_probe(dev, ___err, fmt, ##__VA_ARGS__)); \
+})
+
#endif /* _DEVICE_PRINTK_H_ */

--
2.44.0



2024-03-28 16:22:55

by Nuno Sa via B4 Relay

[permalink] [raw]
Subject: [PATCH v4 3/6] dt-bindings: iio: temperature: ltc2983: document power supply

From: Nuno Sa <[email protected]>

Add a property for the VDD power supply regulator.

Reviewed-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Nuno Sa <[email protected]>
---
Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml b/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml
index dbb85135fd66..312febeeb3bb 100644
--- a/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml
+++ b/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml
@@ -57,6 +57,8 @@ properties:
interrupts:
maxItems: 1

+ vdd-supply: true
+
adi,mux-delay-config-us:
description: |
Extra delay prior to each conversion, in addition to the internal 1ms
@@ -460,6 +462,7 @@ required:
- compatible
- reg
- interrupts
+ - vdd-supply

additionalProperties: false

@@ -489,6 +492,7 @@ examples:
#address-cells = <1>;
#size-cells = <0>;

+ vdd-supply = <&supply>;
interrupts = <20 IRQ_TYPE_EDGE_RISING>;
interrupt-parent = <&gpio>;


--
2.44.0



2024-03-28 16:22:59

by Nuno Sa via B4 Relay

[permalink] [raw]
Subject: [PATCH v4 4/6] iio: temperature: ltc2983: support vdd regulator

From: Nuno Sa <[email protected]>

Add support for the power supply regulator.

Signed-off-by: Nuno Sa <[email protected]>
---
drivers/iio/temperature/ltc2983.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c
index b4a8ca36458a..ff7f0829b575 100644
--- a/drivers/iio/temperature/ltc2983.c
+++ b/drivers/iio/temperature/ltc2983.c
@@ -18,6 +18,7 @@
#include <linux/module.h>
#include <linux/property.h>
#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
#include <linux/spi/spi.h>

#include <asm/byteorder.h>
@@ -1574,6 +1575,10 @@ static int ltc2983_probe(struct spi_device *spi)
if (ret)
return ret;

+ ret = devm_regulator_get_enable(&spi->dev, "vdd");
+ if (ret)
+ return ret;
+
gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
if (IS_ERR(gpio))
return PTR_ERR(gpio);

--
2.44.0



2024-03-28 16:23:53

by Nuno Sa via B4 Relay

[permalink] [raw]
Subject: [PATCH v4 5/6] iio: backend: make use dev_errp_probe()

From: Nuno Sa <[email protected]>

Using dev_errp_probe() to simplify the code.

Signed-off-by: Nuno Sa <[email protected]>
---
drivers/iio/industrialio-backend.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
index 2fea2bbbe47f..e0b08283d667 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -296,11 +296,9 @@ struct iio_backend *devm_iio_backend_get(struct device *dev, const char *name)
}

fwnode = fwnode_find_reference(dev_fwnode(dev), "io-backends", index);
- if (IS_ERR(fwnode)) {
- dev_err_probe(dev, PTR_ERR(fwnode),
- "Cannot get Firmware reference\n");
- return ERR_CAST(fwnode);
- }
+ if (IS_ERR(fwnode))
+ return dev_errp_probe(dev, PTR_ERR(fwnode),
+ "Cannot get Firmware reference\n");

guard(mutex)(&iio_back_lock);
list_for_each_entry(back, &iio_back_list, entry) {

--
2.44.0



2024-03-28 16:24:09

by Nuno Sa via B4 Relay

[permalink] [raw]
Subject: [PATCH v4 6/6] iio: common: scmi_iio: convert to dev_err_probe()

From: Nuno Sa <[email protected]>

Make use of dev_err_probe() and dev_errp_probe() to simplify error paths
during probe.

Signed-off-by: Nuno Sa <[email protected]>
---
drivers/iio/common/scmi_sensors/scmi_iio.c | 45 +++++++++++++-----------------
1 file changed, 19 insertions(+), 26 deletions(-)

diff --git a/drivers/iio/common/scmi_sensors/scmi_iio.c b/drivers/iio/common/scmi_sensors/scmi_iio.c
index 0c2caf3570db..30d58af02b4c 100644
--- a/drivers/iio/common/scmi_sensors/scmi_iio.c
+++ b/drivers/iio/common/scmi_sensors/scmi_iio.c
@@ -626,12 +626,10 @@ scmi_alloc_iiodev(struct scmi_device *sdev,
SCMI_PROTOCOL_SENSOR, SCMI_EVENT_SENSOR_UPDATE,
&sensor->sensor_info->id,
&sensor->sensor_update_nb);
- if (ret) {
- dev_err(&iiodev->dev,
- "Error in registering sensor update notifier for sensor %s err %d",
- sensor->sensor_info->name, ret);
- return ERR_PTR(ret);
- }
+ if (ret)
+ return dev_errp_probe(&iiodev->dev, ret,
+ "Error in registering sensor update notifier for sensor %s err %d",
+ sensor->sensor_info->name, ret);

scmi_iio_set_timestamp_channel(&iio_channels[i], i);
iiodev->channels = iio_channels;
@@ -653,10 +651,9 @@ static int scmi_iio_dev_probe(struct scmi_device *sdev)
return -ENODEV;

sensor_ops = handle->devm_protocol_get(sdev, SCMI_PROTOCOL_SENSOR, &ph);
- if (IS_ERR(sensor_ops)) {
- dev_err(dev, "SCMI device has no sensor interface\n");
- return PTR_ERR(sensor_ops);
- }
+ if (IS_ERR(sensor_ops))
+ return dev_err_probe(dev, PTR_ERR(sensor_ops),
+ "SCMI device has no sensor interface\n");

nr_sensors = sensor_ops->count_get(ph);
if (!nr_sensors) {
@@ -667,8 +664,8 @@ static int scmi_iio_dev_probe(struct scmi_device *sdev)
for (i = 0; i < nr_sensors; i++) {
sensor_info = sensor_ops->info_get(ph, i);
if (!sensor_info) {
- dev_err(dev, "SCMI sensor %d has missing info\n", i);
- return -EINVAL;
+ return dev_err_probe(dev, -EINVAL,
+ "SCMI sensor %d has missing info\n", i);
}

/* This driver only supports 3-axis accel and gyro, skipping other sensors */
@@ -683,29 +680,25 @@ static int scmi_iio_dev_probe(struct scmi_device *sdev)
scmi_iio_dev = scmi_alloc_iiodev(sdev, sensor_ops, ph,
sensor_info);
if (IS_ERR(scmi_iio_dev)) {
- dev_err(dev,
- "failed to allocate IIO device for sensor %s: %ld\n",
- sensor_info->name, PTR_ERR(scmi_iio_dev));
- return PTR_ERR(scmi_iio_dev);
+ return dev_err_probe(dev, PTR_ERR(scmi_iio_dev),
+ "failed to allocate IIO device for sensor %s: %ld\n",
+ sensor_info->name, PTR_ERR(scmi_iio_dev));
}

err = devm_iio_kfifo_buffer_setup(&scmi_iio_dev->dev,
scmi_iio_dev,
&scmi_iio_buffer_ops);
if (err < 0) {
- dev_err(dev,
- "IIO buffer setup error at sensor %s: %d\n",
- sensor_info->name, err);
- return err;
+ return dev_err_probe(dev, err,
+ "IIO buffer setup error at sensor %s: %d\n",
+ sensor_info->name, err);
}

err = devm_iio_device_register(dev, scmi_iio_dev);
- if (err) {
- dev_err(dev,
- "IIO device registration failed at sensor %s: %d\n",
- sensor_info->name, err);
- return err;
- }
+ if (err)
+ return dev_err_probe(dev, err,
+ "IIO device registration failed at sensor %s: %d\n",
+ sensor_info->name, err);
}
return err;
}

--
2.44.0



2024-03-28 16:30:49

by Nuno Sa via B4 Relay

[permalink] [raw]
Subject: [PATCH v4 2/6] iio: temperature: ltc2983: convert to dev_err_probe()

From: Nuno Sa <[email protected]>

Use dev_err_probe() in the probe() path. While at it, made some simple
improvements:
* Declare a struct device *dev helper. This also makes the style more
consistent (some places the helper was used and not in other places);
* Explicitly included the err.h and errno.h headers;
* Removed an useless else if();
* Removed some unnecessary line breaks.

Signed-off-by: Nuno Sa <[email protected]>
---
drivers/iio/temperature/ltc2983.c | 255 +++++++++++++++++---------------------
1 file changed, 115 insertions(+), 140 deletions(-)

diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c
index 3c4524d57b8e..b4a8ca36458a 100644
--- a/drivers/iio/temperature/ltc2983.c
+++ b/drivers/iio/temperature/ltc2983.c
@@ -8,6 +8,8 @@
#include <linux/bitfield.h>
#include <linux/completion.h>
#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.h>
#include <linux/kernel.h>
#include <linux/iio/iio.h>
#include <linux/interrupt.h>
@@ -657,10 +659,11 @@ ltc2983_thermocouple_new(const struct fwnode_handle *child, struct ltc2983_data
const struct ltc2983_sensor *sensor)
{
struct ltc2983_thermocouple *thermo;
+ struct device *dev = &st->spi->dev;
u32 oc_current;
int ret;

- thermo = devm_kzalloc(&st->spi->dev, sizeof(*thermo), GFP_KERNEL);
+ thermo = devm_kzalloc(dev, sizeof(*thermo), GFP_KERNEL);
if (!thermo)
return ERR_PTR(-ENOMEM);

@@ -687,21 +690,19 @@ ltc2983_thermocouple_new(const struct fwnode_handle *child, struct ltc2983_data
LTC2983_THERMOCOUPLE_OC_CURR(3);
break;
default:
- dev_err(&st->spi->dev,
- "Invalid open circuit current:%u", oc_current);
- return ERR_PTR(-EINVAL);
+ return dev_errp_probe(dev, -EINVAL,
+ "Invalid open circuit current:%u",
+ oc_current);
}

thermo->sensor_config |= LTC2983_THERMOCOUPLE_OC_CHECK(1);
}
/* validate channel index */
if (!(thermo->sensor_config & LTC2983_THERMOCOUPLE_DIFF_MASK) &&
- sensor->chan < LTC2983_DIFFERENTIAL_CHAN_MIN) {
- dev_err(&st->spi->dev,
- "Invalid chann:%d for differential thermocouple",
- sensor->chan);
- return ERR_PTR(-EINVAL);
- }
+ sensor->chan < LTC2983_DIFFERENTIAL_CHAN_MIN)
+ return dev_errp_probe(dev, -EINVAL,
+ "Invalid chann:%d for differential thermocouple",
+ sensor->chan);

struct fwnode_handle *ref __free(fwnode_handle) =
fwnode_find_reference(child, "adi,cold-junction-handle", 0);
@@ -709,14 +710,13 @@ ltc2983_thermocouple_new(const struct fwnode_handle *child, struct ltc2983_data
ref = NULL;
} else {
ret = fwnode_property_read_u32(ref, "reg", &thermo->cold_junction_chan);
- if (ret) {
+ if (ret)
/*
* This would be catched later but we can just return
* the error right away.
*/
- dev_err(&st->spi->dev, "Property reg must be given\n");
- return ERR_PTR(ret);
- }
+ return dev_errp_probe(dev, ret,
+ "Property reg must be given\n");
}

/* check custom sensor */
@@ -752,16 +752,14 @@ ltc2983_rtd_new(const struct fwnode_handle *child, struct ltc2983_data *st,

struct fwnode_handle *ref __free(fwnode_handle) =
fwnode_find_reference(child, "adi,rsense-handle", 0);
- if (IS_ERR(ref)) {
- dev_err(dev, "Property adi,rsense-handle missing or invalid");
- return ERR_CAST(ref);
- }
+ if (IS_ERR(ref))
+ return dev_errp_probe(dev, PTR_ERR(ref),
+ "Property adi,rsense-handle missing or invalid");

ret = fwnode_property_read_u32(ref, "reg", &rtd->r_sense_chan);
- if (ret) {
- dev_err(dev, "Property reg must be given\n");
- return ERR_PTR(ret);
- }
+ if (ret)
+ return dev_errp_probe(dev, ret,
+ "Property reg must be given\n");

ret = fwnode_property_read_u32(child, "adi,number-of-wires", &n_wires);
if (!ret) {
@@ -780,19 +778,19 @@ ltc2983_rtd_new(const struct fwnode_handle *child, struct ltc2983_data *st,
rtd->sensor_config = LTC2983_RTD_N_WIRES(3);
break;
default:
- dev_err(dev, "Invalid number of wires:%u\n", n_wires);
- return ERR_PTR(-EINVAL);
+ return dev_errp_probe(dev, -EINVAL,
+ "Invalid number of wires:%u\n",
+ n_wires);
}
}

if (fwnode_property_read_bool(child, "adi,rsense-share")) {
/* Current rotation is only available with rsense sharing */
if (fwnode_property_read_bool(child, "adi,current-rotate")) {
- if (n_wires == 2 || n_wires == 3) {
- dev_err(dev,
- "Rotation not allowed for 2/3 Wire RTDs");
- return ERR_PTR(-EINVAL);
- }
+ if (n_wires == 2 || n_wires == 3)
+ return dev_errp_probe(dev, -EINVAL,
+ "Rotation not allowed for 2/3 Wire RTDs");
+
rtd->sensor_config |= LTC2983_RTD_C_ROTATE(1);
} else {
rtd->sensor_config |= LTC2983_RTD_R_SHARE(1);
@@ -815,29 +813,22 @@ ltc2983_rtd_new(const struct fwnode_handle *child, struct ltc2983_data *st,

if (((rtd->sensor_config & LTC2983_RTD_KELVIN_R_SENSE_MASK)
== LTC2983_RTD_KELVIN_R_SENSE_MASK) &&
- (rtd->r_sense_chan <= min)) {
+ (rtd->r_sense_chan <= min))
/* kelvin rsense*/
- dev_err(dev,
- "Invalid rsense chann:%d to use in kelvin rsense",
- rtd->r_sense_chan);
+ return dev_errp_probe(dev, -EINVAL,
+ "Invalid rsense chann:%d to use in kelvin rsense",
+ rtd->r_sense_chan);

- return ERR_PTR(-EINVAL);
- }
-
- if (sensor->chan < min || sensor->chan > max) {
- dev_err(dev, "Invalid chann:%d for the rtd config",
- sensor->chan);
-
- return ERR_PTR(-EINVAL);
- }
+ if (sensor->chan < min || sensor->chan > max)
+ return dev_errp_probe(dev, -EINVAL,
+ "Invalid chann:%d for the rtd config",
+ sensor->chan);
} else {
/* same as differential case */
- if (sensor->chan < LTC2983_DIFFERENTIAL_CHAN_MIN) {
- dev_err(&st->spi->dev,
- "Invalid chann:%d for RTD", sensor->chan);
-
- return ERR_PTR(-EINVAL);
- }
+ if (sensor->chan < LTC2983_DIFFERENTIAL_CHAN_MIN)
+ return dev_errp_probe(dev, -EINVAL,
+ "Invalid chann:%d for RTD",
+ sensor->chan);
}

/* check custom sensor */
@@ -885,10 +876,9 @@ ltc2983_rtd_new(const struct fwnode_handle *child, struct ltc2983_data *st,
rtd->excitation_current = 0x08;
break;
default:
- dev_err(&st->spi->dev,
- "Invalid value for excitation current(%u)",
- excitation_current);
- return ERR_PTR(-EINVAL);
+ return dev_errp_probe(dev, -EINVAL,
+ "Invalid value for excitation current(%u)",
+ excitation_current);
}
}

@@ -912,16 +902,14 @@ ltc2983_thermistor_new(const struct fwnode_handle *child, struct ltc2983_data *s

struct fwnode_handle *ref __free(fwnode_handle) =
fwnode_find_reference(child, "adi,rsense-handle", 0);
- if (IS_ERR(ref)) {
- dev_err(dev, "Property adi,rsense-handle missing or invalid");
- return ERR_CAST(ref);
- }
+ if (IS_ERR(ref))
+ return dev_errp_probe(dev, PTR_ERR(ref),
+ "Property adi,rsense-handle missing or invalid");

ret = fwnode_property_read_u32(ref, "reg", &thermistor->r_sense_chan);
- if (ret) {
- dev_err(dev, "rsense channel must be configured...\n");
- return ERR_PTR(ret);
- }
+ if (ret)
+ return dev_errp_probe(dev, ret,
+ "rsense channel must be configured...\n");

if (fwnode_property_read_bool(child, "adi,single-ended")) {
thermistor->sensor_config = LTC2983_THERMISTOR_SGL(1);
@@ -936,12 +924,10 @@ ltc2983_thermistor_new(const struct fwnode_handle *child, struct ltc2983_data *s
}
/* validate channel index */
if (!(thermistor->sensor_config & LTC2983_THERMISTOR_DIFF_MASK) &&
- sensor->chan < LTC2983_DIFFERENTIAL_CHAN_MIN) {
- dev_err(&st->spi->dev,
- "Invalid chann:%d for differential thermistor",
- sensor->chan);
- return ERR_PTR(-EINVAL);
- }
+ sensor->chan < LTC2983_DIFFERENTIAL_CHAN_MIN)
+ return dev_errp_probe(dev, -EINVAL,
+ "Invalid chann:%d for differential thermistor",
+ sensor->chan);

/* check custom sensor */
if (sensor->type >= LTC2983_SENSOR_THERMISTOR_STEINHART) {
@@ -980,12 +966,10 @@ ltc2983_thermistor_new(const struct fwnode_handle *child, struct ltc2983_data *s
switch (excitation_current) {
case 0:
/* auto range */
- if (sensor->type >=
- LTC2983_SENSOR_THERMISTOR_STEINHART) {
- dev_err(&st->spi->dev,
- "Auto Range not allowed for custom sensors\n");
- return ERR_PTR(-EINVAL);
- }
+ if (sensor->type >= LTC2983_SENSOR_THERMISTOR_STEINHART)
+ return dev_errp_probe(dev, -EINVAL,
+ "Auto Range not allowed for custom sensors\n");
+
thermistor->excitation_current = 0x0c;
break;
case 250:
@@ -1022,10 +1006,9 @@ ltc2983_thermistor_new(const struct fwnode_handle *child, struct ltc2983_data *s
thermistor->excitation_current = 0x0b;
break;
default:
- dev_err(&st->spi->dev,
- "Invalid value for excitation current(%u)",
- excitation_current);
- return ERR_PTR(-EINVAL);
+ return dev_errp_probe(dev, -EINVAL,
+ "Invalid value for excitation current(%u)",
+ excitation_current);
}
}

@@ -1036,11 +1019,12 @@ static struct ltc2983_sensor *
ltc2983_diode_new(const struct fwnode_handle *child, const struct ltc2983_data *st,
const struct ltc2983_sensor *sensor)
{
+ struct device *dev = &st->spi->dev;
struct ltc2983_diode *diode;
u32 temp = 0, excitation_current = 0;
int ret;

- diode = devm_kzalloc(&st->spi->dev, sizeof(*diode), GFP_KERNEL);
+ diode = devm_kzalloc(dev, sizeof(*diode), GFP_KERNEL);
if (!diode)
return ERR_PTR(-ENOMEM);

@@ -1055,12 +1039,11 @@ ltc2983_diode_new(const struct fwnode_handle *child, const struct ltc2983_data *

/* validate channel index */
if (!(diode->sensor_config & LTC2983_DIODE_DIFF_MASK) &&
- sensor->chan < LTC2983_DIFFERENTIAL_CHAN_MIN) {
- dev_err(&st->spi->dev,
- "Invalid chann:%d for differential thermistor",
- sensor->chan);
- return ERR_PTR(-EINVAL);
- }
+ sensor->chan < LTC2983_DIFFERENTIAL_CHAN_MIN)
+ return dev_errp_probe(dev, -EINVAL,
+ "Invalid chann:%d for differential thermistor",
+ sensor->chan);
+
/* set common parameters */
diode->sensor.fault_handler = ltc2983_common_fault_handler;
diode->sensor.assign_chan = ltc2983_diode_assign_chan;
@@ -1082,10 +1065,9 @@ ltc2983_diode_new(const struct fwnode_handle *child, const struct ltc2983_data *
diode->excitation_current = 0x03;
break;
default:
- dev_err(&st->spi->dev,
- "Invalid value for excitation current(%u)",
- excitation_current);
- return ERR_PTR(-EINVAL);
+ return dev_errp_probe(dev, -EINVAL,
+ "Invalid value for excitation current(%u)",
+ excitation_current);
}
}

@@ -1101,26 +1083,26 @@ static struct ltc2983_sensor *ltc2983_r_sense_new(struct fwnode_handle *child,
struct ltc2983_data *st,
const struct ltc2983_sensor *sensor)
{
+ struct device *dev = &st->spi->dev;
struct ltc2983_rsense *rsense;
int ret;
u32 temp;

- rsense = devm_kzalloc(&st->spi->dev, sizeof(*rsense), GFP_KERNEL);
+ rsense = devm_kzalloc(dev, sizeof(*rsense), GFP_KERNEL);
if (!rsense)
return ERR_PTR(-ENOMEM);

/* validate channel index */
- if (sensor->chan < LTC2983_DIFFERENTIAL_CHAN_MIN) {
- dev_err(&st->spi->dev, "Invalid chann:%d for r_sense",
- sensor->chan);
- return ERR_PTR(-EINVAL);
- }
+ if (sensor->chan < LTC2983_DIFFERENTIAL_CHAN_MIN)
+ return dev_errp_probe(dev, -EINVAL,
+ "Invalid chann:%d for r_sense",
+ sensor->chan);

ret = fwnode_property_read_u32(child, "adi,rsense-val-milli-ohms", &temp);
- if (ret) {
- dev_err(&st->spi->dev, "Property adi,rsense-val-milli-ohms missing\n");
- return ERR_PTR(-EINVAL);
- }
+ if (ret)
+ return dev_errp_probe(dev, -EINVAL,
+ "Property adi,rsense-val-milli-ohms missing\n");
+
/*
* Times 1000 because we have milli-ohms and __convert_to_raw
* expects scales of 1000000 which are used for all other
@@ -1139,21 +1121,21 @@ static struct ltc2983_sensor *ltc2983_adc_new(struct fwnode_handle *child,
struct ltc2983_data *st,
const struct ltc2983_sensor *sensor)
{
+ struct device *dev = &st->spi->dev;
struct ltc2983_adc *adc;

- adc = devm_kzalloc(&st->spi->dev, sizeof(*adc), GFP_KERNEL);
+ adc = devm_kzalloc(dev, sizeof(*adc), GFP_KERNEL);
if (!adc)
return ERR_PTR(-ENOMEM);

if (fwnode_property_read_bool(child, "adi,single-ended"))
adc->single_ended = true;

- if (!adc->single_ended &&
- sensor->chan < LTC2983_DIFFERENTIAL_CHAN_MIN) {
- dev_err(&st->spi->dev, "Invalid chan:%d for differential adc\n",
- sensor->chan);
- return ERR_PTR(-EINVAL);
- }
+ if (!adc->single_ended && sensor->chan < LTC2983_DIFFERENTIAL_CHAN_MIN)
+ return dev_errp_probe(dev, -EINVAL,
+ "Invalid chan:%d for differential adc\n",
+ sensor->chan);
+
/* set common parameters */
adc->sensor.assign_chan = ltc2983_adc_assign_chan;
adc->sensor.fault_handler = ltc2983_common_fault_handler;
@@ -1165,21 +1147,20 @@ static struct ltc2983_sensor *ltc2983_temp_new(struct fwnode_handle *child,
struct ltc2983_data *st,
const struct ltc2983_sensor *sensor)
{
+ struct device *dev = &st->spi->dev;
struct ltc2983_temp *temp;

- temp = devm_kzalloc(&st->spi->dev, sizeof(*temp), GFP_KERNEL);
+ temp = devm_kzalloc(dev, sizeof(*temp), GFP_KERNEL);
if (!temp)
return ERR_PTR(-ENOMEM);

if (fwnode_property_read_bool(child, "adi,single-ended"))
temp->single_ended = true;

- if (!temp->single_ended &&
- sensor->chan < LTC2983_DIFFERENTIAL_CHAN_MIN) {
- dev_err(&st->spi->dev, "Invalid chan:%d for differential temp\n",
- sensor->chan);
- return ERR_PTR(-EINVAL);
- }
+ if (!temp->single_ended && sensor->chan < LTC2983_DIFFERENTIAL_CHAN_MIN)
+ return dev_errp_probe(dev, -EINVAL,
+ "Invalid chan:%d for differential temp\n",
+ sensor->chan);

temp->custom = __ltc2983_custom_sensor_new(st, child, "adi,custom-temp",
false, 4096, true);
@@ -1329,10 +1310,9 @@ static int ltc2983_parse_fw(struct ltc2983_data *st)
device_property_read_u32(dev, "adi,filter-notch-freq", &st->filter_notch_freq);

st->num_channels = device_get_child_node_count(dev);
- if (!st->num_channels) {
- dev_err(&st->spi->dev, "At least one channel must be given!");
- return -EINVAL;
- }
+ if (!st->num_channels)
+ return dev_err_probe(dev, -EINVAL,
+ "At least one channel must be given!");

st->sensors = devm_kcalloc(dev, st->num_channels, sizeof(*st->sensors),
GFP_KERNEL);
@@ -1419,6 +1399,7 @@ static int ltc2983_eeprom_cmd(struct ltc2983_data *st, unsigned int cmd,
unsigned int wait_time, unsigned int status_reg,
unsigned long status_fail_mask)
{
+ struct device *dev = &st->spi->dev;
unsigned long time;
unsigned int val;
int ret;
@@ -1437,19 +1418,16 @@ static int ltc2983_eeprom_cmd(struct ltc2983_data *st, unsigned int cmd,

time = wait_for_completion_timeout(&st->completion,
msecs_to_jiffies(wait_time));
- if (!time) {
- dev_err(&st->spi->dev, "EEPROM command timed out\n");
- return -ETIMEDOUT;
- }
+ if (!time)
+ return dev_err_probe(dev, -ETIMEDOUT, "EEPROM command timed out\n");

ret = regmap_read(st->regmap, status_reg, &val);
if (ret)
return ret;

- if (val & status_fail_mask) {
- dev_err(&st->spi->dev, "EEPROM command failed: 0x%02X\n", val);
- return -EINVAL;
- }
+ if (val & status_fail_mask)
+ return dev_err_probe(dev, -EINVAL,
+ "EEPROM command failed: 0x%02X\n", val);

return 0;
}
@@ -1457,16 +1435,15 @@ static int ltc2983_eeprom_cmd(struct ltc2983_data *st, unsigned int cmd,
static int ltc2983_setup(struct ltc2983_data *st, bool assign_iio)
{
u32 iio_chan_t = 0, iio_chan_v = 0, chan, iio_idx = 0, status;
+ struct device *dev = &st->spi->dev;
int ret;

/* make sure the device is up: start bit (7) is 0 and done bit (6) is 1 */
ret = regmap_read_poll_timeout(st->regmap, LTC2983_STATUS_REG, status,
LTC2983_STATUS_UP(status) == 1, 25000,
25000 * 10);
- if (ret) {
- dev_err(&st->spi->dev, "Device startup timed out\n");
- return ret;
- }
+ if (ret)
+ return dev_err_probe(dev, ret, "Device startup timed out\n");

ret = regmap_update_bits(st->regmap, LTC2983_GLOBAL_CONFIG_REG,
LTC2983_NOTCH_FREQ_MASK,
@@ -1566,12 +1543,13 @@ static const struct iio_info ltc2983_iio_info = {

static int ltc2983_probe(struct spi_device *spi)
{
+ struct device *dev = &spi->dev;
struct ltc2983_data *st;
struct iio_dev *indio_dev;
struct gpio_desc *gpio;
int ret;

- indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
if (!indio_dev)
return -ENOMEM;

@@ -1582,10 +1560,9 @@ static int ltc2983_probe(struct spi_device *spi)
return -ENODEV;

st->regmap = devm_regmap_init_spi(spi, &ltc2983_regmap_config);
- if (IS_ERR(st->regmap)) {
- dev_err(&spi->dev, "Failed to initialize regmap\n");
- return PTR_ERR(st->regmap);
- }
+ if (IS_ERR(st->regmap))
+ return dev_err_probe(dev, PTR_ERR(st->regmap),
+ "Failed to initialize regmap\n");

mutex_init(&st->lock);
init_completion(&st->completion);
@@ -1597,7 +1574,7 @@ static int ltc2983_probe(struct spi_device *spi)
if (ret)
return ret;

- gpio = devm_gpiod_get_optional(&st->spi->dev, "reset", GPIOD_OUT_HIGH);
+ gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
if (IS_ERR(gpio))
return PTR_ERR(gpio);

@@ -1607,7 +1584,7 @@ static int ltc2983_probe(struct spi_device *spi)
gpiod_set_value_cansleep(gpio, 0);
}

- st->iio_chan = devm_kzalloc(&spi->dev,
+ st->iio_chan = devm_kzalloc(dev,
st->iio_channels * sizeof(*st->iio_chan),
GFP_KERNEL);
if (!st->iio_chan)
@@ -1617,12 +1594,10 @@ static int ltc2983_probe(struct spi_device *spi)
if (ret)
return ret;

- ret = devm_request_irq(&spi->dev, spi->irq, ltc2983_irq_handler,
+ ret = devm_request_irq(dev, spi->irq, ltc2983_irq_handler,
IRQF_TRIGGER_RISING, st->info->name, st);
- if (ret) {
- dev_err(&spi->dev, "failed to request an irq, %d", ret);
- return ret;
- }
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to request an irq\n");

if (st->info->has_eeprom) {
ret = ltc2983_eeprom_cmd(st, LTC2983_EEPROM_WRITE_CMD,
@@ -1639,7 +1614,7 @@ static int ltc2983_probe(struct spi_device *spi)
indio_dev->modes = INDIO_DIRECT_MODE;
indio_dev->info = &ltc2983_iio_info;

- return devm_iio_device_register(&spi->dev, indio_dev);
+ return devm_iio_device_register(dev, indio_dev);
}

static int ltc2983_resume(struct device *dev)

--
2.44.0



2024-03-28 17:07:37

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v4 0/6] iio: temperature: ltc2983: small improvements

On Thu, 28 Mar 2024 17:22:00 +0100
Nuno Sa via B4 Relay <[email protected]> wrote:

> The v4 introduces an new dev_errp_probe() helper to deal with cases
> where we want to return error pointers. The refactor in the IIO ltc2983
> is an heavy user of the pattern and was the main motivation for this.
>
> Also added two new patches so we have three users of the new
> dev_errp_probe() helper.

Probably better to do this as 2 series. The other ltc2983 changes in one series
and one with a cover letter title that will get noticed by
those who care about dev_printk helpers.

From a quick look the content of the patches is fine.

Jonathan

>
> ---
> Changes in v4:
> - Link to v3: https://lore.kernel.org/r/[email protected]
> - Patch 1
> * New patch
> - Patch 2
> * Use dev_errp_probe() instead of local variant
> - Patch 5
> * New patch
> - Patch 6
> * New patch
>
> ---
> Nuno Sa (6):
> printk: add new dev_errp_probe() helper
> iio: temperature: ltc2983: convert to dev_err_probe()
> dt-bindings: iio: temperature: ltc2983: document power supply
> iio: temperature: ltc2983: support vdd regulator
> iio: backend: make use dev_errp_probe()
> iio: common: scmi_iio: convert to dev_err_probe()
>
> .../bindings/iio/temperature/adi,ltc2983.yaml | 4 +
> drivers/iio/common/scmi_sensors/scmi_iio.c | 45 ++--
> drivers/iio/industrialio-backend.c | 8 +-
> drivers/iio/temperature/ltc2983.c | 260 ++++++++++-----------
> include/linux/dev_printk.h | 5 +
> 5 files changed, 151 insertions(+), 171 deletions(-)
> ---
> base-commit: 27eea4778db8268cd6dc80a5b853c599bd3099f1
> change-id: 20240227-ltc2983-misc-improv-d9c4a3819b1f
> --
>
> Thanks!
> - Nuno Sá
>
>


2024-03-28 17:34:03

by Nuno Sá

[permalink] [raw]
Subject: Re: [PATCH v4 0/6] iio: temperature: ltc2983: small improvements

On Thu, 2024-03-28 at 16:56 +0000, Jonathan Cameron wrote:
> On Thu, 28 Mar 2024 17:22:00 +0100
> Nuno Sa via B4 Relay <[email protected]> wrote:
>
> > The v4 introduces an new dev_errp_probe() helper to deal with cases
> > where we want to return error pointers. The refactor in the IIO ltc2983
> > is an heavy user of the pattern and was the main motivation for this.
> >
> > Also added two new patches so we have three users of the new
> > dev_errp_probe() helper.
>
> Probably better to do this as 2 series. The other ltc2983 changes in one series
> and one with a cover letter title that will get noticed by
> those who care about dev_printk helpers.
>

That makes sense, yes.

- Nuno Sá

> From a quick look the content of the patches is fine.
>
> Jonathan
>
> >
> > ---
> > Changes in v4:
> > - Link to v3:
> > https://lore.kernel.org/r/[email protected]
> > - Patch 1
> >  * New patch
> > - Patch 2
> >  * Use dev_errp_probe() instead of local variant
> > - Patch 5
> >  * New patch
> > - Patch 6
> >  * New patch
> >
> > ---
> > Nuno Sa (6):
> >       printk: add new dev_errp_probe() helper
> >       iio: temperature: ltc2983: convert to dev_err_probe()
> >       dt-bindings: iio: temperature: ltc2983: document power supply
> >       iio: temperature: ltc2983: support vdd regulator
> >       iio: backend: make use dev_errp_probe()
> >       iio: common: scmi_iio: convert to dev_err_probe()
> >
> >  .../bindings/iio/temperature/adi,ltc2983.yaml      |   4 +
> >  drivers/iio/common/scmi_sensors/scmi_iio.c         |  45 ++--
> >  drivers/iio/industrialio-backend.c                 |   8 +-
> >  drivers/iio/temperature/ltc2983.c                  | 260 ++++++++++-----------
> >  include/linux/dev_printk.h                         |   5 +
> >  5 files changed, 151 insertions(+), 171 deletions(-)
> > ---
> > base-commit: 27eea4778db8268cd6dc80a5b853c599bd3099f1
> > change-id: 20240227-ltc2983-misc-improv-d9c4a3819b1f
> > --
> >
> > Thanks!
> > - Nuno Sá
> >
> >
>