-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


~ Luká? Karas wrote:
| Hi Eric.
|
| There is my today edit for s5k83a sensor:
http://www.karry.wz.cz/download/m5602.20080906.zip
| I was edited m5602 branch. Now is possible controll brightness,
whiteness and gain.
|
| I identify sensor register (0x18), that drasticaly extend expose time.
With maximal value is fps about 3!
| In this case getting good images in dark. You can see at:
http://www.karry.wz.cz/download/m5602-night_mode.png
| I named this properties as GAIN. It is right? I dont find apposites
properties on http://v4l2spec.bytesex.org/spec/x542.htm ...
|
| At end, I have small question. What you use as camera viewer? I find
neither viewer, that support change all parameters
| and work good with m5602...
|
| Regards Karry


Hi again.
This is a diff against the current driver. See my comments inline.
Please address them and send me the updated driver as a patch.


- ---
/home/erik/Utveckling/m560x-driver/m560x/branches/m5602/m5602_s5k83a.c
2008-09-06 16:59:02.000000000 +0200
+++ m5602_s5k83a.c      2008-09-06 13:07:53.000000000 +0200
@@ -18,6 +18,11 @@

~ #include "m5602_s5k83a.h"

+static int brightness = S5K83A_DEFAULT_BRIGHTNESS;
+static int whiteness = S5K83A_DEFAULT_WHITENESS;
+static int gain = S5K83A_DEFAULT_GAIN;
+

Do not use static integers in the code. It's not pretty and can cause
race conditions. Also why shadow the registers in the sensor with local
variables?

+
~ static int s5k83a_probe(struct m5602_camera *cam)
~ {
~       u8 prod_id = 0, ver_id = 0;
@@ -159,6 +164,10 @@
~                       return -EINVAL;
~               }
~       }
+
+    s5k83a_set_brightness(cam,brightness);
+    s5k83a_set_whiteness(cam, whiteness);
+    s5k83a_set_gain(cam, gain);

This is ok I guess, although I would prefer if you altered the init code
in the m5602_s5k83a.h instead.

~       if (dump_sensor)
~               s5k83a_dump_registers(cam);
@@ -173,13 +182,102 @@

~ static int s5k83a_get_ctrl(struct m5602_camera *cam, struct
v4l2_control *ctrl)
~ {
- -     return 0;
+    switch (ctrl->id) {
+        case V4L2_CID_BRIGHTNESS:
+            ctrl->value = brightness;
+            PDEBUG(DBG_V4L2, "get brightness (0x%x)", ctrl->value );
+            return 0;

Probe the actual sensor registers instead of copying the shadowed variable.

+        case V4L2_CID_WHITENESS:
+            ctrl->value = whiteness;
+            PDEBUG(DBG_V4L2, "get whiteness (0x%x)", ctrl->value );

Same here.

+            return 0;
+        case V4L2_CID_GAIN:
+            ctrl->value = gain;
+            PDEBUG(DBG_V4L2, "get gain (0x%x)", ctrl->value );
+            return 0;

And here
+        default:
+            PDEBUG(DBG_V4L2, "s5k83a_get_ctrl 0x%x", ctrl->id);

This should warn that the selected ioctl isn't supported.


+    }
+    return EINVAL;
~ }



~ static int s5k83a_set_ctrl(struct m5602_camera *cam,
~                          const struct v4l2_control *ctrl)
~ {
- -     return 0;
+    switch (ctrl->id) {
+        case V4L2_CID_BRIGHTNESS:
+            brightness = ctrl->value;
+            s5k83a_set_brightness( cam, ctrl->value );
+            PDEBUG(DBG_V4L2, "set brightness to 0x%x", ctrl->value );
+            return 0;

Please remove the spaces around the brackets and replace the return with
a break.


+        case V4L2_CID_WHITENESS:
+            whiteness = ctrl->value;
+            s5k83a_set_whiteness(cam, ctrl->value);
+            PDEBUG(DBG_V4L2, "set whiteness to 0x%x", ctrl->value );
+            return 0;

Same here.

+        case V4L2_CID_GAIN:
+            gain = ctrl->value;
+            s5k83a_set_gain(cam, ctrl->value);
+            PDEBUG(DBG_V4L2, "set whiteness to 0x%x", ctrl->value );
+            return 0;

Same here.
+        default:
+            PDEBUG(DBG_V4L2, "s5k83a_set_ctrl 0x%x to value 0x%x",
ctrl->id, ctrl->value);
+    }
+    return EINVAL;

Check for errors and set the return value properly.

+}
+
+
+static void s5k83a_set_brightness(struct m5602_camera *cam, u16 value)
+{
+    int err = 0;
+    u8 data[2];
+
+    data[0] = 0x00;
+    data[1] = 0x20;
+    err = s5k83a_write_sensor(cam, 0x14, data, 2);
Is this necessary? What does it do?

+
+    data[0] = 0x01;
+    data[1] = (value & 0xff);      // part of expose time (i dont know
what is this...)

If you don't know what this is why do you write to it?

+    err = s5k83a_write_sensor(cam, 0x0d, data, 2);
+
+    data[0] = (value ) >> 3; // brightness, high 5 bits
+    data[1] = (value ) >> 2; // brightness, high 6 bits (range is
7bits, but with the highest bit is unusable)
This doesn't make any sense. Do you really need to write almost the same
value to the two bytes? What happens if you only write to one byte?

+    err = s5k83a_write_sensor(cam, 0x1b, data, 2);
+
+}
+
+static void s5k83a_set_whiteness(struct m5602_camera *cam, u8 value)
+{
+    int tmp, err = 0;
+    u8 data[2];
+    tmp = value;
+
+
+    data[0] = value;
+    data[1] = 0x00;
+    err = s5k83a_write_sensor(cam, 0x0a, data, 1);
+
If you only write the first byte, don't set the next at all.

+
+    /*
+    // this code is for playing :-D
+    info("set to value 0x%x", tmp);
+
+    data[0] = tmp;
+    data[1] = tmp;
+    err = s5k83a_write_sensor(cam, 0xe8, data, 2);
+    */
Remove this comment.

+}
+
+static void s5k83a_set_gain(struct m5602_camera *cam, u8 value)
Don't return void but the error value generated by the
s5k83a_write_sensor() command.

+{
+    // values near 0x3c is greath for nigh mode
Remove this comment.

+    int err = 0;
+    u8 data[2];
+
+    data[0] = 0;
+    data[1] = value;
+    err = s5k83a_write_sensor(cam, 0x18, data, 2);

Is it only the high byte that alters the gain?

+
~ }

~ static void s5k83a_dump_registers(struct m5602_camera *cam)

Thanks,
Erik



|
|
|
| -------------------------------------------------------------------------
| This SF.Net email is sponsored by the Moblin Your Move Developer's
challenge
| Build the coolest Linux based applications with Moblin SDK & win great
prizes
| Grand prize is a trip for two to an Open Source event anywhere in the
world
| http://moblin-contest.org/redirect.php?banner_id=100&url=/
| _______________________________________________
| M560x-driver-devel mailing list
| [email protected]
| https://lists.sourceforge.net/lists/listinfo/m560x-driver-devel
|
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFIwqQcN7qBt+4UG0ERAgiAAJ9VeLNvfTyxm+oHSRNphSpItDOazwCdFXn0
h7BA8goqSrDa0FMyJfEPrNE=
=OpcB
-----END PGP SIGNATURE-----

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
M560x-driver-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/m560x-driver-devel

Reply via email to