[PATCH v6 0/7] iio: ssp_sensors: improve resource cleanup

[PATCH v6 0/7] iio: ssp_sensors: improve resource cleanup

[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 embedded rx buffer for SPI transfer instead of allocating new
  DMA memory in interrupt context for write transaction between AP <-> HUB.
- Replace dynamic resource lifetime management with device‑managed
  (devm_) APIs to simplify cleanup and error paths.
- Refactor common helper API, improving readability and reducing
  duplicatation.
- Address minor codestyle warnings and use dev_err_probe in probe path.

Changes in v6:
- 0005: Drop remove() path using devm action with input from David
- 0007: Replace dynamic memory allocation with embedded fixed size struct buffer
- 0004: new: drop duplication in remove()
- 0006: new: Use dev_err_probe in driver probe()
Changes in v5:
- Drop usage of guard() helpers to avoid mixing mutex_lock() with
  guard()(), based on reviewer feedback.
- 0003: Refactor shared helper API, reducing duplication.
- 0004: Convert resource allocation and teardown to devm_ managed APIs to
  simplify error handling and probe/remove paths.
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 (7):
  iio: ssp_sensors: cleanup codestyle warning
  iio: ssp_sensors: cleanup codestyle check
  iio: ssp_sensors: factor out pending list add/remove helper(s)
  iio: ssp_sensors: drop duplicated wdt timer and work cleanup
  iio: ssp_sensors: convert probe and teardown to devm-managed resources
  iio: ssp_sensors: Use dev_err_probe
  iio: ssp_sensors: reuse embedded RX buffer for SPI transfers

 drivers/iio/common/ssp_sensors/ssp.h     |   3 +
 drivers/iio/common/ssp_sensors/ssp_dev.c | 119 ++++++++++-------------
 drivers/iio/common/ssp_sensors/ssp_spi.c |  83 +++++++---------
 3 files changed, 88 insertions(+), 117 deletions(-)

-- 
2.34.1

[PATCH v6 1/7] 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>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.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 v6 2/7] 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

Re: [PATCH v6 2/7] iio: ssp_sensors: cleanup codestyle check

[Andy Shevchenko]

On Wed, Apr 15, 2026 at 10:37:44AM +0530, Sanjay Chitroda wrote:

> Reported by checkpatch:
> FILE: drivers/iio/common/ssp_sensors/ssp_spi.c
> 
> CHECK: Macro argument '...' may be better as '(...)'
> to avoid precedence issues

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko

[PATCH v6 3/7] iio: ssp_sensors: factor out pending list add/remove helper(s)

[Sanjay Chitroda]

From: Sanjay Chitroda <sanjayembeddedse@gmail.com>

The SSP SPI transfer path manipulates the pending message list in
multiple places, each time open-coding the same locking and list
operations.

Re-factor the pending list add and delete logic into small helper
functions and drop use_no_irq variable to avoid duplication and
simplify transfer flow to follow.

No functional change intended.

Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
Changes in v6:
- Include tag for the suggestion of helper functions
- Drop completely use_no_irq variable with review comment from Andy
- Link to v5: https://lore.kernel.org/all/20260406080852.2727453-1-sanjayembedded@gmail.com/
---
 drivers/iio/common/ssp_sensors/ssp_spi.c | 55 +++++++++++++-----------
 1 file changed, 30 insertions(+), 25 deletions(-)

diff --git a/drivers/iio/common/ssp_sensors/ssp_spi.c b/drivers/iio/common/ssp_sensors/ssp_spi.c
index 08ed92859be0..92418721ff82 100644
--- a/drivers/iio/common/ssp_sensors/ssp_spi.c
+++ b/drivers/iio/common/ssp_sensors/ssp_spi.c
@@ -174,15 +174,32 @@ static int ssp_check_lines(struct ssp_data *data, bool state)
 	return 0;
 }
 
+static inline void ssp_pending_add(struct ssp_data *data,
+				   struct ssp_msg *msg)
+{
+	if (msg->length == 0)
+		return;
+
+	mutex_lock(&data->pending_lock);
+	list_add_tail(&msg->list, &data->pending_list);
+	mutex_unlock(&data->pending_lock);
+}
+
+static inline void ssp_pending_del(struct ssp_data *data,
+				   struct ssp_msg *msg)
+{
+	if (msg->length == 0)
+		return;
+
+	mutex_lock(&data->pending_lock);
+	list_del(&msg->list);
+	mutex_unlock(&data->pending_lock);
+}
+
 static int ssp_do_transfer(struct ssp_data *data, struct ssp_msg *msg,
 			   struct completion *done, int timeout)
 {
 	int status;
-	/*
-	 * check if this is a short one way message or the whole transfer has
-	 * second part after an interrupt
-	 */
-	const bool use_no_irq = msg->length == 0;
 
 	if (data->shut_down)
 		return -EPERM;
@@ -202,35 +219,23 @@ static int ssp_do_transfer(struct ssp_data *data, struct ssp_msg *msg,
 		goto _error_locked;
 	}
 
-	if (!use_no_irq) {
-		mutex_lock(&data->pending_lock);
-		list_add_tail(&msg->list, &data->pending_list);
-		mutex_unlock(&data->pending_lock);
-	}
+	ssp_pending_add(data, msg);
 
 	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);
-		}
+		ssp_pending_del(data, msg);
 		goto _error_locked;
 	}
 
 	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);
+	if (msg->length && done &&
+	    !wait_for_completion_timeout(done, msecs_to_jiffies(timeout))) {
+		ssp_pending_del(data, msg);
 
-			data->timeout_cnt++;
-			return -ETIMEDOUT;
-		}
+		data->timeout_cnt++;
+		return -ETIMEDOUT;
+	}
 
 	return 0;
 
-- 
2.34.1

Re: [PATCH v6 3/7] iio: ssp_sensors: factor out pending list add/remove helper(s)

[Andy Shevchenko]

On Wed, Apr 15, 2026 at 10:37:45AM +0530, Sanjay Chitroda wrote:

> The SSP SPI transfer path manipulates the pending message list in
> multiple places, each time open-coding the same locking and list
> operations.
> 
> Re-factor the pending list add and delete logic into small helper
> functions and drop use_no_irq variable to avoid duplication and
> simplify transfer flow to follow.
> 
> No functional change intended.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
after addressing below nit-picks.

...

> +static inline void ssp_pending_add(struct ssp_data *data,
> +				   struct ssp_msg *msg)

It's perfectly a single line (we have limit 80, so up to and including 80 is
allowed).

static inline void ssp_pending_add(struct ssp_data *data, struct ssp_msg *msg)

(it's only 78 here).

...

> +static inline void ssp_pending_del(struct ssp_data *data,
> +				   struct ssp_msg *msg)

Ditto.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 3/7] iio: ssp_sensors: factor out pending list add/remove helper(s)

[Andy Shevchenko]

On Wed, Apr 15, 2026 at 10:37:45AM +0530, Sanjay Chitroda wrote:

> The SSP SPI transfer path manipulates the pending message list in
> multiple places, each time open-coding the same locking and list
> operations.
> 
> Re-factor the pending list add and delete logic into small helper
> functions and drop use_no_irq variable to avoid duplication and
> simplify transfer flow to follow.
> 
> No functional change intended.

...

> -	/*
> -	 * check if this is a short one way message or the whole transfer has
> -	 * second part after an interrupt
> -	 */

One more thing, please, move this comment to the same condition in ssp_pending_add().
The ssp_pending_del() can survive without it as they are close to each other. Or you
can add a short one to refer to the comment in ssp_pending_add().

While moving, fix the style as well.

	/*
	 * Check if this is a short one way message or the whole transfer has
	 * second part after an interrupt.
	 */

-- 
With Best Regards,
Andy Shevchenko

[PATCH v6 4/7] iio: ssp_sensors: drop duplicated wdt timer and work cleanup

[Sanjay Chitroda]

From: Sanjay Chitroda <sanjayembeddedse@gmail.com>

The SSP remove path cleans up the watchdog timer and associated work
both via ssp_disable_wdt_timer() and through explicit timer and work
teardown.

Remove the duplicated timer_delete_sync() and cancel_work_sync()
calls and rely on ssp_disable_wdt_timer() as the single cleanup helper
to simplify the remove path.

No functional change intended.

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

diff --git a/drivers/iio/common/ssp_sensors/ssp_dev.c b/drivers/iio/common/ssp_sensors/ssp_dev.c
index da09c9f3ceb6..cb4c26a6b76c 100644
--- a/drivers/iio/common/ssp_sensors/ssp_dev.c
+++ b/drivers/iio/common/ssp_sensors/ssp_dev.c
@@ -585,14 +585,11 @@ static void ssp_remove(struct spi_device *spi)
 			"SSP_MSG2SSP_AP_STATUS_SHUTDOWN failed\n");
 
 	ssp_enable_mcu(data, false);
-	ssp_disable_wdt_timer(data);
-
 	ssp_clean_pending_list(data);
 
 	free_irq(data->spi->irq, data);
 
-	timer_delete_sync(&data->wdt_timer);
-	cancel_work_sync(&data->work_wdt);
+	ssp_disable_wdt_timer(data);
 
 	mutex_destroy(&data->comm_lock);
 	mutex_destroy(&data->pending_lock);
-- 
2.34.1

Re: [PATCH v6 4/7] iio: ssp_sensors: drop duplicated wdt timer and work cleanup

[Andy Shevchenko]

On Wed, Apr 15, 2026 at 10:37:46AM +0530, Sanjay Chitroda wrote:

> The SSP remove path cleans up the watchdog timer and associated work
> both via ssp_disable_wdt_timer() and through explicit timer and work
> teardown.
> 
> Remove the duplicated timer_delete_sync() and cancel_work_sync()
> calls and rely on ssp_disable_wdt_timer() as the single cleanup helper
> to simplify the remove path.
> 
> No functional change intended.

...

>  	ssp_enable_mcu(data, false);
> -	ssp_disable_wdt_timer(data);
> -
>  	ssp_clean_pending_list(data);
>  
>  	free_irq(data->spi->irq, data);
>  
> -	timer_delete_sync(&data->wdt_timer);
> -	cancel_work_sync(&data->work_wdt);
> +	ssp_disable_wdt_timer(data);

Hmm... I'm not sure it doesn't change the behaviour. First of all, the
difference is that IRQ handler may be involved in the timer and/or work.

This patch needs much better investigation conducted and commit message
provided with explanation of all possible race conditions and why they
don't happen. Also I am not sure if this dup is a mistake or deliberate
action (again, it's all related to the timings of IRQ handler, timer job,
and work and how they are relies on each other data/flow).


-- 
With Best Regards,
Andy Shevchenko

[PATCH v6 5/7] iio: ssp_sensors: convert probe and teardown to devm-managed resources

[Sanjay Chitroda]

From: Sanjay Chitroda <sanjayembeddedse@gmail.com>

Convert SSP driver resource management to use devm-managed helpers
and devm actions, tying the lifetime of all resources to the device.

Mutex initialization, IRQ registration, work items, timers, and MFD
resource are now managed using devm APIs. Cleanup logic previously
handled explicitly in probe error paths and the remove callback is
replaced with devm_add_action_or_reset(), ensuring correct teardown
ordering and consistent behaviour on probe failure and device unbind.

This simplifies the probe path by removing goto-based error handling,
eliminates the remove callback entirely.

No functional change intended.

Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
Changes in v6:
- Drop remove path completely using devm_action with review comment from David
- Convert MFD device to manage resource along with mutex and IRQ
- Link to v5: https://lore.kernel.org/all/20260406080852.2727453-1-sanjayembedded@gmail.com/
---

 drivers/iio/common/ssp_sensors/ssp_dev.c | 88 ++++++++++--------------
 1 file changed, 38 insertions(+), 50 deletions(-)

diff --git a/drivers/iio/common/ssp_sensors/ssp_dev.c b/drivers/iio/common/ssp_sensors/ssp_dev.c
index cb4c26a6b76c..94ecc794f926 100644
--- a/drivers/iio/common/ssp_sensors/ssp_dev.c
+++ b/drivers/iio/common/ssp_sensors/ssp_dev.c
@@ -194,6 +194,16 @@ static void ssp_disable_wdt_timer(struct ssp_data *data)
 	cancel_work_sync(&data->work_wdt);
 }
 
+static void ssp_shutdown_action(struct ssp_data *data)
+{
+	if (ssp_command(data, SSP_MSG2SSP_AP_STATUS_SHUTDOWN, 0) < 0)
+		dev_err(&data->spi->dev,
+			"SSP_MSG2SSP_AP_STATUS_SHUTDOWN failed\n");
+
+	ssp_enable_mcu(data, false);
+	ssp_clean_pending_list(data);
+}
+
 /**
  * ssp_get_sensor_delay() - gets sensor data acquisition period
  * @data:	sensorhub structure
@@ -491,9 +501,9 @@ static int ssp_probe(struct spi_device *spi)
 		return -ENODEV;
 	}
 
-	ret = mfd_add_devices(&spi->dev, PLATFORM_DEVID_NONE,
-			      sensorhub_sensor_devs,
-			      ARRAY_SIZE(sensorhub_sensor_devs), NULL, 0, NULL);
+	ret = devm_mfd_add_devices(&spi->dev, PLATFORM_DEVID_NONE,
+				   sensorhub_sensor_devs,
+				   ARRAY_SIZE(sensorhub_sensor_devs), NULL, 0, NULL);
 	if (ret < 0) {
 		dev_err(&spi->dev, "mfd add devices fail\n");
 		return ret;
@@ -503,14 +513,16 @@ static int ssp_probe(struct spi_device *spi)
 	ret = spi_setup(spi);
 	if (ret < 0) {
 		dev_err(&spi->dev, "Failed to setup spi\n");
-		goto err_setup_spi;
+		return ret;
 	}
 
 	data->fw_dl_state = SSP_FW_DL_STATE_NONE;
 	data->spi = spi;
 	spi_set_drvdata(spi, data);
 
-	mutex_init(&data->comm_lock);
+	ret = devm_mutex_init(&spi->dev, &data->comm_lock);
+	if (ret < 0)
+		return ret;
 
 	for (i = 0; i < SSP_SENSOR_MAX; ++i) {
 		data->delay_buf[i] = SSP_DEFAULT_POLLING_DELAY;
@@ -523,7 +535,9 @@ static int ssp_probe(struct spi_device *spi)
 
 	data->time_syncing = true;
 
-	mutex_init(&data->pending_lock);
+	ret = devm_mutex_init(&spi->dev, &data->pending_lock);
+	if (ret < 0)
+		return ret;
 	INIT_LIST_HEAD(&data->pending_list);
 
 	atomic_set(&data->enable_refcount, 0);
@@ -533,14 +547,17 @@ static int ssp_probe(struct spi_device *spi)
 
 	timer_setup(&data->wdt_timer, ssp_wdt_timer_func, 0);
 
-	ret = request_threaded_irq(data->spi->irq, NULL,
-				   ssp_irq_thread_fn,
-				   IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
-				   "SSP_Int", data);
-	if (ret < 0) {
-		dev_err(&spi->dev, "Irq request fail\n");
-		goto err_setup_irq;
-	}
+	ret = devm_add_action_or_reset(&spi->dev,
+				       ssp_disable_wdt_timer, data);
+	if (ret)
+		return ret;
+
+	ret = devm_request_threaded_irq(&spi->dev, data->spi->irq, NULL,
+					ssp_irq_thread_fn,
+					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					"SSP_Int", data);
+	if (ret < 0)
+		return ret;
 
 	/* Let's start with enabled one so irq balance could be ok */
 	data->shut_down = false;
@@ -548,53 +565,24 @@ static int ssp_probe(struct spi_device *spi)
 	/* just to avoid unbalanced irq set wake up */
 	enable_irq_wake(data->spi->irq);
 
+	ret = devm_add_action_or_reset(&spi->dev,
+				       ssp_shutdown_action, data);
+	if (ret)
+		return ret;
+
 	data->fw_dl_state = ssp_check_fwbl(data);
 	if (data->fw_dl_state == SSP_FW_DL_STATE_NONE) {
 		ret = ssp_initialize_mcu(data);
 		if (ret < 0) {
 			dev_err(&spi->dev, "Initialize_mcu failed\n");
-			goto err_read_reg;
+			return ret;
 		}
 	} else {
 		dev_err(&spi->dev, "Firmware version not supported\n");
-		ret = -EPERM;
-		goto err_read_reg;
+		return -EPERM;
 	}
 
 	return 0;
-
-err_read_reg:
-	free_irq(data->spi->irq, data);
-err_setup_irq:
-	mutex_destroy(&data->pending_lock);
-	mutex_destroy(&data->comm_lock);
-err_setup_spi:
-	mfd_remove_devices(&spi->dev);
-
-	dev_err(&spi->dev, "Probe failed!\n");
-
-	return ret;
-}
-
-static void ssp_remove(struct spi_device *spi)
-{
-	struct ssp_data *data = spi_get_drvdata(spi);
-
-	if (ssp_command(data, SSP_MSG2SSP_AP_STATUS_SHUTDOWN, 0) < 0)
-		dev_err(&data->spi->dev,
-			"SSP_MSG2SSP_AP_STATUS_SHUTDOWN failed\n");
-
-	ssp_enable_mcu(data, false);
-	ssp_clean_pending_list(data);
-
-	free_irq(data->spi->irq, data);
-
-	ssp_disable_wdt_timer(data);
-
-	mutex_destroy(&data->comm_lock);
-	mutex_destroy(&data->pending_lock);
-
-	mfd_remove_devices(&spi->dev);
 }
 
 static int ssp_suspend(struct device *dev)
-- 
2.34.1

Re: [PATCH v6 5/7] iio: ssp_sensors: convert probe and teardown to devm-managed resources

[Andy Shevchenko]

On Wed, Apr 15, 2026 at 10:37:47AM +0530, Sanjay Chitroda wrote:

> Convert SSP driver resource management to use devm-managed helpers
> and devm actions, tying the lifetime of all resources to the device.
> 
> Mutex initialization, IRQ registration, work items, timers, and MFD
> resource are now managed using devm APIs. Cleanup logic previously
> handled explicitly in probe error paths and the remove callback is
> replaced with devm_add_action_or_reset(), ensuring correct teardown
> ordering and consistent behaviour on probe failure and device unbind.
> 
> This simplifies the probe path by removing goto-based error handling,
> eliminates the remove callback entirely.
> 
> No functional change intended.

...

> +	ret = devm_add_action_or_reset(&spi->dev,
> +				       ssp_disable_wdt_timer, data);

It's perfectly a single line of 79 characters (we allow 80 in strict mode and
100 in relaxed and you seems stick with the former, which is fine).

	ret = devm_add_action_or_reset(&spi->dev, ssp_disable_wdt_timer, data);

...

> +	ret = devm_add_action_or_reset(&spi->dev,
> +				       ssp_shutdown_action, data);

Ditto.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v6 6/7] iio: ssp_sensors: Use dev_err_probe

[Sanjay Chitroda]

From: Sanjay Chitroda <sanjayembeddedse@gmail.com>

dev_err_probe() makes error code handling simpler and handle
deferred probe nicely (avoid spamming logs).

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

diff --git a/drivers/iio/common/ssp_sensors/ssp_dev.c b/drivers/iio/common/ssp_sensors/ssp_dev.c
index 94ecc794f926..0a00ead3195d 100644
--- a/drivers/iio/common/ssp_sensors/ssp_dev.c
+++ b/drivers/iio/common/ssp_sensors/ssp_dev.c
@@ -496,25 +496,21 @@ static int ssp_probe(struct spi_device *spi)
 	struct ssp_data *data;
 
 	data = ssp_parse_dt(&spi->dev);
-	if (!data) {
-		dev_err(&spi->dev, "Failed to find platform data\n");
-		return -ENODEV;
-	}
+	if (!data)
+		return dev_err_probe(&spi->dev, -ENODEV,
+				     "Failed to find platform data\n");
 
 	ret = devm_mfd_add_devices(&spi->dev, PLATFORM_DEVID_NONE,
 				   sensorhub_sensor_devs,
-				   ARRAY_SIZE(sensorhub_sensor_devs), NULL, 0, NULL);
-	if (ret < 0) {
-		dev_err(&spi->dev, "mfd add devices fail\n");
-		return ret;
-	}
+				   ARRAY_SIZE(sensorhub_sensor_devs),
+				   NULL, 0, NULL);
+	if (ret < 0)
+		return dev_err_probe(&spi->dev, ret, "mfd add devices fail\n");
 
 	spi->mode = SPI_MODE_1;
 	ret = spi_setup(spi);
-	if (ret < 0) {
-		dev_err(&spi->dev, "Failed to setup spi\n");
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(&spi->dev, ret, "Failed to setup spi\n");
 
 	data->fw_dl_state = SSP_FW_DL_STATE_NONE;
 	data->spi = spi;
@@ -573,14 +569,12 @@ static int ssp_probe(struct spi_device *spi)
 	data->fw_dl_state = ssp_check_fwbl(data);
 	if (data->fw_dl_state == SSP_FW_DL_STATE_NONE) {
 		ret = ssp_initialize_mcu(data);
-		if (ret < 0) {
-			dev_err(&spi->dev, "Initialize_mcu failed\n");
-			return ret;
-		}
-	} else {
-		dev_err(&spi->dev, "Firmware version not supported\n");
-		return -EPERM;
-	}
+		if (ret < 0)
+			return dev_err_probe(&spi->dev, ret,
+					     "Initialize_mcu failed\n");
+	} else
+		return dev_err_probe(&spi->dev, -EPERM,
+				     "Firmware version not supported\n");
 
 	return 0;
 }
-- 
2.34.1

Re: [PATCH v6 6/7] iio: ssp_sensors: Use dev_err_probe

[Andy Shevchenko]

On Wed, Apr 15, 2026 at 10:37:48AM +0530, Sanjay Chitroda wrote:

> dev_err_probe() makes error code handling simpler and handle
> deferred probe nicely (avoid spamming logs).

...

>  	struct ssp_data *data;

While at it, introduce a temporary variable

	struct device *dev = &spi->dev;

and reuse it below. Note, you may add a new patch to reuse it in the lines that
are not related to this patch but may utilise the local variable as well.

Actually, you can introduce it earlier in the series, exempli gratia when you
moved probe to use managed resources.

>  	data = ssp_parse_dt(&spi->dev);
> -	if (!data) {
> -		dev_err(&spi->dev, "Failed to find platform data\n");
> -		return -ENODEV;
> -	}
> +	if (!data)
> +		return dev_err_probe(&spi->dev, -ENODEV,
> +				     "Failed to find platform data\n");

		return dev_err_probe(dev, -ENODEV, "Failed to find platform data\n");

(And you can leave it one line as trailing string literals are fine to be
 longer 80 even in strict mode. You may even double check with checkpatch,
 it won't complain.)

...and so on...

...

>  	if (data->fw_dl_state == SSP_FW_DL_STATE_NONE) {
>  		ret = ssp_initialize_mcu(data);
> -		if (ret < 0) {
> -			dev_err(&spi->dev, "Initialize_mcu failed\n");
> -			return ret;
> -		}
> -	} else {
> -		dev_err(&spi->dev, "Firmware version not supported\n");
> -		return -EPERM;
> -	}
> +		if (ret < 0)
> +			return dev_err_probe(&spi->dev, ret,
> +					     "Initialize_mcu failed\n");
> +	} else
> +		return dev_err_probe(&spi->dev, -EPERM,
> +				     "Firmware version not supported\n");

I would now negate the conditional and drop redundant 'else'. Also note
the missing {} in the 'else' branch as per Coding Style.

	if (data->fw_dl_state != SSP_FW_DL_STATE_NONE)
		return dev_err_probe(dev, -EPERM, "Firmware version not supported\n");

	ret = ssp_initialize_mcu(data);
	if (ret < 0)
		return dev_err_probe(dev, ret, "Initialize_mcu failed\n");

-- 
With Best Regards,
Andy Shevchenko

[PATCH v6 7/7] iio: ssp_sensors: reuse embedded 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.

Replace the dynamically allocated RX buffer with a fixed-size,
preallocated buffer embedded in the driver structure and reused for all
SPI receive operations. This removes memory allocation from the
IRQ path, simplifies lifetime management.

No functional change intended.

Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
Changes in v6:
- Replace dynamically allocated RX buffer with embedded fixed-size buffer
- Fix struct layout to satisfy DMA alignment constraints comment from David
- Link to v5: https://lore.kernel.org/all/20260406080852.2727453-1-sanjayembedded@gmail.com/
Changes in v5:
- Rebase change on top of latest v5 patch series.
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     |  3 +++
 drivers/iio/common/ssp_sensors/ssp_spi.c | 20 ++------------------
 2 files changed, 5 insertions(+), 18 deletions(-)

diff --git a/drivers/iio/common/ssp_sensors/ssp.h b/drivers/iio/common/ssp_sensors/ssp.h
index f649cdecc277..8295bb7062a3 100644
--- a/drivers/iio/common/ssp_sensors/ssp.h
+++ b/drivers/iio/common/ssp_sensors/ssp.h
@@ -174,6 +174,7 @@ struct ssp_sensorhub_info {
  * @pending_list:	pending list for messages queued to be sent/read
  * @sensor_devs:	registered IIO devices table
  * @enable_refcount:	enable reference count for wdt (watchdog timer)
+ * @rx_buf:		buffer to receive SPI data
  * @header_buffer:	cache aligned buffer for packet header
  */
 struct ssp_data {
@@ -221,6 +222,8 @@ struct ssp_data {
 	struct iio_dev *sensor_devs[SSP_SENSOR_MAX];
 	atomic_t enable_refcount;
 
+	u8 rx_buf[SSP_DATA_PACKET_SIZE];
+
 	__le16 header_buffer[SSP_HEADER_BUFFER_SIZE / sizeof(__le16)] __aligned(IIO_DMA_MINALIGN);
 };
 
diff --git a/drivers/iio/common/ssp_sensors/ssp_spi.c b/drivers/iio/common/ssp_sensors/ssp_spi.c
index 92418721ff82..bab84b25edff 100644
--- a/drivers/iio/common/ssp_sensors/ssp_spi.c
+++ b/drivers/iio/common/ssp_sensors/ssp_spi.c
@@ -336,7 +336,7 @@ 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;
+	char *buffer = data->rx_buf;
 	u8 msg_type;
 	int ret;
 	u16 length, msg_options;
@@ -380,19 +380,12 @@ 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) {
-				ret = -ENOMEM;
-				goto _unlock;
-			}
 
 			/* 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);
 
@@ -425,22 +418,13 @@ int ssp_irq_msg(struct ssp_data *data)
 		mutex_unlock(&data->pending_lock);
 		break;
 	case SSP_HUB2AP_WRITE:
-		buffer = 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