From 30204ef870feda6abe26e7078a0cfb564986769e Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 5 Dec 2023 15:19:21 -0600 Subject: [PATCH 1/9] ASoC: SOF: Intel: hda-mlink: update incorrect comment Likely a copy-paste error, wrong CONFIG used. Signed-off-by: Pierre-Louis Bossart --- include/sound/hda-mlink.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/sound/hda-mlink.h b/include/sound/hda-mlink.h index 228114aca4158d..d849d9b24f13b5 100644 --- a/include/sound/hda-mlink.h +++ b/include/sound/hda-mlink.h @@ -181,4 +181,4 @@ hdac_bus_eml_enable_offload(struct hdac_bus *bus, bool alt, int elid, bool enabl { return 0; } -#endif /* CONFIG_SND_SOC_SOF_HDA */ +#endif /* CONFIG_SND_SOC_SOF_HDA_MLINK */ From 6949ab3a5514ef6c5a2ac4776eb1d2dfb298e18a Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 6 Dec 2023 15:15:51 -0600 Subject: [PATCH 2/9] ASoC: SOF: Intel: hda-stream: export stream_get_position() helper Export this helper so that we can report the DPIB position if the BPT DMA do not complete - this is very useful to see if the DMA started or gets stuck somehow with invalid bandwidth configurations. Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/intel/hda-stream.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index 7d8717ba894e37..93c2c6b37500ba 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -1080,3 +1080,4 @@ snd_pcm_uframes_t hda_dsp_stream_get_position(struct hdac_stream *hstream, return pos; } +EXPORT_SYMBOL_NS(hda_dsp_stream_get_position, SND_SOC_SOF_INTEL_HDA_COMMON); From 9e0246e3b2013d9ef96a9950dee0f7cbf958c0ad Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 1 Nov 2023 15:18:03 -0500 Subject: [PATCH 3/9] soundwire: cadence: fix invalid PDI offset 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 --- drivers/soundwire/cadence_master.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index eb1b87ad1681c2..ab7264bbeed06b 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -1880,7 +1880,7 @@ struct sdw_cdns_pdi *sdw_cdns_alloc_pdi(struct sdw_cdns *cdns, /* check if we found a PDI, else find in bi-directional */ if (!pdi) - pdi = cdns_find_pdi(cdns, 2, stream->num_bd, stream->bd, + pdi = cdns_find_pdi(cdns, 0, stream->num_bd, stream->bd, dai_id); if (pdi) { From daa0825a99f3cf9c16dd9e9a97670957bd36ab40 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 1 Nov 2023 15:30:47 -0500 Subject: [PATCH 4/9] soundwire: cadence: remove PDI offset completely This offset is set to exactly zero and serves no purpose. Remove. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/cadence_master.c | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index ab7264bbeed06b..72522203685345 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -1236,7 +1236,7 @@ EXPORT_SYMBOL(sdw_cdns_enable_interrupt); static int cdns_allocate_pdi(struct sdw_cdns *cdns, struct sdw_cdns_pdi **stream, - u32 num, u32 pdi_offset) + u32 num) { struct sdw_cdns_pdi *pdi; int i; @@ -1249,7 +1249,7 @@ static int cdns_allocate_pdi(struct sdw_cdns *cdns, return -ENOMEM; for (i = 0; i < num; i++) { - pdi[i].num = i + pdi_offset; + pdi[i].num = i; } *stream = pdi; @@ -1266,7 +1266,6 @@ int sdw_cdns_pdi_init(struct sdw_cdns *cdns, struct sdw_cdns_stream_config config) { struct sdw_cdns_streams *stream; - int offset; int ret; cdns->pcm.num_bd = config.pcm_bd; @@ -1277,24 +1276,15 @@ int sdw_cdns_pdi_init(struct sdw_cdns *cdns, stream = &cdns->pcm; /* we allocate PDI0 and PDI1 which are used for Bulk */ - offset = 0; - - ret = cdns_allocate_pdi(cdns, &stream->bd, - stream->num_bd, offset); + ret = cdns_allocate_pdi(cdns, &stream->bd, stream->num_bd); if (ret) return ret; - offset += stream->num_bd; - - ret = cdns_allocate_pdi(cdns, &stream->in, - stream->num_in, offset); + ret = cdns_allocate_pdi(cdns, &stream->in, stream->num_in); if (ret) return ret; - offset += stream->num_in; - - ret = cdns_allocate_pdi(cdns, &stream->out, - stream->num_out, offset); + ret = cdns_allocate_pdi(cdns, &stream->out, stream->num_out); if (ret) return ret; @@ -1802,7 +1792,6 @@ EXPORT_SYMBOL(cdns_set_sdw_stream); * cdns_find_pdi() - Find a free PDI * * @cdns: Cadence instance - * @offset: Starting offset * @num: Number of PDIs * @pdi: PDI instances * @dai_id: DAI id @@ -1811,14 +1800,13 @@ EXPORT_SYMBOL(cdns_set_sdw_stream); * expected to match, return NULL otherwise. */ static struct sdw_cdns_pdi *cdns_find_pdi(struct sdw_cdns *cdns, - unsigned int offset, unsigned int num, struct sdw_cdns_pdi *pdi, int dai_id) { int i; - for (i = offset; i < offset + num; i++) + for (i = 0; i < num; i++) if (pdi[i].num == dai_id) return &pdi[i]; @@ -1872,15 +1860,15 @@ struct sdw_cdns_pdi *sdw_cdns_alloc_pdi(struct sdw_cdns *cdns, struct sdw_cdns_pdi *pdi = NULL; if (dir == SDW_DATA_DIR_RX) - pdi = cdns_find_pdi(cdns, 0, stream->num_in, stream->in, + pdi = cdns_find_pdi(cdns, stream->num_in, stream->in, dai_id); else - pdi = cdns_find_pdi(cdns, 0, stream->num_out, stream->out, + pdi = cdns_find_pdi(cdns, stream->num_out, stream->out, dai_id); /* check if we found a PDI, else find in bi-directional */ if (!pdi) - pdi = cdns_find_pdi(cdns, 0, stream->num_bd, stream->bd, + pdi = cdns_find_pdi(cdns, stream->num_bd, stream->bd, dai_id); if (pdi) { From 19bc7a9807653cdda100ccaf14aeb25d19c2552f Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 10 Oct 2023 14:31:16 -0400 Subject: [PATCH 5/9] soundwire: remove unused sdw_bus_conf structure This is redundant with sdw_bus_params, and was never used. Signed-off-by: Pierre-Louis Bossart --- include/linux/soundwire/sdw.h | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index 66f814b63a435f..e5d0aa58e7f6c9 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -542,21 +542,6 @@ enum sdw_reg_bank { SDW_BANK1, }; -/** - * struct sdw_bus_conf: Bus configuration - * - * @clk_freq: Clock frequency, in Hz - * @num_rows: Number of rows in frame - * @num_cols: Number of columns in frame - * @bank: Next register bank - */ -struct sdw_bus_conf { - unsigned int clk_freq; - unsigned int num_rows; - unsigned int num_cols; - unsigned int bank; -}; - /** * struct sdw_prepare_ch: Prepare/De-prepare Data Port channel * From 58c2b933babb628adfb7030ace1bb8f2ead3aed8 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 6 Nov 2023 20:55:23 -0600 Subject: [PATCH 6/9] soundwire: reconcile dp0_prop and dpn_prop The definitions for DP0 are missing a set of fields that are required to reuse the same configuration code as DPn. Signed-off-by: Pierre-Louis Bossart --- include/linux/soundwire/sdw.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index e5d0aa58e7f6c9..e3a4bccc2a7ef5 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -235,7 +235,8 @@ enum sdw_clk_stop_mode { * @BRA_flow_controlled: Slave implementation results in an OK_NotReady * response * @simple_ch_prep_sm: If channel prepare sequence is required - * @imp_def_interrupts: If set, each bit corresponds to support for + * @ch_prep_timeout: Port-specific timeout value, in milliseconds + * @imp_def_interrupts: If set, each bit corresponds to support for * implementation-defined interrupts * * The wordlengths are specified by Spec as max, min AND number of @@ -249,6 +250,7 @@ struct sdw_dp0_prop { u32 *words; bool BRA_flow_controlled; bool simple_ch_prep_sm; + u32 ch_prep_timeout; bool imp_def_interrupts; }; From 0f7731d8b9626cb0192d5def600051ee80e7c8dc Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 30 Oct 2023 15:42:56 -0500 Subject: [PATCH 7/9] soundwire: clarify maximum allowed address The existing code sets the maximum address at 0x80000000, which is not completely accurate. The last 2 Gbytes are indeed reserved, but so are the 896 Mbytes just before. The maximum address which can be used with paging or BRA is 0x47FFFFFF per Table 131 of the SoundWire 1.2.1 specification. Signed-off-by: Pierre-Louis Bossart --- include/linux/soundwire/sdw_registers.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/soundwire/sdw_registers.h b/include/linux/soundwire/sdw_registers.h index 138bec908c40c5..658b10fa5b20ad 100644 --- a/include/linux/soundwire/sdw_registers.h +++ b/include/linux/soundwire/sdw_registers.h @@ -13,7 +13,7 @@ #define SDW_REG_NO_PAGE 0x00008000 #define SDW_REG_OPTIONAL_PAGE 0x00010000 -#define SDW_REG_MAX 0x80000000 +#define SDW_REG_MAX 0x48000000 #define SDW_DPN_SIZE 0x100 #define SDW_BANK1_OFFSET 0x10 From 385cbea814f4c0c5466a7ee04aa9b253df2ed51b Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 30 Oct 2023 17:44:27 -0500 Subject: [PATCH 8/9] soundwire: debugfs: add interface to read/write commands We have an existing debugfs files to read standard registers (DP0/SCP/DPn). This patch provides a more generic interface to ANY set of read/write contiguous registers in a peripheral device. In follow-up patches, this interface will be extended to use BRA transfers. The sequence is to use the following files added under the existing debugsfs directory for each peripheral device: command (write 0, read 1) num_bytes start_address firmware_file (only for writes) read_buffer (only for reads) Example for a read command - this checks the 6 bytes used for enumeration. cd /sys/kernel/debug/soundwire/master-0-0/sdw\:0\:025d\:0711\:01/ echo 1 > command echo 6 > num_bytes echo 0x50 > start_address echo 1 > go cat read_buffer address 0x50 val 0x30 address 0x51 val 0x02 address 0x52 val 0x5d address 0x53 val 0x07 address 0x54 val 0x11 address 0x55 val 0x01 Example with a 2-byte firmware file written in DP0 address 0x22 od -x /lib/firmware/test_firmware 0000000 0a37 0000002 cd /sys/kernel/debug/soundwire/master-0-0/sdw\:0\:025d\:0711\:01/ echo 0 > command echo 2 > num_bytes echo 0x22 > start_address echo "test_firmware" > firmware_file echo 1 > go cd /sys/kernel/debug/soundwire/master-0-0/sdw\:0\:025d\:0711\:01/ echo 1 > command echo 2 > num_bytes echo 0x22 > start_address echo 1 > go cat read_buffer address 0x22 val 0x37 address 0x23 val 0x0a Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/debugfs.c | 150 ++++++++++++++++++++++++++++++++++++ 1 file changed, 150 insertions(+) diff --git a/drivers/soundwire/debugfs.c b/drivers/soundwire/debugfs.c index 67abd7e52f092a..6d253d69871d90 100644 --- a/drivers/soundwire/debugfs.c +++ b/drivers/soundwire/debugfs.c @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -137,6 +138,145 @@ static int sdw_slave_reg_show(struct seq_file *s_file, void *data) } DEFINE_SHOW_ATTRIBUTE(sdw_slave_reg); +#define MAX_CMD_BYTES 256 + +static int cmd; +static u32 start_addr; +static size_t num_bytes; +static u8 read_buffer[MAX_CMD_BYTES]; +static char *firmware_file; + +static int set_command(void *data, u64 value) +{ + struct sdw_slave *slave = data; + + if (value > 1) + return -EINVAL; + + /* Userspace changed the hardware state behind the kernel's back */ + add_taint(TAINT_USER, LOCKDEP_STILL_OK); + + dev_dbg(&slave->dev, "command: %s\n", value ? "read" : "write"); + cmd = value; + + return 0; +} +DEFINE_DEBUGFS_ATTRIBUTE(set_command_fops, NULL, + set_command, "%llu\n"); + +static int set_start_address(void *data, u64 value) +{ + struct sdw_slave *slave = data; + + /* Userspace changed the hardware state behind the kernel's back */ + add_taint(TAINT_USER, LOCKDEP_STILL_OK); + + dev_dbg(&slave->dev, "start address %#llx\n", value); + + start_addr = value; + + return 0; +} +DEFINE_DEBUGFS_ATTRIBUTE(set_start_address_fops, NULL, + set_start_address, "%llu\n"); + +static int set_num_bytes(void *data, u64 value) +{ + struct sdw_slave *slave = data; + + if (value == 0 || value > MAX_CMD_BYTES) + return -EINVAL; + + /* Userspace changed the hardware state behind the kernel's back */ + add_taint(TAINT_USER, LOCKDEP_STILL_OK); + + dev_dbg(&slave->dev, "number of bytes %lld\n", value); + + num_bytes = value; + + return 0; +} +DEFINE_DEBUGFS_ATTRIBUTE(set_num_bytes_fops, NULL, + set_num_bytes, "%llu\n"); + +static int cmd_go(void *data, u64 value) +{ + struct sdw_slave *slave = data; + int ret; + + if (value != 1) + return -EINVAL; + + /* one last check */ + if (start_addr > SDW_REG_MAX || + num_bytes == 0 || num_bytes > MAX_CMD_BYTES) + return -EINVAL; + + ret = pm_runtime_get_sync(&slave->dev); + if (ret < 0 && ret != -EACCES) { + pm_runtime_put_noidle(&slave->dev); + return ret; + } + + /* Userspace changed the hardware state behind the kernel's back */ + add_taint(TAINT_USER, LOCKDEP_STILL_OK); + + dev_dbg(&slave->dev, "starting command\n"); + + if (cmd == 0) { + const struct firmware *fw; + + ret = request_firmware(&fw, firmware_file, &slave->dev); + if (ret < 0) { + dev_err(&slave->dev, "firmware %s not found\n", firmware_file); + goto out; + } + + if (fw->size != num_bytes) { + dev_err(&slave->dev, + "firmware %s: unexpected size %zd, desired %zd\n", + firmware_file, fw->size, num_bytes); + release_firmware(fw); + goto out; + } + + ret = sdw_nwrite_no_pm(slave, start_addr, num_bytes, fw->data); + release_firmware(fw); + } else { + ret = sdw_nread_no_pm(slave, start_addr, num_bytes, read_buffer); + } + + dev_dbg(&slave->dev, "command completed %d\n", ret); + +out: + pm_runtime_mark_last_busy(&slave->dev); + pm_runtime_put(&slave->dev); + + return ret; +} +DEFINE_DEBUGFS_ATTRIBUTE(cmd_go_fops, NULL, + cmd_go, "%llu\n"); + +#define MAX_LINE_LEN 128 + +static int read_buffer_show(struct seq_file *s_file, void *data) +{ + char buf[MAX_LINE_LEN]; + int i; + + if (num_bytes == 0 || num_bytes > MAX_CMD_BYTES) + return -EINVAL; + + for (i = 0; i < num_bytes; i++) { + scnprintf(buf, MAX_LINE_LEN, "address %#x val 0x%02x\n", + start_addr + i, read_buffer[i]); + seq_printf(s_file, "%s", buf); + } + + return 0; +} +DEFINE_SHOW_ATTRIBUTE(read_buffer); + void sdw_slave_debugfs_init(struct sdw_slave *slave) { struct dentry *master; @@ -151,6 +291,16 @@ void sdw_slave_debugfs_init(struct sdw_slave *slave) debugfs_create_file("registers", 0400, d, slave, &sdw_slave_reg_fops); + /* interface to send arbitrary commands */ + debugfs_create_file("command", 0200, d, slave, &set_command_fops); + debugfs_create_file("start_address", 0200, d, slave, &set_start_address_fops); + debugfs_create_file("num_bytes", 0200, d, slave, &set_num_bytes_fops); + debugfs_create_file("go", 0200, d, slave, &cmd_go_fops); + + debugfs_create_file("read_buffer", 0400, d, slave, &read_buffer_fops); + firmware_file = NULL; + debugfs_create_str("firmware_file", 0200, d, &firmware_file); + slave->debugfs = d; } From 54b0b9726087700c68cacee49529b6d484d4c3cb Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 10 Oct 2023 13:48:37 -0400 Subject: [PATCH 9/9] soundwire: bus: add stream refcount The notion of stream is by construction based on a multi-bus capability, to allow for aggregation of Peripheral devices or functions located on different segments. We currently count how many master_rt contexts are used by a stream, but we don't have the dual refcount of how many streams are allocated on a given bus. This refcount will be useful to check if BTP/BRA streams can be allocated. Note that the stream_refcount is modified in sdw_master_rt_alloc() and sdw_master_rt_free() which are both called with the bus_lock mutex held, so there's no need for refcount_ primitives for additional protection. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 5 +++++ include/linux/soundwire/sdw.h | 2 ++ 2 files changed, 7 insertions(+) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index 9dc6399f206afd..c4ab360e727f20 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1180,6 +1180,8 @@ static struct sdw_master_runtime m_rt->bus = bus; m_rt->stream = stream; + bus->stream_refcount++; + return m_rt; } @@ -1216,6 +1218,7 @@ static void sdw_master_rt_free(struct sdw_master_runtime *m_rt, struct sdw_stream_runtime *stream) { struct sdw_slave_runtime *s_rt, *_s_rt; + struct sdw_bus *bus = m_rt->bus; list_for_each_entry_safe(s_rt, _s_rt, &m_rt->slave_rt_list, m_rt_node) { sdw_slave_port_free(s_rt->slave, stream); @@ -1225,6 +1228,8 @@ static void sdw_master_rt_free(struct sdw_master_runtime *m_rt, list_del(&m_rt->stream_node); list_del(&m_rt->bus_node); kfree(m_rt); + + bus->stream_refcount--; } /** diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index e3a4bccc2a7ef5..71a7031f7b3a2b 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -902,6 +902,7 @@ struct sdw_master_ops { * meaningful if multi_link is set. If set to 1, hardware-based * synchronization will be used even if a stream only uses a single * SoundWire segment. + * @stream_refcount: number of streams currently using this bus */ struct sdw_bus { struct device *dev; @@ -931,6 +932,7 @@ struct sdw_bus { u32 bank_switch_timeout; bool multi_link; int hw_sync_min_links; + int stream_refcount; }; int sdw_bus_master_add(struct sdw_bus *bus, struct device *parent,