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(-) >
