* [PATCH 1/2] [media] imx214: Add imx214 camera sensor driver
@ 2018-09-21 8:54 Ricardo Ribalda Delgado
2018-09-21 8:54 ` [PATCH 2/2] [media] imx214: device tree binding Ricardo Ribalda Delgado
2018-09-21 9:28 ` [PATCH 1/2] [media] imx214: Add imx214 camera sensor driver Sakari Ailus
0 siblings, 2 replies; 8+ messages in thread
From: Ricardo Ribalda Delgado @ 2018-09-21 8:54 UTC (permalink / raw)
To: Hans Verkuil, Laurent Pinchart, Sakari Ailus,
Mauro Carvalho Chehab, linux-kernel, linux-media
Cc: Ricardo Ribalda Delgado
Add a V4L2 sub-device driver for the Sony IMX214 image sensor.
This is a camera sensor using the I2C bus for control and the
CSI-2 bus for data.
Tested on a DB820c alike board with Intrinsyc Open-Q 13MP camera.
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
MAINTAINERS | 8 +
drivers/media/i2c/Kconfig | 12 +
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/imx214.c | 994 +++++++++++++++++++++++++++++++++++++
4 files changed, 1015 insertions(+)
create mode 100644 drivers/media/i2c/imx214.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 9989925f658d..2ae68894e700 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13521,6 +13521,14 @@ S: Maintained
F: drivers/ssb/
F: include/linux/ssb/
+SONY IMX214 SENSOR DRIVER
+M: Ricardo Ribalda <ricardo.ribalda@gmail.com>
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/i2c/imx214.c
+F: Documentation/devicetree/bindings/media/i2c/imx214.txt
+
SONY IMX258 SENSOR DRIVER
M: Sakari Ailus <sakari.ailus@linux.intel.com>
L: linux-media@vger.kernel.org
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index bfdb494686bf..d9416b3a090c 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -595,6 +595,18 @@ config VIDEO_APTINA_PLL
config VIDEO_SMIAPP_PLL
tristate
+config VIDEO_IMX214
+ tristate "Sony IMX214 sensor support"
+ depends on GPIOLIB && I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+ depends on OF
+ depends on MEDIA_CAMERA_SUPPORT
+ help
+ This is a Video4Linux2 sensor driver for the Sony
+ IMX214 camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called imx214.
+
config VIDEO_IMX258
tristate "Sony IMX258 sensor support"
depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index a94eb03d10d4..61305edc6165 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -106,6 +106,7 @@ obj-$(CONFIG_VIDEO_I2C) += video-i2c.o
obj-$(CONFIG_VIDEO_ML86V7667) += ml86v7667.o
obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
obj-$(CONFIG_VIDEO_TC358743) += tc358743.o
+obj-$(CONFIG_VIDEO_IMX214) += imx214.o
obj-$(CONFIG_VIDEO_IMX258) += imx258.o
obj-$(CONFIG_VIDEO_IMX274) += imx274.o
diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c
new file mode 100644
index 000000000000..3a425818036c
--- /dev/null
+++ b/drivers/media/i2c/imx214.c
@@ -0,0 +1,994 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * imx214.c - imx214 sensor driver
+ *
+ * Copyright 2018 Qtechnology A/S
+ *
+ * Ricardo Ribalda <ricardo.ribalda@gmail.com>
+ */
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/clk.h>
+#include <linux/regulator/consumer.h>
+#include <linux/gpio/consumer.h>
+#include <linux/delay.h>
+#include <linux/regmap.h>
+#include <media/media-entity.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-subdev.h>
+#include <media/v4l2-ctrls.h>
+
+#define IMX214_VOLTAGE_AVDD 2700000
+#define IMX214_VOLTAGE_DVDD 1120000
+#define IMX214_VOLTAGE_DOVDD 1800000
+
+#define IMX214_DEFAULT_CLK_FREQ 24000000
+#define IMX214_DEFAULT_PIXEL_RATE 384000000
+#define IMX214_WIDTH 4096
+#define IMX214_HEIGHT 2304
+#define IMX214_FPS 30
+#define IMX214_MBUS_CODE MEDIA_BUS_FMT_SRGGB10_1X10
+
+struct imx214 {
+ struct i2c_client *i2c_client;
+ struct device *dev;
+ struct clk *xclk;
+ struct regmap *regmap;
+
+ struct v4l2_fwnode_endpoint ep;
+ struct v4l2_subdev sd;
+ struct media_pad pad;
+ struct v4l2_mbus_framefmt fmt;
+ struct v4l2_rect crop;
+
+ struct v4l2_ctrl_handler ctrls;
+ struct v4l2_ctrl *pixel_rate;
+ struct v4l2_ctrl *link_freq;
+
+ struct regulator *vdda_regulator;
+ struct regulator *vddd_regulator;
+ struct regulator *vdddo_regulator;
+
+ struct gpio_desc *enable_gpio;
+
+ bool power_on;
+};
+
+struct reg_8 {
+ u16 addr;
+ u8 val;
+};
+
+static struct reg_8 mode_1920x1080[];
+static struct reg_8 mode_4096x2304[];
+
+/*
+ * Declare modes in order, from biggest
+ * to smallest height.
+ */
+struct imx214_mode {
+ u32 width;
+ u32 height;
+ struct reg_8 *reg_table;
+} imx214_modes[] = {
+ {
+ .width = 4096,
+ .height = 2304,
+ .reg_table = mode_4096x2304,
+ },
+ {
+ .width = 1920,
+ .height = 1080,
+ .reg_table = mode_1920x1080,
+ },
+};
+
+static inline struct imx214 *to_imx214(struct v4l2_subdev *sd)
+{
+ return container_of(sd, struct imx214, sd);
+}
+
+static int imx214_set_power_on(struct imx214 *imx214)
+{
+ int ret;
+
+ gpiod_set_value_cansleep(imx214->enable_gpio, 0);
+ usleep_range(10, 20);
+
+ ret = regulator_enable(imx214->vdddo_regulator);
+ if (ret) {
+ dev_err(imx214->dev, "Failed to enable vdddo\n");
+ return ret;
+ }
+
+ ret = regulator_enable(imx214->vdda_regulator);
+ if (ret < 0) {
+ dev_err(imx214->dev, "Failed to enable vdda\n");
+ return ret;
+ }
+
+ ret = regulator_enable(imx214->vddd_regulator);
+ if (ret) {
+ dev_err(imx214->dev, "Failed to enable vddd\n");
+ return ret;
+ }
+
+ usleep_range(2000, 3000);
+
+ ret = clk_prepare_enable(imx214->xclk);
+ if (ret < 0) {
+ dev_err(imx214->dev, "clk prepare enable failed\n");
+ return ret;
+ }
+
+
+ gpiod_set_value_cansleep(imx214->enable_gpio, 1);
+ usleep_range(12000, 15000);
+
+ return 0;
+}
+
+static void imx214_set_power_off(struct imx214 *imx214)
+{
+ gpiod_set_value_cansleep(imx214->enable_gpio, 0);
+
+ clk_disable_unprepare(imx214->xclk);
+
+ regulator_disable(imx214->vdda_regulator);
+ regulator_disable(imx214->vddd_regulator);
+ regulator_disable(imx214->vdddo_regulator);
+}
+
+static int imx214_s_power(struct v4l2_subdev *sd, int on)
+{
+ struct imx214 *imx214 = to_imx214(sd);
+ int ret = 0;
+
+ on = !!on;
+
+ if (imx214->power_on == on)
+ return 0;
+
+ if (on)
+ ret = imx214_set_power_on(imx214);
+ else
+ imx214_set_power_off(imx214);
+
+ imx214->power_on = on;
+ return 0;
+}
+
+static int imx214_enum_mbus_code(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_mbus_code_enum *code)
+{
+ if (code->index > 0)
+ return -EINVAL;
+
+ code->code = IMX214_MBUS_CODE;
+
+ return 0;
+}
+
+static int imx214_enum_frame_size(struct v4l2_subdev *subdev,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_frame_size_enum *fse)
+{
+ if (fse->code != IMX214_MBUS_CODE)
+ return -EINVAL;
+
+ if (fse->index >= ARRAY_SIZE(imx214_modes))
+ return -EINVAL;
+
+ fse->min_width = fse->max_width = imx214_modes[fse->index].width;
+ fse->min_height = fse->max_height = imx214_modes[fse->index].height;
+
+ return 0;
+}
+
+#ifdef CONFIG_VIDEO_ADV_DEBUG
+static int imx214_s_register(struct v4l2_subdev *subdev,
+ const struct v4l2_dbg_register *reg)
+{
+ struct imx214 *imx214 = container_of(subdev, struct imx214, sd);
+
+ regmap_write(imx214->regmap, reg->reg, reg->val);
+ return 0;
+}
+
+static int imx214_g_register(struct v4l2_subdev *subdev,
+ struct v4l2_dbg_register *reg)
+{
+ struct imx214 *imx214 = container_of(subdev, struct imx214, sd);
+ unsigned int aux;
+
+ reg->size = 1;
+ regmap_read(imx214->regmap, reg->reg, &aux);
+ reg->val = aux;
+ return 0;
+}
+#endif
+
+static const struct v4l2_subdev_core_ops imx214_core_ops = {
+#ifdef CONFIG_VIDEO_ADV_DEBUG
+ .g_register = imx214_g_register,
+ .s_register = imx214_s_register,
+#endif
+ .s_power = imx214_s_power,
+};
+
+static struct v4l2_mbus_framefmt *
+__imx214_get_pad_format(struct imx214 *imx214,
+ struct v4l2_subdev_pad_config *cfg,
+ unsigned int pad,
+ enum v4l2_subdev_format_whence which)
+{
+ switch (which) {
+ case V4L2_SUBDEV_FORMAT_TRY:
+ return v4l2_subdev_get_try_format(&imx214->sd, cfg, pad);
+ case V4L2_SUBDEV_FORMAT_ACTIVE:
+ return &imx214->fmt;
+ default:
+ return NULL;
+ }
+}
+
+static int imx214_get_format(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_format *format)
+{
+ struct imx214 *imx214 = to_imx214(sd);
+
+ format->format = *__imx214_get_pad_format(imx214, cfg, format->pad,
+ format->which);
+
+ return 0;
+}
+
+static struct v4l2_rect *
+__imx214_get_pad_crop(struct imx214 *imx214, struct v4l2_subdev_pad_config *cfg,
+ unsigned int pad, enum v4l2_subdev_format_whence which)
+{
+ switch (which) {
+ case V4L2_SUBDEV_FORMAT_TRY:
+ return v4l2_subdev_get_try_crop(&imx214->sd, cfg, pad);
+ case V4L2_SUBDEV_FORMAT_ACTIVE:
+ return &imx214->crop;
+ default:
+ return NULL;
+ }
+}
+
+static int imx214_find_mode(u32 height)
+{
+ int i;
+
+ for (i = 0; (i < ARRAY_SIZE(imx214_modes) - 1) ; i++)
+ if (height >= imx214_modes[i].width)
+ return i;
+ return i;
+}
+
+static int imx214_set_format(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_format *format)
+{
+ struct imx214 *imx214 = to_imx214(sd);
+ struct v4l2_mbus_framefmt *__format;
+ struct v4l2_rect *__crop;
+ int mode;
+
+ __crop = __imx214_get_pad_crop(imx214, cfg, format->pad, format->which);
+
+ mode = format ? imx214_find_mode(format->format.height) : 0;
+
+ __crop->width = imx214_modes[mode].width;
+ __crop->height = imx214_modes[mode].height;
+
+ __format = __imx214_get_pad_format(imx214, cfg, format->pad,
+ format->which);
+ __format->width = __crop->width;
+ __format->height = __crop->height;
+ __format->code = IMX214_MBUS_CODE;
+ __format->field = V4L2_FIELD_NONE;
+ __format->colorspace = V4L2_COLORSPACE_SRGB;
+ __format->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(__format->colorspace);
+ __format->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(true,
+ __format->colorspace, __format->ycbcr_enc);
+ __format->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(__format->colorspace);
+
+ format->format = *__format;
+
+ return 0;
+}
+
+static int imx214_get_selection(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_selection *sel)
+{
+ struct imx214 *imx214 = to_imx214(sd);
+
+ if (sel->target != V4L2_SEL_TGT_CROP)
+ return -EINVAL;
+
+ sel->r = *__imx214_get_pad_crop(imx214, cfg, sel->pad,
+ sel->which);
+ return 0;
+}
+
+static int imx214_entity_init_cfg(struct v4l2_subdev *subdev,
+ struct v4l2_subdev_pad_config *cfg)
+{
+ struct v4l2_subdev_format fmt = { };
+
+ fmt.which = cfg ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
+ fmt.format.width = imx214_modes[0].width;
+ fmt.format.height = imx214_modes[0].height;
+
+ imx214_set_format(subdev, cfg, &fmt);
+
+ return 0;
+}
+
+enum { IMX214_TABLE_WAIT_MS = 0,
+ IMX214_TABLE_END,
+ IMX214_MAX_RETRIES,
+ IMX214_WAIT_MS};
+
+/*From imx214_mode_tbls.h*/
+static struct reg_8 mode_4096x2304[] = {
+ {0x0114, 0x03},
+ {0x0220, 0x00},
+ {0x0221, 0x11},
+ {0x0222, 0x01},
+ {0x0340, 0x0C},
+ {0x0341, 0x7A},
+ {0x0342, 0x13},
+ {0x0343, 0x90},
+ {0x0344, 0x00},
+ {0x0345, 0x38},
+ {0x0346, 0x01},
+ {0x0347, 0x98},
+ {0x0348, 0x10},
+ {0x0349, 0x37},
+ {0x034A, 0x0A},
+ {0x034B, 0x97},
+ {0x0381, 0x01},
+ {0x0383, 0x01},
+ {0x0385, 0x01},
+ {0x0387, 0x01},
+ {0x0900, 0x00},
+ {0x0901, 0x00},
+ {0x0902, 0x00},
+ {0x3000, 0x35},
+ {0x3054, 0x01},
+ {0x305C, 0x11},
+
+ {0x0112, 0x0A},
+ {0x0113, 0x0A},
+ {0x034C, 0x10},
+ {0x034D, 0x00},
+ {0x034E, 0x09},
+ {0x034F, 0x00},
+ {0x0401, 0x00},
+ {0x0404, 0x00},
+ {0x0405, 0x10},
+ {0x0408, 0x00},
+ {0x0409, 0x00},
+ {0x040A, 0x00},
+ {0x040B, 0x00},
+ {0x040C, 0x10},
+ {0x040D, 0x00},
+ {0x040E, 0x09},
+ {0x040F, 0x00},
+
+ {0x0301, 0x05},
+ {0x0303, 0x02},
+ {0x0305, 0x03},
+ {0x0306, 0x00},
+ {0x0307, 0x96},
+ {0x0309, 0x0A},
+ {0x030B, 0x01},
+ {0x0310, 0x00},
+
+ {0x0820, 0x12},
+ {0x0821, 0xC0},
+ {0x0822, 0x00},
+ {0x0823, 0x00},
+
+ {0x3A03, 0x09},
+ {0x3A04, 0x50},
+ {0x3A05, 0x01},
+
+ {0x0B06, 0x01},
+ {0x30A2, 0x00},
+
+ {0x30B4, 0x00},
+
+ {0x3A02, 0xFF},
+
+ {0x3011, 0x00},
+ {0x3013, 0x01},
+
+ {0x0202, 0x0C},
+ {0x0203, 0x70},
+ {0x0224, 0x01},
+ {0x0225, 0xF4},
+
+ {0x0204, 0x00},
+ {0x0205, 0x00},
+ {0x020E, 0x01},
+ {0x020F, 0x00},
+ {0x0210, 0x01},
+ {0x0211, 0x00},
+ {0x0212, 0x01},
+ {0x0213, 0x00},
+ {0x0214, 0x01},
+ {0x0215, 0x00},
+ {0x0216, 0x00},
+ {0x0217, 0x00},
+
+ {0x4170, 0x00},
+ {0x4171, 0x10},
+ {0x4176, 0x00},
+ {0x4177, 0x3C},
+ {0xAE20, 0x04},
+ {0xAE21, 0x5C},
+
+ {IMX214_TABLE_WAIT_MS, 10},
+ {0x0138, 0x01},
+ {IMX214_TABLE_END, 0x00}
+};
+
+static struct reg_8 mode_1920x1080[] = {
+ {0x0114, 0x03},
+ {0x0220, 0x00},
+ {0x0221, 0x11},
+ {0x0222, 0x01},
+ {0x0340, 0x0C},
+ {0x0341, 0x7A},
+ {0x0342, 0x13},
+ {0x0343, 0x90},
+ {0x0344, 0x04},
+ {0x0345, 0x78},
+ {0x0346, 0x03},
+ {0x0347, 0xFC},
+ {0x0348, 0x0B},
+ {0x0349, 0xF7},
+ {0x034A, 0x08},
+ {0x034B, 0x33},
+ {0x0381, 0x01},
+ {0x0383, 0x01},
+ {0x0385, 0x01},
+ {0x0387, 0x01},
+ {0x0900, 0x00},
+ {0x0901, 0x00},
+ {0x0902, 0x00},
+ {0x3000, 0x35},
+ {0x3054, 0x01},
+ {0x305C, 0x11},
+
+ {0x0112, 0x0A},
+ {0x0113, 0x0A},
+ {0x034C, 0x07},
+ {0x034D, 0x80},
+ {0x034E, 0x04},
+ {0x034F, 0x38},
+ {0x0401, 0x00},
+ {0x0404, 0x00},
+ {0x0405, 0x10},
+ {0x0408, 0x00},
+ {0x0409, 0x00},
+ {0x040A, 0x00},
+ {0x040B, 0x00},
+ {0x040C, 0x07},
+ {0x040D, 0x80},
+ {0x040E, 0x04},
+ {0x040F, 0x38},
+
+ {0x0301, 0x05},
+ {0x0303, 0x02},
+ {0x0305, 0x03},
+ {0x0306, 0x00},
+ {0x0307, 0x96},
+ {0x0309, 0x0A},
+ {0x030B, 0x01},
+ {0x0310, 0x00},
+
+ {0x0820, 0x12},
+ {0x0821, 0xC0},
+ {0x0822, 0x00},
+ {0x0823, 0x00},
+
+ {0x3A03, 0x04},
+ {0x3A04, 0xF8},
+ {0x3A05, 0x02},
+
+ {0x0B06, 0x01},
+ {0x30A2, 0x00},
+
+ {0x30B4, 0x00},
+
+ {0x3A02, 0xFF},
+
+ {0x3011, 0x00},
+ {0x3013, 0x01},
+
+ {0x0202, 0x0C},
+ {0x0203, 0x70},
+ {0x0224, 0x01},
+ {0x0225, 0xF4},
+
+ {0x0204, 0x00},
+ {0x0205, 0x00},
+ {0x020E, 0x01},
+ {0x020F, 0x00},
+ {0x0210, 0x01},
+ {0x0211, 0x00},
+ {0x0212, 0x01},
+ {0x0213, 0x00},
+ {0x0214, 0x01},
+ {0x0215, 0x00},
+ {0x0216, 0x00},
+ {0x0217, 0x00},
+
+ {0x4170, 0x00},
+ {0x4171, 0x10},
+ {0x4176, 0x00},
+ {0x4177, 0x3C},
+ {0xAE20, 0x04},
+ {0xAE21, 0x5C},
+
+ {IMX214_TABLE_WAIT_MS, 10},
+ {0x0138, 0x01},
+ {IMX214_TABLE_END, 0x00}
+};
+
+static struct reg_8 mode_table_common[] = {
+ /* software reset */
+
+ /* software standby settings */
+ {0x0100, 0x00},
+
+ /* ATR setting */
+ {0x9300, 0x02},
+
+ /* external clock setting */
+ {0x0136, 0x18},
+ {0x0137, 0x00},
+
+ /* global setting */
+ /* basic config */
+ {0x0101, 0x00},
+ {0x0105, 0x01},
+ {0x0106, 0x01},
+ {0x4550, 0x02},
+ {0x4601, 0x00},
+ {0x4642, 0x05},
+ {0x6227, 0x11},
+ {0x6276, 0x00},
+ {0x900E, 0x06},
+ {0xA802, 0x90},
+ {0xA803, 0x11},
+ {0xA804, 0x62},
+ {0xA805, 0x77},
+ {0xA806, 0xAE},
+ {0xA807, 0x34},
+ {0xA808, 0xAE},
+ {0xA809, 0x35},
+ {0xA80A, 0x62},
+ {0xA80B, 0x83},
+ {0xAE33, 0x00},
+
+ /* analog setting */
+ {0x4174, 0x00},
+ {0x4175, 0x11},
+ {0x4612, 0x29},
+ {0x461B, 0x12},
+ {0x461F, 0x06},
+ {0x4635, 0x07},
+ {0x4637, 0x30},
+ {0x463F, 0x18},
+ {0x4641, 0x0D},
+ {0x465B, 0x12},
+ {0x465F, 0x11},
+ {0x4663, 0x11},
+ {0x4667, 0x0F},
+ {0x466F, 0x0F},
+ {0x470E, 0x09},
+ {0x4909, 0xAB},
+ {0x490B, 0x95},
+ {0x4915, 0x5D},
+ {0x4A5F, 0xFF},
+ {0x4A61, 0xFF},
+ {0x4A73, 0x62},
+ {0x4A85, 0x00},
+ {0x4A87, 0xFF},
+
+ /* embedded data */
+ {0x5041, 0x04},
+ {0x583C, 0x04},
+ {0x620E, 0x04},
+ {0x6EB2, 0x01},
+ {0x6EB3, 0x00},
+ {0x9300, 0x02},
+
+ /* imagequality */
+ /* HDR setting */
+ {0x3001, 0x07},
+ {0x6D12, 0x3F},
+ {0x6D13, 0xFF},
+ {0x9344, 0x03},
+ {0x9706, 0x10},
+ {0x9707, 0x03},
+ {0x9708, 0x03},
+ {0x9E04, 0x01},
+ {0x9E05, 0x00},
+ {0x9E0C, 0x01},
+ {0x9E0D, 0x02},
+ {0x9E24, 0x00},
+ {0x9E25, 0x8C},
+ {0x9E26, 0x00},
+ {0x9E27, 0x94},
+ {0x9E28, 0x00},
+ {0x9E29, 0x96},
+
+ /* CNR parameter setting */
+ {0x69DB, 0x01},
+
+ /* Moire reduction */
+ {0x6957, 0x01},
+
+ /* image enhancment */
+ {0x6987, 0x17},
+ {0x698A, 0x03},
+ {0x698B, 0x03},
+
+ /* white balanace */
+ {0x0B8E, 0x01},
+ {0x0B8F, 0x00},
+ {0x0B90, 0x01},
+ {0x0B91, 0x00},
+ {0x0B92, 0x01},
+ {0x0B93, 0x00},
+ {0x0B94, 0x01},
+ {0x0B95, 0x00},
+
+ /* ATR setting */
+ {0x6E50, 0x00},
+ {0x6E51, 0x32},
+ {0x9340, 0x00},
+ {0x9341, 0x3C},
+ {0x9342, 0x03},
+ {0x9343, 0xFF},
+ {IMX214_TABLE_END, 0x00}
+};
+
+#define MAX_CMD 4
+static int imx214_write_table(struct imx214 *imx214,
+ const struct reg_8 table[])
+{
+ u8 vals[MAX_CMD];
+ int ret;
+ int i;
+
+ for (table = table; table->addr != IMX214_TABLE_END ; table++) {
+ if (table->addr == IMX214_TABLE_WAIT_MS) {
+ msleep(table->val);
+ continue;
+ }
+
+ for (i = 0; i < MAX_CMD; i++) {
+ if (table[i].addr != (table[0].addr + i))
+ break;
+ vals[i] = table[i].val;
+ }
+
+ ret = regmap_bulk_write(imx214->regmap, table->addr, vals, i);
+
+ if (ret) {
+ dev_err(imx214->dev, "write_table error: %d\n", ret);
+ return ret;
+ }
+
+ table += i - 1;
+
+ }
+
+ return 0;
+}
+
+
+static int imx214_s_stream(struct v4l2_subdev *subdev, int enable)
+{
+ struct imx214 *imx214 = to_imx214(subdev);
+ int ret;
+
+ if (enable) {
+ int mode = imx214_find_mode(imx214->fmt.height);
+
+ ret = imx214_write_table(imx214, mode_table_common);
+ if (ret < 0) {
+ dev_err(imx214->dev, "could not sent common table %d\n",
+ ret);
+ return ret;
+ }
+ ret = imx214_write_table(imx214, imx214_modes[mode].reg_table);
+ if (ret < 0) {
+ dev_err(imx214->dev, "could not sent mode table %d\n",
+ ret);
+ return ret;
+ }
+ ret = v4l2_ctrl_handler_setup(&imx214->ctrls);
+ if (ret < 0) {
+ dev_err(imx214->dev, "could not sync v4l2 controls\n");
+ return ret;
+ }
+ ret = regmap_write(imx214->regmap, 0x100, 1);
+ if (ret < 0) {
+ dev_err(imx214->dev, "could not sent start table %d\n",
+ ret);
+ return ret;
+ }
+ } else {
+ ret = regmap_write(imx214->regmap, 0x100, 0);
+ if (ret < 0) {
+ dev_err(imx214->dev, "could not sent stop table %d\n",
+ ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static int imx214_g_frame_interval(struct v4l2_subdev *subdev,
+ struct v4l2_subdev_frame_interval *fival)
+{
+ fival->pad = 0;
+ fival->interval.numerator = 1;
+ fival->interval.denominator = IMX214_FPS;
+
+ return 0;
+}
+
+static int imx214_enum_frame_interval(struct v4l2_subdev *subdev,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_frame_interval_enum *fie)
+{
+ int mode;
+
+ if (fie->index != 0)
+ return -EINVAL;
+
+ mode = imx214_find_mode(fie->height);
+
+ fie->code = IMX214_MBUS_CODE;
+ fie->width = imx214_modes[mode].width;
+ fie->height = imx214_modes[mode].height;
+ fie->interval.numerator = 1;
+ fie->interval.denominator = IMX214_MBUS_CODE;
+
+ return 0;
+}
+
+static const struct v4l2_subdev_video_ops imx214_video_ops = {
+ .s_stream = imx214_s_stream,
+ .g_frame_interval = imx214_g_frame_interval,
+ .s_frame_interval = imx214_g_frame_interval,
+};
+
+static const struct v4l2_subdev_pad_ops imx214_subdev_pad_ops = {
+ .enum_mbus_code = imx214_enum_mbus_code,
+ .enum_frame_size = imx214_enum_frame_size,
+ .enum_frame_interval = imx214_enum_frame_interval,
+ .get_fmt = imx214_get_format,
+ .set_fmt = imx214_set_format,
+ .get_selection = imx214_get_selection,
+ .init_cfg = imx214_entity_init_cfg,
+};
+
+static const struct v4l2_subdev_ops imx214_subdev_ops = {
+ .core = &imx214_core_ops,
+ .video = &imx214_video_ops,
+ .pad = &imx214_subdev_pad_ops,
+};
+
+static const struct regmap_config sensor_regmap_config = {
+ .reg_bits = 16,
+ .val_bits = 8,
+ .cache_type = REGCACHE_RBTREE,
+};
+
+static int imx214_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct imx214 *imx214;
+ struct fwnode_handle *endpoint;
+ int ret;
+ static const s64 link_freq[] = {
+ (IMX214_DEFAULT_PIXEL_RATE * 10LL) / 8,
+ };
+
+ imx214 = devm_kzalloc(dev, sizeof(*imx214), GFP_KERNEL);
+ if (!imx214)
+ return -ENOMEM;
+
+ imx214->i2c_client = client;
+ imx214->dev = dev;
+
+ endpoint = fwnode_graph_get_next_endpoint(
+ of_fwnode_handle(client->dev.of_node), NULL);
+ if (!endpoint) {
+ dev_err(dev, "endpoint node not found\n");
+ return -EINVAL;
+ }
+
+ ret = v4l2_fwnode_endpoint_parse(endpoint, &imx214->ep);
+ fwnode_handle_put(endpoint);
+ if (ret < 0) {
+ dev_err(dev, "parsing endpoint node failed\n");
+ return ret;
+ }
+
+ if (imx214->ep.bus_type != V4L2_MBUS_CSI2) {
+ dev_err(dev, "invalid bus type (%u), must be CSI2 (%u)\n",
+ imx214->ep.bus_type, V4L2_MBUS_CSI2);
+ return -EINVAL;
+ }
+
+ imx214->xclk = devm_clk_get(dev, "xclk");
+ if (IS_ERR(imx214->xclk)) {
+ dev_err(dev, "could not get xclk");
+ return PTR_ERR(imx214->xclk);
+ }
+
+ ret = clk_set_rate(imx214->xclk, IMX214_DEFAULT_CLK_FREQ);
+ if (ret) {
+ dev_err(dev, "could not set xclk frequency\n");
+ return ret;
+ }
+
+ imx214->vdda_regulator = devm_regulator_get(dev, "vdda");
+ if (IS_ERR(imx214->vdda_regulator)) {
+ dev_err(dev, "cannot get vdda regulator\n");
+ return PTR_ERR(imx214->vdda_regulator);
+ }
+
+ ret = regulator_set_voltage(imx214->vdda_regulator,
+ IMX214_VOLTAGE_AVDD,
+ IMX214_VOLTAGE_AVDD);
+ if (ret < 0) {
+ dev_err(dev, "cannot set vdda voltage\n");
+ return ret;
+ }
+
+ imx214->vddd_regulator = devm_regulator_get(dev, "vddd");
+ if (IS_ERR(imx214->vddd_regulator)) {
+ dev_err(dev, "cannot get core regulator\n");
+ return PTR_ERR(imx214->vddd_regulator);
+ }
+
+ ret = regulator_set_voltage(imx214->vddd_regulator,
+ IMX214_VOLTAGE_DVDD,
+ IMX214_VOLTAGE_DVDD);
+ if (ret < 0) {
+ dev_err(dev, "cannot set vddd voltage\n");
+ return ret;
+ }
+
+ imx214->vdddo_regulator = devm_regulator_get(dev, "vdddo");
+ if (IS_ERR(imx214->vdddo_regulator)) {
+ dev_err(dev, "cannot get vdddo regulator\n");
+ return PTR_ERR(imx214->vdddo_regulator);
+ }
+
+ ret = regulator_set_voltage(imx214->vdddo_regulator,
+ IMX214_VOLTAGE_DOVDD,
+ IMX214_VOLTAGE_DOVDD);
+ if (ret < 0) {
+ dev_err(dev, "cannot set vdddo voltage\n");
+ return ret;
+ }
+
+ imx214->enable_gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_LOW);
+ if (IS_ERR(imx214->enable_gpio)) {
+ dev_err(dev, "cannot get enable gpio\n");
+ return PTR_ERR(imx214->enable_gpio);
+ }
+
+ imx214->regmap = devm_regmap_init_i2c(client, &sensor_regmap_config);
+ if (IS_ERR(imx214->regmap)) {
+ dev_err(dev, "regmap init failed\n");
+ return PTR_ERR(imx214->regmap);
+ }
+
+ v4l2_ctrl_handler_init(&imx214->ctrls, 2);
+
+ imx214->pixel_rate = v4l2_ctrl_new_std(&imx214->ctrls, NULL,
+ V4L2_CID_PIXEL_RATE, 0,
+ IMX214_DEFAULT_PIXEL_RATE, 1,
+ IMX214_DEFAULT_PIXEL_RATE);
+ imx214->link_freq = v4l2_ctrl_new_int_menu(&imx214->ctrls, NULL,
+ V4L2_CID_LINK_FREQ,
+ ARRAY_SIZE(link_freq) - 1,
+ 0, link_freq);
+ if (imx214->link_freq)
+ imx214->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+ ret = imx214->ctrls.error;
+ if (ret) {
+ dev_err(&client->dev, "%s control init failed (%d)\n",
+ __func__, ret);
+ return ret;
+ }
+
+ imx214->sd.ctrl_handler = &imx214->ctrls;
+ ret = v4l2_ctrl_handler_setup(imx214->sd.ctrl_handler);
+ if (ret) {
+ dev_err(&client->dev,
+ "Error %d setting default controls\n", ret);
+ return ret;
+ }
+
+ v4l2_i2c_subdev_init(&imx214->sd, client, &imx214_subdev_ops);
+ imx214->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+ imx214->pad.flags = MEDIA_PAD_FL_SOURCE;
+ imx214->sd.dev = &client->dev;
+ imx214->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+
+ ret = media_entity_pads_init(&imx214->sd.entity, 1, &imx214->pad);
+ if (ret < 0) {
+ dev_err(dev, "could not register media entity\n");
+ goto free_ctrl;
+ }
+
+ imx214_entity_init_cfg(&imx214->sd, NULL);
+
+ ret = v4l2_async_register_subdev_sensor_common(&imx214->sd);
+ if (ret < 0) {
+ dev_err(dev, "could not register v4l2 device\n");
+ goto free_entity;
+ }
+
+ return 0;
+
+free_entity:
+ media_entity_cleanup(&imx214->sd.entity);
+free_ctrl:
+ v4l2_ctrl_handler_free(&imx214->ctrls);
+
+ return ret;
+}
+
+static int imx214_remove(struct i2c_client *client)
+{
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct imx214 *imx214 = to_imx214(sd);
+
+ v4l2_async_unregister_subdev(&imx214->sd);
+ media_entity_cleanup(&imx214->sd.entity);
+ v4l2_ctrl_handler_free(&imx214->ctrls);
+
+ return 0;
+}
+
+static const struct of_device_id imx214_of_match[] = {
+ { .compatible = "sony,imx214" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, imx214_of_match);
+
+static struct i2c_driver imx214_i2c_driver = {
+ .driver = {
+ .of_match_table = imx214_of_match,
+ .name = "imx214",
+ },
+ .probe_new = imx214_probe,
+ .remove = imx214_remove,
+};
+
+module_i2c_driver(imx214_i2c_driver);
+
+MODULE_DESCRIPTION("Sony IMX214 Camera drier");
+MODULE_AUTHOR("Ricardo Ribalda <ricardo.ribalda@gmail.com>");
+MODULE_LICENSE("GPL v2");
--
2.18.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] [media] imx214: device tree binding
2018-09-21 8:54 [PATCH 1/2] [media] imx214: Add imx214 camera sensor driver Ricardo Ribalda Delgado
@ 2018-09-21 8:54 ` Ricardo Ribalda Delgado
2018-09-21 9:28 ` [PATCH 1/2] [media] imx214: Add imx214 camera sensor driver Sakari Ailus
1 sibling, 0 replies; 8+ messages in thread
From: Ricardo Ribalda Delgado @ 2018-09-21 8:54 UTC (permalink / raw)
To: Hans Verkuil, Laurent Pinchart, Sakari Ailus,
Mauro Carvalho Chehab, linux-kernel, linux-media
Cc: Ricardo Ribalda Delgado, devicetree
Document bindings for imx214 v4l2 driver.
Cc: devicetree@vger.kernel.org
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
.../devicetree/bindings/media/i2c/imx214.txt | 51 +++++++++++++++++++
1 file changed, 51 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/imx214.txt
diff --git a/Documentation/devicetree/bindings/media/i2c/imx214.txt b/Documentation/devicetree/bindings/media/i2c/imx214.txt
new file mode 100644
index 000000000000..4ff76d96332e
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/imx214.txt
@@ -0,0 +1,51 @@
+* Sony 1/3.06-Inch 13.13Mp CMOS Digital Image Sensor
+
+The Sony imx214 is a 1/3.06-inch CMOS active pixel digital image sensor with
+an active array size of 4224H x 3176V. It is programmable through I2C
+interface. The I2C address can be configured to to 0x1a or 0x10, depending on
+how is wired.
+Image data is sent through MIPI CSI-2, which is configured as 4 lanes
+at 1440 Mbps.
+
+
+Required Properties:
+- compatible: value should be "sony,imx214" for imx214 sensor
+- reg: I2C bus address of the device
+- enable-gpios: Sensor enable GPIO
+- vdddo-supply: Chip digital IO regulator (1.8V).
+- vdda-supply: Chip analog regulator (2.7V).
+- vddd-supply: Chip digital core regulator (1.12V).
+- clocks = Reference to the xclk clock.
+- clock-names = Should be "xclk".
+- clock-frequency = Frequency of the xclk clock. Should be <24000000>;
+
+Optional Properties:
+- flash-leds: See ../video-interfaces.txt
+- lens-focus: See ../video-interfaces.txt
+
+The imx274 device node should contain one 'port' child node with
+an 'endpoint' subnode. For further reading on port node refer to
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example:
+
+ camera_rear@1a {
+ status = "okay";
+ compatible = "sony,imx214";
+ reg = <0x1a>;
+ vdddo-supply = <&pm8994_lvs1>;
+ vddd-supply = <&camera_vddd_1v12>;
+ vdda-supply = <&pm8994_l17>;
+ lens-focus = <&ad5820>;
+ enable-gpios = <&msmgpio 25 GPIO_ACTIVE_HIGH>;
+ clocks = <&mmcc CAMSS_MCLK0_CLK>;
+ clock-names = "xclk";
+ clock-frequency = <24000000>;
+ port {
+ imx214_ep: endpoint {
+ clock-lanes = <1>;
+ data-lanes = <0 2 3 4>;
+ remote-endpoint = <&csiphy0_ep>;
+ };
+ };
+ };
--
2.18.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] [media] imx214: Add imx214 camera sensor driver
2018-09-21 8:54 [PATCH 1/2] [media] imx214: Add imx214 camera sensor driver Ricardo Ribalda Delgado
2018-09-21 8:54 ` [PATCH 2/2] [media] imx214: device tree binding Ricardo Ribalda Delgado
@ 2018-09-21 9:28 ` Sakari Ailus
2018-09-21 10:15 ` Ricardo Ribalda Delgado
1 sibling, 1 reply; 8+ messages in thread
From: Sakari Ailus @ 2018-09-21 9:28 UTC (permalink / raw)
To: Ricardo Ribalda Delgado
Cc: Hans Verkuil, Laurent Pinchart, Mauro Carvalho Chehab,
linux-kernel, linux-media
Hi, Ricardo!
Thanks for the patch.
On Fri, Sep 21, 2018 at 10:54:49AM +0200, Ricardo Ribalda Delgado wrote:
> Add a V4L2 sub-device driver for the Sony IMX214 image sensor.
> This is a camera sensor using the I2C bus for control and the
> CSI-2 bus for data.
>
> Tested on a DB820c alike board with Intrinsyc Open-Q 13MP camera.
>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
> MAINTAINERS | 8 +
> drivers/media/i2c/Kconfig | 12 +
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/imx214.c | 994 +++++++++++++++++++++++++++++++++++++
> 4 files changed, 1015 insertions(+)
> create mode 100644 drivers/media/i2c/imx214.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9989925f658d..2ae68894e700 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13521,6 +13521,14 @@ S: Maintained
> F: drivers/ssb/
> F: include/linux/ssb/
>
> +SONY IMX214 SENSOR DRIVER
> +M: Ricardo Ribalda <ricardo.ribalda@gmail.com>
> +L: linux-media@vger.kernel.org
> +T: git git://linuxtv.org/media_tree.git
> +S: Maintained
> +F: drivers/media/i2c/imx214.c
> +F: Documentation/devicetree/bindings/media/i2c/imx214.txt
DT bindings come before the driver; please swap the order for v2.
> +
> SONY IMX258 SENSOR DRIVER
> M: Sakari Ailus <sakari.ailus@linux.intel.com>
> L: linux-media@vger.kernel.org
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index bfdb494686bf..d9416b3a090c 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -595,6 +595,18 @@ config VIDEO_APTINA_PLL
> config VIDEO_SMIAPP_PLL
> tristate
>
> +config VIDEO_IMX214
> + tristate "Sony IMX214 sensor support"
> + depends on GPIOLIB && I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> + depends on OF
> + depends on MEDIA_CAMERA_SUPPORT
> + help
> + This is a Video4Linux2 sensor driver for the Sony
> + IMX214 camera.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called imx214.
> +
> config VIDEO_IMX258
> tristate "Sony IMX258 sensor support"
> depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index a94eb03d10d4..61305edc6165 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -106,6 +106,7 @@ obj-$(CONFIG_VIDEO_I2C) += video-i2c.o
> obj-$(CONFIG_VIDEO_ML86V7667) += ml86v7667.o
> obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
> obj-$(CONFIG_VIDEO_TC358743) += tc358743.o
> +obj-$(CONFIG_VIDEO_IMX214) += imx214.o
> obj-$(CONFIG_VIDEO_IMX258) += imx258.o
> obj-$(CONFIG_VIDEO_IMX274) += imx274.o
>
> diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c
> new file mode 100644
> index 000000000000..3a425818036c
> --- /dev/null
> +++ b/drivers/media/i2c/imx214.c
> @@ -0,0 +1,994 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * imx214.c - imx214 sensor driver
> + *
> + * Copyright 2018 Qtechnology A/S
> + *
> + * Ricardo Ribalda <ricardo.ribalda@gmail.com>
> + */
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/clk.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/delay.h>
> +#include <linux/regmap.h>
> +#include <media/media-entity.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-subdev.h>
> +#include <media/v4l2-ctrls.h>
> +
> +#define IMX214_VOLTAGE_AVDD 2700000
> +#define IMX214_VOLTAGE_DVDD 1120000
> +#define IMX214_VOLTAGE_DOVDD 1800000
While I guess there's nothing wrong technically here (or using
regulator_set_voltage()), the other sensor drivers depend on the voltages
being set in DT rather than in drivers. The reason could be that as range
of voltages is fine for a sensor, another device requiring slightly
different voltage than the nominal voltage of the sensor could be powered
from the same regulator.
> +
> +#define IMX214_DEFAULT_CLK_FREQ 24000000
> +#define IMX214_DEFAULT_PIXEL_RATE 384000000
> +#define IMX214_WIDTH 4096
> +#define IMX214_HEIGHT 2304
> +#define IMX214_FPS 30
> +#define IMX214_MBUS_CODE MEDIA_BUS_FMT_SRGGB10_1X10
> +
> +struct imx214 {
> + struct i2c_client *i2c_client;
> + struct device *dev;
> + struct clk *xclk;
> + struct regmap *regmap;
> +
> + struct v4l2_fwnode_endpoint ep;
> + struct v4l2_subdev sd;
> + struct media_pad pad;
> + struct v4l2_mbus_framefmt fmt;
> + struct v4l2_rect crop;
> +
> + struct v4l2_ctrl_handler ctrls;
> + struct v4l2_ctrl *pixel_rate;
> + struct v4l2_ctrl *link_freq;
> +
> + struct regulator *vdda_regulator;
> + struct regulator *vddd_regulator;
> + struct regulator *vdddo_regulator;
> +
> + struct gpio_desc *enable_gpio;
> +
> + bool power_on;
> +};
> +
> +struct reg_8 {
> + u16 addr;
> + u8 val;
> +};
> +
> +static struct reg_8 mode_1920x1080[];
> +static struct reg_8 mode_4096x2304[];
Const. Could you rearrange the bits to avoid the forward declarations?
> +
> +/*
> + * Declare modes in order, from biggest
> + * to smallest height.
> + */
> +struct imx214_mode {
> + u32 width;
> + u32 height;
> + struct reg_8 *reg_table;
> +} imx214_modes[] = {
This should be static const.
> + {
> + .width = 4096,
> + .height = 2304,
> + .reg_table = mode_4096x2304,
> + },
> + {
> + .width = 1920,
> + .height = 1080,
> + .reg_table = mode_1920x1080,
> + },
> +};
> +
> +static inline struct imx214 *to_imx214(struct v4l2_subdev *sd)
> +{
> + return container_of(sd, struct imx214, sd);
> +}
> +
> +static int imx214_set_power_on(struct imx214 *imx214)
Runtime PM support would be nice; you could drop the s_power callback as a
bonus.
> +{
> + int ret;
> +
> + gpiod_set_value_cansleep(imx214->enable_gpio, 0);
> + usleep_range(10, 20);
> +
> + ret = regulator_enable(imx214->vdddo_regulator);
Would regulator_bulk_enable() be usable here?
> + if (ret) {
> + dev_err(imx214->dev, "Failed to enable vdddo\n");
> + return ret;
> + }
> +
> + ret = regulator_enable(imx214->vdda_regulator);
> + if (ret < 0) {
> + dev_err(imx214->dev, "Failed to enable vdda\n");
> + return ret;
> + }
> +
> + ret = regulator_enable(imx214->vddd_regulator);
> + if (ret) {
> + dev_err(imx214->dev, "Failed to enable vddd\n");
> + return ret;
> + }
> +
> + usleep_range(2000, 3000);
> +
> + ret = clk_prepare_enable(imx214->xclk);
> + if (ret < 0) {
> + dev_err(imx214->dev, "clk prepare enable failed\n");
> + return ret;
> + }
> +
> +
> + gpiod_set_value_cansleep(imx214->enable_gpio, 1);
> + usleep_range(12000, 15000);
> +
> + return 0;
> +}
> +
> +static void imx214_set_power_off(struct imx214 *imx214)
> +{
> + gpiod_set_value_cansleep(imx214->enable_gpio, 0);
> +
> + clk_disable_unprepare(imx214->xclk);
> +
> + regulator_disable(imx214->vdda_regulator);
> + regulator_disable(imx214->vddd_regulator);
> + regulator_disable(imx214->vdddo_regulator);
regulator_bulk_disable()?
> +}
> +
> +static int imx214_s_power(struct v4l2_subdev *sd, int on)
> +{
> + struct imx214 *imx214 = to_imx214(sd);
> + int ret = 0;
> +
> + on = !!on;
> +
> + if (imx214->power_on == on)
> + return 0;
> +
> + if (on)
> + ret = imx214_set_power_on(imx214);
> + else
> + imx214_set_power_off(imx214);
> +
> + imx214->power_on = on;
> + return 0;
> +}
> +
> +static int imx214_enum_mbus_code(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_mbus_code_enum *code)
> +{
> + if (code->index > 0)
> + return -EINVAL;
> +
> + code->code = IMX214_MBUS_CODE;
> +
> + return 0;
> +}
> +
> +static int imx214_enum_frame_size(struct v4l2_subdev *subdev,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_frame_size_enum *fse)
> +{
> + if (fse->code != IMX214_MBUS_CODE)
> + return -EINVAL;
> +
> + if (fse->index >= ARRAY_SIZE(imx214_modes))
> + return -EINVAL;
> +
> + fse->min_width = fse->max_width = imx214_modes[fse->index].width;
> + fse->min_height = fse->max_height = imx214_modes[fse->index].height;
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_VIDEO_ADV_DEBUG
> +static int imx214_s_register(struct v4l2_subdev *subdev,
> + const struct v4l2_dbg_register *reg)
> +{
> + struct imx214 *imx214 = container_of(subdev, struct imx214, sd);
> +
> + regmap_write(imx214->regmap, reg->reg, reg->val);
> + return 0;
return regmap_write() ?
> +}
> +
> +static int imx214_g_register(struct v4l2_subdev *subdev,
> + struct v4l2_dbg_register *reg)
> +{
> + struct imx214 *imx214 = container_of(subdev, struct imx214, sd);
> + unsigned int aux;
> +
> + reg->size = 1;
> + regmap_read(imx214->regmap, reg->reg, &aux);
> + reg->val = aux;
> + return 0;
> +}
> +#endif
> +
> +static const struct v4l2_subdev_core_ops imx214_core_ops = {
> +#ifdef CONFIG_VIDEO_ADV_DEBUG
> + .g_register = imx214_g_register,
> + .s_register = imx214_s_register,
> +#endif
> + .s_power = imx214_s_power,
> +};
> +
> +static struct v4l2_mbus_framefmt *
> +__imx214_get_pad_format(struct imx214 *imx214,
> + struct v4l2_subdev_pad_config *cfg,
> + unsigned int pad,
> + enum v4l2_subdev_format_whence which)
> +{
> + switch (which) {
> + case V4L2_SUBDEV_FORMAT_TRY:
> + return v4l2_subdev_get_try_format(&imx214->sd, cfg, pad);
> + case V4L2_SUBDEV_FORMAT_ACTIVE:
> + return &imx214->fmt;
> + default:
> + return NULL;
> + }
> +}
> +
> +static int imx214_get_format(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_format *format)
> +{
> + struct imx214 *imx214 = to_imx214(sd);
> +
> + format->format = *__imx214_get_pad_format(imx214, cfg, format->pad,
> + format->which);
> +
> + return 0;
> +}
> +
> +static struct v4l2_rect *
> +__imx214_get_pad_crop(struct imx214 *imx214, struct v4l2_subdev_pad_config *cfg,
> + unsigned int pad, enum v4l2_subdev_format_whence which)
> +{
> + switch (which) {
> + case V4L2_SUBDEV_FORMAT_TRY:
> + return v4l2_subdev_get_try_crop(&imx214->sd, cfg, pad);
> + case V4L2_SUBDEV_FORMAT_ACTIVE:
> + return &imx214->crop;
> + default:
> + return NULL;
> + }
> +}
> +
> +static int imx214_find_mode(u32 height)
> +{
> + int i;
> +
> + for (i = 0; (i < ARRAY_SIZE(imx214_modes) - 1) ; i++)
> + if (height >= imx214_modes[i].width)
> + return i;
> + return i;
> +}
> +
> +static int imx214_set_format(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_format *format)
> +{
> + struct imx214 *imx214 = to_imx214(sd);
> + struct v4l2_mbus_framefmt *__format;
> + struct v4l2_rect *__crop;
> + int mode;
> +
> + __crop = __imx214_get_pad_crop(imx214, cfg, format->pad, format->which);
> +
> + mode = format ? imx214_find_mode(format->format.height) : 0;
> +
> + __crop->width = imx214_modes[mode].width;
> + __crop->height = imx214_modes[mode].height;
> +
> + __format = __imx214_get_pad_format(imx214, cfg, format->pad,
> + format->which);
> + __format->width = __crop->width;
> + __format->height = __crop->height;
> + __format->code = IMX214_MBUS_CODE;
> + __format->field = V4L2_FIELD_NONE;
> + __format->colorspace = V4L2_COLORSPACE_SRGB;
> + __format->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(__format->colorspace);
> + __format->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(true,
> + __format->colorspace, __format->ycbcr_enc);
> + __format->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(__format->colorspace);
> +
> + format->format = *__format;
> +
> + return 0;
> +}
> +
> +static int imx214_get_selection(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_selection *sel)
> +{
> + struct imx214 *imx214 = to_imx214(sd);
> +
> + if (sel->target != V4L2_SEL_TGT_CROP)
> + return -EINVAL;
> +
> + sel->r = *__imx214_get_pad_crop(imx214, cfg, sel->pad,
> + sel->which);
> + return 0;
> +}
> +
> +static int imx214_entity_init_cfg(struct v4l2_subdev *subdev,
> + struct v4l2_subdev_pad_config *cfg)
> +{
> + struct v4l2_subdev_format fmt = { };
> +
> + fmt.which = cfg ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
> + fmt.format.width = imx214_modes[0].width;
> + fmt.format.height = imx214_modes[0].height;
> +
> + imx214_set_format(subdev, cfg, &fmt);
> +
> + return 0;
> +}
> +
> +enum { IMX214_TABLE_WAIT_MS = 0,
> + IMX214_TABLE_END,
> + IMX214_MAX_RETRIES,
> + IMX214_WAIT_MS};
> +
> +/*From imx214_mode_tbls.h*/
> +static struct reg_8 mode_4096x2304[] = {
const, please.
> + {0x0114, 0x03},
> + {0x0220, 0x00},
> + {0x0221, 0x11},
> + {0x0222, 0x01},
> + {0x0340, 0x0C},
> + {0x0341, 0x7A},
> + {0x0342, 0x13},
> + {0x0343, 0x90},
> + {0x0344, 0x00},
> + {0x0345, 0x38},
> + {0x0346, 0x01},
> + {0x0347, 0x98},
> + {0x0348, 0x10},
> + {0x0349, 0x37},
> + {0x034A, 0x0A},
> + {0x034B, 0x97},
> + {0x0381, 0x01},
> + {0x0383, 0x01},
> + {0x0385, 0x01},
> + {0x0387, 0x01},
> + {0x0900, 0x00},
> + {0x0901, 0x00},
> + {0x0902, 0x00},
> + {0x3000, 0x35},
> + {0x3054, 0x01},
> + {0x305C, 0x11},
> +
> + {0x0112, 0x0A},
> + {0x0113, 0x0A},
> + {0x034C, 0x10},
> + {0x034D, 0x00},
> + {0x034E, 0x09},
> + {0x034F, 0x00},
> + {0x0401, 0x00},
> + {0x0404, 0x00},
> + {0x0405, 0x10},
> + {0x0408, 0x00},
> + {0x0409, 0x00},
> + {0x040A, 0x00},
> + {0x040B, 0x00},
> + {0x040C, 0x10},
> + {0x040D, 0x00},
> + {0x040E, 0x09},
> + {0x040F, 0x00},
> +
> + {0x0301, 0x05},
> + {0x0303, 0x02},
> + {0x0305, 0x03},
> + {0x0306, 0x00},
> + {0x0307, 0x96},
> + {0x0309, 0x0A},
> + {0x030B, 0x01},
> + {0x0310, 0x00},
> +
> + {0x0820, 0x12},
> + {0x0821, 0xC0},
> + {0x0822, 0x00},
> + {0x0823, 0x00},
> +
> + {0x3A03, 0x09},
> + {0x3A04, 0x50},
> + {0x3A05, 0x01},
> +
> + {0x0B06, 0x01},
> + {0x30A2, 0x00},
> +
> + {0x30B4, 0x00},
> +
> + {0x3A02, 0xFF},
> +
> + {0x3011, 0x00},
> + {0x3013, 0x01},
> +
> + {0x0202, 0x0C},
> + {0x0203, 0x70},
> + {0x0224, 0x01},
> + {0x0225, 0xF4},
> +
> + {0x0204, 0x00},
> + {0x0205, 0x00},
> + {0x020E, 0x01},
> + {0x020F, 0x00},
> + {0x0210, 0x01},
> + {0x0211, 0x00},
> + {0x0212, 0x01},
> + {0x0213, 0x00},
> + {0x0214, 0x01},
> + {0x0215, 0x00},
> + {0x0216, 0x00},
> + {0x0217, 0x00},
> +
> + {0x4170, 0x00},
> + {0x4171, 0x10},
> + {0x4176, 0x00},
> + {0x4177, 0x3C},
> + {0xAE20, 0x04},
> + {0xAE21, 0x5C},
> +
> + {IMX214_TABLE_WAIT_MS, 10},
> + {0x0138, 0x01},
> + {IMX214_TABLE_END, 0x00}
> +};
> +
> +static struct reg_8 mode_1920x1080[] = {
Ditto. Same below.
> + {0x0114, 0x03},
> + {0x0220, 0x00},
> + {0x0221, 0x11},
> + {0x0222, 0x01},
> + {0x0340, 0x0C},
> + {0x0341, 0x7A},
> + {0x0342, 0x13},
> + {0x0343, 0x90},
> + {0x0344, 0x04},
> + {0x0345, 0x78},
> + {0x0346, 0x03},
> + {0x0347, 0xFC},
> + {0x0348, 0x0B},
> + {0x0349, 0xF7},
> + {0x034A, 0x08},
> + {0x034B, 0x33},
> + {0x0381, 0x01},
> + {0x0383, 0x01},
> + {0x0385, 0x01},
> + {0x0387, 0x01},
> + {0x0900, 0x00},
> + {0x0901, 0x00},
> + {0x0902, 0x00},
> + {0x3000, 0x35},
> + {0x3054, 0x01},
> + {0x305C, 0x11},
> +
> + {0x0112, 0x0A},
> + {0x0113, 0x0A},
> + {0x034C, 0x07},
> + {0x034D, 0x80},
> + {0x034E, 0x04},
> + {0x034F, 0x38},
> + {0x0401, 0x00},
> + {0x0404, 0x00},
> + {0x0405, 0x10},
> + {0x0408, 0x00},
> + {0x0409, 0x00},
> + {0x040A, 0x00},
> + {0x040B, 0x00},
> + {0x040C, 0x07},
> + {0x040D, 0x80},
> + {0x040E, 0x04},
> + {0x040F, 0x38},
> +
> + {0x0301, 0x05},
> + {0x0303, 0x02},
> + {0x0305, 0x03},
> + {0x0306, 0x00},
> + {0x0307, 0x96},
> + {0x0309, 0x0A},
> + {0x030B, 0x01},
> + {0x0310, 0x00},
> +
> + {0x0820, 0x12},
> + {0x0821, 0xC0},
> + {0x0822, 0x00},
> + {0x0823, 0x00},
> +
> + {0x3A03, 0x04},
> + {0x3A04, 0xF8},
> + {0x3A05, 0x02},
> +
> + {0x0B06, 0x01},
> + {0x30A2, 0x00},
> +
> + {0x30B4, 0x00},
> +
> + {0x3A02, 0xFF},
> +
> + {0x3011, 0x00},
> + {0x3013, 0x01},
> +
> + {0x0202, 0x0C},
> + {0x0203, 0x70},
> + {0x0224, 0x01},
> + {0x0225, 0xF4},
> +
> + {0x0204, 0x00},
> + {0x0205, 0x00},
> + {0x020E, 0x01},
> + {0x020F, 0x00},
> + {0x0210, 0x01},
> + {0x0211, 0x00},
> + {0x0212, 0x01},
> + {0x0213, 0x00},
> + {0x0214, 0x01},
> + {0x0215, 0x00},
> + {0x0216, 0x00},
> + {0x0217, 0x00},
> +
> + {0x4170, 0x00},
> + {0x4171, 0x10},
> + {0x4176, 0x00},
> + {0x4177, 0x3C},
> + {0xAE20, 0x04},
> + {0xAE21, 0x5C},
> +
> + {IMX214_TABLE_WAIT_MS, 10},
> + {0x0138, 0x01},
> + {IMX214_TABLE_END, 0x00}
> +};
> +
> +static struct reg_8 mode_table_common[] = {
> + /* software reset */
> +
> + /* software standby settings */
> + {0x0100, 0x00},
> +
> + /* ATR setting */
> + {0x9300, 0x02},
> +
> + /* external clock setting */
> + {0x0136, 0x18},
> + {0x0137, 0x00},
> +
> + /* global setting */
> + /* basic config */
> + {0x0101, 0x00},
> + {0x0105, 0x01},
> + {0x0106, 0x01},
> + {0x4550, 0x02},
> + {0x4601, 0x00},
> + {0x4642, 0x05},
> + {0x6227, 0x11},
> + {0x6276, 0x00},
> + {0x900E, 0x06},
> + {0xA802, 0x90},
> + {0xA803, 0x11},
> + {0xA804, 0x62},
> + {0xA805, 0x77},
> + {0xA806, 0xAE},
> + {0xA807, 0x34},
> + {0xA808, 0xAE},
> + {0xA809, 0x35},
> + {0xA80A, 0x62},
> + {0xA80B, 0x83},
> + {0xAE33, 0x00},
> +
> + /* analog setting */
> + {0x4174, 0x00},
> + {0x4175, 0x11},
> + {0x4612, 0x29},
> + {0x461B, 0x12},
> + {0x461F, 0x06},
> + {0x4635, 0x07},
> + {0x4637, 0x30},
> + {0x463F, 0x18},
> + {0x4641, 0x0D},
> + {0x465B, 0x12},
> + {0x465F, 0x11},
> + {0x4663, 0x11},
> + {0x4667, 0x0F},
> + {0x466F, 0x0F},
> + {0x470E, 0x09},
> + {0x4909, 0xAB},
> + {0x490B, 0x95},
> + {0x4915, 0x5D},
> + {0x4A5F, 0xFF},
> + {0x4A61, 0xFF},
> + {0x4A73, 0x62},
> + {0x4A85, 0x00},
> + {0x4A87, 0xFF},
> +
> + /* embedded data */
> + {0x5041, 0x04},
> + {0x583C, 0x04},
> + {0x620E, 0x04},
> + {0x6EB2, 0x01},
> + {0x6EB3, 0x00},
> + {0x9300, 0x02},
> +
> + /* imagequality */
> + /* HDR setting */
> + {0x3001, 0x07},
> + {0x6D12, 0x3F},
> + {0x6D13, 0xFF},
> + {0x9344, 0x03},
> + {0x9706, 0x10},
> + {0x9707, 0x03},
> + {0x9708, 0x03},
> + {0x9E04, 0x01},
> + {0x9E05, 0x00},
> + {0x9E0C, 0x01},
> + {0x9E0D, 0x02},
> + {0x9E24, 0x00},
> + {0x9E25, 0x8C},
> + {0x9E26, 0x00},
> + {0x9E27, 0x94},
> + {0x9E28, 0x00},
> + {0x9E29, 0x96},
> +
> + /* CNR parameter setting */
> + {0x69DB, 0x01},
> +
> + /* Moire reduction */
> + {0x6957, 0x01},
> +
> + /* image enhancment */
> + {0x6987, 0x17},
> + {0x698A, 0x03},
> + {0x698B, 0x03},
> +
> + /* white balanace */
> + {0x0B8E, 0x01},
> + {0x0B8F, 0x00},
> + {0x0B90, 0x01},
> + {0x0B91, 0x00},
> + {0x0B92, 0x01},
> + {0x0B93, 0x00},
> + {0x0B94, 0x01},
> + {0x0B95, 0x00},
> +
> + /* ATR setting */
> + {0x6E50, 0x00},
> + {0x6E51, 0x32},
> + {0x9340, 0x00},
> + {0x9341, 0x3C},
> + {0x9342, 0x03},
> + {0x9343, 0xFF},
> + {IMX214_TABLE_END, 0x00}
> +};
> +
> +#define MAX_CMD 4
> +static int imx214_write_table(struct imx214 *imx214,
> + const struct reg_8 table[])
> +{
> + u8 vals[MAX_CMD];
> + int ret;
> + int i;
> +
> + for (table = table; table->addr != IMX214_TABLE_END ; table++) {
> + if (table->addr == IMX214_TABLE_WAIT_MS) {
> + msleep(table->val);
> + continue;
> + }
> +
> + for (i = 0; i < MAX_CMD; i++) {
> + if (table[i].addr != (table[0].addr + i))
> + break;
> + vals[i] = table[i].val;
> + }
> +
> + ret = regmap_bulk_write(imx214->regmap, table->addr, vals, i);
> +
> + if (ret) {
> + dev_err(imx214->dev, "write_table error: %d\n", ret);
> + return ret;
> + }
> +
> + table += i - 1;
> +
> + }
> +
> + return 0;
> +}
> +
> +
> +static int imx214_s_stream(struct v4l2_subdev *subdev, int enable)
> +{
> + struct imx214 *imx214 = to_imx214(subdev);
> + int ret;
> +
> + if (enable) {
> + int mode = imx214_find_mode(imx214->fmt.height);
> +
> + ret = imx214_write_table(imx214, mode_table_common);
> + if (ret < 0) {
> + dev_err(imx214->dev, "could not sent common table %d\n",
> + ret);
> + return ret;
> + }
> + ret = imx214_write_table(imx214, imx214_modes[mode].reg_table);
> + if (ret < 0) {
> + dev_err(imx214->dev, "could not sent mode table %d\n",
> + ret);
> + return ret;
> + }
> + ret = v4l2_ctrl_handler_setup(&imx214->ctrls);
> + if (ret < 0) {
> + dev_err(imx214->dev, "could not sync v4l2 controls\n");
> + return ret;
> + }
> + ret = regmap_write(imx214->regmap, 0x100, 1);
> + if (ret < 0) {
> + dev_err(imx214->dev, "could not sent start table %d\n",
> + ret);
> + return ret;
> + }
> + } else {
> + ret = regmap_write(imx214->regmap, 0x100, 0);
> + if (ret < 0) {
> + dev_err(imx214->dev, "could not sent stop table %d\n",
> + ret);
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int imx214_g_frame_interval(struct v4l2_subdev *subdev,
> + struct v4l2_subdev_frame_interval *fival)
> +{
> + fival->pad = 0;
> + fival->interval.numerator = 1;
> + fival->interval.denominator = IMX214_FPS;
> +
> + return 0;
> +}
> +
> +static int imx214_enum_frame_interval(struct v4l2_subdev *subdev,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_frame_interval_enum *fie)
> +{
> + int mode;
> +
> + if (fie->index != 0)
> + return -EINVAL;
> +
> + mode = imx214_find_mode(fie->height);
> +
> + fie->code = IMX214_MBUS_CODE;
> + fie->width = imx214_modes[mode].width;
> + fie->height = imx214_modes[mode].height;
> + fie->interval.numerator = 1;
> + fie->interval.denominator = IMX214_MBUS_CODE;
> +
> + return 0;
> +}
> +
> +static const struct v4l2_subdev_video_ops imx214_video_ops = {
> + .s_stream = imx214_s_stream,
> + .g_frame_interval = imx214_g_frame_interval,
> + .s_frame_interval = imx214_g_frame_interval,
> +};
> +
> +static const struct v4l2_subdev_pad_ops imx214_subdev_pad_ops = {
> + .enum_mbus_code = imx214_enum_mbus_code,
> + .enum_frame_size = imx214_enum_frame_size,
> + .enum_frame_interval = imx214_enum_frame_interval,
> + .get_fmt = imx214_get_format,
> + .set_fmt = imx214_set_format,
> + .get_selection = imx214_get_selection,
> + .init_cfg = imx214_entity_init_cfg,
> +};
> +
> +static const struct v4l2_subdev_ops imx214_subdev_ops = {
> + .core = &imx214_core_ops,
> + .video = &imx214_video_ops,
> + .pad = &imx214_subdev_pad_ops,
> +};
> +
> +static const struct regmap_config sensor_regmap_config = {
> + .reg_bits = 16,
> + .val_bits = 8,
> + .cache_type = REGCACHE_RBTREE,
> +};
> +
> +static int imx214_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct imx214 *imx214;
> + struct fwnode_handle *endpoint;
> + int ret;
> + static const s64 link_freq[] = {
> + (IMX214_DEFAULT_PIXEL_RATE * 10LL) / 8,
You should check the link frequency matches with that from the firmware.
> + };
> +
> + imx214 = devm_kzalloc(dev, sizeof(*imx214), GFP_KERNEL);
> + if (!imx214)
> + return -ENOMEM;
> +
> + imx214->i2c_client = client;
Please drop i2c_client field and use v4l2_get_subdev_data(&imx214->sd)
instead.
> + imx214->dev = dev;
> +
> + endpoint = fwnode_graph_get_next_endpoint(
> + of_fwnode_handle(client->dev.of_node), NULL);
> + if (!endpoint) {
> + dev_err(dev, "endpoint node not found\n");
> + return -EINVAL;
> + }
> +
> + ret = v4l2_fwnode_endpoint_parse(endpoint, &imx214->ep);
> + fwnode_handle_put(endpoint);
> + if (ret < 0) {
> + dev_err(dev, "parsing endpoint node failed\n");
> + return ret;
> + }
> +
> + if (imx214->ep.bus_type != V4L2_MBUS_CSI2) {
> + dev_err(dev, "invalid bus type (%u), must be CSI2 (%u)\n",
> + imx214->ep.bus_type, V4L2_MBUS_CSI2);
> + return -EINVAL;
> + }
> +
> + imx214->xclk = devm_clk_get(dev, "xclk");
> + if (IS_ERR(imx214->xclk)) {
> + dev_err(dev, "could not get xclk");
> + return PTR_ERR(imx214->xclk);
> + }
> +
> + ret = clk_set_rate(imx214->xclk, IMX214_DEFAULT_CLK_FREQ);
> + if (ret) {
> + dev_err(dev, "could not set xclk frequency\n");
> + return ret;
> + }
> +
> + imx214->vdda_regulator = devm_regulator_get(dev, "vdda");
> + if (IS_ERR(imx214->vdda_regulator)) {
> + dev_err(dev, "cannot get vdda regulator\n");
> + return PTR_ERR(imx214->vdda_regulator);
> + }
> +
> + ret = regulator_set_voltage(imx214->vdda_regulator,
> + IMX214_VOLTAGE_AVDD,
> + IMX214_VOLTAGE_AVDD);
> + if (ret < 0) {
> + dev_err(dev, "cannot set vdda voltage\n");
> + return ret;
> + }
> +
> + imx214->vddd_regulator = devm_regulator_get(dev, "vddd");
> + if (IS_ERR(imx214->vddd_regulator)) {
> + dev_err(dev, "cannot get core regulator\n");
> + return PTR_ERR(imx214->vddd_regulator);
> + }
> +
> + ret = regulator_set_voltage(imx214->vddd_regulator,
> + IMX214_VOLTAGE_DVDD,
> + IMX214_VOLTAGE_DVDD);
> + if (ret < 0) {
> + dev_err(dev, "cannot set vddd voltage\n");
> + return ret;
> + }
> +
> + imx214->vdddo_regulator = devm_regulator_get(dev, "vdddo");
> + if (IS_ERR(imx214->vdddo_regulator)) {
> + dev_err(dev, "cannot get vdddo regulator\n");
> + return PTR_ERR(imx214->vdddo_regulator);
> + }
> +
> + ret = regulator_set_voltage(imx214->vdddo_regulator,
> + IMX214_VOLTAGE_DOVDD,
> + IMX214_VOLTAGE_DOVDD);
> + if (ret < 0) {
> + dev_err(dev, "cannot set vdddo voltage\n");
> + return ret;
> + }
> +
> + imx214->enable_gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_LOW);
> + if (IS_ERR(imx214->enable_gpio)) {
> + dev_err(dev, "cannot get enable gpio\n");
> + return PTR_ERR(imx214->enable_gpio);
> + }
> +
> + imx214->regmap = devm_regmap_init_i2c(client, &sensor_regmap_config);
> + if (IS_ERR(imx214->regmap)) {
> + dev_err(dev, "regmap init failed\n");
> + return PTR_ERR(imx214->regmap);
> + }
> +
> + v4l2_ctrl_handler_init(&imx214->ctrls, 2);
> +
> + imx214->pixel_rate = v4l2_ctrl_new_std(&imx214->ctrls, NULL,
> + V4L2_CID_PIXEL_RATE, 0,
> + IMX214_DEFAULT_PIXEL_RATE, 1,
> + IMX214_DEFAULT_PIXEL_RATE);
> + imx214->link_freq = v4l2_ctrl_new_int_menu(&imx214->ctrls, NULL,
> + V4L2_CID_LINK_FREQ,
> + ARRAY_SIZE(link_freq) - 1,
> + 0, link_freq);
Do I understand this correctly that the driver does not support setting
e.g. exposure time or gain? Those are very basic features...
You'll also need to ensure the s_ctrl() callback works without s_power()
being called. My suggestion is to switch to PM runtime; see e.g. the ov1385
driver in the current media tree master.
> + if (imx214->link_freq)
> + imx214->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> + ret = imx214->ctrls.error;
> + if (ret) {
> + dev_err(&client->dev, "%s control init failed (%d)\n",
> + __func__, ret);
goto free_ctrl;
> + return ret;
> + }
> +
> + imx214->sd.ctrl_handler = &imx214->ctrls;
> + ret = v4l2_ctrl_handler_setup(imx214->sd.ctrl_handler);
> + if (ret) {
> + dev_err(&client->dev,
> + "Error %d setting default controls\n", ret);
Ditto.
> + return ret;
> + }
> +
> + v4l2_i2c_subdev_init(&imx214->sd, client, &imx214_subdev_ops);
> + imx214->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> + imx214->pad.flags = MEDIA_PAD_FL_SOURCE;
> + imx214->sd.dev = &client->dev;
> + imx214->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> +
> + ret = media_entity_pads_init(&imx214->sd.entity, 1, &imx214->pad);
> + if (ret < 0) {
> + dev_err(dev, "could not register media entity\n");
> + goto free_ctrl;
> + }
> +
> + imx214_entity_init_cfg(&imx214->sd, NULL);
> +
> + ret = v4l2_async_register_subdev_sensor_common(&imx214->sd);
> + if (ret < 0) {
> + dev_err(dev, "could not register v4l2 device\n");
> + goto free_entity;
> + }
> +
> + return 0;
> +
> +free_entity:
> + media_entity_cleanup(&imx214->sd.entity);
> +free_ctrl:
> + v4l2_ctrl_handler_free(&imx214->ctrls);
> +
> + return ret;
> +}
> +
> +static int imx214_remove(struct i2c_client *client)
> +{
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct imx214 *imx214 = to_imx214(sd);
> +
> + v4l2_async_unregister_subdev(&imx214->sd);
> + media_entity_cleanup(&imx214->sd.entity);
> + v4l2_ctrl_handler_free(&imx214->ctrls);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id imx214_of_match[] = {
> + { .compatible = "sony,imx214" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, imx214_of_match);
> +
> +static struct i2c_driver imx214_i2c_driver = {
> + .driver = {
> + .of_match_table = imx214_of_match,
> + .name = "imx214",
> + },
> + .probe_new = imx214_probe,
> + .remove = imx214_remove,
> +};
> +
> +module_i2c_driver(imx214_i2c_driver);
> +
> +MODULE_DESCRIPTION("Sony IMX214 Camera drier");
> +MODULE_AUTHOR("Ricardo Ribalda <ricardo.ribalda@gmail.com>");
> +MODULE_LICENSE("GPL v2");
--
Kind regards,
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] [media] imx214: Add imx214 camera sensor driver
2018-09-21 9:28 ` [PATCH 1/2] [media] imx214: Add imx214 camera sensor driver Sakari Ailus
@ 2018-09-21 10:15 ` Ricardo Ribalda Delgado
2018-09-24 20:32 ` Sakari Ailus
0 siblings, 1 reply; 8+ messages in thread
From: Ricardo Ribalda Delgado @ 2018-09-21 10:15 UTC (permalink / raw)
To: Sakari Ailus
Cc: Hans Verkuil, Laurent Pinchart, Mauro Carvalho Chehab, LKML, linux-media
Hi Sakari
On Fri, Sep 21, 2018 at 11:28 AM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi, Ricardo!
>
> Thanks for the patch.
Thank you for your review!
>
> On Fri, Sep 21, 2018 at 10:54:49AM +0200, Ricardo Ribalda Delgado wrote:
> > Add a V4L2 sub-device driver for the Sony IMX214 image sensor.
> > This is a camera sensor using the I2C bus for control and the
> > CSI-2 bus for data.
> >
> > Tested on a DB820c alike board with Intrinsyc Open-Q 13MP camera.
> >
> > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> > ---
> > MAINTAINERS | 8 +
> > drivers/media/i2c/Kconfig | 12 +
> > drivers/media/i2c/Makefile | 1 +
> > drivers/media/i2c/imx214.c | 994 +++++++++++++++++++++++++++++++++++++
> > 4 files changed, 1015 insertions(+)
> > create mode 100644 drivers/media/i2c/imx214.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 9989925f658d..2ae68894e700 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -13521,6 +13521,14 @@ S: Maintained
> > F: drivers/ssb/
> > F: include/linux/ssb/
> >
> > +SONY IMX214 SENSOR DRIVER
> > +M: Ricardo Ribalda <ricardo.ribalda@gmail.com>
> > +L: linux-media@vger.kernel.org
> > +T: git git://linuxtv.org/media_tree.git
> > +S: Maintained
> > +F: drivers/media/i2c/imx214.c
> > +F: Documentation/devicetree/bindings/media/i2c/imx214.txt
>
> DT bindings come before the driver; please swap the order for v2.
Done
>
> > +
> > SONY IMX258 SENSOR DRIVER
> > M: Sakari Ailus <sakari.ailus@linux.intel.com>
> > L: linux-media@vger.kernel.org
> > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > index bfdb494686bf..d9416b3a090c 100644
> > --- a/drivers/media/i2c/Kconfig
> > +++ b/drivers/media/i2c/Kconfig
> > @@ -595,6 +595,18 @@ config VIDEO_APTINA_PLL
> > config VIDEO_SMIAPP_PLL
> > tristate
> >
> > +config VIDEO_IMX214
> > + tristate "Sony IMX214 sensor support"
> > + depends on GPIOLIB && I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> > + depends on OF
> > + depends on MEDIA_CAMERA_SUPPORT
> > + help
> > + This is a Video4Linux2 sensor driver for the Sony
> > + IMX214 camera.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called imx214.
> > +
> > config VIDEO_IMX258
> > tristate "Sony IMX258 sensor support"
> > depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> > index a94eb03d10d4..61305edc6165 100644
> > --- a/drivers/media/i2c/Makefile
> > +++ b/drivers/media/i2c/Makefile
> > @@ -106,6 +106,7 @@ obj-$(CONFIG_VIDEO_I2C) += video-i2c.o
> > obj-$(CONFIG_VIDEO_ML86V7667) += ml86v7667.o
> > obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
> > obj-$(CONFIG_VIDEO_TC358743) += tc358743.o
> > +obj-$(CONFIG_VIDEO_IMX214) += imx214.o
> > obj-$(CONFIG_VIDEO_IMX258) += imx258.o
> > obj-$(CONFIG_VIDEO_IMX274) += imx274.o
> >
> > diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c
> > new file mode 100644
> > index 000000000000..3a425818036c
> > --- /dev/null
> > +++ b/drivers/media/i2c/imx214.c
> > @@ -0,0 +1,994 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * imx214.c - imx214 sensor driver
> > + *
> > + * Copyright 2018 Qtechnology A/S
> > + *
> > + * Ricardo Ribalda <ricardo.ribalda@gmail.com>
> > + */
> > +#include <linux/i2c.h>
> > +#include <linux/module.h>
> > +#include <linux/clk.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/delay.h>
> > +#include <linux/regmap.h>
> > +#include <media/media-entity.h>
> > +#include <media/v4l2-fwnode.h>
> > +#include <media/v4l2-subdev.h>
> > +#include <media/v4l2-ctrls.h>
> > +
> > +#define IMX214_VOLTAGE_AVDD 2700000
> > +#define IMX214_VOLTAGE_DVDD 1120000
> > +#define IMX214_VOLTAGE_DOVDD 1800000
>
> While I guess there's nothing wrong technically here (or using
> regulator_set_voltage()), the other sensor drivers depend on the voltages
> being set in DT rather than in drivers. The reason could be that as range
> of voltages is fine for a sensor, another device requiring slightly
> different voltage than the nominal voltage of the sensor could be powered
> from the same regulator.
Done
>
> > +
> > +#define IMX214_DEFAULT_CLK_FREQ 24000000
> > +#define IMX214_DEFAULT_PIXEL_RATE 384000000
> > +#define IMX214_WIDTH 4096
> > +#define IMX214_HEIGHT 2304
> > +#define IMX214_FPS 30
> > +#define IMX214_MBUS_CODE MEDIA_BUS_FMT_SRGGB10_1X10
> > +
> > +struct imx214 {
> > + struct i2c_client *i2c_client;
> > + struct device *dev;
> > + struct clk *xclk;
> > + struct regmap *regmap;
> > +
> > + struct v4l2_fwnode_endpoint ep;
> > + struct v4l2_subdev sd;
> > + struct media_pad pad;
> > + struct v4l2_mbus_framefmt fmt;
> > + struct v4l2_rect crop;
> > +
> > + struct v4l2_ctrl_handler ctrls;
> > + struct v4l2_ctrl *pixel_rate;
> > + struct v4l2_ctrl *link_freq;
> > +
> > + struct regulator *vdda_regulator;
> > + struct regulator *vddd_regulator;
> > + struct regulator *vdddo_regulator;
> > +
> > + struct gpio_desc *enable_gpio;
> > +
> > + bool power_on;
> > +};
> > +
> > +struct reg_8 {
> > + u16 addr;
> > + u8 val;
> > +};
> > +
> > +static struct reg_8 mode_1920x1080[];
> > +static struct reg_8 mode_4096x2304[];
>
> Const. Could you rearrange the bits to avoid the forward declarations?
Const done, but I prefer to keep the forward declaration. Otherwise
the long tables will "hide" the mode declaration.
>
> > +
> > +/*
> > + * Declare modes in order, from biggest
> > + * to smallest height.
> > + */
> > +struct imx214_mode {
> > + u32 width;
> > + u32 height;
> > + struct reg_8 *reg_table;
> > +} imx214_modes[] = {
>
> This should be static const.
>
> > + {
> > + .width = 4096,
> > + .height = 2304,
> > + .reg_table = mode_4096x2304,
> > + },
> > + {
> > + .width = 1920,
> > + .height = 1080,
> > + .reg_table = mode_1920x1080,
> > + },
> > +};
> > +
> > +static inline struct imx214 *to_imx214(struct v4l2_subdev *sd)
> > +{
> > + return container_of(sd, struct imx214, sd);
> > +}
> > +
> > +static int imx214_set_power_on(struct imx214 *imx214)
>
> Runtime PM support would be nice; you could drop the s_power callback as a
> bonus.>
> > +{
> > + int ret;
> > +
> > + gpiod_set_value_cansleep(imx214->enable_gpio, 0);
> > + usleep_range(10, 20);
> > +
> > + ret = regulator_enable(imx214->vdddo_regulator);
>
> Would regulator_bulk_enable() be usable here?
Done
>
> > + if (ret) {
> > + dev_err(imx214->dev, "Failed to enable vdddo\n");
> > + return ret;
> > + }
> > +
> > + ret = regulator_enable(imx214->vdda_regulator);
> > + if (ret < 0) {
> > + dev_err(imx214->dev, "Failed to enable vdda\n");
> > + return ret;
> > + }
> > +
> > + ret = regulator_enable(imx214->vddd_regulator);
> > + if (ret) {
> > + dev_err(imx214->dev, "Failed to enable vddd\n");
> > + return ret;
> > + }
> > +
> > + usleep_range(2000, 3000);
> > +
> > + ret = clk_prepare_enable(imx214->xclk);
> > + if (ret < 0) {
> > + dev_err(imx214->dev, "clk prepare enable failed\n");
> > + return ret;
> > + }
> > +
> > +
> > + gpiod_set_value_cansleep(imx214->enable_gpio, 1);
> > + usleep_range(12000, 15000);
> > +
> > + return 0;
> > +}
> > +
> > +static void imx214_set_power_off(struct imx214 *imx214)
> > +{
> > + gpiod_set_value_cansleep(imx214->enable_gpio, 0);
> > +
> > + clk_disable_unprepare(imx214->xclk);
> > +
> > + regulator_disable(imx214->vdda_regulator);
> > + regulator_disable(imx214->vddd_regulator);
> > + regulator_disable(imx214->vdddo_regulator);
>
> regulator_bulk_disable()?
Done
>
> > +}
> > +
> > +static int imx214_s_power(struct v4l2_subdev *sd, int on)
> > +{
> > + struct imx214 *imx214 = to_imx214(sd);
> > + int ret = 0;
> > +
> > + on = !!on;
> > +
> > + if (imx214->power_on == on)
> > + return 0;
> > +
> > + if (on)
> > + ret = imx214_set_power_on(imx214);
> > + else
> > + imx214_set_power_off(imx214);
> > +
> > + imx214->power_on = on;
> > + return 0;
> > +}
> > +
> > +static int imx214_enum_mbus_code(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_mbus_code_enum *code)
> > +{
> > + if (code->index > 0)
> > + return -EINVAL;
> > +
> > + code->code = IMX214_MBUS_CODE;
> > +
> > + return 0;
> > +}
> > +
> > +static int imx214_enum_frame_size(struct v4l2_subdev *subdev,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_frame_size_enum *fse)
> > +{
> > + if (fse->code != IMX214_MBUS_CODE)
> > + return -EINVAL;
> > +
> > + if (fse->index >= ARRAY_SIZE(imx214_modes))
> > + return -EINVAL;
> > +
> > + fse->min_width = fse->max_width = imx214_modes[fse->index].width;
> > + fse->min_height = fse->max_height = imx214_modes[fse->index].height;
> > +
> > + return 0;
> > +}
> > +
> > +#ifdef CONFIG_VIDEO_ADV_DEBUG
> > +static int imx214_s_register(struct v4l2_subdev *subdev,
> > + const struct v4l2_dbg_register *reg)
> > +{
> > + struct imx214 *imx214 = container_of(subdev, struct imx214, sd);
> > +
> > + regmap_write(imx214->regmap, reg->reg, reg->val);
> > + return 0;
>
> return regmap_write() ?
Done
>
> > +}
> > +
> > +static int imx214_g_register(struct v4l2_subdev *subdev,
> > + struct v4l2_dbg_register *reg)
> > +{
> > + struct imx214 *imx214 = container_of(subdev, struct imx214, sd);
> > + unsigned int aux;
> > +
> > + reg->size = 1;
> > + regmap_read(imx214->regmap, reg->reg, &aux);
> > + reg->val = aux;
> > + return 0;
> > +}
> > +#endif
> > +
> > +static const struct v4l2_subdev_core_ops imx214_core_ops = {
> > +#ifdef CONFIG_VIDEO_ADV_DEBUG
> > + .g_register = imx214_g_register,
> > + .s_register = imx214_s_register,
> > +#endif
> > + .s_power = imx214_s_power,
> > +};
> > +
> > +static struct v4l2_mbus_framefmt *
> > +__imx214_get_pad_format(struct imx214 *imx214,
> > + struct v4l2_subdev_pad_config *cfg,
> > + unsigned int pad,
> > + enum v4l2_subdev_format_whence which)
> > +{
> > + switch (which) {
> > + case V4L2_SUBDEV_FORMAT_TRY:
> > + return v4l2_subdev_get_try_format(&imx214->sd, cfg, pad);
> > + case V4L2_SUBDEV_FORMAT_ACTIVE:
> > + return &imx214->fmt;
> > + default:
> > + return NULL;
> > + }
> > +}
> > +
> > +static int imx214_get_format(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_format *format)
> > +{
> > + struct imx214 *imx214 = to_imx214(sd);
> > +
> > + format->format = *__imx214_get_pad_format(imx214, cfg, format->pad,
> > + format->which);
> > +
> > + return 0;
> > +}
> > +
> > +static struct v4l2_rect *
> > +__imx214_get_pad_crop(struct imx214 *imx214, struct v4l2_subdev_pad_config *cfg,
> > + unsigned int pad, enum v4l2_subdev_format_whence which)
> > +{
> > + switch (which) {
> > + case V4L2_SUBDEV_FORMAT_TRY:
> > + return v4l2_subdev_get_try_crop(&imx214->sd, cfg, pad);
> > + case V4L2_SUBDEV_FORMAT_ACTIVE:
> > + return &imx214->crop;
> > + default:
> > + return NULL;
> > + }
> > +}
> > +
> > +static int imx214_find_mode(u32 height)
> > +{
> > + int i;
> > +
> > + for (i = 0; (i < ARRAY_SIZE(imx214_modes) - 1) ; i++)
> > + if (height >= imx214_modes[i].width)
> > + return i;
> > + return i;
> > +}
> > +
> > +static int imx214_set_format(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_format *format)
> > +{
> > + struct imx214 *imx214 = to_imx214(sd);
> > + struct v4l2_mbus_framefmt *__format;
> > + struct v4l2_rect *__crop;
> > + int mode;
> > +
> > + __crop = __imx214_get_pad_crop(imx214, cfg, format->pad, format->which);
> > +
> > + mode = format ? imx214_find_mode(format->format.height) : 0;
> > +
> > + __crop->width = imx214_modes[mode].width;
> > + __crop->height = imx214_modes[mode].height;
> > +
> > + __format = __imx214_get_pad_format(imx214, cfg, format->pad,
> > + format->which);
> > + __format->width = __crop->width;
> > + __format->height = __crop->height;
> > + __format->code = IMX214_MBUS_CODE;
> > + __format->field = V4L2_FIELD_NONE;
> > + __format->colorspace = V4L2_COLORSPACE_SRGB;
> > + __format->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(__format->colorspace);
> > + __format->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(true,
> > + __format->colorspace, __format->ycbcr_enc);
> > + __format->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(__format->colorspace);
> > +
> > + format->format = *__format;
> > +
> > + return 0;
> > +}
> > +
> > +static int imx214_get_selection(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_selection *sel)
> > +{
> > + struct imx214 *imx214 = to_imx214(sd);
> > +
> > + if (sel->target != V4L2_SEL_TGT_CROP)
> > + return -EINVAL;
> > +
> > + sel->r = *__imx214_get_pad_crop(imx214, cfg, sel->pad,
> > + sel->which);
> > + return 0;
> > +}
> > +
> > +static int imx214_entity_init_cfg(struct v4l2_subdev *subdev,
> > + struct v4l2_subdev_pad_config *cfg)
> > +{
> > + struct v4l2_subdev_format fmt = { };
> > +
> > + fmt.which = cfg ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
> > + fmt.format.width = imx214_modes[0].width;
> > + fmt.format.height = imx214_modes[0].height;
> > +
> > + imx214_set_format(subdev, cfg, &fmt);
> > +
> > + return 0;
> > +}
> > +
> > +enum { IMX214_TABLE_WAIT_MS = 0,
> > + IMX214_TABLE_END,
> > + IMX214_MAX_RETRIES,
> > + IMX214_WAIT_MS};
> > +
> > +/*From imx214_mode_tbls.h*/
> > +static struct reg_8 mode_4096x2304[] = {
>
> const, please.
>
> > + {0x0114, 0x03},
> > + {0x0220, 0x00},
> > + {0x0221, 0x11},
> > + {0x0222, 0x01},
> > + {0x0340, 0x0C},
> > + {0x0341, 0x7A},
> > + {0x0342, 0x13},
> > + {0x0343, 0x90},
> > + {0x0344, 0x00},
> > + {0x0345, 0x38},
> > + {0x0346, 0x01},
> > + {0x0347, 0x98},
> > + {0x0348, 0x10},
> > + {0x0349, 0x37},
> > + {0x034A, 0x0A},
> > + {0x034B, 0x97},
> > + {0x0381, 0x01},
> > + {0x0383, 0x01},
> > + {0x0385, 0x01},
> > + {0x0387, 0x01},
> > + {0x0900, 0x00},
> > + {0x0901, 0x00},
> > + {0x0902, 0x00},
> > + {0x3000, 0x35},
> > + {0x3054, 0x01},
> > + {0x305C, 0x11},
> > +
> > + {0x0112, 0x0A},
> > + {0x0113, 0x0A},
> > + {0x034C, 0x10},
> > + {0x034D, 0x00},
> > + {0x034E, 0x09},
> > + {0x034F, 0x00},
> > + {0x0401, 0x00},
> > + {0x0404, 0x00},
> > + {0x0405, 0x10},
> > + {0x0408, 0x00},
> > + {0x0409, 0x00},
> > + {0x040A, 0x00},
> > + {0x040B, 0x00},
> > + {0x040C, 0x10},
> > + {0x040D, 0x00},
> > + {0x040E, 0x09},
> > + {0x040F, 0x00},
> > +
> > + {0x0301, 0x05},
> > + {0x0303, 0x02},
> > + {0x0305, 0x03},
> > + {0x0306, 0x00},
> > + {0x0307, 0x96},
> > + {0x0309, 0x0A},
> > + {0x030B, 0x01},
> > + {0x0310, 0x00},
> > +
> > + {0x0820, 0x12},
> > + {0x0821, 0xC0},
> > + {0x0822, 0x00},
> > + {0x0823, 0x00},
> > +
> > + {0x3A03, 0x09},
> > + {0x3A04, 0x50},
> > + {0x3A05, 0x01},
> > +
> > + {0x0B06, 0x01},
> > + {0x30A2, 0x00},
> > +
> > + {0x30B4, 0x00},
> > +
> > + {0x3A02, 0xFF},
> > +
> > + {0x3011, 0x00},
> > + {0x3013, 0x01},
> > +
> > + {0x0202, 0x0C},
> > + {0x0203, 0x70},
> > + {0x0224, 0x01},
> > + {0x0225, 0xF4},
> > +
> > + {0x0204, 0x00},
> > + {0x0205, 0x00},
> > + {0x020E, 0x01},
> > + {0x020F, 0x00},
> > + {0x0210, 0x01},
> > + {0x0211, 0x00},
> > + {0x0212, 0x01},
> > + {0x0213, 0x00},
> > + {0x0214, 0x01},
> > + {0x0215, 0x00},
> > + {0x0216, 0x00},
> > + {0x0217, 0x00},
> > +
> > + {0x4170, 0x00},
> > + {0x4171, 0x10},
> > + {0x4176, 0x00},
> > + {0x4177, 0x3C},
> > + {0xAE20, 0x04},
> > + {0xAE21, 0x5C},
> > +
> > + {IMX214_TABLE_WAIT_MS, 10},
> > + {0x0138, 0x01},
> > + {IMX214_TABLE_END, 0x00}
> > +};
> > +
> > +static struct reg_8 mode_1920x1080[] = {
>
> Ditto. Same below.
Done
>
> > + {0x0114, 0x03},
> > + {0x0220, 0x00},
> > + {0x0221, 0x11},
> > + {0x0222, 0x01},
> > + {0x0340, 0x0C},
> > + {0x0341, 0x7A},
> > + {0x0342, 0x13},
> > + {0x0343, 0x90},
> > + {0x0344, 0x04},
> > + {0x0345, 0x78},
> > + {0x0346, 0x03},
> > + {0x0347, 0xFC},
> > + {0x0348, 0x0B},
> > + {0x0349, 0xF7},
> > + {0x034A, 0x08},
> > + {0x034B, 0x33},
> > + {0x0381, 0x01},
> > + {0x0383, 0x01},
> > + {0x0385, 0x01},
> > + {0x0387, 0x01},
> > + {0x0900, 0x00},
> > + {0x0901, 0x00},
> > + {0x0902, 0x00},
> > + {0x3000, 0x35},
> > + {0x3054, 0x01},
> > + {0x305C, 0x11},
> > +
> > + {0x0112, 0x0A},
> > + {0x0113, 0x0A},
> > + {0x034C, 0x07},
> > + {0x034D, 0x80},
> > + {0x034E, 0x04},
> > + {0x034F, 0x38},
> > + {0x0401, 0x00},
> > + {0x0404, 0x00},
> > + {0x0405, 0x10},
> > + {0x0408, 0x00},
> > + {0x0409, 0x00},
> > + {0x040A, 0x00},
> > + {0x040B, 0x00},
> > + {0x040C, 0x07},
> > + {0x040D, 0x80},
> > + {0x040E, 0x04},
> > + {0x040F, 0x38},
> > +
> > + {0x0301, 0x05},
> > + {0x0303, 0x02},
> > + {0x0305, 0x03},
> > + {0x0306, 0x00},
> > + {0x0307, 0x96},
> > + {0x0309, 0x0A},
> > + {0x030B, 0x01},
> > + {0x0310, 0x00},
> > +
> > + {0x0820, 0x12},
> > + {0x0821, 0xC0},
> > + {0x0822, 0x00},
> > + {0x0823, 0x00},
> > +
> > + {0x3A03, 0x04},
> > + {0x3A04, 0xF8},
> > + {0x3A05, 0x02},
> > +
> > + {0x0B06, 0x01},
> > + {0x30A2, 0x00},
> > +
> > + {0x30B4, 0x00},
> > +
> > + {0x3A02, 0xFF},
> > +
> > + {0x3011, 0x00},
> > + {0x3013, 0x01},
> > +
> > + {0x0202, 0x0C},
> > + {0x0203, 0x70},
> > + {0x0224, 0x01},
> > + {0x0225, 0xF4},
> > +
> > + {0x0204, 0x00},
> > + {0x0205, 0x00},
> > + {0x020E, 0x01},
> > + {0x020F, 0x00},
> > + {0x0210, 0x01},
> > + {0x0211, 0x00},
> > + {0x0212, 0x01},
> > + {0x0213, 0x00},
> > + {0x0214, 0x01},
> > + {0x0215, 0x00},
> > + {0x0216, 0x00},
> > + {0x0217, 0x00},
> > +
> > + {0x4170, 0x00},
> > + {0x4171, 0x10},
> > + {0x4176, 0x00},
> > + {0x4177, 0x3C},
> > + {0xAE20, 0x04},
> > + {0xAE21, 0x5C},
> > +
> > + {IMX214_TABLE_WAIT_MS, 10},
> > + {0x0138, 0x01},
> > + {IMX214_TABLE_END, 0x00}
> > +};
> > +
> > +static struct reg_8 mode_table_common[] = {
> > + /* software reset */
> > +
> > + /* software standby settings */
> > + {0x0100, 0x00},
> > +
> > + /* ATR setting */
> > + {0x9300, 0x02},
> > +
> > + /* external clock setting */
> > + {0x0136, 0x18},
> > + {0x0137, 0x00},
> > +
> > + /* global setting */
> > + /* basic config */
> > + {0x0101, 0x00},
> > + {0x0105, 0x01},
> > + {0x0106, 0x01},
> > + {0x4550, 0x02},
> > + {0x4601, 0x00},
> > + {0x4642, 0x05},
> > + {0x6227, 0x11},
> > + {0x6276, 0x00},
> > + {0x900E, 0x06},
> > + {0xA802, 0x90},
> > + {0xA803, 0x11},
> > + {0xA804, 0x62},
> > + {0xA805, 0x77},
> > + {0xA806, 0xAE},
> > + {0xA807, 0x34},
> > + {0xA808, 0xAE},
> > + {0xA809, 0x35},
> > + {0xA80A, 0x62},
> > + {0xA80B, 0x83},
> > + {0xAE33, 0x00},
> > +
> > + /* analog setting */
> > + {0x4174, 0x00},
> > + {0x4175, 0x11},
> > + {0x4612, 0x29},
> > + {0x461B, 0x12},
> > + {0x461F, 0x06},
> > + {0x4635, 0x07},
> > + {0x4637, 0x30},
> > + {0x463F, 0x18},
> > + {0x4641, 0x0D},
> > + {0x465B, 0x12},
> > + {0x465F, 0x11},
> > + {0x4663, 0x11},
> > + {0x4667, 0x0F},
> > + {0x466F, 0x0F},
> > + {0x470E, 0x09},
> > + {0x4909, 0xAB},
> > + {0x490B, 0x95},
> > + {0x4915, 0x5D},
> > + {0x4A5F, 0xFF},
> > + {0x4A61, 0xFF},
> > + {0x4A73, 0x62},
> > + {0x4A85, 0x00},
> > + {0x4A87, 0xFF},
> > +
> > + /* embedded data */
> > + {0x5041, 0x04},
> > + {0x583C, 0x04},
> > + {0x620E, 0x04},
> > + {0x6EB2, 0x01},
> > + {0x6EB3, 0x00},
> > + {0x9300, 0x02},
> > +
> > + /* imagequality */
> > + /* HDR setting */
> > + {0x3001, 0x07},
> > + {0x6D12, 0x3F},
> > + {0x6D13, 0xFF},
> > + {0x9344, 0x03},
> > + {0x9706, 0x10},
> > + {0x9707, 0x03},
> > + {0x9708, 0x03},
> > + {0x9E04, 0x01},
> > + {0x9E05, 0x00},
> > + {0x9E0C, 0x01},
> > + {0x9E0D, 0x02},
> > + {0x9E24, 0x00},
> > + {0x9E25, 0x8C},
> > + {0x9E26, 0x00},
> > + {0x9E27, 0x94},
> > + {0x9E28, 0x00},
> > + {0x9E29, 0x96},
> > +
> > + /* CNR parameter setting */
> > + {0x69DB, 0x01},
> > +
> > + /* Moire reduction */
> > + {0x6957, 0x01},
> > +
> > + /* image enhancment */
> > + {0x6987, 0x17},
> > + {0x698A, 0x03},
> > + {0x698B, 0x03},
> > +
> > + /* white balanace */
> > + {0x0B8E, 0x01},
> > + {0x0B8F, 0x00},
> > + {0x0B90, 0x01},
> > + {0x0B91, 0x00},
> > + {0x0B92, 0x01},
> > + {0x0B93, 0x00},
> > + {0x0B94, 0x01},
> > + {0x0B95, 0x00},
> > +
> > + /* ATR setting */
> > + {0x6E50, 0x00},
> > + {0x6E51, 0x32},
> > + {0x9340, 0x00},
> > + {0x9341, 0x3C},
> > + {0x9342, 0x03},
> > + {0x9343, 0xFF},
> > + {IMX214_TABLE_END, 0x00}
> > +};
> > +
> > +#define MAX_CMD 4
> > +static int imx214_write_table(struct imx214 *imx214,
> > + const struct reg_8 table[])
> > +{
> > + u8 vals[MAX_CMD];
> > + int ret;
> > + int i;
> > +
> > + for (table = table; table->addr != IMX214_TABLE_END ; table++) {
> > + if (table->addr == IMX214_TABLE_WAIT_MS) {
> > + msleep(table->val);
> > + continue;
> > + }
> > +
> > + for (i = 0; i < MAX_CMD; i++) {
> > + if (table[i].addr != (table[0].addr + i))
> > + break;
> > + vals[i] = table[i].val;
> > + }
> > +
> > + ret = regmap_bulk_write(imx214->regmap, table->addr, vals, i);
> > +
> > + if (ret) {
> > + dev_err(imx214->dev, "write_table error: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + table += i - 1;
> > +
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +
> > +static int imx214_s_stream(struct v4l2_subdev *subdev, int enable)
> > +{
> > + struct imx214 *imx214 = to_imx214(subdev);
> > + int ret;
> > +
> > + if (enable) {
> > + int mode = imx214_find_mode(imx214->fmt.height);
> > +
> > + ret = imx214_write_table(imx214, mode_table_common);
> > + if (ret < 0) {
> > + dev_err(imx214->dev, "could not sent common table %d\n",
> > + ret);
> > + return ret;
> > + }
> > + ret = imx214_write_table(imx214, imx214_modes[mode].reg_table);
> > + if (ret < 0) {
> > + dev_err(imx214->dev, "could not sent mode table %d\n",
> > + ret);
> > + return ret;
> > + }
> > + ret = v4l2_ctrl_handler_setup(&imx214->ctrls);
> > + if (ret < 0) {
> > + dev_err(imx214->dev, "could not sync v4l2 controls\n");
> > + return ret;
> > + }
> > + ret = regmap_write(imx214->regmap, 0x100, 1);
> > + if (ret < 0) {
> > + dev_err(imx214->dev, "could not sent start table %d\n",
> > + ret);
> > + return ret;
> > + }
> > + } else {
> > + ret = regmap_write(imx214->regmap, 0x100, 0);
> > + if (ret < 0) {
> > + dev_err(imx214->dev, "could not sent stop table %d\n",
> > + ret);
> > + return ret;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int imx214_g_frame_interval(struct v4l2_subdev *subdev,
> > + struct v4l2_subdev_frame_interval *fival)
> > +{
> > + fival->pad = 0;
> > + fival->interval.numerator = 1;
> > + fival->interval.denominator = IMX214_FPS;
> > +
> > + return 0;
> > +}
> > +
> > +static int imx214_enum_frame_interval(struct v4l2_subdev *subdev,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_frame_interval_enum *fie)
> > +{
> > + int mode;
> > +
> > + if (fie->index != 0)
> > + return -EINVAL;
> > +
> > + mode = imx214_find_mode(fie->height);
> > +
> > + fie->code = IMX214_MBUS_CODE;
> > + fie->width = imx214_modes[mode].width;
> > + fie->height = imx214_modes[mode].height;
> > + fie->interval.numerator = 1;
> > + fie->interval.denominator = IMX214_MBUS_CODE;
> > +
> > + return 0;
> > +}
> > +
> > +static const struct v4l2_subdev_video_ops imx214_video_ops = {
> > + .s_stream = imx214_s_stream,
> > + .g_frame_interval = imx214_g_frame_interval,
> > + .s_frame_interval = imx214_g_frame_interval,
> > +};
> > +
> > +static const struct v4l2_subdev_pad_ops imx214_subdev_pad_ops = {
> > + .enum_mbus_code = imx214_enum_mbus_code,
> > + .enum_frame_size = imx214_enum_frame_size,
> > + .enum_frame_interval = imx214_enum_frame_interval,
> > + .get_fmt = imx214_get_format,
> > + .set_fmt = imx214_set_format,
> > + .get_selection = imx214_get_selection,
> > + .init_cfg = imx214_entity_init_cfg,
> > +};
> > +
> > +static const struct v4l2_subdev_ops imx214_subdev_ops = {
> > + .core = &imx214_core_ops,
> > + .video = &imx214_video_ops,
> > + .pad = &imx214_subdev_pad_ops,
> > +};
> > +
> > +static const struct regmap_config sensor_regmap_config = {
> > + .reg_bits = 16,
> > + .val_bits = 8,
> > + .cache_type = REGCACHE_RBTREE,
> > +};
> > +
> > +static int imx214_probe(struct i2c_client *client)
> > +{
> > + struct device *dev = &client->dev;
> > + struct imx214 *imx214;
> > + struct fwnode_handle *endpoint;
> > + int ret;
> > + static const s64 link_freq[] = {
> > + (IMX214_DEFAULT_PIXEL_RATE * 10LL) / 8,
>
> You should check the link frequency matches with that from the firmware.
I am not sure what you mean here sorry.
>
> > + };
> > +
> > + imx214 = devm_kzalloc(dev, sizeof(*imx214), GFP_KERNEL);
> > + if (!imx214)
> > + return -ENOMEM;
> > +
> > + imx214->i2c_client = client;
>
> Please drop i2c_client field and use v4l2_get_subdev_data(&imx214->sd)
> instead.
i2c_client removed
>
> > + imx214->dev = dev;
> > +
> > + endpoint = fwnode_graph_get_next_endpoint(
> > + of_fwnode_handle(client->dev.of_node), NULL);
> > + if (!endpoint) {
> > + dev_err(dev, "endpoint node not found\n");
> > + return -EINVAL;
> > + }
> > +
> > + ret = v4l2_fwnode_endpoint_parse(endpoint, &imx214->ep);
> > + fwnode_handle_put(endpoint);
> > + if (ret < 0) {
> > + dev_err(dev, "parsing endpoint node failed\n");
> > + return ret;
> > + }
> > +
> > + if (imx214->ep.bus_type != V4L2_MBUS_CSI2) {
> > + dev_err(dev, "invalid bus type (%u), must be CSI2 (%u)\n",
> > + imx214->ep.bus_type, V4L2_MBUS_CSI2);
> > + return -EINVAL;
> > + }
> > +
> > + imx214->xclk = devm_clk_get(dev, "xclk");
> > + if (IS_ERR(imx214->xclk)) {
> > + dev_err(dev, "could not get xclk");
> > + return PTR_ERR(imx214->xclk);
> > + }
> > +
> > + ret = clk_set_rate(imx214->xclk, IMX214_DEFAULT_CLK_FREQ);
> > + if (ret) {
> > + dev_err(dev, "could not set xclk frequency\n");
> > + return ret;
> > + }
> > +
> > + imx214->vdda_regulator = devm_regulator_get(dev, "vdda");
> > + if (IS_ERR(imx214->vdda_regulator)) {
> > + dev_err(dev, "cannot get vdda regulator\n");
> > + return PTR_ERR(imx214->vdda_regulator);
> > + }
> > +
> > + ret = regulator_set_voltage(imx214->vdda_regulator,
> > + IMX214_VOLTAGE_AVDD,
> > + IMX214_VOLTAGE_AVDD);
> > + if (ret < 0) {
> > + dev_err(dev, "cannot set vdda voltage\n");
> > + return ret;
> > + }
> > +
> > + imx214->vddd_regulator = devm_regulator_get(dev, "vddd");
> > + if (IS_ERR(imx214->vddd_regulator)) {
> > + dev_err(dev, "cannot get core regulator\n");
> > + return PTR_ERR(imx214->vddd_regulator);
> > + }
> > +
> > + ret = regulator_set_voltage(imx214->vddd_regulator,
> > + IMX214_VOLTAGE_DVDD,
> > + IMX214_VOLTAGE_DVDD);
> > + if (ret < 0) {
> > + dev_err(dev, "cannot set vddd voltage\n");
> > + return ret;
> > + }
> > +
> > + imx214->vdddo_regulator = devm_regulator_get(dev, "vdddo");
> > + if (IS_ERR(imx214->vdddo_regulator)) {
> > + dev_err(dev, "cannot get vdddo regulator\n");
> > + return PTR_ERR(imx214->vdddo_regulator);
> > + }
> > +
> > + ret = regulator_set_voltage(imx214->vdddo_regulator,
> > + IMX214_VOLTAGE_DOVDD,
> > + IMX214_VOLTAGE_DOVDD);
> > + if (ret < 0) {
> > + dev_err(dev, "cannot set vdddo voltage\n");
> > + return ret;
> > + }
> > +
> > + imx214->enable_gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_LOW);
> > + if (IS_ERR(imx214->enable_gpio)) {
> > + dev_err(dev, "cannot get enable gpio\n");
> > + return PTR_ERR(imx214->enable_gpio);
> > + }
> > +
> > + imx214->regmap = devm_regmap_init_i2c(client, &sensor_regmap_config);
> > + if (IS_ERR(imx214->regmap)) {
> > + dev_err(dev, "regmap init failed\n");
> > + return PTR_ERR(imx214->regmap);
> > + }
> > +
> > + v4l2_ctrl_handler_init(&imx214->ctrls, 2);
> > +
> > + imx214->pixel_rate = v4l2_ctrl_new_std(&imx214->ctrls, NULL,
> > + V4L2_CID_PIXEL_RATE, 0,
> > + IMX214_DEFAULT_PIXEL_RATE, 1,
> > + IMX214_DEFAULT_PIXEL_RATE);
> > + imx214->link_freq = v4l2_ctrl_new_int_menu(&imx214->ctrls, NULL,
> > + V4L2_CID_LINK_FREQ,
> > + ARRAY_SIZE(link_freq) - 1,
> > + 0, link_freq);
>
> Do I understand this correctly that the driver does not support setting
> e.g. exposure time or gain? Those are very basic features...
Yep :), this is just a first step. I do not have the register set from
the device :(. So I am reverse engineering a lot of things.
I will add more controls as I am done with them.
>
> You'll also need to ensure the s_ctrl() callback works without s_power()
> being called. My suggestion is to switch to PM runtime; see e.g. the ov1385
> driver in the current media tree master.
There is one limitation with this chip on the dragonboard. I2c only
works if the camss is ON. Therefore whatever s_ctrl needs to be
cached and written at streamon.
>
> > + if (imx214->link_freq)
> > + imx214->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> > + ret = imx214->ctrls.error;
> > + if (ret) {
> > + dev_err(&client->dev, "%s control init failed (%d)\n",
> > + __func__, ret);
>
> goto free_ctrl;
>
> > + return ret;
> > + }
> > +
> > + imx214->sd.ctrl_handler = &imx214->ctrls;
> > + ret = v4l2_ctrl_handler_setup(imx214->sd.ctrl_handler);
> > + if (ret) {
> > + dev_err(&client->dev,
> > + "Error %d setting default controls\n", ret);
>
> Ditto.
Done
>
> > + return ret;
> > + }
> > +
> > + v4l2_i2c_subdev_init(&imx214->sd, client, &imx214_subdev_ops);
> > + imx214->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > + imx214->pad.flags = MEDIA_PAD_FL_SOURCE;
> > + imx214->sd.dev = &client->dev;
> > + imx214->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > +
> > + ret = media_entity_pads_init(&imx214->sd.entity, 1, &imx214->pad);
> > + if (ret < 0) {
> > + dev_err(dev, "could not register media entity\n");
> > + goto free_ctrl;
> > + }
> > +
> > + imx214_entity_init_cfg(&imx214->sd, NULL);
> > +
> > + ret = v4l2_async_register_subdev_sensor_common(&imx214->sd);
> > + if (ret < 0) {
> > + dev_err(dev, "could not register v4l2 device\n");
> > + goto free_entity;
> > + }
> > +
> > + return 0;
> > +
> > +free_entity:
> > + media_entity_cleanup(&imx214->sd.entity);
> > +free_ctrl:
> > + v4l2_ctrl_handler_free(&imx214->ctrls);
> > +
> > + return ret;
> > +}
> > +
> > +static int imx214_remove(struct i2c_client *client)
> > +{
> > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > + struct imx214 *imx214 = to_imx214(sd);
> > +
> > + v4l2_async_unregister_subdev(&imx214->sd);
> > + media_entity_cleanup(&imx214->sd.entity);
> > + v4l2_ctrl_handler_free(&imx214->ctrls);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id imx214_of_match[] = {
> > + { .compatible = "sony,imx214" },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, imx214_of_match);
> > +
> > +static struct i2c_driver imx214_i2c_driver = {
> > + .driver = {
> > + .of_match_table = imx214_of_match,
> > + .name = "imx214",
> > + },
> > + .probe_new = imx214_probe,
> > + .remove = imx214_remove,
> > +};
> > +
> > +module_i2c_driver(imx214_i2c_driver);
> > +
> > +MODULE_DESCRIPTION("Sony IMX214 Camera drier");
> > +MODULE_AUTHOR("Ricardo Ribalda <ricardo.ribalda@gmail.com>");
> > +MODULE_LICENSE("GPL v2");
>
> --
> Kind regards,
>
> Sakari Ailus
> sakari.ailus@linux.intel.com
I have just sent v2 to the list.
Thanks!
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] [media] imx214: Add imx214 camera sensor driver
2018-09-21 10:15 ` Ricardo Ribalda Delgado
@ 2018-09-24 20:32 ` Sakari Ailus
2018-09-26 5:26 ` Ricardo Ribalda Delgado
0 siblings, 1 reply; 8+ messages in thread
From: Sakari Ailus @ 2018-09-24 20:32 UTC (permalink / raw)
To: Ricardo Ribalda Delgado
Cc: Hans Verkuil, Laurent Pinchart, Mauro Carvalho Chehab, LKML, linux-media
Hi Ricardo,
On Fri, Sep 21, 2018 at 12:15:55PM +0200, Ricardo Ribalda Delgado wrote:
...
> > > +static struct reg_8 mode_1920x1080[];
> > > +static struct reg_8 mode_4096x2304[];
> >
> > Const. Could you rearrange the bits to avoid the forward declarations?
> Const done, but I prefer to keep the forward declaration. Otherwise
> the long tables will "hide" the mode declaration.
Well, I guess the long tables do "hide" a bunch of other stuff, too. :-)
But... I agree there's no trivial way around those tables either.
It appears I'm not the only one who's commented on the matter of the
forward declaration.
...
> > > +static int imx214_probe(struct i2c_client *client)
> > > +{
> > > + struct device *dev = &client->dev;
> > > + struct imx214 *imx214;
> > > + struct fwnode_handle *endpoint;
> > > + int ret;
> > > + static const s64 link_freq[] = {
> > > + (IMX214_DEFAULT_PIXEL_RATE * 10LL) / 8,
> >
> > You should check the link frequency matches with that from the firmware.
>
> I am not sure what you mean here sorry.
The system firmware holds safe frequencies for the CSI-2 bus on that
particular system; you should check that the register lists are valid for
that frequency.
...
> > > + imx214->pixel_rate = v4l2_ctrl_new_std(&imx214->ctrls, NULL,
> > > + V4L2_CID_PIXEL_RATE, 0,
> > > + IMX214_DEFAULT_PIXEL_RATE, 1,
> > > + IMX214_DEFAULT_PIXEL_RATE);
> > > + imx214->link_freq = v4l2_ctrl_new_int_menu(&imx214->ctrls, NULL,
> > > + V4L2_CID_LINK_FREQ,
> > > + ARRAY_SIZE(link_freq) - 1,
> > > + 0, link_freq);
> >
> > Do I understand this correctly that the driver does not support setting
> > e.g. exposure time or gain? Those are very basic features...
>
>
> Yep :), this is just a first step. I do not have the register set from
> the device :(. So I am reverse engineering a lot of things.
> I will add more controls as I am done with them.
Looking at the registers you have in the register list, the sensor's
registers appear similar to those used by the smiapp driver (the old SMIA
standard).
I'd guess the same register would work for setting the exposure time. I'm
less certain about the limits though.
>
> >
> > You'll also need to ensure the s_ctrl() callback works without s_power()
> > being called. My suggestion is to switch to PM runtime; see e.g. the ov1385
> > driver in the current media tree master.
>
>
> There is one limitation with this chip on the dragonboard. I2c only
> works if the camss is ON. Therefore whatever s_ctrl needs to be
> cached and written at streamon.
That's something that doesn't belong to this driver. It's the I²C adapter
driver / camss issue, and not necessarily related to drivers only. Is the
I²C controller part of the camss btw.?
--
Regards,
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] [media] imx214: Add imx214 camera sensor driver
2018-09-24 20:32 ` Sakari Ailus
@ 2018-09-26 5:26 ` Ricardo Ribalda Delgado
2018-09-26 6:06 ` Ricardo Ribalda Delgado
0 siblings, 1 reply; 8+ messages in thread
From: Ricardo Ribalda Delgado @ 2018-09-26 5:26 UTC (permalink / raw)
To: Sakari Ailus
Cc: Hans Verkuil, Laurent Pinchart, Mauro Carvalho Chehab, LKML, linux-media
Hello Sakari
On Mon, Sep 24, 2018 at 10:32 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Ricardo,
>
> On Fri, Sep 21, 2018 at 12:15:55PM +0200, Ricardo Ribalda Delgado wrote:
> ...
> > > > +static struct reg_8 mode_1920x1080[];
> > > > +static struct reg_8 mode_4096x2304[];
> > >
> > > Const. Could you rearrange the bits to avoid the forward declarations?
> > Const done, but I prefer to keep the forward declaration. Otherwise
> > the long tables will "hide" the mode declaration.
>
> Well, I guess the long tables do "hide" a bunch of other stuff, too. :-)
> But... I agree there's no trivial way around those tables either.
>
> It appears I'm not the only one who's commented on the matter of the
> forward declaration.
Ok, I will change it, Eppur si muove ;)
>
> ...
>
> > > > +static int imx214_probe(struct i2c_client *client)
> > > > +{
> > > > + struct device *dev = &client->dev;
> > > > + struct imx214 *imx214;
> > > > + struct fwnode_handle *endpoint;
> > > > + int ret;
> > > > + static const s64 link_freq[] = {
> > > > + (IMX214_DEFAULT_PIXEL_RATE * 10LL) / 8,
> > >
> > > You should check the link frequency matches with that from the firmware.
> >
> > I am not sure what you mean here sorry.
>
> The system firmware holds safe frequencies for the CSI-2 bus on that
> particular system; you should check that the register lists are valid for
> that frequency.
>
> ...
>
> > > > + imx214->pixel_rate = v4l2_ctrl_new_std(&imx214->ctrls, NULL,
> > > > + V4L2_CID_PIXEL_RATE, 0,
> > > > + IMX214_DEFAULT_PIXEL_RATE, 1,
> > > > + IMX214_DEFAULT_PIXEL_RATE);
> > > > + imx214->link_freq = v4l2_ctrl_new_int_menu(&imx214->ctrls, NULL,
> > > > + V4L2_CID_LINK_FREQ,
> > > > + ARRAY_SIZE(link_freq) - 1,
> > > > + 0, link_freq);
> > >
> > > Do I understand this correctly that the driver does not support setting
> > > e.g. exposure time or gain? Those are very basic features...
> >
> >
> > Yep :), this is just a first step. I do not have the register set from
> > the device :(. So I am reverse engineering a lot of things.
> > I will add more controls as I am done with them.
>
> Looking at the registers you have in the register list, the sensor's
> registers appear similar to those used by the smiapp driver (the old SMIA
> standard).
>
> I'd guess the same register would work for setting the exposure time. I'm
> less certain about the limits though.
Thanks for the pointer! I will try this out and probably add it as a patch.
>
> >
> > >
> > > You'll also need to ensure the s_ctrl() callback works without s_power()
> > > being called. My suggestion is to switch to PM runtime; see e.g. the ov1385
> > > driver in the current media tree master.
> >
> >
> > There is one limitation with this chip on the dragonboard. I2c only
> > works if the camss is ON. Therefore whatever s_ctrl needs to be
> > cached and written at streamon.
>
> That's something that doesn't belong to this driver. It's the I涎 adapter
> driver / camss issue, and not necessarily related to drivers only. Is the
> I涎 controller part of the camss btw.?
I am with you here. The i2c controller is a different driver but is
integrated with camss. Checkout
https://patchwork.kernel.org/patch/10569961/ I am interacting with
Todor and Vinod to enable the i2c port indepently with camss. At least
now it does not kill the port after an i2c timeout :)
Will fix the fwd declaration and the csi-2 register. Then I wll upload
it to my github, try it on real hw next monday and send back to the
mailing list
Thanks!
>
> --
> Regards,
>
> Sakari Ailus
> sakari.ailus@linux.intel.com
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] [media] imx214: Add imx214 camera sensor driver
2018-09-26 5:26 ` Ricardo Ribalda Delgado
@ 2018-09-26 6:06 ` Ricardo Ribalda Delgado
2018-09-26 7:54 ` Sakari Ailus
0 siblings, 1 reply; 8+ messages in thread
From: Ricardo Ribalda Delgado @ 2018-09-26 6:06 UTC (permalink / raw)
To: Sakari Ailus
Cc: Hans Verkuil, Laurent Pinchart, Mauro Carvalho Chehab, LKML, linux-media
Hi Sakari
On Wed, Sep 26, 2018 at 7:26 AM Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
>
> Hello Sakari
> On Mon, Sep 24, 2018 at 10:32 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > Hi Ricardo,
> >
> > On Fri, Sep 21, 2018 at 12:15:55PM +0200, Ricardo Ribalda Delgado wrote:
> > ...
> > > > > +static struct reg_8 mode_1920x1080[];
> > > > > +static struct reg_8 mode_4096x2304[];
> > > >
> > > > Const. Could you rearrange the bits to avoid the forward declarations?
> > > Const done, but I prefer to keep the forward declaration. Otherwise
> > > the long tables will "hide" the mode declaration.
> >
> > Well, I guess the long tables do "hide" a bunch of other stuff, too. :-)
> > But... I agree there's no trivial way around those tables either.
> >
> > It appears I'm not the only one who's commented on the matter of the
> > forward declaration.
>
> Ok, I will change it, Eppur si muove ;)
> >
> > ...
> >
> > > > > +static int imx214_probe(struct i2c_client *client)
> > > > > +{
> > > > > + struct device *dev = &client->dev;
> > > > > + struct imx214 *imx214;
> > > > > + struct fwnode_handle *endpoint;
> > > > > + int ret;
> > > > > + static const s64 link_freq[] = {
> > > > > + (IMX214_DEFAULT_PIXEL_RATE * 10LL) / 8,
> > > >
> > > > You should check the link frequency matches with that from the firmware.
> > >
> > > I am not sure what you mean here sorry.
> >
> > The system firmware holds safe frequencies for the CSI-2 bus on that
> > particular system; you should check that the register lists are valid for
> > that frequency.
> >
Can you point me to a driver that does this? Just for the example. Thanks
> > ...
> >
> > > > > + imx214->pixel_rate = v4l2_ctrl_new_std(&imx214->ctrls, NULL,
> > > > > + V4L2_CID_PIXEL_RATE, 0,
> > > > > + IMX214_DEFAULT_PIXEL_RATE, 1,
> > > > > + IMX214_DEFAULT_PIXEL_RATE);
> > > > > + imx214->link_freq = v4l2_ctrl_new_int_menu(&imx214->ctrls, NULL,
> > > > > + V4L2_CID_LINK_FREQ,
> > > > > + ARRAY_SIZE(link_freq) - 1,
> > > > > + 0, link_freq);
> > > >
> > > > Do I understand this correctly that the driver does not support setting
> > > > e.g. exposure time or gain? Those are very basic features...
> > >
> > >
> > > Yep :), this is just a first step. I do not have the register set from
> > > the device :(. So I am reverse engineering a lot of things.
> > > I will add more controls as I am done with them.
> >
> > Looking at the registers you have in the register list, the sensor's
> > registers appear similar to those used by the smiapp driver (the old SMIA
> > standard).
> >
> > I'd guess the same register would work for setting the exposure time. I'm
> > less certain about the limits though.
>
> Thanks for the pointer! I will try this out and probably add it as a patch.
>
> >
> > >
> > > >
> > > > You'll also need to ensure the s_ctrl() callback works without s_power()
> > > > being called. My suggestion is to switch to PM runtime; see e.g. the ov1385
> > > > driver in the current media tree master.
> > >
> > >
> > > There is one limitation with this chip on the dragonboard. I2c only
> > > works if the camss is ON. Therefore whatever s_ctrl needs to be
> > > cached and written at streamon.
> >
> > That's something that doesn't belong to this driver. It's the I涎 adapter
> > driver / camss issue, and not necessarily related to drivers only. Is the
> > I涎 controller part of the camss btw.?
>
> I am with you here. The i2c controller is a different driver but is
> integrated with camss. Checkout
> https://patchwork.kernel.org/patch/10569961/ I am interacting with
> Todor and Vinod to enable the i2c port indepently with camss. At least
> now it does not kill the port after an i2c timeout :)
>
> Will fix the fwd declaration and the csi-2 register. Then I wll upload
> it to my github, try it on real hw next monday and send back to the
> mailing list
>
> Thanks!
>
> >
> > --
> > Regards,
> >
> > Sakari Ailus
> > sakari.ailus@linux.intel.com
>
>
>
> --
> Ricardo Ribalda
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] [media] imx214: Add imx214 camera sensor driver
2018-09-26 6:06 ` Ricardo Ribalda Delgado
@ 2018-09-26 7:54 ` Sakari Ailus
0 siblings, 0 replies; 8+ messages in thread
From: Sakari Ailus @ 2018-09-26 7:54 UTC (permalink / raw)
To: Ricardo Ribalda Delgado
Cc: Hans Verkuil, Laurent Pinchart, Mauro Carvalho Chehab, LKML, linux-media
Hi, Ricardo!
On Wed, Sep 26, 2018 at 08:06:55AM +0200, Ricardo Ribalda Delgado wrote:
> Hi Sakari
> On Wed, Sep 26, 2018 at 7:26 AM Ricardo Ribalda Delgado
> <ricardo.ribalda@gmail.com> wrote:
> >
> > Hello Sakari
> > On Mon, Sep 24, 2018 at 10:32 PM Sakari Ailus
> > <sakari.ailus@linux.intel.com> wrote:
> > >
> > > Hi Ricardo,
> > >
> > > On Fri, Sep 21, 2018 at 12:15:55PM +0200, Ricardo Ribalda Delgado wrote:
> > > ...
> > > > > > +static struct reg_8 mode_1920x1080[];
> > > > > > +static struct reg_8 mode_4096x2304[];
> > > > >
> > > > > Const. Could you rearrange the bits to avoid the forward declarations?
> > > > Const done, but I prefer to keep the forward declaration. Otherwise
> > > > the long tables will "hide" the mode declaration.
> > >
> > > Well, I guess the long tables do "hide" a bunch of other stuff, too. :-)
> > > But... I agree there's no trivial way around those tables either.
> > >
> > > It appears I'm not the only one who's commented on the matter of the
> > > forward declaration.
> >
> > Ok, I will change it, Eppur si muove ;)
> > >
> > > ...
> > >
> > > > > > +static int imx214_probe(struct i2c_client *client)
> > > > > > +{
> > > > > > + struct device *dev = &client->dev;
> > > > > > + struct imx214 *imx214;
> > > > > > + struct fwnode_handle *endpoint;
> > > > > > + int ret;
> > > > > > + static const s64 link_freq[] = {
> > > > > > + (IMX214_DEFAULT_PIXEL_RATE * 10LL) / 8,
> > > > >
> > > > > You should check the link frequency matches with that from the firmware.
> > > >
> > > > I am not sure what you mean here sorry.
> > >
> > > The system firmware holds safe frequencies for the CSI-2 bus on that
> > > particular system; you should check that the register lists are valid for
> > > that frequency.
> > >
>
> Can you point me to a driver that does this? Just for the example. Thanks
The imx319 driver:
<URL:https://patchwork.linuxtv.org/patch/52233/>
> > I am with you here. The i2c controller is a different driver but is
> > integrated with camss. Checkout
> > https://patchwork.kernel.org/patch/10569961/ I am interacting with
> > Todor and Vinod to enable the i2c port indepently with camss. At least
> > now it does not kill the port after an i2c timeout :)
Nice!!
--
Cheers,
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-09-26 7:54 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-21 8:54 [PATCH 1/2] [media] imx214: Add imx214 camera sensor driver Ricardo Ribalda Delgado
2018-09-21 8:54 ` [PATCH 2/2] [media] imx214: device tree binding Ricardo Ribalda Delgado
2018-09-21 9:28 ` [PATCH 1/2] [media] imx214: Add imx214 camera sensor driver Sakari Ailus
2018-09-21 10:15 ` Ricardo Ribalda Delgado
2018-09-24 20:32 ` Sakari Ailus
2018-09-26 5:26 ` Ricardo Ribalda Delgado
2018-09-26 6:06 ` Ricardo Ribalda Delgado
2018-09-26 7:54 ` Sakari Ailus
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).