2024-06-03 06:59:35

by Bard Liao

[permalink] [raw]
Subject: [PATCH RESEND 1/2] soundwire: debugfs: add interface to read/write commands

From: Pierre-Louis Bossart <[email protected]>

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 <[email protected]>
Reviewed-by: Rander Wang <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
---
drivers/soundwire/debugfs.c | 150 ++++++++++++++++++++++++++++++++++++
1 file changed, 150 insertions(+)

diff --git a/drivers/soundwire/debugfs.c b/drivers/soundwire/debugfs.c
index 67abd7e52f09..6d253d69871d 100644
--- a/drivers/soundwire/debugfs.c
+++ b/drivers/soundwire/debugfs.c
@@ -3,6 +3,7 @@

#include <linux/device.h>
#include <linux/debugfs.h>
+#include <linux/firmware.h>
#include <linux/mod_devicetable.h>
#include <linux/pm_runtime.h>
#include <linux/slab.h>
@@ -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;
}

--
2.40.1



2024-06-03 06:59:44

by Bard Liao

[permalink] [raw]
Subject: [PATCH RESEND 2/2] soundwire: bus: add stream refcount

From: Pierre-Louis Bossart <[email protected]>

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 <[email protected]>
Reviewed-by: Rander Wang <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
---
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 4e9e7d2a942d..7aa4900dcf31 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1181,6 +1181,8 @@ static struct sdw_master_runtime
m_rt->bus = bus;
m_rt->stream = stream;

+ bus->stream_refcount++;
+
return m_rt;
}

@@ -1217,6 +1219,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);
@@ -1226,6 +1229,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 13e96d8b7423..94fc1b57c57b 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -903,6 +903,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;
@@ -933,6 +934,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,
--
2.40.1


2024-06-03 12:11:27

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH RESEND 1/2] soundwire: debugfs: add interface to read/write commands


On Mon, 03 Jun 2024 14:58:40 +0800, Bard Liao wrote:
> 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.
>
> [...]

Applied, thanks!

[1/2] soundwire: debugfs: add interface to read/write commands
commit: fe46d2a4301de1299fb32c0317ec316706ceaad6
[2/2] soundwire: bus: add stream refcount
commit: a5b7365f28c191df6b93f60942d2b9a9fe71746c

Best regards,
--
Vinod Koul <[email protected]>