Re: [PATCH v5 3/5] iio: ssp_sensors: factor out pending list add/remove helpers

[Sanjay Chitroda]

On 7 April 2026 1:34:41 am IST, Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> wrote:
>On Mon, Apr 06, 2026 at 01:38:50PM +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 to avoid duplication and simplify transfer flow to follow.
>> 
>> No functional change intended.
>
>Suggested-by?
>

Requesting your input on which email is recommended to tag with your name.

Shall I use intel.com ?

>...
>
>> +static inline void ssp_pending_add(struct ssp_data *data,
>> +				   struct ssp_msg *msg)
>
>Make it rather:
>
>static inline void ssp_pending_add(struct ssp_data *data, struct ssp_msg *msg)
>{
>	if (msg->length)
>		return;
>	...
>}

It should be not, right ?

{
if (!msg->length)
        return;
...
}

 const bool use_no_irq = msg->length == 0;
 if (!use_no_irq) {add/del}

If message length is 0 then no interrupt

>
>> +{
>> +	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)
>> +{
>> +	mutex_lock(&data->pending_lock);
>> +	list_del(&msg->list);
>> +	mutex_unlock(&data->pending_lock);
>> +}
>
>(in the same manner)
>
>...
>
>> -	if (!use_no_irq) {
>> -		mutex_lock(&data->pending_lock);
>> -		list_add_tail(&msg->list, &data->pending_list);
>> -		mutex_unlock(&data->pending_lock);
>> -	}
>> +	if (!use_no_irq)
>> +		ssp_pending_add(data, msg);
>
>...and then it will become
>
>	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);
>> -		}
>> +		if (!use_no_irq)
>> +			ssp_pending_del(data, msg);
>>  		goto _error_locked;
>>  	}
>
>...
>
>>  		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);
>> +			ssp_pending_del(data, msg);
>>  
>>  			data->timeout_cnt++;
>>  			return -ETIMEDOUT;
>
>Also rewrite this to follow the style (missing {}, better indentation):
>
>	if (msg->length && done &&
>	    !wait_for_completion_timeout(done, msecs_to_jiffies(timeout))) {
>		ssp_pending_del(data, msg);
>
>		data->timeout_cnt++;
>		return -ETIMEDOUT;
>	}
>
>And kill that use_no_urq altogether. Move the comment rather to a helper(s).

Understood, will follow and drop this variable in next series.

>