[v4] iio: ssp_sensors: improve resource cleanup with cleanup.h

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

Posted by Sanjay Chitroda 1 week ago

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 4/4] iio: ssp_sensors: reuse preallocated RX buffer for SPI transfers

Posted by Dan Carpenter 5 days, 14 hours ago

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

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

Posted by Andy Shevchenko 1 week ago

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

Posted by Sanjay Chitroda 5 days, 17 hours ago


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.

[PATCH v4 1/4] iio: ssp_sensors: cleanup codestyle warning
[PATCH v4 2/4] iio: ssp_sensors: cleanup codestyle check
[PATCH v4 3/4] iio: ssp_sensors: ssp_spi: use guard() to release mutexes
[PATCH v4 4/4] iio: ssp_sensors: reuse preallocated RX buffer for SPI transfers