On Tuesday 08 January 2013 11:26:57 Guennadi Liakhovetski wrote: > On Tue, 8 Jan 2013, Laurent Pinchart wrote: > > On Tuesday 08 January 2013 10:56:43 Guennadi Liakhovetski wrote: > > > On Tue, 8 Jan 2013, Laurent Pinchart wrote: > > > > On Tuesday 08 January 2013 10:25:15 Guennadi Liakhovetski wrote: > > > > > On Tue, 8 Jan 2013, Laurent Pinchart wrote:
[snip] > > > > > > Could you please explain why you need both a bind notifier and a > > > > > > bound notifier ? I was expecting a single > > > > > > v4l2_async_subdev_register() call in subdev drivers (and, thinking > > > > > > about it, I would probably name it v4l2_subdev_register()). > > > > > > > > > > I think I can, yes. Because between .bind() and .bound() the > > > > > subdevice driver does the actual hardware probing. So, .bind() is > > > > > used to make sure the hardware can be accessed, most importantly to > > > > > provide a clock to the subdevice. You can look at > > > > > soc_camera_async_bind(). There I'm registering the clock for the > > > > > subdevice, about to bind. Why I cannot do it before, is because I > > > > > need subdevice name for clock matching. With I2C subdevices the > > > > > subdevice name contains the name of the driver, adapter number and > > > > > i2c address. The latter 2 I've got from host subdevice list. But not > > > > > the driver name. I thought about also passing the driver name there, > > > > > but that seemed too limiting to me. I also request regulators there, > > > > > because before ->bound() the sensor driver, but that could be done > > > > > on the first call to soc_camera_power_on(), although doing this > > > > > "first call" thingie is kind of hackish too. I could add one more > > > > > soc-camera-power helper like soc_camera_prepare() or similar too. > > > > > > > > I think a soc_camera_power_init() function (or similar) would be a > > > > good idea, yes. > > > > > > > > > So, the main problem is the clock > > > > > > > > > > subdevice name. Also see the comment in soc_camera.c: > > > > > /* > > > > > * It is ok to keep the clock for the whole soc_camera_device > > > > > * life-time, in principle it would be more logical to register > > > > > * the clock on icd creation, the only problem is, that at that > > > > > * time we don't know the driver name yet. > > > > > */ > > > > > > > > I think we should fix that problem instead of shaping the async API > > > > around a workaround :-) > > > > > > > > From the subdevice point of view, the probe function should request > > > > resources, perform whatever initialization is needed (including > > > > verifying that the hardware is functional when possible), and the > > > > register the subdev with the code if everything succeeded. Splitting > > > > registration into bind() and bound() appears a bit as a workaround to > > > > me. > > > > > > > > If we need a workaround, I'd rather pass the device name in addition > > > > to the I2C adapter number and address, instead of embedding the > > > > workaround in this new API. > > > > > > ...or we can change the I2C subdevice name format. The actual need to do > > > > > > snprintf(clk_name, sizeof(clk_name), "%s %d-%04x", > > > > > > asdl->dev->driver->name, > > > i2c_adapter_id(client->adapter), client->addr); > > > > > > in soc-camera now to exactly match the subdevice name, as created by > > > v4l2_i2c_subdev_init(), doesn't make me specifically happy either. What > > > if the latter changes at some point? Or what if one driver wishes to > > > create several subdevices for one I2C device? > > > > The common clock framework uses %d-%04x, maybe we could use that as well > > for clock names ? > > And preserve the subdevice names? Then matching would be more difficult > and less precise. Or change subdevice names too? I think, we can do the > latter, since anyway at any time only one driver can be attached to an I2C > device. That's right. Where else is the subdev name used ? > > > > > > > +void v4l2_async_subdev_unbind(struct v4l2_async_subdev_list > > > > > > > *asdl); > > > > > > > +#endif -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
