[PATCH v4 0/4] iio: ssp_sensors: improve resource cleanup with cleanup.h

[PATCH v4 0/4] 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:

- Reuse preallocate rx buffer for SPI transfer instead of allocating new
  DMA memory in interrupt context for write transaction between AP <-> HUB.
- Use guard() helpers to automatically release mutexes on scope exit,
  avoiding missing unlocks on error paths.
- Address minor codestyle warnings introduced or exposed by the cleanup
  refactoring.

Changes in v4:
- Update sequence and make checkpatch style fix in base change
- Split WARNING and CHECK change with input of Andy as 0001 and 0002
- 0003: Use guard() instead of scoped_guard()
- 0004: Use preallocated buffer to stash memory allocation

No functional behavior changes are intended.

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

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

Feedback and reviews are very welcome.

Thanks,
Sanjay Chitroda

Sanjay Chitroda (4):
  iio: ssp_sensors: cleanup codestyle warning
  iio: ssp_sensors: cleanup codestyle check
  iio: ssp_sensors: ssp_spi: use guard() to release mutexes
  iio: ssp_sensors: reuse preallocated RX buffer for SPI transfers

 drivers/iio/common/ssp_sensors/ssp.h     |  5 ++
 drivers/iio/common/ssp_sensors/ssp_dev.c | 11 ++++
 drivers/iio/common/ssp_sensors/ssp_spi.c | 71 +++++++++---------------
 3 files changed, 41 insertions(+), 46 deletions(-)

-- 
2.34.1

[PATCH v4 1/4] 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__));

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

diff --git a/drivers/iio/common/ssp_sensors/ssp_spi.c b/drivers/iio/common/ssp_sensors/ssp_spi.c
index 6c81c0385fb5..8c1e15d61db7 100644
--- a/drivers/iio/common/ssp_sensors/ssp_spi.c
+++ b/drivers/iio/common/ssp_sensors/ssp_spi.c
@@ -29,7 +29,7 @@ struct ssp_msg_header {
 	__le16 length;
 	__le16 options;
 	__le32 data;
-} __attribute__((__packed__));
+} __packed;
 
 struct ssp_msg {
 	u16 length;
-- 
2.34.1

[PATCH v4 2/4] iio: ssp_sensors: cleanup codestyle check

[Sanjay Chitroda]

From: Sanjay Chitroda <sanjayembeddedse@gmail.com>

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

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 | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/common/ssp_sensors/ssp_spi.c b/drivers/iio/common/ssp_sensors/ssp_spi.c
index 8c1e15d61db7..08ed92859be0 100644
--- a/drivers/iio/common/ssp_sensors/ssp_spi.c
+++ b/drivers/iio/common/ssp_sensors/ssp_spi.c
@@ -6,7 +6,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
@@ -119,9 +119,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

[PATCH v4 3/4] 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>
---
Changes in v4:
- Use guard() instead of scoped_guard() to avoid additional indentation
  as scope is same for both APIs and add scope in switch case
- Link to v3: https://lore.kernel.org/all/20260315125509.857195-2-sanjayembedded@gmail.com/
---
 drivers/iio/common/ssp_sensors/ssp_spi.c | 50 ++++++++++--------------
 1 file changed, 20 insertions(+), 30 deletions(-)

diff --git a/drivers/iio/common/ssp_sensors/ssp_spi.c b/drivers/iio/common/ssp_sensors/ssp_spi.c
index 08ed92859be0..61a4e978d9b2 100644
--- a/drivers/iio/common/ssp_sensors/ssp_spi.c
+++ b/drivers/iio/common/ssp_sensors/ssp_spi.c
@@ -189,55 +189,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);
+		guard(mutex)(&data->pending_lock);
 		list_add_tail(&msg->list, &data->pending_list);
-		mutex_unlock(&data->pending_lock);
 	}
 
 	status = ssp_check_lines(data, true);
 	if (status < 0) {
 		if (!use_no_irq) {
-			mutex_lock(&data->pending_lock);
+			guard(mutex)(&data->pending_lock);
 			list_del(&msg->list);
-			mutex_unlock(&data->pending_lock);
 		}
-		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);
+			guard(mutex)(&data->pending_lock);
 			list_del(&msg->list);
-			mutex_unlock(&data->pending_lock);
 
 			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,
@@ -355,12 +349,12 @@ int ssp_irq_msg(struct ssp_data *data)
 
 	switch (msg_type) {
 	case SSP_AP2HUB_READ:
-	case SSP_AP2HUB_WRITE:
+	case SSP_AP2HUB_WRITE: {
 		/*
 		 * 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 +370,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 +383,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,16 +401,15 @@ 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);
 		if (!buffer)
@@ -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 v4 4/4] iio: ssp_sensors: reuse preallocated RX buffer for SPI transfers

[Sanjay Chitroda]

From: Sanjay Chitroda <sanjayembeddedse@gmail.com>

Avoid allocating a temporary DMA buffer in the interrupt context when
handling hub-to-AP and AP-to-hub SPI write messages.

Preallocate RX buffer during probe and reuse it for SPI receive
operations. This removes repeated kzalloc() calls from the IRQ
path, reduces allocation overhead, and avoids potential allocation
failures under memory pressure.

The RX buffer size is tracked and allocated using devm_kzalloc(), ensuring
proper lifetime management tied to the device.

No functional change intended; this is an internal optimization and
robustness improvement.

Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
Changes in v4:
- Use preallocated buffer and stash a buffer that gets reused each time instead of a fresh allocation.
- Link to v3: https://lore.kernel.org/all/20260315125509.857195-3-sanjayembedded@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.h     |  5 +++++
 drivers/iio/common/ssp_sensors/ssp_dev.c | 11 +++++++++++
 drivers/iio/common/ssp_sensors/ssp_spi.c | 17 +++--------------
 3 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/drivers/iio/common/ssp_sensors/ssp.h b/drivers/iio/common/ssp_sensors/ssp.h
index f649cdecc277..aa125fd1bed5 100644
--- a/drivers/iio/common/ssp_sensors/ssp.h
+++ b/drivers/iio/common/ssp_sensors/ssp.h
@@ -175,6 +175,8 @@ struct ssp_sensorhub_info {
  * @sensor_devs:	registered IIO devices table
  * @enable_refcount:	enable reference count for wdt (watchdog timer)
  * @header_buffer:	cache aligned buffer for packet header
+ * @rx_buf:		buffer to receive SPI data
+ * @rx_buf_size:	allocated size of rx_buf
  */
 struct ssp_data {
 	struct spi_device *spi;
@@ -222,6 +224,9 @@ struct ssp_data {
 	atomic_t enable_refcount;
 
 	__le16 header_buffer[SSP_HEADER_BUFFER_SIZE / sizeof(__le16)] __aligned(IIO_DMA_MINALIGN);
+
+	u8 *rx_buf;
+	size_t rx_buf_size;
 };
 
 void ssp_clean_pending_list(struct ssp_data *data);
diff --git a/drivers/iio/common/ssp_sensors/ssp_dev.c b/drivers/iio/common/ssp_sensors/ssp_dev.c
index da09c9f3ceb6..35e07132c4a1 100644
--- a/drivers/iio/common/ssp_sensors/ssp_dev.c
+++ b/drivers/iio/common/ssp_sensors/ssp_dev.c
@@ -512,6 +512,17 @@ static int ssp_probe(struct spi_device *spi)
 
 	mutex_init(&data->comm_lock);
 
+	data->rx_buf_size = SSP_DATA_PACKET_SIZE;
+	data->rx_buf = devm_kzalloc(&spi->dev,
+				    data->rx_buf_size,
+				    GFP_KERNEL | GFP_DMA);
+
+	if (!data->rx_buf) {
+		dev_err(&spi->dev,
+			"Failed to allocate memory for rx_buf\n");
+		return -ENOMEM;
+	}
+
 	for (i = 0; i < SSP_SENSOR_MAX; ++i) {
 		data->delay_buf[i] = SSP_DEFAULT_POLLING_DELAY;
 		data->batch_latency_buf[i] = 0;
diff --git a/drivers/iio/common/ssp_sensors/ssp_spi.c b/drivers/iio/common/ssp_sensors/ssp_spi.c
index 61a4e978d9b2..08cf31fe9dd4 100644
--- a/drivers/iio/common/ssp_sensors/ssp_spi.c
+++ b/drivers/iio/common/ssp_sensors/ssp_spi.c
@@ -369,17 +369,13 @@ 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);
-			if (!buffer)
-				return -ENOMEM;
+			buffer = data->rx_buf;
 
 			/* got dead packet so it is always an error */
 			ret = spi_read(data->spi, buffer, length);
 			if (ret >= 0)
 				ret = -EPROTO;
 
-			kfree(buffer);
-
 			dev_err(SSP_DEV, "No match error %x\n",
 				msg_options);
 
@@ -411,22 +407,15 @@ int ssp_irq_msg(struct ssp_data *data)
 		break;
 	}
 	case SSP_HUB2AP_WRITE:
-		buffer = kzalloc(length, GFP_KERNEL | GFP_DMA);
-		if (!buffer)
-			return -ENOMEM;
+		buffer = data->rx_buf;
 
 		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

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

[Andy Shevchenko]

On Thu, Mar 26, 2026 at 01:48:14PM +0530, Sanjay Chitroda wrote:

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

NAK. Please, be very careful when do such changes.

...

>  	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);
> +		guard(mutex)(&data->pending_lock);
>  		list_add_tail(&msg->list, &data->pending_list);
> -		mutex_unlock(&data->pending_lock);
>  	}
>  
>  	status = ssp_check_lines(data, true);
>  	if (status < 0) {
>  		if (!use_no_irq) {
> -			mutex_lock(&data->pending_lock);
> +			guard(mutex)(&data->pending_lock);
>  			list_del(&msg->list);
> -			mutex_unlock(&data->pending_lock);
>  		}
> -		goto _error_locked;
> +		data->timeout_cnt++;
> +		return status;
>  	}

> -	mutex_unlock(&data->comm_lock);
> -

Pzzz! See what you are doing here...

>  	if (!use_no_irq && done)
>  		if (wait_for_completion_timeout(done,
>  						msecs_to_jiffies(timeout)) ==
>  		    0) {
> -			mutex_lock(&data->pending_lock);
> +			guard(mutex)(&data->pending_lock);
>  			list_del(&msg->list);
> -			mutex_unlock(&data->pending_lock);
>  
>  			data->timeout_cnt++;
>  			return -ETIMEDOUT;
>  		}
>  
>  	return 0;
> -
> -_error_locked:
> -	mutex_unlock(&data->comm_lock);
> -	data->timeout_cnt++;
> -	return status;
>  }

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 4/4] iio: ssp_sensors: reuse preallocated RX buffer for SPI transfers

[Andy Shevchenko]

On Thu, Mar 26, 2026 at 01:48:15PM +0530, Sanjay Chitroda wrote:

> Avoid allocating a temporary DMA buffer in the interrupt context when
> handling hub-to-AP and AP-to-hub SPI write messages.
> 
> Preallocate RX buffer during probe and reuse it for SPI receive
> operations. This removes repeated kzalloc() calls from the IRQ
> path, reduces allocation overhead, and avoids potential allocation
> failures under memory pressure.
> 
> The RX buffer size is tracked and allocated using devm_kzalloc(), ensuring
> proper lifetime management tied to the device.
> 
> No functional change intended; this is an internal optimization and
> robustness improvement.

NAK.

...

> @@ -512,6 +512,17 @@ static int ssp_probe(struct spi_device *spi)
>  
>  	mutex_init(&data->comm_lock);

Pzzz! The wrong use of managed vs. unmanaged resources.

> +	data->rx_buf_size = SSP_DATA_PACKET_SIZE;
> +	data->rx_buf = devm_kzalloc(&spi->dev,
> +				    data->rx_buf_size,
> +				    GFP_KERNEL | GFP_DMA);

There are plenty of room on the previous lines. I think I already commented on
something like this.

> +

This blank line is redundant.

> +	if (!data->rx_buf) {

> +		dev_err(&spi->dev,
> +			"Failed to allocate memory for rx_buf\n");

Pzzz!
Have you read about error messages for -ENOMEM? Please, take your time and
study this case.

> +		return -ENOMEM;
> +	}
> +
>  	for (i = 0; i < SSP_SENSOR_MAX; ++i) {
>  		data->delay_buf[i] = SSP_DEFAULT_POLLING_DELAY;
>  		data->batch_latency_buf[i] = 0;


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 4/4] iio: ssp_sensors: reuse preallocated RX buffer for SPI transfers

[Sanjay Chitroda]

On 26 March 2026 2:55:49 pm IST, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
>On Thu, Mar 26, 2026 at 01:48:15PM +0530, Sanjay Chitroda wrote:
>
>> Avoid allocating a temporary DMA buffer in the interrupt context when
>> handling hub-to-AP and AP-to-hub SPI write messages.
>> 
>> Preallocate RX buffer during probe and reuse it for SPI receive
>> operations. This removes repeated kzalloc() calls from the IRQ
>> path, reduces allocation overhead, and avoids potential allocation
>> failures under memory pressure.
>> 
>> The RX buffer size is tracked and allocated using devm_kzalloc(), ensuring
>> proper lifetime management tied to the device.
>> 
>> No functional change intended; this is an internal optimization and
>> robustness improvement.
>
>NAK.
>
>...
>
>> @@ -512,6 +512,17 @@ static int ssp_probe(struct spi_device *spi)
>>  
>>  	mutex_init(&data->comm_lock);
>
>Pzzz! The wrong use of managed vs. unmanaged resources.
>
>> +	data->rx_buf_size = SSP_DATA_PACKET_SIZE;
>> +	data->rx_buf = devm_kzalloc(&spi->dev,
>> +				    data->rx_buf_size,
>> +				    GFP_KERNEL | GFP_DMA);
>
>There are plenty of room on the previous lines. I think I already commented on
>something like this.
>
>> +
>
>This blank line is redundant.
>
>> +	if (!data->rx_buf) {
>
>> +		dev_err(&spi->dev,
>> +			"Failed to allocate memory for rx_buf\n");
>
>Pzzz!
>Have you read about error messages for -ENOMEM? Please, take your time and
>study this case.
>
>> +		return -ENOMEM;
>> +	}
>> +
>>  	for (i = 0; i < SSP_SENSOR_MAX; ++i) {
>>  		data->delay_buf[i] = SSP_DEFAULT_POLLING_DELAY;
>>  		data->batch_latency_buf[i] = 0;
>
>

Thanks Andy for the review.

I have addressed the comments related to this change and updated the allocation as follows:

data->rx_buf_size = SSP_DATA_PACKET_SIZE;
data->rx_buf = devm_kzalloc(&spi->dev, data->rx_buf_size, GFP_KERNEL);
if (!data->rx_buf)
    return -ENOMEM;

Regarding the managed vs unmanaged resource comment — since "struct ssp_data" is allocated using "devm_kzalloc()", I aligned "rx_buf" with the same lifetime.

For the IRQ handling and other resource management aspects, I will address those separately to keep this change focused.

Please let me know if you would prefer those fixes to be included as part of this patch.

I will include these updates in the next revision.

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

[Sanjay Chitroda]

On 26 March 2026 2:52:06 pm IST, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
>On Thu, Mar 26, 2026 at 01:48:14PM +0530, Sanjay Chitroda wrote:
>
>> Replace explicit mutex_lock() and mutex_unlock() with the guard() macro
>> for cleaner and safer mutex handling.
>
>NAK. Please, be very careful when do such changes.
>
>...
>
>>  	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);
>> +		guard(mutex)(&data->pending_lock);
>>  		list_add_tail(&msg->list, &data->pending_list);
>> -		mutex_unlock(&data->pending_lock);
>>  	}
>>  
>>  	status = ssp_check_lines(data, true);
>>  	if (status < 0) {
>>  		if (!use_no_irq) {
>> -			mutex_lock(&data->pending_lock);
>> +			guard(mutex)(&data->pending_lock);
>>  			list_del(&msg->list);
>> -			mutex_unlock(&data->pending_lock);
>>  		}
>> -		goto _error_locked;
>> +		data->timeout_cnt++;
>> +		return status;
>>  	}
>
>> -	mutex_unlock(&data->comm_lock);
>> -
>
>Pzzz! See what you are doing here...
>
>>  	if (!use_no_irq && done)
>>  		if (wait_for_completion_timeout(done,
>>  						msecs_to_jiffies(timeout)) ==
>>  		    0) {
>> -			mutex_lock(&data->pending_lock);
>> +			guard(mutex)(&data->pending_lock);
>>  			list_del(&msg->list);
>> -			mutex_unlock(&data->pending_lock);
>>  
>>  			data->timeout_cnt++;
>>  			return -ETIMEDOUT;
>>  		}
>>  
>>  	return 0;
>> -
>> -_error_locked:
>> -	mutex_unlock(&data->comm_lock);
>> -	data->timeout_cnt++;
>> -	return status;
>>  }
>

Thank Andy for pointing this out — you’re right, using "guard(mutex)" here unintentionally extends the lifetime of "comm_lock" and can hold it across the completion wait, which is not safe.

I’ve reworked the change to keep the original locking semantics intact:

- "comm_lock" is still explicitly unlocked before "wait_for_completion_timeout()"
- No sleeping paths are executed while holding "comm_lock"
- Introduced small helpers to simplify the flow:
  - "ssp_send_and_enqueue()" for SPI write + pending list add
  - "ssp_dequeue_msg()" for safe removal from the pending list

Updated flow looks like this:

mutex_lock(&data->comm_lock);

status = ssp_check_lines(data, false);
if (status < 0)
    goto err;

status = ssp_send_and_enqueue(data, msg, use_no_irq);
if (status < 0)
    goto err;

status = ssp_check_lines(data, true);
if (status < 0) {
    if (!use_no_irq)
        ssp_dequeue_msg(data, msg);
    goto err;
}

mutex_unlock(&data->comm_lock);

/* wait outside lock */
if (!use_no_irq && done) {
    if (wait_for_completion_timeout(done,
            msecs_to_jiffies(timeout)) == 0) {

        ssp_dequeue_msg(data, msg);
        data->timeout_cnt++;
        return -ETIMEDOUT;
    }
}

return 0;

err:
mutex_unlock(&data->comm_lock);
data->timeout_cnt++;
return status;

This keeps the synchronization boundary unchanged while reducing duplication around pending list handling. I’ve also limited "guard()" usage to short, local critical sections only.

Please let me know if you’d prefer keeping the list operations inline instead of helpers.