Hi Hans, On Friday 20 May 2011 10:53:32 Hans Verkuil wrote: > On Friday, May 20, 2011 09:29:32 Laurent Pinchart wrote: > > On Friday 20 May 2011 09:14:36 Sylwester Nawrocki wrote: > > > On 05/19/2011 08:34 PM, Laurent Pinchart wrote: > > > > The new v4l2_new_subdev_board() function creates and register a > > > > subdev based on generic board information. The board information > > > > structure includes a bus type and bus type-specific information. > > > > > > > > Only I2C and SPI busses are currently supported. > > > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> [snip] > > > I had an issue when tried to call request_module, to register subdev of > > > platform device type, in probe() of other platform device. Driver's > > > probe() for devices belonging same bus type cannot be nested as the bus > > > lock is taken by the driver core before entering probe(), so this would > > > lead to a deadlock. > > > That exactly happens in __driver_attach(). > > > > > > For the same reason v4l2_new_subdev_board could not be called from > > > probe() of devices belonging to I2C or SPI bus, as request_module is > > > called inside of it. I'm not sure how to solve it, yet:) > > > > Ouch. I wasn't aware of that issue. Looks like it's indeed time to fix > > the subdev registration issue, including the module load race condition. > > Michael, you said you have a patch to add platform subdev support, how > > have you avoided the race condition ? > > > > I've been thinking for some time now about removing the module load code > > completely. I2C, SPI and platform subdevs would be registered either by > > board code (possibly through the device tree on platforms that suppport > > it) for embedded platforms, and by host drivers for pluggable hardware > > (PCI and USB). Module loading would be handled automatically by the kernel > > module auto loader, but asynchronously instead of synchronously. Bus > > notifiers would then be used by host drivers to wait for all subdevs to be > > registered. > > > > I'm not sure yet if this approach is viable. Hans, I think we've briefly > > discussed this (possible quite a long time ago), do you have any opinion > > ? Guennadi, based on your previous experience trying to use bus notifiers > > to solve the module load race, what do you think about the idea ? Others, > > please comment as well :-) > > It's definitely viable (I believe the required bus notification has been > added some time ago), but I am not sure how to implement it in an > efficient manner. > > My initial idea would be to just wait in v4l2_new_subdev_board until you > get the notification on the bus (with a timeout, of course). However, I > suspect that that does not solve the deadlock, although it would solve the > race. > > As an aside: note that if the module is unloaded right after the > request_module, then that will be detected by the code and it will just > return an error. It won't oops or anything like that. Personally I don't > believe it is worth the effort just to solve this race, since it is highly > theoretical. > > The problem of loading another bus module when in a bus probe function is a > separate issue. My initial reaction is: why do you want to do this? Even if > you use delayed module loads, you probably still have to wait for them to > succeed at a higher-level function. For example: in the probe function of > module A it will attempt to load module B. That probably can't succeed as > long as you are in A's probe function due to the bus lock. So you can't > check for a successful load of B until you return from that probe function > and a higher- level function (that likely loaded module A in the first > place) does that check. > > That's all pretty tricky code, and my suggestion would be to simply not do > nested module loads from the same bus. That's unfortunately not an option. Most bridge/host devices in embedded systems are platform devices, and they will need to load platform subdevs. We need to fix that. My idea was to use bus notifiers to delay the bridge/host device initialization. The bridge probe() function would pre-initialize the bridge and register notifiers. The driver would then wait until all subdevs are properly registered, and then proceed from to register V4L2 devices from the bus notifier callback (or possible a work queue). There would be no nested probe() calls. -- Regards, Laurent Pinchart -- 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
