Em Mon, 21 Mar 2016 13:10:33 +0200 Laurent Pinchart laurent.pinchart@ideasonboard.com escreveu:
Hi Mauro,
Thank you for the patch.
On Friday 18 Mar 2016 21:42:16 Mauro Carvalho Chehab wrote:
Now that the media_device can be used by multiple drivers, via devres, we need to be sure that it will be dropped only when all drivers stop using it.
I've discussed this with Shuah previously and I'm surprised to see that the problem hasn't been fixed : using devres for this purpose is just plain wrong.
I didn't follow your discussions with Shuah. I'm pretty sure I didn't see any such reply to the /22 patch series.
For sure there are other approaches, although I wouldn't say that this approach is plain wrong. It was actually suggested by Greg KH at the USB summit, back in 2011: https://lkml.org/lkml/2011/8/21/61
It works fine in the cases like the ones Shuah is currently addressing: USB devices that have multiple interfaces handled by independent drivers.
Going further, right now, as far as I'm aware of, there are only two use cases for a driver-independent media_device struct in the media subsystem (on the upstream Kernel):
- USB devices with USB Audio Class: au0828 and em28xx drivers, plus snd-usb-audio;
- bt878/bt879 PCI devices, where the DVB driver is independent from the V4L2 one (affects bt87x and bttv drivers).
The devres approach fits well for both use cases.
Ok, there are a plenty of OOT SoC drivers that might benefit of some other solution, but we should care about them only if/when they got upstreamed.
The empty media_device_release_devres() function should have given you a hint.
What we need instead is a list of media devices indexed by struct device (for this use case) or by struct device_node (for DT use cases). It will both simplify the code and get rid of the devres abuse.
Yeah, Shuah's approach should be changed to a different one, in order to work for DT use cases. It would be good to have a real DT use case for us to validate the solution, before we start implementing something in the wild.
Still, it would very likely need a kref there, in order to destroy the media controller struct only after all drivers stop using it.
Shuah, if I recall correctly you worked on implementing such a solution after our last discussion on the topic. Could you please update us on the status ?
I saw a Shuah's email proposing to discuss this at the media summit.
In the mean time, let's hold off on this patch, and merge a proper solution instead.
Well, we should send a fix for the current issues for Kernel 4.6.
As the number of drivers that would be using this internal API is small (right now, only 2 drivers), replacing devres by some other strategy in the future should be easy.
Regards, Mauro