On Monday 14 December 2009 21:41:20 Guennadi Liakhovetski wrote:
> On Mon, 14 Dec 2009, Jonathan Cameron wrote:
>
> > Hi All,
> >
> > >> 3) it would be interesting to patch the other sensor drivers to be compatible
> > >> with soc_camera (mt9v011/ov7670);
> > >
> > > Well, this could be done, yes, but does it make sense to do this blindly
> > > without any hardware to test? I would rather add such conversions on a
> > > one-by-one basis as need arises.
> > >
> > I'm working on the ov7670. I've added the media bus stuff to what I've
> > previously posted but I haven't tested as yet. For reference, a quick and dirty
> > cut of the patch is below. Some thought is needed on how to deal with the sections
> > that currently need to be different for the soc-camera and non soc camera uses.
> > (mainly the registration code in probe).
>
> Look at my patch for mt9t031. First you do not have to fail if
> client->dev.platform_data == NULL. You have to look at the bridge driver,
> that's currently working with ov7670. If it is not setting platform_data,
> you can use it to detect whether you're in soc-camera environment or not.
> Otherwise, if the bridge driver were also using that pointer, we would
> have to come up with a ov7670-specific struct to be linked from that
> pointer. I was actually thinking about a way to pass soc-camera data to
> client drivers in a way, that would not imped non soc-camera use. So, how
> about:
>
>
> I soc-camera environment
>
> 1. platform defines a
>
> static struct ov7670_platform_data ov7670 = {
> .link = &ov7670_link,
> };
>
> 2. and a
>
> static struct soc_camera_link ov7670_link = {
> .client_data = &ov7670,
> };
>
> 3. soc-camera core gets &ov7670_link in its probe, same as this is done
> now, then it does
>
> icl->icd = icd;
> icl->board_info->platform_data = icl->client_data;
>
> subdev = v4l2_i2c_new_subdev_board(&ici->v4l2_dev, adap,
> icl->module_name, icl->board_info, NULL);
>
> 4. ov7670 probe is called with client platform data pointing to its own
> data:
>
> static int ov7670_probe(struct i2c_client *client,
> const struct i2c_device_id *did)
> {
> struct ov7670_platform_data *pdata = client->dev.platform_data;
> struct soc_camera_link *icl = pdata->link;
> struct soc_camera_device *icd;
>
> if (icl)
> icd = icl->icd;
>
>
> II Non soc-camera environment
>
> 1. the bridge driver fills in sensor data and does
>
> struct ov7670_platform_data *ov7670_pdata = kzalloc(sizeof(*pdata));
>
> ov7670_pdata->... = ...;
> board_info->platform_data = ov7670_pdata;
>
> subdev = v4l2_i2c_new_subdev_board(v4l2_dev, adap,
> module_name, board_info, NULL);
>
> 2. ov7670_probe() gets called and finds its data as above, but without an
> soc-camera link this time.
>
>
> The advantage of the above is, that each client driver is free to define
> its own platform data struct, and for soc-camera it only has to provide
> one pointer in it. The ugly side is the cross-referencing between I.1. and
> I.2. above.
>
> Alternatively we could agree, that all client drivers get in their i2c
> client platform data a standard struct with only pointers in it to client
> data and to various bridging models:
>
> struct v4l2_client {
> void *client;
> struct soc_camera_link *icl;
> struct int_device_link *int;
> ...
> };
>
> or a bit more hidden
>
> struct v4l2_client {
> void *client;
> void *bridge_data;
> enum V4L2_BRIDGE_TYPE bridge_type;
> };
Before this goes too far let me just say that I will NAK any attempt in this
direction. Subdevice drivers must eventually be completely independent from
bridge driver. The same driver should work out of the box for soc-camera,
gspca, omap3, davinci, saa7134 and whatever other bridge driver is out there.
Having bridge-specific code in a sub-device driver will be a disaster in the
long run. Well, we've seen what happens when you do it that way.
As far as I know the only soc-dependency left now is for bus configuration.
Proposals were made some time ago and we should pick that up again and remove
that last dependency.
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by TANDBERG
--
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
