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

Sanjay Chitroda
2026-04-08T08:29:56Z

On 7 April 2026 1:34:41 am IST, Andy Shevchenko <andriy.shevchenko@intel.com> 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.

>