Hi Hans, On 09/11/2013 04:01 PM, Hans Verkuil wrote: > On 09/11/2013 03:07 PM, Sylwester Nawrocki wrote: >> On 09/09/2013 11:07 AM, Hans Verkuil wrote: >>> On 09/06/2013 12:33 AM, Sylwester Nawrocki wrote: >>>> On 08/07/2013 07:49 PM, Hans Verkuil wrote: >>>>> On 08/07/2013 06:49 PM, Sylwester Nawrocki wrote: >>>>>> On 08/02/2013 03:00 PM, Hans Verkuil wrote: >>>>>>> On 08/02/2013 02:27 PM, Sylwester Nawrocki wrote: [...] >>>> So there is lots of things that may fail after first video node is >>>> registered, and on which open() may be called immediately. >>> >>> The only solution I know off-hand to handle this safely is to unregister all >>> nodes if some fail, but to return 0 in the probe function. If an open() succeeded, >>> then that will just work until the node is closed, at which point the v4l2_device >>> release() is called and you can cleanup. >> >> Another solution would be to properly implement struct video_device::release() >> callback and in video device open() do video_is_registered() check while >> holding the driver private video device lock. Also video_unregister_device() >> would need to be called while holding the video device lock. > > How would that help? By not allowing video_unregister_device() call in the middle of the file op and serializing whatever does video device unregistration with the file ops. I'm not sure it is possible in all cases. For ioctls it's already party taken care of by video_is_registered() check while holding the video dev mutex. For other file ops drivers currently need to do various checks to ensure that all driver private data/resources that may be needed in the file operations callbacks are in place. [...] >>> Is this 'could fail', or 'I have seen it fail'? I have never seen problems in probe() >>> with node creation. The only reason I know of why creating a node might fail is >>> being out-of-memory. >> >> In my case it was top level driver that was triggering device node creation >> in its probe() and if, e.g. some of sub-device's driver was missing, it called, >> through subdev internal unregistered(), op video_unregister_device(), but also >> media_entity_cleanup() which seemed to be the source of trouble. > > Device nodes should always be created at the very end after all other setup > actions (like loaded subdev drivers) have been done successfully. From your > description it seems that device nodes were created earlier? Yes, let me explain in what configuration it happens (perhaps looking at fimc_md_probe() function at drives/media/platform/exynos4-is/media-dev.c makes it easier to understand; the version closer to what we work with currently is available at [1]). There are multiple platform devices that in their driver's probe() just initialize the driver private data and platform device resources, like irq, IO register, etc. There is also a top level driver (struct v4l2_device) that walks through Device Tree and registers those platform devices as v4l2 subdevs. While doing that video nodes are created in some subdevs' .registered() callbacks. After that image sensors are being registered. Some platform devices need to be registered before image sensors, due to resource dependencies. E.g. some device need to be activated so clock for an image sensor is available at an SoC output pin. So this is a bit more complex issue than with a single device, a single driver and its probe() interaction with the file ops. [1] git.linuxtv.org/snawrocki/samsung.git/v3.11-rc2-dts-exynos4-is-clk -- Regards, Sylwester -- 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
