[ Upstream commit ccb7bb13c0 ]
The sdw_dev_lock protects the SoundWire driver callbacks against
the probed flag, which is used to skip the callbacks if the
driver gets removed. For more information see commit bd29c00edd
("soundwire: revisit driver bind/unbind and callbacks").
However, this lock is a frequent source of mutex inversions.
Many audio operations eventually hit the hardware resulting in a
SoundWire callback, this means that typically the driver has the
locking order ALSA/ASoC locks -> sdw_dev_lock. Conversely, the IRQ
comes in directly from the SoundWire hardware, but then will often
want to access ALSA/ASoC, such as updating something in DAPM or
an ALSA control. This gives the other lock order sdw_dev_lock ->
ALSA/ASoC locks.
When the IRQ handling was initially added to SoundWire this was
through a callback mechanism. As such it required being covered by
the lock because the callbacks are part of the sdw_driver structure
and are thus present regardless of if the driver is currently
probed.
Since then a newer mechanism using the IRQ framework has been
added, which is currently covered by the same lock but this isn't
actually required. Handlers for the IRQ framework are registered in
probe and should by released during remove, thus the IRQ framework
will have already unbound the IRQ before the slave driver is
removed. Avoid the aforementioned mutex inversion by moving the
handle_nested_irq call outside of the sdw_dev_lock.
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Link: https://lore.kernel.org/r/20250609143041.495049-3-ckeepax@opensource.cirrus.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit 0383734179 ]
During pm_prepare callback, pm_request_resume() delays SoundWire manager D0
entry sequence. Synchronize runtime resume sequence for amd_manager
instance prior to invoking child devices resume sequence for both the amd
power modes(ClockStop Mode and Power off mode).
Change the power_mode_mask check and use pm_runtime_resume() in
amd_pm_prepare() callback.
Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
Link: https://lore.kernel.org/r/20250530054447.1645807-3-Vijendar.Mukunda@amd.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit f8c35d61ba ]
The SoundWire peripheral enumeration is entirely based on interrupts,
more specifically sticky bits tracking state changes.
This patch adds a defensive programming check on the actual status
reported in PING frames. If for some reason an interrupt was lost or
delayed, the delayed work would detect a peripheral change of status
after the bus starts.
The 100ms defined for the delay is not completely arbitrary, if a
Peripheral didn't join the bus within that delay then probably the
hardware link is broken, and conversely if the detection didn't happen
because of software issues the 100ms is still acceptable in terms of
user experience.
The overhead of the one-shot workqueue is minimal, and the mutual
exclusion ensures that the interrupt and delayed work cannot update
the status concurrently.
Reviewed-by: Liam Girdwood <liam.r.girdwood@intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20240805114921.88007-1-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit 5aedb8d833 ]
The existing code enables the Cadence IP interrupts after the bus
reset sequence. The problem with this sequence is that it might be
pre-empted, giving SoundWire devices time to sync and report as
ATTACHED before the interrupts are enabled. In that case, the Cadence
IP will not detect a state change and will not throw an interrupt to
proceed with the enumeration of a Device0.
In our overnight stress tests, we observed that a slight
sub-millisecond delay in enabling interrupts after the reset was
correlated with detection failures. This problem is more prevalent on
the LunarLake silicon, likely due to SOC integration changes, but it
was observed on earlier generations as well.
This patch reverts the sequence, with the interrupts enabled before
performing the bus reset. This removes the race condition and makes
sure the Cadence IP is able to detect the presence of a Device0 in all
cases.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20240805115003.88035-1-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
commit ab8d66d132 upstream.
Two bitmasks in 'struct sdw_slave_prop' - 'source_ports' and
'sink_ports' - define which ports to program in
sdw_program_slave_port_params(). The masks are used to get the
appropriate data port properties ('struct sdw_get_slave_dpn_prop') from
an array.
Bitmasks can be non-continuous or can start from index different than 0,
thus when looking for matching port property for given port, we must
iterate over mask bits, not from 0 up to number of ports.
This fixes allocation and programming slave ports, when a source or sink
masks start from further index.
Fixes: f8101c74aa ("soundwire: Add Master and Slave port programming")
Cc: stable@vger.kernel.org
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20240729140157.326450-1-krzysztof.kozlowski@linaro.org
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[ Upstream commit 8ee1b439b1 ]
For some reason, we add an offset to the PDI, presumably to skip the
PDI0 and PDI1 which are reserved for BPT.
This code is however completely wrong and leads to an out-of-bounds
access. We were just lucky so far since we used only a couple of PDIs
and remained within the PDI array bounds.
A Fixes: tag is not provided since there are no known platforms where
the out-of-bounds would be accessed, and the initial code had problems
as well.
A follow-up patch completely removes this useless offset.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20240326090122.1051806-2-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit 63dc588e7a ]
When SoundWire Wake interrupt is enabled along with SoundWire Wake
enable register, SoundWire wake interrupt will be reported
when SoundWire manager is in D3 state and ACP is in D3 state.
When SoundWire Wake interrupt is reported, it will invoke runtime
resume of the SoundWire manager device.
In case of system level suspend, for ClockStop Mode SoundWire Wake
interrupt should be disabled.
It should be enabled only for runtime suspend scenario.
Change wake interrupt enable/disable sequence for ClockStop Mode in
system level suspend and runtime suspend sceanrio.
Fixes: 9cf1efc5ed ("soundwire: amd: add pm_prepare callback and pm ops support")
Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
Link: https://lore.kernel.org/r/20240327063143.2266464-2-Vijendar.Mukunda@amd.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit 393cae5f32 ]
Start from ACE1.x, DOAISE is added to AC timing control
register bit 5, it combines with DOAIS to get effective
timing, and has the default value 1.
The current code fills DOAIS, DACTQE and DODS bits to a
variable initialized to zero, and updates the variable
to AC timing control register. With this operation, We
change DOAISE to 0, and force a much more aggressive
timing. The timing is even unable to form a working
waveform on SDA pin.
This patch uses read-modify-write operation for the AC
timing control register access, thus makes sure those
bits not supposed and intended to change are not touched.
Signed-off-by: Chao Song <chao.song@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20231127124735.2080562-1-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
commit e199bf52ff upstream.
If bus is marked as multi_link, but number of masters in the stream is
not higher than bus->hw_sync_min_links (bus->multi_link && m_rt_count >=
bus->hw_sync_min_links), bank switching should not happen. The first
part of do_bank_switch() code properly takes these conditions into
account, but second part (sdw_ml_sync_bank_switch()) relies purely on
bus->multi_link property. This is not balanced and leads to NULL
pointer dereference:
Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
...
Call trace:
wait_for_completion_timeout+0x124/0x1f0
do_bank_switch+0x370/0x6f8
sdw_prepare_stream+0x2d0/0x438
qcom_snd_sdw_prepare+0xa0/0x118
sm8450_snd_prepare+0x128/0x148
snd_soc_link_prepare+0x5c/0xe8
__soc_pcm_prepare+0x28/0x1ec
dpcm_be_dai_prepare+0x1e0/0x2c0
dpcm_fe_dai_prepare+0x108/0x28c
snd_pcm_do_prepare+0x44/0x68
snd_pcm_action_single+0x54/0xc0
snd_pcm_action_nonatomic+0xe4/0xec
snd_pcm_prepare+0xc4/0x114
snd_pcm_common_ioctl+0x1154/0x1cc0
snd_pcm_ioctl+0x54/0x74
Fixes: ce6e74d008 ("soundwire: Add support for multi link bank switch")
Cc: stable@vger.kernel.org
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20231124180136.390621-1-krzysztof.kozlowski@linaro.org
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Pull soundwire updates from Vinod Koul:
"Device numbering and intel driver changes are main features:
- Core support for soundwire device number allocation
- intel driver updates for adding hw_params for DAI ops, hybrid
number allocation and power managemnt callback updates
- DT header include changes for subsystem"
* tag 'soundwire-6.6-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/soundwire:
soundwire: intel_ace2x: add DAI hw_params/prepare/hw_free callbacks
soundwire: intel_auxdevice: add hybrid IDA-based device_number allocation
soundwire: bus: add callbacks for device_number allocation
soundwire: extend parameters of new_peripheral_assigned() callback
soundWire: intel_auxdevice: resume 'sdw-master' on startup and system resume
soundwire: intel_auxdevice: enable pm_runtime earlier on startup
soundwire: Explicitly include correct DT includes
Currently the in-band alerts for SoundWire peripherals can only
be communicated to the driver through the interrupt_callback
function. This however is slightly inconvenient for devices that wish
to share IRQ handling code between SoundWire and I2C/SPI, the later
would normally register an IRQ handler with the IRQ subsystem. However
there is no reason the SoundWire in-band IRQs can not also be
communicated as an actual IRQ to the driver.
Add support for SoundWire peripherals to register a normal IRQ
handler to receive SoundWire in-band alerts, allowing code to be
shared across control buses. Note that we allow users to use both the
interrupt_callback and the IRQ handler, this is useful for devices
which must clear additional chip specific SoundWire registers that are
not a part of the normal IRQ flow, or the SoundWire specification.
Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Acked-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Acked-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Link: https://lore.kernel.org/r/20230804104602.395892-2-ckeepax@opensource.cirrus.com
Signed-off-by: Lee Jones <lee@kernel.org>
The IDA-based allocation is useful to simplify debug, but it was also
introduced as a prerequisite to deal with the Intel Lunar Lake
hardware programming sequences: the wake-ups have to be handled with a
system-unique SDI address at the HDaudio controller level.
At the time, the restriction introduced by the IDA to 8 devices total
seemed perfectly fine, but recently hardware vendors created
configurations with more than 8 devices.
Add a new allocation strategy to allow for more than 8 devices using
information on the type of devices, and only use the IDA-based
allocation for devices capable of generating a wake.
In theory the information on wake capabilities should come from
firmware, but none of the existing ACPI tables provide it. The drivers
set the 'wake_capable' property, but this cannot be used reliably: if
the driver probe happens *after* the enumeration, then that property
is not initialized yet. Trying to modify the device_number on-the-fly
proved to be an impossible task generating race conditions left and
right.
The only reliable work-around to control the enumeration is to add a
quirk table. It's ugly but until platform firmware improves, hopefully as a
result of MIPI/SDCA stardization, we can expect that quirk table to
grow for each new headset or microphone codec.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20230731091333.3593132-4-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
The SoundWire bus is handled with a dedicated device, which is placed
between the Intel auxiliary device and peripheral devices, e.g.
soundwire_intel.link.0/sdw-master-0/sdw:0:025d:0711:01
The functionality of this 'sdw-master' device is limited, specifically
for pm_runtime the ASoC framework will not rely on
pm_runtime_get_sync() since it does not register any components. It
will only change status thanks to the parent-child relationship which
guarantees that the 'sdw-master' device will be pm_runtime resumed
before any peripheral device.
However on startup and system resume it's possible that only the
auxiliary device is pm_runtime active, and the peripheral will only
become active during its io_init routine, leading to another
occurrence of the error reported by the pm_runtime framework:
rt711 sdw:0:025d:0711:00: runtime PM trying to activate child device
sdw:0:025d:0711:00 but parent (sdw-master-0) is not active
This patch suggests aligning the sdw-master device status to that of
the auxiliary device. The difference between the two is completely
notional and their pm_status shouldn't be different during the startup
and system resume steps.
This problem was exposed by recent changes in the timing of the bus
reset, but was present in this driver since we introduced pm_runtime
support.
Closes: https://github.com/thesofproject/linux/issues/4328
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Tested-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Link: https://lore.kernel.org/r/20230803065220.3823269-3-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
As soon as the bus starts, physical peripheral devices may report as
ATTACHED and set their status with pm_runtime_set_active() in their
update_status()/io_init().
This is problematic with the existing code, since the parent
pm_runtime status is changed to "active" after starting the bus. This
creates a time window where the pm_runtime framework can report an
issue, e.g.
"rt711 sdw:0:025d:0711:00: runtime PM trying to activate child device
sdw:0:025d:0711:00 but parent (sdw-master-0) is not active"
This patch enables runtime_pm earlier to make sure the auxiliary
device is pm_runtime active after powering-up, but before starting the
bus.
This problem was exposed by recent changes in the timing of the bus
reset, but was present in this driver since we introduced pm_runtime
support.
Closes: https://github.com/thesofproject/linux/issues/4328
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Tested-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Link: https://lore.kernel.org/r/20230803065220.3823269-2-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
The DT of_device.h and of_platform.h date back to the separate
of_platform_bus_type before it as merged into the regular platform bus.
As part of that merge prepping Arm DT support 13 years ago, they
"temporarily" include each other. They also include platform_device.h
and of.h. As a result, there's a pretty much random mix of those include
files used throughout the tree. In order to detangle these headers and
replace the implicit includes with struct declarations, users need to
explicitly include the correct includes.
Signed-off-by: Rob Herring <robh@kernel.org>
Link: https://lore.kernel.org/r/20230714174946.4063995-1-robh@kernel.org
Signed-off-by: Vinod Koul <vkoul@kernel.org>
The soundwire subsystem uses two completion structures that allow
drivers to wait for soundwire device to become enumerated on the bus and
initialised by their drivers, respectively.
The code implementing the signalling is currently broken as it does not
signal all current and future waiters and also uses the wrong
reinitialisation function, which can potentially lead to memory
corruption if there are still waiters on the queue.
Not signalling future waiters specifically breaks sound card probe
deferrals as codec drivers can not tell that the soundwire device is
already attached when being reprobed. Some codec runtime PM
implementations suffer from similar problems as waiting for enumeration
during resume can also timeout despite the device already having been
enumerated.
Fixes: fb9469e54f ("soundwire: bus: fix race condition with enumeration_complete signaling")
Fixes: a90def0681 ("soundwire: bus: fix race condition with initialization_complete signaling")
Cc: stable@vger.kernel.org # 5.7
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: Rander Wang <rander.wang@linux.intel.com>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20230705123018.30903-2-johan+linaro@kernel.org
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Always add buses to the stream->master_list in a fixed order.
The unique bus->id is used to order the adding of buses to the
list.
This prevents lockdep asserts and possible deadlocks on streams
that have multiple buses.
sdw_acquire_bus_lock() takes bus_lock in the order that buses
are listed in stream->master_list. do_bank_switch() takes all
the msg_lock in the same order.
To prevent a lockdep assert, and a possible real deadlock, the
relative order of taking these mutexes must always be the same.
For example, if a stream takes the mutexes in the order
(bus0, bus1) lockdep will assert if another stream takes them
in the order (bus1, bus0).
More complex relative ordering will also assert, for example
if two streams take (bus0, bus1) and (bus1, bus2), then a third
stream takes (bus2, bus0).
Previously sdw_stream_add_master() simply added the given bus
to the end of the list, requiring the caller to guarantee that
buses are added in a fixed order. This isn't reasonable or
necessary - it's an internal implementation detail that should
not be exposed by the API. It doesn't really make sense when
there could be multiple independent calling drivers, to say
"you must add your buses in the same order as a different driver,
that you don't know about, added them".
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20230615141208.679011-2-rf@opensource.cirrus.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Give the bus_lock and msg_lock of each bus a different unique key
so that it is possible to acquire the locks of multiple buses
without lockdep asserting a possible deadlock.
Using mutex_init() to initialize a mutex gives all those mutexes
the same lock class. Lockdep checking treats it as an error to
attempt to take a mutex while already holding a mutex of the same
class. This causes a lockdep assert when sdw_acquire_bus_lock()
attempts to lock multiple buses, and when do_bank_switch() takes
multiple msg_lock.
[ 138.697350] WARNING: possible recursive locking detected
[ 138.697366] 6.3.0-test #1 Tainted: G E
[ 138.697380] --------------------------------------------
[ 138.697394] play/903 is trying to acquire lock:
[ 138.697409] ffff99b8c41aa8c8 (&bus->bus_lock){+.+.}-{3:3}, at:
sdw_prepare_stream+0x52/0x2e0
[ 138.697443]
but task is already holding lock:
[ 138.697468] ffff99b8c41af8c8 (&bus->bus_lock){+.+.}-{3:3}, at:
sdw_prepare_stream+0x52/0x2e0
[ 138.697493]
other info that might help us debug this:
[ 138.697521] Possible unsafe locking scenario:
[ 138.697540] CPU0
[ 138.697550] ----
[ 138.697559] lock(&bus->bus_lock);
[ 138.697570] lock(&bus->bus_lock);
[ 138.697581]
*** DEADLOCK ***
Giving each mutex a unique key allows multiple to be held
without triggering a lockdep assert. But note that it does not
allow them to be taken in one order then a different order.
If two mutexes are taken in the order A, B then they must
always be taken in that order otherwise they could deadlock.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20230615141208.679011-1-rf@opensource.cirrus.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
The ace2x driver can be build with or without mlink support, but
when SND_SOC_SOF_HDA_MLINK is set to =m and soundwire is built-in,
it fails with a link error:
ld.lld: error: undefined symbol: hdac_bus_eml_sdw_wait_syncpu_unlocked
>>> referenced by intel_ace2x.c
>>> drivers/soundwire/intel_ace2x.o:(intel_link_power_up) in archive vmlinux.a
ld.lld: error: undefined symbol: hdac_bus_eml_sdw_sync_arm_unlocked
>>> referenced by intel_ace2x.c
>>> drivers/soundwire/intel_ace2x.o:(intel_sync_arm) in archive vmlinux.a
Add a Kconfig dependency that prevents that broken configuration but
still allows soundwire to be a loadable module instead.
Fixes: 4d1e2464a1 ("soundwire: intel_ace2x: add sync_arm/sync_go helpers")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Link: https://lore.kernel.org/r/20230616090932.2714714-1-arnd@kernel.org
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Returning an error code in the remove callback yields to an error
message
remove callback returned a non-zero value. This will be ignored.
After that the device is removed anyhow. Improve the error message to at
least say what the actual problem is. While touching that code, convert
the driver to the .remove_new() callback which returns no value with the
same effect as returning zero in a .remove() callback.
As the return value is ignored by the core the only effect of this patch
is to improve the error message. (And the motivating effect is that
there is one less driver using .remove().)
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/20230518200823.249795-1-u.kleine-koenig@pengutronix.de
Signed-off-by: Vinod Koul <vkoul@kernel.org>
