On Friday, May 20, 2011 11:05:00 Laurent Pinchart wrote: > 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. Good point. > 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. Would it be an option to create a new platform bus for the subdevs? That would have its own lock. It makes sense from a hierarchical point of view, but I'm not certain about the amount of work involved. Regards, Hans -- 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
