Re: [RFC v3 13/13] V4L: Add driver for s5k4e5 image sensor

2013-08-03 Thread Sylwester Nawrocki

On 08/02/2013 05:02 PM, Arun Kumar K wrote:

This patch adds subdev driver for Samsung S5K4E5 raw image sensor.
Like s5k6a3, it is also another fimc-is firmware controlled
sensor. This minimal sensor driver doesn't do any I2C communications
as its done by ISP firmware. It can be updated if needed to a
regular sensor driver by adding the I2C communication.

Signed-off-by: Arun Kumar Karun...@samsung.com
---
  drivers/media/i2c/Kconfig  |8 +
  drivers/media/i2c/Makefile |1 +
  drivers/media/i2c/s5k4e5.c |  362 


Hmm, we also need the device tree binding documentation file.


  3 files changed, 371 insertions(+)
  create mode 100644 drivers/media/i2c/s5k4e5.c

[...]

new file mode 100644
index 000..a713c6a
--- /dev/null
+++ b/drivers/media/i2c/s5k4e5.c
@@ -0,0 +1,362 @@
+/*
+ * Samsung S5K4E5 image sensor driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Arun Kumar Karun...@samsung.com
+ *
+ * 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.
+ */
+
+#includelinux/clk.h
+#includelinux/delay.h
+#includelinux/device.h
+#includelinux/errno.h
+#includelinux/gpio.h
+#includelinux/i2c.h
+#includelinux/kernel.h
+#includelinux/module.h
+#includelinux/of_gpio.h
+#includelinux/pm_runtime.h
+#includelinux/regulator/consumer.h
+#includelinux/slab.h
+#includelinux/videodev2.h
+#includemedia/v4l2-async.h
+#includemedia/v4l2-subdev.h
+
+#define S5K4E5_SENSOR_MAX_WIDTH2560
+#define S5K4E5_SENSOR_MAX_HEIGHT   1920
+#define S5K4E5_SENSOR_MIN_WIDTH32
+#define S5K4E5_SENSOR_MIN_HEIGHT   32
+
+#define S5K4E5_WIDTH_PADDING   16
+#define S5K4E5_HEIGHT_PADDING  10
+
+#define S5K4E5_DEF_PIX_WIDTH   1296
+#define S5K4E5_DEF_PIX_HEIGHT  732


Please see my comments to patch 12/13 WRT to these defines.


+#define S5K4E5_DRV_NAMES5K4E5
+#define S5K4E5_CLK_NAMEmclk
+
+#define S5K4E5_NUM_SUPPLIES2
+
+/**
+ * struct s5k4e5 - fimc-is sensor data structure


s/fimc-is/s5k4e5 ?


+ * @dev: pointer to this I2C client device structure
+ * @subdev: the image sensor's v4l2 subdev
+ * @pad: subdev media source pad
+ * @supplies: image sensor's voltage regulator supplies
+ * @gpio_reset: GPIO connected to the sensor's reset pin
+ * @lock: mutex protecting the structure's members below
+ * @format: media bus format at the sensor's source pad
+ */
+struct s5k4e5 {
+   struct device *dev;
+   struct v4l2_subdev subdev;
+   struct media_pad pad;
+   struct regulator_bulk_data supplies[S5K4E5_NUM_SUPPLIES];
+   int gpio_reset;
+   struct mutex lock;
+   struct v4l2_mbus_framefmt format;
+   struct clk *clock;
+   u32 clock_frequency;
+};
+
+static const char * const s5k4e5_supply_names[] = {
+   svdda,
+   svddio
+};

[...]

+static int s5k4e5_probe(struct i2c_client *client,
+   const struct i2c_device_id *id)
+{
+   struct device *dev =client-dev;
+   struct s5k4e5 *sensor;
+   struct v4l2_subdev *sd;
+   int gpio, i, ret;
+
+   sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
+   if (!sensor)
+   return -ENOMEM;
+
+   mutex_init(sensor-lock);
+   sensor-gpio_reset = -EINVAL;
+   sensor-clock = ERR_PTR(-EINVAL);
+   sensor-dev = dev;
+
+   gpio = of_get_gpio_flags(dev-of_node, 0, NULL);
+   if (gpio_is_valid(gpio)) {
+   ret = devm_gpio_request_one(dev, gpio, GPIOF_OUT_INIT_LOW,
+   S5K4E5_DRV_NAME);
+   if (ret  0)
+   return ret;
+   }
+   sensor-gpio_reset = gpio;
+
+   if (of_property_read_u32(dev-of_node, clock-frequency,
+   sensor-clock_frequency)) {
+   dev_err(dev, clock-frequency property not found at %s\n,
+   dev-of_node-full_name);
+   return -EINVAL;


I would make clock-frequency property optional and instead of returning
error use some default frequency value when this property is not specified
in DT. This also need to be put into the DT binding document.


+   }
+
+   for (i = 0; i  S5K4E5_NUM_SUPPLIES; i++)
+   sensor-supplies[i].supply = s5k4e5_supply_names[i];
+
+   ret = devm_regulator_bulk_get(client-dev, S5K4E5_NUM_SUPPLIES,
+ sensor-supplies);
+   if (ret  0)
+   return ret;
+
+   /* Defer probing if the clock is not available yet */
+   sensor-clock = clk_get(dev, S5K4E5_CLK_NAME);
+   if (IS_ERR(sensor-clock))
+   return -EPROBE_DEFER;
+
+   sd =sensor-subdev;
+   v4l2_i2c_subdev_init(sd, client,s5k4e5_subdev_ops);
+   

[RFC v3 13/13] V4L: Add driver for s5k4e5 image sensor

2013-08-02 Thread Arun Kumar K
This patch adds subdev driver for Samsung S5K4E5 raw image sensor.
Like s5k6a3, it is also another fimc-is firmware controlled
sensor. This minimal sensor driver doesn't do any I2C communications
as its done by ISP firmware. It can be updated if needed to a
regular sensor driver by adding the I2C communication.

Signed-off-by: Arun Kumar K arun...@samsung.com
---
 drivers/media/i2c/Kconfig  |8 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/s5k4e5.c |  362 
 3 files changed, 371 insertions(+)
 create mode 100644 drivers/media/i2c/s5k4e5.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index f7e9147..271028b 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -572,6 +572,14 @@ config VIDEO_S5K6A3
  This is a V4L2 sensor-level driver for Samsung S5K6A3 raw
  camera sensor.
 
+config VIDEO_S5K4E5
+   tristate Samsung S5K4E5 sensor support
+   depends on MEDIA_CAMERA_SUPPORT
+   depends on I2C  VIDEO_V4L2  VIDEO_V4L2_SUBDEV_API  OF
+   ---help---
+ This is a V4L2 sensor-level driver for Samsung S5K4E5 raw
+ camera sensor.
+
 config VIDEO_S5K4ECGX
 tristate Samsung S5K4ECGX sensor support
 depends on I2C  VIDEO_V4L2  VIDEO_V4L2_SUBDEV_API
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index cf3cf03..0aeed8e 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -65,6 +65,7 @@ obj-$(CONFIG_VIDEO_SR030PC30) += sr030pc30.o
 obj-$(CONFIG_VIDEO_NOON010PC30)+= noon010pc30.o
 obj-$(CONFIG_VIDEO_S5K6AA) += s5k6aa.o
 obj-$(CONFIG_VIDEO_S5K6A3) += s5k6a3.o
+obj-$(CONFIG_VIDEO_S5K4E5) += s5k4e5.o
 obj-$(CONFIG_VIDEO_S5K4ECGX)   += s5k4ecgx.o
 obj-$(CONFIG_VIDEO_S5C73M3)+= s5c73m3/
 obj-$(CONFIG_VIDEO_ADP1653)+= adp1653.o
diff --git a/drivers/media/i2c/s5k4e5.c b/drivers/media/i2c/s5k4e5.c
new file mode 100644
index 000..a713c6a
--- /dev/null
+++ b/drivers/media/i2c/s5k4e5.c
@@ -0,0 +1,362 @@
+/*
+ * Samsung S5K4E5 image sensor driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Arun Kumar K arun...@samsung.com
+ *
+ * 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.
+ */
+
+#include linux/clk.h
+#include linux/delay.h
+#include linux/device.h
+#include linux/errno.h
+#include linux/gpio.h
+#include linux/i2c.h
+#include linux/kernel.h
+#include linux/module.h
+#include linux/of_gpio.h
+#include linux/pm_runtime.h
+#include linux/regulator/consumer.h
+#include linux/slab.h
+#include linux/videodev2.h
+#include media/v4l2-async.h
+#include media/v4l2-subdev.h
+
+#define S5K4E5_SENSOR_MAX_WIDTH2560
+#define S5K4E5_SENSOR_MAX_HEIGHT   1920
+#define S5K4E5_SENSOR_MIN_WIDTH32
+#define S5K4E5_SENSOR_MIN_HEIGHT   32
+
+#define S5K4E5_WIDTH_PADDING   16
+#define S5K4E5_HEIGHT_PADDING  10
+
+#define S5K4E5_DEF_PIX_WIDTH   1296
+#define S5K4E5_DEF_PIX_HEIGHT  732
+
+#define S5K4E5_DRV_NAMES5K4E5
+#define S5K4E5_CLK_NAMEmclk
+
+#define S5K4E5_NUM_SUPPLIES2
+
+/**
+ * struct s5k4e5 - fimc-is sensor data structure
+ * @dev: pointer to this I2C client device structure
+ * @subdev: the image sensor's v4l2 subdev
+ * @pad: subdev media source pad
+ * @supplies: image sensor's voltage regulator supplies
+ * @gpio_reset: GPIO connected to the sensor's reset pin
+ * @lock: mutex protecting the structure's members below
+ * @format: media bus format at the sensor's source pad
+ */
+struct s5k4e5 {
+   struct device *dev;
+   struct v4l2_subdev subdev;
+   struct media_pad pad;
+   struct regulator_bulk_data supplies[S5K4E5_NUM_SUPPLIES];
+   int gpio_reset;
+   struct mutex lock;
+   struct v4l2_mbus_framefmt format;
+   struct clk *clock;
+   u32 clock_frequency;
+};
+
+static const char * const s5k4e5_supply_names[] = {
+   svdda,
+   svddio
+};
+
+static inline struct s5k4e5 *sd_to_s5k4e5(struct v4l2_subdev *sd)
+{
+   return container_of(sd, struct s5k4e5, subdev);
+}
+
+static const struct v4l2_mbus_framefmt s5k4e5_formats[] = {
+   {
+   .code = V4L2_MBUS_FMT_SGRBG10_1X10,
+   .colorspace = V4L2_COLORSPACE_SRGB,
+   .field = V4L2_FIELD_NONE,
+   }
+};
+
+static const struct v4l2_mbus_framefmt *find_sensor_format(
+   struct v4l2_mbus_framefmt *mf)
+{
+   int i;
+
+   for (i = 0; i  ARRAY_SIZE(s5k4e5_formats); i++)
+   if (mf-code == s5k4e5_formats[i].code)
+   return s5k4e5_formats[i];
+
+   return s5k4e5_formats[0];
+}
+
+static int s5k4e5_enum_mbus_code(struct v4l2_subdev *sd,
+ struct v4l2_subdev_fh *fh,
+