[PATCH v3 0/3] iio: ssp_sensors: improve resource cleanup with cleanup.h

[PATCH v3 0/3] iio: ssp_sensors: improve resource cleanup with cleanup.h

[Sanjay Chitroda]

From: Sanjay Chitroda <sanjayembeddedse@gmail.com>

Hi all,

This patch series improves resource cleanup and error handling in the
SSP IIO SPI driver by adopting the recently introduced cleanup
helpers.

The changes focus on making probe/remove paths more robust and easier
to reason about by reducing manual unwind logic and ensuring that locks
and dynamically allocated resources are released consistently across
all exit paths.

Key highlights of this series:

- Use guard() helpers to automatically release mutexes on scope exit,
  avoiding missing unlocks on error paths.
- Simplify memory and resource cleanup by using the __free cleanup API,
  reducing the need for explicit goto-based unwind code.
- Address minor codestyle warnings introduced or exposed by the cleanup
  refactoring.

No functional behavior changes are intended.

Testing:
  - Compiled with W=1
  - Build-tested on QEMU x86_64

Based on:
  <linux-v7.0-rc2>

Feedback and reviews are very welcome.

Thanks,
Sanjay Chitroda

Sanjay Chitroda (3):
  iio: ssp_sensors: ssp_spi: use guard() to release mutexes
  iio: ssp_sensors: simplify cleanup using __free
  iio: ssp_sensors: cleanup codestyle warning

 drivers/iio/common/ssp_sensors/ssp_spi.c | 76 +++++++++---------------
 1 file changed, 29 insertions(+), 47 deletions(-)

-- 
2.34.1

[PATCH v3 1/3] iio: ssp_sensors: ssp_spi: use guard() to release mutexes

[Sanjay Chitroda]

From: Sanjay Chitroda <sanjayembeddedse@gmail.com>

Replace explicit mutex_lock() and mutex_unlock() with the guard() macro
for cleaner and safer mutex handling.

Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
 drivers/iio/common/ssp_sensors/ssp_spi.c | 54 ++++++++++--------------
 1 file changed, 22 insertions(+), 32 deletions(-)

diff --git a/drivers/iio/common/ssp_sensors/ssp_spi.c b/drivers/iio/common/ssp_sensors/ssp_spi.c
index 6c81c0385fb5..87a38002b218 100644
--- a/drivers/iio/common/ssp_sensors/ssp_spi.c
+++ b/drivers/iio/common/ssp_sensors/ssp_spi.c
@@ -2,6 +2,7 @@
 /*
  *  Copyright (C) 2014, Samsung Electronics Co. Ltd. All Rights Reserved.
  */
+#include <linux/cleanup.h>
 
 #include "ssp.h"
 
@@ -189,55 +190,49 @@ static int ssp_do_transfer(struct ssp_data *data, struct ssp_msg *msg,
 
 	msg->done = done;
 
-	mutex_lock(&data->comm_lock);
+	guard(mutex)(&data->comm_lock);
 
 	status = ssp_check_lines(data, false);
-	if (status < 0)
-		goto _error_locked;
+	if (status < 0) {
+		data->timeout_cnt++;
+		return status;
+	}
 
 	status = spi_write(data->spi, msg->buffer, SSP_HEADER_SIZE);
 	if (status < 0) {
 		gpiod_set_value_cansleep(data->ap_mcu_gpiod, 1);
 		dev_err(SSP_DEV, "%s spi_write fail\n", __func__);
-		goto _error_locked;
+		data->timeout_cnt++;
+		return status;
 	}
 
 	if (!use_no_irq) {
-		mutex_lock(&data->pending_lock);
-		list_add_tail(&msg->list, &data->pending_list);
-		mutex_unlock(&data->pending_lock);
+		scoped_guard(mutex, &data->pending_lock)
+			list_add_tail(&msg->list, &data->pending_list);
 	}
 
 	status = ssp_check_lines(data, true);
 	if (status < 0) {
 		if (!use_no_irq) {
-			mutex_lock(&data->pending_lock);
-			list_del(&msg->list);
-			mutex_unlock(&data->pending_lock);
+			scoped_guard(mutex, &data->pending_lock)
+				list_add_tail(&msg->list, &data->pending_list);
 		}
-		goto _error_locked;
+		data->timeout_cnt++;
+		return status;
 	}
 
-	mutex_unlock(&data->comm_lock);
-
 	if (!use_no_irq && done)
 		if (wait_for_completion_timeout(done,
 						msecs_to_jiffies(timeout)) ==
 		    0) {
-			mutex_lock(&data->pending_lock);
-			list_del(&msg->list);
-			mutex_unlock(&data->pending_lock);
+			scoped_guard(mutex, &data->pending_lock)
+				list_add_tail(&msg->list, &data->pending_list);
 
 			data->timeout_cnt++;
 			return -ETIMEDOUT;
 		}
 
 	return 0;
-
-_error_locked:
-	mutex_unlock(&data->comm_lock);
-	data->timeout_cnt++;
-	return status;
 }
 
 static inline int ssp_spi_sync_command(struct ssp_data *data,
@@ -360,7 +355,7 @@ int ssp_irq_msg(struct ssp_data *data)
 		 * this is a small list, a few elements - the packets can be
 		 * received with no order
 		 */
-		mutex_lock(&data->pending_lock);
+		guard(mutex)(&data->pending_lock);
 		list_for_each_entry_safe(iter, n, &data->pending_list, list) {
 			if (iter->options == msg_options) {
 				list_del(&iter->list);
@@ -376,10 +371,8 @@ int ssp_irq_msg(struct ssp_data *data)
 			 * check but let's handle this
 			 */
 			buffer = kmalloc(length, GFP_KERNEL | GFP_DMA);
-			if (!buffer) {
-				ret = -ENOMEM;
-				goto _unlock;
-			}
+			if (!buffer)
+				return -ENOMEM;
 
 			/* got dead packet so it is always an error */
 			ret = spi_read(data->spi, buffer, length);
@@ -391,7 +384,7 @@ int ssp_irq_msg(struct ssp_data *data)
 			dev_err(SSP_DEV, "No match error %x\n",
 				msg_options);
 
-			goto _unlock;
+			return ret;
 		}
 
 		if (msg_type == SSP_AP2HUB_READ)
@@ -409,15 +402,13 @@ int ssp_irq_msg(struct ssp_data *data)
 				msg->length = 1;
 
 				list_add_tail(&msg->list, &data->pending_list);
-				goto _unlock;
+				return ret;
 			}
 		}
 
 		if (msg->done)
 			if (!completion_done(msg->done))
 				complete(msg->done);
-_unlock:
-		mutex_unlock(&data->pending_lock);
 		break;
 	case SSP_HUB2AP_WRITE:
 		buffer = kzalloc(length, GFP_KERNEL | GFP_DMA);
@@ -448,7 +439,7 @@ void ssp_clean_pending_list(struct ssp_data *data)
 {
 	struct ssp_msg *msg, *n;
 
-	mutex_lock(&data->pending_lock);
+	guard(mutex)(&data->pending_lock);
 	list_for_each_entry_safe(msg, n, &data->pending_list, list) {
 		list_del(&msg->list);
 
@@ -456,7 +447,6 @@ void ssp_clean_pending_list(struct ssp_data *data)
 			if (!completion_done(msg->done))
 				complete(msg->done);
 	}
-	mutex_unlock(&data->pending_lock);
 }
 
 int ssp_command(struct ssp_data *data, char command, int arg)
-- 
2.34.1

[PATCH v3 2/3] iio: ssp_sensors: simplify cleanup using __free

[Sanjay Chitroda]

From: Sanjay Chitroda <sanjayembeddedse@gmail.com>

Replace manual cleanup logic with __free attribute from cleanup.h. This
removes explicit kfree() calls and simplifies the error handling paths.

No functional change intended for kmalloc().

Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
Changes in v3:
- prepare series to have all respective cleanup API support for the ssp_sensors following input from Andy Shevchenko
- Link to v2 https://lore.kernel.org/all/20260311174151.3441429-1-sanjayembedded@gmail.com/
Changes in v2:
- split series to individual patch
- address review comment from Andy Shevchenko
- Link to v1 https://lore.kernel.org/all/20260310200513.2162018-3-sanjayembedded@gmail.com/
---
 drivers/iio/common/ssp_sensors/ssp_spi.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/common/ssp_sensors/ssp_spi.c b/drivers/iio/common/ssp_sensors/ssp_spi.c
index 87a38002b218..9df94bd02e3a 100644
--- a/drivers/iio/common/ssp_sensors/ssp_spi.c
+++ b/drivers/iio/common/ssp_sensors/ssp_spi.c
@@ -326,7 +326,6 @@ static int ssp_parse_dataframe(struct ssp_data *data, char *dataframe, int len)
 /* threaded irq */
 int ssp_irq_msg(struct ssp_data *data)
 {
-	char *buffer;
 	u8 msg_type;
 	int ret;
 	u16 length, msg_options;
@@ -370,7 +369,7 @@ int ssp_irq_msg(struct ssp_data *data)
 			 * but the slave should not send such ones - it is to
 			 * check but let's handle this
 			 */
-			buffer = kmalloc(length, GFP_KERNEL | GFP_DMA);
+			char *buffer __free(kfree) = kmalloc(length, GFP_KERNEL | GFP_DMA);
 			if (!buffer)
 				return -ENOMEM;
 
@@ -379,8 +378,6 @@ int ssp_irq_msg(struct ssp_data *data)
 			if (ret >= 0)
 				ret = -EPROTO;
 
-			kfree(buffer);
-
 			dev_err(SSP_DEV, "No match error %x\n",
 				msg_options);
 
@@ -411,22 +408,17 @@ int ssp_irq_msg(struct ssp_data *data)
 				complete(msg->done);
 		break;
 	case SSP_HUB2AP_WRITE:
-		buffer = kzalloc(length, GFP_KERNEL | GFP_DMA);
+		char *buffer __free(kfree) = kzalloc(length, GFP_KERNEL | GFP_DMA);
 		if (!buffer)
 			return -ENOMEM;
 
 		ret = spi_read(data->spi, buffer, length);
 		if (ret < 0) {
 			dev_err(SSP_DEV, "spi read fail\n");
-			kfree(buffer);
 			break;
 		}
 
-		ret = ssp_parse_dataframe(data, buffer, length);
-
-		kfree(buffer);
-		break;
-
+		return ssp_parse_dataframe(data, buffer, length);
 	default:
 		dev_err(SSP_DEV, "unknown msg type\n");
 		return -EPROTO;
-- 
2.34.1

[PATCH v3 3/3] iio: ssp_sensors: cleanup codestyle warning

[Sanjay Chitroda]

From: Sanjay Chitroda <sanjayembeddedse@gmail.com>

Reported by checkpatch:
FILE: drivers/iio/common/ssp_sensors/ssp_spi.c

WARNING: Prefer __packed over __attribute__((__packed__))
+} __attribute__((__packed__));

CHECK: Macro argument '...' may be better as '(...)'
to avoid precedence issues

Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
 drivers/iio/common/ssp_sensors/ssp_spi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/common/ssp_sensors/ssp_spi.c b/drivers/iio/common/ssp_sensors/ssp_spi.c
index 9df94bd02e3a..8a52bfb0e39c 100644
--- a/drivers/iio/common/ssp_sensors/ssp_spi.c
+++ b/drivers/iio/common/ssp_sensors/ssp_spi.c
@@ -7,7 +7,7 @@
 #include "ssp.h"
 
 #define SSP_DEV (&data->spi->dev)
-#define SSP_GET_MESSAGE_TYPE(data) (data & (3 << SSP_RW))
+#define SSP_GET_MESSAGE_TYPE(data) ((data) & (3 << SSP_RW))
 
 /*
  * SSP -> AP Instruction
@@ -30,7 +30,7 @@ struct ssp_msg_header {
 	__le16 length;
 	__le16 options;
 	__le32 data;
-} __attribute__((__packed__));
+} __packed;
 
 struct ssp_msg {
 	u16 length;
@@ -120,9 +120,9 @@ static inline void ssp_get_buffer(struct ssp_msg *m, unsigned int offset,
 }
 
 #define SSP_GET_BUFFER_AT_INDEX(m, index) \
-	(m->buffer[SSP_HEADER_SIZE_ALIGNED + index])
+	((m)->buffer[SSP_HEADER_SIZE_ALIGNED + (index)])
 #define SSP_SET_BUFFER_AT_INDEX(m, index, val) \
-	(m->buffer[SSP_HEADER_SIZE_ALIGNED + index] = val)
+	((m)->buffer[SSP_HEADER_SIZE_ALIGNED + (index)] = val)
 
 static void ssp_clean_msg(struct ssp_msg *m)
 {
-- 
2.34.1

Re: [PATCH v3 1/3] iio: ssp_sensors: ssp_spi: use guard() to release mutexes

[Jonathan Cameron]

On Sun, 15 Mar 2026 18:25:07 +0530
Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:

> From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> 
> Replace explicit mutex_lock() and mutex_unlock() with the guard() macro
> for cleaner and safer mutex handling.
> 
> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> ---
>  drivers/iio/common/ssp_sensors/ssp_spi.c | 54 ++++++++++--------------
>  1 file changed, 22 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/iio/common/ssp_sensors/ssp_spi.c b/drivers/iio/common/ssp_sensors/ssp_spi.c
> index 6c81c0385fb5..87a38002b218 100644
> --- a/drivers/iio/common/ssp_sensors/ssp_spi.c
> +++ b/drivers/iio/common/ssp_sensors/ssp_spi.c
> @@ -2,6 +2,7 @@
>  /*
>   *  Copyright (C) 2014, Samsung Electronics Co. Ltd. All Rights Reserved.
>   */
> +#include <linux/cleanup.h>
>  
>  #include "ssp.h"
>  
> @@ -189,55 +190,49 @@ static int ssp_do_transfer(struct ssp_data *data, struct ssp_msg *msg,
>  
>  	msg->done = done;
>  
> -	mutex_lock(&data->comm_lock);
> +	guard(mutex)(&data->comm_lock);
>  
>  	status = ssp_check_lines(data, false);
> -	if (status < 0)
> -		goto _error_locked;
> +	if (status < 0) {
> +		data->timeout_cnt++;
> +		return status;
> +	}
>  
>  	status = spi_write(data->spi, msg->buffer, SSP_HEADER_SIZE);
>  	if (status < 0) {
>  		gpiod_set_value_cansleep(data->ap_mcu_gpiod, 1);
>  		dev_err(SSP_DEV, "%s spi_write fail\n", __func__);
> -		goto _error_locked;
> +		data->timeout_cnt++;
guard() isn't always the right answer.  The counter needing updating
here in every error path makes me thing maybe it isn't here.
Pity there is one error path at the top where that's not updated,
otherwise I'd suggest updating in a wrapper function.

> +		return status;
>  	}
>  
>  	if (!use_no_irq) {
> -		mutex_lock(&data->pending_lock);
> -		list_add_tail(&msg->list, &data->pending_list);
> -		mutex_unlock(&data->pending_lock);
> +		scoped_guard(mutex, &data->pending_lock)

The guard brings little here, but if you want to do it.
		guard(mutex)(&data->pending_lock);
is fine - no need for scoped_guard() and increased indent.
Make sure you understand why that is effectively the same in this cases.

> +			list_add_tail(&msg->list, &data->pending_list);
>  	}
>  
>  	status = ssp_check_lines(data, true);
>  	if (status < 0) {
>  		if (!use_no_irq) {
> -			mutex_lock(&data->pending_lock);
> -			list_del(&msg->list);
> -			mutex_unlock(&data->pending_lock);
> +			scoped_guard(mutex, &data->pending_lock)

As above.

> +				list_add_tail(&msg->list, &data->pending_list);
>  		}
> -		goto _error_locked;
> +		data->timeout_cnt++;
> +		return status;
>  	}
>  
> -	mutex_unlock(&data->comm_lock);
> -
>  	if (!use_no_irq && done)
>  		if (wait_for_completion_timeout(done,
>  						msecs_to_jiffies(timeout)) ==
>  		    0) {
> -			mutex_lock(&data->pending_lock);
> -			list_del(&msg->list);
> -			mutex_unlock(&data->pending_lock);
> +			scoped_guard(mutex, &data->pending_lock)
> +				list_add_tail(&msg->list, &data->pending_list);
Slightly trickier but holding the mutex over the timeout_cnt is fine, so
I'd use guard() here as well.

>  
>  			data->timeout_cnt++;
>  			return -ETIMEDOUT;
>  		}
>  
>  	return 0;
> -
> -_error_locked:
> -	mutex_unlock(&data->comm_lock);
> -	data->timeout_cnt++;
> -	return status;
>  }
>  
>  static inline int ssp_spi_sync_command(struct ssp_data *data,
> @@ -360,7 +355,7 @@ int ssp_irq_msg(struct ssp_data *data)
>  		 * this is a small list, a few elements - the packets can be
>  		 * received with no order
>  		 */
> -		mutex_lock(&data->pending_lock);
> +		guard(mutex)(&data->pending_lock);

Check the scope definition.  Given where you remove the unlock below
I'm not sure the scope is what you think it is.
Switch statements are tricky wrt to scope!




>  		list_for_each_entry_safe(iter, n, &data->pending_list, list) {
>  			if (iter->options == msg_options) {
>  				list_del(&iter->list);
> @@ -376,10 +371,8 @@ int ssp_irq_msg(struct ssp_data *data)
>  			 * check but let's handle this
>  			 */
>  			buffer = kmalloc(length, GFP_KERNEL | GFP_DMA);
> -			if (!buffer) {
> -				ret = -ENOMEM;
> -				goto _unlock;
> -			}
> +			if (!buffer)
> +				return -ENOMEM;
>  
>  			/* got dead packet so it is always an error */
>  			ret = spi_read(data->spi, buffer, length);
> @@ -391,7 +384,7 @@ int ssp_irq_msg(struct ssp_data *data)
>  			dev_err(SSP_DEV, "No match error %x\n",
>  				msg_options);
>  
> -			goto _unlock;
> +			return ret;
>  		}
>  
>  		if (msg_type == SSP_AP2HUB_READ)
> @@ -409,15 +402,13 @@ int ssp_irq_msg(struct ssp_data *data)
>  				msg->length = 1;
>  
>  				list_add_tail(&msg->list, &data->pending_list);
> -				goto _unlock;
> +				return ret;
>  			}
>  		}
>  
>  		if (msg->done)
>  			if (!completion_done(msg->done))
>  				complete(msg->done);
> -_unlock:
> -		mutex_unlock(&data->pending_lock);
>  		break;
>  	case SSP_HUB2AP_WRITE:
>  		buffer = kzalloc(length, GFP_KERNEL | GFP_DMA);
> @@ -448,7 +439,7 @@ void ssp_clean_pending_list(struct ssp_data *data)
>  {
>  	struct ssp_msg *msg, *n;
>  
> -	mutex_lock(&data->pending_lock);
> +	guard(mutex)(&data->pending_lock);
>  	list_for_each_entry_safe(msg, n, &data->pending_list, list) {
>  		list_del(&msg->list);
>  
> @@ -456,7 +447,6 @@ void ssp_clean_pending_list(struct ssp_data *data)
>  			if (!completion_done(msg->done))
>  				complete(msg->done);
>  	}
> -	mutex_unlock(&data->pending_lock);
>  }
>  
>  int ssp_command(struct ssp_data *data, char command, int arg)

Re: [PATCH v3 2/3] iio: ssp_sensors: simplify cleanup using __free

[Jonathan Cameron]

On Sun, 15 Mar 2026 18:25:08 +0530
Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:

> From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> 
> Replace manual cleanup logic with __free attribute from cleanup.h. This
> removes explicit kfree() calls and simplifies the error handling paths.
> 
> No functional change intended for kmalloc().
> 
> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Hi Sanjay

I'd suggest slowing down with new versions. 1 weeks is a usual good minimum
because it avoids what has happened here where you get feedback on a later
version that could have been given on the earlier one and hence
waste both your time and reviewer time (a little).

> ---
> Changes in v3:
> - prepare series to have all respective cleanup API support for the ssp_sensors following input from Andy Shevchenko
> - Link to v2 https://lore.kernel.org/all/20260311174151.3441429-1-sanjayembedded@gmail.com/
> Changes in v2:
> - split series to individual patch
> - address review comment from Andy Shevchenko
> - Link to v1 https://lore.kernel.org/all/20260310200513.2162018-3-sanjayembedded@gmail.com/
> ---
>  drivers/iio/common/ssp_sensors/ssp_spi.c | 14 +++-----------
>  1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iio/common/ssp_sensors/ssp_spi.c b/drivers/iio/common/ssp_sensors/ssp_spi.c
> index 87a38002b218..9df94bd02e3a 100644
> --- a/drivers/iio/common/ssp_sensors/ssp_spi.c
> +++ b/drivers/iio/common/ssp_sensors/ssp_spi.c
> @@ -326,7 +326,6 @@ static int ssp_parse_dataframe(struct ssp_data *data, char *dataframe, int len)
>  /* threaded irq */
>  int ssp_irq_msg(struct ssp_data *data)
>  {
> -	char *buffer;
>  	u8 msg_type;
>  	int ret;
>  	u16 length, msg_options;
> @@ -370,7 +369,7 @@ int ssp_irq_msg(struct ssp_data *data)
>  			 * but the slave should not send such ones - it is to
>  			 * check but let's handle this
>  			 */
> -			buffer = kmalloc(length, GFP_KERNEL | GFP_DMA);
> +			char *buffer __free(kfree) = kmalloc(length, GFP_KERNEL | GFP_DMA);
Andy raised if this is the right thing to do at all I think.
Maybe look at whether there is a simple maximum length and stash a buffer that
gets reused each time instead of a fresh allocation. May well simplify
the code.

>  			if (!buffer)
>  				return -ENOMEM;
>  
> @@ -379,8 +378,6 @@ int ssp_irq_msg(struct ssp_data *data)
>  			if (ret >= 0)
>  				ret = -EPROTO;
>  
> -			kfree(buffer);
> -
>  			dev_err(SSP_DEV, "No match error %x\n",
>  				msg_options);
>  
> @@ -411,22 +408,17 @@ int ssp_irq_msg(struct ssp_data *data)
>  				complete(msg->done);
>  		break;
>  	case SSP_HUB2AP_WRITE:
> -		buffer = kzalloc(length, GFP_KERNEL | GFP_DMA);
> +		char *buffer __free(kfree) = kzalloc(length, GFP_KERNEL | GFP_DMA);

Scope isn't what you think it is...


>  		if (!buffer)
>  			return -ENOMEM;
>  
>  		ret = spi_read(data->spi, buffer, length);
>  		if (ret < 0) {
>  			dev_err(SSP_DEV, "spi read fail\n");
> -			kfree(buffer);
>  			break;
>  		}
>  
> -		ret = ssp_parse_dataframe(data, buffer, length);
> -
> -		kfree(buffer);
> -		break;
> -
> +		return ssp_parse_dataframe(data, buffer, length);
>  	default:
>  		dev_err(SSP_DEV, "unknown msg type\n");
>  		return -EPROTO;

Re: [PATCH v3 3/3] iio: ssp_sensors: cleanup codestyle warning

[Jonathan Cameron]

On Sun, 15 Mar 2026 18:25:09 +0530
Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:

> From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> 
> Reported by checkpatch:
> FILE: drivers/iio/common/ssp_sensors/ssp_spi.c
> 
> WARNING: Prefer __packed over __attribute__((__packed__))
> +} __attribute__((__packed__));
> 
> CHECK: Macro argument '...' may be better as '(...)'
> to avoid precedence issues
> 
> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
This looks fine to me.

Jonathan

Re: [PATCH v3 1/3] iio: ssp_sensors: ssp_spi: use guard() to release mutexes

[Andy Shevchenko]

On Sun, Mar 15, 2026 at 06:25:07PM +0530, Sanjay Chitroda wrote:

> Replace explicit mutex_lock() and mutex_unlock() with the guard() macro
> for cleaner and safer mutex handling.

TBH I don't see much benefit in this form. What I am thinking of is to refactor
to have the guard and timeout_cnt++ in the top level, and

static void ...(flag)
{
	if (flag)
		return;

	guard(mutex)(&data->pending_lock);
	list_add_tail(&msg->list, &data->pending_list);
}

helper for three (*yes, 3) repetitive code snippets.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 3/3] iio: ssp_sensors: cleanup codestyle warning

[Andy Shevchenko]

On Sun, Mar 15, 2026 at 06:25:09PM +0530, Sanjay Chitroda wrote:

> Reported by checkpatch:
> FILE: drivers/iio/common/ssp_sensors/ssp_spi.c
> 
> WARNING: Prefer __packed over __attribute__((__packed__))
> +} __attribute__((__packed__));

> CHECK: Macro argument '...' may be better as '(...)'
> to avoid precedence issues

Two problems — two separate changes, please.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/3] iio: ssp_sensors: ssp_spi: use guard() to release mutexes

[Sanjay Chitroda]

On 16 March 2026 8:02:09 pm IST, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
>On Sun, Mar 15, 2026 at 06:25:07PM +0530, Sanjay Chitroda wrote:
>
>> Replace explicit mutex_lock() and mutex_unlock() with the guard() macro
>> for cleaner and safer mutex handling.
>
>TBH I don't see much benefit in this form. What I am thinking of is to refactor
>to have the guard and timeout_cnt++ in the top level, and
>
>static void ...(flag)
>{
>	if (flag)
>		return;
>
>	guard(mutex)(&data->pending_lock);
>	list_add_tail(&msg->list, &data->pending_list);
>}
>
>helper for three (*yes, 3) repetitive code snippets.
>

Thank you Andy for the review.

I understand your point about refactoring if pattern is repeated to simply things.

Also, while revisiting the changes, I noticed that unintended mistake where list_del() was replaced with list_add_tail() in couple of place during guard() conversion. I'll fix that in v4 along with suggestions and input from you and Jonathan.

I’ll send an updated version shortly.