Re: [PATCH v3 0/5] soundwire: Fixes for spurious and missing UNATTACH

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

On 9/14/22 14:09, Richard Fitzgerald wrote:
> The bus and cadence code has several bugs that cause UNATTACH notifications
> to either be sent spuriously or to be missed.
> 
> These can be seen occasionally with a single peripheral on the bus, but are
> much more frequent with multiple peripherals, where several peripherals
> could change state and report in consecutive PINGs.
> 
> The root of all of these bugs seems to be a code design flaw that assumed
> every PING status change would be handled separately. However, PINGs are
> handled by a workqueue function and there is no guarantee when that function
> will be scheduled to run or how much CPU time it will receive. PINGs will
> continue while the work function is handling a snapshot of a previous PING
> so the code must take account that (a) status could change during the
> work function and (b) there can be a backlog of changes before the IRQ work
> function runs again.
> 
> Tested with 4 peripherals on 1 bus, and 8 peripherals on 2 buses.

I added my Reviewed-by tags for the last patches, there's only one typo
which could be dealt with a follow-up patch if that's easier

Thanks again for this contribution, much appreciated.

> CHANGES SINCE V2:
> #4 Add a comment explaining why INTMASK isn't cleared when going around
>    the update_status loop.
> 
> #5 Leave the existing error handling in sdw_program_device_num(),
>    instead of suppressing the error return.
>    Add a comment in sdw_handle_slave_status() explaining why the error
>    is ignored.
>    Re-word the explanation of why sdw_handle_slave_status() must only return
>    early if it programmed a device ID.
> 
> Richard Fitzgerald (4):
>   soundwire: bus: Don't lose unattach notifications
>   soundwire: bus: Don't re-enumerate before status is UNATTACHED
>   soundwire: cadence: Fix lost ATTACHED interrupts when enumerating
>   soundwire: bus: Don't exit early if no device IDs were programmed
> 
> Simon Trimmer (1):
>   soundwire: cadence: fix updating slave status when a bus has multiple
>     peripherals
> 
>  drivers/soundwire/bus.c            | 44 +++++++++++++---
>  drivers/soundwire/cadence_master.c | 80 ++++++++++++++++--------------
>  2 files changed, 80 insertions(+), 44 deletions(-)
>