linux-stable.git/drivers/soundwire, branch v5.19.2

soundwire: revisit driver bind/unbind and callbacks

2022-08-17T13:15:28+00:00

[ Upstream commit bd29c00edd0a5dac8b6e7332bb470cd50f92e893 ]

In the SoundWire probe, we store a pointer from the driver ops into
the 'slave' structure. This can lead to kernel oopses when unbinding
codec drivers, e.g. with the following sequence to remove machine
driver and codec driver.

/sbin/modprobe -r snd_soc_sof_sdw
/sbin/modprobe -r snd_soc_rt711

The full details can be found in the BugLink below, for reference the
two following examples show different cases of driver ops/callbacks
being invoked after the driver .remove().

kernel: BUG: kernel NULL pointer dereference, address: 0000000000000150
kernel: Workqueue: events cdns_update_slave_status_work [soundwire_cadence]
kernel: RIP: 0010:mutex_lock+0x19/0x30
kernel: Call Trace:
kernel: ? sdw_handle_slave_status+0x426/0xe00 [soundwire_bus 94ff184bf398570c3f8ff7efe9e32529f532e4ae]
kernel: ? newidle_balance+0x26a/0x400
kernel: ? cdns_update_slave_status_work+0x1e9/0x200 [soundwire_cadence 1bcf98eebe5ba9833cd433323769ac923c9c6f82]

kernel: BUG: unable to handle page fault for address: ffffffffc07654c8
kernel: Workqueue: pm pm_runtime_work
kernel: RIP: 0010:sdw_bus_prep_clk_stop+0x6f/0x160 [soundwire_bus]
kernel: Call Trace:
kernel:
kernel: sdw_cdns_clock_stop+0xb5/0x1b0 [soundwire_cadence 1bcf98eebe5ba9833cd433323769ac923c9c6f82]
kernel: intel_suspend_runtime+0x5f/0x120 [soundwire_intel aca858f7c87048d3152a4a41bb68abb9b663a1dd]
kernel: ? dpm_sysfs_remove+0x60/0x60

This was not detected earlier in Intel tests since the tests first
remove the parent PCI device and shut down the bus. The sequence
above is a corner case which keeps the bus operational but without a
driver bound.

While trying to solve this kernel oopses, it became clear that the
existing SoundWire bus does not deal well with the unbind case.

Commit 528be501b7d4a ("soundwire: sdw_slave: add probe_complete structure and new fields")
added a 'probed' status variable and a 'probe_complete'
struct completion. This status is however not reset on remove and
likewise the 'probe complete' is not re-initialized, so the
bind/unbind/bind test cases would fail. The timeout used before the
'update_status' callback was also a bad idea in hindsight, there
should really be no timing assumption as to if and when a driver is
bound to a device.

An initial draft was based on device_lock() and device_unlock() was
tested. This proved too complicated, with deadlocks created during the
suspend-resume sequences, which also use the same device_lock/unlock()
as the bind/unbind sequences. On a CometLake device, a bad DSDT/BIOS
caused spurious resumes and the use of device_lock() caused hangs
during suspend. After multiple weeks or testing and painful
reverse-engineering of deadlocks on different devices, we looked for
alternatives that did not interfere with the device core.

A bus notifier was used successfully to keep track of DRIVER_BOUND and
DRIVER_UNBIND events. This solved the bind-unbind-bind case in tests,
but it can still be defeated with a theoretical corner case where the
memory is freed by a .remove while the callback is in use. The
notifier only helps make sure the driver callbacks are valid, but not
that the memory allocated in probe remains valid while the callbacks
are invoked.

This patch suggests the introduction of a new 'sdw_dev_lock' mutex
protecting probe/remove and all driver callbacks. Since this mutex is
'local' to SoundWire only, it does not interfere with existing locks
and does not create deadlocks. In addition, this patch removes the
'probe_complete' completion, instead we directly invoke the
'update_status' from the probe routine. That removes any sort of
timing dependency and a much better support for the device/driver
model, the driver could be bound before the bus started, or eons after
the bus started and the hardware would be properly initialized in all
cases.

BugLink: https://github.com/thesofproject/linux/issues/3531
Fixes: 56d4fe31af77 ("soundwire: Add MIPI DisCo property helpers")
Fixes: 528be501b7d4a ("soundwire: sdw_slave: add probe_complete structure and new fields")
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Reviewed-by: Ranjani Sridharan
Reviewed-by: Bard Liao
Reviewed-by: Péter Ujfalusi
Link: https://lore.kernel.org/r/20220621225641.221170-2-pierre-louis.bossart@linux.intel.com
Signed-off-by: Vinod Koul
Signed-off-by: Sasha Levin

soundwire: bus_type: fix remove and shutdown support

2022-08-17T13:15:28+00:00

[ Upstream commit df6407782964dc7e35ad84230abb38f46314b245 ]

The bus sdw_drv_remove() and sdw_drv_shutdown() helpers are used
conditionally, if the driver provides these routines.

These helpers already test if the driver provides a .remove or
.shutdown callback, so there's no harm in invoking the
sdw_drv_remove() and sdw_drv_shutdown() unconditionally.

In addition, the current code is imbalanced with
dev_pm_domain_attach() called from sdw_drv_probe(), but
dev_pm_domain_detach() called from sdw_drv_remove() only if the driver
provides a .remove callback.

Fixes: 9251345dca24b ("soundwire: Add SoundWire bus type")
Signed-off-by: Pierre-Louis Bossart 
Reviewed-by: Rander Wang 
Signed-off-by: Bard Liao 
Link: https://lore.kernel.org/r/20220610015105.25987-1-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul 
Signed-off-by: Sasha Levin

soundwire: qcom: Check device status before reading devid

2022-08-17T13:13:54+00:00

commit aa1262ca66957183ea1fb32a067e145b995f3744 upstream.

As per hardware datasheet its recommended that we check the device
status before reading devid assigned by auto-enumeration.

Without this patch we see SoundWire devices with invalid enumeration
addresses on the bus.

Cc: stable@vger.kernel.org
Fixes: a6e6581942ca ("soundwire: qcom: add auto enumeration support")
Signed-off-by: Srinivas Kandagatla 
Link: https://lore.kernel.org/r/20220706095644.5852-1-srinivas.kandagatla@linaro.org
Signed-off-by: Vinod Koul 
Signed-off-by: Greg Kroah-Hartman

soundwire: qcom: adjust autoenumeration timeout

2022-05-09T06:33:14+00:00

Currently timeout for autoenumeration during probe and bus reset is set to
2 secs which is really a big value. This can have an adverse effect on
boot time if the slave device is not ready/reset.
This was the case with wcd938x which was not reset yet but we spent 2
secs waiting in the soundwire controller probe. Reduce this time to
1/10 of Hz which should be good enough time to finish autoenumeration
if any slaves are available on the bus.

Reported-by: Srinivasa Rao Mandadapu 
Signed-off-by: Srinivas Kandagatla 
Link: https://lore.kernel.org/r/20220506084705.18525-1-srinivas.kandagatla@linaro.org
Signed-off-by: Vinod Koul

soundwire: qcom: use pm_runtime_resume_and_get()

2022-05-09T06:32:29+00:00

Use pm_runtime_resume_and_get() to replace the pm_runtime_get_sync() and
pm_runtime_put_noidle() pattern.

No functional changes.

Signed-off-by: Pierre-Louis Bossart 
Reviewed-by: Péter Ujfalusi 
Signed-off-by: Bard Liao 
Reviewed-by: Srinivas Kandagatla 
Link: https://lore.kernel.org/r/20220426235623.4253-6-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul

soundwire: intel: use pm_runtime_resume_and_get()

2022-05-09T06:32:29+00:00

Use pm_runtime_resume_and_get() to replace the pm_runtime_get_sync() and
pm_runtime_put_noidle() pattern.

No functional changes.

Signed-off-by: Pierre-Louis Bossart 
Reviewed-by: Péter Ujfalusi 
Signed-off-by: Bard Liao 
Link: https://lore.kernel.org/r/20220426235623.4253-5-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul

soundwire: cadence: use pm_runtime_resume_and_get()

2022-05-09T06:32:29+00:00

Use pm_runtime_resume_and_get() to replace the pm_runtime_get_sync() and
pm_runtime_put_noidle() pattern.

No functional changes.

Signed-off-by: Pierre-Louis Bossart 
Reviewed-by: Péter Ujfalusi 
Signed-off-by: Bard Liao 
Link: https://lore.kernel.org/r/20220426235623.4253-4-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul

soundwire: bus: use pm_runtime_resume_and_get()

2022-05-09T06:32:29+00:00

Use pm_runtime_resume_and_get() to replace the pm_runtime_get_sync() and
pm_runtime_put_noidle() pattern.

No functional changes.

Signed-off-by: Pierre-Louis Bossart 
Reviewed-by: Péter Ujfalusi 
Signed-off-by: Bard Liao 
Link: https://lore.kernel.org/r/20220426235623.4253-3-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul

soundwire: qcom: return error when pm_runtime_get_sync fails

2022-05-09T06:32:29+00:00

For some reason there's a missing error return in two places.

Fixes: 74e79da9fd46a ("soundwire: qcom: add runtime pm support")
Fixes: 04d46a7b38375 ("soundwire: qcom: add in-band wake up interrupt support")
Signed-off-by: Pierre-Louis Bossart 
Reviewed-by: Péter Ujfalusi 
Signed-off-by: Bard Liao 
Reviewed-by: Srinivas Kandagatla 
Link: https://lore.kernel.org/r/20220426235623.4253-2-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul

soundwire: bus: pm_runtime_request_resume on peripheral attachment

2022-05-09T06:31:09+00:00

In typical use cases, the peripheral becomes pm_runtime active as a
result of the ALSA/ASoC framework starting up a DAI. The parent/child
hierarchy guarantees that the manager device will be fully resumed
beforehand.

There is however a corner case where the manager device may become
pm_runtime active, but without ALSA/ASoC requesting any functionality
from the peripherals. In this case, the hardware peripheral device
will report as ATTACHED and its initialization routine will be
executed. If this initialization routine initiates any sort of
deferred processing, there is a possibility that the manager could
suspend without the peripheral suspend sequence being invoked: from
the pm_runtime framework perspective, the peripheral is *already*
suspended.

To avoid such disconnects between hardware state and pm_runtime state,
this patch adds an asynchronous pm_request_resume() upon successful
attach/initialization which will result in the proper resume/suspend
sequence to be followed on the peripheral side.

BugLink: https://github.com/thesofproject/linux/issues/3459
Signed-off-by: Pierre-Louis Bossart 
Reviewed-by: Ranjani Sridharan 
Reviewed-by: Rander Wang 
Signed-off-by: Bard Liao 
Link: https://lore.kernel.org/r/20220420023241.14335-4-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul