Hi Guennadi,
Cc: LKML
On 01/08/2013 11:26 AM, 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:
On Monday 07 January 2013 11:23:55 Guennadi Liakhovetski wrote:
> From 0e1eae338ba898dc25ec60e3dba99e5581edc199 Mon Sep 17 00:00:00
2001
[snip]
+int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
+ struct v4l2_async_notifier *notifier);
+void v4l2_async_notifier_unregister(struct v4l2_async_notifier
*notifier);
+/*
+ * If subdevice probing fails any time after
v4l2_async_subdev_bind(),
no
+ * clean up must be called. This function is only a message of
intention.
+ */
+int v4l2_async_subdev_bind(struct v4l2_async_subdev_list *asdl);
+int v4l2_async_subdev_bound(struct v4l2_async_subdev_list *asdl);
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.
I'm just wondering why we can't associate the clock with relevant device,
rather than its driver ? This could eliminate the problem of unknown
sub-device name at the host driver, before sub-device driver is actually
probed, couldn't it ?
--
Thanks,
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
