AnsweredAssumed Answered

Adding a video sensor for CSI

Question asked by Steve Anderson on Mar 24, 2015
Latest reply on Oct 14, 2016 by gohilurvish

I have been working on adding 2 sensors for video input using CSI0.

 

First is porting a driver for gs29xx for device tree, based on the gs2971 driver from 3.0.35...

Second is to implement a self-test driver to activate the CSI test pattern when included as a camera...

 

I think I found a bug/design issue in mxc_v4l2_capture.c

There are several places which make use of 'struct sensor_data' in this file.

This is the PRIVATE data for the v4l2 internal device.

 

It seems to me that there is no good reason for mxc_v4l2_capture to know about the privates of a v4l2_int_device, much less for it to require a specific structure for those privates...

 

Forcing the use of this structure seems to pretty much exclude the use of anything except an ov564x device.

In my situation, existing members of struct sensor_data are irrelevant, and I do need members which are not at all relevant to either mxc_v4l2_capture, nor ov564x cameras.

 

Am I overlooking something simple?

It seems to me that the members of struct sensor_data used in the fragments below should, instead, be part of v4l2_int_device.

This suggested solution would move the private members io_init and mclk_source and sensor_clk up to v4l2_int_device.

This SHOULD be fine, since the slave v4l2_int_device initializes its own v4l2_int_device structure in probe(...)

 

The uses of this are in:

 

Note that in each of the following cam->sensor is a  struct v4l2_int_device* sensor;

 

mxc_v4l2_s_ctrl(...)

  sensor_data = cam->all_sensors[c->value]->priv;

  if (sensor_data->io_init)

   sensor_data->io_init();

  cam->mclk_source = sensor_data->mclk_source;

 

If a new sensor actually used its own private data structure then the above would result in:

1) An uncontrolled call into an unknown dataspace

2) Assignment of an uncontrolled pointer to be used as a clock structure pointer...

 

mxc_v4l_open(...)

  sensor = cam->sensor->priv;

  clk_prepare_enable(sensor->sensor_clk);

 

mxc_v4l_close(...)

   sensor = cam->sensor->priv;

  clk_disable_unprepare(sensor->sensor_clk);

 

If a new sensor actually used its own private data structure then the above 2 would result in:

1) A clock driver operation on an uncontrolled pointer to an unknown data space.

 

My issue here is not that v4l2_int_device should be unknown here, in fact it must be known here.

The issues is that v4l2_int_device.priv should not be defined here, but should be private for each potential sensor type.

Anything used here should be part of the v4l2_int_device structure which is known on both ends of the driver configuration.

Outcomes