Re: [PATCH/RFC 9/9 v2] mt9t031: make the use of the soc-camera client API optional
On Tue, 10 Nov 2009, Laurent Pinchart wrote: Hi Guennadi, On Thursday 05 November 2009 18:07:09 Karicheri, Muralidharan wrote: Guennadi, in the v4l2_queryctrl struct. I think, this is unrelated. Muralidharan just complained about the soc_camera_find_qctrl() function being used in client subdev drivers, that were to be converted to v4l2-subdev, specifically, in mt9t031.c. And I just explained, that that's just a pretty trivial library function, that does not introduce any restrictions on how that subdev driver can be used in non-soc-camera configurations, apart from the need to build and load the soc-camera module. In other words, any v4l2-device bridge driver should be able to communicate with such a subdev driver, calling that function. If soc_camera_find_qctrl() is such a generic function, why don't you move it to v4l2-common.c so that other platforms doesn't have to build SOC camera sub system to use this function? Your statement reinforce this. I second this. Hans is working on a controls framework that should (hopefully :-)) make drivers simpler by handling common tasks in the v4l core. Well, if you look at the function itself and at how it got replaced in this version of the patch by O(1) operations, you'll, probably, agree, that avoiding that function where possible is better than making it generic. But if there are any legitimate users for it - sure, can make it generic too. Do you have any plan to work on the bus hardware configuration API ? When that will be available the mt9t031 driver could be made completely soc-camera-free. I'd love to first push the proposed image-bus upstream. Even with just that many drivers can already be re-used. As for bus configuration, I thought there were enough people working on it already:-) If not, maybe I could have a look at it, but we better reach some agreement on that beforehand to avoid duplicating the effort. 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 9/9 v2] mt9t031: make the use of the soc-camera client API optional
Now that we have moved most of the functions over to the v4l2-subdev API, only
quering and setting bus parameters are still performed using the legacy
soc-camera client API. Make the use of this API optional for mt9t031.
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
On Mon, 2 Nov 2009, Karicheri, Muralidharan wrote:
+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?
In general I'm sure you understand, that there are lots of functions in
the kernel, that we use in specific modules, not because they interact
with other systems, but because they implement some common functionality
and just reduce code-duplication. And I can well imagine that in many such
cases using just one or a couple of such functions will pull a much larger
pile of unused code with them. But in this case those calls can indeed be
very easily eliminated. Please have a look at the version below.
Did you have a chance to compare the driver file that I had sent to you?
I looked at it, but it is based on an earlier version of the driver, so,
it wasn't very easy to compare. Maybe you could send a diff against the
mainline version, on which it is based?
Thanks
Guennadi
drivers/media/video/mt9t031.c | 167 +
1 files changed, 85 insertions(+), 82 deletions(-)
diff --git a/drivers/media/video/mt9t031.c b/drivers/media/video/mt9t031.c
index c95c277..86bf8f6 100644
--- a/drivers/media/video/mt9t031.c
+++ b/drivers/media/video/mt9t031.c
@@ -204,6 +204,71 @@ static unsigned long mt9t031_query_bus_param(struct
soc_camera_device *icd)
return soc_camera_apply_sensor_flags(icl, MT9T031_BUS_PARAM);
}
+enum {
+ MT9T031_CTRL_VFLIP,
+ MT9T031_CTRL_HFLIP,
+ MT9T031_CTRL_GAIN,
+ MT9T031_CTRL_EXPOSURE,
+ MT9T031_CTRL_EXPOSURE_AUTO,
+};
+
+static const struct v4l2_queryctrl mt9t031_controls[] = {
+ [MT9T031_CTRL_VFLIP] = {
+ .id = V4L2_CID_VFLIP,
+ .type = V4L2_CTRL_TYPE_BOOLEAN,
+ .name = Flip Vertically,
+ .minimum= 0,
+ .maximum= 1,
+ .step = 1,
+ .default_value = 0,
+ },
+ [MT9T031_CTRL_HFLIP] = {
+ .id = V4L2_CID_HFLIP,
+ .type = V4L2_CTRL_TYPE_BOOLEAN,
+ .name = Flip Horizontally,
+ .minimum= 0,
+ .maximum= 1,
+ .step = 1,
+ .default_value = 0,
+ },
+ [MT9T031_CTRL_GAIN] = {
+ .id = V4L2_CID_GAIN,
+ .type = V4L2_CTRL_TYPE_INTEGER,
+ .name = Gain,
+ .minimum= 0,
+ .maximum= 127,
+ .step = 1,
+ .default_value = 64,
+ .flags = V4L2_CTRL_FLAG_SLIDER,
+ },
+ [MT9T031_CTRL_EXPOSURE] = {
+ .id = V4L2_CID_EXPOSURE,
+ .type = V4L2_CTRL_TYPE_INTEGER,
+ .name = Exposure,
+ .minimum= 1,
+ .maximum= 255,
+ .step = 1,
+ .default_value = 255,
+ .flags = V4L2_CTRL_FLAG_SLIDER,
+ },
+ [MT9T031_CTRL_EXPOSURE_AUTO] = {
+ .id = V4L2_CID_EXPOSURE_AUTO,
+
RE: [PATCH/RFC 9/9 v2] mt9t031: make the use of the soc-camera client API optional
Guennadi,
Thanks for the reply. I will have a chance to work on this
sometime in the next two weeks as I am pre-occupied with other
items. I will definitely try to use this version and do my
testing and let you know the result.
Will this apply cleanly to the v4l-dvb linux-next branch?
Murali Karicheri
Software Design Engineer
Texas Instruments Inc.
Germantown, MD 20874
phone: 301-407-9583
email: m-kariche...@ti.com
-Original Message-
From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de]
Sent: Wednesday, November 04, 2009 11:49 AM
To: Karicheri, Muralidharan
Cc: Linux Media Mailing List; Hans Verkuil; Laurent Pinchart; Sakari Ailus
Subject: [PATCH/RFC 9/9 v2] mt9t031: make the use of the soc-camera client
API optional
Now that we have moved most of the functions over to the v4l2-subdev API,
only
quering and setting bus parameters are still performed using the legacy
soc-camera client API. Make the use of this API optional for mt9t031.
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
On Mon, 2 Nov 2009, Karicheri, Muralidharan wrote:
+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?
In general I'm sure you understand, that there are lots of functions in
the kernel, that we use in specific modules, not because they interact
with other systems, but because they implement some common functionality
and just reduce code-duplication. And I can well imagine that in many such
cases using just one or a couple of such functions will pull a much larger
pile of unused code with them. But in this case those calls can indeed be
very easily eliminated. Please have a look at the version below.
Did you have a chance to compare the driver file that I had sent to you?
I looked at it, but it is based on an earlier version of the driver, so,
it wasn't very easy to compare. Maybe you could send a diff against the
mainline version, on which it is based?
Thanks
Guennadi
drivers/media/video/mt9t031.c | 167 +
1 files changed, 85 insertions(+), 82 deletions(-)
diff --git a/drivers/media/video/mt9t031.c b/drivers/media/video/mt9t031.c
index c95c277..86bf8f6 100644
--- a/drivers/media/video/mt9t031.c
+++ b/drivers/media/video/mt9t031.c
@@ -204,6 +204,71 @@ static unsigned long mt9t031_query_bus_param(struct
soc_camera_device *icd)
return soc_camera_apply_sensor_flags(icl, MT9T031_BUS_PARAM);
}
+enum {
+ MT9T031_CTRL_VFLIP,
+ MT9T031_CTRL_HFLIP,
+ MT9T031_CTRL_GAIN,
+ MT9T031_CTRL_EXPOSURE,
+ MT9T031_CTRL_EXPOSURE_AUTO,
+};
+
+static const struct v4l2_queryctrl mt9t031_controls[] = {
+ [MT9T031_CTRL_VFLIP] = {
+ .id = V4L2_CID_VFLIP,
+ .type = V4L2_CTRL_TYPE_BOOLEAN,
+ .name = Flip Vertically,
+ .minimum= 0,
+ .maximum= 1,
+ .step = 1,
+ .default_value = 0,
+ },
+ [MT9T031_CTRL_HFLIP] = {
+ .id = V4L2_CID_HFLIP,
+ .type = V4L2_CTRL_TYPE_BOOLEAN,
+ .name = Flip Horizontally,
+ .minimum= 0,
+ .maximum= 1,
+ .step = 1,
+ .default_value = 0,
+ },
+ [MT9T031_CTRL_GAIN] = {
+ .id = V4L2_CID_GAIN,
+ .type = V4L2_CTRL_TYPE_INTEGER,
+ .name = Gain
RE: [PATCH/RFC 9/9 v2] mt9t031: make the use of the soc-camera client API optional
On Wed, 4 Nov 2009, Karicheri, Muralidharan wrote: Guennadi, Thanks for the reply. I will have a chance to work on this sometime in the next two weeks as I am pre-occupied with other items. I will definitely try to use this version and do my testing and let you know the result. Will this apply cleanly to the v4l-dvb linux-next branch? Maybe you can apply the whole set of 9 patches to it, not sure. Better yet get the complete stack from the location I provided in the introductory mail (0/9) and apply it as instructed there. Just beware, that there are still some older patch versions, which you would have to replace with the ones from this thread. I'll try to update that stack shortly. 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH/RFC 9/9 v2] mt9t031: make the use of the soc-camera client API optional
On Wed, 4 Nov 2009, Guennadi Liakhovetski wrote: On Wed, 4 Nov 2009, Karicheri, Muralidharan wrote: Guennadi, Thanks for the reply. I will have a chance to work on this sometime in the next two weeks as I am pre-occupied with other items. I will definitely try to use this version and do my testing and let you know the result. Will this apply cleanly to the v4l-dvb linux-next branch? Maybe you can apply the whole set of 9 patches to it, not sure. Better yet get the complete stack from the location I provided in the introductory mail (0/9) and apply it as instructed there. Just beware, that there are still some older patch versions, which you would have to replace with the ones from this thread. I'll try to update that stack shortly. The current stack, based on 2.6.32-rc5, is at http://download.open-technology.de/soc-camera/20091105/ 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
