Re: [PATCH v4 3/4] iio: ssp_sensors: ssp_spi: use guard() to release mutexes

[Sanjay Chitroda]

On 29 March 2026 5:14:38 pm IST, Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> wrote:
>On Sat, Mar 28, 2026 at 12:24:11PM +0530, Sanjay Chitroda wrote:
>> On 26 March 2026 2:52:06 pm IST, Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> wrote:
>> >On Thu, Mar 26, 2026 at 01:48:14PM +0530, Sanjay Chitroda wrote:
>
>> >> Replace explicit mutex_lock() and mutex_unlock() with the guard() macro
>> >> for cleaner and safer mutex handling.
>> >
>> >NAK. Please, be very careful when do such changes.
>
>...
>
>> >> -	mutex_unlock(&data->comm_lock);
>> >> -
>> >
>> >Pzzz! See what you are doing here...
>> 
>> Thank Andy for pointing this out — you’re right, using "guard(mutex)" here
>> unintentionally extends the lifetime of "comm_lock" and can hold it across
>> the completion wait, which is not safe.
>> 
>> I’ve reworked the change to keep the original locking semantics intact:
>> 
>> - "comm_lock" is still explicitly unlocked before "wait_for_completion_timeout()"
>> - No sleeping paths are executed while holding "comm_lock"
>> - Introduced small helpers to simplify the flow:
>>   - "ssp_send_and_enqueue()" for SPI write + pending list add
>>   - "ssp_dequeue_msg()" for safe removal from the pending list
>> 
>> Updated flow looks like this:
>
>If you are going to mix mutex_lock() with guard()(), it's even more NAKish
>solution (id est worse than no change).
>

Thanks — agreed on not mixing "mutex_lock()"/"mutex_unlock()" with "guard()".

This patch is intended as a cleanup to switch to "guard()", not to change locking semantics.

I’ll rework this so that it’s a consistent conversion:

- convert it fully to "guard()" without changing the lock scope (e.g., by restructuring the critical section appropriately or limiting scope with {} ).

Will update the patch accordingly.


>> This keeps the synchronization boundary unchanged while reducing duplication
>> around pending list handling. I’ve also limited "guard()" usage to short,
>> local critical sections only.
>> 
>> Please let me know if you’d prefer keeping the list operations inline instead
>> of helpers.
>