Greg, Em Thu, 15 Dec 2016 05:44:38 -0800 Greg KH <greg@xxxxxxxxx> escreveu: > On Thu, Dec 15, 2016 at 10:57:16AM -0200, Mauro Carvalho Chehab wrote: > > Em Thu, 15 Dec 2016 09:42:35 -0300 > > Javier Martinez Canillas <javier@xxxxxxxxxxxxxxx> escreveu: > > > > > Hello Mauro, > > > > > > On 12/15/2016 09:37 AM, Mauro Carvalho Chehab wrote: > > > > > > [snip] > > > > > > > > > > > What happens is that omap3isp driver calls media_device_unregister() > > > > too early. Right now, it is called at omap3isp_video_device_release(), > > > > with happens when a driver unbind is ordered by userspace, and not after > > > > the last usage of all /dev/video?? devices. > > > > > > > > There are two possible fixes: > > > > > > > > 1) at omap3isp_video_device_release(), streamoff all streams and mark > > > > that the media device will be gone. > > > > I actually meant to say: isp_unregister_entities() here. > > > > > > > > > > 2) instead of using video_device_release_empty for the video->video.release, > > > > create a omap3isp_video_device_release() that will call > > > > media_device_unregister() when destroying the last /dev/video?? devnode. > > > > > > > > > > There's also option (3), to have a proper refcounting to make sure that > > > the media device node is not freed until all references to it are gone. > > > > Yes, that's another alternative. > > > > > I understand that's what Sakari's RFC patches do. I'll try to make some > > > time tomorrow to test and review his patches. > > > > The biggest problem with Sakari's patches is that it starts by > > reverting 3 patches, and this will cause regressions on existing > > devices. > > > > Development should be incremental. > > How can reverting patches cause regressions? If a patch that is applied > breaks something else, it needs to be reverted, end of story. If that > patch happened to have fixed a different issue, that's fine, we are back > to the original issue, it's not a "regression" at all, the patch was > wrong in the first place. > > So please, just revert them now. That's the correct thing to do, as we > will be back to the previous release's behavior. The patches that Sakari want to revert are fixes. Before those patches, the cdev logic was broken. So, it used to generate OOPS when the media character device is removed. So, when PCI/USB drivers gained media controller support, a regression was introduced. Those patches fix it, as without them, unbinding (or physically removing) PCI/USB devices cause OOPS because the cdev logic there are broken. What happens is that Sakari is proposing a different approach to solve it on this 21 RFC patch series. Instead of applying his solution on the top of upstream code, he decided to start it by removing the regression fix patches. Also, the way his solution was designed, every single media driver using the media controller needs to be modified in order for it to not cause OOPS at device unbind. However, the series only do such changes on OMAP3 driver (and no te didn't make any tests on this series out of OMAP3). So, applying the entire series will very likely cause regressions on PCI/USB drivers. Thanks, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
