On Tue, Apr 13, 2021 at 10:47:49AM -0500, Pierre-Louis Bossart wrote: > > > On 4/13/21 9:05 AM, Heikki Krogerus wrote: > > On Tue, Apr 13, 2021 at 03:20:45PM +0300, Heikki Krogerus wrote: > > > On Mon, Apr 12, 2021 at 03:36:20PM -0500, Pierre-Louis Bossart wrote: > > > > I took the code and split it in two for BYT/CHT (modified to remove devm_) > > > > and SoundWire parts (added as is). > > > > > > > > https://github.com/thesofproject/linux/pull/2810 > > > > > > > > Both cases result in a refcount error on device_remove_sof when removing the > > > > platform device. I don't understand the code well enough to figure out what > > > > happens, but it's likely a case of the software node being removed twice? > > > > > > Right. Because you are injecting the node to an already existing > > > device, the node does not get linked with the device in sysfs. That > > > would increment the reference count in a normal case. It all happens > > > in the function software_node_notify(). Driver core calls it when a > > > device is added and also when it's removed. In this case it is only > > > called when it's removed. > > > > > > I think the best way to handle this now is to simply not decrementing > > > the ref count when you add the properties, so don't call > > > fwnode_handle_put() there (but add a comment explaining why you are > > > not calling it). > > > > No, sorry... That's just too hacky. Let's not do that after all. > > > > We can also fix this in the software node code. I'm attaching a patch > > that should make it possible to inject the software nodes also > > afterwards safely. This is definitely also not without its problems, > > but we can go with that if it works. Let me know. > > I tested manually on bytcr w/ RT5640 and used the SOF CI farm to test the > SoundWire cases, I don't see any issues with your patch. The refcount issue > is gone and the module load/unload cycles don't report any problems. > > Would you queue it for 5.13-rc1, or is this too late already? I'll send it out now. Let's see what happens. thanks, > > > For a better solution you would call device_reprobe() after you have > > > injected the software node, but before that you need to modify > > > device_reprobe() so it calls device_platform_notify() (which it really > > > should call in any case). But this should probable be done later, > > > separately. > > > > > > thanks, > > > > > > P.S. > > > > > > Have you guys considered the possibility of describing the connections > > > between all these components by using one of the methods that we now > > > have for that in kernel, for example device graph? It can now be > > > used also with software nodes (OF graph and ACPI device graph are of > > > course already fully supported). > > I must admit I've never heard of a 'device graph'. Any pointers or APIs you > can think of? > It's a good comment since we are planning to rework the SOF clients and > machine driver handling. -- heikki
