summaryrefslogtreecommitdiffstats
path: root/drivers/soundwire/stream.c
Commit message (Collapse)AuthorAgeFilesLines
* soundwire: revisit driver bind/unbind and callbacksPierre-Louis Bossart2022-07-061-18/+35
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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: <TASK> 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 <pierre-louis.bossart@linux.intel.com> Reviewed-by: Rander Wang <rander.wang@intel.com> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com> Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com> Link: https://lore.kernel.org/r/20220621225641.221170-2-pierre-louis.bossart@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: Fix error return code in do_bank_switch()Wang Wensheng2022-04-051-0/+1
| | | | | | | | | | 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 <wangwensheng4@huawei.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Link: https://lore.kernel.org/r/20220307074039.117488-1-wangwensheng4@huawei.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: make enable/disable/deprepare idempotentPierre-Louis Bossart2022-02-111-0/+15
| | | | | | | | | | | | | | | | | | | | | | | | | | 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 <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/20220126011715.28204-20-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: sdw_stream_add_ functions can be called multiple timesPierre-Louis Bossart2022-02-111-0/+24
| | | | | | | | | | | | | | | | | | 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 <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/20220126011715.28204-19-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: introduce sdw_slave_rt_find() helperPierre-Louis Bossart2022-02-111-12/+22
| | | | | | | | | | | | Before we split the alloc and config steps, we need a helper to find the Slave runtime for a stream. The helper is based on the search loop in sdw_slave_rt_free(), which can now be simplified. 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/20220126011715.28204-18-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: separate alloc and config within sdw_stream_add_xxx()Pierre-Louis Bossart2022-02-111-32/+48
| | | | | | | | | | | | | | | | Separate alloc and config parts so that follow-up patches can allow for multiple calls to sdw_stream_add_slave/master. This is a feature from the ALSA/ASoC frameworks which is not supported today. This is an invasive patch which modifies the error handling flow, with cleanups only done when an allocation fails. Configuration failures only return an error code. 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/20220126011715.28204-17-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: move list addition to sdw_slave_alloc_rt()Pierre-Louis Bossart2022-02-111-3/+6
| | | | | | | | | | | | Simplify sdw_stream_add_slave() by moving the linked list management inside of the sdw_slave_alloc_rt_free() helper, this also makes the alloc/free helpers more symmetrical. 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/20220126011715.28204-16-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: rename and move master/slave_rt_free routinesPierre-Louis Bossart2022-02-111-36/+36
| | | | | | | | | | | | | The naming is rather inconsistent, use the sdw_<object>_<action> convention, and move the free routine after alloc/config. No functionality change beyond rename/move. 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/20220126011715.28204-15-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: group sdw_stream_ functionsPierre-Louis Bossart2022-02-111-264/+264
| | | | | | | | | | | | | | Group all exported functions prior to split of add in alloc/config stages necessary for support of multiple calls to hw_params() by ALSA/ASoC core. Pure code move, no 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/20220126011715.28204-14-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: split sdw_alloc_slave_rt() in alloc and configPierre-Louis Bossart2022-02-111-10/+25
| | | | | | | | | | | | | | Split the two parts so that we can do multiple configurations during ALSA/ASoC hw_params stage. Also follow existing convention sdw_<object>_<action> used at lower level. No functionality change here. 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/20220126011715.28204-13-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: move sdw_alloc_slave_rt() before 'master' helpersPierre-Louis Bossart2022-02-111-26/+26
| | | | | | | | | | | Code move before splitting the function in two. No 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/20220126011715.28204-12-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: split sdw_alloc_master_rt() in alloc and configPierre-Louis Bossart2022-02-111-17/+34
| | | | | | | | | | | | | | Split the two parts so that we can do multiple configurations during ALSA/ASoC hw_params stage. Also follow existing convention sdw_<object>_<action> used at lower level. No functionality change here. 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/20220126011715.28204-11-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: simplify sdw_alloc_master_rt()Pierre-Louis Bossart2022-02-111-9/+18
| | | | | | | | | | | | | | Only do the allocation in that function, and move check for allocation in the caller. This will it easier to split allocation and configuration. No functionality change in this patch. 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/20220126011715.28204-10-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: group sdw_port and sdw_master/slave_port functionsPierre-Louis Bossart2022-02-111-122/+120
| | | | | | | | | | | | | | | | | re-group all the helpers in one location with a code move. For consistency the 'slave' helpers are placed before the 'master' helpers. Also remove unused arguments and rename the 'release' function to 'free' for consistency. No functional change in this patch. 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/20220126011715.28204-9-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: add 'slave' prefix for port range checksPierre-Louis Bossart2022-02-111-11/+11
| | | | | | | | | | | We can only check for Slave port ranges, the ports are not defined at the Master level. Also move the function to the 'slave port' block. 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/20220126011715.28204-8-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: split alloc and config in two functionsPierre-Louis Bossart2022-02-111-12/+37
| | | | | | | | | | | Continue the split with two functions for master and slave, and remove unused arguments. 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/20220126011715.28204-7-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: split port allocation and configuration loopsPierre-Louis Bossart2022-02-111-0/+8
| | | | | | | | | | | Split loops before moving the allocation and configuration to separate functions. 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/20220126011715.28204-6-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: add alloc/config/free helpers for portsPierre-Louis Bossart2022-02-111-38/+45
| | | | | | | | | | | | | | | | | The existing code only has a config helper that allocates memory, start adding alloc/config/free for ports, as a first step in the simplification of the stream API. This change removes a kfree() on a configuration error, this should have not impact on existing platforms and error handling will be revisited in follow-up patches to make sure invalid configurations have not impact on memory allocation. 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/20220126011715.28204-5-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: simplify check on port rangePierre-Louis Bossart2022-02-111-6/+4
| | | | | | | | | | | Pass the index directly to sdw_is_valid_port_range(), this will be useful for further simplifications. 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/20220126011715.28204-4-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: add slave runtime to list earlierPierre-Louis Bossart2022-02-111-11/+2
| | | | | | | | | | | | | sdw_config_stream() only verifies the compatibility between information provided by the Slave driver and the stream configuration. There is no problem if we add the slave runtime to the list earlier. 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/20220126011715.28204-3-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: remove unused parameter in sdw_stream_add_slavePierre-Louis Bossart2022-02-111-4/+2
| | | | | | | | | | The stream parameter is not used, remove before further simplifications. 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/20220126011715.28204-2-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* ASoC/SoundWire: dai: expand 'stream' concept beyond SoundWirePierre-Louis Bossart2021-12-241-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The HDAudio ASoC support relies on the set_tdm_slots() helper to store the HDaudio stream tag in the tx_mask. This only works because of the pre-existing order in soc-pcm.c, where the hw_params() is handled for codec_dais *before* cpu_dais. When the order is reversed, the stream_tag is used as a mask in the codec fixup functions: /* fixup params based on TDM slot masks */ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK && codec_dai->tx_mask) soc_pcm_codec_params_fixup(&codec_params, codec_dai->tx_mask); As a result of this confusion, the codec_params_fixup() ends-up generating bad channel masks, depending on what stream_tag was allocated. We could add a flag to state that the tx_mask is really not a mask, but it would be quite ugly to persist in overloading concepts. Instead, this patch suggests a more generic get/set 'stream' API based on the existing model for SoundWire. We can expand the concept to store 'stream' opaque information that is specific to different DAI types. In the case of HDAudio DAIs, we only need to store a stream tag as an unsigned char pointer. The TDM rx_ and tx_masks should really only be used to store masks. Rename get_sdw_stream/set_sdw_stream callbacks and helpers as get_stream/set_stream. No functionality change beyond the rename. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Rander Wang <rander.wang@intel.com> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@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-5-yung-chuan.liao@linux.intel.com Signed-off-by: Mark Brown <broonie@kernel.org>
* soundwire: stream: don't program mockup device portsBard Liao2021-08-021-0/+3
| | | | | | | | | | Mockup devices don't take part in command/control operations and their virtual ports shall not be programmed. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com> Reviewed-by: Rander Wang <rander.wang@intel.com> Link: https://lore.kernel.org/r/20210714032209.11284-9-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: don't abort bank switch on Command_Ignored/-ENODATABard Liao2021-08-021-1/+1
| | | | | | | | | | | | | | | | | | This change is needed for support of mockup devices, which by construction will not provide any answer to a bank switch, but it's also legit for regular cases. If for some reason a device loses sync and cannot handle a bank switch, we should go ahead anyways. The devices can always resync later. The only case where the error flow should be used is when there is a Command_Aborted composite answer from SoundWire devices. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com> Reviewed-by: Rander Wang <rander.wang@intel.com> Link: https://lore.kernel.org/r/20210714032209.11284-6-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: Fix test for DP prepare completeRichard Fitzgerald2021-06-201-7/+6
| | | | | | | | | | | | | | | | | | | | | | In sdw_prep_deprep_slave_ports(), after the wait_for_completion() the DP prepare status register is read. If this indicates that the port is now prepared, the code should continue with the port setup. It is irrelevant whether the wait_for_completion() timed out if the port is now ready. The previous implementation would always fail if the wait_for_completion() timed out, even if the port was reporting successful prepare. This patch also fixes a minor bug where the return from sdw_read() was not checked for error - any error code with LSBits clear could be misinterpreted as a successful port prepare. Fixes: 79df15b7d37c ("soundwire: Add helpers for ports operations") 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/20210618144745.30629-1-rf@opensource.cirrus.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: fix memory leak in stream config error pathRander Wang2021-04-061-1/+9
| | | | | | | | | | | | | | | | When stream config is failed, master runtime will release all slave runtime in the slave_rt_list, but slave runtime is not added to the list at this time. This patch frees slave runtime in the config error path to fix the memory leak. Fixes: 89e590535f32 ("soundwire: Add support for SoundWire stream management") Signed-off-by: Rander Wang <rander.wang@intel.com> Reviewed-by: Keyon Jie <yang.jie@intel.com> Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@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/20210331004610.12242-1-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: remove useless bus initializationsPierre-Louis Bossart2021-03-301-2/+2
| | | | | | | | | | | | | There is no need to assign a pointer to NULL if it's only used in a loop and assigned within that loop. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com> Reviewed-by: Rander Wang <rander.wang@intel.com> Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com> Link: https://lore.kernel.org/r/20210302091122.13952-12-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: remove useless initializationPierre-Louis Bossart2021-03-301-2/+2
| | | | | | | | | | | | | | Cppcheck complains about possible null pointer dereferences, but it's more like there are unnecessary initializations before walking through a list. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com> Reviewed-by: Rander Wang <rander.wang@intel.com> Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com> Link: https://lore.kernel.org/r/20210302091122.13952-11-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: add missing \n in dev_err()Pierre-Louis Bossart2021-03-231-5/+5
| | | | | | | | | | | | We fixed a lot of warnings in 2019 but the magic of copy-paste keeps adding new ones... Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Rander Wang <rander.wang@intel.com> Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com> Link: https://lore.kernel.org/r/20210323005855.20890-5-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* Merge tag 'soundwire-5.10-rc1' of ↵Greg Kroah-Hartman2020-10-011-16/+29
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/soundwire into char-misc-next Vinod writes: soundwire updates for 5.10-rc1 This round of update includes: - Generic bandwidth allocation algorithm from Intel folks - PM support for Intel chipsets - Updates to Intel drivers which makes sdw usable on latest laptops - Support for MMIO SDW controllers found in QC chipsets - Update to subsystem to use helpers in bitfield.h to manage register bits * tag 'soundwire-5.10-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/soundwire: (66 commits) soundwire: sysfs: add slave status and device number before probe soundwire: bus: add enumerated Slave device to device list soundwire: remove an unnecessary NULL check soundwire: cadence: add data port test fail interrupt soundwire: intel: enable test modes soundwire: enable Data Port test modes soundwire: intel: use {u32|u16}p_replace_bits soundwire: cadence: use u32p_replace_bits soundwire: qcom: get max rows and cols info from compatible soundwire: qcom: add support to block packing mode soundwire: qcom: clear BIT FIELDs before value set. soundwire: Add generic bandwidth allocation algorithm soundwire: cadence: add parity error injection through debugfs soundwire: bus: export broadcast read/write capability for tests ASoC: codecs: realtek-soundwire: ignore initial PARITY errors soundwire: bus: use quirk to filter out invalid parity errors soundwire: slave: add first_interrupt_done status soundwire: bus: filter-out unwanted interrupt reports ASoC/soundwire: bus: use property to set interrupt masks soundwire: qcom: fix SLIBMUS/SLIMBUS typo ...
| * soundwire: enable Data Port test modesPierre-Louis Bossart2020-09-231-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Test modes are required for all SoundWire IP, and help debug integration issues. In theory each port can be configured with a different mode but to simplify this patch only offers separate configurations for the Master and Slave ports - this covers 99% of the intended cases during platform integration. The test mode value is set via platform-specific ways. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com> Reviewed-by: Rander Wang <rander.wang@linux.intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com> Link: https://lore.kernel.org/r/20200920193207.31241-2-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
| * soundwire: Add generic bandwidth allocation algorithmBard Liao2020-09-181-0/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This algorithm computes bus parameters like clock frequency, frame shape and port transport parameters based on active stream(s) running on the bus. Developers can also implement their own .compute_params() callback for specific resource management algorithm, and set if before calling sdw_add_bus_master() Credits: this patch is based on an earlier internal contribution by Vinod Koul, Sanyog Kale, Shreyas Nc and Hardik Shah. All hard-coded values were removed from the initial contribution to use BIOS information instead. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com> Acked-by: Jaroslav Kysela <perex@perex.cz> Link: https://lore.kernel.org/r/20200908131520.5712-1-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
| * soundwire: stream: fix NULL/IS_ERR confusionPierre-Louis Bossart2020-09-041-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | snd_soc_dai_get_sdw_stream() can only return -ENOTSUPP or the stream, NULL is not a possible value. Fixes: 4550569bd779f ('soundwire: stream: add helper to startup/shutdown streams') Reported-by: Bard Liao <yung-chuan.liao@linux.intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Rander Wang <rander.wang@linux.intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com> Link: https://lore.kernel.org/r/20200903204739.31206-2-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
| * soundwire: stream: use FIELD_{GET|PREP}Vinod Koul2020-09-041-8/+5
| | | | | | | | | | | | | | | | | | | | use FIELD_{GET|PREP} in stream code to get/set field values instead of open coding masks and shift operations. Signed-off-by: Vinod Koul <vkoul@kernel.org> Tested-by: Bard Liao <yung-chuan.liao@linux.intel.com> Link: https://lore.kernel.org/r/20200903114504.1202143-5-vkoul@kernel.org Signed-off-by: Vinod Koul <vkoul@kernel.org>
| * soundwire: stream: enable hw_sync as needed by hardwarePierre-Louis Bossart2020-09-031-6/+9
| | | | | | | | | | | | | | | | | | | | Use platform-specific information to decide when to use hw_sync, not only a number of links > 1. 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/20200901150556.19432-8-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* | soundwire: fix double free of dangling pointerTom Rix2020-09-031-3/+5
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | clang static analysis flags this problem stream.c:844:9: warning: Use of memory after it is freed kfree(bus->defer_msg.msg->buf); ^~~~~~~~~~~~~~~~~~~~~~~ This happens in an error handler cleaning up memory allocated for elements in a list. list_for_each_entry(m_rt, &stream->master_list, stream_node) { bus = m_rt->bus; kfree(bus->defer_msg.msg->buf); kfree(bus->defer_msg.msg); } And is triggered when the call to sdw_bank_switch() fails. There are a two problems. First, when sdw_bank_switch() fails, though it frees memory it does not clear bus's reference 'defer_msg.msg' to that memory. The second problem is the freeing msg->buf. In some cases msg will be NULL so this will dereference a null pointer. Need to check before freeing. Fixes: 99b8a5d608a6 ("soundwire: Add bank switch routine") Signed-off-by: Tom Rix <trix@redhat.com> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Link: https://lore.kernel.org/r/20200902202650.14189-1-trix@redhat.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: fix the kernel-doc commentVinod Koul2020-07-161-2/+2
| | | | | | | | | | | | | | | sdw_startup_stream() and sdw_shutdown_stream() argument has been updated but not the comments, so update these as well to fix warning with W=1 drivers/soundwire/stream.c:1859: warning: Function parameter or member 'sdw_substream' not described in 'sdw_startup_stream' drivers/soundwire/stream.c:1859: warning: Excess function parameter 'stream' description in 'sdw_startup_stream' drivers/soundwire/stream.c:1903: warning: Function parameter or member 'sdw_substream' not described in 'sdw_shutdown_stream' drivers/soundwire/stream.c:1903: warning: Excess function parameter 'stream' description in 'sdw_shutdown_stream' Signed-off-by: Vinod Koul <vkoul@kernel.org> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Link: https://lore.kernel.org/r/20200715095702.1519554-1-vkoul@kernel.org Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: add helper to startup/shutdown streamsPierre-Louis Bossart2020-07-151-0/+98
| | | | | | | | | | | | | To handle streams at the dailink level, expose two helpers that will be called from machine drivers. Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com> Reviewed-by: Kai Vehmanen <kai.vehmanen@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/20200630184356.24939-3-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* Merge branch 'topic/ro_wordlength' into nextVinod Koul2020-03-201-7/+9
|\
| * soundwire: stream: Add read_only_wordlength flag to port propertiesSrinivas Kandagatla2020-03-201-7/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | According to SoundWire Specification Version 1.2. "A Data Port number X (in the range 0-14) which supports only one value of WordLength may implement the WordLength field in the DPX_BlockCtrl1 Register as Read-Only, returning the fixed value of WordLength in response to reads." As WSA881x interfaces in PDM mode making the only field "WordLength" in DPX_BlockCtrl1" fixed and read-only. Behaviour of writing to this register on WSA881x soundwire slave with Qualcomm Soundwire Controller is throwing up an error. Not sure how other controllers deal with writing to readonly registers, but this patch provides a way to avoid writes to DPN_BlockCtrl1 register by providing a read_only_wordlength flag in struct sdw_dpn_prop Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> Link: https://lore.kernel.org/r/20200311113545.23773-2-srinivas.kandagatla@linaro.org Signed-off-by: Vinod Koul <vkoul@kernel.org>
* | soundwire: stream: use sdw_write instead of updateSrinivas Kandagatla2020-03-131-5/+4
| | | | | | | | | | | | | | | | | | | | | | There is no point in using update for registers with write mask as 0xFF, this adds unnecessary traffic on the bus. Just use sdw_write directly. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> Acked-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Link: https://lore.kernel.org/r/20200312100105.5293-1-srinivas.kandagatla@linaro.org Signed-off-by: Vinod Koul <vkoul@kernel.org>
* | soundwire: stream: don't program ports when a stream that has not been preparedRander Wang2020-02-131-5/+17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In the Intel QA multi-pipelines test case, there are two pipelines for playback and capture on the same bus. The test fails with an error when setting port params: [ 599.224812] rt711 sdw:0:25d:711:0: invalid dpn_prop direction 1 port_num 0 [ 599.224815] sdw_program_slave_port_params failed -22 [ 599.224819] intel-sdw sdw-master-0: Program transport params failed: -22 [ 599.224822] intel-sdw sdw-master-0: Program params failed: -22 [ 599.224828] sdw_enable_stream: SDW0 Pin2-Playback: done This problem is root-caused to the programming of the capture stream ports while it is not yet prepared, the calling sequence is: (1) hw_params for playback. The playback stream provide the port information to Bus. (2) stream_prepare for playback, Transport and port parameters are computed for playback. (3) hw_params for capture. The capture stream provide the port information to Bus, but it has not been prepared so is not accounted for in the bandwidth allocation. (4) stream_enable for playback. Program transport and port parameters for all masters and slaves. Since the transport and port parameters are not computed for capture stream, sdw_program_slave_port_params will generate a error when setting port params for capture. in step (4), we should only program the ports for the stream that have been prepared. A stream that is only in CONFIGURED state should be ignored, its ports will be programmed when it becomes PREPARED. Tested on Comet Lake. GitHub issue: https://github.com/thesofproject/linux/issues/1637 Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Rander Wang <rander.wang@intel.com> Link: https://lore.kernel.org/r/20200114235227.14502-6-pierre-louis.bossart@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* | soundwire: stream: fix support for multiple Slaves on the same linkRander Wang2020-02-131-2/+3
| | | | | | | | | | | | | | | | | | | | | | | | The existing code will unconditionally return after dealing with the first Slave on a link. This return should only happen when there is an error case. Tested on Comet Lake platform. Signed-off-by: Rander Wang <rander.wang@intel.com> Link: https://lore.kernel.org/r/20200114235227.14502-5-pierre-louis.bossart@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* | soundwire: stream: do not update parameters during DISABLED-PREPARED transitionPierre-Louis Bossart2020-02-131-2/+17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | After a system suspend, the ALSA/ASoC core will invoke the .prepare() callback and a TRIGGER_START when INFO_RESUME is not supported. Likewise, when an underflow occurs, the .prepare callback will be invoked. In both cases, the stream can be in DISABLED mode, and will transition into the PREPARED mode. We however don't want the bus bandwidth to be recomputed. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Link: https://lore.kernel.org/r/20200114235227.14502-4-pierre-louis.bossart@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* | soundwire: stream: only prepare stream when it is configured.Bard Liao2020-02-131-1/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We don't need to prepare the stream again if the stream is already prepared. sdw_prepare_stream() could be called multiple times without calling sdw_deprepare_stream(). We call sdw_prepare_stream() in the prepare dai ops and sdw_deprepare_stream() in the hw_free dai ops. If an xrun happens, sdw_prepare_stream() will be called but sdw_deprepare_stream() will not, which results in an imbalance and an invalid total bandwidth. 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/20200114235227.14502-3-pierre-louis.bossart@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* | soundwire: stream: update state machine and add state checksPierre-Louis Bossart2020-02-131-0/+37
|/ | | | | | | | | | | | | | | | | The state machine and notes don't accurately explain or allow transitions from STREAM_DEPREPARED and STREAM_DISABLED. Add more explanations and allow for more transitions as a result of a trigger_stop(), trigger_suspend() and prepare(), depending on the ALSA/ASoC layer behavior defined by the INFO_RESUME and INFO_PAUSE flags. Also add basic checks to help debug inconsistent states and illegal state machine transitions. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Link: https://lore.kernel.org/r/20200114235227.14502-2-pierre-louis.bossart@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: remove redundant pr_err tracesPierre-Louis Bossart2020-01-101-8/+0
| | | | | | | | | | | | | | Only keep pr_err to flag critical configuration errors that will typically only happen during system integration. For errors on prepare/deprepare/enable/disable, the caller can do a much better job with more information on the DAI and device that caused the issue. Suggested-by: Cezary Rojewski <cezary.rojewski@intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Link: https://lore.kernel.org/r/20200108175438.13121-2-pierre-louis.bossart@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: make stream name a const pointerSrinivas Kandagatla2019-09-041-1/+1
| | | | | | | | Make stream name const pointer Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> Link: https://lore.kernel.org/r/20190813083550.5877-3-srinivas.kandagatla@linaro.org Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: Add compute_params callbackVinod Koul2019-09-041-0/+10
| | | | | | | | This callback allows masters to compute the bus parameters required. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> Link: https://lore.kernel.org/r/20190813083550.5877-2-srinivas.kandagatla@linaro.org Signed-off-by: Vinod Koul <vkoul@kernel.org>
* soundwire: stream: remove unnecessary variable initializationsPierre-Louis Bossart2019-08-211-26/+38
| | | | | | | | | | | | A number of variables don't need to be initialized. In a couple of cases where we loop on a list of runtimes, the code handling of the 'bus' variable leads to warnings that it may not be initialized. Add a specific error case to trap such cases. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Link: https://lore.kernel.org/r/20190806005522.22642-10-pierre-louis.bossart@linux.intel.com Signed-off-by: Vinod Koul <vkoul@kernel.org>