* [PATCH v2 1/2] media: i2c: Add driver for the Analog Devices ADDI9036 ToF front-end
@ 2020-10-02 13:35 Bogdan Togorean
2020-10-02 13:35 ` [PATCH v2 2/2] media: dt-bindings: media: i2c: Add bindings for ADDI9036 Bogdan Togorean
2020-10-07 11:18 ` [PATCH v2 1/2] media: i2c: Add driver for the Analog Devices ADDI9036 ToF front-end Sakari Ailus
0 siblings, 2 replies; 8+ messages in thread
From: Bogdan Togorean @ 2020-10-02 13:35 UTC (permalink / raw)
To: linux-media
Cc: Bogdan Togorean, Mauro Carvalho Chehab, Rob Herring,
David S. Miller, Sakari Ailus, Jacopo Mondi, Laurent Pinchart,
Kieran Bingham, Bingbu Cao, Shawn Tu, Dongchun Zhu,
Dave Stevenson, Manivannan Sadhasivam, devicetree, linux-kernel
The ADDI9036 is a complete, 45 MHz, front-end solution for charge
coupled device (CCD) time of flight (TOF) imaging applications.
It has 2-lane MIPI CSI-2 RAW12 data output and i2c control interface.
The programming of calibration and firmware is performed by driver
using Linux Firmware API.
Signed-off-by: Bogdan Togorean <bogdan.togorean@analog.com>
---
v2: implemented reading of FW using Linux Firmware API
removed custom controls for FW programming
added custom control to select operating mode
implemented V1 review remarks
cleaned includes list
---
MAINTAINERS | 10 +
drivers/media/i2c/Kconfig | 14 +
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/addi9036.c | 754 +++++++++++++++++++++++++++++
include/uapi/linux/v4l2-controls.h | 6 +
5 files changed, 785 insertions(+)
create mode 100644 drivers/media/i2c/addi9036.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 0d0862b19ce5..4e3878e6c0ba 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -477,6 +477,16 @@ W: http://wiki.analog.com/AD7879
W: http://ez.analog.com/community/linux-device-drivers
F: drivers/input/touchscreen/ad7879.c
+ADDI9036 TOF FRONTEND DRIVER
+M: Bogdan Togorean <bogdan.togorean@analog.com>
+L: linux-media@vger.kernel.org
+S: Supported
+W: https://www.analog.com/en/products/addi9036.html
+W: http://ez.analog.com/community/linux-device-drivers
+T: git git://linuxtv.org/media_tree.git
+F: Documentation/devicetree/bindings/media/i2c/adi,addi9036.yaml
+F: drivers/media/i2c/addi9036.c
+
ADDRESS SPACE LAYOUT RANDOMIZATION (ASLR)
M: Jiri Kosina <jikos@kernel.org>
S: Maintained
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index c7ba76fee599..087dd307505d 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -725,6 +725,20 @@ config VIDEO_APTINA_PLL
config VIDEO_SMIAPP_PLL
tristate
+config VIDEO_ADDI9036
+ tristate "Analog Devices ADDI9036 ToF front-end support"
+ depends on I2C && VIDEO_V4L2
+ select MEDIA_CONTROLLER
+ select VIDEO_V4L2_SUBDEV_API
+ select V4L2_FWNODE
+ select REGMAP_I2C
+ help
+ This is a Video4Linux2 driver for Analog Devices ADDI9036
+ Time of Flight front-end.
+
+ To compile this driver as a module, choose M here: the
+ module will be called addi9036.
+
config VIDEO_HI556
tristate "Hynix Hi-556 sensor support"
depends on I2C && VIDEO_V4L2
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index f0a77473979d..631a7c7612ca 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_VIDEO_AK7375) += ak7375.o
obj-$(CONFIG_VIDEO_DW9714) += dw9714.o
obj-$(CONFIG_VIDEO_DW9768) += dw9768.o
obj-$(CONFIG_VIDEO_DW9807_VCM) += dw9807-vcm.o
+obj-$(CONFIG_VIDEO_ADDI9036) += addi9036.o
obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
obj-$(CONFIG_VIDEO_ADV7180) += adv7180.o
diff --git a/drivers/media/i2c/addi9036.c b/drivers/media/i2c/addi9036.c
new file mode 100644
index 000000000000..e38e70afd23d
--- /dev/null
+++ b/drivers/media/i2c/addi9036.c
@@ -0,0 +1,754 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for the Analog Devices ADDI9036 ToF camera sensor.
+ *
+ * Copyright (C) 2019-2020 Analog Devices, All Rights Reserved.
+ *
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/firmware.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-subdev.h>
+
+#define FW_FILE_NAME "adi/addi9036-fw.bin"
+#define ADDI_MAGIC "ADDI9036"
+
+struct addi9036_mode_info {
+ u32 width;
+ u32 height;
+ u32 pixel_rate;
+ u32 link_freq_idx;
+};
+
+struct addi9036_mode_fw_block {
+ const struct reg_sequence *mode_regs;
+ ssize_t regs_count;
+};
+
+struct addi9036_fw_header {
+ unsigned char magic[8];
+ __le32 modes_nr;
+} __packed;
+
+struct addi9036_mode_block {
+ __le32 mode_id;
+ __le32 size_bytes;
+ __le16 data[];
+} __packed;
+
+struct addi9036 {
+ struct regmap *regmap;
+ struct device *dev;
+ struct v4l2_subdev sd;
+ struct media_pad pad;
+ struct v4l2_fwnode_endpoint ep;
+ struct v4l2_mbus_framefmt fmt;
+ struct v4l2_rect crop;
+
+ const struct addi9036_mode_info *current_mode;
+
+ struct v4l2_ctrl_handler ctrls;
+ struct v4l2_ctrl *pixel_rate;
+ struct v4l2_ctrl *link_freq;
+ /* custom controls */
+ struct v4l2_ctrl *set_operating_range;
+
+ /* lock to protect power state */
+ struct mutex power_lock;
+ int power_count;
+ bool streaming;
+
+ struct gpio_desc *rst_gpio;
+
+ /* firmware blocks for each operating mode */
+ struct addi9036_mode_fw_block *mode_fw_blocks;
+ u8 curr_operating_mode;
+
+ const struct firmware *fw;
+};
+
+static inline struct addi9036 *to_addi9036(struct v4l2_subdev *sd)
+{
+ return container_of(sd, struct addi9036, sd);
+}
+
+#define V4L2_CID_ADDI9036_OPERATING_MODE (V4L2_CID_USER_ADDI9036_BASE + 0)
+
+static const struct reg_sequence addi9036_powerup_setting[] = {
+ { 0xc4c0, 0x001c },
+ { 0xc4c3, 0x001c },
+ { 0xc4d7, 0x0000 },
+ { 0xc4d5, 0x0002 },
+ { 0xc4da, 0x0001 },
+ { 0xc4f0, 0x0000 },
+ { 0xc427, 0x0003 },
+ { 0xc427, 0x0001 },
+ { 0xc427, 0x0000 },
+ { 0xc426, 0x0030 },
+ { 0xc426, 0x0010 },
+ { 0xc426, 0x0000 },
+ { 0xc423, 0x0080 },
+ { 0xc431, 0x0080 },
+ { 0x4001, 0x0007 },
+ { 0x7c22, 0x0004 }
+};
+
+static const struct reg_sequence addi9036_powerdown_setting[] = {
+ { 0xc022, 0x0001 },
+ { 0x4001, 0x0006 },
+ { 0x7c22, 0x0004 },
+ { 0xc431, 0x0082 },
+ { 0xc423, 0x0000 },
+ { 0xc426, 0x0020 },
+ { 0xc427, 0x0002 },
+ { 0xc4c0, 0x003c },
+ { 0xc4c3, 0x003c },
+ { 0xc4d5, 0x0003 },
+ { 0xc4da, 0x0000 },
+ { 0xc4d7, 0x0001 },
+ { 0xc4f0, 0x0001 }
+};
+
+static const s64 link_freq_tbl[] = {
+ 110529000,
+ 221184000,
+};
+
+/* Elements of the structure must be ordered ascending by width & height */
+static const struct addi9036_mode_info addi9036_mode_info_data[] = {
+ {
+ .width = 640,
+ .height = 480,
+ .pixel_rate = 36864000,
+ .link_freq_idx = 0 /* an index in link_freq_tbl[] */
+ },
+ {
+ .width = 640,
+ .height = 960,
+ .pixel_rate = 73728000,
+ .link_freq_idx = 1 /* an index in link_freq_tbl[] */
+ },
+};
+
+static bool addi9306_readable_register(struct device *dev, unsigned int reg)
+{
+ if (((reg >= 0x4000) && (reg <= 0x6fff)) ||
+ ((reg >= 0x7c00) && (reg <= 0x7fff)) ||
+ ((reg >= 0xc000) && (reg <= 0xc200)) ||
+ ((reg >= 0xc300) && (reg <= 0xc6bf)))
+ return true;
+ else
+ return false;
+}
+
+static bool addi9306_writeable_register(struct device *dev, unsigned int reg)
+{
+ if (((reg >= 0x4000) && (reg <= 0x6fff)) ||
+ ((reg >= 0x7c00) && (reg <= 0x7fff)) ||
+ ((reg >= 0xc000) && (reg <= 0xc113)) ||
+ ((reg >= 0xc300) && (reg <= 0xc7ff)))
+ return true;
+ else
+ return false;
+}
+
+static const struct regmap_config addi9036_i2c_regmap_config = {
+ .reg_bits = 16,
+ .val_bits = 16,
+ .max_register = 0xc7ff,
+ .writeable_reg = addi9306_writeable_register,
+ .readable_reg = addi9306_readable_register,
+ .cache_type = REGCACHE_NONE,
+};
+
+static int addi9036_set_power_on(struct addi9036 *addi9036)
+{
+ int ret;
+
+ if (addi9036->rst_gpio)
+ gpiod_set_value_cansleep(addi9036->rst_gpio, 0);
+
+ ret = regmap_register_patch(addi9036->regmap, addi9036_powerup_setting,
+ ARRAY_SIZE(addi9036_powerup_setting));
+ if (ret)
+ dev_err(addi9036->dev, "Could not set power up registers\n");
+
+ return ret;
+}
+
+static int addi9036_set_power_off(struct addi9036 *addi9036)
+{
+ int ret;
+
+ ret = regmap_register_patch(addi9036->regmap,
+ addi9036_powerdown_setting,
+ ARRAY_SIZE(addi9036_powerdown_setting));
+ if (ret)
+ dev_err(addi9036->dev, "could not set power down registers\n");
+
+ if (addi9036->rst_gpio)
+ gpiod_set_value_cansleep(addi9036->rst_gpio, 1);
+
+ return ret;
+}
+
+static int addi9036_s_power(struct v4l2_subdev *sd, int on)
+{
+ struct addi9036 *addi9036 = to_addi9036(sd);
+ int ret = 0;
+
+ dev_dbg(addi9036->dev, "s_power: %d\n", on);
+
+ mutex_lock(&addi9036->power_lock);
+
+ /* If the power count is modified from 0 to != 0 or from != 0 to 0,
+ * update the power state.
+ */
+ if (addi9036->power_count == !on) {
+ if (on)
+ ret = addi9036_set_power_on(addi9036);
+ else
+ ret = addi9036_set_power_off(addi9036);
+ }
+
+ if (!ret) {
+ /* Update the power count. */
+ addi9036->power_count += on ? 1 : -1;
+ WARN(addi9036->power_count < 0, "Unbalanced power count\n");
+ WARN(addi9036->power_count > 1, "Duplicated s_power call\n");
+ }
+
+ mutex_unlock(&addi9036->power_lock);
+
+ return ret;
+}
+
+#ifdef CONFIG_VIDEO_ADV_DEBUG
+static int addi9036_g_register(struct v4l2_subdev *sd,
+ struct v4l2_dbg_register *reg)
+{
+ struct addi9036 *addi9036 = to_addi9036(sd);
+ unsigned int read_val;
+ int ret;
+
+ reg->size = 2;
+ ret = regmap_read(addi9036->regmap, reg->reg, &read_val);
+ reg->val = read_val;
+
+ return ret;
+}
+
+static int addi9036_s_register(struct v4l2_subdev *sd,
+ const struct v4l2_dbg_register *reg)
+{
+ struct addi9036 *addi9036 = to_addi9036(sd);
+
+ return regmap_write(addi9036->regmap, reg->reg, reg->val);
+}
+#endif
+
+static int addi9036_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+ struct addi9036 *addi9036 = container_of(ctrl->handler,
+ struct addi9036, ctrls);
+ int ret = 0;
+
+ switch (ctrl->id) {
+ case V4L2_CID_ADDI9036_OPERATING_MODE:
+ addi9036->curr_operating_mode = ctrl->val;
+ break;
+ case V4L2_CID_PIXEL_RATE:
+ case V4L2_CID_LINK_FREQ:
+ break;
+ default:
+ dev_err(addi9036->dev, "%s > Unhandled: %x param=%x\n",
+ __func__, ctrl->id, ctrl->val);
+ ret = -EINVAL;
+ break;
+ }
+
+ return ret;
+}
+
+static const struct v4l2_ctrl_ops addi9036_ctrl_ops = {
+ .s_ctrl = addi9036_s_ctrl,
+};
+
+static const struct v4l2_ctrl_config addi9036_ctrl_operating_mode = {
+ .ops = &addi9036_ctrl_ops,
+ .id = V4L2_CID_ADDI9036_OPERATING_MODE,
+ .name = "Operating Mode",
+ .type = V4L2_CTRL_TYPE_INTEGER,
+ .def = 0,
+ .min = 0,
+ .max = 1,
+ .step = 1,
+};
+
+static int addi9036_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 = MEDIA_BUS_FMT_SBGGR12_1X12;
+
+ return 0;
+}
+
+static int addi9036_enum_frame_size(struct v4l2_subdev *subdev,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_frame_size_enum *fse)
+{
+ if (fse->code != MEDIA_BUS_FMT_SBGGR12_1X12)
+ return -EINVAL;
+
+ if (fse->index >= ARRAY_SIZE(addi9036_mode_info_data))
+ return -EINVAL;
+
+ fse->min_width = addi9036_mode_info_data[fse->index].width;
+ fse->max_width = addi9036_mode_info_data[fse->index].width;
+ fse->min_height = addi9036_mode_info_data[fse->index].height;
+ fse->max_height = addi9036_mode_info_data[fse->index].height;
+
+ return 0;
+}
+
+static struct v4l2_mbus_framefmt *
+addi9036_get_pad_format(struct addi9036 *addi9036,
+ 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(&addi9036->sd, cfg, pad);
+ case V4L2_SUBDEV_FORMAT_ACTIVE:
+ return &addi9036->fmt;
+ default:
+ return NULL;
+ }
+}
+
+static int addi9036_get_format(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_format *format)
+{
+ struct addi9036 *addi9036 = to_addi9036(sd);
+ struct v4l2_mbus_framefmt *pad_format;
+
+ pad_format = addi9036_get_pad_format(addi9036, cfg, format->pad,
+ format->which);
+
+ if (!pad_format)
+ return -EINVAL;
+
+ format->format = *pad_format;
+
+ return 0;
+}
+
+static struct v4l2_rect *
+addi9036_get_pad_crop(struct addi9036 *addi9036,
+ 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(&addi9036->sd, cfg, pad);
+ case V4L2_SUBDEV_FORMAT_ACTIVE:
+ return &addi9036->crop;
+ default:
+ return NULL;
+ }
+}
+
+static int addi9036_set_format(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_format *format)
+{
+ struct addi9036 *addi9036 = to_addi9036(sd);
+ struct v4l2_mbus_framefmt *framefmt;
+ struct v4l2_rect *crop;
+ const struct addi9036_mode_info *new_mode;
+ int ret;
+
+ dev_dbg(addi9036->dev, "set_fmt: %x %dx%d %d\n",
+ format->format.code, format->format.width,
+ format->format.height, format->which);
+
+ crop = addi9036_get_pad_crop(addi9036, cfg, format->pad,
+ format->which);
+
+ if (!crop)
+ return -EINVAL;
+
+ new_mode = v4l2_find_nearest_size(addi9036_mode_info_data,
+ ARRAY_SIZE(addi9036_mode_info_data),
+ width, height, format->format.width,
+ format->format.height);
+ crop->width = new_mode->width;
+ crop->height = new_mode->height;
+
+ if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
+ ret = v4l2_ctrl_s_ctrl_int64(addi9036->pixel_rate,
+ new_mode->pixel_rate);
+ if (ret < 0)
+ return ret;
+
+ ret = v4l2_ctrl_s_ctrl(addi9036->link_freq,
+ new_mode->link_freq_idx);
+ if (ret < 0)
+ return ret;
+
+ addi9036->current_mode = new_mode;
+ }
+
+ framefmt = addi9036_get_pad_format(addi9036, cfg, format->pad,
+ format->which);
+ framefmt->width = crop->width;
+ framefmt->height = crop->height;
+ framefmt->code = MEDIA_BUS_FMT_SBGGR12_1X12;
+ framefmt->field = V4L2_FIELD_NONE;
+ framefmt->colorspace = V4L2_COLORSPACE_SRGB;
+
+ format->format = *framefmt;
+
+ return 0;
+}
+
+static int addi9036_entity_init_cfg(struct v4l2_subdev *subdev,
+ struct v4l2_subdev_pad_config *cfg)
+{
+ struct v4l2_subdev_format fmt = { 0 };
+
+ fmt.which = cfg ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
+ fmt.format.width = addi9036_mode_info_data[1].width;
+ fmt.format.height = addi9036_mode_info_data[1].height;
+
+ addi9036_set_format(subdev, cfg, &fmt);
+
+ return 0;
+}
+
+static int addi9036_get_selection(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_selection *sel)
+{
+ struct addi9036 *addi9036 = to_addi9036(sd);
+
+ if (sel->target != V4L2_SEL_TGT_CROP)
+ return -EINVAL;
+
+ sel->r = *addi9036_get_pad_crop(addi9036, cfg, sel->pad, sel->which);
+
+ return 0;
+}
+
+static int addi9036_s_stream(struct v4l2_subdev *subdev, int enable)
+{
+ struct addi9036 *addi9036 = to_addi9036(subdev);
+ uint8_t mode = addi9036->curr_operating_mode;
+ int ret = 0;
+
+ dev_dbg(addi9036->dev, "s_stream: %d\n", enable);
+
+ if (addi9036->streaming == enable)
+ return 0;
+
+ if (enable) {
+ if (addi9036->mode_fw_blocks[mode].mode_regs == NULL) {
+ dev_err(addi9036->dev, "Selected mode has no data\n");
+ return -EINVAL;
+ }
+
+ dev_dbg(addi9036->dev, "Applying mode: %u\n", mode);
+ ret = regmap_multi_reg_write(addi9036->regmap,
+ addi9036->mode_fw_blocks[mode].mode_regs,
+ addi9036->mode_fw_blocks[mode].regs_count);
+
+ dev_dbg(addi9036->dev, "Writen %lu registers\n",
+ addi9036->mode_fw_blocks[mode].regs_count);
+ }
+
+ addi9036->streaming = enable;
+ return ret;
+}
+
+static const struct v4l2_subdev_core_ops addi9036_core_ops = {
+ .s_power = addi9036_s_power,
+#ifdef CONFIG_VIDEO_ADV_DEBUG
+ .g_register = addi9036_g_register,
+ .s_register = addi9036_s_register,
+#endif
+};
+
+static const struct v4l2_subdev_video_ops addi9036_video_ops = {
+ .s_stream = addi9036_s_stream,
+};
+
+static const struct v4l2_subdev_pad_ops addi9036_subdev_pad_ops = {
+ .init_cfg = addi9036_entity_init_cfg,
+ .enum_mbus_code = addi9036_enum_mbus_code,
+ .enum_frame_size = addi9036_enum_frame_size,
+ .get_fmt = addi9036_get_format,
+ .set_fmt = addi9036_set_format,
+ .get_selection = addi9036_get_selection,
+};
+
+static const struct v4l2_subdev_ops addi9036_subdev_ops = {
+ .core = &addi9036_core_ops,
+ .video = &addi9036_video_ops,
+ .pad = &addi9036_subdev_pad_ops,
+};
+
+static int addi9036_g_modes_from_firmware(struct v4l2_subdev *sd)
+{
+ struct addi9036 *addi9036 = to_addi9036(sd);
+ const struct firmware *fw = addi9036->fw;
+ const struct addi9036_fw_header *fw_head;
+ const struct addi9036_mode_block *mode_block;
+ unsigned int reg_nr, chunk_len, pos, modes_nr, i, j, k;
+ struct reg_sequence *reg_seq;
+
+ /*
+ * Reject too small or unreasonable large files.
+ */
+
+ if (fw->size < sizeof(struct addi9036_fw_header) ||
+ fw->size >= 0x4000000) {
+ dev_err(addi9036->dev, "FW loading failed: Invalid size\n");
+ return -EINVAL;
+ }
+
+ fw_head = (struct addi9036_fw_header *)fw->data;
+
+ if (memcmp(fw_head->magic, ADDI_MAGIC, ARRAY_SIZE(fw_head->magic))) {
+ dev_err(addi9036->dev, "FW loading failed: Invalid magic\n");
+ return -EINVAL;
+ }
+
+ modes_nr = le32_to_cpu(fw_head->modes_nr);
+
+ if (modes_nr == 0) {
+ dev_err(addi9036->dev, "FW should contain at least 1 mode.\n");
+ return -EINVAL;
+ }
+
+ __v4l2_ctrl_modify_range(addi9036->set_operating_range,
+ addi9036->set_operating_range->minimum,
+ modes_nr - 1, 1, 0);
+
+ addi9036->mode_fw_blocks = devm_kzalloc(addi9036->dev,
+ sizeof(struct addi9036_mode_fw_block) * modes_nr,
+ GFP_KERNEL);
+ if (!addi9036->mode_fw_blocks)
+ return -ENOMEM;
+
+ pos = sizeof(struct addi9036_fw_header);
+
+ for (i = 0; i < modes_nr; i++) {
+ mode_block = (struct addi9036_mode_block *)(fw->data + pos);
+
+ chunk_len = le32_to_cpu(mode_block->size_bytes);
+ reg_nr = chunk_len / sizeof(uint16_t) / 2;
+
+ reg_seq = devm_kzalloc(addi9036->dev,
+ sizeof(struct reg_sequence) * reg_nr,
+ GFP_KERNEL);
+ if (!reg_seq)
+ return -ENOMEM;
+
+ k = 0;
+ for (j = 0; j < reg_nr * 2; j += 2) {
+ reg_seq[k].reg = le16_to_cpu(mode_block->data[j]);
+ reg_seq[k].def = le16_to_cpu(mode_block->data[j + 1]);
+ k++;
+ }
+
+ addi9036->mode_fw_blocks[i].mode_regs = reg_seq;
+ addi9036->mode_fw_blocks[i].regs_count = reg_nr;
+
+ pos += chunk_len + sizeof(struct addi9036_mode_block);
+ }
+ return 0;
+}
+
+static int addi9036_mode_firmware_load(struct v4l2_subdev *sd)
+{
+ struct addi9036 *addi9036 = to_addi9036(sd);
+ int ret;
+
+ ret = request_firmware(&addi9036->fw, FW_FILE_NAME, addi9036->dev);
+ if (ret < 0) {
+ dev_err(addi9036->dev, "FW request failed\n");
+ return ret;
+ }
+
+ ret = addi9036_g_modes_from_firmware(sd);
+
+ release_firmware(addi9036->fw);
+ if (ret < 0) {
+ dev_err(addi9036->dev, "FW parsing failed\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int addi9036_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct fwnode_handle *endpoint;
+ struct addi9036 *addi9036;
+ int ret;
+
+ dev_dbg(dev, "%s: i2c addr = 0x%x\n", __func__, client->addr);
+
+ addi9036 = devm_kzalloc(dev, sizeof(struct addi9036), GFP_KERNEL);
+ if (!addi9036)
+ return -ENOMEM;
+
+ addi9036->dev = dev;
+
+ addi9036->regmap = devm_regmap_init_i2c(client,
+ &addi9036_i2c_regmap_config);
+ if (IS_ERR(addi9036->regmap)) {
+ dev_err(dev, "Error initializing i2c regmap\n");
+ return PTR_ERR(addi9036->regmap);
+ }
+
+ mutex_init(&addi9036->power_lock);
+
+ endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL);
+ if (!endpoint) {
+ dev_err(dev, "endpoint node not found\n");
+ return -EINVAL;
+ }
+
+ ret = v4l2_fwnode_endpoint_parse(endpoint, &addi9036->ep);
+ if (ret < 0) {
+ dev_err(dev, "parsing endpoint node failed\n");
+ return ret;
+ }
+ fwnode_handle_put(endpoint);
+
+ if (addi9036->ep.bus_type != V4L2_MBUS_CSI2_DPHY) {
+ dev_err(dev, "invalid bus type, must be MIPI CSI2\n");
+ return -EINVAL;
+ }
+
+ addi9036->rst_gpio = gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(addi9036->rst_gpio))
+ dev_info(dev, "Unable to get \"reset\" gpio\n");
+
+ v4l2_ctrl_handler_init(&addi9036->ctrls, 3);
+
+ addi9036->pixel_rate = v4l2_ctrl_new_std(&addi9036->ctrls,
+ &addi9036_ctrl_ops,
+ V4L2_CID_PIXEL_RATE,
+ 1, INT_MAX, 1, 1);
+ addi9036->link_freq = v4l2_ctrl_new_int_menu(&addi9036->ctrls,
+ &addi9036_ctrl_ops,
+ V4L2_CID_LINK_FREQ,
+ ARRAY_SIZE(
+ link_freq_tbl) - 1,
+ 0, link_freq_tbl);
+ if (addi9036->link_freq)
+ addi9036->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+
+ addi9036->set_operating_range = v4l2_ctrl_new_custom(&addi9036->ctrls,
+ &addi9036_ctrl_operating_mode,
+ NULL);
+
+ ret = addi9036->ctrls.error;
+ if (ret) {
+ dev_err(dev, "%s: control initialization error %d\n",
+ __func__, ret);
+ goto free_ctrl;
+ }
+ addi9036->sd.ctrl_handler = &addi9036->ctrls;
+
+ v4l2_i2c_subdev_init(&addi9036->sd, client, &addi9036_subdev_ops);
+ addi9036->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+ addi9036->pad.flags = MEDIA_PAD_FL_SOURCE;
+ addi9036->sd.dev = &client->dev;
+ addi9036->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+
+ ret = media_entity_pads_init(&addi9036->sd.entity, 1, &addi9036->pad);
+ if (ret < 0) {
+ dev_err(dev, "could not register media entity\n");
+ goto free_ctrl;
+ }
+
+ ret = addi9036_mode_firmware_load(&addi9036->sd);
+ if (ret < 0)
+ return ret;
+
+ addi9036_entity_init_cfg(&addi9036->sd, NULL);
+
+ ret = v4l2_async_register_subdev(&addi9036->sd);
+ if (ret < 0) {
+ dev_err(dev, "could not register v4l2 device\n");
+ goto free_entity;
+ }
+
+ return 0;
+
+free_entity:
+ media_entity_cleanup(&addi9036->sd.entity);
+free_ctrl:
+ v4l2_ctrl_handler_free(&addi9036->ctrls);
+ mutex_destroy(&addi9036->power_lock);
+
+ return ret;
+}
+
+static int addi9036_remove(struct i2c_client *client)
+{
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct addi9036 *addi9036 = to_addi9036(sd);
+
+ v4l2_async_unregister_subdev(&addi9036->sd);
+ media_entity_cleanup(&addi9036->sd.entity);
+ if (addi9036->rst_gpio)
+ gpiod_put(addi9036->rst_gpio);
+ v4l2_ctrl_handler_free(&addi9036->ctrls);
+ mutex_destroy(&addi9036->power_lock);
+
+ return 0;
+}
+
+static const struct i2c_device_id addi9036_id[] = {
+ { "addi9036", 0 },
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, addi9036_id);
+
+static const struct of_device_id addi9036_of_match[] = {
+ { .compatible = "adi,addi9036" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, addi9036_of_match);
+
+static struct i2c_driver addi9036_i2c_driver = {
+ .driver = {
+ .of_match_table = addi9036_of_match,
+ .name = "addi9036",
+ },
+ .probe_new = addi9036_probe,
+ .remove = addi9036_remove,
+ .id_table = addi9036_id,
+};
+
+module_i2c_driver(addi9036_i2c_driver);
+
+MODULE_DESCRIPTION("Analog Devices ADDI9036 Camera Driver");
+MODULE_AUTHOR("Bogdan Togorean");
+MODULE_LICENSE("GPL v2");
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 62271418c1be..f88b56479bc1 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -198,6 +198,12 @@ enum v4l2_colorfx {
*/
#define V4L2_CID_USER_ATMEL_ISC_BASE (V4L2_CID_USER_BASE + 0x10c0)
+/*
+ * The base for the addi9036 driver controls.
+ * We reserve 16 controls for this driver.
+ */
+#define V4L2_CID_USER_ADDI9036_BASE (V4L2_CID_USER_BASE + 0x10e0)
+
/* MPEG-class control IDs */
/* The MPEG controls are applicable to all codec controls
* and the 'MPEG' part of the define is historical */
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] media: dt-bindings: media: i2c: Add bindings for ADDI9036
2020-10-02 13:35 [PATCH v2 1/2] media: i2c: Add driver for the Analog Devices ADDI9036 ToF front-end Bogdan Togorean
@ 2020-10-02 13:35 ` Bogdan Togorean
2020-10-06 20:47 ` Rob Herring
2020-10-07 11:18 ` [PATCH v2 1/2] media: i2c: Add driver for the Analog Devices ADDI9036 ToF front-end Sakari Ailus
1 sibling, 1 reply; 8+ messages in thread
From: Bogdan Togorean @ 2020-10-02 13:35 UTC (permalink / raw)
To: linux-media
Cc: Bogdan Togorean, Mauro Carvalho Chehab, Rob Herring,
David S. Miller, Sakari Ailus, Jacopo Mondi, Laurent Pinchart,
Kieran Bingham, Bingbu Cao, Dave Stevenson, Shawn Tu,
Dongchun Zhu, devicetree, linux-kernel
Add YAML device tree bindings for Analog Devices Inc. ADDI9036 CCD TOF
front-end.
Signed-off-by: Bogdan Togorean <bogdan.togorean@analog.com>
---
v2: added reg property description
---
.../bindings/media/i2c/adi,addi9036.yaml | 76 +++++++++++++++++++
1 file changed, 76 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/adi,addi9036.yaml
diff --git a/Documentation/devicetree/bindings/media/i2c/adi,addi9036.yaml b/Documentation/devicetree/bindings/media/i2c/adi,addi9036.yaml
new file mode 100644
index 000000000000..7c4af704db98
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/adi,addi9036.yaml
@@ -0,0 +1,76 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/adi,addi9036.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices ADDI9036 VGA CCD Time of Flight Sensor
+
+maintainers:
+ - Bogdan Togorean <bogdan.togorean@analog.com>
+
+description: |-
+ The ADDI9036 is a complete, 45 MHz, front-end solution for charge coupled
+ device (CCD) time of flight (TOF) imaging applications. It is programmable
+ through I2C interface. Image data is sent through MIPI CSI-2 2 lanes and
+ can output two RAW12 packed data streams. One is IR and the other is Depth.
+ Each data stream is on a separate or same MIPI Virtual Channel, depending
+ on configuration and each have 640x480 resolution.
+
+properties:
+ compatible:
+ const: adi,addi9036
+
+ reg:
+ description: I2C device address
+ maxItems: 1
+
+ reset-gpios:
+ description: |-
+ Reference to the GPIO connected to the RST/SYNC pin, if any.
+ Must be released (set high) after all supplies are applied.
+
+ # See ../video-interfaces.txt for more details
+ port:
+ type: object
+ properties:
+ endpoint:
+ type: object
+ properties:
+ data-lanes:
+ description: |-
+ The sensor supports two-lane operation.
+ For two-lane operation the property must be set to <1 2>.
+ items:
+ - const: 1
+ - const: 2
+
+required:
+ - compatible
+ - reg
+ - port
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ addi9036: addi9036_tof@64 {
+ compatible = "adi,addi9036";
+ reg = <0x64>;
+
+ reset-gpios = <&gpio 41 1>;
+
+ port {
+ addi9036_ep: endpoint {
+ remote-endpoint = <&csi1_ep>;
+ data-lanes = <1 2>;
+ };
+ };
+ };
+ };
+
+...
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] media: dt-bindings: media: i2c: Add bindings for ADDI9036
2020-10-02 13:35 ` [PATCH v2 2/2] media: dt-bindings: media: i2c: Add bindings for ADDI9036 Bogdan Togorean
@ 2020-10-06 20:47 ` Rob Herring
2020-10-08 6:23 ` Togorean, Bogdan
0 siblings, 1 reply; 8+ messages in thread
From: Rob Herring @ 2020-10-06 20:47 UTC (permalink / raw)
To: Bogdan Togorean
Cc: linux-media, Mauro Carvalho Chehab, David S. Miller,
Sakari Ailus, Jacopo Mondi, Laurent Pinchart, Kieran Bingham,
Bingbu Cao, Dave Stevenson, Shawn Tu, Dongchun Zhu, devicetree,
linux-kernel
On Fri, Oct 02, 2020 at 04:35:17PM +0300, Bogdan Togorean wrote:
> Add YAML device tree bindings for Analog Devices Inc. ADDI9036 CCD TOF
> front-end.
>
> Signed-off-by: Bogdan Togorean <bogdan.togorean@analog.com>
> ---
> v2: added reg property description
> ---
> .../bindings/media/i2c/adi,addi9036.yaml | 76 +++++++++++++++++++
> 1 file changed, 76 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/adi,addi9036.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/adi,addi9036.yaml b/Documentation/devicetree/bindings/media/i2c/adi,addi9036.yaml
> new file mode 100644
> index 000000000000..7c4af704db98
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/adi,addi9036.yaml
> @@ -0,0 +1,76 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/adi,addi9036.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices ADDI9036 VGA CCD Time of Flight Sensor
> +
> +maintainers:
> + - Bogdan Togorean <bogdan.togorean@analog.com>
> +
> +description: |-
> + The ADDI9036 is a complete, 45 MHz, front-end solution for charge coupled
> + device (CCD) time of flight (TOF) imaging applications. It is programmable
> + through I2C interface. Image data is sent through MIPI CSI-2 2 lanes and
> + can output two RAW12 packed data streams. One is IR and the other is Depth.
> + Each data stream is on a separate or same MIPI Virtual Channel, depending
> + on configuration and each have 640x480 resolution.
> +
> +properties:
> + compatible:
> + const: adi,addi9036
> +
> + reg:
> + description: I2C device address
Can drop this.
> + maxItems: 1
> +
> + reset-gpios:
maxItems: 1
> + description: |-
> + Reference to the GPIO connected to the RST/SYNC pin, if any.
> + Must be released (set high) after all supplies are applied.
> +
> + # See ../video-interfaces.txt for more details
> + port:
> + type: object
> + properties:
> + endpoint:
> + type: object
> + properties:
> + data-lanes:
> + description: |-
> + The sensor supports two-lane operation.
> + For two-lane operation the property must be set to <1 2>.
> + items:
> + - const: 1
> + - const: 2
If this is the only possible setting, then why does it need to be in DT?
> +
> +required:
> + - compatible
> + - reg
> + - port
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + addi9036: addi9036_tof@64 {
> + compatible = "adi,addi9036";
> + reg = <0x64>;
> +
> + reset-gpios = <&gpio 41 1>;
> +
> + port {
> + addi9036_ep: endpoint {
> + remote-endpoint = <&csi1_ep>;
> + data-lanes = <1 2>;
> + };
> + };
> + };
> + };
> +
> +...
> --
> 2.28.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] media: i2c: Add driver for the Analog Devices ADDI9036 ToF front-end
2020-10-02 13:35 [PATCH v2 1/2] media: i2c: Add driver for the Analog Devices ADDI9036 ToF front-end Bogdan Togorean
2020-10-02 13:35 ` [PATCH v2 2/2] media: dt-bindings: media: i2c: Add bindings for ADDI9036 Bogdan Togorean
@ 2020-10-07 11:18 ` Sakari Ailus
2020-10-08 6:49 ` Togorean, Bogdan
1 sibling, 1 reply; 8+ messages in thread
From: Sakari Ailus @ 2020-10-07 11:18 UTC (permalink / raw)
To: Bogdan Togorean
Cc: linux-media, Mauro Carvalho Chehab, Rob Herring, David S. Miller,
Jacopo Mondi, Laurent Pinchart, Kieran Bingham, Bingbu Cao,
Shawn Tu, Dongchun Zhu, Dave Stevenson, Manivannan Sadhasivam,
devicetree, linux-kernel
Hi Bogdan,
On Fri, Oct 02, 2020 at 04:35:16PM +0300, Bogdan Togorean wrote:
> The ADDI9036 is a complete, 45 MHz, front-end solution for charge
> coupled device (CCD) time of flight (TOF) imaging applications.
>
> It has 2-lane MIPI CSI-2 RAW12 data output and i2c control interface.
>
> The programming of calibration and firmware is performed by driver
> using Linux Firmware API.
>
> Signed-off-by: Bogdan Togorean <bogdan.togorean@analog.com>
> ---
>
> v2: implemented reading of FW using Linux Firmware API
> removed custom controls for FW programming
> added custom control to select operating mode
> implemented V1 review remarks
> cleaned includes list
> ---
> MAINTAINERS | 10 +
> drivers/media/i2c/Kconfig | 14 +
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/addi9036.c | 754 +++++++++++++++++++++++++++++
> include/uapi/linux/v4l2-controls.h | 6 +
> 5 files changed, 785 insertions(+)
> create mode 100644 drivers/media/i2c/addi9036.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0d0862b19ce5..4e3878e6c0ba 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -477,6 +477,16 @@ W: http://wiki.analog.com/AD7879
> W: http://ez.analog.com/community/linux-device-drivers
> F: drivers/input/touchscreen/ad7879.c
>
> +ADDI9036 TOF FRONTEND DRIVER
> +M: Bogdan Togorean <bogdan.togorean@analog.com>
> +L: linux-media@vger.kernel.org
> +S: Supported
> +W: https://www.analog.com/en/products/addi9036.html
> +W: http://ez.analog.com/community/linux-device-drivers
> +T: git git://linuxtv.org/media_tree.git
> +F: Documentation/devicetree/bindings/media/i2c/adi,addi9036.yaml
> +F: drivers/media/i2c/addi9036.c
> +
> ADDRESS SPACE LAYOUT RANDOMIZATION (ASLR)
> M: Jiri Kosina <jikos@kernel.org>
> S: Maintained
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index c7ba76fee599..087dd307505d 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -725,6 +725,20 @@ config VIDEO_APTINA_PLL
> config VIDEO_SMIAPP_PLL
> tristate
>
> +config VIDEO_ADDI9036
> + tristate "Analog Devices ADDI9036 ToF front-end support"
> + depends on I2C && VIDEO_V4L2
> + select MEDIA_CONTROLLER
> + select VIDEO_V4L2_SUBDEV_API
> + select V4L2_FWNODE
> + select REGMAP_I2C
> + help
> + This is a Video4Linux2 driver for Analog Devices ADDI9036
> + Time of Flight front-end.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called addi9036.
> +
> config VIDEO_HI556
> tristate "Hynix Hi-556 sensor support"
> depends on I2C && VIDEO_V4L2
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index f0a77473979d..631a7c7612ca 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_VIDEO_AK7375) += ak7375.o
> obj-$(CONFIG_VIDEO_DW9714) += dw9714.o
> obj-$(CONFIG_VIDEO_DW9768) += dw9768.o
> obj-$(CONFIG_VIDEO_DW9807_VCM) += dw9807-vcm.o
> +obj-$(CONFIG_VIDEO_ADDI9036) += addi9036.o
> obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
> obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
> obj-$(CONFIG_VIDEO_ADV7180) += adv7180.o
> diff --git a/drivers/media/i2c/addi9036.c b/drivers/media/i2c/addi9036.c
> new file mode 100644
> index 000000000000..e38e70afd23d
> --- /dev/null
> +++ b/drivers/media/i2c/addi9036.c
> @@ -0,0 +1,754 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for the Analog Devices ADDI9036 ToF camera sensor.
> + *
> + * Copyright (C) 2019-2020 Analog Devices, All Rights Reserved.
> + *
> + */
> +
> +#include <linux/gpio/consumer.h>
> +#include <linux/firmware.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-subdev.h>
> +
> +#define FW_FILE_NAME "adi/addi9036-fw.bin"
> +#define ADDI_MAGIC "ADDI9036"
> +
> +struct addi9036_mode_info {
> + u32 width;
> + u32 height;
> + u32 pixel_rate;
> + u32 link_freq_idx;
> +};
> +
> +struct addi9036_mode_fw_block {
> + const struct reg_sequence *mode_regs;
> + ssize_t regs_count;
> +};
> +
> +struct addi9036_fw_header {
> + unsigned char magic[8];
> + __le32 modes_nr;
> +} __packed;
> +
> +struct addi9036_mode_block {
> + __le32 mode_id;
> + __le32 size_bytes;
> + __le16 data[];
> +} __packed;
> +
> +struct addi9036 {
> + struct regmap *regmap;
> + struct device *dev;
> + struct v4l2_subdev sd;
> + struct media_pad pad;
> + struct v4l2_fwnode_endpoint ep;
> + struct v4l2_mbus_framefmt fmt;
> + struct v4l2_rect crop;
> +
> + const struct addi9036_mode_info *current_mode;
> +
> + struct v4l2_ctrl_handler ctrls;
> + struct v4l2_ctrl *pixel_rate;
> + struct v4l2_ctrl *link_freq;
> + /* custom controls */
> + struct v4l2_ctrl *set_operating_range;
> +
> + /* lock to protect power state */
> + struct mutex power_lock;
> + int power_count;
> + bool streaming;
> +
> + struct gpio_desc *rst_gpio;
> +
> + /* firmware blocks for each operating mode */
> + struct addi9036_mode_fw_block *mode_fw_blocks;
> + u8 curr_operating_mode;
> +
> + const struct firmware *fw;
> +};
> +
> +static inline struct addi9036 *to_addi9036(struct v4l2_subdev *sd)
> +{
> + return container_of(sd, struct addi9036, sd);
> +}
> +
> +#define V4L2_CID_ADDI9036_OPERATING_MODE (V4L2_CID_USER_ADDI9036_BASE + 0)
> +
> +static const struct reg_sequence addi9036_powerup_setting[] = {
> + { 0xc4c0, 0x001c },
> + { 0xc4c3, 0x001c },
> + { 0xc4d7, 0x0000 },
> + { 0xc4d5, 0x0002 },
> + { 0xc4da, 0x0001 },
> + { 0xc4f0, 0x0000 },
> + { 0xc427, 0x0003 },
> + { 0xc427, 0x0001 },
> + { 0xc427, 0x0000 },
> + { 0xc426, 0x0030 },
> + { 0xc426, 0x0010 },
> + { 0xc426, 0x0000 },
> + { 0xc423, 0x0080 },
> + { 0xc431, 0x0080 },
> + { 0x4001, 0x0007 },
> + { 0x7c22, 0x0004 }
> +};
> +
> +static const struct reg_sequence addi9036_powerdown_setting[] = {
> + { 0xc022, 0x0001 },
> + { 0x4001, 0x0006 },
> + { 0x7c22, 0x0004 },
> + { 0xc431, 0x0082 },
> + { 0xc423, 0x0000 },
> + { 0xc426, 0x0020 },
> + { 0xc427, 0x0002 },
> + { 0xc4c0, 0x003c },
> + { 0xc4c3, 0x003c },
> + { 0xc4d5, 0x0003 },
> + { 0xc4da, 0x0000 },
> + { 0xc4d7, 0x0001 },
> + { 0xc4f0, 0x0001 }
> +};
> +
> +static const s64 link_freq_tbl[] = {
> + 110529000,
> + 221184000,
Are these a hardware property or should they come from DT? Either way, not
both of them could be available on all boards, which suggests DT.
> +};
> +
> +/* Elements of the structure must be ordered ascending by width & height */
> +static const struct addi9036_mode_info addi9036_mode_info_data[] = {
> + {
> + .width = 640,
> + .height = 480,
> + .pixel_rate = 36864000,
> + .link_freq_idx = 0 /* an index in link_freq_tbl[] */
> + },
> + {
> + .width = 640,
> + .height = 960,
> + .pixel_rate = 73728000,
> + .link_freq_idx = 1 /* an index in link_freq_tbl[] */
> + },
> +};
> +
> +static bool addi9306_readable_register(struct device *dev, unsigned int reg)
> +{
> + if (((reg >= 0x4000) && (reg <= 0x6fff)) ||
> + ((reg >= 0x7c00) && (reg <= 0x7fff)) ||
> + ((reg >= 0xc000) && (reg <= 0xc200)) ||
> + ((reg >= 0xc300) && (reg <= 0xc6bf)))
> + return true;
> + else
> + return false;
> +}
> +
> +static bool addi9306_writeable_register(struct device *dev, unsigned int reg)
> +{
> + if (((reg >= 0x4000) && (reg <= 0x6fff)) ||
> + ((reg >= 0x7c00) && (reg <= 0x7fff)) ||
> + ((reg >= 0xc000) && (reg <= 0xc113)) ||
> + ((reg >= 0xc300) && (reg <= 0xc7ff)))
> + return true;
> + else
> + return false;
> +}
> +
> +static const struct regmap_config addi9036_i2c_regmap_config = {
> + .reg_bits = 16,
> + .val_bits = 16,
> + .max_register = 0xc7ff,
> + .writeable_reg = addi9306_writeable_register,
> + .readable_reg = addi9306_readable_register,
> + .cache_type = REGCACHE_NONE,
> +};
> +
> +static int addi9036_set_power_on(struct addi9036 *addi9036)
> +{
> + int ret;
> +
> + if (addi9036->rst_gpio)
> + gpiod_set_value_cansleep(addi9036->rst_gpio, 0);
> +
> + ret = regmap_register_patch(addi9036->regmap, addi9036_powerup_setting,
> + ARRAY_SIZE(addi9036_powerup_setting));
> + if (ret)
> + dev_err(addi9036->dev, "Could not set power up registers\n");
> +
> + return ret;
> +}
> +
> +static int addi9036_set_power_off(struct addi9036 *addi9036)
> +{
> + int ret;
> +
> + ret = regmap_register_patch(addi9036->regmap,
> + addi9036_powerdown_setting,
> + ARRAY_SIZE(addi9036_powerdown_setting));
> + if (ret)
> + dev_err(addi9036->dev, "could not set power down registers\n");
> +
> + if (addi9036->rst_gpio)
> + gpiod_set_value_cansleep(addi9036->rst_gpio, 1);
> +
> + return ret;
> +}
> +
> +static int addi9036_s_power(struct v4l2_subdev *sd, int on)
> +{
> + struct addi9036 *addi9036 = to_addi9036(sd);
> + int ret = 0;
> +
> + dev_dbg(addi9036->dev, "s_power: %d\n", on);
> +
> + mutex_lock(&addi9036->power_lock);
> +
> + /* If the power count is modified from 0 to != 0 or from != 0 to 0,
> + * update the power state.
> + */
> + if (addi9036->power_count == !on) {
> + if (on)
> + ret = addi9036_set_power_on(addi9036);
> + else
> + ret = addi9036_set_power_off(addi9036);
> + }
> +
> + if (!ret) {
> + /* Update the power count. */
> + addi9036->power_count += on ? 1 : -1;
> + WARN(addi9036->power_count < 0, "Unbalanced power count\n");
> + WARN(addi9036->power_count > 1, "Duplicated s_power call\n");
> + }
> +
> + mutex_unlock(&addi9036->power_lock);
> +
> + return ret;
> +}
> +
> +#ifdef CONFIG_VIDEO_ADV_DEBUG
> +static int addi9036_g_register(struct v4l2_subdev *sd,
> + struct v4l2_dbg_register *reg)
> +{
> + struct addi9036 *addi9036 = to_addi9036(sd);
> + unsigned int read_val;
> + int ret;
> +
> + reg->size = 2;
> + ret = regmap_read(addi9036->regmap, reg->reg, &read_val);
> + reg->val = read_val;
> +
> + return ret;
> +}
> +
> +static int addi9036_s_register(struct v4l2_subdev *sd,
> + const struct v4l2_dbg_register *reg)
> +{
> + struct addi9036 *addi9036 = to_addi9036(sd);
> +
> + return regmap_write(addi9036->regmap, reg->reg, reg->val);
> +}
> +#endif
> +
> +static int addi9036_s_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct addi9036 *addi9036 = container_of(ctrl->handler,
> + struct addi9036, ctrls);
> + int ret = 0;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_ADDI9036_OPERATING_MODE:
> + addi9036->curr_operating_mode = ctrl->val;
> + break;
> + case V4L2_CID_PIXEL_RATE:
> + case V4L2_CID_LINK_FREQ:
> + break;
> + default:
> + dev_err(addi9036->dev, "%s > Unhandled: %x param=%x\n",
> + __func__, ctrl->id, ctrl->val);
> + ret = -EINVAL;
> + break;
> + }
> +
> + return ret;
> +}
> +
> +static const struct v4l2_ctrl_ops addi9036_ctrl_ops = {
> + .s_ctrl = addi9036_s_ctrl,
> +};
> +
> +static const struct v4l2_ctrl_config addi9036_ctrl_operating_mode = {
> + .ops = &addi9036_ctrl_ops,
> + .id = V4L2_CID_ADDI9036_OPERATING_MODE,
> + .name = "Operating Mode",
> + .type = V4L2_CTRL_TYPE_INTEGER,
> + .def = 0,
> + .min = 0,
> + .max = 1,
> + .step = 1,
> +};
> +
> +static int addi9036_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 = MEDIA_BUS_FMT_SBGGR12_1X12;
> +
> + return 0;
> +}
> +
> +static int addi9036_enum_frame_size(struct v4l2_subdev *subdev,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_frame_size_enum *fse)
> +{
> + if (fse->code != MEDIA_BUS_FMT_SBGGR12_1X12)
> + return -EINVAL;
> +
> + if (fse->index >= ARRAY_SIZE(addi9036_mode_info_data))
> + return -EINVAL;
> +
> + fse->min_width = addi9036_mode_info_data[fse->index].width;
> + fse->max_width = addi9036_mode_info_data[fse->index].width;
> + fse->min_height = addi9036_mode_info_data[fse->index].height;
> + fse->max_height = addi9036_mode_info_data[fse->index].height;
> +
> + return 0;
> +}
> +
> +static struct v4l2_mbus_framefmt *
> +addi9036_get_pad_format(struct addi9036 *addi9036,
> + 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(&addi9036->sd, cfg, pad);
> + case V4L2_SUBDEV_FORMAT_ACTIVE:
> + return &addi9036->fmt;
> + default:
> + return NULL;
I'd suggest never return NULL here. Maybe issue a warning and return either
of the valid ones? Or add NULL checks elsewhere.
> + }
> +}
> +
> +static int addi9036_get_format(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_format *format)
> +{
> + struct addi9036 *addi9036 = to_addi9036(sd);
> + struct v4l2_mbus_framefmt *pad_format;
> +
> + pad_format = addi9036_get_pad_format(addi9036, cfg, format->pad,
> + format->which);
> +
> + if (!pad_format)
> + return -EINVAL;
> +
> + format->format = *pad_format;
> +
> + return 0;
> +}
> +
> +static struct v4l2_rect *
> +addi9036_get_pad_crop(struct addi9036 *addi9036,
> + 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(&addi9036->sd, cfg, pad);
> + case V4L2_SUBDEV_FORMAT_ACTIVE:
> + return &addi9036->crop;
> + default:
> + return NULL;
> + }
> +}
> +
> +static int addi9036_set_format(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_format *format)
> +{
> + struct addi9036 *addi9036 = to_addi9036(sd);
> + struct v4l2_mbus_framefmt *framefmt;
> + struct v4l2_rect *crop;
> + const struct addi9036_mode_info *new_mode;
> + int ret;
> +
> + dev_dbg(addi9036->dev, "set_fmt: %x %dx%d %d\n",
> + format->format.code, format->format.width,
> + format->format.height, format->which);
> +
> + crop = addi9036_get_pad_crop(addi9036, cfg, format->pad,
> + format->which);
> +
> + if (!crop)
> + return -EINVAL;
> +
> + new_mode = v4l2_find_nearest_size(addi9036_mode_info_data,
> + ARRAY_SIZE(addi9036_mode_info_data),
> + width, height, format->format.width,
> + format->format.height);
> + crop->width = new_mode->width;
> + crop->height = new_mode->height;
> +
> + if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> + ret = v4l2_ctrl_s_ctrl_int64(addi9036->pixel_rate,
> + new_mode->pixel_rate);
> + if (ret < 0)
> + return ret;
> +
> + ret = v4l2_ctrl_s_ctrl(addi9036->link_freq,
> + new_mode->link_freq_idx);
> + if (ret < 0)
> + return ret;
> +
> + addi9036->current_mode = new_mode;
> + }
> +
> + framefmt = addi9036_get_pad_format(addi9036, cfg, format->pad,
> + format->which);
I believe you'll need to serialise access to framefmt.
> + framefmt->width = crop->width;
> + framefmt->height = crop->height;
> + framefmt->code = MEDIA_BUS_FMT_SBGGR12_1X12;
> + framefmt->field = V4L2_FIELD_NONE;
> + framefmt->colorspace = V4L2_COLORSPACE_SRGB;
> +
> + format->format = *framefmt;
> +
> + return 0;
> +}
> +
> +static int addi9036_entity_init_cfg(struct v4l2_subdev *subdev,
> + struct v4l2_subdev_pad_config *cfg)
> +{
> + struct v4l2_subdev_format fmt = { 0 };
> +
> + fmt.which = cfg ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
> + fmt.format.width = addi9036_mode_info_data[1].width;
> + fmt.format.height = addi9036_mode_info_data[1].height;
> +
> + addi9036_set_format(subdev, cfg, &fmt);
> +
> + return 0;
> +}
> +
> +static int addi9036_get_selection(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_selection *sel)
> +{
> + struct addi9036 *addi9036 = to_addi9036(sd);
> +
> + if (sel->target != V4L2_SEL_TGT_CROP)
> + return -EINVAL;
> +
> + sel->r = *addi9036_get_pad_crop(addi9036, cfg, sel->pad, sel->which);
> +
> + return 0;
> +}
> +
> +static int addi9036_s_stream(struct v4l2_subdev *subdev, int enable)
> +{
> + struct addi9036 *addi9036 = to_addi9036(subdev);
> + uint8_t mode = addi9036->curr_operating_mode;
> + int ret = 0;
> +
> + dev_dbg(addi9036->dev, "s_stream: %d\n", enable);
> +
> + if (addi9036->streaming == enable)
> + return 0;
> +
> + if (enable) {
> + if (addi9036->mode_fw_blocks[mode].mode_regs == NULL) {
> + dev_err(addi9036->dev, "Selected mode has no data\n");
> + return -EINVAL;
> + }
> +
> + dev_dbg(addi9036->dev, "Applying mode: %u\n", mode);
> + ret = regmap_multi_reg_write(addi9036->regmap,
> + addi9036->mode_fw_blocks[mode].mode_regs,
> + addi9036->mode_fw_blocks[mode].regs_count);
> +
> + dev_dbg(addi9036->dev, "Writen %lu registers\n",
> + addi9036->mode_fw_blocks[mode].regs_count);
> + }
> +
> + addi9036->streaming = enable;
> + return ret;
> +}
> +
> +static const struct v4l2_subdev_core_ops addi9036_core_ops = {
> + .s_power = addi9036_s_power,
Please add support for runtime PM instead.
> +#ifdef CONFIG_VIDEO_ADV_DEBUG
> + .g_register = addi9036_g_register,
> + .s_register = addi9036_s_register,
> +#endif
> +};
> +
> +static const struct v4l2_subdev_video_ops addi9036_video_ops = {
> + .s_stream = addi9036_s_stream,
> +};
> +
> +static const struct v4l2_subdev_pad_ops addi9036_subdev_pad_ops = {
> + .init_cfg = addi9036_entity_init_cfg,
> + .enum_mbus_code = addi9036_enum_mbus_code,
> + .enum_frame_size = addi9036_enum_frame_size,
> + .get_fmt = addi9036_get_format,
> + .set_fmt = addi9036_set_format,
> + .get_selection = addi9036_get_selection,
> +};
> +
> +static const struct v4l2_subdev_ops addi9036_subdev_ops = {
> + .core = &addi9036_core_ops,
> + .video = &addi9036_video_ops,
> + .pad = &addi9036_subdev_pad_ops,
> +};
> +
> +static int addi9036_g_modes_from_firmware(struct v4l2_subdev *sd)
> +{
> + struct addi9036 *addi9036 = to_addi9036(sd);
> + const struct firmware *fw = addi9036->fw;
> + const struct addi9036_fw_header *fw_head;
> + const struct addi9036_mode_block *mode_block;
> + unsigned int reg_nr, chunk_len, pos, modes_nr, i, j, k;
> + struct reg_sequence *reg_seq;
> +
> + /*
> + * Reject too small or unreasonable large files.
> + */
> +
> + if (fw->size < sizeof(struct addi9036_fw_header) ||
> + fw->size >= 0x4000000) {
> + dev_err(addi9036->dev, "FW loading failed: Invalid size\n");
> + return -EINVAL;
> + }
> +
> + fw_head = (struct addi9036_fw_header *)fw->data;
> +
> + if (memcmp(fw_head->magic, ADDI_MAGIC, ARRAY_SIZE(fw_head->magic))) {
> + dev_err(addi9036->dev, "FW loading failed: Invalid magic\n");
> + return -EINVAL;
> + }
> +
> + modes_nr = le32_to_cpu(fw_head->modes_nr);
> +
> + if (modes_nr == 0) {
> + dev_err(addi9036->dev, "FW should contain at least 1 mode.\n");
> + return -EINVAL;
> + }
> +
> + __v4l2_ctrl_modify_range(addi9036->set_operating_range,
> + addi9036->set_operating_range->minimum,
> + modes_nr - 1, 1, 0);
> +
> + addi9036->mode_fw_blocks = devm_kzalloc(addi9036->dev,
> + sizeof(struct addi9036_mode_fw_block) * modes_nr,
devm_kcalloc, please.
> + GFP_KERNEL);
> + if (!addi9036->mode_fw_blocks)
> + return -ENOMEM;
> +
> + pos = sizeof(struct addi9036_fw_header);
> +
> + for (i = 0; i < modes_nr; i++) {
> + mode_block = (struct addi9036_mode_block *)(fw->data + pos);
I think you need more validation here. For instance, make sure the firmware
binary is at least as big as the offset you're accessing.
> +
> + chunk_len = le32_to_cpu(mode_block->size_bytes);
> + reg_nr = chunk_len / sizeof(uint16_t) / 2;
> +
> + reg_seq = devm_kzalloc(addi9036->dev,
> + sizeof(struct reg_sequence) * reg_nr,
> + GFP_KERNEL);
> + if (!reg_seq)
> + return -ENOMEM;
> +
> + k = 0;
> + for (j = 0; j < reg_nr * 2; j += 2) {
j = 0, k = 0
> + reg_seq[k].reg = le16_to_cpu(mode_block->data[j]);
> + reg_seq[k].def = le16_to_cpu(mode_block->data[j + 1]);
> + k++;
> + }
> +
> + addi9036->mode_fw_blocks[i].mode_regs = reg_seq;
> + addi9036->mode_fw_blocks[i].regs_count = reg_nr;
> +
> + pos += chunk_len + sizeof(struct addi9036_mode_block);
> + }
> + return 0;
> +}
> +
> +static int addi9036_mode_firmware_load(struct v4l2_subdev *sd)
> +{
> + struct addi9036 *addi9036 = to_addi9036(sd);
> + int ret;
> +
> + ret = request_firmware(&addi9036->fw, FW_FILE_NAME, addi9036->dev);
> + if (ret < 0) {
> + dev_err(addi9036->dev, "FW request failed\n");
> + return ret;
> + }
> +
> + ret = addi9036_g_modes_from_firmware(sd);
> +
> + release_firmware(addi9036->fw);
> + if (ret < 0) {
> + dev_err(addi9036->dev, "FW parsing failed\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int addi9036_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct fwnode_handle *endpoint;
> + struct addi9036 *addi9036;
> + int ret;
> +
> + dev_dbg(dev, "%s: i2c addr = 0x%x\n", __func__, client->addr);
> +
> + addi9036 = devm_kzalloc(dev, sizeof(struct addi9036), GFP_KERNEL);
> + if (!addi9036)
> + return -ENOMEM;
> +
> + addi9036->dev = dev;
> +
> + addi9036->regmap = devm_regmap_init_i2c(client,
> + &addi9036_i2c_regmap_config);
> + if (IS_ERR(addi9036->regmap)) {
> + dev_err(dev, "Error initializing i2c regmap\n");
> + return PTR_ERR(addi9036->regmap);
> + }
> +
> + mutex_init(&addi9036->power_lock);
> +
> + endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL);
Could you use fwnode_graph_get_endpoint_by_id()?
> + if (!endpoint) {
> + dev_err(dev, "endpoint node not found\n");
> + return -EINVAL;
> + }
> +
> + ret = v4l2_fwnode_endpoint_parse(endpoint, &addi9036->ep);
> + if (ret < 0) {
> + dev_err(dev, "parsing endpoint node failed\n");
> + return ret;
> + }
> + fwnode_handle_put(endpoint);
> +
> + if (addi9036->ep.bus_type != V4L2_MBUS_CSI2_DPHY) {
If you expect D-PHY, please specify the bus type before calling
v4l2_fwnode_endpoint_parse().
> + dev_err(dev, "invalid bus type, must be MIPI CSI2\n");
> + return -EINVAL;
> + }
> +
> + addi9036->rst_gpio = gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(addi9036->rst_gpio))
> + dev_info(dev, "Unable to get \"reset\" gpio\n");
> +
> + v4l2_ctrl_handler_init(&addi9036->ctrls, 3);
> +
> + addi9036->pixel_rate = v4l2_ctrl_new_std(&addi9036->ctrls,
> + &addi9036_ctrl_ops,
> + V4L2_CID_PIXEL_RATE,
> + 1, INT_MAX, 1, 1);
> + addi9036->link_freq = v4l2_ctrl_new_int_menu(&addi9036->ctrls,
> + &addi9036_ctrl_ops,
> + V4L2_CID_LINK_FREQ,
> + ARRAY_SIZE(
> + link_freq_tbl) - 1,
> + 0, link_freq_tbl);
> + if (addi9036->link_freq)
> + addi9036->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> +
> + addi9036->set_operating_range = v4l2_ctrl_new_custom(&addi9036->ctrls,
> + &addi9036_ctrl_operating_mode,
> + NULL);
> +
> + ret = addi9036->ctrls.error;
> + if (ret) {
> + dev_err(dev, "%s: control initialization error %d\n",
> + __func__, ret);
> + goto free_ctrl;
> + }
> + addi9036->sd.ctrl_handler = &addi9036->ctrls;
> +
> + v4l2_i2c_subdev_init(&addi9036->sd, client, &addi9036_subdev_ops);
> + addi9036->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> + addi9036->pad.flags = MEDIA_PAD_FL_SOURCE;
> + addi9036->sd.dev = &client->dev;
> + addi9036->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> +
> + ret = media_entity_pads_init(&addi9036->sd.entity, 1, &addi9036->pad);
> + if (ret < 0) {
> + dev_err(dev, "could not register media entity\n");
> + goto free_ctrl;
> + }
> +
> + ret = addi9036_mode_firmware_load(&addi9036->sd);
> + if (ret < 0)
> + return ret;
> +
> + addi9036_entity_init_cfg(&addi9036->sd, NULL);
> +
> + ret = v4l2_async_register_subdev(&addi9036->sd);
> + if (ret < 0) {
> + dev_err(dev, "could not register v4l2 device\n");
> + goto free_entity;
> + }
> +
> + return 0;
> +
> +free_entity:
> + media_entity_cleanup(&addi9036->sd.entity);
> +free_ctrl:
> + v4l2_ctrl_handler_free(&addi9036->ctrls);
> + mutex_destroy(&addi9036->power_lock);
> +
> + return ret;
> +}
> +
> +static int addi9036_remove(struct i2c_client *client)
> +{
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct addi9036 *addi9036 = to_addi9036(sd);
> +
> + v4l2_async_unregister_subdev(&addi9036->sd);
> + media_entity_cleanup(&addi9036->sd.entity);
> + if (addi9036->rst_gpio)
> + gpiod_put(addi9036->rst_gpio);
> + v4l2_ctrl_handler_free(&addi9036->ctrls);
> + mutex_destroy(&addi9036->power_lock);
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id addi9036_id[] = {
Do you really need the I²C device table? Please remove if not.
> + { "addi9036", 0 },
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, addi9036_id);
> +
> +static const struct of_device_id addi9036_of_match[] = {
> + { .compatible = "adi,addi9036" },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, addi9036_of_match);
> +
> +static struct i2c_driver addi9036_i2c_driver = {
> + .driver = {
> + .of_match_table = addi9036_of_match,
> + .name = "addi9036",
> + },
> + .probe_new = addi9036_probe,
> + .remove = addi9036_remove,
> + .id_table = addi9036_id,
> +};
> +
> +module_i2c_driver(addi9036_i2c_driver);
> +
> +MODULE_DESCRIPTION("Analog Devices ADDI9036 Camera Driver");
> +MODULE_AUTHOR("Bogdan Togorean");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 62271418c1be..f88b56479bc1 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -198,6 +198,12 @@ enum v4l2_colorfx {
> */
> #define V4L2_CID_USER_ATMEL_ISC_BASE (V4L2_CID_USER_BASE + 0x10c0)
>
> +/*
> + * The base for the addi9036 driver controls.
> + * We reserve 16 controls for this driver.
> + */
> +#define V4L2_CID_USER_ADDI9036_BASE (V4L2_CID_USER_BASE + 0x10e0)
> +
> /* MPEG-class control IDs */
> /* The MPEG controls are applicable to all codec controls
> * and the 'MPEG' part of the define is historical */
--
Sakari Ailus
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH v2 2/2] media: dt-bindings: media: i2c: Add bindings for ADDI9036
2020-10-06 20:47 ` Rob Herring
@ 2020-10-08 6:23 ` Togorean, Bogdan
2020-10-08 6:35 ` Sakari Ailus
0 siblings, 1 reply; 8+ messages in thread
From: Togorean, Bogdan @ 2020-10-08 6:23 UTC (permalink / raw)
To: Rob Herring
Cc: linux-media, Mauro Carvalho Chehab, David S. Miller,
Sakari Ailus, Jacopo Mondi, Laurent Pinchart, Kieran Bingham,
Bingbu Cao, Dave Stevenson, Shawn Tu, Dongchun Zhu, devicetree,
linux-kernel
Thank you Rob for review
> On Fri, Oct 02, 2020 at 04:35:17PM +0300, Bogdan Togorean wrote:
> > Add YAML device tree bindings for Analog Devices Inc. ADDI9036 CCD TOF
> > front-end.
> >
> > Signed-off-by: Bogdan Togorean <bogdan.togorean@analog.com>
> > ---
> > v2: added reg property description
> > ---
> > .../bindings/media/i2c/adi,addi9036.yaml | 76 +++++++++++++++++++
> > 1 file changed, 76 insertions(+)
> > create mode 100644
> Documentation/devicetree/bindings/media/i2c/adi,addi9036.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/adi,addi9036.yaml
> b/Documentation/devicetree/bindings/media/i2c/adi,addi9036.yaml
> > new file mode 100644
> > index 000000000000..7c4af704db98
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/adi,addi9036.yaml
> > @@ -0,0 +1,76 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id:
> https://urldefense.com/v3/__http://devicetree.org/schemas/media/i2c/adi,add
> i9036.yaml*__;Iw!!A3Ni8CS0y2Y!vLJoRikiVhmxm8p3bhGjRkFIWgjXvVlcJ8ATa9okn
> JqDbtobtK46hHICblE2i2Yj2sZL$
> > +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-
> schemas/core.yaml*__;Iw!!A3Ni8CS0y2Y!vLJoRikiVhmxm8p3bhGjRkFIWgjXvVlcJ
> 8ATa9oknJqDbtobtK46hHICblE2iz34fu4o$
> > +
> > +title: Analog Devices ADDI9036 VGA CCD Time of Flight Sensor
> > +
> > +maintainers:
> > + - Bogdan Togorean <bogdan.togorean@analog.com>
> > +
> > +description: |-
> > + The ADDI9036 is a complete, 45 MHz, front-end solution for charge coupled
> > + device (CCD) time of flight (TOF) imaging applications. It is programmable
> > + through I2C interface. Image data is sent through MIPI CSI-2 2 lanes and
> > + can output two RAW12 packed data streams. One is IR and the other is
> Depth.
> > + Each data stream is on a separate or same MIPI Virtual Channel, depending
> > + on configuration and each have 640x480 resolution.
> > +
> > +properties:
> > + compatible:
> > + const: adi,addi9036
> > +
> > + reg:
> > + description: I2C device address
>
> Can drop this.
ACK
>
> > + maxItems: 1
> > +
> > + reset-gpios:
>
> maxItems: 1
ACK
>
> > + description: |-
> > + Reference to the GPIO connected to the RST/SYNC pin, if any.
> > + Must be released (set high) after all supplies are applied.
> > +
> > + # See ../video-interfaces.txt for more details
> > + port:
> > + type: object
> > + properties:
> > + endpoint:
> > + type: object
> > + properties:
> > + data-lanes:
> > + description: |-
> > + The sensor supports two-lane operation.
> > + For two-lane operation the property must be set to <1 2>.
> > + items:
> > + - const: 1
> > + - const: 2
>
> If this is the only possible setting, then why does it need to be in DT?
If this is not set the bus_type will be not be correctly set after call of
v4l2_fwnode_endpoint_parse.
>
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - port
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + i2c0 {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + addi9036: addi9036_tof@64 {
> > + compatible = "adi,addi9036";
> > + reg = <0x64>;
> > +
> > + reset-gpios = <&gpio 41 1>;
> > +
> > + port {
> > + addi9036_ep: endpoint {
> > + remote-endpoint = <&csi1_ep>;
> > + data-lanes = <1 2>;
> > + };
> > + };
> > + };
> > + };
> > +
> > +...
> > --
> > 2.28.0
> >
Will send V3 with updates.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] media: dt-bindings: media: i2c: Add bindings for ADDI9036
2020-10-08 6:23 ` Togorean, Bogdan
@ 2020-10-08 6:35 ` Sakari Ailus
2020-10-08 6:42 ` Togorean, Bogdan
0 siblings, 1 reply; 8+ messages in thread
From: Sakari Ailus @ 2020-10-08 6:35 UTC (permalink / raw)
To: Togorean, Bogdan
Cc: Rob Herring, linux-media, Mauro Carvalho Chehab, David S. Miller,
Jacopo Mondi, Laurent Pinchart, Kieran Bingham, Bingbu Cao,
Dave Stevenson, Shawn Tu, Dongchun Zhu, devicetree, linux-kernel
Hi Bogdan,
On Thu, Oct 08, 2020 at 06:23:33AM +0000, Togorean, Bogdan wrote:
> Thank you Rob for review
> > On Fri, Oct 02, 2020 at 04:35:17PM +0300, Bogdan Togorean wrote:
> > > Add YAML device tree bindings for Analog Devices Inc. ADDI9036 CCD TOF
> > > front-end.
> > >
> > > Signed-off-by: Bogdan Togorean <bogdan.togorean@analog.com>
> > > ---
> > > v2: added reg property description
> > > ---
> > > .../bindings/media/i2c/adi,addi9036.yaml | 76 +++++++++++++++++++
> > > 1 file changed, 76 insertions(+)
> > > create mode 100644
> > Documentation/devicetree/bindings/media/i2c/adi,addi9036.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/adi,addi9036.yaml
> > b/Documentation/devicetree/bindings/media/i2c/adi,addi9036.yaml
> > > new file mode 100644
> > > index 000000000000..7c4af704db98
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/i2c/adi,addi9036.yaml
> > > @@ -0,0 +1,76 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id:
> > https://urldefense.com/v3/__http://devicetree.org/schemas/media/i2c/adi,add
> > i9036.yaml*__;Iw!!A3Ni8CS0y2Y!vLJoRikiVhmxm8p3bhGjRkFIWgjXvVlcJ8ATa9okn
> > JqDbtobtK46hHICblE2i2Yj2sZL$
> > > +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-
> > schemas/core.yaml*__;Iw!!A3Ni8CS0y2Y!vLJoRikiVhmxm8p3bhGjRkFIWgjXvVlcJ
> > 8ATa9oknJqDbtobtK46hHICblE2iz34fu4o$
> > > +
> > > +title: Analog Devices ADDI9036 VGA CCD Time of Flight Sensor
> > > +
> > > +maintainers:
> > > + - Bogdan Togorean <bogdan.togorean@analog.com>
> > > +
> > > +description: |-
> > > + The ADDI9036 is a complete, 45 MHz, front-end solution for charge coupled
> > > + device (CCD) time of flight (TOF) imaging applications. It is programmable
> > > + through I2C interface. Image data is sent through MIPI CSI-2 2 lanes and
> > > + can output two RAW12 packed data streams. One is IR and the other is
> > Depth.
> > > + Each data stream is on a separate or same MIPI Virtual Channel, depending
> > > + on configuration and each have 640x480 resolution.
> > > +
> > > +properties:
> > > + compatible:
> > > + const: adi,addi9036
> > > +
> > > + reg:
> > > + description: I2C device address
> >
> > Can drop this.
> ACK
> >
> > > + maxItems: 1
> > > +
> > > + reset-gpios:
> >
> > maxItems: 1
> ACK
> >
> > > + description: |-
> > > + Reference to the GPIO connected to the RST/SYNC pin, if any.
> > > + Must be released (set high) after all supplies are applied.
> > > +
> > > + # See ../video-interfaces.txt for more details
> > > + port:
> > > + type: object
> > > + properties:
> > > + endpoint:
> > > + type: object
> > > + properties:
> > > + data-lanes:
> > > + description: |-
> > > + The sensor supports two-lane operation.
> > > + For two-lane operation the property must be set to <1 2>.
> > > + items:
> > > + - const: 1
> > > + - const: 2
> >
> > If this is the only possible setting, then why does it need to be in DT?
> If this is not set the bus_type will be not be correctly set after call of
> v4l2_fwnode_endpoint_parse.
That's not a DT binding issue. The driver needs to set the field before
calling v4l2_fwnode_endpoint_parse.
Does the device not support one lane operation?
--
Regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH v2 2/2] media: dt-bindings: media: i2c: Add bindings for ADDI9036
2020-10-08 6:35 ` Sakari Ailus
@ 2020-10-08 6:42 ` Togorean, Bogdan
0 siblings, 0 replies; 8+ messages in thread
From: Togorean, Bogdan @ 2020-10-08 6:42 UTC (permalink / raw)
To: Sakari Ailus
Cc: Rob Herring, linux-media, Mauro Carvalho Chehab, David S. Miller,
Jacopo Mondi, Laurent Pinchart, Kieran Bingham, Bingbu Cao,
Dave Stevenson, Shawn Tu, Dongchun Zhu, devicetree, linux-kernel
Hi Sakari,
> Hi Bogdan,
>
> On Thu, Oct 08, 2020 at 06:23:33AM +0000, Togorean, Bogdan wrote:
> > Thank you Rob for review
> > > On Fri, Oct 02, 2020 at 04:35:17PM +0300, Bogdan Togorean wrote:
> > > > Add YAML device tree bindings for Analog Devices Inc. ADDI9036 CCD TOF
> > > > front-end.
> > > >
> > > > Signed-off-by: Bogdan Togorean <bogdan.togorean@analog.com>
> > > > ---
> > > > v2: added reg property description
> > > > ---
> > > > .../bindings/media/i2c/adi,addi9036.yaml | 76 +++++++++++++++++++
> > > > 1 file changed, 76 insertions(+)
> > > > create mode 100644
> > > Documentation/devicetree/bindings/media/i2c/adi,addi9036.yaml
> > > >
> > > > diff --git
> a/Documentation/devicetree/bindings/media/i2c/adi,addi9036.yaml
> > > b/Documentation/devicetree/bindings/media/i2c/adi,addi9036.yaml
> > > > new file mode 100644
> > > > index 000000000000..7c4af704db98
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/media/i2c/adi,addi9036.yaml
> > > > @@ -0,0 +1,76 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id:
> > >
> https://urldefense.com/v3/__http://devicetree.org/schemas/media/i2c/adi,add
> > >
> i9036.yaml*__;Iw!!A3Ni8CS0y2Y!vLJoRikiVhmxm8p3bhGjRkFIWgjXvVlcJ8ATa9okn
> > > JqDbtobtK46hHICblE2i2Yj2sZL$
> > > > +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-
> > >
> schemas/core.yaml*__;Iw!!A3Ni8CS0y2Y!vLJoRikiVhmxm8p3bhGjRkFIWgjXvVlcJ
> > > 8ATa9oknJqDbtobtK46hHICblE2iz34fu4o$
> > > > +
> > > > +title: Analog Devices ADDI9036 VGA CCD Time of Flight Sensor
> > > > +
> > > > +maintainers:
> > > > + - Bogdan Togorean <bogdan.togorean@analog.com>
> > > > +
> > > > +description: |-
> > > > + The ADDI9036 is a complete, 45 MHz, front-end solution for charge
> coupled
> > > > + device (CCD) time of flight (TOF) imaging applications. It is programmable
> > > > + through I2C interface. Image data is sent through MIPI CSI-2 2 lanes and
> > > > + can output two RAW12 packed data streams. One is IR and the other is
> > > Depth.
> > > > + Each data stream is on a separate or same MIPI Virtual Channel,
> depending
> > > > + on configuration and each have 640x480 resolution.
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + const: adi,addi9036
> > > > +
> > > > + reg:
> > > > + description: I2C device address
> > >
> > > Can drop this.
> > ACK
> > >
> > > > + maxItems: 1
> > > > +
> > > > + reset-gpios:
> > >
> > > maxItems: 1
> > ACK
> > >
> > > > + description: |-
> > > > + Reference to the GPIO connected to the RST/SYNC pin, if any.
> > > > + Must be released (set high) after all supplies are applied.
> > > > +
> > > > + # See ../video-interfaces.txt for more details
> > > > + port:
> > > > + type: object
> > > > + properties:
> > > > + endpoint:
> > > > + type: object
> > > > + properties:
> > > > + data-lanes:
> > > > + description: |-
> > > > + The sensor supports two-lane operation.
> > > > + For two-lane operation the property must be set to <1 2>.
> > > > + items:
> > > > + - const: 1
> > > > + - const: 2
> > >
> > > If this is the only possible setting, then why does it need to be in DT?
> > If this is not set the bus_type will be not be correctly set after call of
> > v4l2_fwnode_endpoint_parse.
>
> That's not a DT binding issue. The driver needs to set the field before
> calling v4l2_fwnode_endpoint_parse.
>
> Does the device not support one lane operation?
>
Now I was reading your response to driver review and understood the problem.
Sorry for this an I'll set the bus_type in driver before call of v4l2_fwnode_endpoint_parse.
And the camera does not support one lane.
> --
> Regards,
>
> Sakari Ailus
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH v2 1/2] media: i2c: Add driver for the Analog Devices ADDI9036 ToF front-end
2020-10-07 11:18 ` [PATCH v2 1/2] media: i2c: Add driver for the Analog Devices ADDI9036 ToF front-end Sakari Ailus
@ 2020-10-08 6:49 ` Togorean, Bogdan
0 siblings, 0 replies; 8+ messages in thread
From: Togorean, Bogdan @ 2020-10-08 6:49 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media, Mauro Carvalho Chehab, Rob Herring, David S. Miller,
Jacopo Mondi, Laurent Pinchart, Kieran Bingham, Bingbu Cao,
Shawn Tu, Dongchun Zhu, Dave Stevenson, Manivannan Sadhasivam,
devicetree, linux-kernel
Thank you Sakari for review
> Hi Bogdan,
>
> On Fri, Oct 02, 2020 at 04:35:16PM +0300, Bogdan Togorean wrote:
> > The ADDI9036 is a complete, 45 MHz, front-end solution for charge
> > coupled device (CCD) time of flight (TOF) imaging applications.
> >
> > It has 2-lane MIPI CSI-2 RAW12 data output and i2c control interface.
> >
> > The programming of calibration and firmware is performed by driver
> > using Linux Firmware API.
> >
> > Signed-off-by: Bogdan Togorean <bogdan.togorean@analog.com>
> > ---
> >
> > v2: implemented reading of FW using Linux Firmware API
> > removed custom controls for FW programming
> > added custom control to select operating mode
> > implemented V1 review remarks
> > cleaned includes list
> > ---
> > MAINTAINERS | 10 +
> > drivers/media/i2c/Kconfig | 14 +
> > drivers/media/i2c/Makefile | 1 +
> > drivers/media/i2c/addi9036.c | 754 +++++++++++++++++++++++++++++
> > include/uapi/linux/v4l2-controls.h | 6 +
> > 5 files changed, 785 insertions(+)
> > create mode 100644 drivers/media/i2c/addi9036.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 0d0862b19ce5..4e3878e6c0ba 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -477,6 +477,16 @@ W: http://wiki.analog.com/AD7879
> > W: http://ez.analog.com/community/linux-device-drivers
> > F: drivers/input/touchscreen/ad7879.c
> >
> > +ADDI9036 TOF FRONTEND DRIVER
> > +M: Bogdan Togorean <bogdan.togorean@analog.com>
> > +L: linux-media@vger.kernel.org
> > +S: Supported
> > +W: https://www.analog.com/en/products/addi9036.html
> > +W: http://ez.analog.com/community/linux-device-drivers
> > +T: git git://linuxtv.org/media_tree.git
> > +F: Documentation/devicetree/bindings/media/i2c/adi,addi9036.yaml
> > +F: drivers/media/i2c/addi9036.c
> > +
> > ADDRESS SPACE LAYOUT RANDOMIZATION (ASLR)
> > M: Jiri Kosina <jikos@kernel.org>
> > S: Maintained
> > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > index c7ba76fee599..087dd307505d 100644
> > --- a/drivers/media/i2c/Kconfig
> > +++ b/drivers/media/i2c/Kconfig
> > @@ -725,6 +725,20 @@ config VIDEO_APTINA_PLL
> > config VIDEO_SMIAPP_PLL
> > tristate
> >
> > +config VIDEO_ADDI9036
> > + tristate "Analog Devices ADDI9036 ToF front-end support"
> > + depends on I2C && VIDEO_V4L2
> > + select MEDIA_CONTROLLER
> > + select VIDEO_V4L2_SUBDEV_API
> > + select V4L2_FWNODE
> > + select REGMAP_I2C
> > + help
> > + This is a Video4Linux2 driver for Analog Devices ADDI9036
> > + Time of Flight front-end.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called addi9036.
> > +
> > config VIDEO_HI556
> > tristate "Hynix Hi-556 sensor support"
> > depends on I2C && VIDEO_V4L2
> > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> > index f0a77473979d..631a7c7612ca 100644
> > --- a/drivers/media/i2c/Makefile
> > +++ b/drivers/media/i2c/Makefile
> > @@ -26,6 +26,7 @@ obj-$(CONFIG_VIDEO_AK7375) += ak7375.o
> > obj-$(CONFIG_VIDEO_DW9714) += dw9714.o
> > obj-$(CONFIG_VIDEO_DW9768) += dw9768.o
> > obj-$(CONFIG_VIDEO_DW9807_VCM) += dw9807-vcm.o
> > +obj-$(CONFIG_VIDEO_ADDI9036) += addi9036.o
> > obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
> > obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
> > obj-$(CONFIG_VIDEO_ADV7180) += adv7180.o
> > diff --git a/drivers/media/i2c/addi9036.c b/drivers/media/i2c/addi9036.c
> > new file mode 100644
> > index 000000000000..e38e70afd23d
> > --- /dev/null
> > +++ b/drivers/media/i2c/addi9036.c
> > @@ -0,0 +1,754 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Driver for the Analog Devices ADDI9036 ToF camera sensor.
> > + *
> > + * Copyright (C) 2019-2020 Analog Devices, All Rights Reserved.
> > + *
> > + */
> > +
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/firmware.h>
> > +#include <linux/i2c.h>
> > +#include <linux/module.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/regmap.h>
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-fwnode.h>
> > +#include <media/v4l2-subdev.h>
> > +
> > +#define FW_FILE_NAME "adi/addi9036-fw.bin"
> > +#define ADDI_MAGIC "ADDI9036"
> > +
> > +struct addi9036_mode_info {
> > + u32 width;
> > + u32 height;
> > + u32 pixel_rate;
> > + u32 link_freq_idx;
> > +};
> > +
> > +struct addi9036_mode_fw_block {
> > + const struct reg_sequence *mode_regs;
> > + ssize_t regs_count;
> > +};
> > +
> > +struct addi9036_fw_header {
> > + unsigned char magic[8];
> > + __le32 modes_nr;
> > +} __packed;
> > +
> > +struct addi9036_mode_block {
> > + __le32 mode_id;
> > + __le32 size_bytes;
> > + __le16 data[];
> > +} __packed;
> > +
> > +struct addi9036 {
> > + struct regmap *regmap;
> > + struct device *dev;
> > + struct v4l2_subdev sd;
> > + struct media_pad pad;
> > + struct v4l2_fwnode_endpoint ep;
> > + struct v4l2_mbus_framefmt fmt;
> > + struct v4l2_rect crop;
> > +
> > + const struct addi9036_mode_info *current_mode;
> > +
> > + struct v4l2_ctrl_handler ctrls;
> > + struct v4l2_ctrl *pixel_rate;
> > + struct v4l2_ctrl *link_freq;
> > + /* custom controls */
> > + struct v4l2_ctrl *set_operating_range;
> > +
> > + /* lock to protect power state */
> > + struct mutex power_lock;
> > + int power_count;
> > + bool streaming;
> > +
> > + struct gpio_desc *rst_gpio;
> > +
> > + /* firmware blocks for each operating mode */
> > + struct addi9036_mode_fw_block *mode_fw_blocks;
> > + u8 curr_operating_mode;
> > +
> > + const struct firmware *fw;
> > +};
> > +
> > +static inline struct addi9036 *to_addi9036(struct v4l2_subdev *sd)
> > +{
> > + return container_of(sd, struct addi9036, sd);
> > +}
> > +
> > +#define V4L2_CID_ADDI9036_OPERATING_MODE
> (V4L2_CID_USER_ADDI9036_BASE + 0)
> > +
> > +static const struct reg_sequence addi9036_powerup_setting[] = {
> > + { 0xc4c0, 0x001c },
> > + { 0xc4c3, 0x001c },
> > + { 0xc4d7, 0x0000 },
> > + { 0xc4d5, 0x0002 },
> > + { 0xc4da, 0x0001 },
> > + { 0xc4f0, 0x0000 },
> > + { 0xc427, 0x0003 },
> > + { 0xc427, 0x0001 },
> > + { 0xc427, 0x0000 },
> > + { 0xc426, 0x0030 },
> > + { 0xc426, 0x0010 },
> > + { 0xc426, 0x0000 },
> > + { 0xc423, 0x0080 },
> > + { 0xc431, 0x0080 },
> > + { 0x4001, 0x0007 },
> > + { 0x7c22, 0x0004 }
> > +};
> > +
> > +static const struct reg_sequence addi9036_powerdown_setting[] = {
> > + { 0xc022, 0x0001 },
> > + { 0x4001, 0x0006 },
> > + { 0x7c22, 0x0004 },
> > + { 0xc431, 0x0082 },
> > + { 0xc423, 0x0000 },
> > + { 0xc426, 0x0020 },
> > + { 0xc427, 0x0002 },
> > + { 0xc4c0, 0x003c },
> > + { 0xc4c3, 0x003c },
> > + { 0xc4d5, 0x0003 },
> > + { 0xc4da, 0x0000 },
> > + { 0xc4d7, 0x0001 },
> > + { 0xc4f0, 0x0001 }
> > +};
> > +
> > +static const s64 link_freq_tbl[] = {
> > + 110529000,
> > + 221184000,
>
> Are these a hardware property or should they come from DT? Either way, not
> both of them could be available on all boards, which suggests DT.
>
Actually both of these are available on all boards. Depending on loaded firmware
the camera can output only one stream of data (IR or DEPTH) and in that case the resolution
will be 640x480 at a lower lane-rate or can output both at the same time and in that case
we have a total image of 640x960 and the lane-rate will also be double.
> > +};
> > +
> > +/* Elements of the structure must be ordered ascending by width & height */
> > +static const struct addi9036_mode_info addi9036_mode_info_data[] = {
> > + {
> > + .width = 640,
> > + .height = 480,
> > + .pixel_rate = 36864000,
> > + .link_freq_idx = 0 /* an index in link_freq_tbl[] */
> > + },
> > + {
> > + .width = 640,
> > + .height = 960,
> > + .pixel_rate = 73728000,
> > + .link_freq_idx = 1 /* an index in link_freq_tbl[] */
> > + },
> > +};
> > +
> > +static bool addi9306_readable_register(struct device *dev, unsigned int reg)
> > +{
> > + if (((reg >= 0x4000) && (reg <= 0x6fff)) ||
> > + ((reg >= 0x7c00) && (reg <= 0x7fff)) ||
> > + ((reg >= 0xc000) && (reg <= 0xc200)) ||
> > + ((reg >= 0xc300) && (reg <= 0xc6bf)))
> > + return true;
> > + else
> > + return false;
> > +}
> > +
> > +static bool addi9306_writeable_register(struct device *dev, unsigned int reg)
> > +{
> > + if (((reg >= 0x4000) && (reg <= 0x6fff)) ||
> > + ((reg >= 0x7c00) && (reg <= 0x7fff)) ||
> > + ((reg >= 0xc000) && (reg <= 0xc113)) ||
> > + ((reg >= 0xc300) && (reg <= 0xc7ff)))
> > + return true;
> > + else
> > + return false;
> > +}
> > +
> > +static const struct regmap_config addi9036_i2c_regmap_config = {
> > + .reg_bits = 16,
> > + .val_bits = 16,
> > + .max_register = 0xc7ff,
> > + .writeable_reg = addi9306_writeable_register,
> > + .readable_reg = addi9306_readable_register,
> > + .cache_type = REGCACHE_NONE,
> > +};
> > +
> > +static int addi9036_set_power_on(struct addi9036 *addi9036)
> > +{
> > + int ret;
> > +
> > + if (addi9036->rst_gpio)
> > + gpiod_set_value_cansleep(addi9036->rst_gpio, 0);
> > +
> > + ret = regmap_register_patch(addi9036->regmap,
> addi9036_powerup_setting,
> > + ARRAY_SIZE(addi9036_powerup_setting));
> > + if (ret)
> > + dev_err(addi9036->dev, "Could not set power up registers\n");
> > +
> > + return ret;
> > +}
> > +
> > +static int addi9036_set_power_off(struct addi9036 *addi9036)
> > +{
> > + int ret;
> > +
> > + ret = regmap_register_patch(addi9036->regmap,
> > + addi9036_powerdown_setting,
> > + ARRAY_SIZE(addi9036_powerdown_setting));
> > + if (ret)
> > + dev_err(addi9036->dev, "could not set power down
> registers\n");
> > +
> > + if (addi9036->rst_gpio)
> > + gpiod_set_value_cansleep(addi9036->rst_gpio, 1);
> > +
> > + return ret;
> > +}
> > +
> > +static int addi9036_s_power(struct v4l2_subdev *sd, int on)
> > +{
> > + struct addi9036 *addi9036 = to_addi9036(sd);
> > + int ret = 0;
> > +
> > + dev_dbg(addi9036->dev, "s_power: %d\n", on);
> > +
> > + mutex_lock(&addi9036->power_lock);
> > +
> > + /* If the power count is modified from 0 to != 0 or from != 0 to 0,
> > + * update the power state.
> > + */
> > + if (addi9036->power_count == !on) {
> > + if (on)
> > + ret = addi9036_set_power_on(addi9036);
> > + else
> > + ret = addi9036_set_power_off(addi9036);
> > + }
> > +
> > + if (!ret) {
> > + /* Update the power count. */
> > + addi9036->power_count += on ? 1 : -1;
> > + WARN(addi9036->power_count < 0, "Unbalanced power
> count\n");
> > + WARN(addi9036->power_count > 1, "Duplicated s_power
> call\n");
> > + }
> > +
> > + mutex_unlock(&addi9036->power_lock);
> > +
> > + return ret;
> > +}
> > +
> > +#ifdef CONFIG_VIDEO_ADV_DEBUG
> > +static int addi9036_g_register(struct v4l2_subdev *sd,
> > + struct v4l2_dbg_register *reg)
> > +{
> > + struct addi9036 *addi9036 = to_addi9036(sd);
> > + unsigned int read_val;
> > + int ret;
> > +
> > + reg->size = 2;
> > + ret = regmap_read(addi9036->regmap, reg->reg, &read_val);
> > + reg->val = read_val;
> > +
> > + return ret;
> > +}
> > +
> > +static int addi9036_s_register(struct v4l2_subdev *sd,
> > + const struct v4l2_dbg_register *reg)
> > +{
> > + struct addi9036 *addi9036 = to_addi9036(sd);
> > +
> > + return regmap_write(addi9036->regmap, reg->reg, reg->val);
> > +}
> > +#endif
> > +
> > +static int addi9036_s_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > + struct addi9036 *addi9036 = container_of(ctrl->handler,
> > + struct addi9036, ctrls);
> > + int ret = 0;
> > +
> > + switch (ctrl->id) {
> > + case V4L2_CID_ADDI9036_OPERATING_MODE:
> > + addi9036->curr_operating_mode = ctrl->val;
> > + break;
> > + case V4L2_CID_PIXEL_RATE:
> > + case V4L2_CID_LINK_FREQ:
> > + break;
> > + default:
> > + dev_err(addi9036->dev, "%s > Unhandled: %x param=%x\n",
> > + __func__, ctrl->id, ctrl->val);
> > + ret = -EINVAL;
> > + break;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static const struct v4l2_ctrl_ops addi9036_ctrl_ops = {
> > + .s_ctrl = addi9036_s_ctrl,
> > +};
> > +
> > +static const struct v4l2_ctrl_config addi9036_ctrl_operating_mode = {
> > + .ops = &addi9036_ctrl_ops,
> > + .id = V4L2_CID_ADDI9036_OPERATING_MODE,
> > + .name = "Operating Mode",
> > + .type = V4L2_CTRL_TYPE_INTEGER,
> > + .def = 0,
> > + .min = 0,
> > + .max = 1,
> > + .step = 1,
> > +};
> > +
> > +static int addi9036_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 = MEDIA_BUS_FMT_SBGGR12_1X12;
> > +
> > + return 0;
> > +}
> > +
> > +static int addi9036_enum_frame_size(struct v4l2_subdev *subdev,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_frame_size_enum *fse)
> > +{
> > + if (fse->code != MEDIA_BUS_FMT_SBGGR12_1X12)
> > + return -EINVAL;
> > +
> > + if (fse->index >= ARRAY_SIZE(addi9036_mode_info_data))
> > + return -EINVAL;
> > +
> > + fse->min_width = addi9036_mode_info_data[fse->index].width;
> > + fse->max_width = addi9036_mode_info_data[fse->index].width;
> > + fse->min_height = addi9036_mode_info_data[fse->index].height;
> > + fse->max_height = addi9036_mode_info_data[fse->index].height;
> > +
> > + return 0;
> > +}
> > +
> > +static struct v4l2_mbus_framefmt *
> > +addi9036_get_pad_format(struct addi9036 *addi9036,
> > + 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(&addi9036->sd, cfg, pad);
> > + case V4L2_SUBDEV_FORMAT_ACTIVE:
> > + return &addi9036->fmt;
> > + default:
> > + return NULL;
>
> I'd suggest never return NULL here. Maybe issue a warning and return either
> of the valid ones? Or add NULL checks elsewhere.
ACK
>
> > + }
> > +}
> > +
> > +static int addi9036_get_format(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_format *format)
> > +{
> > + struct addi9036 *addi9036 = to_addi9036(sd);
> > + struct v4l2_mbus_framefmt *pad_format;
> > +
> > + pad_format = addi9036_get_pad_format(addi9036, cfg, format->pad,
> > + format->which);
> > +
> > + if (!pad_format)
> > + return -EINVAL;
> > +
> > + format->format = *pad_format;
> > +
> > + return 0;
> > +}
> > +
> > +static struct v4l2_rect *
> > +addi9036_get_pad_crop(struct addi9036 *addi9036,
> > + 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(&addi9036->sd, cfg, pad);
> > + case V4L2_SUBDEV_FORMAT_ACTIVE:
> > + return &addi9036->crop;
> > + default:
> > + return NULL;
> > + }
> > +}
> > +
> > +static int addi9036_set_format(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_format *format)
> > +{
> > + struct addi9036 *addi9036 = to_addi9036(sd);
> > + struct v4l2_mbus_framefmt *framefmt;
> > + struct v4l2_rect *crop;
> > + const struct addi9036_mode_info *new_mode;
> > + int ret;
> > +
> > + dev_dbg(addi9036->dev, "set_fmt: %x %dx%d %d\n",
> > + format->format.code, format->format.width,
> > + format->format.height, format->which);
> > +
> > + crop = addi9036_get_pad_crop(addi9036, cfg, format->pad,
> > + format->which);
> > +
> > + if (!crop)
> > + return -EINVAL;
> > +
> > + new_mode = v4l2_find_nearest_size(addi9036_mode_info_data,
> > +
> ARRAY_SIZE(addi9036_mode_info_data),
> > + width, height, format->format.width,
> > + format->format.height);
> > + crop->width = new_mode->width;
> > + crop->height = new_mode->height;
> > +
> > + if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> > + ret = v4l2_ctrl_s_ctrl_int64(addi9036->pixel_rate,
> > + new_mode->pixel_rate);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = v4l2_ctrl_s_ctrl(addi9036->link_freq,
> > + new_mode->link_freq_idx);
> > + if (ret < 0)
> > + return ret;
> > +
> > + addi9036->current_mode = new_mode;
> > + }
> > +
> > + framefmt = addi9036_get_pad_format(addi9036, cfg, format->pad,
> > + format->which);
>
> I believe you'll need to serialise access to framefmt.
ACK
>
> > + framefmt->width = crop->width;
> > + framefmt->height = crop->height;
> > + framefmt->code = MEDIA_BUS_FMT_SBGGR12_1X12;
> > + framefmt->field = V4L2_FIELD_NONE;
> > + framefmt->colorspace = V4L2_COLORSPACE_SRGB;
> > +
> > + format->format = *framefmt;
> > +
> > + return 0;
> > +}
> > +
> > +static int addi9036_entity_init_cfg(struct v4l2_subdev *subdev,
> > + struct v4l2_subdev_pad_config *cfg)
> > +{
> > + struct v4l2_subdev_format fmt = { 0 };
> > +
> > + fmt.which = cfg ? V4L2_SUBDEV_FORMAT_TRY :
> V4L2_SUBDEV_FORMAT_ACTIVE;
> > + fmt.format.width = addi9036_mode_info_data[1].width;
> > + fmt.format.height = addi9036_mode_info_data[1].height;
> > +
> > + addi9036_set_format(subdev, cfg, &fmt);
> > +
> > + return 0;
> > +}
> > +
> > +static int addi9036_get_selection(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_selection *sel)
> > +{
> > + struct addi9036 *addi9036 = to_addi9036(sd);
> > +
> > + if (sel->target != V4L2_SEL_TGT_CROP)
> > + return -EINVAL;
> > +
> > + sel->r = *addi9036_get_pad_crop(addi9036, cfg, sel->pad, sel->which);
> > +
> > + return 0;
> > +}
> > +
> > +static int addi9036_s_stream(struct v4l2_subdev *subdev, int enable)
> > +{
> > + struct addi9036 *addi9036 = to_addi9036(subdev);
> > + uint8_t mode = addi9036->curr_operating_mode;
> > + int ret = 0;
> > +
> > + dev_dbg(addi9036->dev, "s_stream: %d\n", enable);
> > +
> > + if (addi9036->streaming == enable)
> > + return 0;
> > +
> > + if (enable) {
> > + if (addi9036->mode_fw_blocks[mode].mode_regs == NULL) {
> > + dev_err(addi9036->dev, "Selected mode has no data\n");
> > + return -EINVAL;
> > + }
> > +
> > + dev_dbg(addi9036->dev, "Applying mode: %u\n", mode);
> > + ret = regmap_multi_reg_write(addi9036->regmap,
> > + addi9036->mode_fw_blocks[mode].mode_regs,
> > + addi9036->mode_fw_blocks[mode].regs_count);
> > +
> > + dev_dbg(addi9036->dev, "Writen %lu registers\n",
> > + addi9036->mode_fw_blocks[mode].regs_count);
> > + }
> > +
> > + addi9036->streaming = enable;
> > + return ret;
> > +}
> > +
> > +static const struct v4l2_subdev_core_ops addi9036_core_ops = {
> > + .s_power = addi9036_s_power,
>
> Please add support for runtime PM instead.
ACK
>
> > +#ifdef CONFIG_VIDEO_ADV_DEBUG
> > + .g_register = addi9036_g_register,
> > + .s_register = addi9036_s_register,
> > +#endif
> > +};
> > +
> > +static const struct v4l2_subdev_video_ops addi9036_video_ops = {
> > + .s_stream = addi9036_s_stream,
> > +};
> > +
> > +static const struct v4l2_subdev_pad_ops addi9036_subdev_pad_ops = {
> > + .init_cfg = addi9036_entity_init_cfg,
> > + .enum_mbus_code = addi9036_enum_mbus_code,
> > + .enum_frame_size = addi9036_enum_frame_size,
> > + .get_fmt = addi9036_get_format,
> > + .set_fmt = addi9036_set_format,
> > + .get_selection = addi9036_get_selection,
> > +};
> > +
> > +static const struct v4l2_subdev_ops addi9036_subdev_ops = {
> > + .core = &addi9036_core_ops,
> > + .video = &addi9036_video_ops,
> > + .pad = &addi9036_subdev_pad_ops,
> > +};
> > +
> > +static int addi9036_g_modes_from_firmware(struct v4l2_subdev *sd)
> > +{
> > + struct addi9036 *addi9036 = to_addi9036(sd);
> > + const struct firmware *fw = addi9036->fw;
> > + const struct addi9036_fw_header *fw_head;
> > + const struct addi9036_mode_block *mode_block;
> > + unsigned int reg_nr, chunk_len, pos, modes_nr, i, j, k;
> > + struct reg_sequence *reg_seq;
> > +
> > + /*
> > + * Reject too small or unreasonable large files.
> > + */
> > +
> > + if (fw->size < sizeof(struct addi9036_fw_header) ||
> > + fw->size >= 0x4000000) {
> > + dev_err(addi9036->dev, "FW loading failed: Invalid size\n");
> > + return -EINVAL;
> > + }
> > +
> > + fw_head = (struct addi9036_fw_header *)fw->data;
> > +
> > + if (memcmp(fw_head->magic, ADDI_MAGIC, ARRAY_SIZE(fw_head-
> >magic))) {
> > + dev_err(addi9036->dev, "FW loading failed: Invalid magic\n");
> > + return -EINVAL;
> > + }
> > +
> > + modes_nr = le32_to_cpu(fw_head->modes_nr);
> > +
> > + if (modes_nr == 0) {
> > + dev_err(addi9036->dev, "FW should contain at least 1 mode.\n");
> > + return -EINVAL;
> > + }
> > +
> > + __v4l2_ctrl_modify_range(addi9036->set_operating_range,
> > + addi9036->set_operating_range->minimum,
> > + modes_nr - 1, 1, 0);
> > +
> > + addi9036->mode_fw_blocks = devm_kzalloc(addi9036->dev,
> > + sizeof(struct addi9036_mode_fw_block) * modes_nr,
>
> devm_kcalloc, please.
ACK
>
> > + GFP_KERNEL);
> > + if (!addi9036->mode_fw_blocks)
> > + return -ENOMEM;
> > +
> > + pos = sizeof(struct addi9036_fw_header);
> > +
> > + for (i = 0; i < modes_nr; i++) {
> > + mode_block = (struct addi9036_mode_block *)(fw->data + pos);
>
> I think you need more validation here. For instance, make sure the firmware
> binary is at least as big as the offset you're accessing.
ACK
>
> > +
> > + chunk_len = le32_to_cpu(mode_block->size_bytes);
> > + reg_nr = chunk_len / sizeof(uint16_t) / 2;
> > +
> > + reg_seq = devm_kzalloc(addi9036->dev,
> > + sizeof(struct reg_sequence) * reg_nr,
> > + GFP_KERNEL);
> > + if (!reg_seq)
> > + return -ENOMEM;
> > +
> > + k = 0;
> > + for (j = 0; j < reg_nr * 2; j += 2) {
>
> j = 0, k = 0
ACK
>
> > + reg_seq[k].reg = le16_to_cpu(mode_block->data[j]);
> > + reg_seq[k].def = le16_to_cpu(mode_block->data[j + 1]);
> > + k++;
> > + }
> > +
> > + addi9036->mode_fw_blocks[i].mode_regs = reg_seq;
> > + addi9036->mode_fw_blocks[i].regs_count = reg_nr;
> > +
> > + pos += chunk_len + sizeof(struct addi9036_mode_block);
> > + }
> > + return 0;
> > +}
> > +
> > +static int addi9036_mode_firmware_load(struct v4l2_subdev *sd)
> > +{
> > + struct addi9036 *addi9036 = to_addi9036(sd);
> > + int ret;
> > +
> > + ret = request_firmware(&addi9036->fw, FW_FILE_NAME, addi9036-
> >dev);
> > + if (ret < 0) {
> > + dev_err(addi9036->dev, "FW request failed\n");
> > + return ret;
> > + }
> > +
> > + ret = addi9036_g_modes_from_firmware(sd);
> > +
> > + release_firmware(addi9036->fw);
> > + if (ret < 0) {
> > + dev_err(addi9036->dev, "FW parsing failed\n");
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int addi9036_probe(struct i2c_client *client)
> > +{
> > + struct device *dev = &client->dev;
> > + struct fwnode_handle *endpoint;
> > + struct addi9036 *addi9036;
> > + int ret;
> > +
> > + dev_dbg(dev, "%s: i2c addr = 0x%x\n", __func__, client->addr);
> > +
> > + addi9036 = devm_kzalloc(dev, sizeof(struct addi9036), GFP_KERNEL);
> > + if (!addi9036)
> > + return -ENOMEM;
> > +
> > + addi9036->dev = dev;
> > +
> > + addi9036->regmap = devm_regmap_init_i2c(client,
> > + &addi9036_i2c_regmap_config);
> > + if (IS_ERR(addi9036->regmap)) {
> > + dev_err(dev, "Error initializing i2c regmap\n");
> > + return PTR_ERR(addi9036->regmap);
> > + }
> > +
> > + mutex_init(&addi9036->power_lock);
> > +
> > + endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(dev),
> NULL);
>
> Could you use fwnode_graph_get_endpoint_by_id()?
ACK
>
> > + if (!endpoint) {
> > + dev_err(dev, "endpoint node not found\n");
> > + return -EINVAL;
> > + }
> > +
> > + ret = v4l2_fwnode_endpoint_parse(endpoint, &addi9036->ep);
> > + if (ret < 0) {
> > + dev_err(dev, "parsing endpoint node failed\n");
> > + return ret;
> > + }
> > + fwnode_handle_put(endpoint);
> > +
> > + if (addi9036->ep.bus_type != V4L2_MBUS_CSI2_DPHY) {
>
> If you expect D-PHY, please specify the bus type before calling
> v4l2_fwnode_endpoint_parse().
ACK
>
> > + dev_err(dev, "invalid bus type, must be MIPI CSI2\n");
> > + return -EINVAL;
> > + }
> > +
> > + addi9036->rst_gpio = gpiod_get_optional(dev, "reset",
> GPIOD_OUT_HIGH);
> > + if (IS_ERR(addi9036->rst_gpio))
> > + dev_info(dev, "Unable to get \"reset\" gpio\n");
> > +
> > + v4l2_ctrl_handler_init(&addi9036->ctrls, 3);
> > +
> > + addi9036->pixel_rate = v4l2_ctrl_new_std(&addi9036->ctrls,
> > + &addi9036_ctrl_ops,
> > + V4L2_CID_PIXEL_RATE,
> > + 1, INT_MAX, 1, 1);
> > + addi9036->link_freq = v4l2_ctrl_new_int_menu(&addi9036->ctrls,
> > + &addi9036_ctrl_ops,
> > + V4L2_CID_LINK_FREQ,
> > + ARRAY_SIZE(
> > + link_freq_tbl) - 1,
> > + 0, link_freq_tbl);
> > + if (addi9036->link_freq)
> > + addi9036->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> > +
> > + addi9036->set_operating_range = v4l2_ctrl_new_custom(&addi9036-
> >ctrls,
> > + &addi9036_ctrl_operating_mode,
> > + NULL);
> > +
> > + ret = addi9036->ctrls.error;
> > + if (ret) {
> > + dev_err(dev, "%s: control initialization error %d\n",
> > + __func__, ret);
> > + goto free_ctrl;
> > + }
> > + addi9036->sd.ctrl_handler = &addi9036->ctrls;
> > +
> > + v4l2_i2c_subdev_init(&addi9036->sd, client, &addi9036_subdev_ops);
> > + addi9036->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > + addi9036->pad.flags = MEDIA_PAD_FL_SOURCE;
> > + addi9036->sd.dev = &client->dev;
> > + addi9036->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > +
> > + ret = media_entity_pads_init(&addi9036->sd.entity, 1, &addi9036->pad);
> > + if (ret < 0) {
> > + dev_err(dev, "could not register media entity\n");
> > + goto free_ctrl;
> > + }
> > +
> > + ret = addi9036_mode_firmware_load(&addi9036->sd);
> > + if (ret < 0)
> > + return ret;
> > +
> > + addi9036_entity_init_cfg(&addi9036->sd, NULL);
> > +
> > + ret = v4l2_async_register_subdev(&addi9036->sd);
> > + if (ret < 0) {
> > + dev_err(dev, "could not register v4l2 device\n");
> > + goto free_entity;
> > + }
> > +
> > + return 0;
> > +
> > +free_entity:
> > + media_entity_cleanup(&addi9036->sd.entity);
> > +free_ctrl:
> > + v4l2_ctrl_handler_free(&addi9036->ctrls);
> > + mutex_destroy(&addi9036->power_lock);
> > +
> > + return ret;
> > +}
> > +
> > +static int addi9036_remove(struct i2c_client *client)
> > +{
> > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > + struct addi9036 *addi9036 = to_addi9036(sd);
> > +
> > + v4l2_async_unregister_subdev(&addi9036->sd);
> > + media_entity_cleanup(&addi9036->sd.entity);
> > + if (addi9036->rst_gpio)
> > + gpiod_put(addi9036->rst_gpio);
> > + v4l2_ctrl_handler_free(&addi9036->ctrls);
> > + mutex_destroy(&addi9036->power_lock);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct i2c_device_id addi9036_id[] = {
>
> Do you really need the I²C device table? Please remove if not.
Not really needed so will be removed in V3
>
> > + { "addi9036", 0 },
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(i2c, addi9036_id);
> > +
> > +static const struct of_device_id addi9036_of_match[] = {
> > + { .compatible = "adi,addi9036" },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, addi9036_of_match);
> > +
> > +static struct i2c_driver addi9036_i2c_driver = {
> > + .driver = {
> > + .of_match_table = addi9036_of_match,
> > + .name = "addi9036",
> > + },
> > + .probe_new = addi9036_probe,
> > + .remove = addi9036_remove,
> > + .id_table = addi9036_id,
> > +};
> > +
> > +module_i2c_driver(addi9036_i2c_driver);
> > +
> > +MODULE_DESCRIPTION("Analog Devices ADDI9036 Camera Driver");
> > +MODULE_AUTHOR("Bogdan Togorean");
> > +MODULE_LICENSE("GPL v2");
> > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-
> controls.h
> > index 62271418c1be..f88b56479bc1 100644
> > --- a/include/uapi/linux/v4l2-controls.h
> > +++ b/include/uapi/linux/v4l2-controls.h
> > @@ -198,6 +198,12 @@ enum v4l2_colorfx {
> > */
> > #define V4L2_CID_USER_ATMEL_ISC_BASE (V4L2_CID_USER_BASE +
> 0x10c0)
> >
> > +/*
> > + * The base for the addi9036 driver controls.
> > + * We reserve 16 controls for this driver.
> > + */
> > +#define V4L2_CID_USER_ADDI9036_BASE (V4L2_CID_USER_BASE +
> 0x10e0)
> > +
> > /* MPEG-class control IDs */
> > /* The MPEG controls are applicable to all codec controls
> > * and the 'MPEG' part of the define is historical */
>
> --
> Sakari Ailus
Regards, Bogdan
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-10-08 6:50 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-02 13:35 [PATCH v2 1/2] media: i2c: Add driver for the Analog Devices ADDI9036 ToF front-end Bogdan Togorean
2020-10-02 13:35 ` [PATCH v2 2/2] media: dt-bindings: media: i2c: Add bindings for ADDI9036 Bogdan Togorean
2020-10-06 20:47 ` Rob Herring
2020-10-08 6:23 ` Togorean, Bogdan
2020-10-08 6:35 ` Sakari Ailus
2020-10-08 6:42 ` Togorean, Bogdan
2020-10-07 11:18 ` [PATCH v2 1/2] media: i2c: Add driver for the Analog Devices ADDI9036 ToF front-end Sakari Ailus
2020-10-08 6:49 ` Togorean, Bogdan
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).