* [PATCH v2 0/4] dev_printk: add dev_errp_probe() helper
@ 2024-04-23 15:20 Nuno Sa via B4 Relay
2024-04-23 15:20 ` [PATCH v2 1/4] dev_printk: add new dev_err_probe() helpers Nuno Sa via B4 Relay
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Nuno Sa via B4 Relay @ 2024-04-23 15:20 UTC (permalink / raw)
To: Petr Mladek, Andy Shevchenko, Chris Down, John Ogness,
Greg Kroah-Hartman, Jonathan Cameron, Lars-Peter Clausen,
Olivier Moysan, Andi Shyti, Jyoti Bhayana
Cc: linux-kernel, linux-iio, Nuno Sa
Hi,
Here it goes the second version of this. Main difference is the
introduction of dev_err_cast_probe(). Full log:
v1:
* https://lore.kernel.org/all/20240404-dev-add_dev_errp_probe-v1-0-d18e3eb7ec3f@analog.com/
v2:
* Patch 1:
- Renamed dev_errp_probe() -> dev_err_ptr_probe();
- Added dev_err_cast_probe().
* Patch 2:
- Make use of dev_err_cast_probe();
- Also use dev_err_ptr_probe() in __ltc2983_custom_sensor_new().
* Patch 3:
- Use dev_err_cast_probe().
---
Nuno Sa (4):
dev_printk: add new dev_err_probe() helpers
iio: temperature: ltc2983: convert to dev_err_probe()
iio: backend: make use of dev_err_cast_probe()
iio: common: scmi_iio: convert to dev_err_probe()
drivers/iio/common/scmi_sensors/scmi_iio.c | 45 ++---
drivers/iio/industrialio-backend.c | 8 +-
drivers/iio/temperature/ltc2983.c | 282 +++++++++++++----------------
include/linux/dev_printk.h | 9 +
4 files changed, 158 insertions(+), 186 deletions(-)
---
base-commit: a59668a9397e7245b26e9be85d23f242ff757ae8
change-id: 20240423-dev-add_dev_errp_probe-1dd04573f4a5
--
Thanks!
- Nuno Sá
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/4] dev_printk: add new dev_err_probe() helpers
2024-04-23 15:20 [PATCH v2 0/4] dev_printk: add dev_errp_probe() helper Nuno Sa via B4 Relay
@ 2024-04-23 15:20 ` Nuno Sa via B4 Relay
2024-04-23 15:31 ` Andy Shevchenko
2024-04-23 15:20 ` [PATCH v2 2/4] iio: temperature: ltc2983: convert to dev_err_probe() Nuno Sa via B4 Relay
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Nuno Sa via B4 Relay @ 2024-04-23 15:20 UTC (permalink / raw)
To: Petr Mladek, Andy Shevchenko, Chris Down, John Ogness,
Greg Kroah-Hartman, Jonathan Cameron, Lars-Peter Clausen,
Olivier Moysan, Andi Shyti, Jyoti Bhayana
Cc: linux-kernel, linux-iio, Nuno Sa
From: Nuno Sa <nuno.sa@analog.com>
This is similar to dev_err_probe() but for cases where an ERR_PTR() or
ERR_CAST() is to be returned simplifying patterns like:
dev_err_probe(dev, ret, ...);
return ERR_PTR(ret)
or
dev_err_probe(dev, PTR_ERR(ptr), ...);
return ERR_CAST(ptr)
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
include/linux/dev_printk.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/include/linux/dev_printk.h b/include/linux/dev_printk.h
index ae80a303c216..b9623ec22350 100644
--- a/include/linux/dev_printk.h
+++ b/include/linux/dev_printk.h
@@ -277,4 +277,13 @@ 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_err_ptr_probe(dev, ___err, fmt, ...) ({ \
+ ERR_PTR(dev_err_probe(dev, ___err, fmt, ##__VA_ARGS__)); \
+})
+
+/* Simple helper for dev_err_probe() when ERR_CAST() is to be returned. */
+#define dev_err_cast_probe(dev, ___err_ptr, fmt, ...) ({ \
+ ERR_PTR(dev_err_probe(dev, PTR_ERR(___err_ptr), fmt, ##__VA_ARGS__)); \
+})
#endif /* _DEVICE_PRINTK_H_ */
--
2.44.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 2/4] iio: temperature: ltc2983: convert to dev_err_probe()
2024-04-23 15:20 [PATCH v2 0/4] dev_printk: add dev_errp_probe() helper Nuno Sa via B4 Relay
2024-04-23 15:20 ` [PATCH v2 1/4] dev_printk: add new dev_err_probe() helpers Nuno Sa via B4 Relay
@ 2024-04-23 15:20 ` Nuno Sa via B4 Relay
2024-04-23 15:42 ` Andy Shevchenko
2024-04-23 15:20 ` [PATCH v2 3/4] iio: backend: make use of dev_err_cast_probe() Nuno Sa via B4 Relay
2024-04-23 15:20 ` [PATCH v2 4/4] iio: common: scmi_iio: convert to dev_err_probe() Nuno Sa via B4 Relay
3 siblings, 1 reply; 14+ messages in thread
From: Nuno Sa via B4 Relay @ 2024-04-23 15:20 UTC (permalink / raw)
To: Petr Mladek, Andy Shevchenko, Chris Down, John Ogness,
Greg Kroah-Hartman, Jonathan Cameron, Lars-Peter Clausen,
Olivier Moysan, Andi Shyti, Jyoti Bhayana
Cc: linux-kernel, linux-iio, Nuno Sa
From: Nuno Sa <nuno.sa@analog.com>
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 <nuno.sa@analog.com>
---
drivers/iio/temperature/ltc2983.c | 282 +++++++++++++++++---------------------
1 file changed, 127 insertions(+), 155 deletions(-)
diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c
index 24d19f3c7292..941b3ab5c927 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>
@@ -432,10 +434,9 @@ __ltc2983_custom_sensor_new(struct ltc2983_data *st, const struct fwnode_handle
else
n_entries = fwnode_property_count_u64(fn, propname);
/* n_entries must be an even number */
- if (!n_entries || (n_entries % 2) != 0) {
- dev_err(dev, "Number of entries either 0 or not even\n");
- return ERR_PTR(-EINVAL);
- }
+ if (!n_entries || (n_entries % 2) != 0)
+ return dev_err_ptr_probe(dev, -EINVAL,
+ "Number of entries either 0 or not even\n");
new_custom = devm_kzalloc(dev, sizeof(*new_custom), GFP_KERNEL);
if (!new_custom)
@@ -443,19 +444,17 @@ __ltc2983_custom_sensor_new(struct ltc2983_data *st, const struct fwnode_handle
new_custom->size = n_entries * n_size;
/* check Steinhart size */
- if (is_steinhart && new_custom->size != LTC2983_CUSTOM_STEINHART_SIZE) {
- dev_err(dev, "Steinhart sensors size(%zu) must be %u\n", new_custom->size,
- LTC2983_CUSTOM_STEINHART_SIZE);
- return ERR_PTR(-EINVAL);
- }
+ if (is_steinhart && new_custom->size != LTC2983_CUSTOM_STEINHART_SIZE)
+ return dev_err_ptr_probe(dev, -EINVAL,
+ "Steinhart sensors size(%zu) must be %u\n",
+ new_custom->size, LTC2983_CUSTOM_STEINHART_SIZE);
+
/* Check space on the table. */
if (st->custom_table_size + new_custom->size >
- (LTC2983_CUST_SENS_TBL_END_REG -
- LTC2983_CUST_SENS_TBL_START_REG) + 1) {
- dev_err(dev, "No space left(%d) for new custom sensor(%zu)",
- st->custom_table_size, new_custom->size);
- return ERR_PTR(-EINVAL);
- }
+ (LTC2983_CUST_SENS_TBL_END_REG - LTC2983_CUST_SENS_TBL_START_REG) + 1)
+ return dev_err_ptr_probe(dev, -EINVAL,
+ "No space left(%d) for new custom sensor(%zu)",
+ st->custom_table_size, new_custom->size);
/* allocate the table */
if (is_steinhart)
@@ -658,10 +657,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);
@@ -688,21 +688,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_err_ptr_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_err_ptr_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);
@@ -710,14 +708,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_err_ptr_probe(dev, ret,
+ "Property reg must be given\n");
}
/* check custom sensor */
@@ -753,16 +750,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_err_cast_probe(dev, 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_err_ptr_probe(dev, ret,
+ "Property reg must be given\n");
ret = fwnode_property_read_u32(child, "adi,number-of-wires", &n_wires);
if (!ret) {
@@ -781,19 +776,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_err_ptr_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_err_ptr_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);
@@ -816,29 +811,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_err_ptr_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_err_ptr_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_err_ptr_probe(dev, -EINVAL,
+ "Invalid chann:%d for RTD",
+ sensor->chan);
}
/* check custom sensor */
@@ -886,10 +874,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_err_ptr_probe(dev, -EINVAL,
+ "Invalid value for excitation current(%u)",
+ excitation_current);
}
}
@@ -913,16 +900,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_err_cast_probe(dev, 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_err_ptr_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);
@@ -937,12 +922,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_err_ptr_probe(dev, -EINVAL,
+ "Invalid chann:%d for differential thermistor",
+ sensor->chan);
/* check custom sensor */
if (sensor->type >= LTC2983_SENSOR_THERMISTOR_STEINHART) {
@@ -981,12 +964,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_err_ptr_probe(dev, -EINVAL,
+ "Auto Range not allowed for custom sensors\n");
+
thermistor->excitation_current = 0x0c;
break;
case 250:
@@ -1023,10 +1004,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_err_ptr_probe(dev, -EINVAL,
+ "Invalid value for excitation current(%u)",
+ excitation_current);
}
}
@@ -1037,11 +1017,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);
@@ -1056,12 +1037,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_err_ptr_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;
@@ -1083,10 +1063,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_err_ptr_probe(dev, -EINVAL,
+ "Invalid value for excitation current(%u)",
+ excitation_current);
}
}
@@ -1102,26 +1081,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_err_ptr_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_err_ptr_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
@@ -1140,21 +1119,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_err_ptr_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;
@@ -1166,21 +1145,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_err_ptr_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);
@@ -1330,10 +1308,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);
@@ -1420,6 +1397,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;
@@ -1438,19 +1416,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;
}
@@ -1458,16 +1433,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,
@@ -1567,12 +1541,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;
@@ -1583,10 +1558,9 @@ static int ltc2983_probe(struct spi_device *spi)
return -ENODEV;
st->regmap = devm_regmap_init_spi(spi, <c2983_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);
@@ -1602,7 +1576,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);
@@ -1612,7 +1586,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)
@@ -1622,12 +1596,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,
@@ -1644,7 +1616,7 @@ static int ltc2983_probe(struct spi_device *spi)
indio_dev->modes = INDIO_DIRECT_MODE;
indio_dev->info = <c2983_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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 3/4] iio: backend: make use of dev_err_cast_probe()
2024-04-23 15:20 [PATCH v2 0/4] dev_printk: add dev_errp_probe() helper Nuno Sa via B4 Relay
2024-04-23 15:20 ` [PATCH v2 1/4] dev_printk: add new dev_err_probe() helpers Nuno Sa via B4 Relay
2024-04-23 15:20 ` [PATCH v2 2/4] iio: temperature: ltc2983: convert to dev_err_probe() Nuno Sa via B4 Relay
@ 2024-04-23 15:20 ` Nuno Sa via B4 Relay
2024-04-23 15:20 ` [PATCH v2 4/4] iio: common: scmi_iio: convert to dev_err_probe() Nuno Sa via B4 Relay
3 siblings, 0 replies; 14+ messages in thread
From: Nuno Sa via B4 Relay @ 2024-04-23 15:20 UTC (permalink / raw)
To: Petr Mladek, Andy Shevchenko, Chris Down, John Ogness,
Greg Kroah-Hartman, Jonathan Cameron, Lars-Peter Clausen,
Olivier Moysan, Andi Shyti, Jyoti Bhayana
Cc: linux-kernel, linux-iio, Nuno Sa
From: Nuno Sa <nuno.sa@analog.com>
Using dev_err_cast_probe() to simplify the code.
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
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 f08ed6d70ae5..2cc6459246ce 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -475,11 +475,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_err_cast_probe(dev, fwnode,
+ "Cannot get Firmware reference\n");
guard(mutex)(&iio_back_lock);
list_for_each_entry(back, &iio_back_list, entry) {
--
2.44.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 4/4] iio: common: scmi_iio: convert to dev_err_probe()
2024-04-23 15:20 [PATCH v2 0/4] dev_printk: add dev_errp_probe() helper Nuno Sa via B4 Relay
` (2 preceding siblings ...)
2024-04-23 15:20 ` [PATCH v2 3/4] iio: backend: make use of dev_err_cast_probe() Nuno Sa via B4 Relay
@ 2024-04-23 15:20 ` Nuno Sa via B4 Relay
2024-04-23 15:50 ` Andy Shevchenko
3 siblings, 1 reply; 14+ messages in thread
From: Nuno Sa via B4 Relay @ 2024-04-23 15:20 UTC (permalink / raw)
To: Petr Mladek, Andy Shevchenko, Chris Down, John Ogness,
Greg Kroah-Hartman, Jonathan Cameron, Lars-Peter Clausen,
Olivier Moysan, Andi Shyti, Jyoti Bhayana
Cc: linux-kernel, linux-iio, Nuno Sa
From: Nuno Sa <nuno.sa@analog.com>
Make use of dev_err_probe() and dev_err_ptr_probe() to simplify error paths
during probe.
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
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..841eda79a8cc 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_err_ptr_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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/4] dev_printk: add new dev_err_probe() helpers
2024-04-23 15:20 ` [PATCH v2 1/4] dev_printk: add new dev_err_probe() helpers Nuno Sa via B4 Relay
@ 2024-04-23 15:31 ` Andy Shevchenko
2024-04-23 15:45 ` Andy Shevchenko
2024-05-02 11:34 ` Nuno Sá
0 siblings, 2 replies; 14+ messages in thread
From: Andy Shevchenko @ 2024-04-23 15:31 UTC (permalink / raw)
To: nuno.sa
Cc: Petr Mladek, Chris Down, John Ogness, Greg Kroah-Hartman,
Jonathan Cameron, Lars-Peter Clausen, Olivier Moysan, Andi Shyti,
Jyoti Bhayana, linux-kernel, linux-iio
On Tue, Apr 23, 2024 at 05:20:30PM +0200, Nuno Sa via B4 Relay wrote:
> From: Nuno Sa <nuno.sa@analog.com>
>
> This is similar to dev_err_probe() but for cases where an ERR_PTR() or
> ERR_CAST() is to be returned simplifying patterns like:
>
> dev_err_probe(dev, ret, ...);
> return ERR_PTR(ret)
> or
> dev_err_probe(dev, PTR_ERR(ptr), ...);
> return ERR_CAST(ptr)
...
> +/* Simple helper for dev_err_probe() when ERR_PTR() is to be returned. */
> +#define dev_err_ptr_probe(dev, ___err, fmt, ...) ({ \
> + ERR_PTR(dev_err_probe(dev, ___err, fmt, ##__VA_ARGS__)); \
> +})
Why ; and hence why ({}) ?
I even believe the compiler may warn if you have double ;; in some cases.
...
> +#define dev_err_cast_probe(dev, ___err_ptr, fmt, ...) ({ \
> + ERR_PTR(dev_err_probe(dev, PTR_ERR(___err_ptr), fmt, ##__VA_ARGS__)); \
> +})
Ditto.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/4] iio: temperature: ltc2983: convert to dev_err_probe()
2024-04-23 15:20 ` [PATCH v2 2/4] iio: temperature: ltc2983: convert to dev_err_probe() Nuno Sa via B4 Relay
@ 2024-04-23 15:42 ` Andy Shevchenko
2024-05-02 11:41 ` Nuno Sá
0 siblings, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2024-04-23 15:42 UTC (permalink / raw)
To: nuno.sa
Cc: Petr Mladek, Chris Down, John Ogness, Greg Kroah-Hartman,
Jonathan Cameron, Lars-Peter Clausen, Olivier Moysan, Andi Shyti,
Jyoti Bhayana, linux-kernel, linux-iio
On Tue, Apr 23, 2024 at 05:20:31PM +0200, Nuno Sa via B4 Relay wrote:
> From: Nuno Sa <nuno.sa@analog.com>
>
> 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.
...
> /* Check space on the table. */
> if (st->custom_table_size + new_custom->size >
> - (LTC2983_CUST_SENS_TBL_END_REG -
> - LTC2983_CUST_SENS_TBL_START_REG) + 1) {
> + (LTC2983_CUST_SENS_TBL_END_REG - LTC2983_CUST_SENS_TBL_START_REG) + 1)
Semi-unrelated change?
...
> + return dev_err_ptr_probe(dev, -EINVAL,
> + "Invalid chann:%d for differential thermocouple",
While at it, add missing \n.
> + sensor->chan);
...
> + return dev_err_cast_probe(dev, ref,
> + "Property adi,rsense-handle missing or invalid");
Ditto.
...
> + return dev_err_ptr_probe(dev, -EINVAL,
> + "Invalid number of wires:%u\n",
> + n_wires);
Can be compressed in terms of LoCs?
...
> + return dev_err_ptr_probe(dev, -EINVAL,
> + "Rotation not allowed for 2/3 Wire RTDs");
\n
...
> + return dev_err_ptr_probe(dev, -EINVAL,
> + "Invalid rsense chann:%d to use in kelvin rsense",
> + rtd->r_sense_chan);
Ditto.
...
> + return dev_err_ptr_probe(dev, -EINVAL,
> + "Invalid chann:%d for the rtd config",
Ditto.
> + sensor->chan);
...
> + return dev_err_ptr_probe(dev, -EINVAL,
> + "Invalid chann:%d for RTD",
Ditto.
> + sensor->chan);
...
> + return dev_err_ptr_probe(dev, -EINVAL,
> + "Invalid value for excitation current(%u)",
Ditto.
> + excitation_current);
...
> + if (IS_ERR(ref))
> + return dev_err_cast_probe(dev, ref,
> + "Property adi,rsense-handle missing or invalid");
Ditto.
...
> + return dev_err_ptr_probe(dev, -EINVAL,
> + "Invalid chann:%d for differential thermistor",
> + sensor->chan);
Ditto.
...
> + return dev_err_ptr_probe(dev, -EINVAL,
> + "Invalid value for excitation current(%u)",
> + excitation_current);
Ditto.
...
> + return dev_err_ptr_probe(dev, -EINVAL,
> + "Invalid chann:%d for differential thermistor",
> + sensor->chan);
Ditto.
...
> + return dev_err_ptr_probe(dev, -EINVAL,
> + "Invalid value for excitation current(%u)",
> + excitation_current);
Ditto.
...
> + return dev_err_ptr_probe(dev, -EINVAL,
> + "Invalid chann:%d for r_sense",
> + sensor->chan);
Ditto.
...
> + if (!st->num_channels)
> + return dev_err_probe(dev, -EINVAL,
> + "At least one channel must be given!");
Ditto.
...
> + return dev_err_probe(dev, -EINVAL,
> + "EEPROM command failed: 0x%02X\n", val);
One line?
...
> + if (IS_ERR(st->regmap))
> + return dev_err_probe(dev, PTR_ERR(st->regmap),
> + "Failed to initialize regmap\n");
Wondering about Andi's proposal in conjunction with %pe to be in use
return dev_???(dev, st->regmap, "Failed to initialize regmap\n");
where it returns an int and uses const void * as an error pointer for %pe.
> - st->iio_chan = devm_kzalloc(&spi->dev,
> + st->iio_chan = devm_kzalloc(dev,
> st->iio_channels * sizeof(*st->iio_chan),
> GFP_KERNEL);
Separate change to devm_kzalloc() before this patch?
In that patch you may also introduce a temporary struct device *dev.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/4] dev_printk: add new dev_err_probe() helpers
2024-04-23 15:31 ` Andy Shevchenko
@ 2024-04-23 15:45 ` Andy Shevchenko
2024-05-02 11:54 ` Nuno Sá
2024-05-02 11:34 ` Nuno Sá
1 sibling, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2024-04-23 15:45 UTC (permalink / raw)
To: nuno.sa
Cc: Petr Mladek, Chris Down, John Ogness, Greg Kroah-Hartman,
Jonathan Cameron, Lars-Peter Clausen, Olivier Moysan, Andi Shyti,
Jyoti Bhayana, linux-kernel, linux-iio
On Tue, Apr 23, 2024 at 06:31:20PM +0300, Andy Shevchenko wrote:
> On Tue, Apr 23, 2024 at 05:20:30PM +0200, Nuno Sa via B4 Relay wrote:
> > From: Nuno Sa <nuno.sa@analog.com>
...
> > +#define dev_err_cast_probe(dev, ___err_ptr, fmt, ...) ({ \
> > + ERR_PTR(dev_err_probe(dev, PTR_ERR(___err_ptr), fmt, ##__VA_ARGS__)); \
> > +})
After looking into the next patch I think this should be rewritten to use %pe,
hence should be an exported function. Or dev_err_probe() should be split to
a version that makes the difference between int and const void * (maybe using
_Generic()).
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 4/4] iio: common: scmi_iio: convert to dev_err_probe()
2024-04-23 15:20 ` [PATCH v2 4/4] iio: common: scmi_iio: convert to dev_err_probe() Nuno Sa via B4 Relay
@ 2024-04-23 15:50 ` Andy Shevchenko
2024-05-02 11:33 ` Nuno Sá
0 siblings, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2024-04-23 15:50 UTC (permalink / raw)
To: nuno.sa
Cc: Petr Mladek, Chris Down, John Ogness, Greg Kroah-Hartman,
Jonathan Cameron, Lars-Peter Clausen, Olivier Moysan, Andi Shyti,
Jyoti Bhayana, linux-kernel, linux-iio
On Tue, Apr 23, 2024 at 05:20:33PM +0200, Nuno Sa via B4 Relay wrote:
> From: Nuno Sa <nuno.sa@analog.com>
>
> Make use of dev_err_probe() and dev_err_ptr_probe() to simplify error paths
> during probe.
...
> + return dev_err_ptr_probe(&iiodev->dev, ret,
> + "Error in registering sensor update notifier for sensor %s err %d",
\n
> + sensor->sensor_info->name, ret);
...
> + return dev_err_probe(dev, -EINVAL,
> + "SCMI sensor %d has missing info\n", i);
One line? (It's 99 if you use relaxed limit).
...
> + 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));
Please, be sure you remove double error code printing, dev_err_probe() does it
for you already. (This applies to all places like this, if any, in the entire
series.)
...
> + return dev_err_probe(dev, err,
> + "IIO buffer setup error at sensor %s: %d\n",
> + sensor_info->name, err);
Ditto.
...
> + return dev_err_probe(dev, err,
> + "IIO device registration failed at sensor %s: %d\n",
> + sensor_info->name, err);
Ditto.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 4/4] iio: common: scmi_iio: convert to dev_err_probe()
2024-04-23 15:50 ` Andy Shevchenko
@ 2024-05-02 11:33 ` Nuno Sá
0 siblings, 0 replies; 14+ messages in thread
From: Nuno Sá @ 2024-05-02 11:33 UTC (permalink / raw)
To: Andy Shevchenko, nuno.sa
Cc: Petr Mladek, Chris Down, John Ogness, Greg Kroah-Hartman,
Jonathan Cameron, Lars-Peter Clausen, Olivier Moysan, Andi Shyti,
Jyoti Bhayana, linux-kernel, linux-iio
On Tue, 2024-04-23 at 18:50 +0300, Andy Shevchenko wrote:
> On Tue, Apr 23, 2024 at 05:20:33PM +0200, Nuno Sa via B4 Relay wrote:
> > From: Nuno Sa <nuno.sa@analog.com>
> >
> > Make use of dev_err_probe() and dev_err_ptr_probe() to simplify error paths
> > during probe.
>
> ...
>
> > + return dev_err_ptr_probe(&iiodev->dev, ret,
> > + "Error in registering sensor update
> > notifier for sensor %s err %d",
>
> \n
sure...
>
> > + sensor->sensor_info->name, ret);
>
> ...
>
> > + return dev_err_probe(dev, -EINVAL,
> > + "SCMI sensor %d has missing
> > info\n", i);
>
> One line? (It's 99 if you use relaxed limit).
Being this IIO, Jonathan prefers to stick the old limit unless readability is hurt...
>
> ...
>
> > + 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));
>
> Please, be sure you remove double error code printing, dev_err_probe() does it
> for you already. (This applies to all places like this, if any, in the entire
> series.)
Up... blind conversion. Thanks!
- Nuno Sá
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/4] dev_printk: add new dev_err_probe() helpers
2024-04-23 15:31 ` Andy Shevchenko
2024-04-23 15:45 ` Andy Shevchenko
@ 2024-05-02 11:34 ` Nuno Sá
1 sibling, 0 replies; 14+ messages in thread
From: Nuno Sá @ 2024-05-02 11:34 UTC (permalink / raw)
To: Andy Shevchenko, nuno.sa
Cc: Petr Mladek, Chris Down, John Ogness, Greg Kroah-Hartman,
Jonathan Cameron, Lars-Peter Clausen, Olivier Moysan, Andi Shyti,
Jyoti Bhayana, linux-kernel, linux-iio
On Tue, 2024-04-23 at 18:31 +0300, Andy Shevchenko wrote:
> On Tue, Apr 23, 2024 at 05:20:30PM +0200, Nuno Sa via B4 Relay wrote:
> > From: Nuno Sa <nuno.sa@analog.com>
> >
> > This is similar to dev_err_probe() but for cases where an ERR_PTR() or
> > ERR_CAST() is to be returned simplifying patterns like:
> >
> > dev_err_probe(dev, ret, ...);
> > return ERR_PTR(ret)
> > or
> > dev_err_probe(dev, PTR_ERR(ptr), ...);
> > return ERR_CAST(ptr)
>
> ...
>
> > +/* Simple helper for dev_err_probe() when ERR_PTR() is to be returned. */
> > +#define dev_err_ptr_probe(dev, ___err, fmt, ...) ({ \
> > + ERR_PTR(dev_err_probe(dev, ___err, fmt, ##__VA_ARGS__)); \
> > +})
>
> Why ; and hence why ({}) ?
>
> I even believe the compiler may warn if you have double ;; in some cases.
>
Oh yes, no need for any of those...
- Nuno Sá
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/4] iio: temperature: ltc2983: convert to dev_err_probe()
2024-04-23 15:42 ` Andy Shevchenko
@ 2024-05-02 11:41 ` Nuno Sá
0 siblings, 0 replies; 14+ messages in thread
From: Nuno Sá @ 2024-05-02 11:41 UTC (permalink / raw)
To: Andy Shevchenko, nuno.sa
Cc: Petr Mladek, Chris Down, John Ogness, Greg Kroah-Hartman,
Jonathan Cameron, Lars-Peter Clausen, Olivier Moysan, Andi Shyti,
Jyoti Bhayana, linux-kernel, linux-iio
On Tue, 2024-04-23 at 18:42 +0300, Andy Shevchenko wrote:
> On Tue, Apr 23, 2024 at 05:20:31PM +0200, Nuno Sa via B4 Relay wrote:
> > From: Nuno Sa <nuno.sa@analog.com>
> >
> > 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.
>
> ...
>
> > /* Check space on the table. */
> > if (st->custom_table_size + new_custom->size >
> > - (LTC2983_CUST_SENS_TBL_END_REG -
> > - LTC2983_CUST_SENS_TBL_START_REG) + 1) {
>
> > + (LTC2983_CUST_SENS_TBL_END_REG - LTC2983_CUST_SENS_TBL_START_REG) +
> > 1)
>
> Semi-unrelated change?
Yeah, indeed. One of those cases where the old limit does hurt readability (IMO)
>
> ...
>
> > + return dev_err_ptr_probe(dev, -EINVAL,
> > + "Invalid chann:%d for differential
> > thermocouple",
>
> While at it, add missing \n.
Will do for all the places...
>
> > + sensor->chan);
>
> ...
>
> > + return dev_err_cast_probe(dev, ref,
> > + "Property adi,rsense-handle missing or
> > invalid");
>
> Ditto.
>
> ...
>
> > + return dev_err_ptr_probe(dev, -EINVAL,
> > + "Invalid number of wires:%u\n",
> > + n_wires);
>
> Can be compressed in terms of LoCs?
>
> ...
>
> > + return dev_err_ptr_probe(dev, -EINVAL,
> > + "Rotation not allowed for
> > 2/3 Wire RTDs");
>
> \n
>
> ...
>
> > + return dev_err_ptr_probe(dev, -EINVAL,
> > + "Invalid rsense chann:%d to use in
> > kelvin rsense",
> > + rtd->r_sense_chan);
>
> Ditto.
>
> ...
>
> > + return dev_err_ptr_probe(dev, -EINVAL,
> > + "Invalid chann:%d for the rtd
> > config",
>
> Ditto.
>
> > + sensor->chan);
>
> ...
>
> > + return dev_err_ptr_probe(dev, -EINVAL,
> > + "Invalid chann:%d for RTD",
>
> Ditto.
>
> > + sensor->chan);
>
> ...
>
> > + return dev_err_ptr_probe(dev, -EINVAL,
> > + "Invalid value for excitation
> > current(%u)",
>
> Ditto.
>
> > + excitation_current);
>
> ...
>
> > + if (IS_ERR(ref))
> > + return dev_err_cast_probe(dev, ref,
> > + "Property adi,rsense-handle missing or
> > invalid");
>
> Ditto.
>
> ...
>
> > + return dev_err_ptr_probe(dev, -EINVAL,
> > + "Invalid chann:%d for differential
> > thermistor",
> > + sensor->chan);
>
>
> Ditto.
>
> ...
>
> > + return dev_err_ptr_probe(dev, -EINVAL,
> > + "Invalid value for excitation
> > current(%u)",
> > + excitation_current);
>
> Ditto.
>
> ...
>
> > + return dev_err_ptr_probe(dev, -EINVAL,
> > + "Invalid chann:%d for differential
> > thermistor",
> > + sensor->chan);
>
> Ditto.
>
> ...
>
> > + return dev_err_ptr_probe(dev, -EINVAL,
> > + "Invalid value for excitation
> > current(%u)",
> > + excitation_current);
>
> Ditto.
>
> ...
>
> > + return dev_err_ptr_probe(dev, -EINVAL,
> > + "Invalid chann:%d for r_sense",
> > + sensor->chan);
>
> Ditto.
>
> ...
>
> > + if (!st->num_channels)
> > + return dev_err_probe(dev, -EINVAL,
> > + "At least one channel must be given!");
>
> Ditto.
>
> ...
>
> > + return dev_err_probe(dev, -EINVAL,
> > + "EEPROM command failed: 0x%02X\n", val);
>
> One line?
>
> ...
>
> > + if (IS_ERR(st->regmap))
> > + return dev_err_probe(dev, PTR_ERR(st->regmap),
> > + "Failed to initialize regmap\n");
>
> Wondering about Andi's proposal in conjunction with %pe to be in use
>
> return dev_???(dev, st->regmap, "Failed to initialize regmap\n");
>
> where it returns an int and uses const void * as an error pointer for %pe.
Yeah, I would like to avoid including that variation in this series (unless everyone
agrees and requires it now). We already have tons of cases where we do
dev_err_probe(dev, PTR_ERR(), ...). Do we want to change all of them or not having
more? Personally, I'm not seeing as a big deal to have to do the PTR_ERR(). Yes,
internally we will go back to ERR_PTR() but still...
>
>
>
> > - st->iio_chan = devm_kzalloc(&spi->dev,
> > + st->iio_chan = devm_kzalloc(dev,
> > st->iio_channels * sizeof(*st->iio_chan),
> > GFP_KERNEL);
>
> Separate change to devm_kzalloc() before this patch?
> In that patch you may also introduce a temporary struct device *dev.
>
If the introduction of the temporary struct device *dev is too much to be included in
here I may just remove it and send a patch afterwards.. (note I'm adding more
temporary *dev in other places to be consistent throughout the driver.
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/4] dev_printk: add new dev_err_probe() helpers
2024-04-23 15:45 ` Andy Shevchenko
@ 2024-05-02 11:54 ` Nuno Sá
2024-05-02 15:37 ` Andy Shevchenko
0 siblings, 1 reply; 14+ messages in thread
From: Nuno Sá @ 2024-05-02 11:54 UTC (permalink / raw)
To: Andy Shevchenko, nuno.sa
Cc: Petr Mladek, Chris Down, John Ogness, Greg Kroah-Hartman,
Jonathan Cameron, Lars-Peter Clausen, Olivier Moysan, Andi Shyti,
Jyoti Bhayana, linux-kernel, linux-iio
On Tue, 2024-04-23 at 18:45 +0300, Andy Shevchenko wrote:
> On Tue, Apr 23, 2024 at 06:31:20PM +0300, Andy Shevchenko wrote:
> > On Tue, Apr 23, 2024 at 05:20:30PM +0200, Nuno Sa via B4 Relay wrote:
> > > From: Nuno Sa <nuno.sa@analog.com>
>
> ...
>
> > > +#define dev_err_cast_probe(dev, ___err_ptr, fmt,
> > > ...) ({ \
> > > + ERR_PTR(dev_err_probe(dev, PTR_ERR(___err_ptr), fmt,
> > > ##__VA_ARGS__)); \
> > > +})
>
> After looking into the next patch I think this should be rewritten to use %pe,
> hence should be an exported function. Or dev_err_probe() should be split to
> a version that makes the difference between int and const void * (maybe using
> _Generic()).
>
I replied a bit in the other patch but I'm of the opinion that's likely just more
complicated than it needs to be (IMO). Why is the PTR_ERR(___err_ptr) that bad? If we
really want to have a version that takes pointer why not just:
#define dev_err_ptr_probe(dev, ___err, fmt, ...) \
dev_err_probe(dev, PTR_ERR(__err), fmt, ##__VA_ARGS__)
(yes, while _Generic() could be fun I'm trying to avoid it. In this case, I think
having explicit defines is more helpful)
- Nuno Sá
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/4] dev_printk: add new dev_err_probe() helpers
2024-05-02 11:54 ` Nuno Sá
@ 2024-05-02 15:37 ` Andy Shevchenko
0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2024-05-02 15:37 UTC (permalink / raw)
To: Nuno Sá
Cc: nuno.sa, Petr Mladek, Chris Down, John Ogness,
Greg Kroah-Hartman, Jonathan Cameron, Lars-Peter Clausen,
Olivier Moysan, Andi Shyti, Jyoti Bhayana, linux-kernel,
linux-iio
On Thu, May 02, 2024 at 01:54:36PM +0200, Nuno Sá wrote:
> On Tue, 2024-04-23 at 18:45 +0300, Andy Shevchenko wrote:
> > On Tue, Apr 23, 2024 at 06:31:20PM +0300, Andy Shevchenko wrote:
> > > On Tue, Apr 23, 2024 at 05:20:30PM +0200, Nuno Sa via B4 Relay wrote:
> > > > From: Nuno Sa <nuno.sa@analog.com>
...
> > > > +#define dev_err_cast_probe(dev, ___err_ptr, fmt,
> > > > ...) ({ \
> > > > + ERR_PTR(dev_err_probe(dev, PTR_ERR(___err_ptr), fmt,
> > > > ##__VA_ARGS__)); \
> > > > +})
> >
> > After looking into the next patch I think this should be rewritten to use %pe,
> > hence should be an exported function. Or dev_err_probe() should be split to
> > a version that makes the difference between int and const void * (maybe using
> > _Generic()).
>
> I replied a bit in the other patch but I'm of the opinion that's likely just more
> complicated than it needs to be (IMO). Why is the PTR_ERR(___err_ptr) that bad? If we
> really want to have a version that takes pointer why not just:
>
> #define dev_err_ptr_probe(dev, ___err, fmt, ...) \
> dev_err_probe(dev, PTR_ERR(__err), fmt, ##__VA_ARGS__)
>
>
> (yes, while _Generic() could be fun I'm trying to avoid it. In this case, I think
> having explicit defines is more helpful)
It seems dev_err_probe() already uses %pe, so we are fine.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2024-05-02 15:37 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-23 15:20 [PATCH v2 0/4] dev_printk: add dev_errp_probe() helper Nuno Sa via B4 Relay
2024-04-23 15:20 ` [PATCH v2 1/4] dev_printk: add new dev_err_probe() helpers Nuno Sa via B4 Relay
2024-04-23 15:31 ` Andy Shevchenko
2024-04-23 15:45 ` Andy Shevchenko
2024-05-02 11:54 ` Nuno Sá
2024-05-02 15:37 ` Andy Shevchenko
2024-05-02 11:34 ` Nuno Sá
2024-04-23 15:20 ` [PATCH v2 2/4] iio: temperature: ltc2983: convert to dev_err_probe() Nuno Sa via B4 Relay
2024-04-23 15:42 ` Andy Shevchenko
2024-05-02 11:41 ` Nuno Sá
2024-04-23 15:20 ` [PATCH v2 3/4] iio: backend: make use of dev_err_cast_probe() Nuno Sa via B4 Relay
2024-04-23 15:20 ` [PATCH v2 4/4] iio: common: scmi_iio: convert to dev_err_probe() Nuno Sa via B4 Relay
2024-04-23 15:50 ` Andy Shevchenko
2024-05-02 11:33 ` Nuno Sá
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).