linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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).