RE: [PATCH/RFC 9/9] mt9t031: make the use of the soc-camera client API optional

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Guennadi,

Thanks for the reply.

>> >+};
>> >+
>> >+static struct soc_camera_ops mt9t031_ops = {
>> >+	.set_bus_param		= mt9t031_set_bus_param,
>> >+	.query_bus_param	= mt9t031_query_bus_param,
>> >+	.controls		= mt9t031_controls,
>> >+	.num_controls		= ARRAY_SIZE(mt9t031_controls),
>> >+};
>> >+
>>
>> [MK] Why don't you implement queryctrl ops in core? query_bus_param
>> & set_bus_param() can be implemented as a sub device operation as well
>> right? I think we need to get the bus parameter RFC implemented and
>> this driver could be targeted for it's first use so that we could
>> work together to get it accepted. I didn't get a chance to study your
>> bus image format RFC, but plan to review it soon and to see if it can be
>> used in my platform as well. For use of this driver in our platform,
>> all reference to soc_ must be removed. I am ok if the structure is
>> re-used, but if this driver calls any soc_camera function, it canot
>> be used in my platform.
>
>Why? Some soc-camera functions are just library functions, you just have
>to build soc-camera into your kernel. (also see below)
>
My point is that the control is for the sensor device, so why to implement
queryctrl in SoC camera? Just for this I need to include SOC camera in my build? That doesn't make any sense at all. IMHO, queryctrl() logically belongs to this sensor driver which can be called from the bridge driver using sudev API call. Any reverse dependency from MT9T031 to SoC camera to be removed if it is to be re-used across other platforms. Can we agree on this? Did you have a chance to compare the driver file that I had sent to you?

Thanks.

Murali
>> BTW, I am attaching a version of the driver that we use in our kernel
>> tree for your reference which will give you an idea of my requirement.
>>
>
>[snip]
>
>> >@@ -565,7 +562,6 @@ static int mt9t031_s_ctrl(struct v4l2_subdev *sd,
>> >struct v4l2_control *ctrl)
>> > {
>> > 	struct i2c_client *client = sd->priv;
>> > 	struct mt9t031 *mt9t031 = to_mt9t031(client);
>> >-	struct soc_camera_device *icd = client->dev.platform_data;
>> > 	const struct v4l2_queryctrl *qctrl;
>> > 	int data;
>> >
>> >@@ -657,7 +653,8 @@ static int mt9t031_s_ctrl(struct v4l2_subdev *sd,
>> >struct v4l2_control *ctrl)
>> >
>> > 			if (set_shutter(client, total_h) < 0)
>> > 				return -EIO;
>> >-			qctrl = soc_camera_find_qctrl(icd->ops,
>> >V4L2_CID_EXPOSURE);
>> >+			qctrl = soc_camera_find_qctrl(&mt9t031_ops,
>> >+						      V4L2_CID_EXPOSURE);
>>
>> [MK] Why do we still need this call? In my version of the sensor driver,
>> I just implement the queryctrl() operation in core_ops. This cannot work
>> since soc_camera_find_qctrl() is implemented only in SoC camera.
>
>As mentioned above, that's just a library function without any further
>dependencies, so, why reimplement it?
>
>Thanks
>Guennadi
>---
>Guennadi Liakhovetski, Ph.D.
>Freelance Open-Source Software Developer
>http://www.open-technology.de/

--
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

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux