Clear the CDNS_MCP_INT_RX_WL interrupt before signaling completion.
This is to prevent the potential race where:
- The main thread is scheduled immediately the completion is signaled,
and starts a new message
- The RX_WL IRQ for this new message happens before sdw_cdns_irq() has
been re-scheduled.
- When sdw_cdns_irq() is re-scheduled it clears the new RX_WL interrupt.
MAIN THREAD | IRQ THREAD
|
_cdns_xfer_msg() |
{ |
write data to FIFO |
wait_for_completion_timeout() |
<BLOCKED> | <---- RX_WL IRQ
| sdw_cdns_irq()
| {
| signal completion
<== RESCHEDULE <==
Handle message completion |
} |
|
Start new message |
_cdns_xfer_msg() |
{ |
write data to FIFO |
wait_for_completion_timeout() |
<BLOCKED> | <---- RX_WL IRQ
==> RESCHEDULE ==>
| // New RX_WL IRQ is cleared before
| // it has been handled.
| clear CDNS_MCP_INTSTAT
| return IRQ_HANDLED;
| }
Before this change, this error message was sometimes seen on kernels
that have large amounts of debugging enabled:
SCP Msg trf timed out
This error indicates that the completion has not been signalled after
500ms.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Fixes: 956baa1992 ("soundwire: cdns: Add sdw_master_ops and IO transfer support")
Reported-by: Norman Bintang <normanbt@google.com>
Closes: https://issuetracker.google.com/issues/477099834
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>
Link: https://patch.msgid.link/20260310113133.1707288-1-rf@opensource.cirrus.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
This was done entirely with mindless brute force, using
git grep -l '\<k[vmz]*alloc_objs*(.*, GFP_KERNEL)' |
xargs sed -i 's/\(alloc_objs*(.*\), GFP_KERNEL)/\1)/'
to convert the new alloc_obj() users that had a simple GFP_KERNEL
argument to just drop that argument.
Note that due to the extreme simplicity of the scripting, any slightly
more complex cases spread over multiple lines would not be triggered:
they definitely exist, but this covers the vast bulk of the cases, and
the resulting diff is also then easier to check automatically.
For the same reason the 'flex' versions will be done as a separate
conversion.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This is the result of running the Coccinelle script from
scripts/coccinelle/api/kmalloc_objs.cocci. The script is designed to
avoid scalar types (which need careful case-by-case checking), and
instead replace kmalloc-family calls that allocate struct or union
object instances:
Single allocations: kmalloc(sizeof(TYPE), ...)
are replaced with: kmalloc_obj(TYPE, ...)
Array allocations: kmalloc_array(COUNT, sizeof(TYPE), ...)
are replaced with: kmalloc_objs(TYPE, COUNT, ...)
Flex array allocations: kmalloc(struct_size(PTR, FAM, COUNT), ...)
are replaced with: kmalloc_flex(*PTR, FAM, COUNT, ...)
(where TYPE may also be *VAR)
The resulting allocations no longer return "void *", instead returning
"TYPE *".
Signed-off-by: Kees Cook <kees@kernel.org>
We can get start_register, data_size, and buffer data from the new
sdw_bpt_section parameter. Also, handle all register sections in the
cdns BRA helpers. No function changes as section number is 1.
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Tested-by: Shuming Fan <shumingf@realtek.com>
Link: https://patch.msgid.link/20251021094355.132943-3-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
We may need to add few fake frames to fit the aligned read dma buffer
size. Add a fake_size parameter to allow the caller to set the fake data
size.
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://patch.msgid.link/20251014031450.3781789-7-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Currently, we calculate the required bandwidth after the PDI buffer size
is calculated. However as we need to add some fake frame to align the
data block size, the final PDI size and the frame number will change.
Besides, we need the required bandwidth to decide the DMA channel number
and the channel number will be used to calculate the data block size.
Therefore, we calculate the required bandwidth and export a helper for
the caller to get the required bandwidth.
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://patch.msgid.link/20251014031450.3781789-5-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
data_per_frame will be used for preparing the TX buffer and we may add
some fake frames to ensure the data in the buffer will be flushed.
So that it should indicate the frame capability even if the required
data bytes are less than the frame capability.
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://patch.msgid.link/20251014031450.3781789-4-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
The existing code uses i as the frame index which is 0 based. And
"bad frame %d/%d", i, num_frames where num_frames is 1 based. Change the
frame index to i + 1 to align with num_frames and more readable.
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Link: https://patch.msgid.link/20251014031450.3781789-2-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
The Cadence IP expects a specific format (detailed in the
Documentation). Add helpers to copy the data into the DMA buffer.
The crc8 table is for now only used by the Cadence driver. This table
might be moved to a common module at a later point if needed by other
controller implementations.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Liam Girdwood <liam.r.girdwood@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Tested-by: shumingf@realtek.com
Link: https://lore.kernel.org/r/20250227140615.8147-10-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
The register definitions are missing a BULK_ENABLE bitfield which must
be set for DP0.
In addition, the existing mapping from PDI to Data Port is 1:1. That's
fine for PCM streams which are by construction in one direction
only. The BTP/BRA protocol is bidirectional and relies on DP0 only,
which breaks the 1:1 mapping. DP0 MUST be mapped to both PDI0 and
PDI1, with PDI0 taking care of the TX direction and PDI1 of the RX
direction.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Liam Girdwood <liam.r.girdwood@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Tested-by: shumingf@realtek.com
Link: https://lore.kernel.org/r/20250227140615.8147-3-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Frame shape and curr_dr_freq could be updated by sdw_compute_bus_params().
Peripherals will set curr_dr_freq as their frequency. Managers
should do the same. Then update frame shape according to the actual
bus frequency.
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>
Link: https://lore.kernel.org/r/20250205074232.87537-2-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
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>
This log is useful when trying different configurations, specifically
to make sure ACPI initrd overrides have been taken into account.
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/20240326092030.1062802-2-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
This offset is set to exactly zero and serves no purpose. Remove.
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-3-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
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>
We want to wait for the CONFIG_UPDATE bit to clear before doing
something else.
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/20230518024119.164160-4-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Combining hardware reset with the multi-link mode leads to a shortened
hardware reset pattern observed on the bus.
The updated hardware programming sequence is to first enable the clock
with the sync_arm/sync_go pattern, and only in a second step to issue
the hardware reset sequence. Since there is no longer a dependency
between sync_arm/sync_go and hw_reset, the behavior of
sdw_cdns_exit_reset() is changed to wait for the self-clearing
CONFIG_UPDATE to go back to zero,
Link: https://github.com/thesofproject/linux/issues/4170
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/20230518024119.164160-3-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
The latest Cadence IP moves MCP_CMD_BASE and MCP_CMD_RESP to the
IP_MCP_CMD_BASE and IP_MCP_CMD_RESP registers located in different
area and accessed with a fixed offset.
Unlike other patches, the fields are not renamed to avoid a very
invasive and low-value set of changes.
For existing solutions, this is an iso-functionality change.
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/20230314015410.487311-17-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
The latest Cadence IP splits the MCP_CMDCTRL fields in two registers:
MCP_CMDCTRL and IP_MCP_CMDCTRL. Rename the relevant fields and change
the access methods used for those fields.
In practice we only use the Parity error insertion in IP_CMD_CTRL.
For existing solutions, this is an iso-functionality change.
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/20230314015410.487311-16-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
The latest Cadence IP splits the MCP_CONTROL fields in two registers:
MCP_CONTROL and IP_MCP_CONTROL. Rename the relevant fields and change
the access methods used for those fields.
For existing solutions, this is an iso-functionality change.
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/20230314015410.487311-15-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
The latest Cadence IP splits the MCP_CONFIG fields in two registers:
MCP_CONFIG and IP_MCP_CONFIG. Rename the relevant fields and change
the access methods used for those fields.
For existing solutions, this is an iso-functionality change.
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/20230314015410.487311-14-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
The latest Cadence IP splits some of the existing registers into two,
separated by a fixed offset. The bitfields themselves remain at the
same position, so we can use new helpers to dynamically add the fixed
offset.
For example, the existing MCP_CONFIG is now split in two with
MCP_CONFIG and IP_MCP_CONFIG (the naming comes directly from the
design document).
This patch adds helpers to access registers with the IP_ prefix. The
addition of the 'ip' prefix for helpers, registers and bitfields is
intentional to help reviewers spot any mistake.
For existing solutions, the offset is exactly zero so there's no
functional change - the MCP_CONFIG and IP_MCP_CONFIG are aliased to
the same address.
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/20230314015410.487311-13-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
This field is not used, and its definition is not aligned with the
hardware specification.
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/20230314015410.487311-12-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
The message pointer is already stored in the bus->defer structure, not
need to pass it as an argument.
Suggested-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20230119073211.85979-5-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Copying the bus sdw_defer structure into the Cadence internals leads
to using stale pointers and kernel oopses on errors. It's just simpler
and safer to use the bus sdw_defer structure directly.
Link: https://github.com/thesofproject/linux/issues/4056
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20230119073211.85979-4-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
A previous patch removed unnecessary zeroing of the page registers
after a paged transaction, so now the reset_page_addr callback is
unused and can be removed.
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/20230123164949.245898-3-rf@opensource.cirrus.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
If wait_for_completion_timeout() times-out in _cdns_xfer_msg() it
is possible that something could have been written to the RX FIFO.
In this case, we should drain the RX FIFO so that anything in it
doesn't carry over and mess up the next transfer.
Obviously, if we got to this state something went wrong, and we
don't really know the state of everything. The cleanup in this
situation cannot be bullet-proof but we should attempt to avoid
breaking future transaction, if only to reduce the amount of
error noise when debugging the failure from a kernel log.
Note that this patch only implements the draining for blocking
(non-deferred) transfers. The deferred API doesn't have any proper
handling of error conditions and would need some re-design before
implementing cleanup. That is a task for a separate patch...
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/20221202161812.4186897-4-rf@opensource.cirrus.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
The response_buf was declared much larger (128 entries) than the number
of responses that could ever be written into it. The Cadence IP is
configurable up to a maximum of 32 entries, and the datasheet says
that RX_FIFO_AVAIL can be 2 larger than this. So allow up to 34
responses.
Also add checking in cdns_read_response() to prevent overflowing
reponse_buf if RX_FIFO_AVAIL contains an unexpectedly large number.
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/20221202161812.4186897-3-rf@opensource.cirrus.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
The command FIFOs in the Cadence IP can be configured during design
up to 32 entries, and the code in cadence_master.c was assuming the
full 32-entry FIFO. But all current Intel implementations use an 8-entry
FIFO.
Up to now the longest message used was 6 entries so this wasn't
causing any problem. But future Cirrus Logic codecs have downloadable
firmware or tuning blobs. It is more efficient for the codec driver to
issue long transfers that can take advantage of any queuing in the
Soundwire controller and avoid the overhead of repeatedly writing the
page registers.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Fixes: 2f52a5177c ("soundwire: cdns: Add cadence library")
Link: https://lore.kernel.org/r/20221202161812.4186897-2-rf@opensource.cirrus.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
The existing 'struct sdw_cdns_dma_data' has really nothing to do with
DMAs. The information is stored in the dai->dma_data, but this is
really private data that should be stored in a different context.
Beyond the academic elegance discussion, using dma_data is a problem
for new Intel hardware where the dma_data structure is already used
for true DMA handling performed by other parts of the code.
This patch prepares a transition away from the use of dma_data, for
now with a rename-only change.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20221101023521.2384586-2-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
- Pierre-Louis Bossart did another round of Intel driver cleanup to prepare
for future code reorg which is expected in next cycle
- Richard Fitzgerald provided bus unattach notifications processing during
re-enumeration along with Cadence driver updates for this.
- Srinivas Kandagatla added Qualcomm driver updates to handle device0 status
-----BEGIN PGP SIGNATURE-----
iQIzBAABCAAdFiEE+vs47OPLdNbVcHzyfBQHDyUjg0cFAmNAJmQACgkQfBQHDyUj
g0eNjw//XQ/pkdB3MiRp6lmUv+99m15i2gzUgVQUKL+cKin5gN/prw8zYZHKcEHi
FkTCUr4j06Q5LUIsmT1v0li4ZYVIaMbDB35IDRRPXKsXDHOzUmXLEeUFQln/HIo3
hYuftjjOhuCFvOBrLRMzp9dUtmjkjix8g0Sn63f/Xj7j+IbwSYlAeW1xkMfyzGkW
8Bp5TulkLwA1ce0JRnxMI7tijrYIfUmadvMGFOIgTwo0KgkbPUZgEQ2TN1p66FJf
20VmM4Uy7gnV7XxU1oDqfxeVRuRHz8ZZSpUU0mnnZWFkP2v00e/x7Jz/mzzZMceW
aTlDCzJF6SlwaW1MyGkHEaypHtR9ZX8Ak+GFfBzYzzwGaNu8YS1745tI3JWGQ17U
cvJonYwCWJI2+YmK1ei29vmX3Z/uCmpxQoxWYyqvDRNIln/2efoAf7SlewhV/sAO
2PZ6kJWGxxWjH6ckvjsqh2plgOt6itcyafKE/yHF9ApbSU4dqZj3lE2Ls2lBROom
q591cenZ2VLFy2co894/TVHibyvBxBIO77uwKMH9pHMmD8FeWiAwoWuK3QM/uD4e
zFmAyDbuZR4JW92jTNGya88VqYGeWzbq9xkY2DOlSbWBkVicZgPnF6pH8wBuudK6
SFbyo+Uo3WmplccgJHSer98ZRlhI8uQNPOvrxckQOXbYvVTIFvc=
=CgPW
-----END PGP SIGNATURE-----
Merge tag 'soundwire-6.1-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/soundwire
Pull soundwire updates from Vinod Koul:
"Updates for Intel, Cadence and Qualcomm drivers:
- another round of Intel driver cleanup to prepare for future code
reorg which is expected in next cycle (Pierre-Louis Bossart)
- bus unattach notifications processing during re-enumeration along
with Cadence driver updates for this (Richard Fitzgerald)
- Qualcomm driver updates to handle device0 status (Srinivas
Kandagatla)"
* tag 'soundwire-6.1-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/soundwire: (42 commits)
soundwire: intel: add helper to stop bus
soundwire: intel: introduce helpers to start bus
soundwire: intel: introduce intel_shim_check_wake() helper
soundwire: intel: simplify read ops assignment
soundwire: intel: remove intel_init() wrapper
soundwire: intel: move shim initialization before power up/down
soundwire: intel: remove clock_stop parameter in intel_shim_init()
soundwire: intel: move all PDI initialization under intel_register_dai()
soundwire: intel: move DAI registration and debugfs init earlier
soundwire: intel: simplify flow and use devm_ for DAI registration
soundwire: intel: fix error handling on dai registration issues
soundwire: cadence: Simplify error paths in cdns_xfer_msg()
soundwire: cadence: Fix error check in cdns_xfer_msg()
soundwire: cadence: Write to correct address for each FIFO chunk
soundwire: bus: Fix wrong port number in sdw_handle_slave_alerts()
soundwire: qcom: do not send status of device 0 during alert
soundwire: qcom: update status from device id 1
soundwire: cadence: Don't overwrite msg->buf during write commands
soundwire: bus: Don't exit early if no device IDs were programmed
soundwire: cadence: Fix lost ATTACHED interrupts when enumerating
...
There's no need to goto an exit label to return from cdns_xfer_msg().
It doesn't do any cleanup, only a return statement.
Replace the gotos with returns.
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/20220917154822.690472-2-rf@opensource.cirrus.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
_cdns_xfer_msg() must add the fragment offset to msg->addr to get the
base target address of each FIFO chunk. Otherwise every chunk will
be written to the first 32 register addresses.
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/20220917123517.229153-1-rf@opensource.cirrus.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
The buf passed in struct sdw_msg must only be written for a READ,
in that case the RDATA part of the response is the data value of the
register.
For a write command there is no RDATA, and buf should be assumed to
be const and unmodifable. The original caller should not expect its data
buffer to be corrupted by an sdw_nwrite().
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/20220916103505.1562210-1-rf@opensource.cirrus.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
The correct way to handle interrupts is to clear the bits we
are about to handle _before_ handling them. Thus if the condition
then re-asserts during the handling we won't lose it.
This patch changes cdns_update_slave_status_work() to do this.
The previous code cleared the interrupts after handling them.
The problem with this is that when handling enumeration of devices
the ATTACH statuses can be accidentally cleared and so some or all
of the devices never complete their enumeration.
Thus we can have a situation like this:
- one or more devices are reverting to ID #0
- accumulated status bits indicate some devices attached and some
on ID #0. (Remember: status bits are sticky until they are handled)
- Because of device on #0 sdw_handle_slave_status() programs the
device ID and exits without handling the other status, expecting
to get an ATTACHED from this reprogrammed device.
- The device immediately starts reporting ATTACHED in PINGs, which
will assert its CDNS_MCP_SLAVE_INTSTAT_ATTACHED bit.
- cdns_update_slave_status_work() clears INTSTAT0/1. If the initial
status had CDNS_MCP_SLAVE_INTSTAT_ATTACHED bit set it will be
cleared.
- The ATTACHED change for the device has now been lost.
- cdns_update_slave_status_work() clears CDNS_MCP_INT_SLAVE_MASK so
if the new ATTACHED state had set it, it will be cleared without
ever having been handled.
Unless there is some other state change from another device to cause
a new interrupt, the ATTACHED state of the reprogrammed device will
never cause an interrupt so its enumeration will not be completed.
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/20220914160248.1047627-5-rf@opensource.cirrus.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
The cadence IP explicitly reports slave status changes with bits for
each possible change. The function cdns_update_slave_status() attempts
to translate this into the current status of each of the slaves.
However when there are multiple peripherals on a bus any slave that did
not have a status change when the work function ran would not have it's
status updated - the array is initialised to a value that equates to
UNATTACHED and this can cause spurious reports that slaves had dropped
off the bus.
In the case where a slave has no status change or has multiple status
changes the value from the last PING command is used.
Signed-off-by: Simon Trimmer <simont@opensource.cirrus.com>
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/20220914160248.1047627-2-rf@opensource.cirrus.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
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 <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20220426235623.4253-4-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
This patch adds a status check after device0 attachment to solve race
conditions observed during attachment with multiple devices per link
The sequence is the following
1) deviceA attaches as device0
2) the hardware detects a device0 status change and throws an
interrupt.
3) the interrupt handler schedules the work function
4) the workqueue starts, we read the status
slave0 = cdns_readl(cdns, CDNS_MCP_SLAVE_INTSTAT0);
slave1 = cdns_readl(cdns, CDNS_MCP_SLAVE_INTSTAT1);
we deal with the status change and program deviceA device number to a
non-zero value.
5) deviceB attaches as device0, the device0 status seen by the
hardware does not change.
6) we clear the CDNS_MCP_SLAVE_INTSTAT0/1 registers -> we will never detect
deviceB!
This patch suggest re-checking in a loop the device0 status with a
PING frame, i.e. using the real device0 status instead of information
on status changes.
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/20220420023039.14144-1-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
While the hardware supports PDM streams, this capability has never
been tested or enabled on any product, so this is dead-code. Let's
remove all this.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Acked-By: Vinod Koul <vkoul@kernel.org>
Link: https://lore.kernel.org/r/20211224021034.26635-8-yung-chuan.liao@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
The duration of the hw_reset is defined as 4096 cycles. The Cadence IP
allows for an additional delay which doesn't seem necessary in
practice: the actual reset sequence duration is defined by the sync_go
mechanism, not by the IP itself.
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/20210818030130.17113-1-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
When we set a source PDI, the target PDI parameters will be overridden
by the source register values. The loopback streams can be
independently enabled on each link.
While the loopback source and target can be configured before any
stream is active on each link, the loopback stream should only be
prepared/triggered when the playback stream is prepared. Otherwise all
registers might be programmed to their reset values and the loopback
will not succeed. The SoundWire bus driver currently does not allow
two streams to be triggered at the same time, so the playback will
have to be started first, and later the loopback.
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/20210714032209.11284-11-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
For debug, it's interesting to create a loopback stream for each link
and use debugfs to set a source and target PDI. The target PDI would
need to be an RX port and use the same register configurations as the
source PDI. This capability allows e.g. for the headphone playback
stream to be snooped on the headset capture stream, or alternatively
for the addition of a dedicated loopback stream, in addition of
regular capture for that link.
This patch only adds the debugfs part, the port/PDI handling will be
handled in the next patches.
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/20210714032209.11284-10-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
The Cadence IP exposes a small number of self-clearing bits in
the MCP_CONTROL and MCP_CONFIG_UPDATE registers.
We currently do not check that those bits are indeed cleared,
e.g. during resume operations. That could lead to resuming peripheral
devices too early.
In addition, if we happen to read these registers, update one of the
fields and write the register back, we may be writing stale data that
might have been cleared in hardware. These sort of race conditions
could lead to e.g. doing a hw_reset twice or stopping a clock that
just restarted. There is no clear way of avoiding these potential race
conditions other than making sure that these registers fields are
cleared before any read-modify-write sequence. If we detect this sort
of errors, we only log them since there is no clear recovery
possible. The only way out is likely to restart the IP with a
suspend/resume cycle.
Note that the checks are performed before updating the registers, as
well as after the Intel 'sync go' sequence in multi-link mode. That
should cover both the start and end of suspend/resume hardware
configurations. The Multi-Master mode gates the configuration updates
until the 'sync go' signal is asserted, so we only check on init and
after the end of the 'sync go' sequence.
The duration of the usleep_range() was defined by the GSYNC frequency
used in multi-master mode. With a 4kHz frequency, any configuration
change might be deferred by up to 250us. Extending the range to
1000-1500us should guarantee that the configuration change is
completed without any significant impact on the overall resume
time.
Suggested-by: Bard Liao <yung-chuan.liao@linux.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/20210714051349.13064-1-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
The ret is not used in the interrupt handler, it is just returned without
any condition or change.
We can return the IRQ_HANDLED directly.
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-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/20210714015555.17685-1-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
