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@xxxxxxxxx> 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.