Hi Andrey, On Fri, Dec 27, 2019 at 03:21:14PM +0300, Andrey Konovalov wrote: > From: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx> > > Adds a driver for the 8MPix Sony IMX219 CSI2 sensor. > Whilst the sensor supports 2 or 4 CSI2 data lanes, this driver > currently only supports 2 lanes. > 8MPix @ 15fps, 1080P @ 30fps (cropped FOV), and 1640x1232 (2x2 binned) > @ 30fps are currently supported. > > Signed-off-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx> > Signed-off-by: Andrey Konovalov <andrey.konovalov@xxxxxxxxxx> > --- > drivers/media/i2c/Kconfig | 12 + > drivers/media/i2c/Makefile | 1 + > drivers/media/i2c/imx219.c | 1240 ++++++++++++++++++++++++++++++++++++ > 3 files changed, 1253 insertions(+) > create mode 100644 drivers/media/i2c/imx219.c > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig > index c68e002d26ea..6fa5af1f72b9 100644 > --- a/drivers/media/i2c/Kconfig > +++ b/drivers/media/i2c/Kconfig > @@ -591,6 +591,18 @@ config VIDEO_IMX214 > To compile this driver as a module, choose M here: the > module will be called imx214. > > +config VIDEO_IMX219 > + tristate "Sony IMX219 sensor support" > + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API > + depends on MEDIA_CAMERA_SUPPORT You can safely drop this line. > + select V4L2_FWNODE > + help > + This is a Video4Linux2 sensor driver for the Sony > + IMX219 camera. > + > + To compile this driver as a module, choose M here: the > + module will be called imx219. > + > config VIDEO_IMX258 > tristate "Sony IMX258 sensor support" > depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile > index c147bb9d28db..77bf7d0b691f 100644 > --- a/drivers/media/i2c/Makefile > +++ b/drivers/media/i2c/Makefile > @@ -111,6 +111,7 @@ obj-$(CONFIG_VIDEO_OV2659) += ov2659.o > obj-$(CONFIG_VIDEO_TC358743) += tc358743.o > obj-$(CONFIG_VIDEO_HI556) += hi556.o > obj-$(CONFIG_VIDEO_IMX214) += imx214.o > +obj-$(CONFIG_VIDEO_IMX219) += imx219.o > obj-$(CONFIG_VIDEO_IMX258) += imx258.o > obj-$(CONFIG_VIDEO_IMX274) += imx274.o > obj-$(CONFIG_VIDEO_IMX290) += imx290.o > diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c > new file mode 100644 > index 000000000000..28b55c61cd77 > --- /dev/null > +++ b/drivers/media/i2c/imx219.c > @@ -0,0 +1,1240 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * A V4L2 driver for Sony IMX219 cameras. > + * Copyright (C) 2019, Raspberry Pi (Trading) Ltd > + * > + * Based on Sony imx258 camera driver > + * Copyright (C) 2018 Intel Corporation > + * > + * DT / fwnode changes, and regulator / GPIO control taken from imx214 driver > + * Copyright 2018 Qtechnology A/S > + * > + * Flip handling taken from the Sony IMX319 driver. > + * Copyright (C) 2018 Intel Corporation > + * > + */ > + > +#include <linux/clk.h> > +#include <linux/clk-provider.h> > +#include <linux/clkdev.h> > +#include <linux/delay.h> > +#include <linux/gpio/consumer.h> > +#include <linux/i2c.h> > +#include <linux/module.h> > +#include <linux/pm_runtime.h> > +#include <linux/regulator/consumer.h> > +#include <media/v4l2-ctrls.h> > +#include <media/v4l2-device.h> > +#include <media/v4l2-event.h> > +#include <media/v4l2-fwnode.h> > +#include <media/v4l2-mediabus.h> > +#include <asm/unaligned.h> > + > +#define IMX219_REG_VALUE_08BIT 1 > +#define IMX219_REG_VALUE_16BIT 2 > + > +#define IMX219_REG_MODE_SELECT 0x0100 > +#define IMX219_MODE_STANDBY 0x00 > +#define IMX219_MODE_STREAMING 0x01 > + > +/* Chip ID */ > +#define IMX219_REG_CHIP_ID 0x0000 > +#define IMX219_CHIP_ID 0x0219 > + > +/* External clock frequency is 24.0M */ > +#define IMX219_XCLK_FREQ 24000000 > + > +/* Pixel rate is fixed at 182.4M for all the modes */ > +#define IMX219_PIXEL_RATE 182400000 > + > +/* V_TIMING internal */ > +#define IMX219_REG_VTS 0x0160 > +#define IMX219_VTS_15FPS 0x0dc6 > +#define IMX219_VTS_30FPS_1080P 0x06e3 > +#define IMX219_VTS_30FPS_BINNED 0x06e3 > +#define IMX219_VTS_MAX 0xffff > + > +#define IMX219_VBLANK_MIN 4 > + > +/*Frame Length Line*/ > +#define IMX219_FLL_MIN 0x08a6 > +#define IMX219_FLL_MAX 0xffff > +#define IMX219_FLL_STEP 1 > +#define IMX219_FLL_DEFAULT 0x0c98 > + > +/* HBLANK control - read only */ > +#define IMX219_PPL_DEFAULT 3448 > + > +/* Exposure control */ > +#define IMX219_REG_EXPOSURE 0x015a > +#define IMX219_EXPOSURE_MIN 4 > +#define IMX219_EXPOSURE_STEP 1 > +#define IMX219_EXPOSURE_DEFAULT 0x640 > +#define IMX219_EXPOSURE_MAX 65535 > + > +/* Analog gain control */ > +#define IMX219_REG_ANALOG_GAIN 0x0157 > +#define IMX219_ANA_GAIN_MIN 0 > +#define IMX219_ANA_GAIN_MAX 232 > +#define IMX219_ANA_GAIN_STEP 1 > +#define IMX219_ANA_GAIN_DEFAULT 0x0 > + > +/* Digital gain control */ > +#define IMX219_REG_DIGITAL_GAIN 0x0158 > +#define IMX219_DGTL_GAIN_MIN 0x0100 > +#define IMX219_DGTL_GAIN_MAX 0x0fff > +#define IMX219_DGTL_GAIN_DEFAULT 0x0100 > +#define IMX219_DGTL_GAIN_STEP 1 > + > +#define IMX219_REG_ORIENTATION 0x0172 > + > +/* Test Pattern Control */ > +#define IMX219_REG_TEST_PATTERN 0x0600 > +#define IMX219_TEST_PATTERN_DISABLE 0 > +#define IMX219_TEST_PATTERN_SOLID_COLOR 1 > +#define IMX219_TEST_PATTERN_COLOR_BARS 2 > +#define IMX219_TEST_PATTERN_GREY_COLOR 3 > +#define IMX219_TEST_PATTERN_PN9 4 > + > +/* Test pattern colour components */ > +#define IMX219_REG_TESTP_RED 0x0602 > +#define IMX219_REG_TESTP_GREENR 0x0604 > +#define IMX219_REG_TESTP_BLUE 0x0606 > +#define IMX219_REG_TESTP_GREENB 0x0608 > +#define IMX219_TESTP_COLOUR_MIN 0 > +#define IMX219_TESTP_COLOUR_MAX 0x03ff > +#define IMX219_TESTP_COLOUR_STEP 1 > +#define IMX219_TESTP_RED_DEFAULT IMX219_TESTP_COLOUR_MAX > +#define IMX219_TESTP_GREENR_DEFAULT 0 > +#define IMX219_TESTP_BLUE_DEFAULT 0 > +#define IMX219_TESTP_GREENB_DEFAULT 0 > + > +struct imx219_reg { > + u16 address; > + u8 val; > +}; > + > +struct imx219_reg_list { > + unsigned int num_of_regs; > + const struct imx219_reg *regs; > +}; > + > +/* Mode : resolution and related config&values */ > +struct imx219_mode { > + /* Frame width */ > + unsigned int width; > + /* Frame height */ > + unsigned int height; > + > + /* V-timing */ > + unsigned int vts_def; > + > + /* Default register values */ > + struct imx219_reg_list reg_list; > +}; > + > +/* > + * Register sets lifted off the i2C interface from the Raspberry Pi firmware > + * driver. > + * 3280x2464 = mode 2, 1920x1080 = mode 1, and 1640x1232 = mode 4. > + */ > +static const struct imx219_reg mode_3280x2464_regs[] = { > + {0x0100, 0x00}, > + {0x30eb, 0x0c}, > + {0x30eb, 0x05}, > + {0x300a, 0xff}, > + {0x300b, 0xff}, > + {0x30eb, 0x05}, > + {0x30eb, 0x09}, > + {0x0114, 0x01}, > + {0x0128, 0x00}, > + {0x012a, 0x18}, > + {0x012b, 0x00}, > + {0x0164, 0x00}, > + {0x0165, 0x00}, > + {0x0166, 0x0c}, > + {0x0167, 0xcf}, > + {0x0168, 0x00}, > + {0x0169, 0x00}, > + {0x016a, 0x09}, > + {0x016b, 0x9f}, > + {0x016c, 0x0c}, > + {0x016d, 0xd0}, > + {0x016e, 0x09}, > + {0x016f, 0xa0}, > + {0x0170, 0x01}, > + {0x0171, 0x01}, > + {0x0174, 0x00}, > + {0x0175, 0x00}, > + {0x018c, 0x0a}, > + {0x018d, 0x0a}, > + {0x0301, 0x05}, > + {0x0303, 0x01}, > + {0x0304, 0x03}, > + {0x0305, 0x03}, > + {0x0306, 0x00}, > + {0x0307, 0x39}, > + {0x0309, 0x0a}, > + {0x030b, 0x01}, > + {0x030c, 0x00}, > + {0x030d, 0x72}, > + {0x0624, 0x0c}, > + {0x0625, 0xd0}, > + {0x0626, 0x09}, > + {0x0627, 0xa0}, > + {0x455e, 0x00}, > + {0x471e, 0x4b}, > + {0x4767, 0x0f}, > + {0x4750, 0x14}, > + {0x4540, 0x00}, > + {0x47b4, 0x14}, > + {0x4713, 0x30}, > + {0x478b, 0x10}, > + {0x478f, 0x10}, > + {0x4793, 0x10}, > + {0x4797, 0x0e}, > + {0x479b, 0x0e}, > + {0x0162, 0x0d}, > + {0x0163, 0x78}, > +}; > + > +static const struct imx219_reg mode_1920_1080_regs[] = { > + {0x0100, 0x00}, > + {0x30eb, 0x05}, > + {0x30eb, 0x0c}, > + {0x300a, 0xff}, > + {0x300b, 0xff}, > + {0x30eb, 0x05}, > + {0x30eb, 0x09}, > + {0x0114, 0x01}, > + {0x0128, 0x00}, > + {0x012a, 0x18}, > + {0x012b, 0x00}, > + {0x0162, 0x0d}, > + {0x0163, 0x78}, > + {0x0164, 0x02}, > + {0x0165, 0xa8}, > + {0x0166, 0x0a}, > + {0x0167, 0x27}, > + {0x0168, 0x02}, > + {0x0169, 0xb4}, > + {0x016a, 0x06}, > + {0x016b, 0xeb}, > + {0x016c, 0x07}, > + {0x016d, 0x80}, > + {0x016e, 0x04}, > + {0x016f, 0x38}, > + {0x0170, 0x01}, > + {0x0171, 0x01}, > + {0x0174, 0x00}, > + {0x0175, 0x00}, > + {0x018c, 0x0a}, > + {0x018d, 0x0a}, > + {0x0301, 0x05}, > + {0x0303, 0x01}, > + {0x0304, 0x03}, > + {0x0305, 0x03}, > + {0x0306, 0x00}, > + {0x0307, 0x39}, > + {0x0309, 0x0a}, > + {0x030b, 0x01}, > + {0x030c, 0x00}, > + {0x030d, 0x72}, > + {0x0624, 0x07}, > + {0x0625, 0x80}, > + {0x0626, 0x04}, > + {0x0627, 0x38}, > + {0x455e, 0x00}, > + {0x471e, 0x4b}, > + {0x4767, 0x0f}, > + {0x4750, 0x14}, > + {0x4540, 0x00}, > + {0x47b4, 0x14}, > + {0x4713, 0x30}, > + {0x478b, 0x10}, > + {0x478f, 0x10}, > + {0x4793, 0x10}, > + {0x4797, 0x0e}, > + {0x479b, 0x0e}, > + {0x0162, 0x0d}, > + {0x0163, 0x78}, > +}; > + > +static const struct imx219_reg mode_1640_1232_regs[] = { > + {0x0100, 0x00}, > + {0x30eb, 0x0c}, > + {0x30eb, 0x05}, > + {0x300a, 0xff}, > + {0x300b, 0xff}, > + {0x30eb, 0x05}, > + {0x30eb, 0x09}, > + {0x0114, 0x01}, > + {0x0128, 0x00}, > + {0x012a, 0x18}, > + {0x012b, 0x00}, > + {0x0164, 0x00}, > + {0x0165, 0x00}, > + {0x0166, 0x0c}, > + {0x0167, 0xcf}, > + {0x0168, 0x00}, > + {0x0169, 0x00}, > + {0x016a, 0x09}, > + {0x016b, 0x9f}, > + {0x016c, 0x06}, > + {0x016d, 0x68}, > + {0x016e, 0x04}, > + {0x016f, 0xd0}, > + {0x0170, 0x01}, > + {0x0171, 0x01}, > + {0x0174, 0x01}, > + {0x0175, 0x01}, > + {0x018c, 0x0a}, > + {0x018d, 0x0a}, > + {0x0301, 0x05}, > + {0x0303, 0x01}, > + {0x0304, 0x03}, > + {0x0305, 0x03}, > + {0x0306, 0x00}, > + {0x0307, 0x39}, > + {0x0309, 0x0a}, > + {0x030b, 0x01}, > + {0x030c, 0x00}, > + {0x030d, 0x72}, > + {0x0624, 0x06}, > + {0x0625, 0x68}, > + {0x0626, 0x04}, > + {0x0627, 0xd0}, > + {0x455e, 0x00}, > + {0x471e, 0x4b}, > + {0x4767, 0x0f}, > + {0x4750, 0x14}, > + {0x4540, 0x00}, > + {0x47b4, 0x14}, > + {0x4713, 0x30}, > + {0x478b, 0x10}, > + {0x478f, 0x10}, > + {0x4793, 0x10}, > + {0x4797, 0x0e}, > + {0x479b, 0x0e}, > + {0x0162, 0x0d}, > + {0x0163, 0x78}, > +}; > + > +static const char * const imx219_test_pattern_menu[] = { > + "Disabled", > + "Color Bars", > + "Solid Color", > + "Grey Color Bars", > + "PN9" > +}; > + > +static const int imx219_test_pattern_val[] = { > + IMX219_TEST_PATTERN_DISABLE, > + IMX219_TEST_PATTERN_COLOR_BARS, > + IMX219_TEST_PATTERN_SOLID_COLOR, > + IMX219_TEST_PATTERN_GREY_COLOR, > + IMX219_TEST_PATTERN_PN9, > +}; > + > +/* regulator supplies */ > +static const char * const imx219_supply_name[] = { > + /* Supplies can be enabled in any order */ > + "VANA", /* Analog (2.8V) supply */ > + "VDIG", /* Digital Core (1.8V) supply */ > + "VDDL", /* IF (1.2V) supply */ > +}; > + > +#define IMX219_NUM_SUPPLIES ARRAY_SIZE(imx219_supply_name) > + > +#define IMX219_XCLR_DELAY_MS 10 /* Initialisation delay after XCLR low->high */ > + > +/* Mode configs */ > +static const struct imx219_mode supported_modes[] = { > + { > + /* 8MPix 15fps mode */ > + .width = 3280, > + .height = 2464, > + .vts_def = IMX219_VTS_15FPS, > + .reg_list = { > + .num_of_regs = ARRAY_SIZE(mode_3280x2464_regs), > + .regs = mode_3280x2464_regs, > + }, > + }, > + { > + /* 1080P 30fps cropped */ > + .width = 1920, > + .height = 1080, > + .vts_def = IMX219_VTS_30FPS_1080P, > + .reg_list = { > + .num_of_regs = ARRAY_SIZE(mode_1920_1080_regs), > + .regs = mode_1920_1080_regs, > + }, > + }, > + { > + /* 2x2 binned 30fps mode */ > + .width = 1640, > + .height = 1232, > + .vts_def = IMX219_VTS_30FPS_BINNED, > + .reg_list = { > + .num_of_regs = ARRAY_SIZE(mode_1640_1232_regs), > + .regs = mode_1640_1232_regs, > + }, > + }, > +}; > + > +struct imx219 { > + struct device *dev; > + > + struct v4l2_subdev sd; > + struct media_pad pad; > + > + struct v4l2_fwnode_endpoint ep; /* the parsed DT endpoint info */ > + struct clk *xclk; /* system clock to IMX219 */ > + u32 xclk_freq; > + > + struct gpio_desc *xclr_gpio; > + struct regulator_bulk_data supplies[IMX219_NUM_SUPPLIES]; > + > + struct v4l2_ctrl_handler ctrl_handler; > + /* V4L2 Controls */ > + struct v4l2_ctrl *pixel_rate; > + struct v4l2_ctrl *exposure; > + struct v4l2_ctrl *vflip; > + struct v4l2_ctrl *hflip; > + struct v4l2_ctrl *vblank; > + struct v4l2_ctrl *hblank; > + > + /* Current mode */ > + const struct imx219_mode *mode; > + > + /* > + * Mutex for serialized access: > + * Protect sensor module set pad format and start/stop streaming safely. > + */ > + struct mutex mutex; > + > + /* Streaming on/off */ > + bool streaming; > +}; > + > +static inline struct imx219 *to_imx219(struct v4l2_subdev *_sd) > +{ > + return container_of(_sd, struct imx219, sd); > +} > + > +/* Read registers up to 2 at a time */ > +static int imx219_read_reg(struct imx219 *imx219, u16 reg, u32 len, u32 *val) > +{ > + struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd); > + struct i2c_msg msgs[2]; > + u8 addr_buf[2] = { reg >> 8, reg & 0xff }; > + u8 data_buf[4] = { 0, }; > + int ret; > + > + if (len > 4) > + return -EINVAL; > + > + /* Write register address */ > + msgs[0].addr = client->addr; > + msgs[0].flags = 0; > + msgs[0].len = ARRAY_SIZE(addr_buf); > + msgs[0].buf = addr_buf; > + > + /* Read data from register */ > + msgs[1].addr = client->addr; > + msgs[1].flags = I2C_M_RD; > + msgs[1].len = len; > + msgs[1].buf = &data_buf[4 - len]; > + > + ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs)); > + if (ret != ARRAY_SIZE(msgs)) > + return -EIO; > + > + *val = get_unaligned_be32(data_buf); > + > + return 0; > +} > + > +/* Write registers up to 2 at a time */ > +static int imx219_write_reg(struct imx219 *imx219, u16 reg, u32 len, u32 val) > +{ > + struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd); > + u8 buf[6]; > + > + if (len > 4) > + return -EINVAL; > + > + put_unaligned_be16(reg, buf); > + put_unaligned_be32(val << (8 * (4 - len)), buf + 2); > + if (i2c_master_send(client, buf, len + 2) != len + 2) > + return -EIO; > + > + return 0; > +} > + > +/* Write a list of registers */ > +static int imx219_write_regs(struct imx219 *imx219, > + const struct imx219_reg *regs, u32 len) > +{ > + struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd); > + unsigned int i; > + int ret; > + > + for (i = 0; i < len; i++) { > + ret = imx219_write_reg(imx219, regs[i].address, 1, regs[i].val); > + if (ret) { > + dev_err_ratelimited(&client->dev, > + "Failed to write reg 0x%4.4x. error = %d\n", > + regs[i].address, ret); > + > + return ret; > + } > + } > + > + return 0; > +} > + > +/* Get bayer order based on flip setting. */ > +static u32 imx219_get_format_code(struct imx219 *imx219) > +{ > + /* > + * Only one bayer order is supported. > + * It depends on the flip settings. > + */ > + static const u32 codes[2][2] = { > + { MEDIA_BUS_FMT_SRGGB10_1X10, MEDIA_BUS_FMT_SGRBG10_1X10, }, > + { MEDIA_BUS_FMT_SGBRG10_1X10, MEDIA_BUS_FMT_SBGGR10_1X10, }, > + }; > + > + lockdep_assert_held(&imx219->mutex); > + return codes[imx219->vflip->val][imx219->hflip->val]; > +} > + > +static int imx219_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) > +{ > + struct imx219 *imx219 = to_imx219(sd); > + struct v4l2_mbus_framefmt *try_fmt = > + v4l2_subdev_get_try_format(sd, fh->pad, 0); > + > + mutex_lock(&imx219->mutex); > + > + /* Initialize try_fmt */ > + try_fmt->width = supported_modes[0].width; > + try_fmt->height = supported_modes[0].height; > + try_fmt->code = imx219_get_format_code(imx219); Please assign this when getting the format (subdev pad get_fmt callback). > + try_fmt->field = V4L2_FIELD_NONE; > + > + mutex_unlock(&imx219->mutex); > + > + return 0; > +} > + > +static int imx219_set_ctrl(struct v4l2_ctrl *ctrl) > +{ > + struct imx219 *imx219 = > + container_of(ctrl->handler, struct imx219, ctrl_handler); > + struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd); > + int ret; > + > + if (ctrl->id == V4L2_CID_VBLANK) { > + int exposure_max, exposure_def; > + > + /* Update max exposure while meeting expected vblanking */ > + exposure_max = imx219->mode->height + ctrl->val - 4; > + exposure_def = (exposure_max < IMX219_EXPOSURE_DEFAULT) ? > + exposure_max : IMX219_EXPOSURE_DEFAULT; > + __v4l2_ctrl_modify_range(imx219->exposure, > + imx219->exposure->minimum, > + exposure_max, imx219->exposure->step, > + exposure_def); > + } > + > + /* > + * Applying V4L2 control value only happens > + * when power is up for streaming > + */ > + if (pm_runtime_get_if_in_use(&client->dev) == 0) > + return 0; > + > + switch (ctrl->id) { > + case V4L2_CID_ANALOGUE_GAIN: > + ret = imx219_write_reg(imx219, IMX219_REG_ANALOG_GAIN, > + IMX219_REG_VALUE_08BIT, ctrl->val); > + break; > + case V4L2_CID_EXPOSURE: > + ret = imx219_write_reg(imx219, IMX219_REG_EXPOSURE, > + IMX219_REG_VALUE_16BIT, ctrl->val); > + break; > + case V4L2_CID_DIGITAL_GAIN: > + ret = imx219_write_reg(imx219, IMX219_REG_DIGITAL_GAIN, > + IMX219_REG_VALUE_16BIT, ctrl->val); > + break; > + case V4L2_CID_TEST_PATTERN: > + ret = imx219_write_reg(imx219, IMX219_REG_TEST_PATTERN, > + IMX219_REG_VALUE_16BIT, > + imx219_test_pattern_val[ctrl->val]); > + break; > + case V4L2_CID_HFLIP: > + case V4L2_CID_VFLIP: > + ret = imx219_write_reg(imx219, IMX219_REG_ORIENTATION, 1, > + imx219->hflip->val | > + imx219->vflip->val << 1); > + break; > + case V4L2_CID_VBLANK: > + ret = imx219_write_reg(imx219, IMX219_REG_VTS, > + IMX219_REG_VALUE_16BIT, > + imx219->mode->height + ctrl->val); > + break; > + case V4L2_CID_TEST_PATTERN_RED: > + ret = imx219_write_reg(imx219, IMX219_REG_TESTP_RED, > + IMX219_REG_VALUE_16BIT, ctrl->val); > + break; > + case V4L2_CID_TEST_PATTERN_GREENR: > + ret = imx219_write_reg(imx219, IMX219_REG_TESTP_GREENR, > + IMX219_REG_VALUE_16BIT, ctrl->val); > + break; > + case V4L2_CID_TEST_PATTERN_BLUE: > + ret = imx219_write_reg(imx219, IMX219_REG_TESTP_BLUE, > + IMX219_REG_VALUE_16BIT, ctrl->val); > + break; > + case V4L2_CID_TEST_PATTERN_GREENB: > + ret = imx219_write_reg(imx219, IMX219_REG_TESTP_GREENB, > + IMX219_REG_VALUE_16BIT, ctrl->val); > + break; > + default: > + dev_info(&client->dev, > + "ctrl(id:0x%x,val:0x%x) is not handled\n", > + ctrl->id, ctrl->val); > + ret = -EINVAL; > + break; > + } > + > + pm_runtime_put(&client->dev); > + > + return ret; > +} > + > +static const struct v4l2_ctrl_ops imx219_ctrl_ops = { > + .s_ctrl = imx219_set_ctrl, > +}; > + > +static int imx219_enum_mbus_code(struct v4l2_subdev *sd, > + struct v4l2_subdev_pad_config *cfg, > + struct v4l2_subdev_mbus_code_enum *code) > +{ > + struct imx219 *imx219 = to_imx219(sd); > + > + /* > + * Only one bayer order is supported (though it depends on the flip > + * settings) > + */ > + if (code->index > 0) > + return -EINVAL; > + > + code->code = imx219_get_format_code(imx219); > + > + return 0; > +} > + > +static int imx219_enum_frame_size(struct v4l2_subdev *sd, > + struct v4l2_subdev_pad_config *cfg, > + struct v4l2_subdev_frame_size_enum *fse) > +{ > + struct imx219 *imx219 = to_imx219(sd); > + > + if (fse->index >= ARRAY_SIZE(supported_modes)) > + return -EINVAL; > + > + if (fse->code != imx219_get_format_code(imx219)) > + return -EINVAL; > + > + fse->min_width = supported_modes[fse->index].width; > + fse->max_width = fse->min_width; > + fse->min_height = supported_modes[fse->index].height; > + fse->max_height = fse->min_height; > + > + return 0; > +} > + > +static void imx219_reset_colorspace(struct v4l2_mbus_framefmt *fmt) > +{ > + fmt->colorspace = V4L2_COLORSPACE_SRGB; > + fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt->colorspace); > + fmt->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(true, > + fmt->colorspace, > + fmt->ycbcr_enc); > + fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt->colorspace); > +} > + > +static void imx219_update_pad_format(struct imx219 *imx219, > + const struct imx219_mode *mode, > + struct v4l2_subdev_format *fmt) > +{ > + fmt->format.width = mode->width; > + fmt->format.height = mode->height; > + fmt->format.code = imx219_get_format_code(imx219); > + fmt->format.field = V4L2_FIELD_NONE; > + > + imx219_reset_colorspace(&fmt->format); > +} > + > +static int __imx219_get_pad_format(struct imx219 *imx219, > + struct v4l2_subdev_pad_config *cfg, > + struct v4l2_subdev_format *fmt) > +{ > + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) > + fmt->format = *v4l2_subdev_get_try_format(&imx219->sd, cfg, > + fmt->pad); > + else > + imx219_update_pad_format(imx219, imx219->mode, fmt); > + > + return 0; > +} > + > +static int imx219_get_pad_format(struct v4l2_subdev *sd, > + struct v4l2_subdev_pad_config *cfg, > + struct v4l2_subdev_format *fmt) > +{ > + struct imx219 *imx219 = to_imx219(sd); > + int ret; > + > + mutex_lock(&imx219->mutex); > + ret = __imx219_get_pad_format(imx219, cfg, fmt); > + mutex_unlock(&imx219->mutex); > + > + return ret; > +} > + > +static int imx219_set_pad_format(struct v4l2_subdev *sd, > + struct v4l2_subdev_pad_config *cfg, > + struct v4l2_subdev_format *fmt) > +{ > + struct imx219 *imx219 = to_imx219(sd); > + const struct imx219_mode *mode; > + struct v4l2_mbus_framefmt *framefmt; > + int exposure_max, exposure_def, hblank; > + > + mutex_lock(&imx219->mutex); > + > + /* Bayer order varies with flips */ > + fmt->format.code = imx219_get_format_code(imx219); > + > + mode = v4l2_find_nearest_size(supported_modes, > + ARRAY_SIZE(supported_modes), > + width, height, > + fmt->format.width, fmt->format.height); > + imx219_update_pad_format(imx219, mode, fmt); > + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) { > + framefmt = v4l2_subdev_get_try_format(sd, cfg, fmt->pad); > + *framefmt = fmt->format; > + } else if (imx219->mode != mode) { > + imx219->mode = mode; > + /* Update limits and set FPS to default */ > + __v4l2_ctrl_modify_range(imx219->vblank, IMX219_VBLANK_MIN, > + IMX219_VTS_MAX - mode->height, 1, > + mode->vts_def - mode->height); > + __v4l2_ctrl_s_ctrl(imx219->vblank, > + mode->vts_def - mode->height); > + /* Update max exposure while meeting expected vblanking */ > + exposure_max = mode->vts_def - 4; > + exposure_def = (exposure_max < IMX219_EXPOSURE_DEFAULT) ? > + exposure_max : IMX219_EXPOSURE_DEFAULT; > + __v4l2_ctrl_modify_range(imx219->exposure, > + imx219->exposure->minimum, > + exposure_max, imx219->exposure->step, > + exposure_def); > + /* > + * Currently PPL is fixed to IMX219_PPL_DEFAULT, so hblank > + * depends on mode->width only, and is not changeble in any > + * way other than changing the mode. > + */ > + hblank = IMX219_PPL_DEFAULT - mode->width; > + __v4l2_ctrl_modify_range(imx219->hblank, hblank, hblank, 1, > + hblank); > + } > + > + mutex_unlock(&imx219->mutex); > + > + return 0; > +} > + > +static int imx219_start_streaming(struct imx219 *imx219) > +{ > + struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd); > + const struct imx219_reg_list *reg_list; > + int ret; > + > + /* Apply default values of current mode */ > + reg_list = &imx219->mode->reg_list; > + ret = imx219_write_regs(imx219, reg_list->regs, reg_list->num_of_regs); > + if (ret) { > + dev_err(&client->dev, "%s failed to set mode\n", __func__); > + return ret; > + } > + > + /* Apply customized values from user */ > + ret = __v4l2_ctrl_handler_setup(imx219->sd.ctrl_handler); > + if (ret) > + return ret; > + > + /* set stream on register */ > + return imx219_write_reg(imx219, IMX219_REG_MODE_SELECT, > + IMX219_REG_VALUE_08BIT, IMX219_MODE_STREAMING); > +} > + > +static void imx219_stop_streaming(struct imx219 *imx219) > +{ > + struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd); > + int ret; > + > + /* set stream off register */ > + ret = imx219_write_reg(imx219, IMX219_REG_MODE_SELECT, > + IMX219_REG_VALUE_08BIT, IMX219_MODE_STANDBY); > + if (ret) > + dev_err(&client->dev, "%s failed to set stream\n", __func__); > +} > + > +static int imx219_set_stream(struct v4l2_subdev *sd, int enable) > +{ > + struct imx219 *imx219 = to_imx219(sd); > + struct i2c_client *client = v4l2_get_subdevdata(sd); > + int ret = 0; > + > + mutex_lock(&imx219->mutex); > + if (imx219->streaming == enable) { > + mutex_unlock(&imx219->mutex); > + return 0; > + } > + > + if (enable) { > + ret = pm_runtime_get_sync(&client->dev); > + if (ret < 0) { > + pm_runtime_put_noidle(&client->dev); > + goto err_unlock; > + } > + > + /* > + * Apply default & customized values > + * and then start streaming. > + */ > + ret = imx219_start_streaming(imx219); > + if (ret) > + goto err_rpm_put; > + } else { > + imx219_stop_streaming(imx219); > + pm_runtime_put(&client->dev); > + } > + > + imx219->streaming = enable; > + > + /* vflip and hflip cannot change during streaming */ > + __v4l2_ctrl_grab(imx219->vflip, enable); > + __v4l2_ctrl_grab(imx219->hflip, enable); > + > + mutex_unlock(&imx219->mutex); > + > + return ret; > + > +err_rpm_put: > + pm_runtime_put(&client->dev); > +err_unlock: > + mutex_unlock(&imx219->mutex); > + > + return ret; > +} > + > +/* Power/clock management functions */ > +static int imx219_power_on(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct v4l2_subdev *sd = i2c_get_clientdata(client); > + struct imx219 *imx219 = to_imx219(sd); > + int ret; > + > + ret = regulator_bulk_enable(IMX219_NUM_SUPPLIES, > + imx219->supplies); > + if (ret) { > + dev_err(&client->dev, "%s: failed to enable regulators\n", > + __func__); > + return ret; > + } > + > + ret = clk_prepare_enable(imx219->xclk); > + if (ret) { > + dev_err(&client->dev, "%s: failed to enable clock\n", > + __func__); > + goto reg_off; > + } > + > + gpiod_set_value_cansleep(imx219->xclr_gpio, 1); > + msleep(IMX219_XCLR_DELAY_MS); I guess 10 ms is ok albeit it'd be nicer if you calculated the required delay instead. > + > + return 0; Empty line here, please. > +reg_off: > + regulator_bulk_disable(IMX219_NUM_SUPPLIES, imx219->supplies); And here. > + return ret; > +} > + > +static int imx219_power_off(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct v4l2_subdev *sd = i2c_get_clientdata(client); > + struct imx219 *imx219 = to_imx219(sd); > + > + gpiod_set_value_cansleep(imx219->xclr_gpio, 0); > + regulator_bulk_disable(IMX219_NUM_SUPPLIES, imx219->supplies); > + clk_disable_unprepare(imx219->xclk); > + > + return 0; > +} > + > +static int __maybe_unused imx219_suspend(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct v4l2_subdev *sd = i2c_get_clientdata(client); > + struct imx219 *imx219 = to_imx219(sd); > + > + if (imx219->streaming) > + imx219_stop_streaming(imx219); > + > + return 0; > +} > + > +static int __maybe_unused imx219_resume(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct v4l2_subdev *sd = i2c_get_clientdata(client); > + struct imx219 *imx219 = to_imx219(sd); > + int ret; > + > + if (imx219->streaming) { > + ret = imx219_start_streaming(imx219); > + if (ret) > + goto error; > + } > + > + return 0; > + > +error: > + imx219_stop_streaming(imx219); > + imx219->streaming = 0; Please add an empty line here. > + return ret; > +} > + > +static int imx219_get_regulators(struct imx219 *imx219) > +{ > + struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd); > + int i; unsigned int > + > + for (i = 0; i < IMX219_NUM_SUPPLIES; i++) > + imx219->supplies[i].supply = imx219_supply_name[i]; > + > + return devm_regulator_bulk_get(&client->dev, > + IMX219_NUM_SUPPLIES, > + imx219->supplies); > +} > + > +/* Verify chip ID */ > +static int imx219_identify_module(struct imx219 *imx219) > +{ > + struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd); > + int ret; > + u32 val; > + > + ret = imx219_read_reg(imx219, IMX219_REG_CHIP_ID, > + IMX219_REG_VALUE_16BIT, &val); > + if (ret) { > + dev_err(&client->dev, "failed to read chip id %x\n", > + IMX219_CHIP_ID); > + return ret; > + } > + > + if (val != IMX219_CHIP_ID) { > + dev_err(&client->dev, "chip id mismatch: %x!=%x\n", > + IMX219_CHIP_ID, val); > + return -EIO; > + } > + > + return 0; > +} > + > +static const struct v4l2_subdev_core_ops imx219_core_ops = { > + .subscribe_event = v4l2_ctrl_subdev_subscribe_event, > + .unsubscribe_event = v4l2_event_subdev_unsubscribe, > +}; > + > +static const struct v4l2_subdev_video_ops imx219_video_ops = { > + .s_stream = imx219_set_stream, > +}; > + > +static const struct v4l2_subdev_pad_ops imx219_pad_ops = { > + .enum_mbus_code = imx219_enum_mbus_code, > + .get_fmt = imx219_get_pad_format, > + .set_fmt = imx219_set_pad_format, > + .enum_frame_size = imx219_enum_frame_size, > +}; > + > +static const struct v4l2_subdev_ops imx219_subdev_ops = { > + .core = &imx219_core_ops, > + .video = &imx219_video_ops, > + .pad = &imx219_pad_ops, > +}; > + > +static const struct v4l2_subdev_internal_ops imx219_internal_ops = { > + .open = imx219_open, > +}; > + > +/* Initialize control handlers */ > +static int imx219_init_controls(struct imx219 *imx219) > +{ > + struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd); > + struct v4l2_ctrl_handler *ctrl_hdlr; > + unsigned int height = imx219->mode->height; > + int exposure_max, exposure_def, hblank; > + int i, ret; > + > + ctrl_hdlr = &imx219->ctrl_handler; > + ret = v4l2_ctrl_handler_init(ctrl_hdlr, 9); > + if (ret) > + return ret; > + > + mutex_init(&imx219->mutex); > + ctrl_hdlr->lock = &imx219->mutex; > + > + /* By default, PIXEL_RATE is read only */ > + imx219->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops, > + V4L2_CID_PIXEL_RATE, > + IMX219_PIXEL_RATE, > + IMX219_PIXEL_RATE, 1, > + IMX219_PIXEL_RATE); > + > + /* Initial vblank/hblank/exposure parameters based on current mode */ > + imx219->vblank = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops, > + V4L2_CID_VBLANK, IMX219_VBLANK_MIN, > + IMX219_VTS_MAX - height, 1, > + imx219->mode->vts_def - height); > + hblank = IMX219_PPL_DEFAULT - imx219->mode->width; > + imx219->hblank = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops, > + V4L2_CID_HBLANK, hblank, hblank, > + 1, hblank); > + if (imx219->hblank) > + imx219->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY; > + exposure_max = imx219->mode->vts_def - 4; > + exposure_def = (exposure_max < IMX219_EXPOSURE_DEFAULT) ? > + exposure_max : IMX219_EXPOSURE_DEFAULT; > + imx219->exposure = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops, > + V4L2_CID_EXPOSURE, > + IMX219_EXPOSURE_MIN, exposure_max, > + IMX219_EXPOSURE_STEP, > + exposure_def); > + > + v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops, V4L2_CID_ANALOGUE_GAIN, > + IMX219_ANA_GAIN_MIN, IMX219_ANA_GAIN_MAX, > + IMX219_ANA_GAIN_STEP, IMX219_ANA_GAIN_DEFAULT); > + > + v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops, V4L2_CID_DIGITAL_GAIN, > + IMX219_DGTL_GAIN_MIN, IMX219_DGTL_GAIN_MAX, > + IMX219_DGTL_GAIN_STEP, IMX219_DGTL_GAIN_DEFAULT); > + > + imx219->hflip = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops, > + V4L2_CID_HFLIP, 0, 1, 1, 0); > + if (imx219->hflip) > + imx219->hflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT; > + > + imx219->vflip = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops, > + V4L2_CID_VFLIP, 0, 1, 1, 0); > + if (imx219->vflip) > + imx219->vflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT; > + > + v4l2_ctrl_new_std_menu_items(ctrl_hdlr, &imx219_ctrl_ops, > + V4L2_CID_TEST_PATTERN, > + ARRAY_SIZE(imx219_test_pattern_menu) - 1, > + 0, 0, imx219_test_pattern_menu); > + for (i = 0; i < 4; i++) { > + /* > + * The assumption is that > + * V4L2_CID_TEST_PATTERN_GREENR == V4L2_CID_TEST_PATTERN_RED + 1 > + * V4L2_CID_TEST_PATTERN_BLUE == V4L2_CID_TEST_PATTERN_RED + 2 > + * V4L2_CID_TEST_PATTERN_GREENB == V4L2_CID_TEST_PATTERN_RED + 3 > + */ > + v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops, > + V4L2_CID_TEST_PATTERN_RED + i, > + IMX219_TESTP_COLOUR_MIN, > + IMX219_TESTP_COLOUR_MAX, > + IMX219_TESTP_COLOUR_STEP, > + IMX219_TESTP_COLOUR_MAX); > + /* The "Solid color" pattern is white by default */ > + } > + > + if (ctrl_hdlr->error) { > + ret = ctrl_hdlr->error; > + dev_err(&client->dev, "%s control init failed (%d)\n", > + __func__, ret); > + goto error; > + } > + > + imx219->sd.ctrl_handler = ctrl_hdlr; > + > + return 0; > + > +error: > + v4l2_ctrl_handler_free(ctrl_hdlr); > + mutex_destroy(&imx219->mutex); > + > + return ret; > +} > + > +static void imx219_free_controls(struct imx219 *imx219) > +{ > + v4l2_ctrl_handler_free(imx219->sd.ctrl_handler); > + mutex_destroy(&imx219->mutex); > +} > + > +static int imx219_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct device *dev = &client->dev; > + struct fwnode_handle *endpoint; > + struct imx219 *imx219; > + int ret; > + > + imx219 = devm_kzalloc(&client->dev, sizeof(*imx219), GFP_KERNEL); > + if (!imx219) > + return -ENOMEM; > + > + imx219->dev = dev; Instead of putting a dev field to struct imx219, you can find the device in struct i2c_client.dev, which you can get by: v4l2_get_subdevdata(imx219->sd) > + > + v4l2_i2c_subdev_init(&imx219->sd, client, &imx219_subdev_ops); > + > + /* Get CSI2 bus config */ > + endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev), > + NULL); > + if (!endpoint) { > + dev_err(dev, "endpoint node not found\n"); > + return -EINVAL; > + } > + > + ret = v4l2_fwnode_endpoint_parse(endpoint, &imx219->ep); > + fwnode_handle_put(endpoint); > + if (ret) { > + dev_err(dev, "Could not parse endpoint\n"); > + return ret; > + } > + > + /* Get system clock (xclk) */ > + imx219->xclk = devm_clk_get(dev, "xclk"); > + if (IS_ERR(imx219->xclk)) { > + dev_err(dev, "failed to get xclk\n"); > + return PTR_ERR(imx219->xclk); > + } > + > + imx219->xclk_freq = clk_get_rate(imx219->xclk); > + if (imx219->xclk_freq != IMX219_XCLK_FREQ) { > + dev_err(dev, "xclk frequency not supported: %d Hz\n", > + imx219->xclk_freq); > + return -EINVAL; > + } Could you also check the link frequencies (the link-frequencies property that also should be added to DT bindings) matches with what is possible with the given xclk frequency? Please see e.g. imx319 driver for an example. > + > + ret = imx219_get_regulators(imx219); > + if (ret) > + return ret; > + > + /* Request optional enable pin */ > + imx219->xclr_gpio = devm_gpiod_get_optional(dev, "xclr", > + GPIOD_OUT_HIGH); > + > + /* > + * The sensor must be powered for imx219_identify_module() > + * to be able to read the CHIP_ID register > + */ > + ret = imx219_power_on(dev); > + if (ret) > + return ret; > + > + ret = imx219_identify_module(imx219); > + if (ret) > + goto error_power_off; > + > + /* Set default mode to max resolution */ > + imx219->mode = &supported_modes[0]; > + > + ret = imx219_init_controls(imx219); > + if (ret) > + goto error_power_off; > + > + /* Initialize subdev */ > + imx219->sd.internal_ops = &imx219_internal_ops; > + imx219->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > + imx219->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR; > + > + /* Initialize source pad */ > + imx219->pad.flags = MEDIA_PAD_FL_SOURCE; > + > + ret = media_entity_pads_init(&imx219->sd.entity, 1, &imx219->pad); > + if (ret) > + goto error_handler_free; > + > + ret = v4l2_async_register_subdev_sensor_common(&imx219->sd); > + if (ret < 0) > + goto error_media_entity; > + > + /* Enable runtime PM and turn off the device */ > + pm_runtime_set_active(dev); > + pm_runtime_enable(dev); > + pm_runtime_idle(dev); > + > + return 0; > + > +error_media_entity: > + media_entity_cleanup(&imx219->sd.entity); > + > +error_handler_free: > + imx219_free_controls(imx219); > + > +error_power_off: > + imx219_power_off(dev); > + > + return ret; > +} > + > +static int imx219_remove(struct i2c_client *client) > +{ > + struct v4l2_subdev *sd = i2c_get_clientdata(client); > + struct imx219 *imx219 = to_imx219(sd); > + > + v4l2_async_unregister_subdev(sd); > + media_entity_cleanup(&sd->entity); > + imx219_free_controls(imx219); > + > + pm_runtime_disable(&client->dev); > + pm_runtime_set_suspended(&client->dev); imx219_power_off(), if the sensor is powered on here. Please see e.g. the smiapp driver regarding this. > + > + return 0; > +} > + > +static const struct of_device_id imx219_dt_ids[] = { > + { .compatible = "sony,imx219" }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, imx219_dt_ids); > + > +static const struct dev_pm_ops imx219_pm_ops = { > + SET_SYSTEM_SLEEP_PM_OPS(imx219_suspend, imx219_resume) > + SET_RUNTIME_PM_OPS(imx219_power_off, imx219_power_on, NULL) > +}; > + > +static struct i2c_driver imx219_i2c_driver = { > + .driver = { > + .name = "imx219", > + .of_match_table = imx219_dt_ids, > + .pm = &imx219_pm_ops, > + }, > + .probe = imx219_probe, Could you use .probe_new, and remove the i2c_device_id argument? > + .remove = imx219_remove, > +}; > + > +module_i2c_driver(imx219_i2c_driver); > + > +MODULE_AUTHOR("Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx"); > +MODULE_DESCRIPTION("Sony IMX219 sensor driver"); > +MODULE_LICENSE("GPL v2"); -- Regards, Sakari Ailus