On 06-05-19, 09:48, Pierre-Louis Bossart wrote:
+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.
Not really. The Soundwire code is dual licensed. Many of the soundwire symbols are indeed exported as EXPORT_SYMBOL. But I agree this one is 'linux' specific so can be made _GPL.
Pierre, does Intel still care about this being dual licensed or not?
But why is this exported at all? No one calls this function.
I will have to check.
It is used by codec driver which are not upstream yet. So my suggestion would be NOT to export this and only do so when we have users for it That would be true for other APIs exported out as well.
+struct sdw_slave_debugfs {
- struct sdw_slave *slave;
Same question as above, why do you need this pointer?
will check.
The deubugfs code does hold a ref to slave object to read the data and dump, this particular instance might be able to get rid of (should be doable)
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.
Yes, but then device exit routine is supposed to do debugfs cleanup as well, so that would ensure these references are dropped at that point of time. Greg should that not take care of it or we *should* always do refcounting.
Thanks