Hi Hans, On Monday 08 October 2012 17:41:43 Hans Verkuil wrote: > On Mon October 8 2012 17:15:53 Guennadi Liakhovetski wrote: > > On Mon, 8 Oct 2012, Hans Verkuil wrote: > > > On Mon October 8 2012 16:30:53 Guennadi Liakhovetski wrote: > > > > On Mon, 8 Oct 2012, Hans Verkuil wrote: [snip] > > > > > I wonder, don't we have the necessary code already? V4L2 subdev > > > > > drivers can have internal_ops with register/unregister ops. These > > > > > are called by v4l2_device_register_subdev. This happens during the > > > > > bridge driver's probe. > > > > > > > > > > Suppose the subdev's probe does not actually access the i2c device, > > > > > but instead defers that to the register callback? The bridge driver > > > > > will turn on the clock before calling v4l2_device_register_subdev to > > > > > ensure that the register callback can access the i2c registers. The > > > > > register callback will do any initialization and can return an > > > > > error. In case of an error the i2c client is automatically > > > > > unregistered as well. > > > > > > > > Yes, if v4l2_i2c_new_subdev_board() is used. This has been discussed > > > > several times before and always what I didn't like in this is, that > > > > I2C device probe() in this case succeeds without even trying to access > > > > the hardware. And think about DT. In this case we don't instantiate > > > > the I2C device, OF code does it for us. What do you do then? If you > > > > let probe() succeed, then you will have to somehow remember the > > > > subdevice to later match it against bridges... > > > > > > Yes, but you need that information anyway. The bridge still needs to > > > call v4l2_device_register_subdev so it needs to know which subdevs are > > > loaded. > > > > But how do you get the subdev pointer? With the notifier I get it from > > i2c_get_clientdata(client) and what do you do without it? How do you get > > to the client? > > > > > And can't it get that from DT as well? > > > > No, I don't think there is a way to get a device pointer from a DT node. > > Not a device pointer, but the i2c bus and i2c address. With that information > you can get the i2c_client, and with that you can get the subdev pointer. That could work as well, but it might be easier to keep a mapping from the DT node to struct device or struct v4l2_subdev instead. I have no strong opinion on this at the moment. > If there is no way to get that information from the proposed V4L2 DT, then > it needs to be modified since a bridge driver really needs to know which > subdevs it has to register with the v4l2_device struct. That information is > also board specific so it should be part of the DT. > > > > In my view you cannot do a proper initialization unless you have both > > > the bridge driver and all subdev drivers loaded and instantiated. They > > > need one another. So I am perfectly fine with letting the probe function > > > do next to nothing and postponing that until register() is called. I2C > > > and actual probing to check if it's the right device is a bad idea in > > > general since you have no idea what a hardware access to an unknown i2c > > > device will do. There are still some corner cases where that is needed, > > > but I do not think that that is an issue here. > > > > > > It would simplify things a lot IMHO. Also note that the register() op > > > will work with any device, not just i2c. That may be a useful property > > > as well. > > > > And what if the subdevice device is not yet instantiated by OF by the time > > your bridge driver probes? > > That is where you still need to have a bus notifier mechanism. You have to > be able to wait until all dependent drivers are loaded/instantiated, or > alternatively you have to be able to load them explicitly. But this should > be relatively easy to implement in a generic manner. > > I still think this sucks (excuse my language), but I see no way around it as > long as there is no explicit probe order one can rely on. -- 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