On 21-01-21, 17:23, Srinivas Kandagatla wrote:
On 21/01/2021 15:12, Pierre-Louis Bossart wrote:
On 1/21/21 6:03 AM, Srinivas Kandagatla wrote:
On 19/01/2021 19:09, Pierre-Louis Bossart wrote:
currently we have /sys/kernel/debug/soundwire/master-*
Are you suggesting that we have something like this:
/sys/kernel/debug/soundwire/xyz-controller/master-<LINK-ID> ??
Yes this is what I was thinking about.
Vinod/Pierre,
One Question here,
Why is link_id part of "struct sdw_bus", should it not be part of "struct sdw_master_device " ?
Given that "There is one Link per each Master"
it's true that link == master == bus at the concept level.
but we have an existing code base with different structures and we can't break too many things at once.
In the existing flow, the 'bus' is created and setup first, the sdw_bus_master_add() routine takes a 'bus' argument, and the link_id is already set. This routine only creates a device and in the rest of the code we keep using the 'bus' pointer, so there's no real short-term scope for moving the information into the 'sdw_master_device' structure
- that would be a lot of surgery when nothing is really broken.
I totally agree!
If I understand it correctly in Intel case there will be only one Link ID per bus.
Yes IIUC there would be one link id per bus.
the ida approach gives us unique id for each master,bus I would like to propose using that everywhere
Does this change look good to you?
---------------->cut<---------------
diff --git a/drivers/soundwire/debugfs.c b/drivers/soundwire/debugfs.c index b6cad0d59b7b..f22868614f09 100644 --- a/drivers/soundwire/debugfs.c +++ b/drivers/soundwire/debugfs.c @@ -19,13 +19,14 @@ void sdw_bus_debugfs_init(struct sdw_bus *bus) return;
/* create the debugfs master-N */
bus->controller_debugfs = debugfs_create_dir(dev_name(bus->dev),sdw_debugfs_root); snprintf(name, sizeof(name), "master-%d", bus->link_id);
bus->debugfs = debugfs_create_dir(name, sdw_debugfs_root);
bus->debugfs = debugfs_create_dir(name, bus->controller_debugfs);}
void sdw_bus_debugfs_exit(struct sdw_bus *bus) {
debugfs_remove_recursive(bus->debugfs);
debugfs_remove_recursive(bus->controller_debugfs);}
#define RD_BUF (3 * PAGE_SIZE) diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index b198f471bea8..242bde30d8bd 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -877,6 +877,7 @@ struct sdw_bus { struct sdw_master_prop prop; struct list_head m_rt_list; #ifdef CONFIG_DEBUG_FS
struct dentry *controller_debugfs; struct dentry *debugfs;#endif struct sdw_defer defer_msg;
---------------->cut<---------------
With this change I get something like this on my board:
~# find /sys/kernel/debug/soundwire/ /sys/kernel/debug/soundwire/ /sys/kernel/debug/soundwire/sdw-master-2 /sys/kernel/debug/soundwire/sdw-master-2/master-0 /sys/kernel/debug/soundwire/sdw-master-2/master-0/sdw:0:217:2110:0:4 /sys/kernel/debug/soundwire/sdw-master-2/master-0/sdw:0:217:2110:0:4/registers /sys/kernel/debug/soundwire/sdw-master-2/master-0/sdw:0:217:2110:0:3 /sys/kernel/debug/soundwire/sdw-master-2/master-0/sdw:0:217:2110:0:3/registers /sys/kernel/debug/soundwire/sdw-master-1 /sys/kernel/debug/soundwire/sdw-master-1/master-0 /sys/kernel/debug/soundwire/sdw-master-1/master-0/sdw:0:217:10d:0:3 /sys/kernel/debug/soundwire/sdw-master-1/master-0/sdw:0:217:10d:0:3/registers /sys/kernel/debug/soundwire/sdw-master-0 /sys/kernel/debug/soundwire/sdw-master-0/master-0 /sys/kernel/debug/soundwire/sdw-master-0/master-0/sdw:0:217:10d:0:4 /sys/kernel/debug/soundwire/sdw-master-0/master-0/sdw:0:217:10d:0:4/registers
Thanks, srini