* [PATCH 0/6] iio: adc: simplify with cleanup.h
@ 2024-07-05 10:40 Krzysztof Kozlowski
2024-07-05 10:40 ` [PATCH 1/6] iio: accel: bma400: " Krzysztof Kozlowski
` (6 more replies)
0 siblings, 7 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2024-07-05 10:40 UTC (permalink / raw)
To: Dan Robertson, Jonathan Cameron, Lars-Peter Clausen,
Michael Hennerich, Nicolas Ferre, Alexandre Belloni,
Claudiu Beznea, Oleksij Rempel, kernel
Cc: linux-iio, linux-kernel, linux-arm-kernel, Krzysztof Kozlowski
Allocate the memory with scoped/cleanup.h to reduce error handling and
make the code a bit simpler.
Best regards,
Krzysztof
---
Krzysztof Kozlowski (6):
iio: accel: bma400: simplify with cleanup.h
iio: adc: ad7280a: simplify with cleanup.h
iio: adc: at91: simplify with cleanup.h
iio: adc: max1363: simplify with cleanup.h
iio: adc: ti-tsc2046: simplify with cleanup.h
iio: adc: ad5755: drop redundant devm_kfree()
drivers/iio/accel/bma400_core.c | 11 +++++------
drivers/iio/adc/ad7280a.c | 10 ++++------
drivers/iio/adc/at91_adc.c | 13 +++++--------
drivers/iio/adc/max1363.c | 34 +++++++++++++---------------------
drivers/iio/adc/ti-tsc2046.c | 29 ++++++++++++-----------------
drivers/iio/dac/ad5755.c | 1 -
6 files changed, 39 insertions(+), 59 deletions(-)
---
base-commit: 0b58e108042b0ed28a71cd7edf5175999955b233
change-id: 20240705-cleanup-h-iio-c90ca38865a4
Best regards,
--
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/6] iio: accel: bma400: simplify with cleanup.h
2024-07-05 10:40 [PATCH 0/6] iio: adc: simplify with cleanup.h Krzysztof Kozlowski
@ 2024-07-05 10:40 ` Krzysztof Kozlowski
2024-07-05 10:40 ` [PATCH 2/6] iio: adc: ad7280a: " Krzysztof Kozlowski
` (5 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2024-07-05 10:40 UTC (permalink / raw)
To: Dan Robertson, Jonathan Cameron, Lars-Peter Clausen,
Michael Hennerich, Nicolas Ferre, Alexandre Belloni,
Claudiu Beznea, Oleksij Rempel, kernel
Cc: linux-iio, linux-kernel, linux-arm-kernel, Krzysztof Kozlowski
Allocate the memory with scoped/cleanup.h to reduce error handling and
make the code a bit simpler.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
drivers/iio/accel/bma400_core.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
index e90e2f01550a..89db242f06e0 100644
--- a/drivers/iio/accel/bma400_core.c
+++ b/drivers/iio/accel/bma400_core.c
@@ -13,6 +13,7 @@
#include <linux/bitfield.h>
#include <linux/bitops.h>
+#include <linux/cleanup.h>
#include <linux/device.h>
#include <linux/kernel.h>
#include <linux/module.h>
@@ -795,21 +796,19 @@ static int bma400_enable_steps(struct bma400_data *data, int val)
static int bma400_get_steps_reg(struct bma400_data *data, int *val)
{
- u8 *steps_raw;
int ret;
- steps_raw = kmalloc(BMA400_STEP_RAW_LEN, GFP_KERNEL);
+ u8 *steps_raw __free(kfree) = kmalloc(BMA400_STEP_RAW_LEN, GFP_KERNEL);
if (!steps_raw)
return -ENOMEM;
ret = regmap_bulk_read(data->regmap, BMA400_STEP_CNT0_REG,
steps_raw, BMA400_STEP_RAW_LEN);
- if (ret) {
- kfree(steps_raw);
+ if (ret)
return ret;
- }
+
*val = get_unaligned_le24(steps_raw);
- kfree(steps_raw);
+
return IIO_VAL_INT;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/6] iio: adc: ad7280a: simplify with cleanup.h
2024-07-05 10:40 [PATCH 0/6] iio: adc: simplify with cleanup.h Krzysztof Kozlowski
2024-07-05 10:40 ` [PATCH 1/6] iio: accel: bma400: " Krzysztof Kozlowski
@ 2024-07-05 10:40 ` Krzysztof Kozlowski
2024-07-05 11:43 ` Nuno Sá
2024-07-05 10:40 ` [PATCH 3/6] iio: adc: at91: " Krzysztof Kozlowski
` (4 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2024-07-05 10:40 UTC (permalink / raw)
To: Dan Robertson, Jonathan Cameron, Lars-Peter Clausen,
Michael Hennerich, Nicolas Ferre, Alexandre Belloni,
Claudiu Beznea, Oleksij Rempel, kernel
Cc: linux-iio, linux-kernel, linux-arm-kernel, Krzysztof Kozlowski
Allocate the memory with scoped/cleanup.h to reduce error handling and
make the code a bit simpler.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
drivers/iio/adc/ad7280a.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/adc/ad7280a.c b/drivers/iio/adc/ad7280a.c
index d4a4e15c8244..7bfa090da4df 100644
--- a/drivers/iio/adc/ad7280a.c
+++ b/drivers/iio/adc/ad7280a.c
@@ -7,6 +7,7 @@
#include <linux/bitfield.h>
#include <linux/bits.h>
+#include <linux/cleanup.h>
#include <linux/crc8.h>
#include <linux/delay.h>
#include <linux/device.h>
@@ -803,16 +804,16 @@ static irqreturn_t ad7280_event_handler(int irq, void *private)
{
struct iio_dev *indio_dev = private;
struct ad7280_state *st = iio_priv(indio_dev);
- unsigned int *channels;
int i, ret;
- channels = kcalloc(st->scan_cnt, sizeof(*channels), GFP_KERNEL);
+ unsigned int *channels __free(kfree) = kcalloc(st->scan_cnt, sizeof(*channels),
+ GFP_KERNEL);
if (!channels)
return IRQ_HANDLED;
ret = ad7280_read_all_channels(st, st->scan_cnt, channels);
if (ret < 0)
- goto out;
+ return IRQ_HANDLED;
for (i = 0; i < st->scan_cnt; i++) {
unsigned int val;
@@ -852,9 +853,6 @@ static irqreturn_t ad7280_event_handler(int irq, void *private)
}
}
-out:
- kfree(channels);
-
return IRQ_HANDLED;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/6] iio: adc: at91: simplify with cleanup.h
2024-07-05 10:40 [PATCH 0/6] iio: adc: simplify with cleanup.h Krzysztof Kozlowski
2024-07-05 10:40 ` [PATCH 1/6] iio: accel: bma400: " Krzysztof Kozlowski
2024-07-05 10:40 ` [PATCH 2/6] iio: adc: ad7280a: " Krzysztof Kozlowski
@ 2024-07-05 10:40 ` Krzysztof Kozlowski
2024-07-05 10:40 ` [PATCH 4/6] iio: adc: max1363: " Krzysztof Kozlowski
` (3 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2024-07-05 10:40 UTC (permalink / raw)
To: Dan Robertson, Jonathan Cameron, Lars-Peter Clausen,
Michael Hennerich, Nicolas Ferre, Alexandre Belloni,
Claudiu Beznea, Oleksij Rempel, kernel
Cc: linux-iio, linux-kernel, linux-arm-kernel, Krzysztof Kozlowski
Allocate the memory with scoped/cleanup.h to reduce error handling and
make the code a bit simpler.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
drivers/iio/adc/at91_adc.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
index eb501e3c86a5..7bcda203436b 100644
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -7,6 +7,7 @@
#include <linux/bitmap.h>
#include <linux/bitops.h>
+#include <linux/cleanup.h>
#include <linux/clk.h>
#include <linux/err.h>
#include <linux/io.h>
@@ -543,22 +544,18 @@ static int at91_adc_get_trigger_value_by_name(struct iio_dev *idev,
int i;
for (i = 0; i < st->caps->trigger_number; i++) {
- char *name = kasprintf(GFP_KERNEL,
- "%s-dev%d-%s",
- idev->name,
- iio_device_id(idev),
- triggers[i].name);
+ char *name __free(kfree) = kasprintf(GFP_KERNEL, "%s-dev%d-%s",
+ idev->name,
+ iio_device_id(idev),
+ triggers[i].name);
if (!name)
return -ENOMEM;
if (strcmp(trigger_name, name) == 0) {
- kfree(name);
if (triggers[i].value == 0)
return -EINVAL;
return triggers[i].value;
}
-
- kfree(name);
}
return -EINVAL;
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 4/6] iio: adc: max1363: simplify with cleanup.h
2024-07-05 10:40 [PATCH 0/6] iio: adc: simplify with cleanup.h Krzysztof Kozlowski
` (2 preceding siblings ...)
2024-07-05 10:40 ` [PATCH 3/6] iio: adc: at91: " Krzysztof Kozlowski
@ 2024-07-05 10:40 ` Krzysztof Kozlowski
2024-07-05 11:45 ` Nuno Sá
2024-07-05 10:40 ` [PATCH 5/6] iio: adc: ti-tsc2046: " Krzysztof Kozlowski
` (2 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2024-07-05 10:40 UTC (permalink / raw)
To: Dan Robertson, Jonathan Cameron, Lars-Peter Clausen,
Michael Hennerich, Nicolas Ferre, Alexandre Belloni,
Claudiu Beznea, Oleksij Rempel, kernel
Cc: linux-iio, linux-kernel, linux-arm-kernel, Krzysztof Kozlowski
Allocate the memory with scoped/cleanup.h to reduce error handling and
make the code a bit simpler.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
drivers/iio/adc/max1363.c | 34 +++++++++++++---------------------
1 file changed, 13 insertions(+), 21 deletions(-)
diff --git a/drivers/iio/adc/max1363.c b/drivers/iio/adc/max1363.c
index bf4b6dc53fd2..d0c6e94f7204 100644
--- a/drivers/iio/adc/max1363.c
+++ b/drivers/iio/adc/max1363.c
@@ -13,6 +13,7 @@
*/
#include <linux/interrupt.h>
+#include <linux/cleanup.h>
#include <linux/device.h>
#include <linux/kernel.h>
#include <linux/sysfs.h>
@@ -818,7 +819,6 @@ static int max1363_read_event_config(struct iio_dev *indio_dev,
static int max1363_monitor_mode_update(struct max1363_state *st, int enabled)
{
- u8 *tx_buf;
int ret, i = 3, j;
unsigned long numelements;
int len;
@@ -850,11 +850,10 @@ static int max1363_monitor_mode_update(struct max1363_state *st, int enabled)
}
numelements = bitmap_weight(modemask, MAX1363_MAX_CHANNELS);
len = 3 * numelements + 3;
- tx_buf = kmalloc(len, GFP_KERNEL);
- if (!tx_buf) {
- ret = -ENOMEM;
- goto error_ret;
- }
+ u8 *tx_buf __free(kfree) = kmalloc(len, GFP_KERNEL);
+ if (!tx_buf)
+ return -ENOMEM;
+
tx_buf[0] = st->configbyte;
tx_buf[1] = st->setupbyte;
tx_buf[2] = (st->monitor_speed << 1);
@@ -893,11 +892,9 @@ static int max1363_monitor_mode_update(struct max1363_state *st, int enabled)
ret = st->send(st->client, tx_buf, len);
if (ret < 0)
- goto error_ret;
- if (ret != len) {
- ret = -EIO;
- goto error_ret;
- }
+ return ret;
+ if (ret != len)
+ return -EIO;
/*
* Now that we hopefully have sensible thresholds in place it is
@@ -910,18 +907,13 @@ static int max1363_monitor_mode_update(struct max1363_state *st, int enabled)
tx_buf[1] = MAX1363_MON_INT_ENABLE | (st->monitor_speed << 1) | 0xF0;
ret = st->send(st->client, tx_buf, 2);
if (ret < 0)
- goto error_ret;
- if (ret != 2) {
- ret = -EIO;
- goto error_ret;
- }
- ret = 0;
+ return ret;
+ if (ret != 2)
+ return -EIO;
+
st->monitor_on = true;
-error_ret:
- kfree(tx_buf);
-
- return ret;
+ return 0;
}
/*
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 5/6] iio: adc: ti-tsc2046: simplify with cleanup.h
2024-07-05 10:40 [PATCH 0/6] iio: adc: simplify with cleanup.h Krzysztof Kozlowski
` (3 preceding siblings ...)
2024-07-05 10:40 ` [PATCH 4/6] iio: adc: max1363: " Krzysztof Kozlowski
@ 2024-07-05 10:40 ` Krzysztof Kozlowski
2024-07-06 5:08 ` Oleksij Rempel
2024-07-05 10:40 ` [PATCH 6/6] iio: adc: ad5755: drop redundant devm_kfree() Krzysztof Kozlowski
2024-07-06 10:55 ` [PATCH 0/6] iio: adc: simplify with cleanup.h Jonathan Cameron
6 siblings, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2024-07-05 10:40 UTC (permalink / raw)
To: Dan Robertson, Jonathan Cameron, Lars-Peter Clausen,
Michael Hennerich, Nicolas Ferre, Alexandre Belloni,
Claudiu Beznea, Oleksij Rempel, kernel
Cc: linux-iio, linux-kernel, linux-arm-kernel, Krzysztof Kozlowski
Allocate the memory with scoped/cleanup.h to reduce error handling and
make the code a bit simpler.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
drivers/iio/adc/ti-tsc2046.c | 29 ++++++++++++-----------------
1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/drivers/iio/adc/ti-tsc2046.c b/drivers/iio/adc/ti-tsc2046.c
index edcef8f11522..24b1d4390872 100644
--- a/drivers/iio/adc/ti-tsc2046.c
+++ b/drivers/iio/adc/ti-tsc2046.c
@@ -6,6 +6,7 @@
*/
#include <linux/bitfield.h>
+#include <linux/cleanup.h>
#include <linux/delay.h>
#include <linux/module.h>
#include <linux/regulator/consumer.h>
@@ -273,7 +274,6 @@ static int tsc2046_adc_read_one(struct tsc2046_adc_priv *priv, int ch_idx,
u32 *effective_speed_hz)
{
struct tsc2046_adc_ch_cfg *ch = &priv->ch_cfg[ch_idx];
- struct tsc2046_adc_atom *rx_buf, *tx_buf;
unsigned int val, val_normalized = 0;
int ret, i, count_skip = 0, max_count;
struct spi_transfer xfer;
@@ -287,18 +287,20 @@ static int tsc2046_adc_read_one(struct tsc2046_adc_priv *priv, int ch_idx,
max_count = 1;
}
- if (sizeof(*tx_buf) * max_count > PAGE_SIZE)
+ if (sizeof(struct tsc2046_adc_atom) * max_count > PAGE_SIZE)
return -ENOSPC;
- tx_buf = kcalloc(max_count, sizeof(*tx_buf), GFP_KERNEL);
+ struct tsc2046_adc_atom *tx_buf __free(kfree) = kcalloc(max_count,
+ sizeof(*tx_buf),
+ GFP_KERNEL);
if (!tx_buf)
return -ENOMEM;
- rx_buf = kcalloc(max_count, sizeof(*rx_buf), GFP_KERNEL);
- if (!rx_buf) {
- ret = -ENOMEM;
- goto free_tx;
- }
+ struct tsc2046_adc_atom *rx_buf __free(kfree) = kcalloc(max_count,
+ sizeof(*rx_buf),
+ GFP_KERNEL);
+ if (!rx_buf)
+ return -ENOMEM;
/*
* Do not enable automatic power down on working samples. Otherwise the
@@ -326,7 +328,7 @@ static int tsc2046_adc_read_one(struct tsc2046_adc_priv *priv, int ch_idx,
if (ret) {
dev_err_ratelimited(&priv->spi->dev, "SPI transfer failed %pe\n",
ERR_PTR(ret));
- goto free_bufs;
+ return ret;
}
if (effective_speed_hz)
@@ -337,14 +339,7 @@ static int tsc2046_adc_read_one(struct tsc2046_adc_priv *priv, int ch_idx,
val_normalized += val;
}
- ret = DIV_ROUND_UP(val_normalized, max_count - count_skip);
-
-free_bufs:
- kfree(rx_buf);
-free_tx:
- kfree(tx_buf);
-
- return ret;
+ return DIV_ROUND_UP(val_normalized, max_count - count_skip);
}
static size_t tsc2046_adc_group_set_layout(struct tsc2046_adc_priv *priv,
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 6/6] iio: adc: ad5755: drop redundant devm_kfree()
2024-07-05 10:40 [PATCH 0/6] iio: adc: simplify with cleanup.h Krzysztof Kozlowski
` (4 preceding siblings ...)
2024-07-05 10:40 ` [PATCH 5/6] iio: adc: ti-tsc2046: " Krzysztof Kozlowski
@ 2024-07-05 10:40 ` Krzysztof Kozlowski
2024-07-05 11:47 ` Nuno Sá
2024-07-06 10:52 ` Jonathan Cameron
2024-07-06 10:55 ` [PATCH 0/6] iio: adc: simplify with cleanup.h Jonathan Cameron
6 siblings, 2 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2024-07-05 10:40 UTC (permalink / raw)
To: Dan Robertson, Jonathan Cameron, Lars-Peter Clausen,
Michael Hennerich, Nicolas Ferre, Alexandre Belloni,
Claudiu Beznea, Oleksij Rempel, kernel
Cc: linux-iio, linux-kernel, linux-arm-kernel, Krzysztof Kozlowski
The driver calls ad5755_parse_fw() only from probe() function, so
devm_kfree() during error path is not necessary and only makes code
weirder.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
drivers/iio/dac/ad5755.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c
index 0b24cb19ac9d..bfbfc3c1b6a5 100644
--- a/drivers/iio/dac/ad5755.c
+++ b/drivers/iio/dac/ad5755.c
@@ -803,7 +803,6 @@ static struct ad5755_platform_data *ad5755_parse_fw(struct device *dev)
error_out:
fwnode_handle_put(pp);
- devm_kfree(dev, pdata);
return NULL;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/6] iio: adc: ad7280a: simplify with cleanup.h
2024-07-05 10:40 ` [PATCH 2/6] iio: adc: ad7280a: " Krzysztof Kozlowski
@ 2024-07-05 11:43 ` Nuno Sá
0 siblings, 0 replies; 14+ messages in thread
From: Nuno Sá @ 2024-07-05 11:43 UTC (permalink / raw)
To: Krzysztof Kozlowski, Dan Robertson, Jonathan Cameron,
Lars-Peter Clausen, Michael Hennerich, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea, Oleksij Rempel, kernel
Cc: linux-iio, linux-kernel, linux-arm-kernel
On Fri, 2024-07-05 at 12:40 +0200, Krzysztof Kozlowski wrote:
> Allocate the memory with scoped/cleanup.h to reduce error handling and
> make the code a bit simpler.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
Reviewed-by: Nuno Sa <nuno.sa@analog.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/6] iio: adc: max1363: simplify with cleanup.h
2024-07-05 10:40 ` [PATCH 4/6] iio: adc: max1363: " Krzysztof Kozlowski
@ 2024-07-05 11:45 ` Nuno Sá
0 siblings, 0 replies; 14+ messages in thread
From: Nuno Sá @ 2024-07-05 11:45 UTC (permalink / raw)
To: Krzysztof Kozlowski, Dan Robertson, Jonathan Cameron,
Lars-Peter Clausen, Michael Hennerich, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea, Oleksij Rempel, kernel
Cc: linux-iio, linux-kernel, linux-arm-kernel
On Fri, 2024-07-05 at 12:40 +0200, Krzysztof Kozlowski wrote:
> Allocate the memory with scoped/cleanup.h to reduce error handling and
> make the code a bit simpler.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
Reviewed-by: Nuno Sa <nuno.sa@analog.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 6/6] iio: adc: ad5755: drop redundant devm_kfree()
2024-07-05 10:40 ` [PATCH 6/6] iio: adc: ad5755: drop redundant devm_kfree() Krzysztof Kozlowski
@ 2024-07-05 11:47 ` Nuno Sá
2024-07-06 10:52 ` Jonathan Cameron
1 sibling, 0 replies; 14+ messages in thread
From: Nuno Sá @ 2024-07-05 11:47 UTC (permalink / raw)
To: Krzysztof Kozlowski, Dan Robertson, Jonathan Cameron,
Lars-Peter Clausen, Michael Hennerich, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea, Oleksij Rempel, kernel
Cc: linux-iio, linux-kernel, linux-arm-kernel
On Fri, 2024-07-05 at 12:40 +0200, Krzysztof Kozlowski wrote:
> The driver calls ad5755_parse_fw() only from probe() function, so
> devm_kfree() during error path is not necessary and only makes code
> weirder.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
Reviewed-by: Nuno Sa <nuno.sa@analog.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 5/6] iio: adc: ti-tsc2046: simplify with cleanup.h
2024-07-05 10:40 ` [PATCH 5/6] iio: adc: ti-tsc2046: " Krzysztof Kozlowski
@ 2024-07-06 5:08 ` Oleksij Rempel
0 siblings, 0 replies; 14+ messages in thread
From: Oleksij Rempel @ 2024-07-06 5:08 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Dan Robertson, Jonathan Cameron, Lars-Peter Clausen,
Michael Hennerich, Nicolas Ferre, Alexandre Belloni,
Claudiu Beznea, kernel, linux-iio, linux-kernel, linux-arm-kernel
On Fri, Jul 05, 2024 at 12:40:48PM +0200, Krzysztof Kozlowski wrote:
> Allocate the memory with scoped/cleanup.h to reduce error handling and
> make the code a bit simpler.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>
Thank you!
> ---
> drivers/iio/adc/ti-tsc2046.c | 29 ++++++++++++-----------------
> 1 file changed, 12 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/iio/adc/ti-tsc2046.c b/drivers/iio/adc/ti-tsc2046.c
> index edcef8f11522..24b1d4390872 100644
> --- a/drivers/iio/adc/ti-tsc2046.c
> +++ b/drivers/iio/adc/ti-tsc2046.c
> @@ -6,6 +6,7 @@
> */
>
> #include <linux/bitfield.h>
> +#include <linux/cleanup.h>
> #include <linux/delay.h>
> #include <linux/module.h>
> #include <linux/regulator/consumer.h>
> @@ -273,7 +274,6 @@ static int tsc2046_adc_read_one(struct tsc2046_adc_priv *priv, int ch_idx,
> u32 *effective_speed_hz)
> {
> struct tsc2046_adc_ch_cfg *ch = &priv->ch_cfg[ch_idx];
> - struct tsc2046_adc_atom *rx_buf, *tx_buf;
> unsigned int val, val_normalized = 0;
> int ret, i, count_skip = 0, max_count;
> struct spi_transfer xfer;
> @@ -287,18 +287,20 @@ static int tsc2046_adc_read_one(struct tsc2046_adc_priv *priv, int ch_idx,
> max_count = 1;
> }
>
> - if (sizeof(*tx_buf) * max_count > PAGE_SIZE)
> + if (sizeof(struct tsc2046_adc_atom) * max_count > PAGE_SIZE)
> return -ENOSPC;
>
> - tx_buf = kcalloc(max_count, sizeof(*tx_buf), GFP_KERNEL);
> + struct tsc2046_adc_atom *tx_buf __free(kfree) = kcalloc(max_count,
> + sizeof(*tx_buf),
> + GFP_KERNEL);
> if (!tx_buf)
> return -ENOMEM;
>
> - rx_buf = kcalloc(max_count, sizeof(*rx_buf), GFP_KERNEL);
> - if (!rx_buf) {
> - ret = -ENOMEM;
> - goto free_tx;
> - }
> + struct tsc2046_adc_atom *rx_buf __free(kfree) = kcalloc(max_count,
> + sizeof(*rx_buf),
> + GFP_KERNEL);
> + if (!rx_buf)
> + return -ENOMEM;
>
> /*
> * Do not enable automatic power down on working samples. Otherwise the
> @@ -326,7 +328,7 @@ static int tsc2046_adc_read_one(struct tsc2046_adc_priv *priv, int ch_idx,
> if (ret) {
> dev_err_ratelimited(&priv->spi->dev, "SPI transfer failed %pe\n",
> ERR_PTR(ret));
> - goto free_bufs;
> + return ret;
> }
>
> if (effective_speed_hz)
> @@ -337,14 +339,7 @@ static int tsc2046_adc_read_one(struct tsc2046_adc_priv *priv, int ch_idx,
> val_normalized += val;
> }
>
> - ret = DIV_ROUND_UP(val_normalized, max_count - count_skip);
> -
> -free_bufs:
> - kfree(rx_buf);
> -free_tx:
> - kfree(tx_buf);
> -
> - return ret;
> + return DIV_ROUND_UP(val_normalized, max_count - count_skip);
> }
>
> static size_t tsc2046_adc_group_set_layout(struct tsc2046_adc_priv *priv,
>
> --
> 2.43.0
>
>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 6/6] iio: adc: ad5755: drop redundant devm_kfree()
2024-07-05 10:40 ` [PATCH 6/6] iio: adc: ad5755: drop redundant devm_kfree() Krzysztof Kozlowski
2024-07-05 11:47 ` Nuno Sá
@ 2024-07-06 10:52 ` Jonathan Cameron
2024-07-07 11:34 ` Krzysztof Kozlowski
1 sibling, 1 reply; 14+ messages in thread
From: Jonathan Cameron @ 2024-07-06 10:52 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Dan Robertson, Lars-Peter Clausen, Michael Hennerich,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea, Oleksij Rempel,
kernel, linux-iio, linux-kernel, linux-arm-kernel, Sean Nyekjaer
On Fri, 05 Jul 2024 12:40:49 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> The driver calls ad5755_parse_fw() only from probe() function, so
> devm_kfree() during error path is not necessary and only makes code
> weirder.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
The path this is in doesn't result in the driver failing to probe as it
falls back to a const default structure.
So whilst it's not a 'bug' to remove this free, we are removing data the driver
is not going to use - so to my eye at least this is a deliberate design
decision.
Mind you it's not a particularly big allocation so maybe worth not cleaning
up until driver remove in order to save on complexity.
Sean, your code I think. Do you care either way?
Jonathan
> ---
> drivers/iio/dac/ad5755.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c
> index 0b24cb19ac9d..bfbfc3c1b6a5 100644
> --- a/drivers/iio/dac/ad5755.c
> +++ b/drivers/iio/dac/ad5755.c
> @@ -803,7 +803,6 @@ static struct ad5755_platform_data *ad5755_parse_fw(struct device *dev)
>
> error_out:
> fwnode_handle_put(pp);
> - devm_kfree(dev, pdata);
> return NULL;
> }
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/6] iio: adc: simplify with cleanup.h
2024-07-05 10:40 [PATCH 0/6] iio: adc: simplify with cleanup.h Krzysztof Kozlowski
` (5 preceding siblings ...)
2024-07-05 10:40 ` [PATCH 6/6] iio: adc: ad5755: drop redundant devm_kfree() Krzysztof Kozlowski
@ 2024-07-06 10:55 ` Jonathan Cameron
6 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2024-07-06 10:55 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Dan Robertson, Lars-Peter Clausen, Michael Hennerich,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea, Oleksij Rempel,
kernel, linux-iio, linux-kernel, linux-arm-kernel
On Fri, 05 Jul 2024 12:40:43 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> Allocate the memory with scoped/cleanup.h to reduce error handling and
> make the code a bit simpler.
For now I've applied 1-5 as I think 6 needs a little more discussion.
Note applied only to the testing branch of iio.git for now.
That will become the togreg branch and get picked up by linux-next etc
after I've rebased on 6.11-rc1.
Jonathan
>
> Best regards,
> Krzysztof
>
> ---
> Krzysztof Kozlowski (6):
> iio: accel: bma400: simplify with cleanup.h
> iio: adc: ad7280a: simplify with cleanup.h
> iio: adc: at91: simplify with cleanup.h
> iio: adc: max1363: simplify with cleanup.h
> iio: adc: ti-tsc2046: simplify with cleanup.h
> iio: adc: ad5755: drop redundant devm_kfree()
>
> drivers/iio/accel/bma400_core.c | 11 +++++------
> drivers/iio/adc/ad7280a.c | 10 ++++------
> drivers/iio/adc/at91_adc.c | 13 +++++--------
> drivers/iio/adc/max1363.c | 34 +++++++++++++---------------------
> drivers/iio/adc/ti-tsc2046.c | 29 ++++++++++++-----------------
> drivers/iio/dac/ad5755.c | 1 -
> 6 files changed, 39 insertions(+), 59 deletions(-)
> ---
> base-commit: 0b58e108042b0ed28a71cd7edf5175999955b233
> change-id: 20240705-cleanup-h-iio-c90ca38865a4
>
> Best regards,
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 6/6] iio: adc: ad5755: drop redundant devm_kfree()
2024-07-06 10:52 ` Jonathan Cameron
@ 2024-07-07 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2024-07-07 11:34 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Dan Robertson, Lars-Peter Clausen, Michael Hennerich,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea, Oleksij Rempel,
kernel, linux-iio, linux-kernel, linux-arm-kernel, Sean Nyekjaer
On 06/07/2024 12:52, Jonathan Cameron wrote:
> On Fri, 05 Jul 2024 12:40:49 +0200
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>
>> The driver calls ad5755_parse_fw() only from probe() function, so
>> devm_kfree() during error path is not necessary and only makes code
>> weirder.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
> The path this is in doesn't result in the driver failing to probe as it
> falls back to a const default structure.
> So whilst it's not a 'bug' to remove this free, we are removing data the driver
> is not going to use - so to my eye at least this is a deliberate design
> decision.
Ah, I missed that part - just looked at !pdata in the probe.
>
> Mind you it's not a particularly big allocation so maybe worth not cleaning
> up until driver remove in order to save on complexity.
>
> Sean, your code I think. Do you care either way?
I think the code was correct and my patch can be abandoned.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2024-07-07 11:34 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-05 10:40 [PATCH 0/6] iio: adc: simplify with cleanup.h Krzysztof Kozlowski
2024-07-05 10:40 ` [PATCH 1/6] iio: accel: bma400: " Krzysztof Kozlowski
2024-07-05 10:40 ` [PATCH 2/6] iio: adc: ad7280a: " Krzysztof Kozlowski
2024-07-05 11:43 ` Nuno Sá
2024-07-05 10:40 ` [PATCH 3/6] iio: adc: at91: " Krzysztof Kozlowski
2024-07-05 10:40 ` [PATCH 4/6] iio: adc: max1363: " Krzysztof Kozlowski
2024-07-05 11:45 ` Nuno Sá
2024-07-05 10:40 ` [PATCH 5/6] iio: adc: ti-tsc2046: " Krzysztof Kozlowski
2024-07-06 5:08 ` Oleksij Rempel
2024-07-05 10:40 ` [PATCH 6/6] iio: adc: ad5755: drop redundant devm_kfree() Krzysztof Kozlowski
2024-07-05 11:47 ` Nuno Sá
2024-07-06 10:52 ` Jonathan Cameron
2024-07-07 11:34 ` Krzysztof Kozlowski
2024-07-06 10:55 ` [PATCH 0/6] iio: adc: simplify with cleanup.h Jonathan Cameron
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).