[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.

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

[Dan Carpenter]

Hi Sanjay,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Sanjay-Chitroda/iio-ssp_sensors-cleanup-codestyle-warning/20260327-131514
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20260326081815.925373-5-sanjayembedded%40gmail.com
patch subject: [PATCH v4 4/4] iio: ssp_sensors: reuse preallocated RX buffer for SPI transfers
config: x86_64-randconfig-161-20260328 (https://download.01.org/0day-ci/archive/20260328/202603281105.UIsb0ZYk-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
smatch: v0.5.0-9004-gb810ac53

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202603281105.UIsb0ZYk-lkp@intel.com/

smatch warnings:
drivers/iio/common/ssp_sensors/ssp_dev.c:523 ssp_probe() warn: missing unwind goto?

vim +523 drivers/iio/common/ssp_sensors/ssp_dev.c

50dd64d57eee8ae Karol Wrona         2015-01-28  483  static int ssp_probe(struct spi_device *spi)
50dd64d57eee8ae Karol Wrona         2015-01-28  484  {
50dd64d57eee8ae Karol Wrona         2015-01-28  485  	int ret, i;
50dd64d57eee8ae Karol Wrona         2015-01-28  486  	struct ssp_data *data;
50dd64d57eee8ae Karol Wrona         2015-01-28  487  
50dd64d57eee8ae Karol Wrona         2015-01-28  488  	data = ssp_parse_dt(&spi->dev);
50dd64d57eee8ae Karol Wrona         2015-01-28  489  	if (!data) {
50dd64d57eee8ae Karol Wrona         2015-01-28  490  		dev_err(&spi->dev, "Failed to find platform data\n");
50dd64d57eee8ae Karol Wrona         2015-01-28  491  		return -ENODEV;
50dd64d57eee8ae Karol Wrona         2015-01-28  492  	}
50dd64d57eee8ae Karol Wrona         2015-01-28  493  
4c6e3dbc6b4877c Krzysztof Kozlowski 2020-09-21  494  	ret = mfd_add_devices(&spi->dev, PLATFORM_DEVID_NONE,
4c6e3dbc6b4877c Krzysztof Kozlowski 2020-09-21  495  			      sensorhub_sensor_devs,
50dd64d57eee8ae Karol Wrona         2015-01-28  496  			      ARRAY_SIZE(sensorhub_sensor_devs), NULL, 0, NULL);
50dd64d57eee8ae Karol Wrona         2015-01-28  497  	if (ret < 0) {
50dd64d57eee8ae Karol Wrona         2015-01-28  498  		dev_err(&spi->dev, "mfd add devices fail\n");
50dd64d57eee8ae Karol Wrona         2015-01-28  499  		return ret;
50dd64d57eee8ae Karol Wrona         2015-01-28  500  	}
50dd64d57eee8ae Karol Wrona         2015-01-28  501  
50dd64d57eee8ae Karol Wrona         2015-01-28  502  	spi->mode = SPI_MODE_1;
50dd64d57eee8ae Karol Wrona         2015-01-28  503  	ret = spi_setup(spi);
50dd64d57eee8ae Karol Wrona         2015-01-28  504  	if (ret < 0) {
50dd64d57eee8ae Karol Wrona         2015-01-28  505  		dev_err(&spi->dev, "Failed to setup spi\n");
21553258b94861a Christophe JAILLET  2025-10-10  506  		goto err_setup_spi;
50dd64d57eee8ae Karol Wrona         2015-01-28  507  	}
50dd64d57eee8ae Karol Wrona         2015-01-28  508  
50dd64d57eee8ae Karol Wrona         2015-01-28  509  	data->fw_dl_state = SSP_FW_DL_STATE_NONE;
50dd64d57eee8ae Karol Wrona         2015-01-28  510  	data->spi = spi;
50dd64d57eee8ae Karol Wrona         2015-01-28  511  	spi_set_drvdata(spi, data);
50dd64d57eee8ae Karol Wrona         2015-01-28  512  
50dd64d57eee8ae Karol Wrona         2015-01-28  513  	mutex_init(&data->comm_lock);
50dd64d57eee8ae Karol Wrona         2015-01-28  514  
5981e993e348918 Sanjay Chitroda     2026-03-26  515  	data->rx_buf_size = SSP_DATA_PACKET_SIZE;
5981e993e348918 Sanjay Chitroda     2026-03-26  516  	data->rx_buf = devm_kzalloc(&spi->dev,
5981e993e348918 Sanjay Chitroda     2026-03-26  517  				    data->rx_buf_size,
5981e993e348918 Sanjay Chitroda     2026-03-26  518  				    GFP_KERNEL | GFP_DMA);
5981e993e348918 Sanjay Chitroda     2026-03-26  519  
5981e993e348918 Sanjay Chitroda     2026-03-26  520  	if (!data->rx_buf) {
5981e993e348918 Sanjay Chitroda     2026-03-26  521  		dev_err(&spi->dev,
5981e993e348918 Sanjay Chitroda     2026-03-26  522  			"Failed to allocate memory for rx_buf\n");
5981e993e348918 Sanjay Chitroda     2026-03-26 @523  		return -ENOMEM;

Need to goto destroy_comm_lock before returning.

5981e993e348918 Sanjay Chitroda     2026-03-26  524  	}
5981e993e348918 Sanjay Chitroda     2026-03-26  525  
50dd64d57eee8ae Karol Wrona         2015-01-28  526  	for (i = 0; i < SSP_SENSOR_MAX; ++i) {
50dd64d57eee8ae Karol Wrona         2015-01-28  527  		data->delay_buf[i] = SSP_DEFAULT_POLLING_DELAY;
50dd64d57eee8ae Karol Wrona         2015-01-28  528  		data->batch_latency_buf[i] = 0;
50dd64d57eee8ae Karol Wrona         2015-01-28  529  		data->batch_opt_buf[i] = 0;
50dd64d57eee8ae Karol Wrona         2015-01-28  530  		data->check_status[i] = SSP_INITIALIZATION_STATE;
50dd64d57eee8ae Karol Wrona         2015-01-28  531  	}
50dd64d57eee8ae Karol Wrona         2015-01-28  532  
50dd64d57eee8ae Karol Wrona         2015-01-28  533  	data->delay_buf[SSP_BIO_HRM_LIB] = 100;
50dd64d57eee8ae Karol Wrona         2015-01-28  534  
50dd64d57eee8ae Karol Wrona         2015-01-28  535  	data->time_syncing = true;
50dd64d57eee8ae Karol Wrona         2015-01-28  536  
50dd64d57eee8ae Karol Wrona         2015-01-28  537  	mutex_init(&data->pending_lock);
50dd64d57eee8ae Karol Wrona         2015-01-28  538  	INIT_LIST_HEAD(&data->pending_list);
50dd64d57eee8ae Karol Wrona         2015-01-28  539  
50dd64d57eee8ae Karol Wrona         2015-01-28  540  	atomic_set(&data->enable_refcount, 0);
50dd64d57eee8ae Karol Wrona         2015-01-28  541  
50dd64d57eee8ae Karol Wrona         2015-01-28  542  	INIT_WORK(&data->work_wdt, ssp_wdt_work_func);
50dd64d57eee8ae Karol Wrona         2015-01-28  543  	INIT_DELAYED_WORK(&data->work_refresh, ssp_refresh_task);
50dd64d57eee8ae Karol Wrona         2015-01-28  544  
e99e88a9d2b0674 Kees Cook           2017-10-16  545  	timer_setup(&data->wdt_timer, ssp_wdt_timer_func, 0);
50dd64d57eee8ae Karol Wrona         2015-01-28  546  
50dd64d57eee8ae Karol Wrona         2015-01-28  547  	ret = request_threaded_irq(data->spi->irq, NULL,
50dd64d57eee8ae Karol Wrona         2015-01-28  548  				   ssp_irq_thread_fn,
50dd64d57eee8ae Karol Wrona         2015-01-28  549  				   IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
50dd64d57eee8ae Karol Wrona         2015-01-28  550  				   "SSP_Int", data);
50dd64d57eee8ae Karol Wrona         2015-01-28  551  	if (ret < 0) {
50dd64d57eee8ae Karol Wrona         2015-01-28  552  		dev_err(&spi->dev, "Irq request fail\n");
50dd64d57eee8ae Karol Wrona         2015-01-28  553  		goto err_setup_irq;
50dd64d57eee8ae Karol Wrona         2015-01-28  554  	}
50dd64d57eee8ae Karol Wrona         2015-01-28  555  
50dd64d57eee8ae Karol Wrona         2015-01-28  556  	/* Let's start with enabled one so irq balance could be ok */
50dd64d57eee8ae Karol Wrona         2015-01-28  557  	data->shut_down = false;
50dd64d57eee8ae Karol Wrona         2015-01-28  558  
50dd64d57eee8ae Karol Wrona         2015-01-28  559  	/* just to avoid unbalanced irq set wake up */
50dd64d57eee8ae Karol Wrona         2015-01-28  560  	enable_irq_wake(data->spi->irq);
50dd64d57eee8ae Karol Wrona         2015-01-28  561  
50dd64d57eee8ae Karol Wrona         2015-01-28  562  	data->fw_dl_state = ssp_check_fwbl(data);
50dd64d57eee8ae Karol Wrona         2015-01-28  563  	if (data->fw_dl_state == SSP_FW_DL_STATE_NONE) {
50dd64d57eee8ae Karol Wrona         2015-01-28  564  		ret = ssp_initialize_mcu(data);
50dd64d57eee8ae Karol Wrona         2015-01-28  565  		if (ret < 0) {
50dd64d57eee8ae Karol Wrona         2015-01-28  566  			dev_err(&spi->dev, "Initialize_mcu failed\n");
50dd64d57eee8ae Karol Wrona         2015-01-28  567  			goto err_read_reg;
50dd64d57eee8ae Karol Wrona         2015-01-28  568  		}
50dd64d57eee8ae Karol Wrona         2015-01-28  569  	} else {
50dd64d57eee8ae Karol Wrona         2015-01-28  570  		dev_err(&spi->dev, "Firmware version not supported\n");
50dd64d57eee8ae Karol Wrona         2015-01-28  571  		ret = -EPERM;
50dd64d57eee8ae Karol Wrona         2015-01-28  572  		goto err_read_reg;
50dd64d57eee8ae Karol Wrona         2015-01-28  573  	}
50dd64d57eee8ae Karol Wrona         2015-01-28  574  
50dd64d57eee8ae Karol Wrona         2015-01-28  575  	return 0;
50dd64d57eee8ae Karol Wrona         2015-01-28  576  
50dd64d57eee8ae Karol Wrona         2015-01-28  577  err_read_reg:
50dd64d57eee8ae Karol Wrona         2015-01-28  578  	free_irq(data->spi->irq, data);
50dd64d57eee8ae Karol Wrona         2015-01-28  579  err_setup_irq:
50dd64d57eee8ae Karol Wrona         2015-01-28  580  	mutex_destroy(&data->pending_lock);

Add a destroy_comm_lock: label here.

50dd64d57eee8ae Karol Wrona         2015-01-28  581  	mutex_destroy(&data->comm_lock);
21553258b94861a Christophe JAILLET  2025-10-10  582  err_setup_spi:
21553258b94861a Christophe JAILLET  2025-10-10  583  	mfd_remove_devices(&spi->dev);
50dd64d57eee8ae Karol Wrona         2015-01-28  584  
50dd64d57eee8ae Karol Wrona         2015-01-28  585  	dev_err(&spi->dev, "Probe failed!\n");
50dd64d57eee8ae Karol Wrona         2015-01-28  586  
50dd64d57eee8ae Karol Wrona         2015-01-28  587  	return ret;
50dd64d57eee8ae Karol Wrona         2015-01-28  588  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki