@@ -136,6 +139,8 @@ static int sdw_delete_slave(struct device *dev, void *data) void sdw_delete_bus_master(struct sdw_bus *bus) { sdw_sysfs_bus_exit(bus);
- if (bus->debugfs)
sdw_bus_debugfs_exit(bus->debugfs);No need to check, just call it.
That was on my todo list, will remove.
+struct sdw_bus_debugfs {
- struct sdw_bus *bus;
Why do you need to save this pointer?
- struct dentry *fs;
This really is all you need to have around, right?
will check.
+struct dentry *sdw_bus_debugfs_get_root(struct sdw_bus_debugfs *d) +{
- if (d)
return d->fs;- return NULL;
+} +EXPORT_SYMBOL(sdw_bus_debugfs_get_root);
_GPL()?
Oops, that's a big miss. will fix, thanks for spotting this.
But why is this exported at all? No one calls this function.
I will have to check.
+struct sdw_slave_debugfs {
- struct sdw_slave *slave;
Same question as above, why do you need this pointer?
will check.
And meta-comment, if you _EVER_ save off a pointer to a reference counted object (like this and the above one), you HAVE to grab a reference to it, otherwise it can go away at any point in time as that is the point of reference counted objects.
So even if you do need/want this, you have to properly handle the reference count by incrementing/decrementing it as needed.
good comment, thank you for the guidance.
+struct sdw_slave_debugfs *sdw_slave_debugfs_init(struct sdw_slave *slave) +{
- struct sdw_bus_debugfs *master;
- struct sdw_slave_debugfs *d;
- char name[32];
- master = slave->bus->debugfs;
- if (!master)
return NULL;- d = kzalloc(sizeof(*d), GFP_KERNEL);
- if (!d)
return NULL;- /* create the debugfs slave-name */
- snprintf(name, sizeof(name), "%s", dev_name(&slave->dev));
- d->fs = debugfs_create_dir(name, master->fs);
- if (IS_ERR_OR_NULL(d->fs)) {
dev_err(&slave->dev, "slave debugfs root creation failed\n");goto err;- }
You never care about the return value of a debugfs call. I have a 100+ patch series stripping all of this out of the kernel, please don't force me to add another one to it :)
Just call debugfs and move on, you can always put the return value of one call into another one just fine, and your function logic should never change if debugfs returns an error or not, you do not care.
Yes, it's agreed that we should not depend on debugfs or fail here. will fix, no worries.
+void sdw_debugfs_init(void) +{
- sdw_debugfs_root = debugfs_create_dir("soundwire", NULL);
- if (IS_ERR_OR_NULL(sdw_debugfs_root)) {
pr_warn("SoundWire: Failed to create debugfs directory\n");sdw_debugfs_root = NULL;return;Same here, just call the function and return.
yep, will do.