Hi Spencer,
thanks for the patch.
Just having a glimpse and giving some feedback.
Am Mittwoch, 26. Juni 2024, 23:15:28 CEST schrieb Spencer Hill:
> Add a driver for the Sony IMX728 image sensor.
>
> Signed-off-by: Spencer Hill <shill@xxxxxxxxxxxxxxxxx>
> ---
> drivers/media/i2c/Kconfig | 11 +
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/imx728.c | 1167 ++++++++++++
> drivers/media/i2c/imx728.h | 3458 ++++++++++++++++++++++++++++++++++++
> 4 files changed, 4637 insertions(+)
> create mode 100644 drivers/media/i2c/imx728.c
> create mode 100644 drivers/media/i2c/imx728.h
>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index c6d3ee472d81..46b6463c558a 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -233,6 +233,17 @@ config VIDEO_IMX415
> To compile this driver as a module, choose M here: the
> module will be called imx415.
>
> +config VIDEO_IMX728
> + tristate "Sony IMX728 sensor support"
> + depends on OF_GPIO
> + select V4L2_CCI_I2C
> + help
> + This is a Video4Linux2 sensor driver for the Sony
> + IMX728 camera.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called imx728.
> +
> config VIDEO_MAX9271_LIB
> tristate
>
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index dfbe6448b549..1188420ee1b4 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -56,6 +56,7 @@ obj-$(CONFIG_VIDEO_IMX335) += imx335.o
> obj-$(CONFIG_VIDEO_IMX355) += imx355.o
> obj-$(CONFIG_VIDEO_IMX412) += imx412.o
> obj-$(CONFIG_VIDEO_IMX415) += imx415.o
> +obj-$(CONFIG_VIDEO_IMX728) += imx728.o
> obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o
> obj-$(CONFIG_VIDEO_ISL7998X) += isl7998x.o
> obj-$(CONFIG_VIDEO_KS0127) += ks0127.o
> diff --git a/drivers/media/i2c/imx728.c b/drivers/media/i2c/imx728.c
> new file mode 100644
> index 000000000000..b23359133a22
> --- /dev/null
> +++ b/drivers/media/i2c/imx728.c
> @@ -0,0 +1,1167 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Sony IMX728 CMOS Image Sensor Driver
> + *
> + * Copyright (c) 2024 Define Design Deploy Corp
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/clk.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/v4l2-mediabus.h>
> +#include <linux/videodev2.h>
> +#include <media/v4l2-subdev.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-event.h>
> +
> +#include "imx728.h"
> +
> +static inline struct imx728 *to_imx728(struct v4l2_subdev *sd)
> +{
> + return container_of(sd, struct imx728, subdev);
> +}
> +
> +static int imx728_read(struct imx728 *imx728, u16 addr, u32 *val, size_t nbytes)
Use cci_read instead and remove this function.
> +{
> + int ret;
> + __le32 val_le = 0;
> +
> + ret = regmap_bulk_read(imx728->regmap, addr, &val_le, nbytes);
> + if (ret < 0) {
> + dev_err(imx728->dev, "%s: failed to read reg 0x%04x: %d\n",
> + __func__, addr, ret);
> + return ret;
> + }
> +
> + *val = le32_to_cpu(val_le);
> +
> + return 0;
> +}
> +
> +static int imx728_write(struct imx728 *imx728, u16 addr, u32 val, size_t nbytes)
Use cci_write instead and remove this function.
> +{
> + int ret;
> + __le32 val_le = cpu_to_le32(val);
> +
> + ret = regmap_bulk_write(imx728->regmap, addr, &val_le, nbytes);
> + if (ret < 0)
> + dev_err(imx728->dev, "%s: failed to write reg 0x%04x: %d\n",
> + __func__, addr, ret);
> +
> + return ret;
> +}
> +
> +static int imx728_update_bits(struct imx728 *imx728, u16 addr, u32 val, u32 mask, size_t nbytes)
Use cci_update_bits instead and remove this function.
> +{
> + int ret;
> + u32 cfg;
> +
> + ret = imx728_read(imx728, addr, &cfg, nbytes);
> + if (ret < 0)
> + return ret;
> +
> + cfg = (val) ? (cfg | mask) : (cfg & (~mask));
> +
> + return imx728_write(imx728, addr, cfg, nbytes);
> +}
> +
> +static int imx728_write_table(struct imx728 *imx728,
> + const struct reg_sequence *regs,
> + unsigned int nr_regs)
Use cci_multi_reg_write instead and remove this function.
> +{
> + int ret;
> +
> + ret = regmap_multi_reg_write(imx728->regmap, regs, nr_regs);
> + if (ret < 0)
> + dev_err(imx728->dev,
> + "%s: failed to write reg table (%d)!\n", __func__, ret);
> + return ret;
> +}
> +
> +static int imx728_wait_for_state(struct imx728 *imx728, enum imx728_sensor_state state)
> +{
> + int ret, i;
> + u32 val;
> +
> + for (i = 0; i < 50; i++) {
> + ret = imx728_read(imx728, 0x2CAC, &val, 1);
Please add proper register defines using CCI_REG* macros.
> + if (ret == 0 && val == state) {
> + dev_info(imx728->dev, "%s: Enter state %u\n", __func__, val);
> + return 0;
> + }
> + usleep_range(1000, 10000);
> + }
> +
> + return -EBUSY;
> +}
> +
> +static void imx728_init_formats(struct v4l2_subdev_state *state)
> +{
> + struct v4l2_mbus_framefmt *format;
> +
> + format = v4l2_subdev_state_get_format(state, 0, 0);
> + format->code = imx728_mbus_formats[0];
> + format->width = imx728_framesizes[0].width;
> + format->height = imx728_framesizes[0].height;
> + format->field = V4L2_FIELD_NONE;
> + format->colorspace = V4L2_COLORSPACE_SMPTE170M;
Are you sure about this colorspace? I would have expected
V4L2_COLORSPACE_RAW, but I don't know any details on this hardware.
Also set ycbcr_enc, quantization and xfer_func.
> +}
> +
> +static int _imx728_set_routing(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state)
Why this special variant with a underscore? Just move the code
into imx728_set_routing.
> +{
> + struct v4l2_subdev_route routes[] = {
> + {
> + .source_pad = 0,
> + .source_stream = 0,
> + .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> + },
> + {
> + .source_pad = 0,
> + .source_stream = 1,
> + }
> + };
> +
> + struct v4l2_subdev_krouting routing = {
> + .num_routes = ARRAY_SIZE(routes),
> + .routes = routes,
> + };
> +
> + int ret;
> +
> + ret = v4l2_subdev_set_routing(sd, state, &routing);
> + if (ret < 0)
> + return ret;
> +
> + imx728_init_formats(state);
> +
> + return 0;
> +}
> +
> +static int imx728_enum_mbus_code(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + struct v4l2_subdev_mbus_code_enum *code)
> +{
> +
> + if (code->index >= ARRAY_SIZE(imx728_mbus_formats))
> + return -EINVAL;
> +
> + code->code = imx728_mbus_formats[code->index];
> +
> + return 0;
> +}
> +
> +static int imx728_enum_frame_sizes(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + struct v4l2_subdev_frame_size_enum *fse)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(imx728_mbus_formats); ++i) {
> + if (imx728_mbus_formats[i] == fse->code)
> + break;
> + }
> +
> + if (i == ARRAY_SIZE(imx728_mbus_formats))
> + return -EINVAL;
> +
> + if (fse->index >= ARRAY_SIZE(imx728_framesizes))
> + return -EINVAL;
> +
> + fse->min_width = imx728_framesizes[fse->index].width;
> + fse->max_width = fse->min_width;
> + fse->min_height = imx728_framesizes[fse->index].height;
> + fse->max_height = fse->min_height;
> +
> + return 0;
> +}
> +
> +static int imx728_set_fmt(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct imx728 *imx728 = to_imx728(sd);
> + struct v4l2_mbus_framefmt *format;
> + const struct v4l2_area *fsize;
> + unsigned int i;
> + u32 code;
> + int ret = 0;
> +
> + if (fmt->pad != 0)
> + return -EINVAL;
> +
> + if (fmt->stream != 0)
> + return -EINVAL;
> +
> + for (i = 0; i < ARRAY_SIZE(imx728_mbus_formats); ++i) {
> + if (imx728_mbus_formats[i] == fmt->format.code) {
> + code = fmt->format.code;
> + break;
> + }
> + }
> +
> + if (i == ARRAY_SIZE(imx728_mbus_formats))
> + code = imx728_mbus_formats[0];
> +
> + fsize = v4l2_find_nearest_size(imx728_framesizes,
> + ARRAY_SIZE(imx728_framesizes), width,
> + height, fmt->format.width,
> + fmt->format.height);
> +
> + mutex_lock(&imx728->lock);
> +
> + format = v4l2_subdev_state_get_format(state, fmt->pad, fmt->stream);
> +
> + if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE && imx728->streaming) {
> + ret = -EBUSY;
> + goto done;
> + }
> +
> + format->code = code;
> + format->width = fsize->width;
> + format->height = fsize->height;
This should be done before calling v4l2_subdev_state_get_format, no?
Also ycbcr_enc, quantization and xfer_func are missing.
> +
> + fmt->format = *format;
> +
> +done:
> + mutex_unlock(&imx728->lock);
> +
> + return ret;
> +}
> +
> +static int imx728_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> + struct v4l2_mbus_frame_desc *fd)
> +{
> + struct v4l2_subdev_state *state;
> + struct v4l2_mbus_framefmt *fmt;
> + u32 bpp;
> + int ret = 0;
> +
> + if (pad != 0)
> + return -EINVAL;
> +
> + state = v4l2_subdev_lock_and_get_active_state(sd);
> +
> + fmt = v4l2_subdev_state_get_format(state, 0, 0);
> + if (!fmt) {
> + ret = -EPIPE;
> + goto out;
> + }
> +
> + memset(fd, 0, sizeof(*fd));
> +
> + fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
> +
> + bpp = 10;
> +
> + fd->entry[fd->num_entries].stream = 0;
> +
> + fd->entry[fd->num_entries].flags = V4L2_MBUS_FRAME_DESC_FL_LEN_MAX;
> + fd->entry[fd->num_entries].length = fmt->width * fmt->height * bpp / 8;
> + fd->entry[fd->num_entries].pixelcode = fmt->code;
> + fd->entry[fd->num_entries].bus.csi2.vc = 0;
> + fd->entry[fd->num_entries].bus.csi2.dt = 0x2b;
> +
> + fd->num_entries++;
> +
> +out:
> +
> + v4l2_subdev_unlock_state(state);
> +
> + return ret;
> +}
> +
> +static int imx728_set_routing(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + enum v4l2_subdev_format_whence which,
> + struct v4l2_subdev_krouting *routing)
> +{
> + int ret;
> +
> + if (routing->num_routes == 0 || routing->num_routes > 1)
> + return -EINVAL;
> +
> + ret = _imx728_set_routing(sd, state);
> +
> + return ret;
> +}
> +
> +static int imx728_set_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct imx728 *imx728 = container_of(ctrl->handler,
> + struct imx728, ctrl.handler);
> + int ret = 0;
> +
> + dev_dbg(imx728->dev, "%s: %s, value: %d\n",
> + __func__, ctrl->name, ctrl->val);
> +
> + if (!pm_runtime_get_if_in_use(imx728->dev))
> + return 0;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_EXPOSURE:
> + ret = imx728_write(imx728, 0x98DC, ctrl->val, 4);
> + break;
> + case V4L2_CID_ANALOGUE_GAIN:
> + ret = imx728_update_bits(imx728, 0x98F8,
> + (ctrl->val * 10),
> + 0x1FFFF, 4);
> + break;
> + case V4L2_CID_HFLIP:
> + ret = imx728_update_bits(imx728, 0x9651,
> + ctrl->val, 0x1, 1);
> + ret |= imx728_update_bits(imx728, 0xB67C,
> + ctrl->val, 0x1, 1);
> + break;
> + case V4L2_CID_VFLIP:
> + ret = imx728_update_bits(imx728, 0x9651,
> + ctrl->val << 1, 0x2, 1);
> + ret = imx728_update_bits(imx728, 0xB67D,
> + ctrl->val, 0x1, 1);
> + break;
> + case V4L2_CID_WIDE_DYNAMIC_RANGE:
> + case V4L2_CID_TEST_PATTERN:
> + // Both of these are configured during start stream.
Are they not configurable while streaming?
> + ret = 0;
> + break;
> + default:
> + ret = -EINVAL;
> + }
> +
> + pm_runtime_put_noidle(imx728->dev);
> + return ret;
> +}
> +
> +static int imx728_detect(struct imx728 *imx728)
> +{
> + int ret;
> + u32 minor, major;
> +
> + ret = imx728_read(imx728, 0x6002, &major, 1);
> + if (ret != 0) {
> + dev_err(imx728->dev, "Could not read PARAM_MAJOR_VER!");
> + return ret;
> + }
> + ret = imx728_read(imx728, 0x6000, &minor, 1);
> + if (ret != 0) {
> + dev_err(imx728->dev, "Could not read PARAM_MINOR_VER!");
> + return ret;
> + }
> + dev_dbg(imx728->dev, "Got version: %d.%d", major, minor);
> +
> + return 0;
> +}
> +
> +static int imx728_reset(struct imx728 *imx728)
> +{
> +
> + if (!IS_ERR_OR_NULL(imx728->xclr_gpio)) {
> + gpiod_set_value_cansleep(imx728->xclr_gpio, 1);
> + usleep_range(1000, 10000);
> + gpiod_set_value_cansleep(imx728->xclr_gpio, 0);
> + msleep(100);
> +
> + return 0;
> + }
> +
> + return -1;
> +}
> +
> +static int imx728_power_on(struct imx728 *imx728)
> +{
> + int ret;
> +
> + ret = clk_prepare_enable(imx728->clk);
> + if (ret < 0)
> + return ret;
> +
> + imx728_reset(imx728);
> + return 0;
> +}
> +
> +static int imx728_power_off(struct imx728 *imx728)
> +{
> +
> + if (imx728->xclr_gpio) {
> + gpiod_set_value_cansleep(imx728->xclr_gpio, 1);
> +
> + usleep_range(1, 10);
> + }
> + clk_disable_unprepare(imx728->clk);
> + return 0;
> +}
> +
> +static int imx728_get_frame_interval(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_frame_interval *fi)
> +{
> + struct imx728 *imx728 = to_imx728(sd);
> +
> + fi->interval.numerator = 1;
> + fi->interval.denominator = imx728->fps;
> + return 0;
> +}
> +
> +static int imx728_set_frame_interval(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_frame_interval *fi)
> +{
> + struct imx728 *imx728 = to_imx728(sd);
> + u32 req_fps;
> +
> + mutex_lock(&imx728->lock);
> +
> + if (fi->interval.numerator == 0 || fi->interval.denominator == 0) {
> + fi->interval.denominator = IMX728_FRAMERATE_DEFAULT;
> + fi->interval.numerator = 1;
> + }
> +
> + req_fps = clamp_val(DIV_ROUND_CLOSEST(fi->interval.denominator,
> + fi->interval.numerator),
> + IMX728_FRAMERATE_MIN, IMX728_FRAMERATE_MAX);
> +
> + fi->interval.numerator = 1;
> + fi->interval.denominator = req_fps;
> +
> + imx728->fps = req_fps;
> +
> + mutex_unlock(&imx728->lock);
> + dev_dbg(imx728->dev, "%s frame rate = %d\n", __func__, imx728->fps);
> +
> + return 0;
> +}
> +
> +static int imx728_powerup_to_standby(struct imx728 *imx728)
> +{
> + int ret;
> +
> + dev_info(imx728->dev, "powerup -> standby...");
> +
> + ret = imx728_reset(imx728);
> + if (ret) {
> + dev_err(imx728->dev, "Error resetting: %i", ret);
> + return ret;
> + }
> +
> + ret = imx728_wait_for_state(imx728, IMX728_SENSOR_STATE_SLEEP);
> + if (ret < 0) {
> + dev_err(imx728->dev, "Could not transition to Sleep state!");
> + return ret;
> + }
> +
> + ret = imx728_write(imx728, 0x1B20, imx728->clk_rate / 1000000, 1);
> + if (ret < 0) {
> + dev_err(imx728->dev, "Couldn't write INCK frequency!");
> + return ret;
> + }
> +
> + ret = imx728_write(imx728, 0x1B1C, 0x1, 1);
> + if (ret < 0) {
> + dev_err(imx728->dev, "Couldn't write INCK frequency!");
Error message doesn't seem to match. This is a fixed write, independent from any frequency.
> + return ret;
> + }
> +
> + ret = imx728_write(imx728, 0x1B05, 0xFF, 1);
> + if (ret < 0) {
> + dev_err(imx728->dev, "Couldn't write to CK_SLEEP!");
> + return ret;
> + }
> +
> + ret = imx728_wait_for_state(imx728, IMX728_SENSOR_STATE_STANDBY);
> + if (ret < 0) {
> + dev_err(imx728->dev, "Couldn't transition from Sleep to Standby state!");
> + return ret;
> + }
> +
> + ret = imx728_write(imx728, 0xFFFF, IMX728_REMAP_MODE_STANDBY, 1);
> + if (ret < 0) {
> + dev_err(imx728->dev, "Couldn't write regmap mode!");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int imx728_hdr_fixed_brightness(struct imx728 *imx728)
> +{
> + int ret;
> +
> + // Sony recommended values
> + unsigned int exposure_sp1_sp2_us = 10000;
> + unsigned int exposure_sp1vs_us = 56;
> + unsigned int sp1h_gain = 240;
> + unsigned int sp1l_gain = 75;
> + unsigned int sp1ec_gain = 21;
> + unsigned int sp2_gain = 33;
> + unsigned int sp1vs_gain = 84;
> +
> + ret = imx728_write(imx728, 0x98DC, exposure_sp1_sp2_us, 4);
> + ret |= imx728_write(imx728, 0x98E4, exposure_sp1_sp2_us, 4);
> + ret |= imx728_write(imx728, 0x98EC, exposure_sp1vs_us, 4);
> + if (ret < 0) {
> + dev_err(imx728->dev, "Failed to write fixed exposure values.");
> + return ret;
> + }
> +
> + ret = imx728_update_bits(imx728, 0x98F8,
> + sp1h_gain,
> + 0x1FFFF,
> + 4);
> + ret |= imx728_update_bits(imx728, 0x98FC,
> + sp1l_gain,
> + 0x1FFFF,
> + 4);
> + ret |= imx728_update_bits(imx728, 0x9900,
> + sp1ec_gain,
> + 0x1FFFF,
> + 4);
> + ret |= imx728_update_bits(imx728, 0x9904,
> + sp2_gain,
> + 0x1FFFF,
> + 4);
> + ret |= imx728_update_bits(imx728, 0x9908,
> + sp1vs_gain,
> + 0x1FFFF,
> + 4);
> +
> + if (ret < 0) {
> + dev_err(imx728->dev, "Failed to write fixed gain values.");
> + return ret;
> + }
> +
> + dev_info(imx728->dev, "Wrote fixed brightness for HDR");
> +
> + return 0;
> +}
> +
> +static int imx728_hdr_configure(
> + struct imx728 *imx728,
> + struct imx728_ctrl_point *points,
> + int hdr_bits)
> +{
> + u32 hdr_norm_x0;
> + u32 hdr_norm_x1;
> + u32 hdr_norm_y0;
> + u32 hdr_norm_y1;
> +
> + int ret;
> + int i;
> +
> + switch (hdr_bits) {
> + case 20:
> + hdr_norm_x0 = 0x2000;
> + hdr_norm_x1 = 0x5000;
> +
> + hdr_norm_y0 = 0x0;
> + hdr_norm_y1 = 0xd000;
> + break;
> + case 24:
> + hdr_norm_x0 = 0x3000;
> + hdr_norm_x1 = 0x5000;
> +
> + hdr_norm_y0 = 0x0;
> + hdr_norm_y1 = 0xe000;
> + break;
> + default:
> + dev_err(imx728->dev, "%i bit HDR not supported.", hdr_bits);
> + break;
> + }
> +
> + ret = imx728_write(imx728, 0x9C60, hdr_norm_x0, 4);
> + ret |= imx728_write(imx728, 0x9C6C, hdr_norm_x0, 4);
> + ret |= imx728_write(imx728, 0x9C64, hdr_norm_x1, 4);
> + ret |= imx728_write(imx728, 0x9C70, hdr_norm_x1, 4);
> + ret |= imx728_write(imx728, 0x9C68, hdr_norm_y0, 2);
> + ret |= imx728_write(imx728, 0x9C74, hdr_norm_y0, 2);
> + ret |= imx728_write(imx728, 0x9C6A, hdr_norm_y1, 2);
> + ret |= imx728_write(imx728, 0x9C76, hdr_norm_y1, 2);
> +
> + if (ret < 0) {
> + dev_err(imx728->dev, "Failed when setting HDR Normalization gains: %i", ret);
> + return ret;
> + }
> +
> + for (i = 0; i < 16; i++) {
> + ret = imx728_write(
> + imx728,
> + IMX728_REG_CTRL_POINT_X(i),
> + points->x, 4);
> +
> + ret |= imx728_write(
> + imx728,
> + IMX728_REG_CTRL_POINT_Y(i),
> + points->y, 4);
> +
> + if (ret < 0) {
> + dev_err(imx728->dev, "Failed to write control point %i", i);
> + return ret;
> + }
> +
> + if ((points+1)->x >= 0 && (points+1)->y >= 0)
> + points++;
> + }
> +
> + return imx728_hdr_fixed_brightness(imx728);
> +}
> +
> +static int imx728_configure(struct imx728 *imx728)
> +{
> + int ret;
> + bool hdr = imx728->ctrl.wdr->val;
> + enum imx728_img_raw_mode img_out_mode;
> + enum imx728_drive_mode mode_sel;
> + enum imx728_aemode ae_mode;
> +
> + if (hdr) {
> + img_out_mode = IMX728_IMG_MODE_HDR;
> + mode_sel = IMX728_MODE_3856x2176_40_4LANE_RAW10;
> + ae_mode = IMX728_AEMODE_FULL_ME;
> + } else {
> + img_out_mode = IMX728_IMG_MODE_LINEAR;
> + mode_sel = IMX728_MODE_3856x2176_40_4LANE_RAW10;
> + ae_mode = IMX728_AEMODE_FULL_ME;
> + }
> +
> + ret = imx728_write(imx728, 0x98AC, ae_mode, 1);
> + if (ret < 0) {
> + dev_err(imx728->dev, "Couldn't set AE mode!");
> + return ret;
> + }
> +
> + ret = imx728_write(imx728, 0xA248, IMX728_AWBMODE_FULL_MWB, 1);
> + if (ret < 0) {
> + dev_err(imx728->dev, "Couldn't set full manual white balance mode!");
> + return ret;
> + }
> +
> + ret = imx728_update_bits(imx728, 0x1808, 0x1, 0x1, 1);
> + if (ret < 0) {
> + dev_err(imx728->dev, "Couldn't enable full manual white balance mode!");
> + return ret;
> + }
> +
> + ret = imx728_write(imx728, 0x98E0, IMX728_FME_SHTVAL_UNIT_MICROSECONDS, 1);
> + if (ret < 0) {
> + dev_err(imx728->dev, "Couldn't write exposure time unit to microseconds!");
> + return ret;
> + }
> +
> + ret = imx728_write(imx728, 0x98E8, IMX728_FME_SHTVAL_UNIT_MICROSECONDS, 1);
> + if (ret < 0) {
> + dev_err(imx728->dev, "Couldn't write exposure time unit to microseconds!");
> + return ret;
> + }
> +
> + ret = imx728_write(imx728, 0x98F0, IMX728_FME_SHTVAL_UNIT_MICROSECONDS, 1);
> + if (ret < 0) {
> + dev_err(imx728->dev, "Couldn't write exposure time unit to microseconds!");
> + return ret;
> + }
> +
> + if (hdr) {
> + ret = imx728_hdr_configure(imx728, imx728_hdr_20bit, 20);
> + if (ret < 0) {
> + dev_err(imx728->dev, "Couldn't configure sensor for HDR mode: %i", ret);
> + return ret;
> + }
> + }
> +
> + dev_info(imx728->dev, "Disabling metadata...");
> + ret = imx728_write(imx728, 0x1708, 0x00, 1);
> + ret |= imx728_write(imx728, 0x1709, 0x00, 1);
> + ret |= imx728_write(imx728, 0x170A, 0x00, 1);
> + ret |= imx728_write(imx728, 0x1B40, 0x00, 1);
> + if (ret < 0) {
> + dev_err(imx728->dev, "Error disabling metadata: %i", ret);
> + return ret;
> + }
> +
> + ret = imx728_update_bits(imx728, 0x9728,
> + mode_sel,
> + 0x7FFF,
> + 2);
> + if (ret < 0) {
> + dev_err(imx728->dev, "Couldn't write mode select.");
> + return ret;
> + }
> +
> + ret = imx728_update_bits(imx728, 0xEC7E,
> + img_out_mode,
> + 0x7,
> + 1);
> + if (ret < 0) {
> + dev_err(imx728->dev, "Couldn't select image out mode.");
> + return ret;
> + }
> +
> + ret = imx728_write(imx728, 0xEC12, 0x28, 2);
> + ret |= imx728_write(imx728, 0xEC14, 0x0, 1);
> + if (ret < 0) {
> + dev_err(imx728->dev, "Error disabling OB output.");
> + return ret;
> + }
> +
> + ret = imx728_write(imx728, 0x1761, 0x0, 1);
> + if (ret < 0)
> + dev_err(imx728->dev, "Error disabling skew calibration from sensor to SER");
> +
> + switch (imx728->ctrl.pg_mode->val) {
> + case 0:
> + break;
> + case 1:
> + // Horizontal Color Bars
> + ret = imx728_write(imx728, 0x1A2A, 8, 2);
> + ret |= imx728_write(imx728, 0x1A30, 0, 3);
> + ret |= imx728_write(imx728, 0x1A38, 1, 3);
> + ret |= imx728_write(imx728, 0xB58F, 0x0, 1);
> + ret |= imx728_write(imx728, 0xB6C5, 0x0, 1);
> + break;
> + case 2:
> + // Vertical Color Bars
> + ret = imx728_write(imx728, 0x1A2C, 16, 2);
> + ret |= imx728_write(imx728, 0x1A30, 0, 3);
> + ret |= imx728_write(imx728, 0x1A38, 1, 3);
> + ret |= imx728_write(imx728, 0xB58F, 0x1, 1);
> + ret |= imx728_write(imx728, 0xB6C5, 0x1, 1);
> + break;
> + default:
> + break;
> + }
> +
> + // Assume that everything except 'disabled' requires pattern gen enable
> + if (imx728->ctrl.pg_mode->val != 0) {
> + ret |= imx728_write(imx728, 0xB58E, 0x1, 1);
> + ret |= imx728_write(imx728, 0xB6C4, 0x1, 1);
> +
> + if (ret < 0) {
> + dev_err(imx728->dev, "Failed to enable PG mode.");
> + return ret;
> + }
> + }
> +
> + ret = imx728_update_bits(imx728, 0x9714,
> + IMX728_RAW_SEL_SP1H,
> + 0x7,
> + 1);
> + ret |= imx728_update_bits(imx728, 0xB684,
> + IMX728_RAW_SEL_SP1H,
> + 0x7,
> + 1);
> + if (ret < 0) {
> + dev_err(imx728->dev, "Failed to set subpixel register");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int imx728_start_stream(struct imx728 *imx728)
> +{
> + int ret;
> +
> + ret = imx728_powerup_to_standby(imx728);
> + if (ret < 0)
> + return ret;
> +
> + ret = imx728_write_table(imx728, imx728_3840x2160, ARRAY_SIZE(imx728_3840x2160));
> + if (ret < 0)
> + return ret;
> +
> + msleep(100);
> +
> + ret = imx728_configure(imx728);
> + if (ret < 0)
> + return ret;
> +
> + ret = __v4l2_ctrl_handler_setup(imx728->subdev.ctrl_handler);
> + if (ret < 0) {
> + dev_err(imx728->dev,
> + "%s: failed to apply v4l2 ctrls: %d\n", __func__, ret);
> + return ret;
> + }
> +
> + ret = imx728_write(imx728, 0x1B04, 0x5C, 1);
> + if (ret < 0)
> + return ret;
> + ret = imx728_write(imx728, 0x1B04, 0xA3, 1);
> + if (ret < 0)
> + return ret;
> +
> + ret = imx728_wait_for_state(imx728, IMX728_SENSOR_STATE_STREAMING);
> + if (ret < 0) {
> + dev_err(imx728->dev, "Could not transition to Streaming!");
> + return ret;
> + }
> +
> + msleep(20);
> +
> + return 0;
> +}
> +
> +static int imx728_stop_stream(struct imx728 *imx728)
> +{
> + int ret;
> +
> + ret = imx728_write(imx728, 0x1B04, 0xFF, 1);
> + if (ret < 0) {
> + dev_err(imx728->dev, "Error sending stop stream command: %i", ret);
> + return ret;
> + }
> +
> + imx728_wait_for_state(imx728, IMX728_SENSOR_STATE_STANDBY);
> + if (ret < 0) {
> + dev_err(imx728->dev, "Couldn't transition out of Streaming mode!");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int imx728_set_stream(struct v4l2_subdev *sd, int enable)
> +{
> + struct imx728 *imx728 = to_imx728(sd);
> + int ret;
> +
> + mutex_lock(&imx728->lock);
> + if (imx728->streaming == enable) {
> + mutex_unlock(&imx728->lock);
> + return 0;
> + }
> +
> + if (enable) {
> + ret = pm_runtime_get_sync(imx728->dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(imx728->dev);
> + goto err_unlock;
> + }
> +
> + ret = imx728_start_stream(imx728);
> + if (ret < 0)
> + goto err_runtime_put;
> + } else {
> + ret = imx728_stop_stream(imx728);
> + if (ret < 0)
> + goto err_runtime_put;
> + pm_runtime_mark_last_busy(imx728->dev);
> + pm_runtime_put_autosuspend(imx728->dev);
> + }
> +
> + imx728->streaming = enable;
> +
> + __v4l2_ctrl_grab(imx728->ctrl.wdr, enable);
> + __v4l2_ctrl_grab(imx728->ctrl.h_flip, enable);
> + __v4l2_ctrl_grab(imx728->ctrl.v_flip, enable);
> + __v4l2_ctrl_grab(imx728->ctrl.pg_mode, enable);
> +
> + mutex_unlock(&imx728->lock);
> + return 0;
> +
> +err_runtime_put:
> + pm_runtime_put(imx728->dev);
> +
> +err_unlock:
> + mutex_unlock(&imx728->lock);
> + dev_err(imx728->dev,
> + "%s: failed to setup streaming %d\n", __func__, ret);
> + return ret;
> +}
> +
> +static const struct v4l2_subdev_core_ops imx728_core_ops = {
> + .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
> + .unsubscribe_event = v4l2_event_subdev_unsubscribe,
> +};
> +
> +static const struct v4l2_subdev_video_ops imx728_subdev_video_ops = {
> + .s_stream = imx728_set_stream,
> +};
> +
> +static const struct v4l2_subdev_pad_ops imx728_subdev_pad_ops = {
> + .enum_mbus_code = imx728_enum_mbus_code,
> + .enum_frame_size = imx728_enum_frame_sizes,
> + .get_fmt = v4l2_subdev_get_fmt,
> + .set_fmt = imx728_set_fmt,
> + .get_frame_interval = imx728_get_frame_interval,
> + .set_frame_interval = imx728_set_frame_interval,
> + .set_routing = imx728_set_routing,
> + .get_frame_desc = imx728_get_frame_desc,
> +};
> +
> +static const struct v4l2_subdev_ops imx728_subdev_ops = {
> + .core = &imx728_core_ops,
> + .video = &imx728_subdev_video_ops,
> + .pad = &imx728_subdev_pad_ops,
> +};
> +
> +static const struct v4l2_ctrl_ops imx728_ctrl_ops = {
> + .s_ctrl = imx728_set_ctrl,
> +};
> +
> +static int imx728_probe(struct i2c_client *client)
> +{
> + struct imx728 *imx728;
> + struct v4l2_subdev *sd;
> + struct v4l2_ctrl_handler *ctrl_hdr;
> + int ret;
> +
> + imx728 = devm_kzalloc(&client->dev, sizeof(*imx728), GFP_KERNEL);
> + if (!imx728)
> + return -ENOMEM;
> +
> + imx728->dev = &client->dev;
> +
> + imx728->regmap = devm_regmap_init_i2c(client, &imx728_regmap_config);
Please use devm_cci_regmap_init_i2c().
> + if (IS_ERR(imx728->regmap))
> + return PTR_ERR(imx728->regmap);
> +
> + imx728->xclr_gpio = devm_gpiod_get_optional(imx728->dev,
> + "xclr", GPIOD_OUT_LOW);
> + if (IS_ERR(imx728->xclr_gpio))
> + return PTR_ERR(imx728->xclr_gpio);
> +
> + imx728->clk = devm_clk_get(imx728->dev, "inck");
> + if (IS_ERR(imx728->clk))
> + return PTR_ERR(imx728->clk);
> +
> + imx728->clk_rate = clk_get_rate(imx728->clk);
> + dev_info(imx728->dev, "inck rate: %lu Hz\n", imx728->clk_rate);
No need for this. clock frequency is available in debugfs if necessary.
> +
> + if (imx728->clk_rate < 18000000 || imx728->clk_rate > 30000000)
> + return -EINVAL;
> +
> + ret = imx728_power_on(imx728);
> + if (ret < 0)
> + return ret;
> +
> + ret = imx728_detect(imx728);
> + if (ret < 0)
> + return ret;
> +
> + sd = &imx728->subdev;
> + v4l2_i2c_subdev_init(sd, client, &imx728_subdev_ops);
> +
> + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> + V4L2_SUBDEV_FL_HAS_EVENTS |
> + V4L2_SUBDEV_FL_STREAMS;
> +
> + imx728->pad.flags = MEDIA_PAD_FL_SOURCE;
> + sd->entity.function = MEDIA_ENT_F_CAM_SENSOR;
> + ret = media_entity_pads_init(&sd->entity, 1, &imx728->pad);
> + if (ret < 0) {
> + dev_err(imx728->dev,
> + "%s: media entity init failed %d\n", __func__, ret);
> + return ret;
> + }
> +
> + ctrl_hdr = &imx728->ctrl.handler;
> + ret = v4l2_ctrl_handler_init(ctrl_hdr, 8);
> + if (ret < 0) {
> + dev_err(imx728->dev,
> + "%s: ctrl handler init failed: %d\n", __func__, ret);
> + goto err_media_cleanup;
> + }
> +
> + mutex_init(&imx728->lock);
> + imx728->ctrl.handler.lock = &imx728->lock;
> + imx728->fps = IMX728_FRAMERATE_DEFAULT;
> +
> + imx728->ctrl.exposure = v4l2_ctrl_new_std(ctrl_hdr, &imx728_ctrl_ops,
> + V4L2_CID_EXPOSURE, 0,
> + 33000, 1,
> + IMX728_EXPOSURE_DEFAULT);
> +
> + imx728->ctrl.again = v4l2_ctrl_new_std(ctrl_hdr, &imx728_ctrl_ops,
> + V4L2_CID_ANALOGUE_GAIN, 0,
> + 102, 1,
> + 24);
> +
> + imx728->ctrl.wdr = v4l2_ctrl_new_std(ctrl_hdr, &imx728_ctrl_ops,
> + V4L2_CID_WIDE_DYNAMIC_RANGE,
> + 0, 1, 1, 1);
> +
> + imx728->ctrl.h_flip = v4l2_ctrl_new_std(ctrl_hdr, &imx728_ctrl_ops,
> + V4L2_CID_HFLIP, 0, 1, 1, 0);
> +
> + imx728->ctrl.v_flip = v4l2_ctrl_new_std(ctrl_hdr, &imx728_ctrl_ops,
> + V4L2_CID_VFLIP, 0, 1, 1, 0);
> +
> + imx728->ctrl.pg_mode = v4l2_ctrl_new_std_menu_items(ctrl_hdr,
> + &imx728_ctrl_ops, V4L2_CID_TEST_PATTERN,
> + ARRAY_SIZE(imx728_ctrl_pg_qmenu) - 1,
> + 0, 0, imx728_ctrl_pg_qmenu);
> +
> + imx728->ctrl.pixel_rate = v4l2_ctrl_new_std(ctrl_hdr, &imx728_ctrl_ops,
> + V4L2_CID_PIXEL_RATE,
> + IMX728_PIXEL_RATE,
> + IMX728_PIXEL_RATE, 1,
> + IMX728_PIXEL_RATE);
> +
> + imx728->ctrl.link_freq = v4l2_ctrl_new_int_menu(ctrl_hdr, &imx728_ctrl_ops,
> + V4L2_CID_LINK_FREQ,
> + ARRAY_SIZE(imx728_link_freq_menu) - 1, 0,
> + imx728_link_freq_menu);
> +
> + if (imx728->ctrl.link_freq)
> + imx728->ctrl.link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> +
> + imx728->subdev.ctrl_handler = ctrl_hdr;
> + if (imx728->ctrl.handler.error) {
> + ret = imx728->ctrl.handler.error;
> + dev_err(imx728->dev,
> + "%s: failed to add the ctrls: %d\n", __func__, ret);
> + goto err_ctrl_free;
> + }
> +
> + pm_runtime_set_active(imx728->dev);
> + pm_runtime_enable(imx728->dev);
> + pm_runtime_set_autosuspend_delay(imx728->dev, IMX728_PM_IDLE_TIMEOUT);
> + pm_runtime_use_autosuspend(imx728->dev);
> + pm_runtime_get_noresume(imx728->dev);
> +
> + ret = v4l2_subdev_init_finalize(sd);
> + if (ret < 0)
> + goto err_pm_disable;
> +
> + ret = v4l2_async_register_subdev(sd);
> + if (ret < 0) {
> + dev_err(imx728->dev,
> + "%s: v4l2 subdev register failed %d\n", __func__, ret);
> + goto err_subdev_cleanup;
> + }
> +
> + dev_info(imx728->dev, "imx728 probed\n");
> + pm_runtime_mark_last_busy(imx728->dev);
> + pm_runtime_put_autosuspend(imx728->dev);
> + return 0;
> +
> +err_subdev_cleanup:
> + v4l2_subdev_cleanup(&imx728->subdev);
> +
> +err_pm_disable:
> + pm_runtime_dont_use_autosuspend(imx728->dev);
> + pm_runtime_put_noidle(imx728->dev);
> + pm_runtime_disable(imx728->dev);
> +
> +err_ctrl_free:
> + v4l2_ctrl_handler_free(ctrl_hdr);
> + mutex_destroy(&imx728->lock);
> +
> +err_media_cleanup:
> + media_entity_cleanup(&imx728->subdev.entity);
> +
> + return ret;
> +}
> +
> +static int __maybe_unused imx728_runtime_resume(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct imx728 *imx728 = to_imx728(sd);
> + int ret;
> +
> + ret = imx728_power_on(imx728);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int __maybe_unused imx728_runtime_suspend(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct imx728 *imx728 = to_imx728(sd);
> +
> + imx728_power_off(imx728);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused imx728_suspend(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct imx728 *imx728 = to_imx728(sd);
> + int ret;
> +
> + if (imx728->streaming)
> + imx728_stop_stream(imx728);
> +
> + ret = pm_runtime_force_suspend(dev);
> + if (ret < 0)
> + dev_warn(dev, "%s: failed to suspend: %i\n", __func__, ret);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused imx728_resume(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct imx728 *imx728 = to_imx728(sd);
> + int ret;
> +
> + ret = pm_runtime_force_resume(dev);
> + if (ret < 0)
> + dev_warn(dev, "%s: failed to resume: %i\n", __func__, ret);
> +
> + if (imx728->streaming)
> + ret = imx728_start_stream(imx728);
> +
> + if (ret < 0) {
> + imx728_stop_stream(imx728);
> + imx728->streaming = 0;
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static void imx728_remove(struct i2c_client *client)
> +{
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct imx728 *imx728 = to_imx728(sd);
> +
> + v4l2_async_unregister_subdev(sd);
> + v4l2_ctrl_handler_free(&imx728->ctrl.handler);
> + v4l2_subdev_cleanup(&imx728->subdev);
> + media_entity_cleanup(&sd->entity);
> + mutex_destroy(&imx728->lock);
> +
> + pm_runtime_disable(imx728->dev);
> + pm_runtime_dont_use_autosuspend(imx728->dev);
> + pm_runtime_set_suspended(imx728->dev);
> +}
> +
> +static const struct dev_pm_ops imx728_pm_ops = {
> + SET_RUNTIME_PM_OPS(imx728_runtime_suspend,
> + imx728_runtime_resume, NULL)
> + SET_SYSTEM_SLEEP_PM_OPS(imx728_suspend, imx728_resume)
> +};
> +
> +static const struct of_device_id imx728_dt_id[] = {
> + { .compatible = "sony,imx728" },
> + { /* sentinel */ }
> +};
> +
> +MODULE_DEVICE_TABLE(of, imx728_dt_id);
> +
> +static struct i2c_driver imx728_i2c_driver = {
> + .driver = {
> + .name = "imx728",
> + .of_match_table = of_match_ptr(imx728_dt_id),
> + .pm = &imx728_pm_ops,
> + },
> + .probe = imx728_probe,
> + .remove = imx728_remove,
> +};
> +
> +module_i2c_driver(imx728_i2c_driver);
> +
> +MODULE_DESCRIPTION("Camera Sensor Driver for Sony IMX728");
> +MODULE_AUTHOR("Spencer Hill <shill@xxxxxxxxxxxxxxxxx>");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/media/i2c/imx728.h b/drivers/media/i2c/imx728.h
> new file mode 100644
> index 000000000000..6f320214b780
> --- /dev/null
> +++ b/drivers/media/i2c/imx728.h
There is only a single user of this header, move this into the c file.
> @@ -0,0 +1,3458 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Sony IMX728 CMOS Image Sensor Driver
> + *
> + * Copyright (c) 2024 Define Design Deploy Corp
> + */
> +
> +#include <linux/types.h>
> +
> +#define IMX728_OUT_WIDTH 3840
> +#define IMX728_OUT_HEIGHT 2160
> +
> +#define IMX728_FRAMERATE_MAX 30
> +#define IMX728_FRAMERATE_DEFAULT 30
> +#define IMX728_FRAMERATE_MIN 10
> +
> +#define IMX728_PIXEL_RATE 225504000
> +#define IMX728_LINK_FREQ 800000000
> +
> +#define IMX728_EXPOSURE_DEFAULT 10000
> +
> +#define IMX728_PM_IDLE_TIMEOUT 1000
> +
> +
> +#define IMX728_REG_CTRL_POINT_X(i) (0xA198 + (i) * 8)
> +#define IMX728_REG_CTRL_POINT_Y(i) (IMX728_REG_CTRL_POINT_X(i) + 4)
> +
> +enum imx728_sensor_state {
> + IMX728_SENSOR_STATE_SLEEP = 0x02,
> + IMX728_SENSOR_STATE_STANDBY = 0x04,
> + IMX728_SENSOR_STATE_STREAMING = 0x10,
> + IMX728_SENSOR_STATE_SAFE = 0x20,
> +};
> +
> +
> +enum imx728_remap_mode_id {
> + IMX728_REMAP_MODE_STANDBY = 0x00,
> + IMX728_REMAP_MODE_STANDBY_PIXEL_SHADING_COMPENSATION = 0x01,
> + IMX728_REMAP_MODE_STANDBY_SPOT_PIXEL_COMPENSATION = 0x02,
> + IMX728_REMAP_MODE_STREAMING = 0x04,
> + IMX728_REMAP_MODE_STREAMING_PIXEL_SHADING_COMPENSATION = 0x05,
> + IMX728_REMAP_MODE_STREAMING_SPOT_PIXEL_COMPENSATION = 0x06,
> + IMX728_REMAP_MODE_SLEEP = 0x20,
> +};
> +
> +enum imx728_drive_mode {
> + IMX728_MODE_3856x2176_45_4LANE_RAW10 = 0x01,
> + IMX728_MODE_3856x2176_45_4LANE_RAW12 = 0x02,
> + IMX728_MODE_3856x2176_45_4LANE_RAW16 = 0x03,
> + IMX728_MODE_3856x2176_45_4LANE_RAW20 = 0x04,
> + IMX728_MODE_3856x2176_45_4LANE_RAW12_HDR = 0x05,
> + IMX728_MODE_3856x2176_40_4LANE_RAW10 = 0x11,
> + IMX728_MODE_3856x2176_40_4LANE_RAW12 = 0x12,
> + IMX728_MODE_3856x2176_40_4LANE_RAW16 = 0x13,
> + IMX728_MODE_3856x2176_40_4LANE_RAW20 = 0x14,
> + IMX728_MODE_3856x2176_40_4LANE_RAW12_HDR = 0x16,
> +};
> +
> +enum imx728_awbmode {
> + IMX728_AWBMODE_ATW = 0,
> + IMX728_AWBMODE_ALL_PULL_IN = 1,
> + IMX728_AWBMODE_USER_PRESET = 2,
> + IMX728_AWBMODE_FULL_MWB = 3,
> + IMX728_AWBMODE_HOLD = 4,
> +};
> +
> +enum imx728_img_raw_mode {
> + IMX728_IMG_MODE_LINEAR = 0x0,
> + IMX728_IMG_MODE_LI = 0x1,
> + IMX728_IMG_MODE_HDR = 0x2,
> + IMX728_IMG_MODE_LI_HDR = 0x3,
> +};
> +
> +enum imx728_aemode {
> + IMX728_AEMODE_AE_AUTO = 0,
> + IMX728_AEMODE_AE_HOLD = 1,
> + IMX728_AEMODE_SCALE_ME = 2,
> + IMX728_AEMODE_FULL_ME = 3,
> +};
> +
> +enum imx728_fme_shtval_unit {
> + IMX728_FME_SHTVAL_UNIT_LINES = 1,
> + IMX728_FME_SHTVAL_UNIT_MICROSECONDS = 3,
> + IMX728_FME_SHTVAL_UNIT_FRAMES = 4,
> +};
> +
> +enum imx728_linear_raw_sel {
> + IMX728_RAW_SEL_SP1H = 0x0,
> + IMX728_RAW_SEL_SP1L = 0x1,
> + IMX728_RAW_SEL_SP1EC = 0x2,
> + IMX728_RAW_SEL_SP2 = 0x3,
> + IMX728_RAW_SEL_SP1VS = 0x4
> +};
> +
> +enum imx728_binn_avg {
> + IMX728_BINN_SIMPLE_AVG,
> + IMX728_BINN_WEIGHTED_AVG,
> +};
> +
> +struct imx728_ctrl {
> + struct v4l2_ctrl_handler handler;
> + struct v4l2_ctrl *wdr;
> + struct v4l2_ctrl *exposure;
> + struct v4l2_ctrl *again;
> + struct v4l2_ctrl *h_flip;
> + struct v4l2_ctrl *v_flip;
> + struct v4l2_ctrl *pg_mode;
> + struct v4l2_ctrl *pixel_rate;
> + struct v4l2_ctrl *link_freq;
> +};
> +
> +struct imx728_ctrl_point {
What does ctrl_point mean? What is x and y?
> + int x, y;
> +};
> +
> +/*
> + * struct imx728 - imx728 device structure
> + * @dev: Device handle
> + * @clk: Pointer to imx728 clock
> + * @client: Pointer to I2C client
> + * @regmap: Pointer to regmap structure
> + * @xclr_gpio: Pointer to XCLR gpio
> + * @subdev: V4L2 subdevice structure
> + * @format: V4L2 media bus frame format structure
> + * (width and height are in sync with the compose rect)
> + * @pad: Media pad structure
> + * @ctrl: imx728 control structure
> + * @clk_rate: Frequency of imx728 clock
> + * @lock: Mutex structure for V4L2 ctrl handler
> + * @streaming: Flag to store streaming on/off status
> + */
> +struct imx728 {
> + struct device *dev;
> +
> + struct clk *clk;
> + struct i2c_client *client;
> + struct regmap *regmap;
> + struct gpio_desc *xclr_gpio;
> +
> + struct v4l2_subdev subdev;
> + struct v4l2_mbus_framefmt format;
> + struct media_pad pad;
> +
> + struct imx728_ctrl ctrl;
> +
> + unsigned long clk_rate;
> + u32 fps;
> +
> + struct mutex lock;
> + bool streaming;
> +};
> +
> +static const struct v4l2_area imx728_framesizes[] = {
> + {
> + .width = IMX728_OUT_WIDTH,
> + .height = IMX728_OUT_HEIGHT,
> + },
Are you sure this is the only supported resolution? I would prefer using
actual numbers here.
> +};
> +
> +static const u32 imx728_mbus_formats[] = {
> + MEDIA_BUS_FMT_SRGGB10_1X10,
> +};
> +
> +static const s64 imx728_link_freq_menu[] = {
> + IMX728_LINK_FREQ,
> +};
> +
> +static const struct regmap_config imx728_regmap_config = {
> + .reg_bits = 16,
> + .val_bits = 8,
> +};
> +
> +static const char *const imx728_ctrl_pg_qmenu[] = {
> + "Disabled",
> + "Horizontal Color Bars",
> + "Vertical Color Bars",
> +};
> +
> +static struct imx728_ctrl_point imx728_hdr_20bit[] = {
> + {0, 0},
> + {1566 >> 4, 938},
> + {105740 >> 4, 1863},
> + {387380 >> 4, 2396},
> + {3818601 >> 4, 3251},
> + {16777215 >> 4, 4095},
> + {-1, -1}
> +};
> +
> +static const struct reg_sequence imx728_3840x2160[] = {
Please use struct cci_reg_sequence.
Best regards,
Alexander
> + {0xFFFF, 0x00, 1000},
> + {0x1749, 0x01},
> + {0x174B, 0x01},
> [snip]
> +};
> --
> 2.40.1
>
> Please be aware that this email includes email addresses outside of the organization.
>
>
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/
