Re: [RFC PATCH v7] media: add v4l2 subdev driver for S5K4ECGX sensor
Hi Francesco
On 12 September 2012 19:07, Francesco Lavra francescolavra...@gmail.com wrote:
Hi Sangwook,
two remarks from my review on September 9th haven't been addressed.
Thanks for the review.
I missed those, please let me correct them and send patch again.
Regards
Sangwook
I believe those remarks are correct, but please let me know if I'm
missing something.
See below.
On 09/12/2012 01:26 PM, Sangwook Lee wrote:
+static int s5k4ecgx_s_power(struct v4l2_subdev *sd, int on)
+{
+ struct s5k4ecgx *priv = to_s5k4ecgx(sd);
+ int ret;
+
+ v4l2_dbg(1, debug, sd, Switching %s\n, on ? on : off);
+
+ if (on) {
+ ret = __s5k4ecgx_power_on(priv);
+ if (ret 0)
+ return ret;
+ /* Time to stabilize sensor */
+ msleep(100);
+ ret = s5k4ecgx_init_sensor(sd);
+ if (ret 0)
+ __s5k4ecgx_power_off(priv);
+ else
+ priv-set_params = 1;
+ } else {
+ ret = __s5k4ecgx_power_off(priv);
+ }
+
+ return 0;
return ret;
+static int s5k4ecgx_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ int ret, i;
+ struct v4l2_subdev *sd;
+ struct s5k4ecgx *priv;
+ struct s5k4ecgx_platform_data *pdata = client-dev.platform_data;
+
+ if (pdata == NULL) {
+ dev_err(client-dev, platform data is missing!\n);
+ return -EINVAL;
+ }
+ priv = devm_kzalloc(client-dev, sizeof(struct s5k4ecgx), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ mutex_init(priv-lock);
+ priv-streaming = 0;
+
+ sd = priv-sd;
+ /* Registering subdev */
+ v4l2_i2c_subdev_init(sd, client, s5k4ecgx_ops);
+ strlcpy(sd-name, S5K4ECGX_DRIVER_NAME, sizeof(sd-name));
+
+ sd-internal_ops = s5k4ecgx_subdev_internal_ops;
+ /* Support v4l2 sub-device user space API */
+ sd-flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+
+ priv-pad.flags = MEDIA_PAD_FL_SOURCE;
+ sd-entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
+ ret = media_entity_init(sd-entity, 1, priv-pad, 0);
+ if (ret)
+ return ret;
+
+ ret = s5k4ecgx_config_gpios(priv, pdata);
+ if (ret) {
+ dev_err(client-dev, Failed to set gpios\n);
+ goto out_err1;
+ }
+ for (i = 0; i S5K4ECGX_NUM_SUPPLIES; i++)
+ priv-supplies[i].supply = s5k4ecgx_supply_names[i];
+
+ ret = devm_regulator_bulk_get(client-dev, S5K4ECGX_NUM_SUPPLIES,
+ priv-supplies);
+ if (ret) {
+ dev_err(client-dev, Failed to get regulators\n);
+ goto out_err2;
+ }
+ ret = s5k4ecgx_init_v4l2_ctrls(priv);
+ if (ret)
+ goto out_err2;
+
+ priv-curr_pixfmt = s5k4ecgx_formats[0];
+ priv-curr_frmsize = s5k4ecgx_prev_sizes[0];
+
+ return 0;
+
+out_err2:
+ s5k4ecgx_free_gpios(priv);
+out_err1:
+ media_entity_cleanup(priv-sd.entity);
+
+ return ret;
+}
+
+static int s5k4ecgx_remove(struct i2c_client *client)
+{
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct s5k4ecgx *priv = to_s5k4ecgx(sd);
+
+ mutex_destroy(priv-lock);
+ v4l2_device_unregister_subdev(sd);
+ v4l2_ctrl_handler_free(priv-handler);
+ media_entity_cleanup(sd-entity);
+
+ return 0;
s5k4ecgx_free_gpios() should be called to release the GPIOs
Thanks,
Francesco
--
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
[RFC PATCH v8] media: add v4l2 subdev driver for S5K4ECGX sensor
This patch adds driver for S5K4ECGX sensor with embedded ISP SoC, S5K4ECGX, which is a 5M CMOS Image sensor from Samsung The driver implements preview mode of the S5K4ECGX sensor. capture (snapshot) operation, face detection are missing now. Following controls are supported: contrast/saturation/brightness/sharpness Signed-off-by: Sangwook Lee sangwook@linaro.org Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com Cc: Francesco Lavra francescolavra...@gmail.com Cc: Scott Bambrough scott.bambro...@linaro.org Cc: Homin Lee suap...@insignal.co.kr --- Changes since v7: - added gpio free function - fixed return value of power function Changes since v6: - fixed alignment issue from Francesco, Sylwester Changes since v5: - deleted dummy lines - fixed pointer errors in handling firmware - updated comments - added le32_to_cpu,le16_to_cpu Changes since v4: - replaced register tables with the function from Sylwester - updated firmware parsing function with CRC32 check firmware generator from user space: git://git.linaro.org/people/sangwook/fimc-v4l2-app.git Changes since v3: - used request_firmware to configure initial settings - added parsing functions to read initial settings - updated regulator API - reduced preview setting tables by experiment Changes since v2: - added GPIO (reset/stby) and regulators - updated I2C read/write, based on s5k6aa datasheet - fixed set_fmt errors - reduced register tables a bit - removed vmalloc Changes since v1: - fixed s_stream(0) when it called twice - changed mutex_X position to be used when strictly necessary - add additional s_power(0) in case that error happens - update more accurate debugging statements - remove dummy else drivers/media/i2c/Kconfig|7 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/s5k4ecgx.c | 1017 ++ include/media/s5k4ecgx.h | 37 ++ 4 files changed, 1062 insertions(+) create mode 100644 drivers/media/i2c/s5k4ecgx.c create mode 100644 include/media/s5k4ecgx.h diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index 9a5a059..55b3bbb 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -484,6 +484,13 @@ config VIDEO_S5K6AA This is a V4L2 sensor-level driver for Samsung S5K6AA(FX) 1.3M camera sensor with an embedded SoC image signal processor. +config VIDEO_S5K4ECGX +tristate Samsung S5K4ECGX sensor support +depends on I2C VIDEO_V4L2 VIDEO_V4L2_SUBDEV_API +---help--- + This is a V4L2 sensor-level driver for Samsung S5K4ECGX 5M + camera sensor with an embedded SoC image signal processor. + source drivers/media/i2c/smiapp/Kconfig comment Flash devices diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index 088a460..a720812 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -55,6 +55,7 @@ obj-$(CONFIG_VIDEO_MT9V032) += mt9v032.o obj-$(CONFIG_VIDEO_SR030PC30) += sr030pc30.o obj-$(CONFIG_VIDEO_NOON010PC30)+= noon010pc30.o obj-$(CONFIG_VIDEO_S5K6AA) += s5k6aa.o +obj-$(CONFIG_VIDEO_S5K4ECGX) += s5k4ecgx.o obj-$(CONFIG_VIDEO_ADP1653)+= adp1653.o obj-$(CONFIG_VIDEO_AS3645A)+= as3645a.o obj-$(CONFIG_VIDEO_SMIAPP_PLL) += smiapp-pll.o diff --git a/drivers/media/i2c/s5k4ecgx.c b/drivers/media/i2c/s5k4ecgx.c new file mode 100644 index 000..7b455e1 --- /dev/null +++ b/drivers/media/i2c/s5k4ecgx.c @@ -0,0 +1,1017 @@ +/* + * Driver for s5k4ecgx (5MP Camera) from Samsung + * a quarter-inch optical format 1.4 micron 5 megapixel (Mp) + * CMOS image sensor. + * + * Copyright (C) 2012, Linaro, Sangwook Lee sangwook@linaro.org + * Copyright (C) 2012, Insignal Co,. Ltd, Homin Lee suap...@insignal.co.kr + * + * Based on s5k6aa, noon010pc30 driver + * Copyright (C) 2011, Samsung Electronics Co., Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#include linux/clk.h +#include linux/crc32.h +#include linux/ctype.h +#include linux/delay.h +#include linux/firmware.h +#include linux/gpio.h +#include linux/i2c.h +#include linux/module.h +#include linux/regulator/consumer.h +#include linux/slab.h +#include asm/unaligned.h + +#include media/media-entity.h +#include media/s5k4ecgx.h +#include media/v4l2-ctrls.h +#include media/v4l2-device.h +#include media/v4l2-mediabus.h +#include media/v4l2-subdev.h + +static int debug; +module_param(debug, int, 0644); + +#define S5K4ECGX_DRIVER_NAME s5k4ecgx +#define S5K4ECGX_FIRMWARE s5k4ecgx.bin + +/* Firmware revision information */ +#define REG_FW_REVISION0x71a6 +#define REG_FW_VERSION 0x71a4 +#define S5K4ECGX_REVISION_1_1 0x11 +#define S5K4ECGX_FW_VERSION0x4ec0
[RFC PATCH v7] media: add v4l2 subdev driver for S5K4ECGX sensor
This patch adds driver for S5K4ECGX sensor with embedded ISP SoC, S5K4ECGX, which is a 5M CMOS Image sensor from Samsung The driver implements preview mode of the S5K4ECGX sensor. capture (snapshot) operation, face detection are missing now. Following controls are supported: contrast/saturation/brightness/sharpness Signed-off-by: Sangwook Lee sangwook@linaro.org Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com Cc: Francesco Lavra francescolavra...@gmail.com CC: Scott Bambrough scott.bambro...@linaro.org Cc: Homin Lee suap...@insignal.co.kr --- Changes since v6: - fix alignment issue from Francesco, Sylwester Changes since v5: - deleted dummy lines - fixed pointer errors in handling firmware - updated comments - added le32_to_cpu,le16_to_cpu Changes since v4: - replaced register tables with the function from Sylwester - updated firmware parsing function with CRC32 check firmware generator from user space: git://git.linaro.org/people/sangwook/fimc-v4l2-app.git Changes since v3: - used request_firmware to configure initial settings - added parsing functions to read initial settings - updated regulator API - reduced preview setting tables by experiment Changes since v2: - added GPIO (reset/stby) and regulators - updated I2C read/write, based on s5k6aa datasheet - fixed set_fmt errors - reduced register tables a bit - removed vmalloc Changes since v1: - fixed s_stream(0) when it called twice - changed mutex_X position to be used when strictly necessary - add additional s_power(0) in case that error happens - update more accurate debugging statements - remove dummy else drivers/media/i2c/Kconfig|7 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/s5k4ecgx.c | 1016 ++ include/media/s5k4ecgx.h | 37 ++ 4 files changed, 1061 insertions(+) create mode 100644 drivers/media/i2c/s5k4ecgx.c create mode 100644 include/media/s5k4ecgx.h diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index 9a5a059..55b3bbb 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -484,6 +484,13 @@ config VIDEO_S5K6AA This is a V4L2 sensor-level driver for Samsung S5K6AA(FX) 1.3M camera sensor with an embedded SoC image signal processor. +config VIDEO_S5K4ECGX +tristate Samsung S5K4ECGX sensor support +depends on I2C VIDEO_V4L2 VIDEO_V4L2_SUBDEV_API +---help--- + This is a V4L2 sensor-level driver for Samsung S5K4ECGX 5M + camera sensor with an embedded SoC image signal processor. + source drivers/media/i2c/smiapp/Kconfig comment Flash devices diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index 088a460..a720812 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -55,6 +55,7 @@ obj-$(CONFIG_VIDEO_MT9V032) += mt9v032.o obj-$(CONFIG_VIDEO_SR030PC30) += sr030pc30.o obj-$(CONFIG_VIDEO_NOON010PC30)+= noon010pc30.o obj-$(CONFIG_VIDEO_S5K6AA) += s5k6aa.o +obj-$(CONFIG_VIDEO_S5K4ECGX) += s5k4ecgx.o obj-$(CONFIG_VIDEO_ADP1653)+= adp1653.o obj-$(CONFIG_VIDEO_AS3645A)+= as3645a.o obj-$(CONFIG_VIDEO_SMIAPP_PLL) += smiapp-pll.o diff --git a/drivers/media/i2c/s5k4ecgx.c b/drivers/media/i2c/s5k4ecgx.c new file mode 100644 index 000..dd2f68d --- /dev/null +++ b/drivers/media/i2c/s5k4ecgx.c @@ -0,0 +1,1016 @@ +/* + * Driver for s5k4ecgx (5MP Camera) from Samsung + * a quarter-inch optical format 1.4 micron 5 megapixel (Mp) + * CMOS image sensor. + * + * Copyright (C) 2012, Linaro, Sangwook Lee sangwook@linaro.org + * Copyright (C) 2012, Insignal Co,. Ltd, Homin Lee suap...@insignal.co.kr + * + * Based on s5k6aa, noon010pc30 driver + * Copyright (C) 2011, Samsung Electronics Co., Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#include linux/clk.h +#include linux/crc32.h +#include linux/ctype.h +#include linux/delay.h +#include linux/firmware.h +#include linux/gpio.h +#include linux/i2c.h +#include linux/module.h +#include linux/regulator/consumer.h +#include linux/slab.h +#include asm/unaligned.h + +#include media/media-entity.h +#include media/s5k4ecgx.h +#include media/v4l2-ctrls.h +#include media/v4l2-device.h +#include media/v4l2-mediabus.h +#include media/v4l2-subdev.h + +static int debug; +module_param(debug, int, 0644); + +#define S5K4ECGX_DRIVER_NAME s5k4ecgx +#define S5K4ECGX_FIRMWARE s5k4ecgx.bin + +/* Firmware revision information */ +#define REG_FW_REVISION0x71a6 +#define REG_FW_VERSION 0x71a4 +#define S5K4ECGX_REVISION_1_1 0x11 +#define S5K4ECGX_FW_VERSION0x4ec0 + +/* General purpose parameters */ +#define REG_USER_BRIGHTNESS0x722c +#define
Re: [RFC PATCH v6] media: add v4l2 subdev driver for S5K4ECGX sensor
Hi Francesco
Thanks for your advice.
@ Sylwester
Thanks for your nice patch, and I will squash and then send it again.
Thanks
Sangwook
On 10 September 2012 21:29, Sylwester Nawrocki
sylvester.nawro...@gmail.com wrote:
On 09/10/2012 08:52 PM, Francesco Lavra wrote:
On 09/10/2012 05:04 PM, Sylwester Nawrocki wrote:
On 09/09/2012 06:01 PM, Francesco Lavra wrote:
+static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd)
+{
+ const struct firmware *fw;
+ int err, i, regs_num;
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ u16 val;
+ u32 addr, crc, crc_file, addr_inc = 0;
+
+ err = request_firmware(fw, S5K4ECGX_FIRMWARE, sd-v4l2_dev-dev);
+ if (err) {
+ v4l2_err(sd, Failed to read firmware %s\n,
S5K4ECGX_FIRMWARE);
+ return err;
+ }
+ regs_num = *(u32 *)(fw-data);
+ v4l2_dbg(3, debug, sd, FW: %s size %d register sets %d\n,
+ S5K4ECGX_FIRMWARE, fw-size, regs_num);
+ regs_num++; /* Add header */
+ if (fw-size != regs_num * FW_RECORD_SIZE + FW_CRC_SIZE) {
+ err = -EINVAL;
+ goto fw_out;
+ }
+ crc_file = *(u32 *)(fw-data + regs_num * FW_RECORD_SIZE);
Depending on the value of regs_num, this may result in unaligned access
Thanks for the catch. I think it is not the only place where unaligned
issues are possible. Since the data records are 4-byte address + 2-byte
value there is also an issue with reading the address entries. Assuming
fw-data is aligned to at least 2-bytes (not quite sure if we can assume
that) there should be no problem with reading 2-byte register values.
I'm not sure 2-byte alignment can be safely assumed, either.
We could change the data types of the register values from u16 to u32,
wasting some memory (there is approximately 3 000 records), so there is
no other data types in the file structure than u32. Or use a patch as
below. Not sure what's better.
I prefer the approach of your patch below, but I would use get_unaligned
to get the 2-byte values, too. Also there are another couple of
glitches, see below.
OK, thanks for the feedback. It was also my preference. The performance
impact seems insignificant, given a write of each record takes time of
1 ms order.
8-
From a970480b99bdb74e2bf48e1a321724231e6516a0 Mon Sep 17 00:00:00 2001
From: Sylwester Nawrockisylvester.nawro...@gmail.com
Date: Sun, 9 Sep 2012 19:56:31 +0200
Subject: [PATCH] s5k4ecgx: Fix unaligned access issues
Signed-off-by: Sylwester Nawrockisylvester.nawro...@gmail.com
---
drivers/media/i2c/s5k4ecgx.c | 16
1 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/s5k4ecgx.c b/drivers/media/i2c/s5k4ecgx.c
index 0ef0b7d..4c6439a 100644
--- a/drivers/media/i2c/s5k4ecgx.c
+++ b/drivers/media/i2c/s5k4ecgx.c
@@ -24,6 +24,7 @@
#includelinux/module.h
#includelinux/regulator/consumer.h
#includelinux/slab.h
+#includeasm/unaligned.h
#includemedia/media-entity.h
#includemedia/s5k4ecgx.h
@@ -331,6 +332,7 @@ static int s5k4ecgx_load_firmware(struct v4l2_subdev
*sd)
const struct firmware *fw;
int err, i, regs_num;
u32 addr, crc, crc_file, addr_inc = 0;
+const u8 *ptr;
u16 val;
err = request_firmware(fw, S5K4ECGX_FIRMWARE, sd-v4l2_dev-dev);
@@ -338,7 +340,7 @@ static int s5k4ecgx_load_firmware(struct v4l2_subdev
*sd)
v4l2_err(sd, Failed to read firmware %s\n,
S5K4ECGX_FIRMWARE);
return err;
}
-regs_num = le32_to_cpu(*(u32 *)fw-data);
+regs_num = le32_to_cpu(get_unaligned((__le32 *)fw-data));
v4l2_dbg(3, debug, sd, FW: %s size %d register sets %d\n,
S5K4ECGX_FIRMWARE, fw-size, regs_num);
@@ -349,7 +351,8 @@ static int s5k4ecgx_load_firmware(struct v4l2_subdev
*sd)
goto fw_out;
}
-crc_file = *(u32 *)(fw-data + regs_num * FW_RECORD_SIZE);
+memcpy(crc_file, fw-data + regs_num * FW_RECORD_SIZE, sizeof(u32));
crc_file should be converted from little endian to native endian.
Right, I should have verified that crc32_le() return value is in native
endianness.
+
crc = crc32_le(~0, fw-data, regs_num * FW_RECORD_SIZE);
if (crc != crc_file) {
v4l2_err(sd, FW: invalid crc (%#x:%#x)\n, crc, crc_file);
@@ -357,9 +360,14 @@ static int s5k4ecgx_load_firmware(struct v4l2_subdev
*sd)
goto fw_out;
}
+ptr = fw-data + FW_RECORD_SIZE;
+
for (i = 1; i regs_num; i++) {
-addr = le32_to_cpu(*(u32 *)(fw-data + i * FW_RECORD_SIZE));
-val = le16_to_cpu(*(u16 *)(fw-data + i * FW_RECORD_SIZE + 4));
+addr = le32_to_cpu(get_unaligned((__le32 *)ptr));
+ptr += 4;
+val = le16_to_cpu(*(__le16 *)ptr);
+ptr += FW_RECORD_SIZE;
ptr is being incremented by (4 + FW_RECORD_SIZE) bytes at each iteration.
Oops, I was to quick in
Re: [RFC PATCH v5] media: add v4l2 subdev driver for S5K4ECGX sensor
Hi Sylwester
Thank you for the review again.
On 5 September 2012 22:56, Sylwester Nawrocki
sylvester.nawro...@gmail.com wrote:
Hi Sangwook,
On 09/05/2012 02:28 PM, Sangwook Lee wrote:
[snip]
+#includelinux/vmalloc.h
What do we need this header for ?
Ok, let me delete this.
+
+#includemedia/media-entity.h
+#includemedia/s5k4ecgx.h
+#includemedia/v4l2-ctrls.h
+#includemedia/v4l2-device.h
+#includemedia/v4l2-mediabus.h
+#includemedia/v4l2-subdev.h
...
+
+static int s5k4ecgx_set_ahb_address(struct v4l2_subdev *sd)
+{
+ int ret;
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+
+ /* Set APB peripherals start address */
+ ret = s5k4ecgx_i2c_write(client, AHB_MSB_ADDR_PTR, GEN_REG_OFFSH);
+ if (ret)
+ return ret;
+ /*
+ * FIXME: This is copied from s5k6aa, because of no information
+ * in s5k4ecgx's datasheet.
+ * sw_reset is activated to put device into idle status
+ */
+ ret = s5k4ecgx_i2c_write(client, 0x0010, 0x0001);
+ if (ret)
+ return ret;
+
+ /* FIXME: no information available about this register */
Let's drop that comment, we will fix all magic numbers once proper
documentation is available.
Ok.
+ ret = s5k4ecgx_i2c_write(client, 0x1030, 0x);
+ if (ret)
+ return ret;
+ /* Halt ARM CPU */
+ ret = s5k4ecgx_i2c_write(client, 0x0014, 0x0001);
+
+ return ret;
Just do
return s5k4ecgx_i2c_write(client, 0x0014, 0x0001);
OK, I will fix this.
+}
+
+#define FW_HEAD 6
+/* Register address, value are 4, 2 bytes */
+#define FW_REG_SIZE 6
FW_REG_SIZE is a bit confusing, maybe we could name this FW_RECORD_SIZE
or something similar ?
Fair enough
+/*
+ * Firmware has the following format:
+ *total number of records (4-bytes + 2-bytes padding) N, record 0,
+ * record N - 1, CRC32-CCITT (4-bytes)
+ * where record is a 4-byte register address followed by 2-byte
+ * register value (little endian)
+ */
+static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd)
+{
+ const struct firmware *fw;
+ int err, i, regs_num;
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ u16 val;
+ u32 addr, crc, crc_file, addr_inc = 0;
+ u8 *fwbuf;
+
+ err = request_firmware(fw, S5K4ECGX_FIRMWARE, sd-v4l2_dev-dev);
+ if (err) {
+ v4l2_err(sd, Failed to read firmware %s\n,
S5K4ECGX_FIRMWARE);
+ goto fw_out1;
return err;
OK, I will fix this.
?
+ }
+ fwbuf = kmemdup(fw-data, fw-size, GFP_KERNEL);
Why do we need this kmemdup ? Couldn't we just use fw-data ?
OK, Iet me reconsider this.
+ if (!fwbuf) {
+ err = -ENOMEM;
+ goto fw_out2;
+ }
+ crc_file = *(u32 *)(fwbuf + regs_num * FW_REG_SIZE);
regs_num is uninitialized ?
+ crc = crc32_le(~0, fwbuf, regs_num * FW_REG_SIZE);
+ if (crc != crc_file) {
+ v4l2_err(sd, FW: invalid crc (%#x:%#x)\n, crc, crc_file);
+ err = -EINVAL;
+ goto fw_out3;
+ }
+ regs_num = *(u32 *)(fwbuf);
I guess this needs to be moved up. I would make it
regs_num = le32_to_cpu(*(u32 *)fw-data);
And perhaps we need a check like:
if (fw-size regs_num * FW_REG_SIZE)
return -EINVAL;
?
+ v4l2_dbg(3, debug, sd, FW: %s size %d register sets %d\n,
+ S5K4ECGX_FIRMWARE, fw-size, regs_num);
+ regs_num++; /* Add header */
+ for (i = 1; i regs_num; i++) {
+ addr = *(u32 *)(fwbuf + i * FW_REG_SIZE);
+ val = *(u16 *)(fwbuf + i * FW_REG_SIZE + 4);
I think you need to access addr and val through le32_to_cpu() as well,
even though your ARM system might be little-endian by default, this
driver could possibly be used on machines with different endianness.
Something like this could be more optimal:
const u8 *ptr = fw-data + FW_REG_SIZE;
for (i = 1; i regs_num; i++) {
addr = le32_to_cpu(*(u32 *)ptr);
ptr += 4;
val = le16_to_cpu(*(u16 *)ptr);
ptr += FW_REG_SIZE;
Thanks for your advice. I will take le32(16)_to_cpu.
+ if (addr - addr_inc != 2)
+ err = s5k4ecgx_write(client, addr, val);
+ else
+ err = s5k4ecgx_i2c_write(client, REG_CMDBUF0_ADDR,
val);
+ if (err)
+ goto fw_out3;
nit: break instead of goto ?
Ok, I will fix this.
+ addr_inc = addr;
+ }
+fw_out3:
+ kfree(fwbuf);
+fw_out2:
+ release_firmware(fw);
+fw_out1:
+
+ return err;
+}
...
+static int s5k4ecgx_init_sensor(struct v4l2_subdev *sd)
+{
+ int ret;
+
+ ret = s5k4ecgx_set_ahb_address(sd);
+ /* The delay is from manufacturer's settings */
+ msleep(100);
+
+ ret |= s5k4ecgx_load_firmware(sd);
if (!ret)
ret
[RFC PATCH v6] media: add v4l2 subdev driver for S5K4ECGX sensor
This patch adds driver for S5K4ECGX sensor with embedded ISP SoC, S5K4ECGX, which is a 5M CMOS Image sensor from Samsung The driver implements preview mode of the S5K4ECGX sensor. capture (snapshot) operation, face detection are missing now. Following controls are supported: contrast/saturation/brightness/sharpness Signed-off-by: Sangwook Lee sangwook@linaro.org Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com --- Changes since v5: - deleted dummy lines - fixed pointer errors in handling firmware - updated comments - added le32_to_cpu,le16_to_cpu Changes since v4: - replaced register tables with the function from Sylwester - updated firmware parsing function with CRC32 check firmware generator from user space: git://git.linaro.org/people/sangwook/fimc-v4l2-app.git Changes since v3: - used request_firmware to configure initial settings - added parsing functions to read initial settings - updated regulator API - reduced preview setting tables by experiment Changes since v2: - added GPIO (reset/stby) and regulators - updated I2C read/write, based on s5k6aa datasheet - fixed set_fmt errors - reduced register tables a bit - removed vmalloc Changes since v1: - fixed s_stream(0) when it called twice - changed mutex_X position to be used when strictly necessary - add additional s_power(0) in case that error happens - update more accurate debugging statements - remove dummy else drivers/media/i2c/Kconfig|7 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/s5k4ecgx.c | 1011 ++ include/media/s5k4ecgx.h | 37 ++ 4 files changed, 1056 insertions(+) create mode 100644 drivers/media/i2c/s5k4ecgx.c create mode 100644 include/media/s5k4ecgx.h diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index 9a5a059..55b3bbb 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -484,6 +484,13 @@ config VIDEO_S5K6AA This is a V4L2 sensor-level driver for Samsung S5K6AA(FX) 1.3M camera sensor with an embedded SoC image signal processor. +config VIDEO_S5K4ECGX +tristate Samsung S5K4ECGX sensor support +depends on I2C VIDEO_V4L2 VIDEO_V4L2_SUBDEV_API +---help--- + This is a V4L2 sensor-level driver for Samsung S5K4ECGX 5M + camera sensor with an embedded SoC image signal processor. + source drivers/media/i2c/smiapp/Kconfig comment Flash devices diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index 088a460..a720812 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -55,6 +55,7 @@ obj-$(CONFIG_VIDEO_MT9V032) += mt9v032.o obj-$(CONFIG_VIDEO_SR030PC30) += sr030pc30.o obj-$(CONFIG_VIDEO_NOON010PC30)+= noon010pc30.o obj-$(CONFIG_VIDEO_S5K6AA) += s5k6aa.o +obj-$(CONFIG_VIDEO_S5K4ECGX) += s5k4ecgx.o obj-$(CONFIG_VIDEO_ADP1653)+= adp1653.o obj-$(CONFIG_VIDEO_AS3645A)+= as3645a.o obj-$(CONFIG_VIDEO_SMIAPP_PLL) += smiapp-pll.o diff --git a/drivers/media/i2c/s5k4ecgx.c b/drivers/media/i2c/s5k4ecgx.c new file mode 100644 index 000..66107c8 --- /dev/null +++ b/drivers/media/i2c/s5k4ecgx.c @@ -0,0 +1,1011 @@ +/* + * Driver for s5k4ecgx (5MP Camera) from Samsung + * a quarter-inch optical format 1.4 micron 5 megapixel (Mp) + * CMOS image sensor. + * + * Copyright (C) 2012, Linaro, Sangwook Lee sangwook@linaro.org + * Copyright (C) 2012, Insignal Co,. Ltd, Homin Lee suap...@insignal.co.kr + * + * Based on s5k6aa, noon010pc30 driver + * Copyright (C) 2011, Samsung Electronics Co., Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#include linux/clk.h +#include linux/crc32.h +#include linux/ctype.h +#include linux/delay.h +#include linux/firmware.h +#include linux/gpio.h +#include linux/i2c.h +#include linux/module.h +#include linux/regulator/consumer.h +#include linux/slab.h + +#include media/media-entity.h +#include media/s5k4ecgx.h +#include media/v4l2-ctrls.h +#include media/v4l2-device.h +#include media/v4l2-mediabus.h +#include media/v4l2-subdev.h + +static int debug; +module_param(debug, int, 0644); + +#define S5K4ECGX_DRIVER_NAME s5k4ecgx +#define S5K4ECGX_FIRMWARE s5k4ecgx.bin + +/* Firmware revision information */ +#define REG_FW_REVISION0x71a6 +#define REG_FW_VERSION 0x71a4 +#define S5K4ECGX_REVISION_1_1 0x11 +#define S5K4ECGX_FW_VERSION0x4ec0 + +/* General purpose parameters */ +#define REG_USER_BRIGHTNESS0x722c +#define REG_USER_CONTRAST 0x722e +#define REG_USER_SATURATION0x7230 + +#define REG_G_NEW_CFG_SYNC 0x724a +#define REG_G_PREV_IN_WIDTH0x7250 +#define REG_G_PREV_IN_HEIGHT
[RFC PATCH v5] media: add v4l2 subdev driver for S5K4ECGX sensor
This patch adds driver for S5K4ECGX sensor with embedded ISP SoC, S5K4ECGX, which is a 5M CMOS Image sensor from Samsung The driver implements preview mode of the S5K4ECGX sensor. capture (snapshot) operation, face detection are missing now. Following controls are supported: contrast/saturation/brightness/sharpness Signed-off-by: Sangwook Lee sangwook@linaro.org Cc: Sylwester Nawrocki s.nawro...@samsung.com Cc: Scott Bambrough scott.bambro...@linaro.org --- Changes since v4: - replaced register tables with the function from Sylwester - updated firmware parsing function with CRC32 check firmware generator from user space: git://git.linaro.org/people/sangwook/fimc-v4l2-app.git Changes since v3: - used request_firmware to configure initial settings - added parsing functions to read initial settings - updated regulator API - reduced preview setting tables by experiment Changes since v2: - added GPIO (reset/stby) and regulators - updated I2C read/write, based on s5k6aa datasheet - fixed set_fmt errors - reduced register tables a bit - removed vmalloc Changes since v1: - fixed s_stream(0) when it called twice - changed mutex_X position to be used when strictly necessary - add additional s_power(0) in case that error happens - update more accurate debugging statements - remove dummy else drivers/media/i2c/Kconfig|7 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/s5k4ecgx.c | 1019 ++ include/media/s5k4ecgx.h | 37 ++ 4 files changed, 1064 insertions(+) create mode 100644 drivers/media/i2c/s5k4ecgx.c create mode 100644 include/media/s5k4ecgx.h diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index 9a5a059..55b3bbb 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -484,6 +484,13 @@ config VIDEO_S5K6AA This is a V4L2 sensor-level driver for Samsung S5K6AA(FX) 1.3M camera sensor with an embedded SoC image signal processor. +config VIDEO_S5K4ECGX +tristate Samsung S5K4ECGX sensor support +depends on I2C VIDEO_V4L2 VIDEO_V4L2_SUBDEV_API +---help--- + This is a V4L2 sensor-level driver for Samsung S5K4ECGX 5M + camera sensor with an embedded SoC image signal processor. + source drivers/media/i2c/smiapp/Kconfig comment Flash devices diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index 088a460..a720812 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -55,6 +55,7 @@ obj-$(CONFIG_VIDEO_MT9V032) += mt9v032.o obj-$(CONFIG_VIDEO_SR030PC30) += sr030pc30.o obj-$(CONFIG_VIDEO_NOON010PC30)+= noon010pc30.o obj-$(CONFIG_VIDEO_S5K6AA) += s5k6aa.o +obj-$(CONFIG_VIDEO_S5K4ECGX) += s5k4ecgx.o obj-$(CONFIG_VIDEO_ADP1653)+= adp1653.o obj-$(CONFIG_VIDEO_AS3645A)+= as3645a.o obj-$(CONFIG_VIDEO_SMIAPP_PLL) += smiapp-pll.o diff --git a/drivers/media/i2c/s5k4ecgx.c b/drivers/media/i2c/s5k4ecgx.c new file mode 100644 index 000..0f12d46 --- /dev/null +++ b/drivers/media/i2c/s5k4ecgx.c @@ -0,0 +1,1019 @@ +/* + * Driver for s5k4ecgx (5MP Camera) from Samsung + * a quarter-inch optical format 1.4 micron 5 megapixel (Mp) + * CMOS image sensor. + * + * Copyright (C) 2012, Linaro, Sangwook Lee sangwook@linaro.org + * Copyright (C) 2012, Insignal Co,. Ltd, Homin Lee suap...@insignal.co.kr + * + * Based on s5k6aa, noon010pc30 driver + * Copyright (C) 2011, Samsung Electronics Co., Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#include linux/clk.h +#include linux/crc32.h +#include linux/ctype.h +#include linux/delay.h +#include linux/firmware.h +#include linux/gpio.h +#include linux/i2c.h +#include linux/module.h +#include linux/regulator/consumer.h +#include linux/slab.h +#include linux/vmalloc.h + +#include media/media-entity.h +#include media/s5k4ecgx.h +#include media/v4l2-ctrls.h +#include media/v4l2-device.h +#include media/v4l2-mediabus.h +#include media/v4l2-subdev.h + +static int debug; +module_param(debug, int, 0644); + +#define S5K4ECGX_DRIVER_NAME s5k4ecgx +#define S5K4ECGX_FIRMWARE s5k4ecgx.bin + +/* Firmware revision information */ +#define REG_FW_REVISION0x71a6 +#define REG_FW_VERSION 0x71a4 +#define S5K4ECGX_REVISION_1_1 0x11 +#define S5K4ECGX_FW_VERSION0x4ec0 + +/* General purpose parameters */ +#define REG_USER_BRIGHTNESS0x722c +#define REG_USER_CONTRAST 0x722e +#define REG_USER_SATURATION0x7230 + +#define REG_G_NEW_CFG_SYNC 0x724a +#define REG_G_PREV_IN_WIDTH0x7250 +#define REG_G_PREV_IN_HEIGHT 0x7252 +#define REG_G_PREV_IN_XOFFS0x7254
Re: [PATH v3 0/2] Add v4l2 subdev driver for S5K4ECGX sensor with embedded SoC ISP
Hi Sylwester On 19 August 2012 22:29, Sylwester Nawrocki sylvester.nawro...@gmail.com wrote: Hi Sangwook, On 08/03/2012 04:24 PM, Sangwook Lee wrote: I was thinking about this, but this seems to be is a bit time-consuming because I have to do this just due to lack of s5k4ecgx hardware information. let me try it later once this patch is accepted. I've converted this driver to use function calls instead of the register arrays. It can be pulled, along with a couple of minor fixes/improvements, from following git tree: git://linuxtv.org/snawrocki/media.git s5k4ecgx (gitweb: http://git.linuxtv.org/snawrocki/media.git/s5k4ecgx) I don't own any Origen board thus it's untested. Could you give it a try ? Wow! Great, let me download from this git and then test. The register write sequence should be identical as in the case of using the arrays. You won't find much regarding the face detection features in V4L2, unfortunately. _Maybe_ I'll try to address this as well on some day, for now private controls might be your only solution. Unless you feel like adding face detection features support to V4L2.. ;) BTW, are your requirements to support both EVT1.0 and EVT1.1 S5K4ECGX revisions ? I have only EVT 1.1. So I have no chance to run EVT 1.0 version. Thanks Sangwook -- Regards, Sylwester -- 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: [PATH v3 0/2] Add v4l2 subdev driver for S5K4ECGX sensor with embedded SoC ISP
Hi Sylwester On 20 August 2012 09:12, Sangwook Lee sangwook@linaro.org wrote: Hi Sylwester On 19 August 2012 22:29, Sylwester Nawrocki sylvester.nawro...@gmail.com wrote: Hi Sangwook, On 08/03/2012 04:24 PM, Sangwook Lee wrote: I was thinking about this, but this seems to be is a bit time-consuming because I have to do this just due to lack of s5k4ecgx hardware information. let me try it later once this patch is accepted. I've converted this driver to use function calls instead of the register arrays. It can be pulled, along with a couple of minor fixes/improvements, from following git tree: git://linuxtv.org/snawrocki/media.git s5k4ecgx (gitweb: http://git.linuxtv.org/snawrocki/media.git/s5k4ecgx) I don't own any Origen board thus it's untested. Could you give it a try ? Sorry, It doesn't work. I will send pictures to you by another mail thread. Previously, I tested preview array and found out that + /* +* FIXME: according to the datasheet, +* 0x7496~ 0x749c seems to be only for capture mode, +* but without these value, it doesn't work with preview mode. +*/ Do we need to set those values ? Thanks Sangwook Wow! Great, let me download from this git and then test. Thanks -- 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 v4 0/2] Add v4l2 subdev driver for S5K4ECGX sensor with embedded SoC ISP
The following 2 patches add driver for S5K4ECGX sensor with embedded ISP SoC, and minor v4l2 control API enhancement. S5K4ECGX is 5M CMOS Image sensor from Samsung Changes since v3: - used request_firmware to configure initial settings - added parsing functions to read initial settings - updated regulator API - reduced preview setting tables by experiment Changes since v2: - added GPIO (reset/stby) and regulators - updated I2C read/write, based on s5k6aa datasheet - fixed set_fmt errors - reduced register tables a bit - removed vmalloc Changes since v1: - fixed s_stream(0) when it called twice - changed mutex_X position to be used when strictly necessary - add additional s_power(0) in case that error happens - update more accurate debugging statements - remove dummy else Sangwook Lee (2): v4l: Add factory register values form S5K4ECGX sensor v4l: Add v4l2 subdev driver for S5K4ECGX sensor drivers/media/video/Kconfig |8 + drivers/media/video/Makefile|1 + drivers/media/video/s5k4ecgx.c | 941 +++ drivers/media/video/s5k4ecgx_regs.h | 138 + include/media/s5k4ecgx.h| 37 ++ 5 files changed, 1125 insertions(+) create mode 100644 drivers/media/video/s5k4ecgx.c create mode 100644 drivers/media/video/s5k4ecgx_regs.h create mode 100644 include/media/s5k4ecgx.h -- 1.7.9.5 -- 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 v4 1/2] v4l: Add factory register values form S5K4ECGX sensor
Add preview default settings for S5K4ECGX sensor registers,
which was copied from the reference code of Samsung S.LSI.
Signed-off-by: Sangwook Lee sangwook@linaro.org
---
drivers/media/video/s5k4ecgx_regs.h | 138 +++
1 file changed, 138 insertions(+)
create mode 100644 drivers/media/video/s5k4ecgx_regs.h
diff --git a/drivers/media/video/s5k4ecgx_regs.h
b/drivers/media/video/s5k4ecgx_regs.h
new file mode 100644
index 000..e99b0e6
--- /dev/null
+++ b/drivers/media/video/s5k4ecgx_regs.h
@@ -0,0 +1,138 @@
+/*
+ * Samsung S5K4ECGX register tables for default values
+ *
+ * Copyright (C) 2012 Linaro
+ * Copyright (C) 2012 Insignal Co,. Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __DRIVERS_MEDIA_VIDEO_S5K4ECGX_H__
+#define __DRIVERS_MEDIA_VIDEO_S5K4ECGX_H__
+
+struct regval_list {
+ u32 addr;
+ u16 val;
+};
+
+/* Configure 720x480 preview size */
+static const struct regval_list s5k4ecgx_720_prev[] = {
+ { 0x7250, 0x0a00 },
+ { 0x7252, 0x06a8 },
+ { 0x7254, 0x0010 },
+ { 0x7256, 0x0078 },
+ { 0x7258, 0x0a00 },
+ { 0x725a, 0x06a8 },
+ { 0x725c, 0x0010 },
+ { 0x725e, 0x0078 },
+ { 0x7494, 0x0a00 },
+
+ /*
+* FIXME: according to the datasheet,
+* 0x7496~ 0x749c seems to be only for capture mode,
+* but without these value, it doesn't work with preview mode.
+*/
+ { 0x7496, 0x06a8 },
+ { 0x7498, 0x },
+ { 0x749a, 0x },
+ { 0x749c, 0x0a00 },
+
+ { 0x749e, 0x06a8 },
+ { 0x72a6, 0x02d0 },
+ { 0x72a8, 0x01e0 },
+ { 0x, 0x }, /* End token */
+};
+
+/* Configure 640x480 preview size */
+static const struct regval_list s5k4ecgx_640_prev[] = {
+ { 0x7250, 0x0a00 },
+ { 0x7252, 0x0780 },
+ { 0x7254, 0x0010 },
+ { 0x7256, 0x000c },
+ { 0x7258, 0x0a00 },
+ { 0x725a, 0x0780 },
+ { 0x725c, 0x0010 },
+ { 0x725e, 0x000c },
+ { 0x7494, 0x0a00 },
+ { 0x7496, 0x0780 },
+ { 0x7498, 0x },
+ { 0x749a, 0x },
+ { 0x749c, 0x0a00 },
+ { 0x749e, 0x0780 },
+ { 0x72a6, 0x0280 },
+ { 0x72a8, 0x01e0 },
+ { 0x, 0x }, /* End token */
+};
+
+/* Configure 352x288 preview size */
+static const struct regval_list s5k4ecgx_352_prev[] = {
+ { 0x7250, 0x0928 },
+ { 0x7252, 0x0780 },
+ { 0x7254, 0x007c },
+ { 0x7256, 0x000c },
+ { 0x7258, 0x0928 },
+ { 0x725a, 0x0780 },
+ { 0x725c, 0x007c },
+ { 0x725e, 0x000c },
+ { 0x7494, 0x0928 },
+ { 0x7496, 0x0780 },
+ { 0x7498, 0x },
+ { 0x749a, 0x },
+ { 0x749c, 0x0928 },
+ { 0x749e, 0x0780 },
+ { 0x72a6, 0x0160 },
+ { 0x72a8, 0x0120 },
+ { 0x, 0x }, /* End token */
+};
+
+/* Configure 176x144 preview size */
+static const struct regval_list s5k4ecgx_176_prev[] = {
+ { 0x7250, 0x0928 },
+ { 0x7252, 0x0780 },
+ { 0x7254, 0x007c },
+ { 0x7256, 0x000c },
+ { 0x7258, 0x0928 },
+ { 0x725a, 0x0780 },
+ { 0x725c, 0x007c },
+ { 0x725e, 0x000c },
+ { 0x7494, 0x0928 },
+ { 0x7496, 0x0780 },
+ { 0x7498, 0x },
+ { 0x749a, 0x },
+ { 0x749c, 0x0928 },
+ { 0x749e, 0x0780 },
+ { 0x72a6, 0x00b0 },
+ { 0x72a8, 0x0090 },
+ { 0x, 0x }, /* End token */
+};
+
+/* Common setting 1 for preview */
+static const struct regval_list s5k4ecgx_com1_prev[] = {
+ { 0x74a0, 0x },
+ { 0x74a2, 0x },
+ { 0x7262, 0x0001 },
+ { 0x7a1e, 0x0028 },
+ { 0x7ad4, 0x003c },
+ { 0x, 0x }, /* End token */
+};
+
+/* Common setting 2 for preview */
+static const struct regval_list s5k4ecgx_com2_prev[] = {
+ { 0x72aa, 0x0005 },
+ { 0x72b4, 0x0052 },
+ { 0x72be, 0x },
+ { 0x72c0, 0x0001 },
+ { 0x72c2, 0x029a },
+ { 0x72c4, 0x014d },
+ { 0x72d0, 0x },
+ { 0x72d2, 0x },
+ { 0x7266, 0x },
+ { 0x726a, 0x0001 },
+ { 0x724e, 0x0001 },
+ { 0x7268, 0x0001 },
+ { 0x, 0x }, /* End token */
+};
+
+#endif /* __DRIVERS_MEDIA_VIDEO_S5K4ECGX_H__ */
--
1.7.9.5
--
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
[PATCH v4 2/2] v4l: Add v4l2 subdev driver for S5K4ECGX sensor
This driver implements preview mode of the S5K4ECGX sensor.
capture (snapshot) operation, face detection are missing now.
Following controls are supported:
contrast/saturation/brightness/sharpness
Signed-off-by: Sangwook Lee sangwook@linaro.org
---
drivers/media/video/Kconfig|8 +
drivers/media/video/Makefile |1 +
drivers/media/video/s5k4ecgx.c | 941
include/media/s5k4ecgx.h | 37 ++
4 files changed, 987 insertions(+)
create mode 100644 drivers/media/video/s5k4ecgx.c
create mode 100644 include/media/s5k4ecgx.h
diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index c128fac..d405cb1 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -580,6 +580,14 @@ config VIDEO_S5K6AA
This is a V4L2 sensor-level driver for Samsung S5K6AA(FX) 1.3M
camera sensor with an embedded SoC image signal processor.
+config VIDEO_S5K4ECGX
+ tristate Samsung S5K4ECGX sensor support
+ depends on MEDIA_CAMERA_SUPPORT
+ depends on I2C VIDEO_V4L2 VIDEO_V4L2_SUBDEV_API
+ ---help---
+ This is a V4L2 sensor-level driver for Samsung S5K4ECGX 5M
+ camera sensor with an embedded SoC image signal processor.
+
source drivers/media/video/smiapp/Kconfig
comment Flash devices
diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
index b7da9fa..ec39c47 100644
--- a/drivers/media/video/Makefile
+++ b/drivers/media/video/Makefile
@@ -80,6 +80,7 @@ obj-$(CONFIG_VIDEO_SR030PC30) += sr030pc30.o
obj-$(CONFIG_VIDEO_NOON010PC30)+= noon010pc30.o
obj-$(CONFIG_VIDEO_M5MOLS) += m5mols/
obj-$(CONFIG_VIDEO_S5K6AA) += s5k6aa.o
+obj-$(CONFIG_VIDEO_S5K4ECGX)+= s5k4ecgx.o
obj-$(CONFIG_VIDEO_SMIAPP) += smiapp/
obj-$(CONFIG_VIDEO_ADP1653)+= adp1653.o
obj-$(CONFIG_VIDEO_AS3645A)+= as3645a.o
diff --git a/drivers/media/video/s5k4ecgx.c b/drivers/media/video/s5k4ecgx.c
new file mode 100644
index 000..4e57738
--- /dev/null
+++ b/drivers/media/video/s5k4ecgx.c
@@ -0,0 +1,941 @@
+/*
+ * Driver for s5k4ecgx (5MP Camera) from Samsung
+ * a quarter-inch optical format 1.4 micron 5 megapixel (Mp)
+ * CMOS image sensor.
+ *
+ * Copyright (C) 2012, Linaro, Sangwook Lee sangwook@linaro.org
+ * Copyright (C) 2012, Insignal Co,. Ltd, Homin Lee suap...@insignal.co.kr
+ *
+ * Based on s5k6aa, noon010pc30 driver
+ * Copyright (C) 2011, Samsung Electronics Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include linux/clk.h
+#include linux/ctype.h
+#include linux/delay.h
+#include linux/firmware.h
+#include linux/gpio.h
+#include linux/i2c.h
+#include linux/module.h
+#include linux/regulator/consumer.h
+#include linux/slab.h
+#include linux/vmalloc.h
+
+#include media/media-entity.h
+#include media/s5k4ecgx.h
+#include media/v4l2-ctrls.h
+#include media/v4l2-device.h
+#include media/v4l2-mediabus.h
+#include media/v4l2-subdev.h
+
+#include s5k4ecgx_regs.h
+
+static int debug;
+module_param(debug, int, 0644);
+
+#define S5K4ECGX_DRIVER_NAME s5k4ecgx
+
+#define S5K4ECGX_FIRMWARE s5k4ecgx.fw
+/* Firmware parsing token */
+#define FW_START_TOKEN '{'
+#define FW_END_TOKEN '}'
+
+/* Firmware revision information */
+#define REG_FW_REVISION0x71a6
+#define REG_FW_VERSION 0x71a4
+#define S5K4ECGX_REVISION_1_1 0x11
+#define S5K4ECGX_FW_VERSION0x4ec0
+
+/* General purpose parameters */
+#define REG_USER_BRIGHTNESS0x722c /* Brigthness */
+#define REG_USER_CONTRAST 0x722e /* Contrast */
+#define REG_USER_SATURATION0x7230 /* Saturation */
+
+#define REG_USER_SHARP10x7a28
+#define REG_USER_SHARP20x7ade
+#define REG_USER_SHARP30x7b94
+#define REG_USER_SHARP40x7c4a
+#define REG_USER_SHARP50x7d00
+
+/* Reduce sharpness range for user space API */
+#define SHARPNESS_DIV 8208
+#define TOK_TERM 0x
+
+/*
+ * FIMXE: This is copied from s5k6aa, because of no information
+ * in s5k4ecgx's datasheet
+ * H/W register Interface (0xd000 - 0xdfff)
+ */
+#define AHB_MSB_ADDR_PTR 0xfcfc
+#define GEN_REG_OFFSH 0xd000
+#define REG_CMDWR_ADDRH0x0028
+#define REG_CMDWR_ADDRL0x002a
+#define REG_CMDRD_ADDRH0x002c
+#define REG_CMDRD_ADDRL0x002e
+#define REG_CMDBUF0_ADDR 0x0f12
+
+/*
+ * Preview size lists supported by sensor
+ */
+static const
Re: [PATH v3 0/2] Add v4l2 subdev driver for S5K4ECGX sensor with embedded SoC ISP
Hi Sylwester
Thank you for the review.
On 2 August 2012 21:11, Sylwester Nawrocki sylvester.nawro...@gmail.com wrote:
Hi Sangwook,
On 08/02/2012 03:42 PM, Sangwook Lee wrote:
The following 2 patches add driver for S5K4ECGX sensor with embedded ISP
SoC,
and minor v4l2 control API enhancement. S5K4ECGX is 5M CMOS Image sensor
from Samsung
Changes since v2:
- added GPIO (reset/stby) and regulators
- updated I2C read/write, based on s5k6aa datasheet
- fixed set_fmt errors
- reduced register tables a bit
- removed vmalloc
It looks like a great improvement, well done! Thanks!
In the S5K4CAGX sensor datasheet, that can be found on the internet,
there is 0x...0x002E registers description, which look very much
same as in S5K6AAFX case and likely is also valid for S5K4CAGX.
[snip]
What do you think about converting s5k4ecgx_img_regs arrays (it has
over 2700 entries) to a firmware file and adding some simple parser
to the driver ? Then we would have the problem solved in most part.
Thanks, fair enough. let me try this.
Regarding various preview resolution set up, the difference in all
those s5k4ecgx_*_preview[] arrays is rather small, only register
values differ, e.g. for 640x480 and 720x480 there is only 8 different
entries:
Ok, let me reduce table size again.
$ diff -a s5k4ec_640.txt s5k4ec_720.txt
1c1
static const struct regval_list s5k4ecgx_640_preview[] = {
---
static const struct regval_list s5k4ecgx_720_preview[] = {
3c3
{ 0x7252, 0x0780 },
---
{ 0x7252, 0x06a8 },
5c5
[snip]
{ 0x7256, 0x000c },
{ 0x72a6, 0x02d
[snip]
Could you please try to implement a function that replaces those tables,
based s5k6aa_set_prev_config() and s5k6aa_set_output_framefmt() ?
I was thinking about this, but this seems to be is a bit time-consuming because
I have to do this just due to lack of s5k4ecgx hardware information.
let me try it later once
this patch is accepted.
Thanks
Sangwook
Regards,
Sylwester
--
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: [PATH v3 1/2] v4l: Add factory register values form S5K4ECGX sensor
Hi Sylwester
On 2 August 2012 21:50, Sylwester Nawrocki sylvester.nawro...@gmail.com wrote:
On 08/02/2012 03:42 PM, Sangwook Lee wrote:
Add factory default settings for S5K4ECGX sensor registers,
which was copied from the reference code of Samsung S.LSI.
Signed-off-by: Sangwook Leesangwook@linaro.org
---
drivers/media/video/s5k4ecgx_regs.h | 3105
+++
1 file changed, 3105 insertions(+)
create mode 100644 drivers/media/video/s5k4ecgx_regs.h
diff --git a/drivers/media/video/s5k4ecgx_regs.h
b/drivers/media/video/s5k4ecgx_regs.h
new file mode 100644
index 000..ef87c09
--- /dev/null
+++ b/drivers/media/video/s5k4ecgx_regs.h
@@ -0,0 +1,3105 @@
+/*
+ * Samsung S5K4ECGX register tables for default values
+ *
+ * Copyright (C) 2012 Linaro
+ * Copyright (C) 2012 Insignal Co,. Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __DRIVERS_MEDIA_VIDEO_S5K4ECGX_H__
+#define __DRIVERS_MEDIA_VIDEO_S5K4ECGX_H__
+
+struct regval_list {
+ u32 addr;
+ u16 val;
+};
+
+/*
+ * FIXME:
+ * The tables are default values of a S5K4ECGX sensor EVT1.1
+ * from Samsung LSI. currently there is no information available
+ * to the public in order to reduce these tables size.
+ */
+static const struct regval_list s5k4ecgx_apb_regs[] = {
sniiip
+/* configure 30 fps */
+static const struct regval_list s5k4ecgx_fps_30[] = {
It really depends on sensor master clock frequency (as specified
in FIMC driver platform data) and PLL setting what the resulting
frame rate will be.
+ { 0x72b4, 0x0052 },
Looks surprising! Are we really just disabling horizontal/vertical
image mirror here ?
I believe, this setting values are used still in Galaxy Nexus.
It might be some reasons to set this values in the product, but I am not
sure of this.
+ { 0x72d2, 0x },
REG_0TC_PCFG_uCaptureMirror
+ { 0x7266, 0x },
REG_TC_GP_ActivePrevConfig
+ { 0x726a, 0x0001 },
REG_TC_GP_PrevOpenAfterChange
+ { 0x724e, 0x0001 },
REG_TC_GP_NewConfigSync
+ { 0x7268, 0x0001 },
REG_TC_GP_PrevConfigChanged
Please have a look how it is handled in s5k6aa driver, it all looks
pretty similar.
+ { 0x, 0x },
+};
+
+static const struct regval_list s5k4ecgx_effect_normal[] = {
+ { 0x723c, 0x },
Just one register, why do we need an array for it ? And of course
0x is default value after reset, so it seems sort of pointless
doing this I2C write to set the default image effect value (disabled).
These are possible values as found in the datasheet:
0x723C REG_TC_GP_SpecialEffects 0x 2 RW Special effect
0 : Normal
1 : MONOCHROME (BW)
2 : Negative Mono
3 : Negative Color
4 : Sepia
5 : AQUA
6 : Reddish
7 : Bluish
8 : Greenish
9 : Sketch
10 : Emboss color
11 : Emboss Mono
+ { 0x, 0x },
+};
+
+static const struct regval_list s5k4ecgx_wb_auto[] = {
+ { 0x74e6, 0x077f },
Ditto - register REG_TC_DBG_AutoAlgEnBits. And 0x077f is the default
value after reset...
+ { 0x, 0x },
+};
+
+static const struct regval_list s5k4ecgx_iso_auto[] = {
+ { 0x7938, 0x },
+ { 0x7f2a, 0x0001 },
+ { 0x74e6, 0x077f },
+ { 0x74d0, 0x },
+ { 0x74d2, 0x },
+ { 0x74d4, 0x0001 },
+ { 0x76c2, 0x0200 },
+ { 0x, 0x },
+};
+
+static const struct regval_list s5k4ecgx_contrast_default[] = {
+ { 0x7232, 0x },
No need for an array, it's REG_TC_UserContrast.
+ { 0x, 0x },
+};
+
[snip]
+ { 0x, 0x },
+};
You already use a sequence of i2c writes in s5k4ecgx_s_ctrl() function
for V4L2_CID_SHARPNESS control. So why not just create e.g.
s5k4ecgx_set_saturation() and send this array to /dev/null ?
Also, invoking v4l2_ctrl_handler_setup() will cause a call to
s5k4ecgx_s_ctrl()
with default sharpness value (as specified during the control's creation).
So I would say this array is redundant in two ways... :)
Thanks, let me change this.
--
Regards,
Sylwester
--
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
[PATH v3 0/2] Add v4l2 subdev driver for S5K4ECGX sensor with embedded SoC ISP
The following 2 patches add driver for S5K4ECGX sensor with embedded ISP SoC, and minor v4l2 control API enhancement. S5K4ECGX is 5M CMOS Image sensor from Samsung Changes since v2: - added GPIO (reset/stby) and regulators - updated I2C read/write, based on s5k6aa datasheet - fixed set_fmt errors - reduced register tables a bit - removed vmalloc Changes since v1: - fixed s_stream(0) when it called twice - changed mutex_X position to be used when strictly necessary - add additional s_power(0) in case that error happens - update more accurate debugging statements - remove dummy else Sangwook Lee (2): v4l: Add factory register values form S5K4ECGX sensor v4l: Add v4l2 subdev driver for S5K4ECGX sensor drivers/media/video/Kconfig |8 + drivers/media/video/Makefile|1 + drivers/media/video/s5k4ecgx.c | 839 ++ drivers/media/video/s5k4ecgx_regs.h | 3105 +++ include/media/s5k4ecgx.h| 39 + 5 files changed, 3992 insertions(+) create mode 100644 drivers/media/video/s5k4ecgx.c create mode 100644 drivers/media/video/s5k4ecgx_regs.h create mode 100644 include/media/s5k4ecgx.h -- 1.7.9.5 -- 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
[PATH v3 2/2] v4l: Add v4l2 subdev driver for S5K4ECGX sensor
This driver implements preview mode of the S5K4ECGX sensor.
capture (snapshot) operation, face detection are missing now.
Following controls are supported:
contrast/saturation/brightness/sharpness
Signed-off-by: Sangwook Lee sangwook@linaro.org
---
drivers/media/video/Kconfig|8 +
drivers/media/video/Makefile |1 +
drivers/media/video/s5k4ecgx.c | 839
include/media/s5k4ecgx.h | 39 ++
4 files changed, 887 insertions(+)
create mode 100644 drivers/media/video/s5k4ecgx.c
create mode 100644 include/media/s5k4ecgx.h
diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index c128fac..2c3f434 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -580,6 +580,14 @@ config VIDEO_S5K6AA
This is a V4L2 sensor-level driver for Samsung S5K6AA(FX) 1.3M
camera sensor with an embedded SoC image signal processor.
+config VIDEO_S5K4ECGX
+tristate Samsung S5K4ECGX sensor support
+ depends on MEDIA_CAMERA_SUPPORT
+depends on I2C VIDEO_V4L2 VIDEO_V4L2_SUBDEV_API
+---help---
+ This is a V4L2 sensor-level driver for Samsung S5K4ECGX 5M
+ camera sensor with an embedded SoC image signal processor.
+
source drivers/media/video/smiapp/Kconfig
comment Flash devices
diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
index b7da9fa..ec39c47 100644
--- a/drivers/media/video/Makefile
+++ b/drivers/media/video/Makefile
@@ -80,6 +80,7 @@ obj-$(CONFIG_VIDEO_SR030PC30) += sr030pc30.o
obj-$(CONFIG_VIDEO_NOON010PC30)+= noon010pc30.o
obj-$(CONFIG_VIDEO_M5MOLS) += m5mols/
obj-$(CONFIG_VIDEO_S5K6AA) += s5k6aa.o
+obj-$(CONFIG_VIDEO_S5K4ECGX)+= s5k4ecgx.o
obj-$(CONFIG_VIDEO_SMIAPP) += smiapp/
obj-$(CONFIG_VIDEO_ADP1653)+= adp1653.o
obj-$(CONFIG_VIDEO_AS3645A)+= as3645a.o
diff --git a/drivers/media/video/s5k4ecgx.c b/drivers/media/video/s5k4ecgx.c
new file mode 100644
index 000..cfc90b8
--- /dev/null
+++ b/drivers/media/video/s5k4ecgx.c
@@ -0,0 +1,839 @@
+/*
+ * Driver for s5k4ecgx (5MP Camera) from Samsung
+ * a quarter-inch optical format 1.4 micron 5 megapixel (Mp)
+ * CMOS image sensor.
+ *
+ * Copyright (C) 2012, Linaro, Sangwook Lee sangwook@linaro.org
+ * Copyright (C) 2012, Insignal Co,. Ltd, Homin Lee suap...@insignal.co.kr
+ *
+ * Based on s5k6aa, noon010pc30 driver
+ * Copyright (C) 2011, Samsung Electronics Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include linux/clk.h
+#include linux/delay.h
+#include linux/gpio.h
+#include linux/i2c.h
+#include linux/module.h
+#include linux/regulator/consumer.h
+#include linux/slab.h
+#include linux/vmalloc.h
+
+#include media/media-entity.h
+#include media/s5k4ecgx.h
+#include media/v4l2-ctrls.h
+#include media/v4l2-device.h
+#include media/v4l2-mediabus.h
+#include media/v4l2-subdev.h
+
+#include s5k4ecgx_regs.h
+
+static int debug;
+module_param(debug, int, 0644);
+
+#define S5K4ECGX_DRIVER_NAME s5k4ecgx
+
+/* Firmware revision information */
+#define REG_FW_REVISION0x71a6
+#define REG_FW_VERSION 0x71a4
+#define S5K4ECGX_REVISION_1_1 0x11
+#define S5K4ECGX_FW_VERSION0x4ec0
+
+/* General purpose parameters */
+#define REG_USER_BRIGHTNESS0x722c /* Brigthness */
+#define REG_USER_CONTRAST 0x722e /* Contrast */
+#define REG_USER_SATURATION0x7230 /* Saturation */
+
+#define REG_USER_SHARP10x7a28
+#define REG_USER_SHARP20x7ade
+#define REG_USER_SHARP30x7b94
+#define REG_USER_SHARP40x7c4a
+#define REG_USER_SHARP50x7d00
+
+/* Reduce sharpness range for user space API */
+#define SHARPNESS_DIV 8208
+
+#define TOK_TERM 0x
+
+/*
+ * FIMXE: This is copied from s5k6aa, because of no information
+ * in s5k4ecgx's datasheet
+ * H/W register Interface (0xd000 - 0xdfff)
+ */
+#define AHB_MSB_ADDR_PTR 0xfcfc
+#define GEN_REG_OFFSH 0xd000
+#define REG_CMDWR_ADDRH0x0028
+#define REG_CMDWR_ADDRL0x002a
+#define REG_CMDRD_ADDRH0x002c
+#define REG_CMDRD_ADDRL0x002e
+#define REG_CMDBUF0_ADDR 0x0f12
+
+/*
+ * Preview size lists supported by sensor
+ */
+static const struct regval_list *prev_regs[] = {
+ s5k4ecgx_176_preview,
+ s5k4ecgx_352_preview,
+ s5k4ecgx_640_preview,
+ s5k4ecgx_720_preview,
+};
+
+struct s5k4ecgx_frmsize {
+ u32 idx; /* Should
Re: [PATCH v2 1/2] v4l: Add factory register values form S5K4ECGX sensor
Opps, the previous email has a HTML part, so resending. Hi Sylwester Thank for the review. On 19 July 2012 20:40, Sylwester Nawrocki sylvester.nawro...@gmail.com wrote: Hi Sangwook, On 07/19/2012 02:14 PM, Sangwook Lee wrote: Add factory default settings for S5K4ECGX sensor registers. I copied them from the reference code of Samsung S.LSI. I'm pretty sure we can do better than that. I've started S5K6AAFX sensor driver development with similar set of write-only register address/value arrays, that stored mainly register default values after the device reset, or were configuring presets that were never used. If you lok at the s5k6aa driver, you'll find only one relatively small array of register values for the analog processing block settings. It's true that I had to reverse engineer a couple of things, but I also had a relatively good datasheet for the sensor. Yes, I already saw analog settings in s5k6aa. Compared to s5k6aa, I couldn't also understand why the sensor has lots of initial values. Is it because s5k4ecgx is slightly more complicated than s5k6aa ? According to comments from the reference code, they do not recommend any changes of these settings. Yes, but it doesn't mean cannot convert, at least part of, those ugly tables into function calls. Yes, the biggest table seems to be one time for boot-up, at least I need to remove one more macro (token) Have you tried to contact Samsung S.LSI for a datasheet that would contain better registers' description ? As you might know, there is a limitation for me to get those information. :-) Instead, if I look into the source code of Google Nexus S which uses s5k4ecgx, https://android.googlesource.com/kernel/samsung.git I can discover that both Google and Samsung are using the same huge table just for initial settings from the sensor booting-up. I added the original author of this sensor driver. Hopes he might add some comments :-) Thanks Sangwook -- Thanks, Sylwester -- 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 v2 2/2] v4l: Add v4l2 subdev driver for S5K4ECGX sensor
Hi Sylwester
Thank you for the great review!
On 19 July 2012 22:40, Sylwester Nawrocki sylvester.nawro...@gmail.com wrote:
Hi Sangwook,
A few review comments for you below...
On 07/19/2012 02:14 PM, Sangwook Lee wrote:
This dirver implements preview mode of the S5K4ECGX sensor.
dirver - driver
OK, I will fix this.
capture (snapshot) operation, face detection are missing now.
Following controls are supported:
contrast/saturation/birghtness/sharpness
birghtness - brightness
ditto
Signed-off-by: Sangwook Leesangwook@linaro.org
---
+ * Driver for s5k4ecgx (5MP Camera) from SAMSUNG
+ * a quarter-inch optical format 1.4 micron 5 megapixel (Mp)
+ * CMOS image sensor, as reffering to s5k6aa.c
I think this should be mentioned after your own copyright notice,
e.g. in form of:
Based on s5k6aa driver,
Copyright (C) 2011, Samsung Electronics Co., Ltd.
ditto
+ *
+ * Copyright (C) 2012, Linaro, Sangwook Leesangwook@linaro.org
+ * Copyright (C) 2012, Insignal Co,. Ltd, Homin
Leesuap...@insignal.co.kr
+ * Copyright (C) 2011, SAMSUNG ELECTRONICS
No need to shout, Samsung Electronics Co., Ltd. would be just enough.
ditto
+#includemedia/v4l2-device.h
+#includemedia/v4l2-subdev.h
+#includemedia/media-entity.h
+#includemedia/v4l2-ctrls.h
+#includemedia/v4l2-mediabus.h
+#includemedia/s5k4ecgx.h
Can we, please, have these sorted alphabetically ?
OK, I will fix this.
+#include s5k4ecgx_regs.h
+
+static int debug;
+module_param(debug, int, 0644);
+
[snip]
+/* General purpose parameters */
+#define REG_USER_BRIGHTNESS 0x722C /* Brigthness */
availble - available
ditto
+#define REG_USER_SHARP1 0x7A28
+#define REG_USER_SHARP2 0x7ADE
+#define REG_USER_SHARP3 0x7B94
+#define REG_USER_SHARP4 0x7C4A
+#define REG_USER_SHARP5 0x7D00
+
+#define LSB(X) (((X) 0xFF))
+#define MSB(Y) (((Y) 8) 0xFF)
Lower case for hex numbers is preferred.
ditto
+
+/*
+ * Preview size lists supported by sensor
+ */
+struct regval_list *pview_size[] = {
+ s5k4ecgx_176_preview,
+ s5k4ecgx_352_preview,
+ s5k4ecgx_640_preview,
+ s5k4ecgx_720_preview,
+};
+
+struct s5k4ecgx_framesize {
+ u32 idx; /* Should indicate index of pview_size */
+ u32 width;
+ u32 height;
+};
+
+/*
+ * TODO: currently only preview is supported and snapshopt(capture)
+ * is not implemented yet
+ */
+static struct s5k4ecgx_framesize p_sets[] = {
p_sets - s5k4ecgx_framesizes ?
Hmm, the structure name needs to be changed properly.
+ {0, 176, 144},
+ {1, 352, 288},
+ {2, 640, 480},
+ {3, 720, 480},
I believe we can do without presets for just the preview operation mode.
This (p_sets[]) is only used to configure preview size dynamically
when _s_fmt is called and then s_stream become on.
Then, the number of registers to configure when device is powered on
should then decrease significantly.
Those presets are meant to speed up switching the device context, e.g.
from preview to capture, but they just slow down initialization, because
you have to configure all presets beforehand.
+};
+
+#define S5K4ECGX_NUM_PREV ARRAY_SIZE(p_sets)
+struct s5k4ecgx_pixfmt {
+ enum v4l2_mbus_pixelcode code;
+ u32 colorspace;
+};
+
+/* By defaut value, output from sensor will be YUV422 0-255 */
+static const struct s5k4ecgx_pixfmt s5k4ecgx_formats[] = {
+ { V4L2_MBUS_FMT_YUYV8_2X8, V4L2_COLORSPACE_JPEG},
Nit: missing whitespace before }.
Thanks, I will fix this.
+};
+
+struct s5k4ecgx_preset {
+ /* output pixel format and resolution */
+ struct v4l2_mbus_framefmt mbus_fmt;
+ u8 clk_id;
+ u8 index;
+};
+
+struct s5k4ecgx {
+ struct v4l2_subdev sd;
+ struct media_pad pad;
+ struct v4l2_ctrl_handler handler;
+
+ struct s5k4ecgx_platform_data *pdata;
+ struct s5k4ecgx_preset presets[S5K4ECGX_MAX_PRESETS];
+ struct s5k4ecgx_preset *preset;
+ struct s5k4ecgx_framesize *p_now; /* Current frame size */
+ struct v4l2_fract timeperframe;
+
+ /* protects the struct members below */
+ struct mutex lock;
+ int streaming;
+
+ /* Token for I2C burst write */
+ enum token_type reg_type;
+ u16 reg_addr_high;
+ u16 reg_addr_low;
+
+ /* Platform specific field */
+ int (*set_power)(int);
This need to be replaced with regulator/GPIO API, if possible.
Platform data callbacks cannot be supported on device tree platforms,
hence we really need to avoid such callbacks.
Thanks for reminding me about device tree.
let me update the driver with regulator/GPIO API.
+ int mdelay;
+};
+
+static inline struct s5k4ecgx *to_s5k4ecgx(struct
[PATCH v2 0/2] Add v4l2 subdev driver for S5K4ECGX sensor with embedded SoC ISP
The following 2 patches add driver for S5K4ECGX sensor with embedded ISP SoC, and minor v4l2 control API enhancement. S5K4ECGX is 5M CMOS Image sensor from Samsung. Changes since v1: - fixed s_stream(0) when it called twice - changed mutex_X position to be used when strictly necessary - add additional s_power(0) in case that error happens - update more accurate debugging statements - remove dummy else Sangwook Lee (2): v4l: Add factory register values form S5K4ECGX sensor v4l: Add v4l2 subdev driver for S5K4ECGX sensor drivers/media/video/Kconfig |7 + drivers/media/video/Makefile|1 + drivers/media/video/s5k4ecgx.c | 881 ++ drivers/media/video/s5k4ecgx_regs.h | 3121 +++ include/media/s5k4ecgx.h| 29 + 5 files changed, 4039 insertions(+) create mode 100644 drivers/media/video/s5k4ecgx.c create mode 100644 drivers/media/video/s5k4ecgx_regs.h create mode 100644 include/media/s5k4ecgx.h -- 1.7.9.5 -- 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 v2 2/2] v4l: Add v4l2 subdev driver for S5K4ECGX sensor
This dirver implements preview mode of the S5K4ECGX sensor.
capture (snapshot) operation, face detection are missing now.
Following controls are supported:
contrast/saturation/birghtness/sharpness
Signed-off-by: Sangwook Lee sangwook@linaro.org
---
drivers/media/video/Kconfig|7 +
drivers/media/video/Makefile |1 +
drivers/media/video/s5k4ecgx.c | 881
include/media/s5k4ecgx.h | 29 ++
4 files changed, 918 insertions(+)
create mode 100644 drivers/media/video/s5k4ecgx.c
create mode 100644 include/media/s5k4ecgx.h
diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index 99937c9..45d7f99 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -559,6 +559,13 @@ config VIDEO_S5K6AA
This is a V4L2 sensor-level driver for Samsung S5K6AA(FX) 1.3M
camera sensor with an embedded SoC image signal processor.
+config VIDEO_S5K4ECGX
+tristate Samsung S5K4ECGX sensor support
+depends on I2C VIDEO_V4L2 VIDEO_V4L2_SUBDEV_API
+---help---
+ This is a V4L2 sensor-level driver for Samsung S5K4ECGX 5M
+ camera sensor with an embedded SoC image signal processor.
+
source drivers/media/video/smiapp/Kconfig
comment Flash devices
diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
index d209de0..605bf35 100644
--- a/drivers/media/video/Makefile
+++ b/drivers/media/video/Makefile
@@ -79,6 +79,7 @@ obj-$(CONFIG_VIDEO_SR030PC30) += sr030pc30.o
obj-$(CONFIG_VIDEO_NOON010PC30)+= noon010pc30.o
obj-$(CONFIG_VIDEO_M5MOLS) += m5mols/
obj-$(CONFIG_VIDEO_S5K6AA) += s5k6aa.o
+obj-$(CONFIG_VIDEO_S5K4ECGX)+= s5k4ecgx.o
obj-$(CONFIG_VIDEO_SMIAPP) += smiapp/
obj-$(CONFIG_VIDEO_ADP1653)+= adp1653.o
obj-$(CONFIG_VIDEO_AS3645A)+= as3645a.o
diff --git a/drivers/media/video/s5k4ecgx.c b/drivers/media/video/s5k4ecgx.c
new file mode 100644
index 000..e277d71
--- /dev/null
+++ b/drivers/media/video/s5k4ecgx.c
@@ -0,0 +1,881 @@
+/*
+ * Driver for s5k4ecgx (5MP Camera) from SAMSUNG
+ * a quarter-inch optical format 1.4 micron 5 megapixel (Mp)
+ * CMOS image sensor, as reffering to s5k6aa.c
+ *
+ * Copyright (C) 2012, Linaro, Sangwook Lee sangwook@linaro.org
+ * Copyright (C) 2012, Insignal Co,. Ltd, Homin Lee suap...@insignal.co.kr
+ * Copyright (C) 2011, SAMSUNG ELECTRONICS
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include linux/module.h
+#include linux/i2c.h
+#include linux/delay.h
+#include linux/vmalloc.h
+#include linux/slab.h
+
+#include media/v4l2-device.h
+#include media/v4l2-subdev.h
+#include media/media-entity.h
+#include media/v4l2-ctrls.h
+#include media/v4l2-mediabus.h
+#include media/s5k4ecgx.h
+
+#include s5k4ecgx_regs.h
+
+static int debug;
+module_param(debug, int, 0644);
+
+#define S5K4ECGX_DRIVER_NAME s5k4ecgx
+
+/* Basic windows sizes */
+#define S5K4ECGX_OUT_WIDTH_DEF 640
+#define S5K4ECGX_OUT_HEIGHT_DEF480
+#define S5K4ECGX_WIN_WIDTH_MAX 1024
+#define S5K4ECGX_WIN_HEIGHT_MAX600
+#define S5K4ECGX_WIN_WIDTH_MIN 176
+#define S5K4ECGX_WIN_HEIGHT_MIN144
+
+/* Firmware revision information */
+#define S5K4ECGX_REVISION_1_1 0x11
+#define S5K4ECGX_FW_VERSION0x4EC0
+#define REG_FW_VERSION 0x71A4
+#define REG_FW_REVISION0x71A6
+
+/* For now we use only one user configuration register set */
+#define S5K4ECGX_MAX_PRESETS 1
+
+/* Review this depending on system */
+#define S5K4ECGX_POLL_TIME 1 /* ms */
+
+/* General purpose parameters */
+#define REG_USER_BRIGHTNESS0x722C /* Brigthness */
+#define REG_USER_CONTRAST 0x722E /* Contrast */
+#define REG_USER_SATURATION0x7230 /* Saturation */
+
+/* FIXME: No information availble about these register from the datasheet */
+#define REG_USER_SHARP10x7A28
+#define REG_USER_SHARP20x7ADE
+#define REG_USER_SHARP30x7B94
+#define REG_USER_SHARP40x7C4A
+#define REG_USER_SHARP50x7D00
+
+#define LSB(X) (((X) 0xFF))
+#define MSB(Y) (((Y) 8) 0xFF)
+
+/*
+ * Preview size lists supported by sensor
+ */
+struct regval_list *pview_size[] = {
+ s5k4ecgx_176_preview,
+ s5k4ecgx_352_preview,
+ s5k4ecgx_640_preview,
+ s5k4ecgx_720_preview,
+};
+
+struct s5k4ecgx_framesize {
+ u32 idx; /* Should indicate index of pview_size */
+ u32 width;
+ u32 height;
+};
+
+/*
+ * TODO: currently only preview is supported and snapshopt(capture
[PATCH 0/2] Add v4l2 subdev driver for S5K4ECGX sensor with embedded SoC ISP
The following 2 patches add driver for S5K4ECGX sensor with embedded ISP SoC, and minor v4l2 control API enhancement. S5K4ECGX is 5M CMOS Image sensor from Samsung. Currenlty ony preview mode is supported. (no capture mode/face detection) Sangwook Lee (2): v4l: Add factory register values form S5K4ECGX sensor v4l: Add v4l2 subdev driver for S5K4ECGX sensor drivers/media/video/Kconfig |7 + drivers/media/video/Makefile|1 + drivers/media/video/s5k4ecgx.c | 871 ++ drivers/media/video/s5k4ecgx_regs.h | 3121 +++ include/media/s5k4ecgx.h| 29 + 5 files changed, 4029 insertions(+) create mode 100644 drivers/media/video/s5k4ecgx.c create mode 100644 drivers/media/video/s5k4ecgx_regs.h create mode 100644 include/media/s5k4ecgx.h -- 1.7.9.5 -- 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 2/2] v4l: Add v4l2 subdev driver for S5K4ECGX sensor
This dirver implements preview mode of the S5K4ECGX sensor.
capture (snapshot) operation, face detection are missing now.
Following controls are supported:
contrast/saturation/birghtness/sharpness
Signed-off-by: Sangwook Lee sangwook@linaro.org
---
drivers/media/video/Kconfig|7 +
drivers/media/video/Makefile |1 +
drivers/media/video/s5k4ecgx.c | 871
include/media/s5k4ecgx.h | 29 ++
4 files changed, 908 insertions(+)
create mode 100644 drivers/media/video/s5k4ecgx.c
create mode 100644 include/media/s5k4ecgx.h
diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index 99937c9..45d7f99 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -559,6 +559,13 @@ config VIDEO_S5K6AA
This is a V4L2 sensor-level driver for Samsung S5K6AA(FX) 1.3M
camera sensor with an embedded SoC image signal processor.
+config VIDEO_S5K4ECGX
+tristate Samsung S5K4ECGX sensor support
+depends on I2C VIDEO_V4L2 VIDEO_V4L2_SUBDEV_API
+---help---
+ This is a V4L2 sensor-level driver for Samsung S5K4ECGX 5M
+ camera sensor with an embedded SoC image signal processor.
+
source drivers/media/video/smiapp/Kconfig
comment Flash devices
diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
index d209de0..605bf35 100644
--- a/drivers/media/video/Makefile
+++ b/drivers/media/video/Makefile
@@ -79,6 +79,7 @@ obj-$(CONFIG_VIDEO_SR030PC30) += sr030pc30.o
obj-$(CONFIG_VIDEO_NOON010PC30)+= noon010pc30.o
obj-$(CONFIG_VIDEO_M5MOLS) += m5mols/
obj-$(CONFIG_VIDEO_S5K6AA) += s5k6aa.o
+obj-$(CONFIG_VIDEO_S5K4ECGX)+= s5k4ecgx.o
obj-$(CONFIG_VIDEO_SMIAPP) += smiapp/
obj-$(CONFIG_VIDEO_ADP1653)+= adp1653.o
obj-$(CONFIG_VIDEO_AS3645A)+= as3645a.o
diff --git a/drivers/media/video/s5k4ecgx.c b/drivers/media/video/s5k4ecgx.c
new file mode 100644
index 000..68a1977
--- /dev/null
+++ b/drivers/media/video/s5k4ecgx.c
@@ -0,0 +1,871 @@
+/*
+ * Driver for s5k4ecgx (5MP Camera) from SAMSUNG
+ * a quarter-inch optical format 1.4 micron 5 megapixel (Mp)
+ * CMOS image sensor, as reffering to s5k6aa.c
+ *
+ * Copyright (C) 2012, Linaro, Sangwook Lee sangwook@linaro.org
+ * Copyright (C) 2012, Insignal Co,. Ltd, Homin Lee suap...@insignal.co.kr
+ * Copyright (C) 2011, SAMSUNG ELECTRONICS
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include linux/module.h
+#include linux/i2c.h
+#include linux/delay.h
+#include linux/vmalloc.h
+#include linux/slab.h
+
+#include media/v4l2-device.h
+#include media/v4l2-subdev.h
+#include media/media-entity.h
+#include media/v4l2-ctrls.h
+#include media/v4l2-mediabus.h
+#include media/s5k4ecgx.h
+
+#include s5k4ecgx_regs.h
+
+static int debug;
+module_param(debug, int, 0644);
+
+#define S5K4ECGX_DRIVER_NAME s5k4ecgx
+
+/* Basic windows sizes */
+#define S5K4ECGX_OUT_WIDTH_DEF 640
+#define S5K4ECGX_OUT_HEIGHT_DEF480
+#define S5K4ECGX_WIN_WIDTH_MAX 1024
+#define S5K4ECGX_WIN_HEIGHT_MAX600
+#define S5K4ECGX_WIN_WIDTH_MIN 176
+#define S5K4ECGX_WIN_HEIGHT_MIN144
+
+/* Firmware revision information */
+#define S5K4ECGX_REVISION_1_1 0x11
+#define S5K4ECGX_FW_VERSION0x4EC0
+#define REG_FW_VERSION 0x71A4
+#define REG_FW_REVISION0x71A6
+
+/* For now we use only one user configuration register set */
+#define S5K4ECGX_MAX_PRESETS 1
+
+/* Review this depending on system */
+#define S5K4ECGX_POLL_TIME 1 /* ms */
+
+/* General purpose parameters */
+#define REG_USER_BRIGHTNESS0x722C /* Brigthness */
+#define REG_USER_CONTRAST 0x722E /* Contrast */
+#define REG_USER_SATURATION0x7230 /* Saturation */
+
+/* FIXME: No information availble about these register from the datasheet */
+#define REG_USER_SHARP10x7A28
+#define REG_USER_SHARP20x7ADE
+#define REG_USER_SHARP30x7B94
+#define REG_USER_SHARP40x7C4A
+#define REG_USER_SHARP50x7D00
+
+#define LSB(X) (((X) 0xFF))
+#define MSB(Y) (((Y) 8) 0xFF)
+
+/*
+ * Preview size lists supported by sensor
+ */
+struct regval_list *pview_size[] = {
+ s5k4ecgx_176_preview,
+ s5k4ecgx_352_preview,
+ s5k4ecgx_640_preview,
+ s5k4ecgx_720_preview,
+};
+
+struct s5k4ecgx_framesize {
+ u32 idx; /* Should indicate index of pview_size */
+ u32 width;
+ u32 height;
+};
+
+/*
+ * TODO: currently only preview is supported and snapshopt(capture
