[alsa-devel] [PATCH 0/3] soundwire: debugfs support for 5.4
This patchset enables debugfs support and corrects all the feedback provided on an earlier RFC ('soundwire: updates for 5.4')
There is one remaining hard-coded value in intel.c that will need to be fixed in a follow-up patchset not specific to debugfs: we need to remove hard-coded Intel-specific configurations from cadence_master.c (PDI offsets, etc).
Changes since RFC (Feedback from GKH, Vinod, Guennadi, Cezary, Sanyog): removed error checks used DEFINE_SHOW_ATTRIBUTE and seq_file fixed copyright dates fixed SPDX license info to use GPL2.0 only fixed Makefile to include debugfs only if CONFIG_DEBUG_FS is selected used static inlines for fallback compilation removed intermediate variables removed hard-coded constants in loops (used registers offsets and hardware capabilities) squashed patch 3
Pierre-Louis Bossart (3): soundwire: add debugfs support soundwire: cadence_master: add debugfs register dump soundwire: intel: add debugfs register dump
drivers/soundwire/Makefile | 5 + drivers/soundwire/bus.c | 6 ++ drivers/soundwire/bus.h | 24 +++++ drivers/soundwire/bus_type.c | 3 + drivers/soundwire/cadence_master.c | 104 ++++++++++++++++++++ drivers/soundwire/cadence_master.h | 2 + drivers/soundwire/debugfs.c | 151 +++++++++++++++++++++++++++++ drivers/soundwire/intel.c | 114 ++++++++++++++++++++++ drivers/soundwire/slave.c | 1 + include/linux/soundwire/sdw.h | 4 + 10 files changed, 414 insertions(+) create mode 100644 drivers/soundwire/debugfs.c
Add base debugfs mechanism for SoundWire bus by creating soundwire root and master-N and slave-x hierarchy.
Also add SDW Slave SCP, DP0 and DP-N register debug file.
Registers not implemented will print as "XX"
Credits: this patch is based on an earlier internal contribution by Vinod Koul, Sanyog Kale, Shreyas Nc and Hardik Shah.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- drivers/soundwire/Makefile | 5 ++ drivers/soundwire/bus.c | 6 ++ drivers/soundwire/bus.h | 24 ++++++ drivers/soundwire/bus_type.c | 3 + drivers/soundwire/debugfs.c | 151 ++++++++++++++++++++++++++++++++++ drivers/soundwire/slave.c | 1 + include/linux/soundwire/sdw.h | 4 + 7 files changed, 194 insertions(+) create mode 100644 drivers/soundwire/debugfs.c
diff --git a/drivers/soundwire/Makefile b/drivers/soundwire/Makefile index fd99a831b92a..05d4cb9ef7d6 100644 --- a/drivers/soundwire/Makefile +++ b/drivers/soundwire/Makefile @@ -5,6 +5,11 @@
#Bus Objs soundwire-bus-objs := bus_type.o bus.o slave.o mipi_disco.o stream.o + +ifdef CONFIG_DEBUG_FS +soundwire-bus-objs += debugfs.o +endif + obj-$(CONFIG_SOUNDWIRE_BUS) += soundwire-bus.o
#Cadence Objs diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 49f64b2115b9..89d5f1537d9b 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -49,6 +49,8 @@ int sdw_add_bus_master(struct sdw_bus *bus) } }
+ bus->debugfs = sdw_bus_debugfs_init(bus); + /* * Device numbers in SoundWire are 0 through 15. Enumeration device * number (0), Broadcast device number (15), Group numbers (12 and @@ -109,6 +111,8 @@ static int sdw_delete_slave(struct device *dev, void *data) struct sdw_slave *slave = dev_to_sdw_dev(dev); struct sdw_bus *bus = slave->bus;
+ sdw_slave_debugfs_exit(slave->debugfs); + mutex_lock(&bus->bus_lock);
if (slave->dev_num) /* clear dev_num if assigned */ @@ -130,6 +134,8 @@ static int sdw_delete_slave(struct device *dev, void *data) void sdw_delete_bus_master(struct sdw_bus *bus) { device_for_each_child(bus->dev, NULL, sdw_delete_slave); + + sdw_bus_debugfs_exit(bus->debugfs); } EXPORT_SYMBOL(sdw_delete_bus_master);
diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h index 3048ca153f22..648436a995a3 100644 --- a/drivers/soundwire/bus.h +++ b/drivers/soundwire/bus.h @@ -18,6 +18,30 @@ static inline int sdw_acpi_find_slaves(struct sdw_bus *bus) void sdw_extract_slave_id(struct sdw_bus *bus, u64 addr, struct sdw_slave_id *id);
+#ifdef CONFIG_DEBUG_FS +struct dentry *sdw_bus_debugfs_init(struct sdw_bus *bus); +void sdw_bus_debugfs_exit(struct dentry *d); +struct dentry *sdw_slave_debugfs_init(struct sdw_slave *slave); +void sdw_slave_debugfs_exit(struct dentry *d); +void sdw_debugfs_init(void); +void sdw_debugfs_exit(void); +#else +static inline struct dentry *sdw_bus_debugfs_init(struct sdw_bus *bus) +{ return NULL; } + +static inline void sdw_bus_debugfs_exit(struct dentry *d) {} + +static inline struct dentry *sdw_slave_debugfs_init(struct sdw_slave *slave) +{ return NULL; } + +static inline void sdw_slave_debugfs_exit(struct dentry *d) {} + +static inline void sdw_debugfs_init(void) {} + +static inline void sdw_debugfs_exit(void) {} + +#endif + enum { SDW_MSG_FLAG_READ = 0, SDW_MSG_FLAG_WRITE, diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c index 2655602f0cfb..4a465f55039f 100644 --- a/drivers/soundwire/bus_type.c +++ b/drivers/soundwire/bus_type.c @@ -6,6 +6,7 @@ #include <linux/pm_domain.h> #include <linux/soundwire/sdw.h> #include <linux/soundwire/sdw_type.h> +#include "bus.h"
/** * sdw_get_device_id - find the matching SoundWire device id @@ -177,11 +178,13 @@ EXPORT_SYMBOL_GPL(sdw_unregister_driver);
static int __init sdw_bus_init(void) { + sdw_debugfs_init(); return bus_register(&sdw_bus_type); }
static void __exit sdw_bus_exit(void) { + sdw_debugfs_exit(); bus_unregister(&sdw_bus_type); }
diff --git a/drivers/soundwire/debugfs.c b/drivers/soundwire/debugfs.c new file mode 100644 index 000000000000..200eb98ec99b --- /dev/null +++ b/drivers/soundwire/debugfs.c @@ -0,0 +1,151 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright(c) 2017-2019 Intel Corporation. + +#include <linux/device.h> +#include <linux/debugfs.h> +#include <linux/mod_devicetable.h> +#include <linux/slab.h> +#include <linux/soundwire/sdw.h> +#include <linux/soundwire/sdw_registers.h> +#include "bus.h" + +static struct dentry *sdw_debugfs_root; + +struct dentry *sdw_bus_debugfs_init(struct sdw_bus *bus) +{ + char name[16]; + + if (!sdw_debugfs_root) + return NULL; + + /* create the debugfs master-N */ + snprintf(name, sizeof(name), "master-%d", bus->link_id); + return debugfs_create_dir(name, sdw_debugfs_root); +} + +void sdw_bus_debugfs_exit(struct dentry *d) +{ + debugfs_remove_recursive(d); +} + +#define RD_BUF (3 * PAGE_SIZE) + +static ssize_t sdw_sprintf(struct sdw_slave *slave, + char *buf, size_t pos, unsigned int reg) +{ + int value; + + value = sdw_read(slave, reg); + + if (value < 0) + return scnprintf(buf + pos, RD_BUF - pos, "%3x\tXX\n", reg); + else + return scnprintf(buf + pos, RD_BUF - pos, + "%3x\t%2x\n", reg, value); +} + +static int sdw_slave_reg_show(struct seq_file *s_file, void *data) +{ + struct sdw_slave *slave = s_file->private; + char *buf; + ssize_t ret; + int i, j; + + buf = kzalloc(RD_BUF, GFP_KERNEL); + if (!buf) + return -ENOMEM; + + ret = scnprintf(buf, RD_BUF, "Register Value\n"); + + /* DP0 non-banked registers */ + ret += scnprintf(buf + ret, RD_BUF - ret, "\nDP0\n"); + for (i = SDW_DP0_INT; i <= SDW_DP0_PREPARECTRL; i++) + ret += sdw_sprintf(slave, buf, ret, i); + + /* DP0 Bank 0 registers */ + ret += scnprintf(buf + ret, RD_BUF - ret, "Bank0\n"); + ret += sdw_sprintf(slave, buf, ret, SDW_DP0_CHANNELEN); + for (i = SDW_DP0_SAMPLECTRL1; i <= SDW_DP0_LANECTRL; i++) + ret += sdw_sprintf(slave, buf, ret, i); + + /* DP0 Bank 1 registers */ + ret += scnprintf(buf + ret, RD_BUF - ret, "Bank1\n"); + ret += sdw_sprintf(slave, buf, ret, + SDW_DP0_CHANNELEN + SDW_BANK1_OFFSET); + for (i = SDW_DP0_SAMPLECTRL1 + SDW_BANK1_OFFSET; + i <= SDW_DP0_LANECTRL + SDW_BANK1_OFFSET; i++) + ret += sdw_sprintf(slave, buf, ret, i); + + /* SCP registers */ + ret += scnprintf(buf + ret, RD_BUF - ret, "\nSCP\n"); + for (i = SDW_SCP_INT1; i <= SDW_SCP_BANKDELAY; i++) + ret += sdw_sprintf(slave, buf, ret, i); + for (i = SDW_SCP_DEVID_0; i <= SDW_SCP_DEVID_5; i++) + ret += sdw_sprintf(slave, buf, ret, i); + + /* + * SCP Bank 0/1 registers are read-only and cannot be + * retrieved from the Slave. The Master typically keeps track + * of the current frame size so the information can be found + * in other places + */ + + /* DP1..14 registers */ + for (i = 1; SDW_VALID_PORT_RANGE(i); i++) { + + /* DPi registers */ + ret += scnprintf(buf + ret, RD_BUF - ret, "\nDP%d\n", i); + for (j = SDW_DPN_INT(i); j <= SDW_DPN_PREPARECTRL(i); j++) + ret += sdw_sprintf(slave, buf, ret, j); + + /* DPi Bank0 registers */ + ret += scnprintf(buf + ret, RD_BUF - ret, "Bank0\n"); + for (j = SDW_DPN_CHANNELEN_B0(i); + j <= SDW_DPN_LANECTRL_B0(i); j++) + ret += sdw_sprintf(slave, buf, ret, j); + + /* DPi Bank1 registers */ + ret += scnprintf(buf + ret, RD_BUF - ret, "Bank1\n"); + for (j = SDW_DPN_CHANNELEN_B1(i); + j <= SDW_DPN_LANECTRL_B1(i); j++) + ret += sdw_sprintf(slave, buf, ret, j); + } + + seq_printf(s_file, "%s", buf); + kfree(buf); + + return 0; +} +DEFINE_SHOW_ATTRIBUTE(sdw_slave_reg); + +struct dentry *sdw_slave_debugfs_init(struct sdw_slave *slave) +{ + struct dentry *master; + struct dentry *d; + char name[32]; + + master = slave->bus->debugfs; + + /* create the debugfs slave-name */ + snprintf(name, sizeof(name), "%s", dev_name(&slave->dev)); + d = debugfs_create_dir(name, master); + + debugfs_create_file("registers", 0400, d, slave, &sdw_slave_reg_fops); + + return d; +} + +void sdw_slave_debugfs_exit(struct dentry *d) +{ + debugfs_remove_recursive(d); +} + +void sdw_debugfs_init(void) +{ + sdw_debugfs_root = debugfs_create_dir("soundwire", NULL); +} + +void sdw_debugfs_exit(void) +{ + debugfs_remove_recursive(sdw_debugfs_root); +} diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c index f39a5815e25d..34d8bb995f45 100644 --- a/drivers/soundwire/slave.c +++ b/drivers/soundwire/slave.c @@ -56,6 +56,7 @@ static int sdw_slave_add(struct sdw_bus *bus, mutex_unlock(&bus->bus_lock); put_device(&slave->dev); } + slave->debugfs = sdw_slave_debugfs_init(slave);
return ret; } diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index 3b231472464a..a49028e9d666 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -544,6 +544,7 @@ struct sdw_slave_ops { * @bus: Bus handle * @ops: Slave callback ops * @prop: Slave properties + * @debugfs: Slave debugfs * @node: node for bus list * @port_ready: Port ready completion flag for each Slave port * @dev_num: Device Number assigned by Bus @@ -555,6 +556,7 @@ struct sdw_slave { struct sdw_bus *bus; const struct sdw_slave_ops *ops; struct sdw_slave_prop prop; + struct dentry *debugfs; struct list_head node; struct completion *port_ready; u16 dev_num; @@ -731,6 +733,7 @@ struct sdw_master_ops { * @m_rt_list: List of Master instance of all stream(s) running on Bus. This * is used to compute and program bus bandwidth, clock, frame shape, * transport and port parameters + * @debugfs: Bus debugfs * @defer_msg: Defer message * @clk_stop_timeout: Clock stop timeout computed * @bank_switch_timeout: Bank switch timeout computed @@ -750,6 +753,7 @@ struct sdw_bus { struct sdw_bus_params params; struct sdw_master_prop prop; struct list_head m_rt_list; + struct dentry *debugfs; struct sdw_defer defer_msg; unsigned int clk_stop_timeout; u32 bank_switch_timeout;
On Fri, Aug 09, 2019 at 05:43:39PM -0500, Pierre-Louis Bossart wrote:
Add base debugfs mechanism for SoundWire bus by creating soundwire root and master-N and slave-x hierarchy.
Also add SDW Slave SCP, DP0 and DP-N register debug file.
Registers not implemented will print as "XX"
Credits: this patch is based on an earlier internal contribution by Vinod Koul, Sanyog Kale, Shreyas Nc and Hardik Shah.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
drivers/soundwire/Makefile | 5 ++ drivers/soundwire/bus.c | 6 ++ drivers/soundwire/bus.h | 24 ++++++ drivers/soundwire/bus_type.c | 3 + drivers/soundwire/debugfs.c | 151 ++++++++++++++++++++++++++++++++++ drivers/soundwire/slave.c | 1 + include/linux/soundwire/sdw.h | 4 + 7 files changed, 194 insertions(+) create mode 100644 drivers/soundwire/debugfs.c
diff --git a/drivers/soundwire/Makefile b/drivers/soundwire/Makefile index fd99a831b92a..05d4cb9ef7d6 100644 --- a/drivers/soundwire/Makefile +++ b/drivers/soundwire/Makefile @@ -5,6 +5,11 @@
#Bus Objs soundwire-bus-objs := bus_type.o bus.o slave.o mipi_disco.o stream.o
+ifdef CONFIG_DEBUG_FS +soundwire-bus-objs += debugfs.o +endif
obj-$(CONFIG_SOUNDWIRE_BUS) += soundwire-bus.o
#Cadence Objs diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 49f64b2115b9..89d5f1537d9b 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -49,6 +49,8 @@ int sdw_add_bus_master(struct sdw_bus *bus) } }
- bus->debugfs = sdw_bus_debugfs_init(bus);
It's "nicer" to just put that assignment into sdw_bus_debugfs_init().
That way you just call the function, no need to return anything.
/* * Device numbers in SoundWire are 0 through 15. Enumeration device * number (0), Broadcast device number (15), Group numbers (12 and @@ -109,6 +111,8 @@ static int sdw_delete_slave(struct device *dev, void *data) struct sdw_slave *slave = dev_to_sdw_dev(dev); struct sdw_bus *bus = slave->bus;
- sdw_slave_debugfs_exit(slave->debugfs);
Same here, just pass in slave: sdw_slave_debugfs_exit(slave); and have that function remove the debugfs entry in the structure. That way, if you are really paranoid about size, you could even drop the debugfs structure member from non-debugfs builds without any changes to bus.c or other non-debugfs files.
thanks,
greg k-h
#Cadence Objs diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 49f64b2115b9..89d5f1537d9b 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -49,6 +49,8 @@ int sdw_add_bus_master(struct sdw_bus *bus) } }
- bus->debugfs = sdw_bus_debugfs_init(bus);
It's "nicer" to just put that assignment into sdw_bus_debugfs_init().
That way you just call the function, no need to return anything.
ok, thanks for the suggestion.
/* * Device numbers in SoundWire are 0 through 15. Enumeration device * number (0), Broadcast device number (15), Group numbers (12 and @@ -109,6 +111,8 @@ static int sdw_delete_slave(struct device *dev, void *data) struct sdw_slave *slave = dev_to_sdw_dev(dev); struct sdw_bus *bus = slave->bus;
- sdw_slave_debugfs_exit(slave->debugfs);
Same here, just pass in slave: sdw_slave_debugfs_exit(slave); and have that function remove the debugfs entry in the structure. That way, if you are really paranoid about size, you could even drop the debugfs structure member from non-debugfs builds without any changes to bus.c or other non-debugfs files.
ok makes sense. will fix in v2.
Add debugfs file to dump the Cadence master registers
Credits: this patch is based on an earlier internal contribution by Vinod Koul, Sanyog Kale, Shreyas Nc and Hardik Shah.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- drivers/soundwire/cadence_master.c | 104 +++++++++++++++++++++++++++++ drivers/soundwire/cadence_master.h | 2 + 2 files changed, 106 insertions(+)
diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 665632dbdeb5..4c65b78adcf9 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -8,6 +8,7 @@
#include <linux/delay.h> #include <linux/device.h> +#include <linux/debugfs.h> #include <linux/interrupt.h> #include <linux/io.h> #include <linux/module.h> @@ -223,6 +224,109 @@ static int cdns_clear_bit(struct sdw_cdns *cdns, int offset, u32 value) return -EAGAIN; }
+/* + * debugfs + */ + +#define RD_BUF (2 * PAGE_SIZE) + +static ssize_t cdns_sprintf(struct sdw_cdns *cdns, + char *buf, size_t pos, unsigned int reg) +{ + return scnprintf(buf + pos, RD_BUF - pos, + "%4x\t%8x\n", reg, cdns_readl(cdns, reg)); +} + +static int cdns_reg_show(struct seq_file *s, void *data) +{ + struct sdw_cdns *cdns = s->private; + char *buf; + ssize_t ret; + int num_ports; + int i, j; + + buf = kzalloc(RD_BUF, GFP_KERNEL); + if (!buf) + return -ENOMEM; + + ret = scnprintf(buf, RD_BUF, "Register Value\n"); + ret += scnprintf(buf + ret, RD_BUF - ret, "\nMCP Registers\n"); + /* 8 MCP registers */ + for (i = CDNS_MCP_CONFIG; i <= CDNS_MCP_PHYCTRL; i += sizeof(u32)) + ret += cdns_sprintf(cdns, buf, ret, i); + + ret += scnprintf(buf + ret, RD_BUF - ret, + "\nStatus & Intr Registers\n"); + /* 13 Status & Intr registers (offsets 0x70 and 0x74 not defined) */ + for (i = CDNS_MCP_STAT; i <= CDNS_MCP_FIFOSTAT; i += sizeof(u32)) + ret += cdns_sprintf(cdns, buf, ret, i); + + ret += scnprintf(buf + ret, RD_BUF - ret, + "\nSSP & Clk ctrl Registers\n"); + ret += cdns_sprintf(cdns, buf, ret, CDNS_MCP_SSP_CTRL0); + ret += cdns_sprintf(cdns, buf, ret, CDNS_MCP_SSP_CTRL1); + ret += cdns_sprintf(cdns, buf, ret, CDNS_MCP_CLK_CTRL0); + ret += cdns_sprintf(cdns, buf, ret, CDNS_MCP_CLK_CTRL1); + + ret += scnprintf(buf + ret, RD_BUF - ret, + "\nDPn B0 Registers\n"); + + /* + * in sdw_cdns_pdi_init() we filter out the Bulk PDIs, + * so the indices need to be corrected again + */ + num_ports = cdns->num_ports + CDNS_PCM_PDI_OFFSET; + + for (i = 0; i < num_ports; i++) { + ret += scnprintf(buf + ret, RD_BUF - ret, + "\nDP-%d\n", i); + for (j = CDNS_DPN_B0_CONFIG(i); + j < CDNS_DPN_B0_ASYNC_CTRL(i); j += sizeof(u32)) + ret += cdns_sprintf(cdns, buf, ret, j); + } + + ret += scnprintf(buf + ret, RD_BUF - ret, + "\nDPn B1 Registers\n"); + for (i = 0; i < num_ports; i++) { + ret += scnprintf(buf + ret, RD_BUF - ret, + "\nDP-%d\n", i); + + for (j = CDNS_DPN_B1_CONFIG(i); + j < CDNS_DPN_B1_ASYNC_CTRL(i); j += sizeof(u32)) + ret += cdns_sprintf(cdns, buf, ret, j); + } + + ret += scnprintf(buf + ret, RD_BUF - ret, + "\nDPn Control Registers\n"); + for (i = 0; i < num_ports; i++) + ret += cdns_sprintf(cdns, buf, ret, + CDNS_PORTCTRL + i * CDNS_PORT_OFFSET); + + ret += scnprintf(buf + ret, RD_BUF - ret, + "\nPDIn Config Registers\n"); + + /* number of PDI and ports is interchangeable */ + for (i = 0; i < num_ports; i++) + ret += cdns_sprintf(cdns, buf, ret, CDNS_PDI_CONFIG(i)); + + seq_printf(s, "%s", buf); + kfree(buf); + + return 0; +} +DEFINE_SHOW_ATTRIBUTE(cdns_reg); + +/** + * sdw_cdns_debugfs_init() - Cadence debugfs init + * @cdns: Cadence instance + * @root: debugfs root + */ +void sdw_cdns_debugfs_init(struct sdw_cdns *cdns, struct dentry *root) +{ + debugfs_create_file("cdns-registers", 0400, root, cdns, &cdns_reg_fops); +} +EXPORT_SYMBOL_GPL(sdw_cdns_debugfs_init); + /* * IO Calls */ diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h index fe2af62958b1..c0bf6ff00a44 100644 --- a/drivers/soundwire/cadence_master.h +++ b/drivers/soundwire/cadence_master.h @@ -163,6 +163,8 @@ int sdw_cdns_pdi_init(struct sdw_cdns *cdns, struct sdw_cdns_stream_config config); int sdw_cdns_enable_interrupt(struct sdw_cdns *cdns);
+void sdw_cdns_debugfs_init(struct sdw_cdns *cdns, struct dentry *root); + int sdw_cdns_get_stream(struct sdw_cdns *cdns, struct sdw_cdns_streams *stream, u32 ch, u32 dir);
On Fri, Aug 09, 2019 at 05:43:40PM -0500, Pierre-Louis Bossart wrote:
+/**
- sdw_cdns_debugfs_init() - Cadence debugfs init
- @cdns: Cadence instance
- @root: debugfs root
- */
+void sdw_cdns_debugfs_init(struct sdw_cdns *cdns, struct dentry *root) +{
- debugfs_create_file("cdns-registers", 0400, root, cdns, &cdns_reg_fops);
+} +EXPORT_SYMBOL_GPL(sdw_cdns_debugfs_init);
You create this function but never actually call it. Don't add apis that no one uses :(
thanks,
greg k-h
On 8/10/19 2:03 AM, Greg KH wrote:
On Fri, Aug 09, 2019 at 05:43:40PM -0500, Pierre-Louis Bossart wrote:
+/**
- sdw_cdns_debugfs_init() - Cadence debugfs init
- @cdns: Cadence instance
- @root: debugfs root
- */
+void sdw_cdns_debugfs_init(struct sdw_cdns *cdns, struct dentry *root) +{
- debugfs_create_file("cdns-registers", 0400, root, cdns, &cdns_reg_fops);
+} +EXPORT_SYMBOL_GPL(sdw_cdns_debugfs_init);
You create this function but never actually call it. Don't add apis that no one uses :(
it is used in the follow-up patch.
+static void intel_debugfs_init(struct sdw_intel *sdw) +{ + struct dentry *root = sdw->cdns.bus.debugfs; + + if (!root) + return; + + sdw->fs = debugfs_create_dir("intel-sdw", root); + + debugfs_create_file("intel-registers", 0400, sdw->fs, sdw, + &intel_reg_fops); + + sdw_cdns_debugfs_init(&sdw->cdns, sdw->fs); <<< HERE! +}
The Cadence IP might be used by others so having a function that can be exported and used by others seems useful.
Add debugfs file to dump the Intel SoundWire registers
Credits: this patch is based on an earlier internal contribution by Vinod Koul, Sanyog Kale, Shreyas Nc and Hardik Shah.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- drivers/soundwire/intel.c | 114 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index c82ca4e13de7..9ae69dad1437 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -6,6 +6,7 @@ */
#include <linux/acpi.h> +#include <linux/debugfs.h> #include <linux/delay.h> #include <linux/module.h> #include <linux/interrupt.h> @@ -16,6 +17,7 @@ #include <linux/soundwire/sdw.h> #include <linux/soundwire/sdw_intel.h> #include "cadence_master.h" +#include "bus.h" #include "intel.h"
/* Intel SHIM Registers Definition */ @@ -83,6 +85,7 @@
/* Intel ALH Register definitions */ #define SDW_ALH_STRMZCFG(x) (0x000 + (0x4 * (x))) +#define SDW_ALH_NUM_STREAMS 64
#define SDW_ALH_STRMZCFG_DMAT_VAL 0x3 #define SDW_ALH_STRMZCFG_DMAT GENMASK(7, 0) @@ -98,6 +101,7 @@ struct sdw_intel { struct sdw_cdns cdns; int instance; struct sdw_intel_link_res *res; + struct dentry *fs; };
#define cdns_to_intel(_cdns) container_of(_cdns, struct sdw_intel, cdns) @@ -161,6 +165,113 @@ static int intel_set_bit(void __iomem *base, int offset, u32 value, u32 mask) return -EAGAIN; }
+/* + * debugfs + */ + +#define RD_BUF (2 * PAGE_SIZE) + +static ssize_t intel_sprintf(void __iomem *mem, bool l, + char *buf, size_t pos, unsigned int reg) +{ + int value; + + if (l) + value = intel_readl(mem, reg); + else + value = intel_readw(mem, reg); + + return scnprintf(buf + pos, RD_BUF - pos, "%4x\t%4x\n", reg, value); +} + +static int intel_reg_show(struct seq_file *s_file, void *data) +{ + struct sdw_intel *sdw = s_file->private; + void __iomem *s = sdw->res->shim; + void __iomem *a = sdw->res->alh; + char *buf; + ssize_t ret; + int i, j; + unsigned int links, reg; + + buf = kzalloc(RD_BUF, GFP_KERNEL); + if (!buf) + return -ENOMEM; + + links = intel_readl(s, SDW_SHIM_LCAP) & GENMASK(2, 0); + + ret = scnprintf(buf, RD_BUF, "Register Value\n"); + ret += scnprintf(buf + ret, RD_BUF - ret, "\nShim\n"); + + for (i = 0; i < links; i++) { + reg = SDW_SHIM_LCAP + i * 4; + ret += intel_sprintf(s, true, buf, ret, reg); + } + + for (i = 0; i < links; i++) { + ret += scnprintf(buf + ret, RD_BUF - ret, "\nLink%d\n", i); + ret += intel_sprintf(s, false, buf, ret, SDW_SHIM_CTLSCAP(i)); + ret += intel_sprintf(s, false, buf, ret, SDW_SHIM_CTLS0CM(i)); + ret += intel_sprintf(s, false, buf, ret, SDW_SHIM_CTLS1CM(i)); + ret += intel_sprintf(s, false, buf, ret, SDW_SHIM_CTLS2CM(i)); + ret += intel_sprintf(s, false, buf, ret, SDW_SHIM_CTLS3CM(i)); + ret += intel_sprintf(s, false, buf, ret, SDW_SHIM_PCMSCAP(i)); + + ret += scnprintf(buf + ret, RD_BUF - ret, "\n PCMSyCH registers\n"); + + /* + * the value 10 is the number of PDIs. We will need a + * cleanup to remove hard-coded Intel configurations + * from cadence_master.c + */ + for (j = 0; j < 10; j++) { + ret += intel_sprintf(s, false, buf, ret, + SDW_SHIM_PCMSYCHM(i, j)); + ret += intel_sprintf(s, false, buf, ret, + SDW_SHIM_PCMSYCHC(i, j)); + } + ret += scnprintf(buf + ret, RD_BUF - ret, "\n PDMSCAP, IOCTL, CTMCTL\n"); + + ret += intel_sprintf(s, false, buf, ret, SDW_SHIM_PDMSCAP(i)); + ret += intel_sprintf(s, false, buf, ret, SDW_SHIM_IOCTL(i)); + ret += intel_sprintf(s, false, buf, ret, SDW_SHIM_CTMCTL(i)); + } + + ret += scnprintf(buf + ret, RD_BUF - ret, "\nWake registers\n"); + ret += intel_sprintf(s, false, buf, ret, SDW_SHIM_WAKEEN); + ret += intel_sprintf(s, false, buf, ret, SDW_SHIM_WAKESTS); + + ret += scnprintf(buf + ret, RD_BUF - ret, "\nALH STRMzCFG\n"); + for (i = 0; i < SDW_ALH_NUM_STREAMS; i++) + ret += intel_sprintf(a, true, buf, ret, SDW_ALH_STRMZCFG(i)); + + seq_printf(s_file, "%s", buf); + kfree(buf); + + return 0; +} +DEFINE_SHOW_ATTRIBUTE(intel_reg); + +static void intel_debugfs_init(struct sdw_intel *sdw) +{ + struct dentry *root = sdw->cdns.bus.debugfs; + + if (!root) + return; + + sdw->fs = debugfs_create_dir("intel-sdw", root); + + debugfs_create_file("intel-registers", 0400, sdw->fs, sdw, + &intel_reg_fops); + + sdw_cdns_debugfs_init(&sdw->cdns, sdw->fs); +} + +static void intel_debugfs_exit(struct sdw_intel *sdw) +{ + debugfs_remove_recursive(sdw->fs); +} + /* * shim ops */ @@ -885,6 +996,8 @@ static int intel_probe(struct platform_device *pdev) goto err_dai; }
+ intel_debugfs_init(sdw); + return 0;
err_dai: @@ -901,6 +1014,7 @@ static int intel_remove(struct platform_device *pdev)
sdw = platform_get_drvdata(pdev);
+ intel_debugfs_exit(sdw); free_irq(sdw->res->irq, sdw); snd_soc_unregister_component(sdw->cdns.dev); sdw_delete_bus_master(&sdw->cdns.bus);
On Fri, Aug 09, 2019 at 05:43:41PM -0500, Pierre-Louis Bossart wrote:
@@ -885,6 +996,8 @@ static int intel_probe(struct platform_device *pdev) goto err_dai; }
- intel_debugfs_init(sdw);
See, make patch 1/3 work the same like this. It's much cleaner.
thanks,
greg k-h
participants (2)
-
Greg KH -
Pierre-Louis Bossart