linux-stable.git/drivers/soundwire/stream.c, branch linux-6.3.y

soundwire: stream: Add missing clear of alloc_slave_rt

2023-06-14T09:17:03+00:00

[ Upstream commit 58d95889f3c2064c6139ee94bb0e4d86e1ad4eab ]

The current path that skips allocating the slave runtime does not clear
the alloc_slave_rt flag, this is clearly incorrect. Add the missing
clear, so the runtime won't be erroneously cleaned up.

Fixes: f3016b891c8c ("soundwire: stream: sdw_stream_add_ functions can be called multiple times")
Reviewed-by: Pierre-Louis Bossart 
Signed-off-by: Charles Keepax 
Link: https://lore.kernel.org/r/20230602101140.2040141-1-ckeepax@opensource.cirrus.com
Signed-off-by: Vinod Koul 
Signed-off-by: Sasha Levin

Merge tag 'soundwire-6.3-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/soundwire

2023-02-25T01:29:52+00:00

Pull soundwire updates from Vinod Koul:
 "This is a small update which features a bit of core changes and driver
  updates in Intel and cadence driver.

  Core:

   - sdw_transfer_defer() API change to drop an argument

   - Reset page address rework

   - Export sdw_nwrite_no_pm and sdw_nread_no_pm APIs

  Drivers:

   - Cadence and related intel driver updates for FIFO handling and low
     level msg transfers"

* tag 'soundwire-6.3-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/soundwire:
  soundwire: cadence: further simplify low-level xfer_msg_defer() callback
  soundwire: cadence: use directly bus sdw_defer structure
  soundwire: bus: remove sdw_defer argument in sdw_transfer_defer()
  soundwire: stream: use consistent pattern for freeing buffers
  soundwire: bus: Remove unused reset_page_addr() callback
  soundwire: bus: Don't zero page registers after every transaction
  soundwire: bus_type: Avoid lockdep assert in sdw_drv_probe()
  soundwire: stream: Move remaining register accesses over to no_pm
  soundwire: debugfs: Switch to sdw_read_no_pm
  soundwire: Provide build stubs for common functions
  soundwire: bus: export sdw_nwrite_no_pm and sdw_nread_no_pm functions
  soundwire: cadence: remove unused sdw_cdns_master_ops declaration
  soundwire: enable optional clock registers for SoundWire 1.2 devices
  ASoC/soundwire: remove is_sdca boolean property
  soundwire: cadence: Drain the RX FIFO after an IO timeout
  soundwire: cadence: Remove wasted space in response_buf
  soundwire: cadence: Don't overflow the command FIFOs
  soundwire: intel: remove DAI startup/shutdown

soundwire: bus: remove sdw_defer argument in sdw_transfer_defer()

2023-01-31T14:27:17+00:00

There's no point in passing an argument that is a pointer to a bus
member. We can directly get the member and do an indirection when
needed.

This is a first step before simplifying the hardware-specific
callbacks further.

Signed-off-by: Pierre-Louis Bossart 
Reviewed-by: Ranjani Sridharan 
Signed-off-by: Bard Liao 
Link: https://lore.kernel.org/r/20230119073211.85979-3-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul

soundwire: stream: use consistent pattern for freeing buffers

2023-01-31T14:27:17+00:00

The code should free the message buffer used for data, the message
structure used for control and assign the latter to NULL. The last
part is missing for multi-link cases, and the order is inconsistent
for single-link cases.

Link: https://github.com/thesofproject/linux/issues/4056
Signed-off-by: Pierre-Louis Bossart 
Reviewed-by: Ranjani Sridharan 
Signed-off-by: Bard Liao 
Link: https://lore.kernel.org/r/20230119073211.85979-2-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul

soundwire: stream: Add specific prep/deprep commands to port_prep callback

2023-01-31T12:10:45+00:00

Currently, port_prep callback only has commands for PRE_PREP, PREP,
and POST_PREP, which doesn't directly say whether this is for a
prepare or deprepare call. Extend the command list enum to say
whether the call is for prepare or deprepare aswell.

Also remove SDW_OPS_PORT_PREP from sdw_port_prep_ops as this is unused,
and update this enum to be simpler and more consistent with enum
sdw_clk_stop_type.

Note: Currently, the only users of SDW_OPS_PORT_POST_PREP are codec
drivers sound/soc/codecs/wsa881x.c and sound/soc/codecs/wsa883x.c, both
of which seem to assume that POST_PREP only occurs after a prepare,
even though it would also have occurred after a deprepare. Since it
doesn't make sense to mark the port prepared after a deprepare, changing
the enum to separate PORT_DEPREP from PORT_PREP should make the check
for PORT_PREP in those drivers be more logical.

Signed-off-by: Stefan Binding 
Reviewed-by: Pierre-Louis Bossart 
Acked-By: Vinod Koul 
Link: https://lore.kernel.org/r/20230127165111.3010960-2-sbinding@opensource.cirrus.com
Signed-off-by: Mark Brown

soundwire: stream: Move remaining register accesses over to no_pm

2023-01-09T16:16:27+00:00

There is no need to play with the runtime reference everytime a register
is accessed. All the remaining "pm" style register accesses trace back
to 4 functions:

sdw_prepare_stream
sdw_deprepare_stream
sdw_enable_stream
sdw_disable_stream

Any sensible implementation will need to hold a runtime reference
across all those functions, it makes no sense to be allowing the
device/bus to suspend whilst streams are being prepared/enabled. And
certainly in the case of the all existing users, they all call these
functions from hw_params/prepare/trigger/hw_free callbacks in ALSA,
which will have already runtime resumed all the audio devices
associated during the open callback.

Reviewed-by: Pierre-Louis Bossart 
Signed-off-by: Charles Keepax 
Link: https://lore.kernel.org/r/20221125142028.1118618-5-ckeepax@opensource.cirrus.com
Signed-off-by: Vinod Koul

soundwire: revisit driver bind/unbind and callbacks

2022-07-06T07:04:21+00:00

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

soundwire: stream: Fix error return code in do_bank_switch()

2022-04-05T04:19:25+00:00

Fix to return a negative error code from the error handling case instead
of 0, as done elsewhere in this function.

Signed-off-by: Wang Wensheng 
Reviewed-by: Pierre-Louis Bossart 
Link: https://lore.kernel.org/r/20220307074039.117488-1-wangwensheng4@huawei.com
Signed-off-by: Vinod Koul

soundwire: stream: make enable/disable/deprepare idempotent

2022-02-11T06:47:55+00:00

The stream management currently flags an 'inconsistent state' error
when a change is requested multiple times. This was added on purpose
to identify programming mistakes.

In hindsight, there was no real reason to fail if the logic at the
ASoC-DPCM level invokes the same callback multiple times. It's
perfectly acceptable to just return and not flag an error when there
is nothing to do. The main concern with the state management is to
trap errors such as trying to enable a stream that was not prepared
first.

This patch suggests allowing the stream functions to be idempotent,
i.e. they can be called multiple times.

Note that the prepare case was already handling multiple calls, this
was added in commit c32464c9393d ("soundwire: stream: only prepare
stream when it is configured.")

Signed-off-by: Pierre-Louis Bossart 
Reviewed-by: Rander Wang 
Signed-off-by: Bard Liao 
Link: https://lore.kernel.org/r/20220126011715.28204-20-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul

soundwire: stream: sdw_stream_add_ functions can be called multiple times

2022-02-11T06:47:55+00:00

The sdw_stream_add_slave/master() functions are called from the
.hw_params stage. We need to make sure the functions can be called
multiple times.

In this version, we assume that only 'audio' parameters provide in the
hw_params() can change. If the number of ports could change
dynamically depending on the stream configuration (number of channels,
etc), we would need to free-up all the stream resources and reallocate
them.

Signed-off-by: Pierre-Louis Bossart 
Reviewed-by: Rander Wang 
Signed-off-by: Bard Liao 
Link: https://lore.kernel.org/r/20220126011715.28204-19-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul