| Message ID | 20220512074037.3829926-6-benjamin.mugnier@foss.st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: Add ST VGXY61 camera sensor driver | expand |
Hi Benjamin, A few comments below... An my apologies for taking so long to review this set. On Thu, May 12, 2022 at 09:40:37AM +0200, Benjamin Mugnier wrote: > The VGXY61 has a quad lanes CSI-2 output port running at 800mbps per > lane, and supports RAW8, RAW10, RAW12, RAW14 and RAW16 formats. > The driver handles both sensor types: > - VG5661 and VG6661: 1.6 Mpx (1464 x 1104) 75fps. > - VG5761 and VG6761: 2.3 Mpx (1944 x 1204) 60 fps. > The driver supports: > - HDR linearize mode, HDR substraction mode, and no HDR > - GPIOs LEDs strobing > - Digital binning and analog subsampling > - Horizontal and vertical flip > - Manual exposure > - Analog and digital gains > - Test patterns > > Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com> > --- > drivers/media/i2c/Kconfig | 10 + > drivers/media/i2c/Makefile | 1 + > drivers/media/i2c/st-vgxy61.c | 1940 +++++++++++++++++++++++++++++++++ > 3 files changed, 1951 insertions(+) > create mode 100644 drivers/media/i2c/st-vgxy61.c > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig > index fae2baabb773..ee7e8e921783 100644 > --- a/drivers/media/i2c/Kconfig > +++ b/drivers/media/i2c/Kconfig > @@ -753,6 +753,16 @@ config VIDEO_SR030PC30 > help > This driver supports SR030PC30 VGA camera from Siliconfile > > +config VIDEO_ST_VGXY61 > + tristate "ST VGXY61 sensor support" > + depends on OF && GPIOLIB && VIDEO_DEV && I2C > + select MEDIA_CONTROLLER > + select VIDEO_V4L2_SUBDEV_API > + select V4L2_FWNODE > + help > + This is a Video4Linux2 sensor driver for the ST VGXY61 > + camera sensor. > + > config VIDEO_VS6624 > tristate "ST VS6624 sensor support" > depends on VIDEO_DEV && I2C > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile > index 3e1696963e7f..7f2362228484 100644 > --- a/drivers/media/i2c/Makefile > +++ b/drivers/media/i2c/Makefile > @@ -116,6 +116,7 @@ obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o > obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o > obj-$(CONFIG_VIDEO_SR030PC30) += sr030pc30.o > obj-$(CONFIG_VIDEO_ST_MIPID02) += st-mipid02.o > +obj-$(CONFIG_VIDEO_ST_VGXY61) += st-vgxy61.o > obj-$(CONFIG_VIDEO_TC358743) += tc358743.o > obj-$(CONFIG_VIDEO_TDA1997X) += tda1997x.o > obj-$(CONFIG_VIDEO_TDA7432) += tda7432.o > diff --git a/drivers/media/i2c/st-vgxy61.c b/drivers/media/i2c/st-vgxy61.c > new file mode 100644 > index 000000000000..bad8f10f88fe > --- /dev/null > +++ b/drivers/media/i2c/st-vgxy61.c > @@ -0,0 +1,1940 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Driver for VGXY61 global shutter sensor family driver > + * > + * Copyright (C) 2022 STMicroelectronics SA > + */ > + > +#include <linux/clk.h> > +#include <linux/delay.h> > +#include <linux/gpio/consumer.h> > +#include <linux/i2c.h> > +#include <linux/iopoll.h> > +#include <linux/module.h> > +#include <linux/pm_runtime.h> > +#include <linux/regulator/consumer.h> > +#include <linux/units.h> > +#include <media/mipi-csi2.h> > +#include <media/v4l2-async.h> > +#include <media/v4l2-ctrls.h> > +#include <media/v4l2-device.h> > +#include <media/v4l2-fwnode.h> > +#include <media/v4l2-subdev.h> > +#include <uapi/linux/st-vgxy61.h> > + > +#define VGXY61_REG_8BIT(n) ((1 << 16) | (n)) > +#define VGXY61_REG_16BIT(n) ((2 << 16) | (n)) > +#define VGXY61_REG_32BIT(n) ((4 << 16) | (n)) > +#define VGXY61_REG_SIZE_SHIFT 16 > +#define VGXY61_REG_ADDR_MASK 0xffff > + > +#define VGXY61_REG_MODEL_ID VGXY61_REG_16BIT(0x0000) > +#define VG5661_MODEL_ID 0x5661 > +#define VG5761_MODEL_ID 0x5761 > +#define VGXY61_REG_REVISION VGXY61_REG_16BIT(0x0002) > +#define VGXY61_REG_FWPATCH_REVISION VGXY61_REG_16BIT(0x0014) > +#define VGXY61_REG_FWPATCH_START_ADDR VGXY61_REG_8BIT(0x2000) > +#define VGXY61_REG_SYSTEM_FSM VGXY61_REG_8BIT(0x0020) > +#define VGXY61_SYSTEM_FSM_SW_STBY 0x03 > +#define VGXY61_SYSTEM_FSM_STREAMING 0x04 > +#define VGXY61_REG_NVM VGXY61_REG_8BIT(0x0023) > +#define VGXY61_NVM_OK 0x04 > +#define VGXY61_REG_STBY VGXY61_REG_8BIT(0x0201) > +#define VGXY61_STBY_NO_REQ 0 > +#define VGXY61_STBY_REQ_TMP_READ BIT(2) > +#define VGXY61_REG_STREAMING VGXY61_REG_8BIT(0x0202) > +#define VGXY61_STREAMING_NO_REQ 0 > +#define VGXY61_STREAMING_REQ_STOP BIT(0) > +#define VGXY61_STREAMING_REQ_START BIT(1) > +#define VGXY61_REG_EXT_CLOCK VGXY61_REG_32BIT(0x0220) > +#define VGXY61_REG_CLK_PLL_PREDIV VGXY61_REG_8BIT(0x0224) > +#define VGXY61_REG_CLK_SYS_PLL_MULT VGXY61_REG_8BIT(0x0225) > +#define VGXY61_REG_GPIO_0_CTRL VGXY61_REG_8BIT(0x0236) > +#define VGXY61_REG_GPIO_1_CTRL VGXY61_REG_8BIT(0x0237) > +#define VGXY61_REG_GPIO_2_CTRL VGXY61_REG_8BIT(0x0238) > +#define VGXY61_REG_GPIO_3_CTRL VGXY61_REG_8BIT(0x0239) > +#define VGXY61_REG_SIGNALS_POLARITY_CTRL VGXY61_REG_8BIT(0x023b) > +#define VGXY61_REG_LINE_LENGTH VGXY61_REG_16BIT(0x0300) > +#define VGXY61_REG_ORIENTATION VGXY61_REG_8BIT(0x0302) > +#define VGXY61_REG_VT_CTRL VGXY61_REG_8BIT(0x0304) > +#define VGXY61_REG_FORMAT_CTRL VGXY61_REG_8BIT(0x0305) > +#define VGXY61_REG_OIF_CTRL VGXY61_REG_16BIT(0x0306) > +#define VGXY61_REG_OIF_ROI0_CTRL VGXY61_REG_8BIT(0x030a) > +#define VGXY61_REG_ROI0_START_H VGXY61_REG_16BIT(0x0400) > +#define VGXY61_REG_ROI0_START_V VGXY61_REG_16BIT(0x0402) > +#define VGXY61_REG_ROI0_END_H VGXY61_REG_16BIT(0x0404) > +#define VGXY61_REG_ROI0_END_V VGXY61_REG_16BIT(0x0406) > +#define VGXY61_REG_PATGEN_CTRL VGXY61_REG_32BIT(0x0440) > +#define VGXY61_PATGEN_LONG_ENABLE BIT(16) > +#define VGXY61_PATGEN_SHORT_ENABLE BIT(0) > +#define VGXY61_PATGEN_LONG_TYPE_SHIFT 18 > +#define VGXY61_PATGEN_SHORT_TYPE_SHIFT 4 > +#define VGXY61_REG_FRAME_CONTENT_CTRL VGXY61_REG_8BIT(0x0478) > +#define VGXY61_REG_COARSE_EXPOSURE_LONG VGXY61_REG_16BIT(0x0500) > +#define VGXY61_REG_COARSE_EXPOSURE_SHORT VGXY61_REG_16BIT(0x0504) > +#define VGXY61_REG_ANALOG_GAIN VGXY61_REG_8BIT(0x0508) > +#define VGXY61_REG_DIGITAL_GAIN_LONG VGXY61_REG_16BIT(0x050a) > +#define VGXY61_REG_DIGITAL_GAIN_SHORT VGXY61_REG_16BIT(0x0512) > +#define VGXY61_REG_FRAME_LENGTH VGXY61_REG_16BIT(0x051a) > +#define VGXY61_REG_SIGNALS_CTRL VGXY61_REG_16BIT(0x0522) > +#define VGXY61_SIGNALS_GPIO_ID_SHIFT 4 > +#define VGXY61_REG_READOUT_CTRL VGXY61_REG_8BIT(0x0530) > +#define VGXY61_REG_HDR_CTRL VGXY61_REG_8BIT(0x0532) > +#define VGXY61_REG_PATGEN_LONG_DATA_GR VGXY61_REG_16BIT(0x092c) > +#define VGXY61_REG_PATGEN_LONG_DATA_R VGXY61_REG_16BIT(0x092e) > +#define VGXY61_REG_PATGEN_LONG_DATA_B VGXY61_REG_16BIT(0x0930) > +#define VGXY61_REG_PATGEN_LONG_DATA_GB VGXY61_REG_16BIT(0x0932) > +#define VGXY61_REG_PATGEN_SHORT_DATA_GR VGXY61_REG_16BIT(0x0950) > +#define VGXY61_REG_PATGEN_SHORT_DATA_R VGXY61_REG_16BIT(0x0952) > +#define VGXY61_REG_PATGEN_SHORT_DATA_B VGXY61_REG_16BIT(0x0954) > +#define VGXY61_REG_PATGEN_SHORT_DATA_GB VGXY61_REG_16BIT(0x0956) > +#define VGXY61_REG_BYPASS_CTRL VGXY61_REG_8BIT(0x0a60) > + > +#define VGX661_WIDTH 1464 > +#define VGX661_HEIGHT 1104 > +#define VGX761_WIDTH 1944 > +#define VGX761_HEIGHT 1204 > +#define VGX661_DEFAULT_MODE 1 > +#define VGX761_DEFAULT_MODE 1 > +#define VGX661_SHORT_ROT_TERM 93 > +#define VGX761_SHORT_ROT_TERM 90 > +#define VGXY61_EXPOS_ROT_TERM 66 > +#define VGXY61_WRITE_MULTIPLE_CHUNK_MAX 16 > +#define VGXY61_NB_GPIOS 4 > +#define VGXY61_NB_POLARITIES 5 > +#define VGXY61_FRAME_LENGTH_DEF 1313 > +#define VGXY61_MIN_FRAME_LENGTH 1288 > +#define VGXY61_MIN_EXPOSURE 10 > +#define VGXY61_HDR_LINEAR_RATIO 10 > +#define VGXY61_TIMEOUT_MS 500 > +#define VGXY61_MEDIA_BUS_FMT_DEF MEDIA_BUS_FMT_Y8_1X8 > + > +#define VGXY61_FWPATCH_REVISION_MAJOR 2 > +#define VGXY61_FWPATCH_REVISION_MINOR 0 > +#define VGXY61_FWPATCH_REVISION_MICRO 5 > + > +static const u8 patch_array[] = { > + 0xbf, 0x00, 0x05, 0x20, 0x06, 0x01, 0xe0, 0xe0, 0x04, 0x80, 0xe6, 0x45, > + 0xed, 0x6f, 0xfe, 0xff, 0x14, 0x80, 0x1f, 0x84, 0x10, 0x42, 0x05, 0x7c, > + 0x01, 0xc4, 0x1e, 0x80, 0xb6, 0x42, 0x00, 0xe0, 0x1e, 0x82, 0x1e, 0xc0, > + 0x93, 0xdd, 0xc3, 0xc1, 0x0c, 0x04, 0x00, 0xfa, 0x86, 0x0d, 0x70, 0xe1, > + 0x04, 0x98, 0x15, 0x00, 0x28, 0xe0, 0x14, 0x02, 0x08, 0xfc, 0x15, 0x40, > + 0x28, 0xe0, 0x98, 0x58, 0xe0, 0xef, 0x04, 0x98, 0x0e, 0x04, 0x00, 0xf0, > + 0x15, 0x00, 0x28, 0xe0, 0x19, 0xc8, 0x15, 0x40, 0x28, 0xe0, 0xc6, 0x41, > + 0xfc, 0xe0, 0x14, 0x80, 0x1f, 0x84, 0x14, 0x02, 0xa0, 0xfc, 0x1e, 0x80, > + 0x14, 0x80, 0x14, 0x02, 0x80, 0xfb, 0x14, 0x02, 0xe0, 0xfc, 0x1e, 0x80, > + 0x14, 0xc0, 0x1f, 0x84, 0x14, 0x02, 0xa4, 0xfc, 0x1e, 0xc0, 0x14, 0xc0, > + 0x14, 0x02, 0x80, 0xfb, 0x14, 0x02, 0xe4, 0xfc, 0x1e, 0xc0, 0x0c, 0x0c, > + 0x00, 0xf2, 0x93, 0xdd, 0x86, 0x00, 0xf8, 0xe0, 0x04, 0x80, 0xc6, 0x03, > + 0x70, 0xe1, 0x0e, 0x84, 0x93, 0xdd, 0xc3, 0xc1, 0x0c, 0x04, 0x00, 0xfa, > + 0x6b, 0x80, 0x06, 0x40, 0x6c, 0xe1, 0x04, 0x80, 0x09, 0x00, 0xe0, 0xe0, > + 0x0b, 0xa1, 0x95, 0x84, 0x05, 0x0c, 0x1c, 0xe0, 0x86, 0x02, 0xf9, 0x60, > + 0xe0, 0xcf, 0x78, 0x6e, 0x80, 0xef, 0x25, 0x0c, 0x18, 0xe0, 0x05, 0x4c, > + 0x1c, 0xe0, 0x86, 0x02, 0xf9, 0x60, 0xe0, 0xcf, 0x0b, 0x84, 0xd8, 0x6d, > + 0x80, 0xef, 0x05, 0x4c, 0x18, 0xe0, 0x04, 0xd8, 0x0b, 0xa5, 0x95, 0x84, > + 0x05, 0x0c, 0x2c, 0xe0, 0x06, 0x02, 0x01, 0x60, 0xe0, 0xce, 0x18, 0x6d, > + 0x80, 0xef, 0x25, 0x0c, 0x30, 0xe0, 0x05, 0x4c, 0x2c, 0xe0, 0x06, 0x02, > + 0x01, 0x60, 0xe0, 0xce, 0x0b, 0x84, 0x78, 0x6c, 0x80, 0xef, 0x05, 0x4c, > + 0x30, 0xe0, 0x0c, 0x0c, 0x00, 0xf2, 0x93, 0xdd, 0x46, 0x01, 0x70, 0xe1, > + 0x08, 0x80, 0x0b, 0xa1, 0x08, 0x5c, 0x00, 0xda, 0x06, 0x01, 0x68, 0xe1, > + 0x04, 0x80, 0x4a, 0x40, 0x84, 0xe0, 0x08, 0x5c, 0x00, 0x9a, 0x06, 0x01, > + 0xe0, 0xe0, 0x04, 0x80, 0x15, 0x00, 0x60, 0xe0, 0x19, 0xc4, 0x15, 0x40, > + 0x60, 0xe0, 0x15, 0x00, 0x78, 0xe0, 0x19, 0xc4, 0x15, 0x40, 0x78, 0xe0, > + 0x93, 0xdd, 0xc3, 0xc1, 0x46, 0x01, 0x70, 0xe1, 0x08, 0x80, 0x0b, 0xa1, > + 0x08, 0x5c, 0x00, 0xda, 0x06, 0x01, 0x68, 0xe1, 0x04, 0x80, 0x4a, 0x40, > + 0x84, 0xe0, 0x08, 0x5c, 0x00, 0x9a, 0x06, 0x01, 0xe0, 0xe0, 0x14, 0x80, > + 0x25, 0x02, 0x54, 0xe0, 0x29, 0xc4, 0x25, 0x42, 0x54, 0xe0, 0x24, 0x80, > + 0x35, 0x04, 0x6c, 0xe0, 0x39, 0xc4, 0x35, 0x44, 0x6c, 0xe0, 0x25, 0x02, > + 0x64, 0xe0, 0x29, 0xc4, 0x25, 0x42, 0x64, 0xe0, 0x04, 0x80, 0x15, 0x00, > + 0x7c, 0xe0, 0x19, 0xc4, 0x15, 0x40, 0x7c, 0xe0, 0x93, 0xdd, 0xc3, 0xc1, > + 0x4c, 0x04, 0x7c, 0xfa, 0x86, 0x40, 0x98, 0xe0, 0x14, 0x80, 0x1b, 0xa1, > + 0x06, 0x00, 0x00, 0xc0, 0x08, 0x42, 0x38, 0xdc, 0x08, 0x64, 0xa0, 0xef, > + 0x86, 0x42, 0x3c, 0xe0, 0x68, 0x49, 0x80, 0xef, 0x6b, 0x80, 0x78, 0x53, > + 0xc8, 0xef, 0xc6, 0x54, 0x6c, 0xe1, 0x7b, 0x80, 0xb5, 0x14, 0x0c, 0xf8, > + 0x05, 0x14, 0x14, 0xf8, 0x1a, 0xac, 0x8a, 0x80, 0x0b, 0x90, 0x38, 0x55, > + 0x80, 0xef, 0x1a, 0xae, 0x17, 0xc2, 0x03, 0x82, 0x88, 0x65, 0x80, 0xef, > + 0x1b, 0x80, 0x0b, 0x8e, 0x68, 0x65, 0x80, 0xef, 0x9b, 0x80, 0x0b, 0x8c, > + 0x08, 0x65, 0x80, 0xef, 0x6b, 0x80, 0x0b, 0x92, 0x1b, 0x8c, 0x98, 0x64, > + 0x80, 0xef, 0x1a, 0xec, 0x9b, 0x80, 0x0b, 0x90, 0x95, 0x54, 0x10, 0xe0, > + 0xa8, 0x53, 0x80, 0xef, 0x1a, 0xee, 0x17, 0xc2, 0x03, 0x82, 0xf8, 0x63, > + 0x80, 0xef, 0x1b, 0x80, 0x0b, 0x8e, 0xd8, 0x63, 0x80, 0xef, 0x1b, 0x8c, > + 0x68, 0x63, 0x80, 0xef, 0x6b, 0x80, 0x0b, 0x92, 0x65, 0x54, 0x14, 0xe0, > + 0x08, 0x65, 0x84, 0xef, 0x68, 0x63, 0x80, 0xef, 0x7b, 0x80, 0x0b, 0x8c, > + 0xa8, 0x64, 0x84, 0xef, 0x08, 0x63, 0x80, 0xef, 0x14, 0xe8, 0x46, 0x44, > + 0x94, 0xe1, 0x24, 0x88, 0x4a, 0x4e, 0x04, 0xe0, 0x14, 0xea, 0x1a, 0x04, > + 0x08, 0xe0, 0x0a, 0x40, 0x84, 0xed, 0x0c, 0x04, 0x00, 0xe2, 0x4a, 0x40, > + 0x04, 0xe0, 0x19, 0x16, 0xc0, 0xe0, 0x0a, 0x40, 0x84, 0xed, 0x21, 0x54, > + 0x60, 0xe0, 0x0c, 0x04, 0x00, 0xe2, 0x1b, 0xa5, 0x0e, 0xea, 0x01, 0x89, > + 0x21, 0x54, 0x64, 0xe0, 0x7e, 0xe8, 0x65, 0x82, 0x1b, 0xa7, 0x26, 0x00, > + 0x00, 0x80, 0xa5, 0x82, 0x1b, 0xa9, 0x65, 0x82, 0x1b, 0xa3, 0x01, 0x85, > + 0x16, 0x00, 0x00, 0xc0, 0x01, 0x54, 0x04, 0xf8, 0x06, 0xaa, 0x01, 0x83, > + 0x06, 0xa8, 0x65, 0x81, 0x06, 0xa8, 0x01, 0x54, 0x04, 0xf8, 0x01, 0x83, > + 0x06, 0xaa, 0x09, 0x14, 0x18, 0xf8, 0x0b, 0xa1, 0x05, 0x84, 0xc6, 0x42, > + 0xd4, 0xe0, 0x14, 0x84, 0x01, 0x83, 0x01, 0x54, 0x60, 0xe0, 0x01, 0x54, > + 0x64, 0xe0, 0x0b, 0x02, 0x90, 0xe0, 0x10, 0x02, 0x90, 0xe5, 0x01, 0x54, > + 0x88, 0xe0, 0xb5, 0x81, 0xc6, 0x40, 0xd4, 0xe0, 0x14, 0x80, 0x0b, 0x02, > + 0xe0, 0xe4, 0x10, 0x02, 0x31, 0x66, 0x02, 0xc0, 0x01, 0x54, 0x88, 0xe0, > + 0x1a, 0x84, 0x29, 0x14, 0x10, 0xe0, 0x1c, 0xaa, 0x2b, 0xa1, 0xf5, 0x82, > + 0x25, 0x14, 0x10, 0xf8, 0x2b, 0x04, 0xa8, 0xe0, 0x20, 0x44, 0x0d, 0x70, > + 0x03, 0xc0, 0x2b, 0xa1, 0x04, 0x00, 0x80, 0x9a, 0x02, 0x40, 0x84, 0x90, > + 0x03, 0x54, 0x04, 0x80, 0x4c, 0x0c, 0x7c, 0xf2, 0x93, 0xdd, 0x00, 0x00, > + 0x02, 0xa9, 0x00, 0x00, 0x64, 0x4a, 0x40, 0x00, 0x08, 0x2d, 0x58, 0xe0, > + 0xa8, 0x98, 0x40, 0x00, 0x28, 0x07, 0x34, 0xe0, 0x05, 0xb9, 0x00, 0x00, > + 0x28, 0x00, 0x41, 0x05, 0x88, 0x00, 0x41, 0x3c, 0x98, 0x00, 0x41, 0x52, > + 0x04, 0x01, 0x41, 0x79, 0x3c, 0x01, 0x41, 0x6a, 0x3d, 0xfe, 0x00, 0x00, > +}; > + > +static const char * const vgxy61_test_pattern_menu[] = { > + "Disabled", > + "Solid", > + "Colorbar", > + "Gradbar", > + "Hgrey", > + "Vgrey", > + "Dgrey", > + "PN28", > +}; > + > +static const char * const vgxy61_hdr_mode_menu[] = { > + "HDR linearize", > + "HDR substraction", > + "no HDR", > +}; > + > +/* Regulator supplies */ > +static const char * const vgxy61_supply_name[] = { > + "VCORE", > + "VDDIO", > + "VANA", > +}; > + > +static const s64 link_freq[] = { > + /* > + * MIPI output freq is 804Mhz / 2, as it uses both rising edge and falling edges to send > + * data > + */ > + 402000000ULL > +}; > + > +#define VGXY61_NUM_SUPPLIES ARRAY_SIZE(vgxy61_supply_name) This would be nicer just after the supply names. > + > +enum vgxy61_bin_mode { > + VGXY61_BIN_MODE_NORMAL, > + VGXY61_BIN_MODE_DIGITAL_X2, > + VGXY61_BIN_MODE_DIGITAL_X4, > +}; > + > +enum vgxy61_hdr_mode { > + VGXY61_HDR_LINEAR, > + VGXY61_HDR_SUB, > + VGXY61_NO_HDR, > +}; > + > +enum vgxy61_strobe_mode { > + VGXY61_STROBE_DISABLED, > + VGXY61_STROBE_LONG, > + VGXY61_STROBE_ENABLED, > +}; > + > +struct vgxy61_mode_info { > + u32 width; > + u32 height; > + enum vgxy61_bin_mode bin_mode; > + struct v4l2_rect crop; > +}; > + > +struct vgxy61_fmt_desc { > + u32 code; > + u8 bpp; > + u8 data_type; > +}; > + > +static const struct vgxy61_fmt_desc vgxy61_supported_codes[] = { > + { > + .code = MEDIA_BUS_FMT_Y8_1X8, > + .bpp = 8, > + .data_type = MIPI_CSI2_DT_RAW8, > + }, > + { > + .code = MEDIA_BUS_FMT_Y10_1X10, > + .bpp = 10, > + .data_type = MIPI_CSI2_DT_RAW10, > + }, > + { > + .code = MEDIA_BUS_FMT_Y12_1X12, > + .bpp = 12, > + .data_type = MIPI_CSI2_DT_RAW12, > + }, > + { > + .code = MEDIA_BUS_FMT_Y14_1X14, > + .bpp = 14, > + .data_type = MIPI_CSI2_DT_RAW14, > + }, > + { > + .code = MEDIA_BUS_FMT_Y16_1X16, > + .bpp = 16, > + .data_type = MIPI_CSI2_DT_RAW16, > + }, > +}; > + > +static const struct vgxy61_mode_info vgx661_mode_data[] = { > + { > + .width = VGX661_WIDTH, > + .height = VGX661_HEIGHT, > + .bin_mode = VGXY61_BIN_MODE_NORMAL, > + .crop = { > + .left = 0, > + .top = 0, > + .width = VGX661_WIDTH, > + .height = VGX661_HEIGHT, > + }, > + }, > + { > + .width = 1280, > + .height = 720, > + .bin_mode = VGXY61_BIN_MODE_NORMAL, > + .crop = { > + .left = 92, > + .top = 192, > + .width = 1280, > + .height = 720, > + }, > + }, > + { > + .width = 640, > + .height = 480, > + .bin_mode = VGXY61_BIN_MODE_DIGITAL_X2, > + .crop = { > + .left = 92, > + .top = 72, > + .width = 1280, > + .height = 960, > + }, > + }, > + { > + .width = 320, > + .height = 240, > + .bin_mode = VGXY61_BIN_MODE_DIGITAL_X4, > + .crop = { > + .left = 92, > + .top = 72, > + .width = 1280, > + .height = 960, > + }, > + }, > +}; > + > +static const struct vgxy61_mode_info vgx761_mode_data[] = { > + { > + .width = VGX761_WIDTH, > + .height = VGX761_HEIGHT, > + .bin_mode = VGXY61_BIN_MODE_NORMAL, > + .crop = { > + .left = 0, > + .top = 0, > + .width = VGX761_WIDTH, > + .height = VGX761_HEIGHT, > + }, > + }, > + { > + .width = 1920, > + .height = 1080, > + .bin_mode = VGXY61_BIN_MODE_NORMAL, > + .crop = { > + .left = 12, > + .top = 62, > + .width = 1920, > + .height = 1080, > + }, > + }, > + { > + .width = 1280, > + .height = 720, > + .bin_mode = VGXY61_BIN_MODE_NORMAL, > + .crop = { > + .left = 332, > + .top = 242, > + .width = 1280, > + .height = 720, > + }, > + }, > + { > + .width = 640, > + .height = 480, > + .bin_mode = VGXY61_BIN_MODE_DIGITAL_X2, > + .crop = { > + .left = 332, > + .top = 122, > + .width = 1280, > + .height = 960, > + }, > + }, > + { > + .width = 320, > + .height = 240, > + .bin_mode = VGXY61_BIN_MODE_DIGITAL_X4, > + .crop = { > + .left = 332, > + .top = 122, > + .width = 1280, > + .height = 960, > + }, > + }, > +}; > + > +struct vgxy61_dev { > + struct i2c_client *i2c_client; > + struct v4l2_subdev sd; > + struct media_pad pad; > + struct regulator_bulk_data supplies[VGXY61_NUM_SUPPLIES]; > + struct gpio_desc *reset_gpio; > + struct clk *xclk; > + u32 clk_freq; > + int id; > + int sensor_width; > + int sensor_height; > + u16 oif_ctrl; > + int nb_of_lane; > + int data_rate_in_mbps; > + int pclk; > + u16 line_length; > + int rot_term; rot_term value is always the same. No need for a field here for that. > + bool gpios_polarity; > + bool slave_mode; > + /* Lock to protect all members below */ > + struct mutex lock; > + struct v4l2_ctrl_handler ctrl_handler; > + struct v4l2_ctrl *pixel_rate_ctrl; > + struct v4l2_ctrl *expo_ctrl; > + struct v4l2_ctrl *vblank_ctrl; > + struct v4l2_ctrl *vflip_ctrl; > + struct v4l2_ctrl *hflip_ctrl; > + bool streaming; > + struct v4l2_mbus_framefmt fmt; > + const struct vgxy61_mode_info *sensor_modes; > + int sensor_modes_nb; > + const struct vgxy61_mode_info *default_mode; > + const struct vgxy61_mode_info *current_mode; > + bool hflip; > + bool vflip; > + enum vgxy61_hdr_mode hdr; > + int expo_long; > + int expo_short; > + int expo_max; > + int expo_min; > + int vblank; > + int vblank_min; > + u16 frame_length; > + int digital_gain; > + int analog_gain; > + enum vgxy61_strobe_mode strobe_mode; > + int pattern; > +}; > + > +static u8 get_bpp_by_code(__u32 code) > +{ > + int i; unsigned int? Same for the rest of similar instances. > + > + for (i = 0; i < ARRAY_SIZE(vgxy61_supported_codes); i++) { > + if (vgxy61_supported_codes[i].code == code) > + return vgxy61_supported_codes[i].bpp; > + } > + /* Should never happen */ > + WARN(1, "Unsupported code %d. default to 8 bpp", code); > + return 8; > +} > + > +static u8 get_data_type_by_code(__u32 code) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(vgxy61_supported_codes); i++) { > + if (vgxy61_supported_codes[i].code == code) > + return vgxy61_supported_codes[i].data_type; > + } > + /* Should never happen */ > + WARN(1, "Unsupported code %d. default to MIPI_CSI2_DT_RAW8 data type", code); > + return MIPI_CSI2_DT_RAW8; > +} > + > +static void compute_pll_parameters_by_freq(u32 freq, unsigned int *prediv, unsigned int *mult) > +{ > + const unsigned int predivs[] = {1, 2, 4}; > + int i; > + > + /* > + * Freq range is [6Mhz-27Mhz] already checked. > + * Output of divider should be in [6Mhz-12Mhz[. > + */ > + for (i = 0; i < ARRAY_SIZE(predivs); i++) { > + *prediv = predivs[i]; > + if (freq / *prediv < 12 * HZ_PER_MHZ) > + break; > + } > + WARN_ON(i == ARRAY_SIZE(predivs)); > + > + /* > + * Target freq is 804Mhz. Don't change this as it will impact image quality. > + */ > + *mult = ((804 * HZ_PER_MHZ) * (*prediv) + freq / 2) / freq; > +} > + > +static s32 get_pixel_rate(struct vgxy61_dev *sensor) > +{ > + return div64_u64((u64)sensor->data_rate_in_mbps * sensor->nb_of_lane, > + get_bpp_by_code(sensor->fmt.code)); > +} > + > +static inline struct vgxy61_dev *to_vgxy61_dev(struct v4l2_subdev *sd) > +{ > + return container_of(sd, struct vgxy61_dev, sd); > +} > + > +static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl) > +{ > + return &container_of(ctrl->handler, struct vgxy61_dev, ctrl_handler)->sd; Could you run $ checkpatch.pl --strict --max-line-length=80 on this and address those instances where it's reasonable (I'd presume most)? > +} > + > +static int get_chunk_size(struct vgxy61_dev *sensor) > +{ > + struct i2c_adapter *adapter = sensor->i2c_client->adapter; > + int max_write_len = VGXY61_WRITE_MULTIPLE_CHUNK_MAX; > + > + if (adapter->quirks && adapter->quirks->max_write_len) > + max_write_len = adapter->quirks->max_write_len - 2; > + > + max_write_len = min(max_write_len, VGXY61_WRITE_MULTIPLE_CHUNK_MAX); > + > + return max(max_write_len, 1); > +} > + > +static int vgxy61_read_multiple(struct vgxy61_dev *sensor, u32 reg, int len) > +{ > + struct i2c_client *client = sensor->i2c_client; > + struct i2c_msg msg[2]; > + u8 buf[2]; > + u8 val[sizeof(int)] = {0}; sizeof(u32) > + int ret; > + > + if (len > sizeof(int)) Same here. > + return -EINVAL; > + buf[0] = reg >> 8; > + buf[1] = reg & 0xff; > + > + msg[0].addr = client->addr; > + msg[0].flags = client->flags; > + msg[0].buf = buf; > + msg[0].len = sizeof(buf); > + > + msg[1].addr = client->addr; > + msg[1].flags = client->flags | I2C_M_RD; > + msg[1].buf = val; > + msg[1].len = len; > + > + ret = i2c_transfer(client->adapter, msg, 2); > + if (ret < 0) { > + dev_dbg(&client->dev, "%s: %x i2c_transfer, reg: %x => %d\n", __func__, > + client->addr, reg, ret); > + return ret; > + } > + > + return (val[3] << 24) | (val[2] << 16) | (val[1] << 8) | val[0]; get_unaligned_le32() > +} > + > +static inline int vgxy61_read_reg(struct vgxy61_dev *sensor, u32 reg) > +{ > + return vgxy61_read_multiple(sensor, reg & VGXY61_REG_ADDR_MASK, > + (reg >> VGXY61_REG_SIZE_SHIFT) & 7); > +} > + > +static int vgxy61_write_multiple(struct vgxy61_dev *sensor, u32 reg, const u8 *data, int len, > + int *err) > +{ > + struct i2c_client *client = sensor->i2c_client; > + struct i2c_msg msg; > + u8 buf[VGXY61_WRITE_MULTIPLE_CHUNK_MAX + 2]; > + int i; > + int ret; > + > + if (err && *err) > + return *err; > + > + if (len > VGXY61_WRITE_MULTIPLE_CHUNK_MAX) > + return -EINVAL; > + buf[0] = reg >> 8; > + buf[1] = reg & 0xff; > + for (i = 0; i < len; i++) > + buf[i + 2] = data[i]; > + > + msg.addr = client->addr; > + msg.flags = client->flags; > + msg.buf = buf; > + msg.len = len + 2; > + > + ret = i2c_transfer(client->adapter, &msg, 1); > + if (ret < 0) { > + dev_dbg(&client->dev, "%s: i2c_transfer, reg: %x => %d\n", __func__, reg, ret); > + if (err) > + *err = ret; > + return ret; > + } > + > + return 0; > +} > + > +static int vgxy61_write_array(struct vgxy61_dev *sensor, u32 reg, int nb, const u8 *array) nb should probably be unsigned int. > +{ > + const int chunk_size = get_chunk_size(sensor); > + int ret; > + int sz; Same for sz and chunk_size. I believe this also applies to other signed variables in this driver. > + > + while (nb) { > + sz = min(nb, chunk_size); > + ret = vgxy61_write_multiple(sensor, reg, array, sz, NULL); > + if (ret < 0) > + return ret; > + nb -= sz; > + reg += sz; > + array += sz; > + } > + > + return 0; > +} > + > +static inline int vgxy61_write_reg(struct vgxy61_dev *sensor, u32 reg, u32 val, int *err) > +{ > + return vgxy61_write_multiple(sensor, reg & VGXY61_REG_ADDR_MASK, (u8 *)&val, > + (reg >> VGXY61_REG_SIZE_SHIFT) & 7, err); > +} > + > +static int vgxy61_poll_reg(struct vgxy61_dev *sensor, u32 reg, u8 poll_val, int timeout_ms) > +{ > + const int loop_delay_ms = 10; > + int ret, timeout; > + > + timeout = read_poll_timeout(vgxy61_read_reg, ret, ((ret < 0) || (ret == poll_val)), > + loop_delay_ms * 1000, timeout_ms * 1000, false, > + sensor, reg); > + if (timeout) > + return timeout; > + > + return 0; > +} > + > +static int vgxy61_wait_state(struct vgxy61_dev *sensor, int state, int timeout_ms) > +{ > + return vgxy61_poll_reg(sensor, VGXY61_REG_SYSTEM_FSM, state, timeout_ms); > +} > + > +static int vgxy61_check_bw(struct vgxy61_dev *sensor) > +{ > + struct i2c_client *client = sensor->i2c_client; > + /* Correction factor for time required between 2 lines */ > + const int mipi_margin = 1056; What does this value signify? Is it really constant or somehow dependent by the configuration set by the driver? > + int binning_scale = sensor->current_mode->crop.height / sensor->current_mode->height; > + int bpp = get_bpp_by_code(sensor->fmt.code); > + int max_bit_per_line; > + int bit_per_line; > + u64 line_rate; > + > + line_rate = sensor->nb_of_lane * (u64)sensor->data_rate_in_mbps * sensor->line_length; > + max_bit_per_line = div64_u64(line_rate, sensor->pclk) - mipi_margin; > + bit_per_line = (bpp * sensor->current_mode->width) / binning_scale; > + > + dev_dbg(&client->dev, "max_bit_per_line = %d\n", max_bit_per_line); > + dev_dbg(&client->dev, "required bit_per_line = %d\n", bit_per_line); > + > + return bit_per_line > max_bit_per_line ? -EINVAL : 0; > +} > + > +static int vgxy61_apply_exposure(struct vgxy61_dev *sensor) > +{ > + struct i2c_client *client = sensor->i2c_client; > + int ret = 0; > + > + /* We first set expo to zero to avoid forbidden parameters couple */ > + ret = vgxy61_write_reg(sensor, VGXY61_REG_COARSE_EXPOSURE_SHORT, 0, &ret); > + ret = vgxy61_write_reg(sensor, VGXY61_REG_COARSE_EXPOSURE_LONG, sensor->expo_long, &ret); > + ret = vgxy61_write_reg(sensor, VGXY61_REG_COARSE_EXPOSURE_SHORT, sensor->expo_short, &ret); > + if (ret) > + return ret; > + > + dev_dbg(&client->dev, "%s applied expo %d (short: %d)\n", __func__, > + sensor->expo_long, sensor->expo_short); > + > + return 0; > +} > + > +static int vgxy61_get_regulators(struct vgxy61_dev *sensor) > +{ > + int i; > + > + for (i = 0; i < VGXY61_NUM_SUPPLIES; i++) > + sensor->supplies[i].supply = vgxy61_supply_name[i]; unsigned int. > + > + return devm_regulator_bulk_get(&sensor->i2c_client->dev, VGXY61_NUM_SUPPLIES, > + sensor->supplies); > +} > + > +static int vgxy61_apply_reset(struct vgxy61_dev *sensor) > +{ > + struct i2c_client *client = sensor->i2c_client; > + > + dev_dbg(&client->dev, "%s applied reset\n", __func__); > + gpiod_set_value_cansleep(sensor->reset_gpio, 0); > + usleep_range(5000, 10000); > + gpiod_set_value_cansleep(sensor->reset_gpio, 1); > + usleep_range(5000, 10000); > + gpiod_set_value_cansleep(sensor->reset_gpio, 0); > + usleep_range(40000, 100000); > + return vgxy61_wait_state(sensor, VGXY61_SYSTEM_FSM_SW_STBY, VGXY61_TIMEOUT_MS); > +} > + > +static void vgxy61_fill_framefmt(struct vgxy61_dev *sensor, const struct vgxy61_mode_info *mode, > + struct v4l2_mbus_framefmt *fmt, u32 code) > +{ > + fmt->code = code; > + fmt->width = mode->width; > + fmt->height = mode->height; > + fmt->colorspace = V4L2_COLORSPACE_RAW; > + fmt->field = V4L2_FIELD_NONE; > + fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; > + fmt->quantization = V4L2_QUANTIZATION_DEFAULT; > + fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT; > +} > + > +static int vgxy61_try_fmt_internal(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *fmt, > + const struct vgxy61_mode_info **new_mode) > +{ > + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); > + const struct vgxy61_mode_info *mode = sensor->sensor_modes; > + unsigned int index; > + > + /* Select code */ > + for (index = 0; index < ARRAY_SIZE(vgxy61_supported_codes); index++) { > + if (vgxy61_supported_codes[index].code == fmt->code) > + break; > + } > + if (index == ARRAY_SIZE(vgxy61_supported_codes)) > + index = 0; > + > + /* Select size */ > + mode = v4l2_find_nearest_size(sensor->sensor_modes, sensor->sensor_modes_nb, > + width, height, fmt->width, fmt->height); > + if (new_mode) > + *new_mode = mode; > + > + vgxy61_fill_framefmt(sensor, mode, fmt, vgxy61_supported_codes[index].code); Please run the set through this: $ ./scripts/checkpatch.pl --strict --max-line-length=80 > + > + return 0; > +} > + > +static int vgxy61_get_selection(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *sd_state, > + struct v4l2_subdev_selection *sel) > +{ > + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); > + > + switch (sel->target) { > + case V4L2_SEL_TGT_CROP: > + sel->r = sensor->current_mode->crop; > + return 0; > + case V4L2_SEL_TGT_NATIVE_SIZE: > + case V4L2_SEL_TGT_CROP_DEFAULT: > + case V4L2_SEL_TGT_CROP_BOUNDS: > + sel->r.top = 0; > + sel->r.left = 0; > + sel->r.width = sensor->sensor_width; > + sel->r.height = sensor->sensor_height; > + return 0; > + } > + > + return -EINVAL; > +} > + > +static int vgxy61_enum_mbus_code(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *sd_state, > + struct v4l2_subdev_mbus_code_enum *code) > +{ > + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); > + struct i2c_client *client = sensor->i2c_client; > + > + dev_dbg(&client->dev, "%s probe index %d\n", __func__, code->index); This is hardly necessary anymore, please remove. > + if (code->index >= ARRAY_SIZE(vgxy61_supported_codes)) > + return -EINVAL; > + > + code->code = vgxy61_supported_codes[code->index].code; > + > + return 0; > +} > + > +static int vgxy61_get_fmt(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *sd_state, > + struct v4l2_subdev_format *format) > +{ > + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); > + struct i2c_client *client = sensor->i2c_client; > + struct v4l2_mbus_framefmt *fmt; > + > + dev_dbg(&client->dev, "%s probe %d\n", __func__, format->pad); > + dev_dbg(&client->dev, "%s %dx%d\n", __func__, format->format.width, format->format.height); These seem hardly useful. > + > + mutex_lock(&sensor->lock); > + > + if (format->which == V4L2_SUBDEV_FORMAT_TRY) > + fmt = v4l2_subdev_get_try_format(&sensor->sd, sd_state, format->pad); > + else > + fmt = &sensor->fmt; > + > + format->format = *fmt; > + > + mutex_unlock(&sensor->lock); > + > + return 0; > +} > + > +static int vgxy61_get_vblank_min(struct vgxy61_dev *sensor, enum vgxy61_hdr_mode hdr) > +{ > + int min_vblank = VGXY61_MIN_FRAME_LENGTH - sensor->current_mode->crop.height; > + /* Ensure the first rule of thumb can't be negative */ > + int min_vblank_hdr = VGXY61_MIN_EXPOSURE + sensor->rot_term + 1; > + > + if (hdr != VGXY61_NO_HDR) > + return max(min_vblank, min_vblank_hdr); > + return min_vblank; > +} > + > +static int vgxy61_enum_frame_size(struct v4l2_subdev *sd, struct v4l2_subdev_state *sd_state, > + struct v4l2_subdev_frame_size_enum *fse) > +{ > + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); > + struct i2c_client *client = sensor->i2c_client; > + > + dev_dbg(&client->dev, "%s for index %d\n", __func__, fse->index); Ditto. > + if (fse->index >= sensor->sensor_modes_nb) > + return -EINVAL; > + > + fse->min_width = sensor->sensor_modes[fse->index].width; > + fse->max_width = fse->min_width; > + fse->min_height = sensor->sensor_modes[fse->index].height; > + fse->max_height = fse->min_height; > + > + return 0; > +} > + > +static int vgxy61_update_analog_gain(struct vgxy61_dev *sensor, u32 target) > +{ > + struct i2c_client *client = sensor->i2c_client; > + > + dev_dbg(&client->dev, "Set analog gain: 0x%04x\n", target); Ditto. > + sensor->analog_gain = target; > + > + /* Apply gain */ Ditto. The same for rest of similar comments and debug prints. > + if (sensor->streaming) > + return vgxy61_write_reg(sensor, VGXY61_REG_ANALOG_GAIN, target, NULL); > + return 0; > +} > + > +static int vgxy61_apply_digital_gain(struct vgxy61_dev *sensor, u32 digital_gain) > +{ > + int ret = 0; > + > + /* > + * For a monochrome version, configuring DIGITAL_GAIN_LONG_CH0 and > + * DIGITAL_GAIN_SHORT_CH0 is enough to configure the gain of all > + * four sub pixels. > + */ > + vgxy61_write_reg(sensor, VGXY61_REG_DIGITAL_GAIN_LONG, digital_gain, &ret); > + vgxy61_write_reg(sensor, VGXY61_REG_DIGITAL_GAIN_SHORT, digital_gain, &ret); > + > + return ret; > +} > + > +static int vgxy61_update_digital_gain(struct vgxy61_dev *sensor, u32 target) > +{ > + struct i2c_client *client = sensor->i2c_client; > + > + dev_dbg(&client->dev, "Set digital gain: 0x%04x\n", target); > + sensor->digital_gain = target; > + > + if (sensor->streaming) > + return vgxy61_apply_digital_gain(sensor, sensor->digital_gain); > + return 0; > +} > + > +static int vgxy61_apply_patgen(struct vgxy61_dev *sensor, u32 index) > +{ > + static const u8 index2val[] = {0x0, 0x1, 0x2, 0x3, 0x10, 0x11, 0x12, 0x13}; > + u32 pattern = index2val[index]; > + u32 reg = (pattern << VGXY61_PATGEN_LONG_TYPE_SHIFT) | > + (pattern << VGXY61_PATGEN_SHORT_TYPE_SHIFT); > + > + if (pattern) > + reg |= VGXY61_PATGEN_LONG_ENABLE | VGXY61_PATGEN_SHORT_ENABLE; > + return vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_CTRL, reg, NULL); > +} > + > +static int vgxy61_update_patgen(struct vgxy61_dev *sensor, int pattern) > +{ > + sensor->pattern = pattern; > + > + if (sensor->streaming) > + return vgxy61_apply_patgen(sensor, sensor->pattern); > + return 0; > +} > + > +static int vgxy61_apply_gpiox_strobe_mode(struct vgxy61_dev *sensor, enum vgxy61_strobe_mode mode, > + int idx) > +{ > + static const u8 index2val[] = {0x0, 0x1, 0x3}; > + u16 reg; > + > + reg = vgxy61_read_reg(sensor, VGXY61_REG_SIGNALS_CTRL); > + if (reg < 0) > + return reg; > + reg &= ~(0xf << (idx * VGXY61_SIGNALS_GPIO_ID_SHIFT)); > + reg |= index2val[mode] << (idx * VGXY61_SIGNALS_GPIO_ID_SHIFT); > + > + return vgxy61_write_reg(sensor, VGXY61_REG_SIGNALS_CTRL, reg, NULL); > +} > + > +static int vgxy61_update_gpios_strobe_mode(struct vgxy61_dev *sensor, enum vgxy61_hdr_mode hdr) > +{ > + int i, ret; > + > + switch (hdr) { > + case VGXY61_HDR_LINEAR: > + sensor->strobe_mode = VGXY61_STROBE_ENABLED; > + break; > + case VGXY61_HDR_SUB: > + case VGXY61_NO_HDR: > + sensor->strobe_mode = VGXY61_STROBE_LONG; > + break; > + default: > + /* Should never happen */ > + WARN_ON(true); > + break; > + } > + > + if (sensor->streaming) { > + for (i = 0; i < VGXY61_NB_GPIOS; i++) { > + ret = vgxy61_apply_gpiox_strobe_mode(sensor, sensor->strobe_mode, i); > + if (ret) > + return ret; > + } > + } > + > + return 0; > +} > + > +static int vgxy61_update_gpios_strobe_polarity(struct vgxy61_dev *sensor, int polarity) > +{ > + struct i2c_client *client = sensor->i2c_client; > + int ret = 0; > + > + if (sensor->streaming) > + return -EBUSY; > + > + dev_dbg(&client->dev, "setting gpios polarity: %d\n", polarity); > + > + vgxy61_write_reg(sensor, VGXY61_REG_GPIO_0_CTRL, polarity << 1, &ret); > + vgxy61_write_reg(sensor, VGXY61_REG_GPIO_1_CTRL, polarity << 1, &ret); > + vgxy61_write_reg(sensor, VGXY61_REG_GPIO_2_CTRL, polarity << 1, &ret); > + vgxy61_write_reg(sensor, VGXY61_REG_GPIO_3_CTRL, polarity << 1, &ret); > + vgxy61_write_reg(sensor, VGXY61_REG_SIGNALS_POLARITY_CTRL, polarity, &ret); > + > + return ret; > +} > + > +static int vgxy61_get_expo_long_max(struct vgxy61_dev *sensor, int short_expo_ratio) > +{ > + int first_rot_max_expo, second_rot_max_expo, third_rot_max_expo; > + > + /* Apply sensor's rules of thumb */ > + /* > + * Short exposure + height must be less than frame length to avoid bad pixel line at the > + * botom of the image > + */ > + first_rot_max_expo = > + ((sensor->frame_length - sensor->current_mode->crop.height - sensor->rot_term) * > + short_expo_ratio) - 1; > + > + /* Total exposition time must be less than frame length to avoid sensor crash */ > + second_rot_max_expo = > + (((sensor->frame_length - VGXY61_EXPOS_ROT_TERM) * short_expo_ratio) / > + (short_expo_ratio + 1)) - 1; > + > + /* Short exposure times 71 must be less than frame length to avoid sensor crash */ > + third_rot_max_expo = (sensor->frame_length / 71) * short_expo_ratio; > + > + /* Take the minimum from all rules */ > + return min(min(first_rot_max_expo, second_rot_max_expo), third_rot_max_expo); > +} > + > +static int vgxy61_update_exposure(struct vgxy61_dev *sensor, int new_expo_long, > + enum vgxy61_hdr_mode hdr) > +{ > + struct i2c_client *client = sensor->i2c_client; > + int new_expo_short = 0; > + int expo_short_max = 0; > + int expo_long_min = VGXY61_MIN_EXPOSURE; > + int expo_long_max; > + > + /* Compute short exposure according to hdr mode and long exposure */ > + switch (hdr) { > + case VGXY61_HDR_LINEAR: > + /* Take ratio into account for minimal exposures in VGXY61_HDR_LINEAR */ > + expo_long_min = VGXY61_MIN_EXPOSURE * VGXY61_HDR_LINEAR_RATIO; > + new_expo_long = max(expo_long_min, new_expo_long); > + > + expo_long_max = vgxy61_get_expo_long_max(sensor, VGXY61_HDR_LINEAR_RATIO); > + expo_short_max = > + (expo_long_max + (VGXY61_HDR_LINEAR_RATIO / 2)) / VGXY61_HDR_LINEAR_RATIO; > + new_expo_short = > + (new_expo_long + (VGXY61_HDR_LINEAR_RATIO / 2)) / VGXY61_HDR_LINEAR_RATIO; > + break; > + case VGXY61_HDR_SUB: > + new_expo_long = max(expo_long_min, new_expo_long); > + > + expo_long_max = vgxy61_get_expo_long_max(sensor, 1); > + /* Short and long are the same in VGXY61_HDR_SUB */ > + expo_short_max = expo_long_max; > + new_expo_short = new_expo_long; > + break; > + case VGXY61_NO_HDR: > + new_expo_long = max(expo_long_min, new_expo_long); > + > + /* > + * As short expo is 0 here, only the second rule of thumb applies, see > + * vgxy61_get_expo_long_max for more > + */ > + expo_long_max = sensor->frame_length - VGXY61_EXPOS_ROT_TERM; > + break; > + default: > + /* Should never happen */ > + WARN_ON(true); > + break; > + } > + > + /* If this happens, something is wrong with formulas */ > + WARN_ON(expo_long_min > expo_long_max); > + > + if (new_expo_long > expo_long_max) { > + dev_warn(&client->dev, "Exposure %d too high, clamping to %d\n", > + new_expo_long, expo_long_max); > + new_expo_long = expo_long_max; > + new_expo_short = expo_short_max; > + } > + > + dev_dbg(&client->dev, "frame_length %d, new_expo_long %d, new_expo_short %d\n", > + sensor->frame_length, new_expo_long, new_expo_short); > + dev_dbg(&client->dev, "expo_long_min %d, expo_long_max %d\n", > + expo_long_min, expo_long_max); > + > + /* Apply exposure */ > + sensor->expo_long = new_expo_long; > + sensor->expo_short = new_expo_short; > + sensor->expo_max = expo_long_max; > + sensor->expo_min = expo_long_min; > + > + if (sensor->streaming) > + return vgxy61_apply_exposure(sensor); > + return 0; > +} > + > +static int vgxy61_apply_framelength(struct vgxy61_dev *sensor) > +{ > + return vgxy61_write_reg(sensor, VGXY61_REG_FRAME_LENGTH, sensor->frame_length, NULL); > +} > + > +static int vgxy61_update_vblank(struct vgxy61_dev *sensor, int vblank, enum vgxy61_hdr_mode hdr) > +{ > + int ret; > + > + sensor->vblank_min = vgxy61_get_vblank_min(sensor, hdr); > + sensor->vblank = max(sensor->vblank_min, vblank); > + sensor->frame_length = sensor->current_mode->crop.height + sensor->vblank; > + > + /* Update exposure according to vblank */ > + ret = vgxy61_update_exposure(sensor, sensor->expo_long, hdr); > + if (ret) > + return ret; > + > + if (sensor->streaming) > + return vgxy61_apply_framelength(sensor); > + return 0; > +} > + > +static int vgxy61_apply_hdr(struct vgxy61_dev *sensor, enum vgxy61_hdr_mode index) > +{ > + static const u8 index2val[] = {0x1, 0x4, 0xa}; > + > + return vgxy61_write_reg(sensor, VGXY61_REG_HDR_CTRL, index2val[index], NULL); > +} > + > +static int vgxy61_update_hdr(struct vgxy61_dev *sensor, enum vgxy61_hdr_mode index) > +{ > + int ret; > + > + /* > + * vblank and short exposure change according to HDR mode, do it first as it can > + * violate sensors 'rule of thumbs' and therefore will require to change the long exposure > + */ > + ret = vgxy61_update_vblank(sensor, sensor->vblank, index); > + if (ret) > + return ret; > + > + /* Update strobe mode according to HDR */ > + ret = vgxy61_update_gpios_strobe_mode(sensor, index); > + if (ret) > + return ret; > + > + sensor->hdr = index; > + > + if (sensor->streaming) > + return vgxy61_apply_hdr(sensor, sensor->hdr); > + return 0; > +} > + > +static int vgxy61_apply_settings(struct vgxy61_dev *sensor) > +{ > + int ret, i; > + > + ret = vgxy61_apply_hdr(sensor, sensor->hdr); > + if (ret) > + return ret; > + > + ret = vgxy61_apply_framelength(sensor); > + if (ret) > + return ret; > + > + ret = vgxy61_apply_exposure(sensor); > + if (ret) > + return ret; > + > + ret = vgxy61_write_reg(sensor, VGXY61_REG_ANALOG_GAIN, sensor->analog_gain, NULL); > + if (ret) > + return ret; > + ret = vgxy61_apply_digital_gain(sensor, sensor->digital_gain); > + if (ret) > + return ret; > + > + ret = vgxy61_write_reg(sensor, VGXY61_REG_ORIENTATION, > + sensor->hflip | (sensor->vflip << 1), NULL); > + if (ret) > + return ret; Instead of doing this, please use v4l2_ctrl_handler_setup(). And check the power state of the sensor in the s_ctrl callback using pm_runtime_get_if_in_use(), not streaming state. > + > + ret = vgxy61_apply_patgen(sensor, sensor->pattern); > + if (ret) > + return ret; > + > + for (i = 0; i < VGXY61_NB_GPIOS; i++) { > + ret = vgxy61_apply_gpiox_strobe_mode(sensor, sensor->strobe_mode, i); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > +static int vgxy61_stream_enable(struct vgxy61_dev *sensor) > +{ > + struct i2c_client *client = v4l2_get_subdevdata(&sensor->sd); > + const struct v4l2_rect *crop = &sensor->current_mode->crop; > + int ret = 0; > + > + ret = vgxy61_check_bw(sensor); > + if (ret) > + return ret; > + > + ret = pm_runtime_get_sync(&client->dev); > + if (ret < 0) { > + pm_runtime_put_noidle(&client->dev); > + return ret; > + } > + > + /* Configure sensor */ > + vgxy61_write_reg(sensor, VGXY61_REG_FORMAT_CTRL, get_bpp_by_code(sensor->fmt.code), &ret); > + vgxy61_write_reg(sensor, VGXY61_REG_OIF_ROI0_CTRL, > + get_data_type_by_code(sensor->fmt.code), &ret); > + > + vgxy61_write_reg(sensor, VGXY61_REG_READOUT_CTRL, sensor->current_mode->bin_mode, &ret); > + vgxy61_write_reg(sensor, VGXY61_REG_ROI0_START_H, crop->left, &ret); > + vgxy61_write_reg(sensor, VGXY61_REG_ROI0_END_H, crop->left + crop->width - 1, &ret); > + vgxy61_write_reg(sensor, VGXY61_REG_ROI0_START_V, crop->top, &ret); > + vgxy61_write_reg(sensor, VGXY61_REG_ROI0_END_V, crop->top + crop->height - 1, &ret); > + if (ret) > + goto err_rpm_put; > + > + /* Apply controls cached values */ > + ret = vgxy61_apply_settings(sensor); > + if (ret) > + goto err_rpm_put; > + > + /* Start streaming */ > + ret = vgxy61_write_reg(sensor, VGXY61_REG_STREAMING, VGXY61_STREAMING_REQ_START, NULL); > + if (ret) > + goto err_rpm_put; > + > + ret = vgxy61_poll_reg(sensor, VGXY61_REG_STREAMING, VGXY61_STREAMING_NO_REQ, > + VGXY61_TIMEOUT_MS); > + if (ret) > + goto err_rpm_put; > + > + ret = vgxy61_wait_state(sensor, VGXY61_SYSTEM_FSM_STREAMING, VGXY61_TIMEOUT_MS); > + if (ret) > + goto err_rpm_put; > + > + /* vflip and hflip cannot change during streaming */ > + __v4l2_ctrl_grab(sensor->vflip_ctrl, true); > + __v4l2_ctrl_grab(sensor->hflip_ctrl, true); > + > + return 0; > + > +err_rpm_put: > + pm_runtime_put(&client->dev); > + return ret; > +} > + > +static int vgxy61_stream_disable(struct vgxy61_dev *sensor) > +{ > + struct i2c_client *client = v4l2_get_subdevdata(&sensor->sd); > + int ret; > + > + ret = vgxy61_write_reg(sensor, VGXY61_REG_STREAMING, VGXY61_STREAMING_REQ_STOP, NULL); > + if (ret) > + return ret; > + > + ret = vgxy61_poll_reg(sensor, VGXY61_REG_STREAMING, VGXY61_STREAMING_NO_REQ, 2000); > + if (ret) > + return ret; > + > + ret = vgxy61_wait_state(sensor, VGXY61_SYSTEM_FSM_SW_STBY, VGXY61_TIMEOUT_MS); > + if (ret) > + return ret; The caller won't be able to do anything reasonable to handle these errors. Just print a warning and proceed to call pm_runtime_put(). > + > + __v4l2_ctrl_grab(sensor->vflip_ctrl, false); > + __v4l2_ctrl_grab(sensor->hflip_ctrl, false); > + > + pm_runtime_put(&client->dev); > + > + return 0; > +} > + > +static int vgxy61_s_stream(struct v4l2_subdev *sd, int enable) > +{ > + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); > + struct i2c_client *client = sensor->i2c_client; > + int ret = 0; > + > + mutex_lock(&sensor->lock); > + dev_dbg(&client->dev, "%s : requested %d / current = %d\n", __func__, enable, > + sensor->streaming); > + > + ret = enable ? vgxy61_stream_enable(sensor) : vgxy61_stream_disable(sensor); > + if (!ret) > + sensor->streaming = enable; > + > + dev_dbg(&client->dev, "%s current now = %d / %d\n", __func__, sensor->streaming, ret); > + mutex_unlock(&sensor->lock); > + > + return ret; > +} > + > +static int vgxy61_set_fmt(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *sd_state, > + struct v4l2_subdev_format *format) > +{ > + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); > + struct i2c_client *client = sensor->i2c_client; > + const struct vgxy61_mode_info *new_mode; > + struct v4l2_mbus_framefmt *fmt; > + int ret; > + > + dev_dbg(&client->dev, "%s probe %d\n", __func__, format->pad); > + dev_dbg(&client->dev, "%s %dx%d\n", __func__, format->format.width, format->format.height); > + > + mutex_lock(&sensor->lock); > + > + if (sensor->streaming) { > + ret = -EBUSY; > + goto out; > + } > + > + /* Find best format */ > + ret = vgxy61_try_fmt_internal(sd, &format->format, &new_mode); > + if (ret) > + goto out; > + > + if (format->which == V4L2_SUBDEV_FORMAT_TRY) { > + fmt = v4l2_subdev_get_try_format(sd, sd_state, 0); > + *fmt = format->format; > + } else if (sensor->current_mode != new_mode || sensor->fmt.code != format->format.code) { > + fmt = &sensor->fmt; > + *fmt = format->format; > + > + sensor->current_mode = new_mode; > + > + /* Reset vblank and framelength to default */ > + ret = vgxy61_update_vblank(sensor, > + VGXY61_FRAME_LENGTH_DEF - > + sensor->current_mode->crop.height, > + sensor->hdr); > + > + /* Update controls to reflect new mode */ > + __v4l2_ctrl_s_ctrl_int64(sensor->pixel_rate_ctrl, get_pixel_rate(sensor)); > + __v4l2_ctrl_modify_range(sensor->vblank_ctrl, sensor->vblank_min, > + 0xffff - sensor->current_mode->crop.height, 1, > + sensor->vblank); > + __v4l2_ctrl_s_ctrl(sensor->vblank_ctrl, sensor->vblank); > + __v4l2_ctrl_modify_range(sensor->expo_ctrl, sensor->expo_min, > + sensor->expo_max, 1, > + sensor->expo_long); > + } > + > +out: > + mutex_unlock(&sensor->lock); > + > + return ret; > +} > + > +static int vgxy61_init_cfg(struct v4l2_subdev *sd, struct v4l2_subdev_state *sd_state) > +{ > + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); > + struct v4l2_subdev_format fmt = { 0 }; > + > + sensor->current_mode = sensor->default_mode; > + vgxy61_fill_framefmt(sensor, sensor->current_mode, &fmt.format, VGXY61_MEDIA_BUS_FMT_DEF); > + > + return vgxy61_set_fmt(sd, sd_state, &fmt); > +} > + > +static int vgxy61_s_ctrl(struct v4l2_ctrl *ctrl) > +{ > + struct v4l2_subdev *sd = ctrl_to_sd(ctrl); > + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); > + int ret; > + > + switch (ctrl->id) { > + case V4L2_CID_EXPOSURE: > + ret = vgxy61_update_exposure(sensor, ctrl->val, sensor->hdr); > + ctrl->val = sensor->expo_long; > + break; > + case V4L2_CID_ANALOGUE_GAIN: > + ret = vgxy61_update_analog_gain(sensor, ctrl->val); > + break; > + case V4L2_CID_DIGITAL_GAIN: > + ret = vgxy61_update_digital_gain(sensor, ctrl->val); > + break; > + case V4L2_CID_VFLIP: > + case V4L2_CID_HFLIP: > + if (sensor->streaming) { > + ret = -EBUSY; > + break; > + } > + if (ctrl->id == V4L2_CID_VFLIP) > + sensor->vflip = ctrl->val; > + if (ctrl->id == V4L2_CID_HFLIP) > + sensor->hflip = ctrl->val; > + ret = 0; > + break; > + case V4L2_CID_TEST_PATTERN: > + ret = vgxy61_update_patgen(sensor, ctrl->val); > + break; > + case V4L2_CID_STVGXY61_HDR: > + ret = vgxy61_update_hdr(sensor, ctrl->val); > + /* Update vblank and exposure controls to match new hdr */ > + __v4l2_ctrl_modify_range(sensor->vblank_ctrl, sensor->vblank_min, > + 0xffff - sensor->current_mode->crop.height, 1, > + sensor->vblank); > + __v4l2_ctrl_modify_range(sensor->expo_ctrl, sensor->expo_min, > + sensor->expo_max, 1, sensor->expo_long); > + break; > + case V4L2_CID_VBLANK: > + ret = vgxy61_update_vblank(sensor, ctrl->val, sensor->hdr); > + /* Update exposure control to match new vblank */ > + __v4l2_ctrl_modify_range(sensor->expo_ctrl, sensor->expo_min, > + sensor->expo_max, 1, sensor->expo_long); > + break; > + default: > + ret = -EINVAL; > + break; > + } > + > + return ret; > +} > + > +static const struct v4l2_ctrl_ops vgxy61_ctrl_ops = { > + .s_ctrl = vgxy61_s_ctrl, > +}; > + > +static const struct v4l2_ctrl_config vgxy61_hdr_ctrl = { > + .ops = &vgxy61_ctrl_ops, > + .id = V4L2_CID_STVGXY61_HDR, > + .name = "HDR mode", > + .type = V4L2_CTRL_TYPE_MENU, > + .min = 0, > + .max = ARRAY_SIZE(vgxy61_hdr_mode_menu) - 1, > + .def = VGXY61_NO_HDR, > + .qmenu = vgxy61_hdr_mode_menu, > +}; > + > +static int vgxy61_init_controls(struct vgxy61_dev *sensor) > +{ > + const struct v4l2_ctrl_ops *ops = &vgxy61_ctrl_ops; > + struct v4l2_ctrl_handler *hdl = &sensor->ctrl_handler; > + struct v4l2_ctrl *ctrl; > + int ret; > + > + v4l2_ctrl_handler_init(hdl, 16); > + /* We can use our own mutex for the ctrl lock */ > + hdl->lock = &sensor->lock; > + /* Standard controls */ > + v4l2_ctrl_new_std(hdl, ops, V4L2_CID_ANALOGUE_GAIN, 0, 0x1c, 1, sensor->analog_gain); > + v4l2_ctrl_new_std(hdl, ops, V4L2_CID_DIGITAL_GAIN, 0, 0xfff, 1, sensor->digital_gain); > + v4l2_ctrl_new_std_menu_items(hdl, ops, V4L2_CID_TEST_PATTERN, > + ARRAY_SIZE(vgxy61_test_pattern_menu) - 1, 0, 0, > + vgxy61_test_pattern_menu); > + ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HBLANK, 0, sensor->line_length, 1, > + sensor->line_length - sensor->current_mode->width); > + ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; Note that ctrl may be NULL here. Same for the rest of similar cases below. You could move these after checking the handler error state. > + ctrl = v4l2_ctrl_new_int_menu(hdl, ops, V4L2_CID_LINK_FREQ, ARRAY_SIZE(link_freq) - 1, 0, > + link_freq); > + ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; > + /* Custom controls */ > + v4l2_ctrl_new_custom(hdl, &vgxy61_hdr_ctrl, NULL); > + /* Keep a pointer to these controls as we need to update them when setting the format */ > + sensor->pixel_rate_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_PIXEL_RATE, 1, INT_MAX, 1, > + get_pixel_rate(sensor)); > + sensor->pixel_rate_ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; > + sensor->expo_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_EXPOSURE, sensor->expo_min, > + sensor->expo_max, 1, sensor->expo_long); > + sensor->vblank_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VBLANK, sensor->vblank_min, > + 0xffff - sensor->current_mode->crop.height, 1, > + sensor->vblank); > + sensor->vflip_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VFLIP, 0, 1, 1, sensor->vflip); > + sensor->hflip_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HFLIP, 0, 1, 1, sensor->hflip); You don't seem to have the link frequency control. I suppose you should, as well as have it in DT. <URL:https://hverkuil.home.xs4all.nl/spec/driver-api/camera-sensor.html#handling-clocks> > + > + if (hdl->error) { > + ret = hdl->error; > + goto free_ctrls; > + } > + > + sensor->sd.ctrl_handler = hdl; > + return 0; > + > +free_ctrls: > + v4l2_ctrl_handler_free(hdl); > + return ret; > +} > + > +static const struct v4l2_subdev_video_ops vgxy61_video_ops = { > + .s_stream = vgxy61_s_stream, > +}; > + > +static const struct v4l2_subdev_pad_ops vgxy61_pad_ops = { > + .init_cfg = vgxy61_init_cfg, > + .enum_mbus_code = vgxy61_enum_mbus_code, > + .get_fmt = vgxy61_get_fmt, > + .set_fmt = vgxy61_set_fmt, > + .get_selection = vgxy61_get_selection, > + .enum_frame_size = vgxy61_enum_frame_size, > +}; > + > +static const struct v4l2_subdev_ops vgxy61_subdev_ops = { > + .video = &vgxy61_video_ops, > + .pad = &vgxy61_pad_ops, > +}; > + > +static const struct media_entity_operations vgxy61_subdev_entity_ops = { > + .link_validate = v4l2_subdev_link_validate, > +}; > + > +/* Set phy polarities */ > +static int vgxy61_tx_from_ep(struct vgxy61_dev *sensor, struct fwnode_handle *endpoint) > +{ > + struct v4l2_fwnode_endpoint ep = { .bus_type = V4L2_MBUS_CSI2_DPHY }; > + struct i2c_client *client = sensor->i2c_client; > + u32 log2phy[VGXY61_NB_POLARITIES] = {~0, ~0, ~0, ~0, ~0}; > + u32 phy2log[VGXY61_NB_POLARITIES] = {~0, ~0, ~0, ~0, ~0}; > + int polarities[VGXY61_NB_POLARITIES] = {0, 0, 0, 0, 0}; > + int l_nb; > + int p, l; > + int ret; > + int i; > + > + ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &ep); > + if (ret) > + goto error_alloc; Just return ret here, no need for a label. > + > + l_nb = ep.bus.mipi_csi2.num_data_lanes; > + if (l_nb != 1 && l_nb != 2 && l_nb != 4) { > + dev_err(&client->dev, "invalid data lane number %d\n", l_nb); > + goto error_ep; > + } > + > + /* Build log2phy, phy2log and polarities from ep info */ > + log2phy[0] = ep.bus.mipi_csi2.clock_lane; > + phy2log[log2phy[0]] = 0; > + for (l = 1; l < l_nb + 1; l++) { Please use unsigned type for array indices and other values that are obviously not going to be negative. > + log2phy[l] = ep.bus.mipi_csi2.data_lanes[l - 1]; > + phy2log[log2phy[l]] = l; > + } > + /* > + * Then fill remaining slots for every physical slot to have something valid for hardware > + * stuff. > + */ > + for (p = 0; p < VGXY61_NB_POLARITIES; p++) { > + if (phy2log[p] != ~0) > + continue; > + phy2log[p] = l; > + log2phy[l] = p; > + l++; > + } > + for (l = 0; l < l_nb + 1; l++) > + polarities[l] = ep.bus.mipi_csi2.lane_polarities[l]; > + > + if (log2phy[0] != 0) { > + dev_err(&client->dev, "clk lane must be map to physical lane 0\n"); > + goto error_ep; > + } > + sensor->oif_ctrl = (polarities[4] << 15) + ((phy2log[4] - 1) << 13) + > + (polarities[3] << 12) + ((phy2log[3] - 1) << 10) + > + (polarities[2] << 9) + ((phy2log[2] - 1) << 7) + > + (polarities[1] << 6) + ((phy2log[1] - 1) << 4) + > + (polarities[0] << 3) + > + l_nb; > + sensor->nb_of_lane = l_nb; > + > + dev_dbg(&client->dev, "tx uses %d lanes", l_nb); > + for (i = 0; i < 5; i++) { > + dev_dbg(&client->dev, "log2phy[%d] = %d\n", i, log2phy[i]); > + dev_dbg(&client->dev, "phy2log[%d] = %d\n", i, phy2log[i]); > + dev_dbg(&client->dev, "polarity[%d] = %d\n", i, polarities[i]); > + } > + dev_dbg(&client->dev, "oif_ctrl = 0x%04x\n", sensor->oif_ctrl); > + > + v4l2_fwnode_endpoint_free(&ep); > + > + return 0; > + > +error_ep: > + v4l2_fwnode_endpoint_free(&ep); > +error_alloc: > + > + return -EINVAL; > +} > + > +static int vgxy61_configure(struct vgxy61_dev *sensor) > +{ > + struct i2c_client *client = sensor->i2c_client; > + int sensor_freq; > + unsigned int prediv; > + unsigned int mult; > + int line_length; > + int ret = 0; > + > + compute_pll_parameters_by_freq(sensor->clk_freq, &prediv, &mult); > + sensor_freq = (mult * sensor->clk_freq) / prediv; > + /* Frequency to data rate is 1:1 ratio for MIPI */ > + sensor->data_rate_in_mbps = sensor_freq; > + /* Video timing ISP path (pixel clock) requires 804/5 mhz = 160 mhz */ > + sensor->pclk = sensor_freq / 5; > + > + /* Cache line_length value */ > + line_length = vgxy61_read_reg(sensor, VGXY61_REG_LINE_LENGTH); > + if (line_length < 0) What does line_length < 0 actually signify? An error? > + return ret; > + sensor->line_length = line_length; > + /* Configure clocks */ > + vgxy61_write_reg(sensor, VGXY61_REG_EXT_CLOCK, sensor->clk_freq, &ret); > + vgxy61_write_reg(sensor, VGXY61_REG_CLK_PLL_PREDIV, prediv, &ret); > + vgxy61_write_reg(sensor, VGXY61_REG_CLK_SYS_PLL_MULT, mult, &ret); > + /* Configure interface */ > + vgxy61_write_reg(sensor, VGXY61_REG_OIF_CTRL, sensor->oif_ctrl, &ret); > + /* Disable pwm compression */ > + vgxy61_write_reg(sensor, VGXY61_REG_FRAME_CONTENT_CTRL, 0, &ret); > + /* Disable asil lines */ > + vgxy61_write_reg(sensor, VGXY61_REG_BYPASS_CTRL, 4, &ret); > + if (ret) > + return ret; > + /* Set gpios polarity according to device tree value */ > + vgxy61_update_gpios_strobe_polarity(sensor, sensor->gpios_polarity); > + /* Slave mode */ > + ret = vgxy61_write_reg(sensor, VGXY61_REG_VT_CTRL, sensor->slave_mode, &ret); > + /* Set pattern generator solid to middle value */ > + vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_LONG_DATA_GR, 0x800, &ret); > + vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_LONG_DATA_R, 0x800, &ret); > + vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_LONG_DATA_B, 0x800, &ret); > + vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_LONG_DATA_GB, 0x800, &ret); > + vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_SHORT_DATA_GR, 0x800, &ret); > + vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_SHORT_DATA_R, 0x800, &ret); > + vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_SHORT_DATA_B, 0x800, &ret); > + vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_SHORT_DATA_GB, 0x800, &ret); > + if (ret) > + return ret; > + > + dev_dbg(&client->dev, "clock prediv = %d\n", prediv); > + dev_dbg(&client->dev, "clock mult = %d\n", mult); > + dev_dbg(&client->dev, "data rate = %d mbps\n", sensor->data_rate_in_mbps); > + > + return 0; > +} > + > +static int vgxy61_patch(struct vgxy61_dev *sensor) > +{ > + struct i2c_client *client = sensor->i2c_client; > + u16 patch; > + int ret; > + > + ret = vgxy61_write_array(sensor, VGXY61_REG_FWPATCH_START_ADDR, sizeof(patch_array), > + patch_array); > + if (ret) > + return ret; > + > + ret = vgxy61_write_reg(sensor, VGXY61_REG_STBY, 0x10, NULL); > + if (ret) > + return ret; > + > + ret = vgxy61_poll_reg(sensor, VGXY61_REG_STBY, 0, VGXY61_TIMEOUT_MS); > + if (ret) > + return ret; > + > + patch = vgxy61_read_reg(sensor, VGXY61_REG_FWPATCH_REVISION); > + if (patch < 0) > + return patch; > + > + if (patch != (VGXY61_FWPATCH_REVISION_MAJOR << 12) + > + (VGXY61_FWPATCH_REVISION_MINOR << 8) + > + VGXY61_FWPATCH_REVISION_MICRO) { > + dev_err(&client->dev, "bad patch version expected %d.%d.%d got %d.%d.%d\n", > + VGXY61_FWPATCH_REVISION_MAJOR, > + VGXY61_FWPATCH_REVISION_MINOR, > + VGXY61_FWPATCH_REVISION_MICRO, > + patch >> 12, (patch >> 8) & 0x0f, patch & 0xff); > + return -ENODEV; > + } > + dev_dbg(&client->dev, "patch %d.%d.%d applied\n", > + patch >> 12, (patch >> 8) & 0x0f, patch & 0xff); > + > + return 0; > +} > + > +static int vgxy61_detect_cut_version(struct vgxy61_dev *sensor) > +{ > + struct i2c_client *client = sensor->i2c_client; > + u16 device_rev; > + > + device_rev = vgxy61_read_reg(sensor, VGXY61_REG_REVISION); > + if (device_rev < 0) > + return device_rev; > + > + switch (device_rev >> 8) { > + case 0xA: > + dev_dbg(&client->dev, "Cut1 detected\n"); > + dev_err(&client->dev, "Cut1 not supported by this driver\n"); > + return -ENODEV; > + case 0xB: > + dev_dbg(&client->dev, "Cut2 detected\n"); > + return 0; > + case 0xC: > + dev_dbg(&client->dev, "Cut3 detected\n"); > + return 0; > + default: > + dev_err(&client->dev, "Unable to detect cut version\n"); > + return -ENODEV; > + } > +} > + > +static int vgxy61_detect(struct vgxy61_dev *sensor) > +{ > + struct i2c_client *client = sensor->i2c_client; > + u16 id = 0; > + int ret; > + u8 st; > + > + id = vgxy61_read_reg(sensor, VGXY61_REG_MODEL_ID); > + if (id < 0) > + return id; > + if (id != VG5661_MODEL_ID && id != VG5761_MODEL_ID) { > + dev_warn(&client->dev, "Unsupported sensor id %x\n", id); > + return -ENODEV; > + } > + dev_dbg(&client->dev, "detected sensor id = 0x%04x\n", id); > + sensor->id = id; > + > + ret = vgxy61_wait_state(sensor, VGXY61_SYSTEM_FSM_SW_STBY, VGXY61_TIMEOUT_MS); > + if (ret) > + return ret; > + > + st = vgxy61_read_reg(sensor, VGXY61_REG_NVM); > + if (st < 0) > + return st; > + if (st != VGXY61_NVM_OK) > + dev_warn(&client->dev, "Bad nvm state got %d\n", st); > + > + /* Detect cut version */ > + ret = vgxy61_detect_cut_version(sensor); > + if (ret) > + return ret; > + > + return 0; > +} > + > +/* Power/clock management functions */ > +static int vgxy61_power_on(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct v4l2_subdev *sd = i2c_get_clientdata(client); > + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); > + int ret; > + > + ret = vgxy61_get_regulators(sensor); This should take place during probe only. > + if (ret) { > + dev_err(&client->dev, "failed to get regulators %d\n", ret); > + return ret; > + } > + > + ret = regulator_bulk_enable(VGXY61_NUM_SUPPLIES, sensor->supplies); > + if (ret) { > + dev_err(&client->dev, "failed to enable regulators %d\n", ret); > + return ret; > + } > + > + ret = clk_prepare_enable(sensor->xclk); > + if (ret) { > + dev_err(&client->dev, "failed to enable clock %d\n", ret); > + goto disable_bulk; > + } > + > + /* Apply reset sequence */ > + if (sensor->reset_gpio) { > + ret = vgxy61_apply_reset(sensor); > + if (ret) { > + dev_err(&client->dev, "sensor reset failed %d\n", ret); > + goto disable_clock; > + } > + } > + > + ret = vgxy61_patch(sensor); > + if (ret) { > + dev_err(&client->dev, "sensor patch failed %d\n", ret); > + goto disable_clock; > + } > + > + ret = vgxy61_configure(sensor); > + if (ret) { > + dev_err(&client->dev, "sensor configuration failed %d\n", ret); > + goto disable_clock; > + } > + > + return 0; > + > +disable_clock: > + clk_disable_unprepare(sensor->xclk); > +disable_bulk: > + regulator_bulk_disable(VGXY61_NUM_SUPPLIES, sensor->supplies); > + > + return ret; > +} > + > +static int vgxy61_power_off(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct v4l2_subdev *sd = i2c_get_clientdata(client); > + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); > + > + clk_disable_unprepare(sensor->xclk); > + regulator_bulk_disable(VGXY61_NUM_SUPPLIES, sensor->supplies); > + return 0; > +} > + > +static void vgxy61_fill_sensor_param(struct vgxy61_dev *sensor) > +{ > + if (sensor->id == VG5761_MODEL_ID) { > + sensor->sensor_width = VGX761_WIDTH; > + sensor->sensor_height = VGX761_HEIGHT; > + sensor->sensor_modes = vgx761_mode_data; > + sensor->sensor_modes_nb = ARRAY_SIZE(vgx761_mode_data); > + sensor->default_mode = &vgx761_mode_data[VGX761_DEFAULT_MODE]; > + sensor->rot_term = VGX761_SHORT_ROT_TERM; > + } else if (sensor->id == VG5661_MODEL_ID) { > + sensor->sensor_width = VGX661_WIDTH; > + sensor->sensor_height = VGX661_HEIGHT; > + sensor->sensor_modes = vgx661_mode_data; > + sensor->sensor_modes_nb = ARRAY_SIZE(vgx661_mode_data); > + sensor->default_mode = &vgx661_mode_data[VGX661_DEFAULT_MODE]; > + sensor->rot_term = VGX661_SHORT_ROT_TERM; > + } else { > + /* Should never happen */ > + WARN_ON(true); > + } > + sensor->current_mode = sensor->default_mode; > + sensor->vblank = sensor->frame_length - sensor->current_mode->crop.height; > +} > + > +static int vgxy61_probe(struct i2c_client *client) > +{ > + struct device *dev = &client->dev; > + struct fwnode_handle *endpoint; > + struct vgxy61_dev *sensor; > + int ret; > + > + sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL); > + if (!sensor) > + return -ENOMEM; > + > + sensor->i2c_client = client; > + sensor->streaming = false; > + sensor->hdr = VGXY61_NO_HDR; > + sensor->expo_long = 200; > + sensor->expo_short = 0; > + sensor->hflip = false; > + sensor->vflip = false; > + sensor->analog_gain = 0; > + sensor->digital_gain = 256; > + > + endpoint = fwnode_graph_get_next_endpoint(of_fwnode_handle(dev->of_node), NULL); > + if (!endpoint) { > + dev_err(dev, "endpoint node not found\n"); > + return -EINVAL; > + } > + > + ret = vgxy61_tx_from_ep(sensor, endpoint); > + fwnode_handle_put(endpoint); > + if (ret) { > + dev_err(dev, "Failed to parse endpoint %d\n", ret); > + return ret; > + } > + > + sensor->xclk = devm_clk_get(dev, NULL); > + if (IS_ERR(sensor->xclk)) { > + dev_err(dev, "failed to get xclk\n"); > + return PTR_ERR(sensor->xclk); > + } > + sensor->clk_freq = clk_get_rate(sensor->xclk); > + if (sensor->clk_freq < 6 * HZ_PER_MHZ || sensor->clk_freq > 27 * HZ_PER_MHZ) { > + dev_err(dev, "Only 6Mhz-27Mhz clock range supported. provide %lu MHz\n", > + sensor->clk_freq / HZ_PER_MHZ); > + return -EINVAL; > + } > + sensor->gpios_polarity = of_property_read_bool(dev->of_node, "invert-gpios-polarity"); > + sensor->slave_mode = of_property_read_bool(dev->of_node, "slave-mode"); What does this actually do? On parallel bus the slave mode tells the synchronisation signals originate from the receiver. On CSI-2 there are no specific synchronisation signals. > + > + v4l2_i2c_subdev_init(&sensor->sd, client, &vgxy61_subdev_ops); > + sensor->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > + sensor->pad.flags = MEDIA_PAD_FL_SOURCE; > + sensor->sd.entity.ops = &vgxy61_subdev_entity_ops; > + sensor->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR; > + > + /* Request optional reset pin */ > + sensor->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); > + > + ret = vgxy61_power_on(dev); > + if (ret) > + return ret; > + > + ret = vgxy61_detect(sensor); > + if (ret) { > + dev_err(&client->dev, "sensor detect failed %d\n", ret); > + return ret; > + } > + > + vgxy61_fill_sensor_param(sensor); > + vgxy61_fill_framefmt(sensor, sensor->current_mode, &sensor->fmt, VGXY61_MEDIA_BUS_FMT_DEF); > + > + /* HDR mode */ > + ret = vgxy61_update_hdr(sensor, sensor->hdr); > + if (ret) > + return ret; > + > + mutex_init(&sensor->lock); > + > + ret = vgxy61_init_controls(sensor); > + if (ret) { > + dev_err(&client->dev, "controls initialization failed %d\n", ret); > + goto error_power_off; > + } > + > + ret = media_entity_pads_init(&sensor->sd.entity, 1, &sensor->pad); > + if (ret) { > + dev_err(&client->dev, "pads init failed %d\n", ret); > + goto error_handler_free; > + } > + > + /* Enable runtime PM and turn off the device */ > + pm_runtime_set_active(dev); > + pm_runtime_enable(dev); > + pm_runtime_idle(dev); > + > + ret = v4l2_async_register_subdev(&sensor->sd); > + if (ret) { > + dev_err(&client->dev, "async subdev register failed %d\n", ret); > + goto error_pm_runtime; > + } > + > + pm_runtime_set_autosuspend_delay(&client->dev, 1000); > + pm_runtime_use_autosuspend(&client->dev); > + pm_runtime_put_autosuspend(&client->dev); > + > + dev_dbg(&client->dev, "vgxy61 probe successfully\n"); > + > + return 0; > + > +error_pm_runtime: > + pm_runtime_disable(&client->dev); > + pm_runtime_set_suspended(&client->dev); > + media_entity_cleanup(&sensor->sd.entity); > +error_handler_free: > + v4l2_ctrl_handler_free(sensor->sd.ctrl_handler); > + mutex_destroy(&sensor->lock); > +error_power_off: > + vgxy61_power_off(dev); > + > + return ret; > +} > + > +static int vgxy61_remove(struct i2c_client *client) > +{ > + struct v4l2_subdev *sd = i2c_get_clientdata(client); > + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); > + > + v4l2_async_unregister_subdev(&sensor->sd); > + mutex_destroy(&sensor->lock); > + media_entity_cleanup(&sensor->sd.entity); > + > + pm_runtime_disable(&client->dev); > + if (!pm_runtime_status_suspended(&client->dev)) > + vgxy61_power_off(&client->dev); > + pm_runtime_set_suspended(&client->dev); > + > + return 0; > +} > + > +static const struct of_device_id vgxy61_dt_ids[] = { > + { .compatible = "st,st-vgxy61" }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, vgxy61_dt_ids); > + > +static const struct dev_pm_ops vgxy61_pm_ops = { > + SET_RUNTIME_PM_OPS(vgxy61_power_off, vgxy61_power_on, NULL) > +}; > + > +static struct i2c_driver vgxy61_i2c_driver = { > + .driver = { > + .name = "st-vgxy61", > + .of_match_table = vgxy61_dt_ids, > + .pm = &vgxy61_pm_ops, > + }, > + .probe_new = vgxy61_probe, > + .remove = vgxy61_remove, > +}; > + > +module_i2c_driver(vgxy61_i2c_driver); > + > +MODULE_AUTHOR("Benjamin Mugnier <benjamin.mugnier@foss.st.com>"); > +MODULE_AUTHOR("Mickael Guene <mickael.guene@st.com>"); > +MODULE_AUTHOR("Sylvain Petinot <sylvain.petinot@foss.st.com>"); > +MODULE_DESCRIPTION("VGXY61 camera subdev driver"); > +MODULE_LICENSE("GPL");
Hi Sakari, Thank you for your review. Please consider everything not commented as queued for v4. On 8/22/22 12:04, Sakari Ailus wrote: > Hi Benjamin, > > A few comments below... > > An my apologies for taking so long to review this set. No worries :) > > On Thu, May 12, 2022 at 09:40:37AM +0200, Benjamin Mugnier wrote: >> The VGXY61 has a quad lanes CSI-2 output port running at 800mbps per >> lane, and supports RAW8, RAW10, RAW12, RAW14 and RAW16 formats. >> The driver handles both sensor types: >> - VG5661 and VG6661: 1.6 Mpx (1464 x 1104) 75fps. >> - VG5761 and VG6761: 2.3 Mpx (1944 x 1204) 60 fps. >> The driver supports: >> - HDR linearize mode, HDR substraction mode, and no HDR >> - GPIOs LEDs strobing >> - Digital binning and analog subsampling >> - Horizontal and vertical flip >> - Manual exposure >> - Analog and digital gains >> - Test patterns >> >> Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com> >> --- >> drivers/media/i2c/Kconfig | 10 + >> drivers/media/i2c/Makefile | 1 + >> drivers/media/i2c/st-vgxy61.c | 1940 +++++++++++++++++++++++++++++++++ >> 3 files changed, 1951 insertions(+) >> create mode 100644 drivers/media/i2c/st-vgxy61.c >> >> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig >> index fae2baabb773..ee7e8e921783 100644 >> --- a/drivers/media/i2c/Kconfig >> +++ b/drivers/media/i2c/Kconfig >> @@ -753,6 +753,16 @@ config VIDEO_SR030PC30 >> help >> This driver supports SR030PC30 VGA camera from Siliconfile >> >> +config VIDEO_ST_VGXY61 >> + tristate "ST VGXY61 sensor support" >> + depends on OF && GPIOLIB && VIDEO_DEV && I2C >> + select MEDIA_CONTROLLER >> + select VIDEO_V4L2_SUBDEV_API >> + select V4L2_FWNODE >> + help >> + This is a Video4Linux2 sensor driver for the ST VGXY61 >> + camera sensor. >> + >> config VIDEO_VS6624 >> tristate "ST VS6624 sensor support" >> depends on VIDEO_DEV && I2C >> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile >> index 3e1696963e7f..7f2362228484 100644 >> --- a/drivers/media/i2c/Makefile >> +++ b/drivers/media/i2c/Makefile >> @@ -116,6 +116,7 @@ obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o >> obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o >> obj-$(CONFIG_VIDEO_SR030PC30) += sr030pc30.o >> obj-$(CONFIG_VIDEO_ST_MIPID02) += st-mipid02.o >> +obj-$(CONFIG_VIDEO_ST_VGXY61) += st-vgxy61.o >> obj-$(CONFIG_VIDEO_TC358743) += tc358743.o >> obj-$(CONFIG_VIDEO_TDA1997X) += tda1997x.o >> obj-$(CONFIG_VIDEO_TDA7432) += tda7432.o >> diff --git a/drivers/media/i2c/st-vgxy61.c b/drivers/media/i2c/st-vgxy61.c >> new file mode 100644 >> index 000000000000..bad8f10f88fe >> --- /dev/null >> +++ b/drivers/media/i2c/st-vgxy61.c >> @@ -0,0 +1,1940 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Driver for VGXY61 global shutter sensor family driver >> + * >> + * Copyright (C) 2022 STMicroelectronics SA >> + */ >> + >> +#include <linux/clk.h> >> +#include <linux/delay.h> >> +#include <linux/gpio/consumer.h> >> +#include <linux/i2c.h> >> +#include <linux/iopoll.h> >> +#include <linux/module.h> >> +#include <linux/pm_runtime.h> >> +#include <linux/regulator/consumer.h> >> +#include <linux/units.h> >> +#include <media/mipi-csi2.h> >> +#include <media/v4l2-async.h> >> +#include <media/v4l2-ctrls.h> >> +#include <media/v4l2-device.h> >> +#include <media/v4l2-fwnode.h> >> +#include <media/v4l2-subdev.h> >> +#include <uapi/linux/st-vgxy61.h> >> + >> +#define VGXY61_REG_8BIT(n) ((1 << 16) | (n)) >> +#define VGXY61_REG_16BIT(n) ((2 << 16) | (n)) >> +#define VGXY61_REG_32BIT(n) ((4 << 16) | (n)) >> +#define VGXY61_REG_SIZE_SHIFT 16 >> +#define VGXY61_REG_ADDR_MASK 0xffff >> + >> +#define VGXY61_REG_MODEL_ID VGXY61_REG_16BIT(0x0000) >> +#define VG5661_MODEL_ID 0x5661 >> +#define VG5761_MODEL_ID 0x5761 >> +#define VGXY61_REG_REVISION VGXY61_REG_16BIT(0x0002) >> +#define VGXY61_REG_FWPATCH_REVISION VGXY61_REG_16BIT(0x0014) >> +#define VGXY61_REG_FWPATCH_START_ADDR VGXY61_REG_8BIT(0x2000) >> +#define VGXY61_REG_SYSTEM_FSM VGXY61_REG_8BIT(0x0020) >> +#define VGXY61_SYSTEM_FSM_SW_STBY 0x03 >> +#define VGXY61_SYSTEM_FSM_STREAMING 0x04 >> +#define VGXY61_REG_NVM VGXY61_REG_8BIT(0x0023) >> +#define VGXY61_NVM_OK 0x04 >> +#define VGXY61_REG_STBY VGXY61_REG_8BIT(0x0201) >> +#define VGXY61_STBY_NO_REQ 0 >> +#define VGXY61_STBY_REQ_TMP_READ BIT(2) >> +#define VGXY61_REG_STREAMING VGXY61_REG_8BIT(0x0202) >> +#define VGXY61_STREAMING_NO_REQ 0 >> +#define VGXY61_STREAMING_REQ_STOP BIT(0) >> +#define VGXY61_STREAMING_REQ_START BIT(1) >> +#define VGXY61_REG_EXT_CLOCK VGXY61_REG_32BIT(0x0220) >> +#define VGXY61_REG_CLK_PLL_PREDIV VGXY61_REG_8BIT(0x0224) >> +#define VGXY61_REG_CLK_SYS_PLL_MULT VGXY61_REG_8BIT(0x0225) >> +#define VGXY61_REG_GPIO_0_CTRL VGXY61_REG_8BIT(0x0236) >> +#define VGXY61_REG_GPIO_1_CTRL VGXY61_REG_8BIT(0x0237) >> +#define VGXY61_REG_GPIO_2_CTRL VGXY61_REG_8BIT(0x0238) >> +#define VGXY61_REG_GPIO_3_CTRL VGXY61_REG_8BIT(0x0239) >> +#define VGXY61_REG_SIGNALS_POLARITY_CTRL VGXY61_REG_8BIT(0x023b) >> +#define VGXY61_REG_LINE_LENGTH VGXY61_REG_16BIT(0x0300) >> +#define VGXY61_REG_ORIENTATION VGXY61_REG_8BIT(0x0302) >> +#define VGXY61_REG_VT_CTRL VGXY61_REG_8BIT(0x0304) >> +#define VGXY61_REG_FORMAT_CTRL VGXY61_REG_8BIT(0x0305) >> +#define VGXY61_REG_OIF_CTRL VGXY61_REG_16BIT(0x0306) >> +#define VGXY61_REG_OIF_ROI0_CTRL VGXY61_REG_8BIT(0x030a) >> +#define VGXY61_REG_ROI0_START_H VGXY61_REG_16BIT(0x0400) >> +#define VGXY61_REG_ROI0_START_V VGXY61_REG_16BIT(0x0402) >> +#define VGXY61_REG_ROI0_END_H VGXY61_REG_16BIT(0x0404) >> +#define VGXY61_REG_ROI0_END_V VGXY61_REG_16BIT(0x0406) >> +#define VGXY61_REG_PATGEN_CTRL VGXY61_REG_32BIT(0x0440) >> +#define VGXY61_PATGEN_LONG_ENABLE BIT(16) >> +#define VGXY61_PATGEN_SHORT_ENABLE BIT(0) >> +#define VGXY61_PATGEN_LONG_TYPE_SHIFT 18 >> +#define VGXY61_PATGEN_SHORT_TYPE_SHIFT 4 >> +#define VGXY61_REG_FRAME_CONTENT_CTRL VGXY61_REG_8BIT(0x0478) >> +#define VGXY61_REG_COARSE_EXPOSURE_LONG VGXY61_REG_16BIT(0x0500) >> +#define VGXY61_REG_COARSE_EXPOSURE_SHORT VGXY61_REG_16BIT(0x0504) >> +#define VGXY61_REG_ANALOG_GAIN VGXY61_REG_8BIT(0x0508) >> +#define VGXY61_REG_DIGITAL_GAIN_LONG VGXY61_REG_16BIT(0x050a) >> +#define VGXY61_REG_DIGITAL_GAIN_SHORT VGXY61_REG_16BIT(0x0512) >> +#define VGXY61_REG_FRAME_LENGTH VGXY61_REG_16BIT(0x051a) >> +#define VGXY61_REG_SIGNALS_CTRL VGXY61_REG_16BIT(0x0522) >> +#define VGXY61_SIGNALS_GPIO_ID_SHIFT 4 >> +#define VGXY61_REG_READOUT_CTRL VGXY61_REG_8BIT(0x0530) >> +#define VGXY61_REG_HDR_CTRL VGXY61_REG_8BIT(0x0532) >> +#define VGXY61_REG_PATGEN_LONG_DATA_GR VGXY61_REG_16BIT(0x092c) >> +#define VGXY61_REG_PATGEN_LONG_DATA_R VGXY61_REG_16BIT(0x092e) >> +#define VGXY61_REG_PATGEN_LONG_DATA_B VGXY61_REG_16BIT(0x0930) >> +#define VGXY61_REG_PATGEN_LONG_DATA_GB VGXY61_REG_16BIT(0x0932) >> +#define VGXY61_REG_PATGEN_SHORT_DATA_GR VGXY61_REG_16BIT(0x0950) >> +#define VGXY61_REG_PATGEN_SHORT_DATA_R VGXY61_REG_16BIT(0x0952) >> +#define VGXY61_REG_PATGEN_SHORT_DATA_B VGXY61_REG_16BIT(0x0954) >> +#define VGXY61_REG_PATGEN_SHORT_DATA_GB VGXY61_REG_16BIT(0x0956) >> +#define VGXY61_REG_BYPASS_CTRL VGXY61_REG_8BIT(0x0a60) >> + >> +#define VGX661_WIDTH 1464 >> +#define VGX661_HEIGHT 1104 >> +#define VGX761_WIDTH 1944 >> +#define VGX761_HEIGHT 1204 >> +#define VGX661_DEFAULT_MODE 1 >> +#define VGX761_DEFAULT_MODE 1 >> +#define VGX661_SHORT_ROT_TERM 93 >> +#define VGX761_SHORT_ROT_TERM 90 >> +#define VGXY61_EXPOS_ROT_TERM 66 >> +#define VGXY61_WRITE_MULTIPLE_CHUNK_MAX 16 >> +#define VGXY61_NB_GPIOS 4 >> +#define VGXY61_NB_POLARITIES 5 >> +#define VGXY61_FRAME_LENGTH_DEF 1313 >> +#define VGXY61_MIN_FRAME_LENGTH 1288 >> +#define VGXY61_MIN_EXPOSURE 10 >> +#define VGXY61_HDR_LINEAR_RATIO 10 >> +#define VGXY61_TIMEOUT_MS 500 >> +#define VGXY61_MEDIA_BUS_FMT_DEF MEDIA_BUS_FMT_Y8_1X8 >> + >> +#define VGXY61_FWPATCH_REVISION_MAJOR 2 >> +#define VGXY61_FWPATCH_REVISION_MINOR 0 >> +#define VGXY61_FWPATCH_REVISION_MICRO 5 >> + >> +static const u8 patch_array[] = { >> + 0xbf, 0x00, 0x05, 0x20, 0x06, 0x01, 0xe0, 0xe0, 0x04, 0x80, 0xe6, 0x45, >> + 0xed, 0x6f, 0xfe, 0xff, 0x14, 0x80, 0x1f, 0x84, 0x10, 0x42, 0x05, 0x7c, >> + 0x01, 0xc4, 0x1e, 0x80, 0xb6, 0x42, 0x00, 0xe0, 0x1e, 0x82, 0x1e, 0xc0, >> + 0x93, 0xdd, 0xc3, 0xc1, 0x0c, 0x04, 0x00, 0xfa, 0x86, 0x0d, 0x70, 0xe1, >> + 0x04, 0x98, 0x15, 0x00, 0x28, 0xe0, 0x14, 0x02, 0x08, 0xfc, 0x15, 0x40, >> + 0x28, 0xe0, 0x98, 0x58, 0xe0, 0xef, 0x04, 0x98, 0x0e, 0x04, 0x00, 0xf0, >> + 0x15, 0x00, 0x28, 0xe0, 0x19, 0xc8, 0x15, 0x40, 0x28, 0xe0, 0xc6, 0x41, >> + 0xfc, 0xe0, 0x14, 0x80, 0x1f, 0x84, 0x14, 0x02, 0xa0, 0xfc, 0x1e, 0x80, >> + 0x14, 0x80, 0x14, 0x02, 0x80, 0xfb, 0x14, 0x02, 0xe0, 0xfc, 0x1e, 0x80, >> + 0x14, 0xc0, 0x1f, 0x84, 0x14, 0x02, 0xa4, 0xfc, 0x1e, 0xc0, 0x14, 0xc0, >> + 0x14, 0x02, 0x80, 0xfb, 0x14, 0x02, 0xe4, 0xfc, 0x1e, 0xc0, 0x0c, 0x0c, >> + 0x00, 0xf2, 0x93, 0xdd, 0x86, 0x00, 0xf8, 0xe0, 0x04, 0x80, 0xc6, 0x03, >> + 0x70, 0xe1, 0x0e, 0x84, 0x93, 0xdd, 0xc3, 0xc1, 0x0c, 0x04, 0x00, 0xfa, >> + 0x6b, 0x80, 0x06, 0x40, 0x6c, 0xe1, 0x04, 0x80, 0x09, 0x00, 0xe0, 0xe0, >> + 0x0b, 0xa1, 0x95, 0x84, 0x05, 0x0c, 0x1c, 0xe0, 0x86, 0x02, 0xf9, 0x60, >> + 0xe0, 0xcf, 0x78, 0x6e, 0x80, 0xef, 0x25, 0x0c, 0x18, 0xe0, 0x05, 0x4c, >> + 0x1c, 0xe0, 0x86, 0x02, 0xf9, 0x60, 0xe0, 0xcf, 0x0b, 0x84, 0xd8, 0x6d, >> + 0x80, 0xef, 0x05, 0x4c, 0x18, 0xe0, 0x04, 0xd8, 0x0b, 0xa5, 0x95, 0x84, >> + 0x05, 0x0c, 0x2c, 0xe0, 0x06, 0x02, 0x01, 0x60, 0xe0, 0xce, 0x18, 0x6d, >> + 0x80, 0xef, 0x25, 0x0c, 0x30, 0xe0, 0x05, 0x4c, 0x2c, 0xe0, 0x06, 0x02, >> + 0x01, 0x60, 0xe0, 0xce, 0x0b, 0x84, 0x78, 0x6c, 0x80, 0xef, 0x05, 0x4c, >> + 0x30, 0xe0, 0x0c, 0x0c, 0x00, 0xf2, 0x93, 0xdd, 0x46, 0x01, 0x70, 0xe1, >> + 0x08, 0x80, 0x0b, 0xa1, 0x08, 0x5c, 0x00, 0xda, 0x06, 0x01, 0x68, 0xe1, >> + 0x04, 0x80, 0x4a, 0x40, 0x84, 0xe0, 0x08, 0x5c, 0x00, 0x9a, 0x06, 0x01, >> + 0xe0, 0xe0, 0x04, 0x80, 0x15, 0x00, 0x60, 0xe0, 0x19, 0xc4, 0x15, 0x40, >> + 0x60, 0xe0, 0x15, 0x00, 0x78, 0xe0, 0x19, 0xc4, 0x15, 0x40, 0x78, 0xe0, >> + 0x93, 0xdd, 0xc3, 0xc1, 0x46, 0x01, 0x70, 0xe1, 0x08, 0x80, 0x0b, 0xa1, >> + 0x08, 0x5c, 0x00, 0xda, 0x06, 0x01, 0x68, 0xe1, 0x04, 0x80, 0x4a, 0x40, >> + 0x84, 0xe0, 0x08, 0x5c, 0x00, 0x9a, 0x06, 0x01, 0xe0, 0xe0, 0x14, 0x80, >> + 0x25, 0x02, 0x54, 0xe0, 0x29, 0xc4, 0x25, 0x42, 0x54, 0xe0, 0x24, 0x80, >> + 0x35, 0x04, 0x6c, 0xe0, 0x39, 0xc4, 0x35, 0x44, 0x6c, 0xe0, 0x25, 0x02, >> + 0x64, 0xe0, 0x29, 0xc4, 0x25, 0x42, 0x64, 0xe0, 0x04, 0x80, 0x15, 0x00, >> + 0x7c, 0xe0, 0x19, 0xc4, 0x15, 0x40, 0x7c, 0xe0, 0x93, 0xdd, 0xc3, 0xc1, >> + 0x4c, 0x04, 0x7c, 0xfa, 0x86, 0x40, 0x98, 0xe0, 0x14, 0x80, 0x1b, 0xa1, >> + 0x06, 0x00, 0x00, 0xc0, 0x08, 0x42, 0x38, 0xdc, 0x08, 0x64, 0xa0, 0xef, >> + 0x86, 0x42, 0x3c, 0xe0, 0x68, 0x49, 0x80, 0xef, 0x6b, 0x80, 0x78, 0x53, >> + 0xc8, 0xef, 0xc6, 0x54, 0x6c, 0xe1, 0x7b, 0x80, 0xb5, 0x14, 0x0c, 0xf8, >> + 0x05, 0x14, 0x14, 0xf8, 0x1a, 0xac, 0x8a, 0x80, 0x0b, 0x90, 0x38, 0x55, >> + 0x80, 0xef, 0x1a, 0xae, 0x17, 0xc2, 0x03, 0x82, 0x88, 0x65, 0x80, 0xef, >> + 0x1b, 0x80, 0x0b, 0x8e, 0x68, 0x65, 0x80, 0xef, 0x9b, 0x80, 0x0b, 0x8c, >> + 0x08, 0x65, 0x80, 0xef, 0x6b, 0x80, 0x0b, 0x92, 0x1b, 0x8c, 0x98, 0x64, >> + 0x80, 0xef, 0x1a, 0xec, 0x9b, 0x80, 0x0b, 0x90, 0x95, 0x54, 0x10, 0xe0, >> + 0xa8, 0x53, 0x80, 0xef, 0x1a, 0xee, 0x17, 0xc2, 0x03, 0x82, 0xf8, 0x63, >> + 0x80, 0xef, 0x1b, 0x80, 0x0b, 0x8e, 0xd8, 0x63, 0x80, 0xef, 0x1b, 0x8c, >> + 0x68, 0x63, 0x80, 0xef, 0x6b, 0x80, 0x0b, 0x92, 0x65, 0x54, 0x14, 0xe0, >> + 0x08, 0x65, 0x84, 0xef, 0x68, 0x63, 0x80, 0xef, 0x7b, 0x80, 0x0b, 0x8c, >> + 0xa8, 0x64, 0x84, 0xef, 0x08, 0x63, 0x80, 0xef, 0x14, 0xe8, 0x46, 0x44, >> + 0x94, 0xe1, 0x24, 0x88, 0x4a, 0x4e, 0x04, 0xe0, 0x14, 0xea, 0x1a, 0x04, >> + 0x08, 0xe0, 0x0a, 0x40, 0x84, 0xed, 0x0c, 0x04, 0x00, 0xe2, 0x4a, 0x40, >> + 0x04, 0xe0, 0x19, 0x16, 0xc0, 0xe0, 0x0a, 0x40, 0x84, 0xed, 0x21, 0x54, >> + 0x60, 0xe0, 0x0c, 0x04, 0x00, 0xe2, 0x1b, 0xa5, 0x0e, 0xea, 0x01, 0x89, >> + 0x21, 0x54, 0x64, 0xe0, 0x7e, 0xe8, 0x65, 0x82, 0x1b, 0xa7, 0x26, 0x00, >> + 0x00, 0x80, 0xa5, 0x82, 0x1b, 0xa9, 0x65, 0x82, 0x1b, 0xa3, 0x01, 0x85, >> + 0x16, 0x00, 0x00, 0xc0, 0x01, 0x54, 0x04, 0xf8, 0x06, 0xaa, 0x01, 0x83, >> + 0x06, 0xa8, 0x65, 0x81, 0x06, 0xa8, 0x01, 0x54, 0x04, 0xf8, 0x01, 0x83, >> + 0x06, 0xaa, 0x09, 0x14, 0x18, 0xf8, 0x0b, 0xa1, 0x05, 0x84, 0xc6, 0x42, >> + 0xd4, 0xe0, 0x14, 0x84, 0x01, 0x83, 0x01, 0x54, 0x60, 0xe0, 0x01, 0x54, >> + 0x64, 0xe0, 0x0b, 0x02, 0x90, 0xe0, 0x10, 0x02, 0x90, 0xe5, 0x01, 0x54, >> + 0x88, 0xe0, 0xb5, 0x81, 0xc6, 0x40, 0xd4, 0xe0, 0x14, 0x80, 0x0b, 0x02, >> + 0xe0, 0xe4, 0x10, 0x02, 0x31, 0x66, 0x02, 0xc0, 0x01, 0x54, 0x88, 0xe0, >> + 0x1a, 0x84, 0x29, 0x14, 0x10, 0xe0, 0x1c, 0xaa, 0x2b, 0xa1, 0xf5, 0x82, >> + 0x25, 0x14, 0x10, 0xf8, 0x2b, 0x04, 0xa8, 0xe0, 0x20, 0x44, 0x0d, 0x70, >> + 0x03, 0xc0, 0x2b, 0xa1, 0x04, 0x00, 0x80, 0x9a, 0x02, 0x40, 0x84, 0x90, >> + 0x03, 0x54, 0x04, 0x80, 0x4c, 0x0c, 0x7c, 0xf2, 0x93, 0xdd, 0x00, 0x00, >> + 0x02, 0xa9, 0x00, 0x00, 0x64, 0x4a, 0x40, 0x00, 0x08, 0x2d, 0x58, 0xe0, >> + 0xa8, 0x98, 0x40, 0x00, 0x28, 0x07, 0x34, 0xe0, 0x05, 0xb9, 0x00, 0x00, >> + 0x28, 0x00, 0x41, 0x05, 0x88, 0x00, 0x41, 0x3c, 0x98, 0x00, 0x41, 0x52, >> + 0x04, 0x01, 0x41, 0x79, 0x3c, 0x01, 0x41, 0x6a, 0x3d, 0xfe, 0x00, 0x00, >> +}; >> + >> +static const char * const vgxy61_test_pattern_menu[] = { >> + "Disabled", >> + "Solid", >> + "Colorbar", >> + "Gradbar", >> + "Hgrey", >> + "Vgrey", >> + "Dgrey", >> + "PN28", >> +}; >> + >> +static const char * const vgxy61_hdr_mode_menu[] = { >> + "HDR linearize", >> + "HDR substraction", >> + "no HDR", >> +}; >> + >> +/* Regulator supplies */ >> +static const char * const vgxy61_supply_name[] = { >> + "VCORE", >> + "VDDIO", >> + "VANA", >> +}; >> + >> +static const s64 link_freq[] = { >> + /* >> + * MIPI output freq is 804Mhz / 2, as it uses both rising edge and falling edges to send >> + * data >> + */ >> + 402000000ULL >> +}; >> + >> +#define VGXY61_NUM_SUPPLIES ARRAY_SIZE(vgxy61_supply_name) > > This would be nicer just after the supply names. > >> + >> +enum vgxy61_bin_mode { >> + VGXY61_BIN_MODE_NORMAL, >> + VGXY61_BIN_MODE_DIGITAL_X2, >> + VGXY61_BIN_MODE_DIGITAL_X4, >> +}; >> + >> +enum vgxy61_hdr_mode { >> + VGXY61_HDR_LINEAR, >> + VGXY61_HDR_SUB, >> + VGXY61_NO_HDR, >> +}; >> + >> +enum vgxy61_strobe_mode { >> + VGXY61_STROBE_DISABLED, >> + VGXY61_STROBE_LONG, >> + VGXY61_STROBE_ENABLED, >> +}; >> + >> +struct vgxy61_mode_info { >> + u32 width; >> + u32 height; >> + enum vgxy61_bin_mode bin_mode; >> + struct v4l2_rect crop; >> +}; >> + >> +struct vgxy61_fmt_desc { >> + u32 code; >> + u8 bpp; >> + u8 data_type; >> +}; >> + >> +static const struct vgxy61_fmt_desc vgxy61_supported_codes[] = { >> + { >> + .code = MEDIA_BUS_FMT_Y8_1X8, >> + .bpp = 8, >> + .data_type = MIPI_CSI2_DT_RAW8, >> + }, >> + { >> + .code = MEDIA_BUS_FMT_Y10_1X10, >> + .bpp = 10, >> + .data_type = MIPI_CSI2_DT_RAW10, >> + }, >> + { >> + .code = MEDIA_BUS_FMT_Y12_1X12, >> + .bpp = 12, >> + .data_type = MIPI_CSI2_DT_RAW12, >> + }, >> + { >> + .code = MEDIA_BUS_FMT_Y14_1X14, >> + .bpp = 14, >> + .data_type = MIPI_CSI2_DT_RAW14, >> + }, >> + { >> + .code = MEDIA_BUS_FMT_Y16_1X16, >> + .bpp = 16, >> + .data_type = MIPI_CSI2_DT_RAW16, >> + }, >> +}; >> + >> +static const struct vgxy61_mode_info vgx661_mode_data[] = { >> + { >> + .width = VGX661_WIDTH, >> + .height = VGX661_HEIGHT, >> + .bin_mode = VGXY61_BIN_MODE_NORMAL, >> + .crop = { >> + .left = 0, >> + .top = 0, >> + .width = VGX661_WIDTH, >> + .height = VGX661_HEIGHT, >> + }, >> + }, >> + { >> + .width = 1280, >> + .height = 720, >> + .bin_mode = VGXY61_BIN_MODE_NORMAL, >> + .crop = { >> + .left = 92, >> + .top = 192, >> + .width = 1280, >> + .height = 720, >> + }, >> + }, >> + { >> + .width = 640, >> + .height = 480, >> + .bin_mode = VGXY61_BIN_MODE_DIGITAL_X2, >> + .crop = { >> + .left = 92, >> + .top = 72, >> + .width = 1280, >> + .height = 960, >> + }, >> + }, >> + { >> + .width = 320, >> + .height = 240, >> + .bin_mode = VGXY61_BIN_MODE_DIGITAL_X4, >> + .crop = { >> + .left = 92, >> + .top = 72, >> + .width = 1280, >> + .height = 960, >> + }, >> + }, >> +}; >> + >> +static const struct vgxy61_mode_info vgx761_mode_data[] = { >> + { >> + .width = VGX761_WIDTH, >> + .height = VGX761_HEIGHT, >> + .bin_mode = VGXY61_BIN_MODE_NORMAL, >> + .crop = { >> + .left = 0, >> + .top = 0, >> + .width = VGX761_WIDTH, >> + .height = VGX761_HEIGHT, >> + }, >> + }, >> + { >> + .width = 1920, >> + .height = 1080, >> + .bin_mode = VGXY61_BIN_MODE_NORMAL, >> + .crop = { >> + .left = 12, >> + .top = 62, >> + .width = 1920, >> + .height = 1080, >> + }, >> + }, >> + { >> + .width = 1280, >> + .height = 720, >> + .bin_mode = VGXY61_BIN_MODE_NORMAL, >> + .crop = { >> + .left = 332, >> + .top = 242, >> + .width = 1280, >> + .height = 720, >> + }, >> + }, >> + { >> + .width = 640, >> + .height = 480, >> + .bin_mode = VGXY61_BIN_MODE_DIGITAL_X2, >> + .crop = { >> + .left = 332, >> + .top = 122, >> + .width = 1280, >> + .height = 960, >> + }, >> + }, >> + { >> + .width = 320, >> + .height = 240, >> + .bin_mode = VGXY61_BIN_MODE_DIGITAL_X4, >> + .crop = { >> + .left = 332, >> + .top = 122, >> + .width = 1280, >> + .height = 960, >> + }, >> + }, >> +}; >> + >> +struct vgxy61_dev { >> + struct i2c_client *i2c_client; >> + struct v4l2_subdev sd; >> + struct media_pad pad; >> + struct regulator_bulk_data supplies[VGXY61_NUM_SUPPLIES]; >> + struct gpio_desc *reset_gpio; >> + struct clk *xclk; >> + u32 clk_freq; >> + int id; >> + int sensor_width; >> + int sensor_height; >> + u16 oif_ctrl; >> + int nb_of_lane; >> + int data_rate_in_mbps; >> + int pclk; >> + u16 line_length; >> + int rot_term; > > rot_term value is always the same. No need for a field here for that. > It changes according to the sensor's model id and could be either VGX761_SHORT_ROT_TERM or VGX661_SHORT_ROT_TERM. See the vgxy61_fill_sensor_param function. >> + bool gpios_polarity; >> + bool slave_mode; >> + /* Lock to protect all members below */ >> + struct mutex lock; >> + struct v4l2_ctrl_handler ctrl_handler; >> + struct v4l2_ctrl *pixel_rate_ctrl; >> + struct v4l2_ctrl *expo_ctrl; >> + struct v4l2_ctrl *vblank_ctrl; >> + struct v4l2_ctrl *vflip_ctrl; >> + struct v4l2_ctrl *hflip_ctrl; >> + bool streaming; >> + struct v4l2_mbus_framefmt fmt; >> + const struct vgxy61_mode_info *sensor_modes; >> + int sensor_modes_nb; >> + const struct vgxy61_mode_info *default_mode; >> + const struct vgxy61_mode_info *current_mode; >> + bool hflip; >> + bool vflip; >> + enum vgxy61_hdr_mode hdr; >> + int expo_long; >> + int expo_short; >> + int expo_max; >> + int expo_min; >> + int vblank; >> + int vblank_min; >> + u16 frame_length; >> + int digital_gain; >> + int analog_gain; >> + enum vgxy61_strobe_mode strobe_mode; >> + int pattern; >> +}; >> + >> +static u8 get_bpp_by_code(__u32 code) >> +{ >> + int i; > > unsigned int? > > Same for the rest of similar instances. > I'll make a pass on data types. Thank you for your vigilance. >> + >> + for (i = 0; i < ARRAY_SIZE(vgxy61_supported_codes); i++) { >> + if (vgxy61_supported_codes[i].code == code) >> + return vgxy61_supported_codes[i].bpp; >> + } >> + /* Should never happen */ >> + WARN(1, "Unsupported code %d. default to 8 bpp", code); >> + return 8; >> +} >> + >> +static u8 get_data_type_by_code(__u32 code) >> +{ >> + int i; >> + >> + for (i = 0; i < ARRAY_SIZE(vgxy61_supported_codes); i++) { >> + if (vgxy61_supported_codes[i].code == code) >> + return vgxy61_supported_codes[i].data_type; >> + } >> + /* Should never happen */ >> + WARN(1, "Unsupported code %d. default to MIPI_CSI2_DT_RAW8 data type", code); >> + return MIPI_CSI2_DT_RAW8; >> +} >> + >> +static void compute_pll_parameters_by_freq(u32 freq, unsigned int *prediv, unsigned int *mult) >> +{ >> + const unsigned int predivs[] = {1, 2, 4}; >> + int i; >> + >> + /* >> + * Freq range is [6Mhz-27Mhz] already checked. >> + * Output of divider should be in [6Mhz-12Mhz[. >> + */ >> + for (i = 0; i < ARRAY_SIZE(predivs); i++) { >> + *prediv = predivs[i]; >> + if (freq / *prediv < 12 * HZ_PER_MHZ) >> + break; >> + } >> + WARN_ON(i == ARRAY_SIZE(predivs)); >> + >> + /* >> + * Target freq is 804Mhz. Don't change this as it will impact image quality. >> + */ >> + *mult = ((804 * HZ_PER_MHZ) * (*prediv) + freq / 2) / freq; >> +} >> + >> +static s32 get_pixel_rate(struct vgxy61_dev *sensor) >> +{ >> + return div64_u64((u64)sensor->data_rate_in_mbps * sensor->nb_of_lane, >> + get_bpp_by_code(sensor->fmt.code)); >> +} >> + >> +static inline struct vgxy61_dev *to_vgxy61_dev(struct v4l2_subdev *sd) >> +{ >> + return container_of(sd, struct vgxy61_dev, sd); >> +} >> + >> +static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl) >> +{ >> + return &container_of(ctrl->handler, struct vgxy61_dev, ctrl_handler)->sd; > > Could you run > > $ checkpatch.pl --strict --max-line-length=80 > > on this and address those instances where it's reasonable (I'd presume > most)? > I wrongly used 100 as a limit for the subsystem. My apologies. I'm refactoring the whole driver to use 80. >> +} >> + >> +static int get_chunk_size(struct vgxy61_dev *sensor) >> +{ >> + struct i2c_adapter *adapter = sensor->i2c_client->adapter; >> + int max_write_len = VGXY61_WRITE_MULTIPLE_CHUNK_MAX; >> + >> + if (adapter->quirks && adapter->quirks->max_write_len) >> + max_write_len = adapter->quirks->max_write_len - 2; >> + >> + max_write_len = min(max_write_len, VGXY61_WRITE_MULTIPLE_CHUNK_MAX); >> + >> + return max(max_write_len, 1); >> +} >> + >> +static int vgxy61_read_multiple(struct vgxy61_dev *sensor, u32 reg, int len) >> +{ >> + struct i2c_client *client = sensor->i2c_client; >> + struct i2c_msg msg[2]; >> + u8 buf[2]; >> + u8 val[sizeof(int)] = {0}; > > sizeof(u32) > >> + int ret; >> + >> + if (len > sizeof(int)) > > Same here. > >> + return -EINVAL; >> + buf[0] = reg >> 8; >> + buf[1] = reg & 0xff; >> + >> + msg[0].addr = client->addr; >> + msg[0].flags = client->flags; >> + msg[0].buf = buf; >> + msg[0].len = sizeof(buf); >> + >> + msg[1].addr = client->addr; >> + msg[1].flags = client->flags | I2C_M_RD; >> + msg[1].buf = val; >> + msg[1].len = len; >> + >> + ret = i2c_transfer(client->adapter, msg, 2); >> + if (ret < 0) { >> + dev_dbg(&client->dev, "%s: %x i2c_transfer, reg: %x => %d\n", __func__, >> + client->addr, reg, ret); >> + return ret; >> + } >> + >> + return (val[3] << 24) | (val[2] << 16) | (val[1] << 8) | val[0]; > > get_unaligned_le32() > >> +} >> + >> +static inline int vgxy61_read_reg(struct vgxy61_dev *sensor, u32 reg) >> +{ >> + return vgxy61_read_multiple(sensor, reg & VGXY61_REG_ADDR_MASK, >> + (reg >> VGXY61_REG_SIZE_SHIFT) & 7); >> +} >> + >> +static int vgxy61_write_multiple(struct vgxy61_dev *sensor, u32 reg, const u8 *data, int len, >> + int *err) >> +{ >> + struct i2c_client *client = sensor->i2c_client; >> + struct i2c_msg msg; >> + u8 buf[VGXY61_WRITE_MULTIPLE_CHUNK_MAX + 2]; >> + int i; >> + int ret; >> + >> + if (err && *err) >> + return *err; >> + >> + if (len > VGXY61_WRITE_MULTIPLE_CHUNK_MAX) >> + return -EINVAL; >> + buf[0] = reg >> 8; >> + buf[1] = reg & 0xff; >> + for (i = 0; i < len; i++) >> + buf[i + 2] = data[i]; >> + >> + msg.addr = client->addr; >> + msg.flags = client->flags; >> + msg.buf = buf; >> + msg.len = len + 2; >> + >> + ret = i2c_transfer(client->adapter, &msg, 1); >> + if (ret < 0) { >> + dev_dbg(&client->dev, "%s: i2c_transfer, reg: %x => %d\n", __func__, reg, ret); >> + if (err) >> + *err = ret; >> + return ret; >> + } >> + >> + return 0; >> +} >> + >> +static int vgxy61_write_array(struct vgxy61_dev *sensor, u32 reg, int nb, const u8 *array) > > nb should probably be unsigned int. > >> +{ >> + const int chunk_size = get_chunk_size(sensor); >> + int ret; >> + int sz; > > Same for sz and chunk_size. I believe this also applies to other signed > variables in this driver. > >> + >> + while (nb) { >> + sz = min(nb, chunk_size); >> + ret = vgxy61_write_multiple(sensor, reg, array, sz, NULL); >> + if (ret < 0) >> + return ret; >> + nb -= sz; >> + reg += sz; >> + array += sz; >> + } >> + >> + return 0; >> +} >> + >> +static inline int vgxy61_write_reg(struct vgxy61_dev *sensor, u32 reg, u32 val, int *err) >> +{ >> + return vgxy61_write_multiple(sensor, reg & VGXY61_REG_ADDR_MASK, (u8 *)&val, >> + (reg >> VGXY61_REG_SIZE_SHIFT) & 7, err); >> +} >> + >> +static int vgxy61_poll_reg(struct vgxy61_dev *sensor, u32 reg, u8 poll_val, int timeout_ms) >> +{ >> + const int loop_delay_ms = 10; >> + int ret, timeout; >> + >> + timeout = read_poll_timeout(vgxy61_read_reg, ret, ((ret < 0) || (ret == poll_val)), >> + loop_delay_ms * 1000, timeout_ms * 1000, false, >> + sensor, reg); >> + if (timeout) >> + return timeout; >> + >> + return 0; >> +} >> + >> +static int vgxy61_wait_state(struct vgxy61_dev *sensor, int state, int timeout_ms) >> +{ >> + return vgxy61_poll_reg(sensor, VGXY61_REG_SYSTEM_FSM, state, timeout_ms); >> +} >> + >> +static int vgxy61_check_bw(struct vgxy61_dev *sensor) >> +{ >> + struct i2c_client *client = sensor->i2c_client; >> + /* Correction factor for time required between 2 lines */ >> + const int mipi_margin = 1056; > > What does this value signify? Is it really constant or somehow dependent by > the configuration set by the driver? It is a simplification of time needed to send short packets and for the MIPI to add transition times (EoT, LPS, and SoT packet delimiters) needed by the protocol to go in low power between 2 packets of data. This is a mipi IP constant for the sensor. >> + int binning_scale = sensor->current_mode->crop.height / sensor->current_mode->height; >> + int bpp = get_bpp_by_code(sensor->fmt.code); >> + int max_bit_per_line; >> + int bit_per_line; >> + u64 line_rate; >> + >> + line_rate = sensor->nb_of_lane * (u64)sensor->data_rate_in_mbps * sensor->line_length; >> + max_bit_per_line = div64_u64(line_rate, sensor->pclk) - mipi_margin; >> + bit_per_line = (bpp * sensor->current_mode->width) / binning_scale; >> + >> + dev_dbg(&client->dev, "max_bit_per_line = %d\n", max_bit_per_line); >> + dev_dbg(&client->dev, "required bit_per_line = %d\n", bit_per_line); >> + >> + return bit_per_line > max_bit_per_line ? -EINVAL : 0; >> +} >> + >> +static int vgxy61_apply_exposure(struct vgxy61_dev *sensor) >> +{ >> + struct i2c_client *client = sensor->i2c_client; >> + int ret = 0; >> + >> + /* We first set expo to zero to avoid forbidden parameters couple */ >> + ret = vgxy61_write_reg(sensor, VGXY61_REG_COARSE_EXPOSURE_SHORT, 0, &ret); >> + ret = vgxy61_write_reg(sensor, VGXY61_REG_COARSE_EXPOSURE_LONG, sensor->expo_long, &ret); >> + ret = vgxy61_write_reg(sensor, VGXY61_REG_COARSE_EXPOSURE_SHORT, sensor->expo_short, &ret); >> + if (ret) >> + return ret; >> + >> + dev_dbg(&client->dev, "%s applied expo %d (short: %d)\n", __func__, >> + sensor->expo_long, sensor->expo_short); >> + >> + return 0; >> +} >> + >> +static int vgxy61_get_regulators(struct vgxy61_dev *sensor) >> +{ >> + int i; >> + >> + for (i = 0; i < VGXY61_NUM_SUPPLIES; i++) >> + sensor->supplies[i].supply = vgxy61_supply_name[i]; > > unsigned int. > >> + >> + return devm_regulator_bulk_get(&sensor->i2c_client->dev, VGXY61_NUM_SUPPLIES, >> + sensor->supplies); >> +} >> + >> +static int vgxy61_apply_reset(struct vgxy61_dev *sensor) >> +{ >> + struct i2c_client *client = sensor->i2c_client; >> + >> + dev_dbg(&client->dev, "%s applied reset\n", __func__); >> + gpiod_set_value_cansleep(sensor->reset_gpio, 0); >> + usleep_range(5000, 10000); >> + gpiod_set_value_cansleep(sensor->reset_gpio, 1); >> + usleep_range(5000, 10000); >> + gpiod_set_value_cansleep(sensor->reset_gpio, 0); >> + usleep_range(40000, 100000); >> + return vgxy61_wait_state(sensor, VGXY61_SYSTEM_FSM_SW_STBY, VGXY61_TIMEOUT_MS); >> +} >> + >> +static void vgxy61_fill_framefmt(struct vgxy61_dev *sensor, const struct vgxy61_mode_info *mode, >> + struct v4l2_mbus_framefmt *fmt, u32 code) >> +{ >> + fmt->code = code; >> + fmt->width = mode->width; >> + fmt->height = mode->height; >> + fmt->colorspace = V4L2_COLORSPACE_RAW; >> + fmt->field = V4L2_FIELD_NONE; >> + fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; >> + fmt->quantization = V4L2_QUANTIZATION_DEFAULT; >> + fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT; >> +} >> + >> +static int vgxy61_try_fmt_internal(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *fmt, >> + const struct vgxy61_mode_info **new_mode) >> +{ >> + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); >> + const struct vgxy61_mode_info *mode = sensor->sensor_modes; >> + unsigned int index; >> + >> + /* Select code */ >> + for (index = 0; index < ARRAY_SIZE(vgxy61_supported_codes); index++) { >> + if (vgxy61_supported_codes[index].code == fmt->code) >> + break; >> + } >> + if (index == ARRAY_SIZE(vgxy61_supported_codes)) >> + index = 0; >> + >> + /* Select size */ >> + mode = v4l2_find_nearest_size(sensor->sensor_modes, sensor->sensor_modes_nb, >> + width, height, fmt->width, fmt->height); >> + if (new_mode) >> + *new_mode = mode; >> + >> + vgxy61_fill_framefmt(sensor, mode, fmt, vgxy61_supported_codes[index].code); > > Please run the set through this: > > $ ./scripts/checkpatch.pl --strict --max-line-length=80 > >> + >> + return 0; >> +} >> + >> +static int vgxy61_get_selection(struct v4l2_subdev *sd, >> + struct v4l2_subdev_state *sd_state, >> + struct v4l2_subdev_selection *sel) >> +{ >> + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); >> + >> + switch (sel->target) { >> + case V4L2_SEL_TGT_CROP: >> + sel->r = sensor->current_mode->crop; >> + return 0; >> + case V4L2_SEL_TGT_NATIVE_SIZE: >> + case V4L2_SEL_TGT_CROP_DEFAULT: >> + case V4L2_SEL_TGT_CROP_BOUNDS: >> + sel->r.top = 0; >> + sel->r.left = 0; >> + sel->r.width = sensor->sensor_width; >> + sel->r.height = sensor->sensor_height; >> + return 0; >> + } >> + >> + return -EINVAL; >> +} >> + >> +static int vgxy61_enum_mbus_code(struct v4l2_subdev *sd, >> + struct v4l2_subdev_state *sd_state, >> + struct v4l2_subdev_mbus_code_enum *code) >> +{ >> + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); >> + struct i2c_client *client = sensor->i2c_client; >> + >> + dev_dbg(&client->dev, "%s probe index %d\n", __func__, code->index); > > This is hardly necessary anymore, please remove. > I will remove all these. >> + if (code->index >= ARRAY_SIZE(vgxy61_supported_codes)) >> + return -EINVAL; >> + >> + code->code = vgxy61_supported_codes[code->index].code; >> + >> + return 0; >> +} >> + >> +static int vgxy61_get_fmt(struct v4l2_subdev *sd, >> + struct v4l2_subdev_state *sd_state, >> + struct v4l2_subdev_format *format) >> +{ >> + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); >> + struct i2c_client *client = sensor->i2c_client; >> + struct v4l2_mbus_framefmt *fmt; >> + >> + dev_dbg(&client->dev, "%s probe %d\n", __func__, format->pad); >> + dev_dbg(&client->dev, "%s %dx%d\n", __func__, format->format.width, format->format.height); > > These seem hardly useful. > >> + >> + mutex_lock(&sensor->lock); >> + >> + if (format->which == V4L2_SUBDEV_FORMAT_TRY) >> + fmt = v4l2_subdev_get_try_format(&sensor->sd, sd_state, format->pad); >> + else >> + fmt = &sensor->fmt; >> + >> + format->format = *fmt; >> + >> + mutex_unlock(&sensor->lock); >> + >> + return 0; >> +} >> + >> +static int vgxy61_get_vblank_min(struct vgxy61_dev *sensor, enum vgxy61_hdr_mode hdr) >> +{ >> + int min_vblank = VGXY61_MIN_FRAME_LENGTH - sensor->current_mode->crop.height; >> + /* Ensure the first rule of thumb can't be negative */ >> + int min_vblank_hdr = VGXY61_MIN_EXPOSURE + sensor->rot_term + 1; >> + >> + if (hdr != VGXY61_NO_HDR) >> + return max(min_vblank, min_vblank_hdr); >> + return min_vblank; >> +} >> + >> +static int vgxy61_enum_frame_size(struct v4l2_subdev *sd, struct v4l2_subdev_state *sd_state, >> + struct v4l2_subdev_frame_size_enum *fse) >> +{ >> + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); >> + struct i2c_client *client = sensor->i2c_client; >> + >> + dev_dbg(&client->dev, "%s for index %d\n", __func__, fse->index); > > Ditto. > >> + if (fse->index >= sensor->sensor_modes_nb) >> + return -EINVAL; >> + >> + fse->min_width = sensor->sensor_modes[fse->index].width; >> + fse->max_width = fse->min_width; >> + fse->min_height = sensor->sensor_modes[fse->index].height; >> + fse->max_height = fse->min_height; >> + >> + return 0; >> +} >> + >> +static int vgxy61_update_analog_gain(struct vgxy61_dev *sensor, u32 target) >> +{ >> + struct i2c_client *client = sensor->i2c_client; >> + >> + dev_dbg(&client->dev, "Set analog gain: 0x%04x\n", target); > > Ditto. > >> + sensor->analog_gain = target; >> + >> + /* Apply gain */ > > Ditto. The same for rest of similar comments and debug prints. > >> + if (sensor->streaming) >> + return vgxy61_write_reg(sensor, VGXY61_REG_ANALOG_GAIN, target, NULL); >> + return 0; >> +} >> + >> +static int vgxy61_apply_digital_gain(struct vgxy61_dev *sensor, u32 digital_gain) >> +{ >> + int ret = 0; >> + >> + /* >> + * For a monochrome version, configuring DIGITAL_GAIN_LONG_CH0 and >> + * DIGITAL_GAIN_SHORT_CH0 is enough to configure the gain of all >> + * four sub pixels. >> + */ >> + vgxy61_write_reg(sensor, VGXY61_REG_DIGITAL_GAIN_LONG, digital_gain, &ret); >> + vgxy61_write_reg(sensor, VGXY61_REG_DIGITAL_GAIN_SHORT, digital_gain, &ret); >> + >> + return ret; >> +} >> + >> +static int vgxy61_update_digital_gain(struct vgxy61_dev *sensor, u32 target) >> +{ >> + struct i2c_client *client = sensor->i2c_client; >> + >> + dev_dbg(&client->dev, "Set digital gain: 0x%04x\n", target); >> + sensor->digital_gain = target; >> + >> + if (sensor->streaming) >> + return vgxy61_apply_digital_gain(sensor, sensor->digital_gain); >> + return 0; >> +} >> + >> +static int vgxy61_apply_patgen(struct vgxy61_dev *sensor, u32 index) >> +{ >> + static const u8 index2val[] = {0x0, 0x1, 0x2, 0x3, 0x10, 0x11, 0x12, 0x13}; >> + u32 pattern = index2val[index]; >> + u32 reg = (pattern << VGXY61_PATGEN_LONG_TYPE_SHIFT) | >> + (pattern << VGXY61_PATGEN_SHORT_TYPE_SHIFT); >> + >> + if (pattern) >> + reg |= VGXY61_PATGEN_LONG_ENABLE | VGXY61_PATGEN_SHORT_ENABLE; >> + return vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_CTRL, reg, NULL); >> +} >> + >> +static int vgxy61_update_patgen(struct vgxy61_dev *sensor, int pattern) >> +{ >> + sensor->pattern = pattern; >> + >> + if (sensor->streaming) >> + return vgxy61_apply_patgen(sensor, sensor->pattern); >> + return 0; >> +} >> + >> +static int vgxy61_apply_gpiox_strobe_mode(struct vgxy61_dev *sensor, enum vgxy61_strobe_mode mode, >> + int idx) >> +{ >> + static const u8 index2val[] = {0x0, 0x1, 0x3}; >> + u16 reg; >> + >> + reg = vgxy61_read_reg(sensor, VGXY61_REG_SIGNALS_CTRL); >> + if (reg < 0) >> + return reg; >> + reg &= ~(0xf << (idx * VGXY61_SIGNALS_GPIO_ID_SHIFT)); >> + reg |= index2val[mode] << (idx * VGXY61_SIGNALS_GPIO_ID_SHIFT); >> + >> + return vgxy61_write_reg(sensor, VGXY61_REG_SIGNALS_CTRL, reg, NULL); >> +} >> + >> +static int vgxy61_update_gpios_strobe_mode(struct vgxy61_dev *sensor, enum vgxy61_hdr_mode hdr) >> +{ >> + int i, ret; >> + >> + switch (hdr) { >> + case VGXY61_HDR_LINEAR: >> + sensor->strobe_mode = VGXY61_STROBE_ENABLED; >> + break; >> + case VGXY61_HDR_SUB: >> + case VGXY61_NO_HDR: >> + sensor->strobe_mode = VGXY61_STROBE_LONG; >> + break; >> + default: >> + /* Should never happen */ >> + WARN_ON(true); >> + break; >> + } >> + >> + if (sensor->streaming) { >> + for (i = 0; i < VGXY61_NB_GPIOS; i++) { >> + ret = vgxy61_apply_gpiox_strobe_mode(sensor, sensor->strobe_mode, i); >> + if (ret) >> + return ret; >> + } >> + } >> + >> + return 0; >> +} >> + >> +static int vgxy61_update_gpios_strobe_polarity(struct vgxy61_dev *sensor, int polarity) >> +{ >> + struct i2c_client *client = sensor->i2c_client; >> + int ret = 0; >> + >> + if (sensor->streaming) >> + return -EBUSY; >> + >> + dev_dbg(&client->dev, "setting gpios polarity: %d\n", polarity); >> + >> + vgxy61_write_reg(sensor, VGXY61_REG_GPIO_0_CTRL, polarity << 1, &ret); >> + vgxy61_write_reg(sensor, VGXY61_REG_GPIO_1_CTRL, polarity << 1, &ret); >> + vgxy61_write_reg(sensor, VGXY61_REG_GPIO_2_CTRL, polarity << 1, &ret); >> + vgxy61_write_reg(sensor, VGXY61_REG_GPIO_3_CTRL, polarity << 1, &ret); >> + vgxy61_write_reg(sensor, VGXY61_REG_SIGNALS_POLARITY_CTRL, polarity, &ret); >> + >> + return ret; >> +} >> + >> +static int vgxy61_get_expo_long_max(struct vgxy61_dev *sensor, int short_expo_ratio) >> +{ >> + int first_rot_max_expo, second_rot_max_expo, third_rot_max_expo; >> + >> + /* Apply sensor's rules of thumb */ >> + /* >> + * Short exposure + height must be less than frame length to avoid bad pixel line at the >> + * botom of the image >> + */ >> + first_rot_max_expo = >> + ((sensor->frame_length - sensor->current_mode->crop.height - sensor->rot_term) * >> + short_expo_ratio) - 1; >> + >> + /* Total exposition time must be less than frame length to avoid sensor crash */ >> + second_rot_max_expo = >> + (((sensor->frame_length - VGXY61_EXPOS_ROT_TERM) * short_expo_ratio) / >> + (short_expo_ratio + 1)) - 1; >> + >> + /* Short exposure times 71 must be less than frame length to avoid sensor crash */ >> + third_rot_max_expo = (sensor->frame_length / 71) * short_expo_ratio; >> + >> + /* Take the minimum from all rules */ >> + return min(min(first_rot_max_expo, second_rot_max_expo), third_rot_max_expo); >> +} >> + >> +static int vgxy61_update_exposure(struct vgxy61_dev *sensor, int new_expo_long, >> + enum vgxy61_hdr_mode hdr) >> +{ >> + struct i2c_client *client = sensor->i2c_client; >> + int new_expo_short = 0; >> + int expo_short_max = 0; >> + int expo_long_min = VGXY61_MIN_EXPOSURE; >> + int expo_long_max; >> + >> + /* Compute short exposure according to hdr mode and long exposure */ >> + switch (hdr) { >> + case VGXY61_HDR_LINEAR: >> + /* Take ratio into account for minimal exposures in VGXY61_HDR_LINEAR */ >> + expo_long_min = VGXY61_MIN_EXPOSURE * VGXY61_HDR_LINEAR_RATIO; >> + new_expo_long = max(expo_long_min, new_expo_long); >> + >> + expo_long_max = vgxy61_get_expo_long_max(sensor, VGXY61_HDR_LINEAR_RATIO); >> + expo_short_max = >> + (expo_long_max + (VGXY61_HDR_LINEAR_RATIO / 2)) / VGXY61_HDR_LINEAR_RATIO; >> + new_expo_short = >> + (new_expo_long + (VGXY61_HDR_LINEAR_RATIO / 2)) / VGXY61_HDR_LINEAR_RATIO; >> + break; >> + case VGXY61_HDR_SUB: >> + new_expo_long = max(expo_long_min, new_expo_long); >> + >> + expo_long_max = vgxy61_get_expo_long_max(sensor, 1); >> + /* Short and long are the same in VGXY61_HDR_SUB */ >> + expo_short_max = expo_long_max; >> + new_expo_short = new_expo_long; >> + break; >> + case VGXY61_NO_HDR: >> + new_expo_long = max(expo_long_min, new_expo_long); >> + >> + /* >> + * As short expo is 0 here, only the second rule of thumb applies, see >> + * vgxy61_get_expo_long_max for more >> + */ >> + expo_long_max = sensor->frame_length - VGXY61_EXPOS_ROT_TERM; >> + break; >> + default: >> + /* Should never happen */ >> + WARN_ON(true); >> + break; >> + } >> + >> + /* If this happens, something is wrong with formulas */ >> + WARN_ON(expo_long_min > expo_long_max); >> + >> + if (new_expo_long > expo_long_max) { >> + dev_warn(&client->dev, "Exposure %d too high, clamping to %d\n", >> + new_expo_long, expo_long_max); >> + new_expo_long = expo_long_max; >> + new_expo_short = expo_short_max; >> + } >> + >> + dev_dbg(&client->dev, "frame_length %d, new_expo_long %d, new_expo_short %d\n", >> + sensor->frame_length, new_expo_long, new_expo_short); >> + dev_dbg(&client->dev, "expo_long_min %d, expo_long_max %d\n", >> + expo_long_min, expo_long_max); >> + >> + /* Apply exposure */ >> + sensor->expo_long = new_expo_long; >> + sensor->expo_short = new_expo_short; >> + sensor->expo_max = expo_long_max; >> + sensor->expo_min = expo_long_min; >> + >> + if (sensor->streaming) >> + return vgxy61_apply_exposure(sensor); >> + return 0; >> +} >> + >> +static int vgxy61_apply_framelength(struct vgxy61_dev *sensor) >> +{ >> + return vgxy61_write_reg(sensor, VGXY61_REG_FRAME_LENGTH, sensor->frame_length, NULL); >> +} >> + >> +static int vgxy61_update_vblank(struct vgxy61_dev *sensor, int vblank, enum vgxy61_hdr_mode hdr) >> +{ >> + int ret; >> + >> + sensor->vblank_min = vgxy61_get_vblank_min(sensor, hdr); >> + sensor->vblank = max(sensor->vblank_min, vblank); >> + sensor->frame_length = sensor->current_mode->crop.height + sensor->vblank; >> + >> + /* Update exposure according to vblank */ >> + ret = vgxy61_update_exposure(sensor, sensor->expo_long, hdr); >> + if (ret) >> + return ret; >> + >> + if (sensor->streaming) >> + return vgxy61_apply_framelength(sensor); >> + return 0; >> +} >> + >> +static int vgxy61_apply_hdr(struct vgxy61_dev *sensor, enum vgxy61_hdr_mode index) >> +{ >> + static const u8 index2val[] = {0x1, 0x4, 0xa}; >> + >> + return vgxy61_write_reg(sensor, VGXY61_REG_HDR_CTRL, index2val[index], NULL); >> +} >> + >> +static int vgxy61_update_hdr(struct vgxy61_dev *sensor, enum vgxy61_hdr_mode index) >> +{ >> + int ret; >> + >> + /* >> + * vblank and short exposure change according to HDR mode, do it first as it can >> + * violate sensors 'rule of thumbs' and therefore will require to change the long exposure >> + */ >> + ret = vgxy61_update_vblank(sensor, sensor->vblank, index); >> + if (ret) >> + return ret; >> + >> + /* Update strobe mode according to HDR */ >> + ret = vgxy61_update_gpios_strobe_mode(sensor, index); >> + if (ret) >> + return ret; >> + >> + sensor->hdr = index; >> + >> + if (sensor->streaming) >> + return vgxy61_apply_hdr(sensor, sensor->hdr); >> + return 0; >> +} >> + >> +static int vgxy61_apply_settings(struct vgxy61_dev *sensor) >> +{ >> + int ret, i; >> + >> + ret = vgxy61_apply_hdr(sensor, sensor->hdr); >> + if (ret) >> + return ret; >> + >> + ret = vgxy61_apply_framelength(sensor); >> + if (ret) >> + return ret; >> + >> + ret = vgxy61_apply_exposure(sensor); >> + if (ret) >> + return ret; >> + >> + ret = vgxy61_write_reg(sensor, VGXY61_REG_ANALOG_GAIN, sensor->analog_gain, NULL); >> + if (ret) >> + return ret; >> + ret = vgxy61_apply_digital_gain(sensor, sensor->digital_gain); >> + if (ret) >> + return ret; >> + >> + ret = vgxy61_write_reg(sensor, VGXY61_REG_ORIENTATION, >> + sensor->hflip | (sensor->vflip << 1), NULL); >> + if (ret) >> + return ret; > > Instead of doing this, please use v4l2_ctrl_handler_setup(). > > And check the power state of the sensor in the s_ctrl callback using > pm_runtime_get_if_in_use(), not streaming state. > >> + >> + ret = vgxy61_apply_patgen(sensor, sensor->pattern); >> + if (ret) >> + return ret; >> + >> + for (i = 0; i < VGXY61_NB_GPIOS; i++) { >> + ret = vgxy61_apply_gpiox_strobe_mode(sensor, sensor->strobe_mode, i); >> + if (ret) >> + return ret; >> + } >> + >> + return 0; >> +} >> + >> +static int vgxy61_stream_enable(struct vgxy61_dev *sensor) >> +{ >> + struct i2c_client *client = v4l2_get_subdevdata(&sensor->sd); >> + const struct v4l2_rect *crop = &sensor->current_mode->crop; >> + int ret = 0; >> + >> + ret = vgxy61_check_bw(sensor); >> + if (ret) >> + return ret; >> + >> + ret = pm_runtime_get_sync(&client->dev); >> + if (ret < 0) { >> + pm_runtime_put_noidle(&client->dev); >> + return ret; >> + } >> + >> + /* Configure sensor */ >> + vgxy61_write_reg(sensor, VGXY61_REG_FORMAT_CTRL, get_bpp_by_code(sensor->fmt.code), &ret); >> + vgxy61_write_reg(sensor, VGXY61_REG_OIF_ROI0_CTRL, >> + get_data_type_by_code(sensor->fmt.code), &ret); >> + >> + vgxy61_write_reg(sensor, VGXY61_REG_READOUT_CTRL, sensor->current_mode->bin_mode, &ret); >> + vgxy61_write_reg(sensor, VGXY61_REG_ROI0_START_H, crop->left, &ret); >> + vgxy61_write_reg(sensor, VGXY61_REG_ROI0_END_H, crop->left + crop->width - 1, &ret); >> + vgxy61_write_reg(sensor, VGXY61_REG_ROI0_START_V, crop->top, &ret); >> + vgxy61_write_reg(sensor, VGXY61_REG_ROI0_END_V, crop->top + crop->height - 1, &ret); >> + if (ret) >> + goto err_rpm_put; >> + >> + /* Apply controls cached values */ >> + ret = vgxy61_apply_settings(sensor); >> + if (ret) >> + goto err_rpm_put; >> + >> + /* Start streaming */ >> + ret = vgxy61_write_reg(sensor, VGXY61_REG_STREAMING, VGXY61_STREAMING_REQ_START, NULL); >> + if (ret) >> + goto err_rpm_put; >> + >> + ret = vgxy61_poll_reg(sensor, VGXY61_REG_STREAMING, VGXY61_STREAMING_NO_REQ, >> + VGXY61_TIMEOUT_MS); >> + if (ret) >> + goto err_rpm_put; >> + >> + ret = vgxy61_wait_state(sensor, VGXY61_SYSTEM_FSM_STREAMING, VGXY61_TIMEOUT_MS); >> + if (ret) >> + goto err_rpm_put; >> + >> + /* vflip and hflip cannot change during streaming */ >> + __v4l2_ctrl_grab(sensor->vflip_ctrl, true); >> + __v4l2_ctrl_grab(sensor->hflip_ctrl, true); >> + >> + return 0; >> + >> +err_rpm_put: >> + pm_runtime_put(&client->dev); >> + return ret; >> +} >> + >> +static int vgxy61_stream_disable(struct vgxy61_dev *sensor) >> +{ >> + struct i2c_client *client = v4l2_get_subdevdata(&sensor->sd); >> + int ret; >> + >> + ret = vgxy61_write_reg(sensor, VGXY61_REG_STREAMING, VGXY61_STREAMING_REQ_STOP, NULL); >> + if (ret)w >> + return ret; >> + >> + ret = vgxy61_poll_reg(sensor, VGXY61_REG_STREAMING, VGXY61_STREAMING_NO_REQ, 2000); >> + if (ret) >> + return ret; >> + >> + ret = vgxy61_wait_state(sensor, VGXY61_SYSTEM_FSM_SW_STBY, VGXY61_TIMEOUT_MS); >> + if (ret) >> + return ret; > > The caller won't be able to do anything reasonable to handle these errors. > Just print a warning and proceed to call pm_runtime_put(). > >> + >> + __v4l2_ctrl_grab(sensor->vflip_ctrl, false); >> + __v4l2_ctrl_grab(sensor->hflip_ctrl, false); >> + >> + pm_runtime_put(&client->dev); >> + >> + return 0; >> +} >> + >> +static int vgxy61_s_stream(struct v4l2_subdev *sd, int enable) >> +{ >> + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); >> + struct i2c_client *client = sensor->i2c_client; >> + int ret = 0; >> + >> + mutex_lock(&sensor->lock); >> + dev_dbg(&client->dev, "%s : requested %d / current = %d\n", __func__, enable, >> + sensor->streaming); >> + >> + ret = enable ? vgxy61_stream_enable(sensor) : vgxy61_stream_disable(sensor); >> + if (!ret) >> + sensor->streaming = enable; >> + >> + dev_dbg(&client->dev, "%s current now = %d / %d\n", __func__, sensor->streaming, ret); >> + mutex_unlock(&sensor->lock); >> + >> + return ret; >> +} >> + >> +static int vgxy61_set_fmt(struct v4l2_subdev *sd, >> + struct v4l2_subdev_state *sd_state, >> + struct v4l2_subdev_format *format) >> +{ >> + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); >> + struct i2c_client *client = sensor->i2c_client; >> + const struct vgxy61_mode_info *new_mode; >> + struct v4l2_mbus_framefmt *fmt; >> + int ret; >> + >> + dev_dbg(&client->dev, "%s probe %d\n", __func__, format->pad); >> + dev_dbg(&client->dev, "%s %dx%d\n", __func__, format->format.width, format->format.height); >> + >> + mutex_lock(&sensor->lock); >> + >> + if (sensor->streaming) { >> + ret = -EBUSY; >> + goto out; >> + } >> + >> + /* Find best format */ >> + ret = vgxy61_try_fmt_internal(sd, &format->format, &new_mode); >> + if (ret) >> + goto out; >> + >> + if (format->which == V4L2_SUBDEV_FORMAT_TRY) { >> + fmt = v4l2_subdev_get_try_format(sd, sd_state, 0); >> + *fmt = format->format; >> + } else if (sensor->current_mode != new_mode || sensor->fmt.code != format->format.code) { >> + fmt = &sensor->fmt; >> + *fmt = format->format; >> + >> + sensor->current_mode = new_mode; >> + >> + /* Reset vblank and framelength to default */ >> + ret = vgxy61_update_vblank(sensor, >> + VGXY61_FRAME_LENGTH_DEF - >> + sensor->current_mode->crop.height, >> + sensor->hdr); >> + >> + /* Update controls to reflect new mode */ >> + __v4l2_ctrl_s_ctrl_int64(sensor->pixel_rate_ctrl, get_pixel_rate(sensor)); >> + __v4l2_ctrl_modify_range(sensor->vblank_ctrl, sensor->vblank_min, >> + 0xffff - sensor->current_mode->crop.height, 1, >> + sensor->vblank); >> + __v4l2_ctrl_s_ctrl(sensor->vblank_ctrl, sensor->vblank); >> + __v4l2_ctrl_modify_range(sensor->expo_ctrl, sensor->expo_min, >> + sensor->expo_max, 1, >> + sensor->expo_long); >> + } >> + >> +out: >> + mutex_unlock(&sensor->lock); >> + >> + return ret; >> +} >> + >> +static int vgxy61_init_cfg(struct v4l2_subdev *sd, struct v4l2_subdev_state *sd_state) >> +{ >> + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); >> + struct v4l2_subdev_format fmt = { 0 }; >> + >> + sensor->current_mode = sensor->default_mode; >> + vgxy61_fill_framefmt(sensor, sensor->current_mode, &fmt.format, VGXY61_MEDIA_BUS_FMT_DEF); >> + >> + return vgxy61_set_fmt(sd, sd_state, &fmt); >> +} >> + >> +static int vgxy61_s_ctrl(struct v4l2_ctrl *ctrl) >> +{ >> + struct v4l2_subdev *sd = ctrl_to_sd(ctrl); >> + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); >> + int ret; >> + >> + switch (ctrl->id) { >> + case V4L2_CID_EXPOSURE: >> + ret = vgxy61_update_exposure(sensor, ctrl->val, sensor->hdr); >> + ctrl->val = sensor->expo_long; >> + break; >> + case V4L2_CID_ANALOGUE_GAIN: >> + ret = vgxy61_update_analog_gain(sensor, ctrl->val); >> + break; >> + case V4L2_CID_DIGITAL_GAIN: >> + ret = vgxy61_update_digital_gain(sensor, ctrl->val); >> + break; >> + case V4L2_CID_VFLIP: >> + case V4L2_CID_HFLIP: >> + if (sensor->streaming) { >> + ret = -EBUSY; >> + break; >> + } >> + if (ctrl->id == V4L2_CID_VFLIP) >> + sensor->vflip = ctrl->val; >> + if (ctrl->id == V4L2_CID_HFLIP) >> + sensor->hflip = ctrl->val; >> + ret = 0; >> + break; >> + case V4L2_CID_TEST_PATTERN: >> + ret = vgxy61_update_patgen(sensor, ctrl->val); >> + break; >> + case V4L2_CID_STVGXY61_HDR: >> + ret = vgxy61_update_hdr(sensor, ctrl->val); >> + /* Update vblank and exposure controls to match new hdr */ >> + __v4l2_ctrl_modify_range(sensor->vblank_ctrl, sensor->vblank_min, >> + 0xffff - sensor->current_mode->crop.height, 1, >> + sensor->vblank); >> + __v4l2_ctrl_modify_range(sensor->expo_ctrl, sensor->expo_min, >> + sensor->expo_max, 1, sensor->expo_long); >> + break; >> + case V4L2_CID_VBLANK: >> + ret = vgxy61_update_vblank(sensor, ctrl->val, sensor->hdr); >> + /* Update exposure control to match new vblank */ >> + __v4l2_ctrl_modify_range(sensor->expo_ctrl, sensor->expo_min, >> + sensor->expo_max, 1, sensor->expo_long); >> + break; >> + default: >> + ret = -EINVAL; >> + break; >> + } >> + >> + return ret; >> +} >> + >> +static const struct v4l2_ctrl_ops vgxy61_ctrl_ops = { >> + .s_ctrl = vgxy61_s_ctrl, >> +}; >> + >> +static const struct v4l2_ctrl_config vgxy61_hdr_ctrl = { >> + .ops = &vgxy61_ctrl_ops, >> + .id = V4L2_CID_STVGXY61_HDR, >> + .name = "HDR mode", >> + .type = V4L2_CTRL_TYPE_MENU, >> + .min = 0, >> + .max = ARRAY_SIZE(vgxy61_hdr_mode_menu) - 1, >> + .def = VGXY61_NO_HDR, >> + .qmenu = vgxy61_hdr_mode_menu, >> +}; >> + >> +static int vgxy61_init_controls(struct vgxy61_dev *sensor) >> +{ >> + const struct v4l2_ctrl_ops *ops = &vgxy61_ctrl_ops; >> + struct v4l2_ctrl_handler *hdl = &sensor->ctrl_handler; >> + struct v4l2_ctrl *ctrl; >> + int ret; >> + >> + v4l2_ctrl_handler_init(hdl, 16); >> + /* We can use our own mutex for the ctrl lock */ >> + hdl->lock = &sensor->lock; >> + /* Standard controls */ >> + v4l2_ctrl_new_std(hdl, ops, V4L2_CID_ANALOGUE_GAIN, 0, 0x1c, 1, sensor->analog_gain); >> + v4l2_ctrl_new_std(hdl, ops, V4L2_CID_DIGITAL_GAIN, 0, 0xfff, 1, sensor->digital_gain); >> + v4l2_ctrl_new_std_menu_items(hdl, ops, V4L2_CID_TEST_PATTERN, >> + ARRAY_SIZE(vgxy61_test_pattern_menu) - 1, 0, 0, >> + vgxy61_test_pattern_menu); >> + ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HBLANK, 0, sensor->line_length, 1, >> + sensor->line_length - sensor->current_mode->width); >> + ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; > > Note that ctrl may be NULL here. > > Same for the rest of similar cases below. > > You could move these after checking the handler error state. > Ah thank you. As I'm overriding ctrl each time I can't move the flag setting after the handler check. I'm adding a 'if (ctrl)' right before if it is ok for you. >> + ctrl = v4l2_ctrl_new_int_menu(hdl, ops, V4L2_CID_LINK_FREQ, ARRAY_SIZE(link_freq) - 1, 0, >> + link_freq); >> + ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; >> + /* Custom controls */ >> + v4l2_ctrl_new_custom(hdl, &vgxy61_hdr_ctrl, NULL); >> + /* Keep a pointer to these controls as we need to update them when setting the format */ >> + sensor->pixel_rate_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_PIXEL_RATE, 1, INT_MAX, 1, >> + get_pixel_rate(sensor)); >> + sensor->pixel_rate_ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; >> + sensor->expo_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_EXPOSURE, sensor->expo_min, >> + sensor->expo_max, 1, sensor->expo_long); >> + sensor->vblank_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VBLANK, sensor->vblank_min, >> + 0xffff - sensor->current_mode->crop.height, 1, >> + sensor->vblank); >> + sensor->vflip_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VFLIP, 0, 1, 1, sensor->vflip); >> + sensor->hflip_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HFLIP, 0, 1, 1, sensor->hflip); > > You don't seem to have the link frequency control. I suppose you should, as > well as have it in DT. > > <URL:https://hverkuil.home.xs4all.nl/spec/driver-api/camera-sensor.html#handling-clocks> > Are you referring to V4L2_CID_LINK_FREQ? I defined it above as a read only control as it is fixed for this sensor. >> + >> + if (hdl->error) { >> + ret = hdl->error; >> + goto free_ctrls; >> + } >> + >> + sensor->sd.ctrl_handler = hdl; >> + return 0; >> + >> +free_ctrls: >> + v4l2_ctrl_handler_free(hdl); >> + return ret; >> +} >> + >> +static const struct v4l2_subdev_video_ops vgxy61_video_ops = { >> + .s_stream = vgxy61_s_stream, >> +}; >> + >> +static const struct v4l2_subdev_pad_ops vgxy61_pad_ops = { >> + .init_cfg = vgxy61_init_cfg, >> + .enum_mbus_code = vgxy61_enum_mbus_code, >> + .get_fmt = vgxy61_get_fmt, >> + .set_fmt = vgxy61_set_fmt, >> + .get_selection = vgxy61_get_selection, >> + .enum_frame_size = vgxy61_enum_frame_size, >> +}; >> + >> +static const struct v4l2_subdev_ops vgxy61_subdev_ops = { >> + .video = &vgxy61_video_ops, >> + .pad = &vgxy61_pad_ops, >> +}; >> + >> +static const struct media_entity_operations vgxy61_subdev_entity_ops = { >> + .link_validate = v4l2_subdev_link_validate, >> +}; >> + >> +/* Set phy polarities */ >> +static int vgxy61_tx_from_ep(struct vgxy61_dev *sensor, struct fwnode_handle *endpoint) >> +{ >> + struct v4l2_fwnode_endpoint ep = { .bus_type = V4L2_MBUS_CSI2_DPHY }; >> + struct i2c_client *client = sensor->i2c_client; >> + u32 log2phy[VGXY61_NB_POLARITIES] = {~0, ~0, ~0, ~0, ~0}; >> + u32 phy2log[VGXY61_NB_POLARITIES] = {~0, ~0, ~0, ~0, ~0}; >> + int polarities[VGXY61_NB_POLARITIES] = {0, 0, 0, 0, 0}; >> + int l_nb; >> + int p, l; >> + int ret; >> + int i; >> + >> + ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &ep); >> + if (ret) >> + goto error_alloc; > > Just return ret here, no need for a label. > >> + >> + l_nb = ep.bus.mipi_csi2.num_data_lanes; >> + if (l_nb != 1 && l_nb != 2 && l_nb != 4) { >> + dev_err(&client->dev, "invalid data lane number %d\n", l_nb); >> + goto error_ep; >> + } >> + >> + /* Build log2phy, phy2log and polarities from ep info */ >> + log2phy[0] = ep.bus.mipi_csi2.clock_lane; >> + phy2log[log2phy[0]] = 0; >> + for (l = 1; l < l_nb + 1; l++) { > > Please use unsigned type for array indices and other values that are > obviously not going to be negative. > >> + log2phy[l] = ep.bus.mipi_csi2.data_lanes[l - 1]; >> + phy2log[log2phy[l]] = l; >> + } >> + /* >> + * Then fill remaining slots for every physical slot to have something valid for hardware >> + * stuff. >> + */ >> + for (p = 0; p < VGXY61_NB_POLARITIES; p++) { >> + if (phy2log[p] != ~0) >> + continue; >> + phy2log[p] = l; >> + log2phy[l] = p; >> + l++; >> + } >> + for (l = 0; l < l_nb + 1; l++) >> + polarities[l] = ep.bus.mipi_csi2.lane_polarities[l]; >> + >> + if (log2phy[0] != 0) { >> + dev_err(&client->dev, "clk lane must be map to physical lane 0\n"); >> + goto error_ep; >> + } >> + sensor->oif_ctrl = (polarities[4] << 15) + ((phy2log[4] - 1) << 13) + >> + (polarities[3] << 12) + ((phy2log[3] - 1) << 10) + >> + (polarities[2] << 9) + ((phy2log[2] - 1) << 7) + >> + (polarities[1] << 6) + ((phy2log[1] - 1) << 4) + >> + (polarities[0] << 3) + >> + l_nb; >> + sensor->nb_of_lane = l_nb; >> + >> + dev_dbg(&client->dev, "tx uses %d lanes", l_nb); >> + for (i = 0; i < 5; i++) { >> + dev_dbg(&client->dev, "log2phy[%d] = %d\n", i, log2phy[i]); >> + dev_dbg(&client->dev, "phy2log[%d] = %d\n", i, phy2log[i]); >> + dev_dbg(&client->dev, "polarity[%d] = %d\n", i, polarities[i]); >> + } >> + dev_dbg(&client->dev, "oif_ctrl = 0x%04x\n", sensor->oif_ctrl); >> + >> + v4l2_fwnode_endpoint_free(&ep); >> + >> + return 0; >> + >> +error_ep: >> + v4l2_fwnode_endpoint_free(&ep); >> +error_alloc: >> + >> + return -EINVAL; >> +} >> + >> +static int vgxy61_configure(struct vgxy61_dev *sensor) >> +{ >> + struct i2c_client *client = sensor->i2c_client; >> + int sensor_freq; >> + unsigned int prediv; >> + unsigned int mult; >> + int line_length; >> + int ret = 0; >> + >> + compute_pll_parameters_by_freq(sensor->clk_freq, &prediv, &mult); >> + sensor_freq = (mult * sensor->clk_freq) / prediv; >> + /* Frequency to data rate is 1:1 ratio for MIPI */ >> + sensor->data_rate_in_mbps = sensor_freq; >> + /* Video timing ISP path (pixel clock) requires 804/5 mhz = 160 mhz */ >> + sensor->pclk = sensor_freq / 5; >> + >> + /* Cache line_length value */ >> + line_length = vgxy61_read_reg(sensor, VGXY61_REG_LINE_LENGTH); >> + if (line_length < 0) > > What does line_length < 0 actually signify? An error? > Yes, I meant to return 'line_length' not 'ret', thanks. >> + return ret; >> + sensor->line_length = line_length; >> + /* Configure clocks */ >> + vgxy61_write_reg(sensor, VGXY61_REG_EXT_CLOCK, sensor->clk_freq, &ret); >> + vgxy61_write_reg(sensor, VGXY61_REG_CLK_PLL_PREDIV, prediv, &ret); >> + vgxy61_write_reg(sensor, VGXY61_REG_CLK_SYS_PLL_MULT, mult, &ret); >> + /* Configure interface */ >> + vgxy61_write_reg(sensor, VGXY61_REG_OIF_CTRL, sensor->oif_ctrl, &ret); >> + /* Disable pwm compression */ >> + vgxy61_write_reg(sensor, VGXY61_REG_FRAME_CONTENT_CTRL, 0, &ret); >> + /* Disable asil lines */ >> + vgxy61_write_reg(sensor, VGXY61_REG_BYPASS_CTRL, 4, &ret); >> + if (ret) >> + return ret; >> + /* Set gpios polarity according to device tree value */ >> + vgxy61_update_gpios_strobe_polarity(sensor, sensor->gpios_polarity); >> + /* Slave mode */ >> + ret = vgxy61_write_reg(sensor, VGXY61_REG_VT_CTRL, sensor->slave_mode, &ret); >> + /* Set pattern generator solid to middle value */ >> + vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_LONG_DATA_GR, 0x800, &ret); >> + vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_LONG_DATA_R, 0x800, &ret); >> + vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_LONG_DATA_B, 0x800, &ret); >> + vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_LONG_DATA_GB, 0x800, &ret); >> + vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_SHORT_DATA_GR, 0x800, &ret); >> + vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_SHORT_DATA_R, 0x800, &ret); >> + vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_SHORT_DATA_B, 0x800, &ret); >> + vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_SHORT_DATA_GB, 0x800, &ret); >> + if (ret) >> + return ret; >> + >> + dev_dbg(&client->dev, "clock prediv = %d\n", prediv); >> + dev_dbg(&client->dev, "clock mult = %d\n", mult); >> + dev_dbg(&client->dev, "data rate = %d mbps\n", sensor->data_rate_in_mbps); >> + >> + return 0; >> +} >> + >> +static int vgxy61_patch(struct vgxy61_dev *sensor) >> +{ >> + struct i2c_client *client = sensor->i2c_client; >> + u16 patch; >> + int ret; >> + >> + ret = vgxy61_write_array(sensor, VGXY61_REG_FWPATCH_START_ADDR, sizeof(patch_array), >> + patch_array); >> + if (ret) >> + return ret; >> + >> + ret = vgxy61_write_reg(sensor, VGXY61_REG_STBY, 0x10, NULL); >> + if (ret) >> + return ret; >> + >> + ret = vgxy61_poll_reg(sensor, VGXY61_REG_STBY, 0, VGXY61_TIMEOUT_MS); >> + if (ret) >> + return ret; >> + >> + patch = vgxy61_read_reg(sensor, VGXY61_REG_FWPATCH_REVISION); >> + if (patch < 0) >> + return patch; >> + >> + if (patch != (VGXY61_FWPATCH_REVISION_MAJOR << 12) + >> + (VGXY61_FWPATCH_REVISION_MINOR << 8) + >> + VGXY61_FWPATCH_REVISION_MICRO) { >> + dev_err(&client->dev, "bad patch version expected %d.%d.%d got %d.%d.%d\n", >> + VGXY61_FWPATCH_REVISION_MAJOR, >> + VGXY61_FWPATCH_REVISION_MINOR, >> + VGXY61_FWPATCH_REVISION_MICRO, >> + patch >> 12, (patch >> 8) & 0x0f, patch & 0xff); >> + return -ENODEV; >> + } >> + dev_dbg(&client->dev, "patch %d.%d.%d applied\n", >> + patch >> 12, (patch >> 8) & 0x0f, patch & 0xff); >> + >> + return 0; >> +} >> + >> +static int vgxy61_detect_cut_version(struct vgxy61_dev *sensor) >> +{ >> + struct i2c_client *client = sensor->i2c_client; >> + u16 device_rev; >> + >> + device_rev = vgxy61_read_reg(sensor, VGXY61_REG_REVISION); >> + if (device_rev < 0) >> + return device_rev; >> + >> + switch (device_rev >> 8) { >> + case 0xA: >> + dev_dbg(&client->dev, "Cut1 detected\n"); >> + dev_err(&client->dev, "Cut1 not supported by this driver\n"); >> + return -ENODEV; >> + case 0xB: >> + dev_dbg(&client->dev, "Cut2 detected\n"); >> + return 0; >> + case 0xC: >> + dev_dbg(&client->dev, "Cut3 detected\n"); >> + return 0; >> + default: >> + dev_err(&client->dev, "Unable to detect cut version\n"); >> + return -ENODEV; >> + } >> +} >> + >> +static int vgxy61_detect(struct vgxy61_dev *sensor) >> +{ >> + struct i2c_client *client = sensor->i2c_client; >> + u16 id = 0; >> + int ret; >> + u8 st; >> + >> + id = vgxy61_read_reg(sensor, VGXY61_REG_MODEL_ID); >> + if (id < 0) >> + return id; >> + if (id != VG5661_MODEL_ID && id != VG5761_MODEL_ID) { >> + dev_warn(&client->dev, "Unsupported sensor id %x\n", id); >> + return -ENODEV; >> + } >> + dev_dbg(&client->dev, "detected sensor id = 0x%04x\n", id); >> + sensor->id = id; >> + >> + ret = vgxy61_wait_state(sensor, VGXY61_SYSTEM_FSM_SW_STBY, VGXY61_TIMEOUT_MS); >> + if (ret) >> + return ret; >> + >> + st = vgxy61_read_reg(sensor, VGXY61_REG_NVM); >> + if (st < 0) >> + return st; >> + if (st != VGXY61_NVM_OK) >> + dev_warn(&client->dev, "Bad nvm state got %d\n", st); >> + >> + /* Detect cut version */ >> + ret = vgxy61_detect_cut_version(sensor); >> + if (ret) >> + return ret; >> + >> + return 0; >> +} >> + >> +/* Power/clock management functions */ >> +static int vgxy61_power_on(struct device *dev) >> +{ >> + struct i2c_client *client = to_i2c_client(dev); >> + struct v4l2_subdev *sd = i2c_get_clientdata(client); >> + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); >> + int ret; >> + >> + ret = vgxy61_get_regulators(sensor); > > This should take place during probe only. > >> + if (ret) { >> + dev_err(&client->dev, "failed to get regulators %d\n", ret); >> + return ret; >> + } >> + >> + ret = regulator_bulk_enable(VGXY61_NUM_SUPPLIES, sensor->supplies); >> + if (ret) { >> + dev_err(&client->dev, "failed to enable regulators %d\n", ret); >> + return ret; >> + } >> + >> + ret = clk_prepare_enable(sensor->xclk); >> + if (ret) { >> + dev_err(&client->dev, "failed to enable clock %d\n", ret); >> + goto disable_bulk; >> + } >> + >> + /* Apply reset sequence */ >> + if (sensor->reset_gpio) { >> + ret = vgxy61_apply_reset(sensor); >> + if (ret) { >> + dev_err(&client->dev, "sensor reset failed %d\n", ret); >> + goto disable_clock; >> + } >> + } >> + >> + ret = vgxy61_patch(sensor); >> + if (ret) { >> + dev_err(&client->dev, "sensor patch failed %d\n", ret); >> + goto disable_clock; >> + } >> + >> + ret = vgxy61_configure(sensor); >> + if (ret) { >> + dev_err(&client->dev, "sensor configuration failed %d\n", ret); >> + goto disable_clock; >> + } >> + >> + return 0; >> + >> +disable_clock: >> + clk_disable_unprepare(sensor->xclk); >> +disable_bulk: >> + regulator_bulk_disable(VGXY61_NUM_SUPPLIES, sensor->supplies); >> + >> + return ret; >> +} >> + >> +static int vgxy61_power_off(struct device *dev) >> +{ >> + struct i2c_client *client = to_i2c_client(dev); >> + struct v4l2_subdev *sd = i2c_get_clientdata(client); >> + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); >> + >> + clk_disable_unprepare(sensor->xclk); >> + regulator_bulk_disable(VGXY61_NUM_SUPPLIES, sensor->supplies); >> + return 0; >> +} >> + >> +static void vgxy61_fill_sensor_param(struct vgxy61_dev *sensor) >> +{ >> + if (sensor->id == VG5761_MODEL_ID) { >> + sensor->sensor_width = VGX761_WIDTH; >> + sensor->sensor_height = VGX761_HEIGHT; >> + sensor->sensor_modes = vgx761_mode_data; >> + sensor->sensor_modes_nb = ARRAY_SIZE(vgx761_mode_data); >> + sensor->default_mode = &vgx761_mode_data[VGX761_DEFAULT_MODE]; >> + sensor->rot_term = VGX761_SHORT_ROT_TERM; >> + } else if (sensor->id == VG5661_MODEL_ID) { >> + sensor->sensor_width = VGX661_WIDTH; >> + sensor->sensor_height = VGX661_HEIGHT; >> + sensor->sensor_modes = vgx661_mode_data; >> + sensor->sensor_modes_nb = ARRAY_SIZE(vgx661_mode_data); >> + sensor->default_mode = &vgx661_mode_data[VGX661_DEFAULT_MODE]; >> + sensor->rot_term = VGX661_SHORT_ROT_TERM; >> + } else { >> + /* Should never happen */ >> + WARN_ON(true); >> + } >> + sensor->current_mode = sensor->default_mode; >> + sensor->vblank = sensor->frame_length - sensor->current_mode->crop.height; >> +} >> + >> +static int vgxy61_probe(struct i2c_client *client) >> +{ >> + struct device *dev = &client->dev; >> + struct fwnode_handle *endpoint; >> + struct vgxy61_dev *sensor; >> + int ret; >> + >> + sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL); >> + if (!sensor) >> + return -ENOMEM; >> + >> + sensor->i2c_client = client; >> + sensor->streaming = false; >> + sensor->hdr = VGXY61_NO_HDR; >> + sensor->expo_long = 200; >> + sensor->expo_short = 0; >> + sensor->hflip = false; >> + sensor->vflip = false; >> + sensor->analog_gain = 0; >> + sensor->digital_gain = 256; >> + >> + endpoint = fwnode_graph_get_next_endpoint(of_fwnode_handle(dev->of_node), NULL); >> + if (!endpoint) { >> + dev_err(dev, "endpoint node not found\n"); >> + return -EINVAL; >> + } >> + >> + ret = vgxy61_tx_from_ep(sensor, endpoint); >> + fwnode_handle_put(endpoint); >> + if (ret) { >> + dev_err(dev, "Failed to parse endpoint %d\n", ret); >> + return ret; >> + } >> + >> + sensor->xclk = devm_clk_get(dev, NULL); >> + if (IS_ERR(sensor->xclk)) { >> + dev_err(dev, "failed to get xclk\n"); >> + return PTR_ERR(sensor->xclk); >> + } >> + sensor->clk_freq = clk_get_rate(sensor->xclk); >> + if (sensor->clk_freq < 6 * HZ_PER_MHZ || sensor->clk_freq > 27 * HZ_PER_MHZ) { >> + dev_err(dev, "Only 6Mhz-27Mhz clock range supported. provide %lu MHz\n", >> + sensor->clk_freq / HZ_PER_MHZ); >> + return -EINVAL; >> + } >> + sensor->gpios_polarity = of_property_read_bool(dev->of_node, "invert-gpios-polarity"); >> + sensor->slave_mode = of_property_read_bool(dev->of_node, "slave-mode"); > > What does this actually do? > > On parallel bus the slave mode tells the synchronisation signals originate > from the receiver. On CSI-2 there are no specific synchronisation signals. > We discussed about this on v1 a while ago: https://lore.kernel.org/all/c610a2c9-31b1-1950-00fa-a6b3fd3517a1@foss.st.com/ Tell me if there is any changes to be made, or maybe some documentation to clarify? >> + >> + v4l2_i2c_subdev_init(&sensor->sd, client, &vgxy61_subdev_ops); >> + sensor->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; >> + sensor->pad.flags = MEDIA_PAD_FL_SOURCE; >> + sensor->sd.entity.ops = &vgxy61_subdev_entity_ops; >> + sensor->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR; >> + >> + /* Request optional reset pin */ >> + sensor->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); >> + >> + ret = vgxy61_power_on(dev); >> + if (ret) >> + return ret; >> + >> + ret = vgxy61_detect(sensor); >> + if (ret) { >> + dev_err(&client->dev, "sensor detect failed %d\n", ret); >> + return ret; >> + } >> + >> + vgxy61_fill_sensor_param(sensor); >> + vgxy61_fill_framefmt(sensor, sensor->current_mode, &sensor->fmt, VGXY61_MEDIA_BUS_FMT_DEF); >> + >> + /* HDR mode */ >> + ret = vgxy61_update_hdr(sensor, sensor->hdr); >> + if (ret) >> + return ret; >> + >> + mutex_init(&sensor->lock); >> + >> + ret = vgxy61_init_controls(sensor); >> + if (ret) { >> + dev_err(&client->dev, "controls initialization failed %d\n", ret); >> + goto error_power_off; >> + } >> + >> + ret = media_entity_pads_init(&sensor->sd.entity, 1, &sensor->pad); >> + if (ret) { >> + dev_err(&client->dev, "pads init failed %d\n", ret); >> + goto error_handler_free; >> + } >> + >> + /* Enable runtime PM and turn off the device */ >> + pm_runtime_set_active(dev); >> + pm_runtime_enable(dev); >> + pm_runtime_idle(dev); >> + >> + ret = v4l2_async_register_subdev(&sensor->sd); >> + if (ret) { >> + dev_err(&client->dev, "async subdev register failed %d\n", ret); >> + goto error_pm_runtime; >> + } >> + >> + pm_runtime_set_autosuspend_delay(&client->dev, 1000); >> + pm_runtime_use_autosuspend(&client->dev); >> + pm_runtime_put_autosuspend(&client->dev); >> + >> + dev_dbg(&client->dev, "vgxy61 probe successfully\n"); >> + >> + return 0; >> + >> +error_pm_runtime: >> + pm_runtime_disable(&client->dev); >> + pm_runtime_set_suspended(&client->dev); >> + media_entity_cleanup(&sensor->sd.entity); >> +error_handler_free: >> + v4l2_ctrl_handler_free(sensor->sd.ctrl_handler); >> + mutex_destroy(&sensor->lock); >> +error_power_off: >> + vgxy61_power_off(dev); >> + >> + return ret; >> +} >> + >> +static int vgxy61_remove(struct i2c_client *client) >> +{ >> + struct v4l2_subdev *sd = i2c_get_clientdata(client); >> + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); >> + >> + v4l2_async_unregister_subdev(&sensor->sd); >> + mutex_destroy(&sensor->lock); >> + media_entity_cleanup(&sensor->sd.entity); >> + >> + pm_runtime_disable(&client->dev); >> + if (!pm_runtime_status_suspended(&client->dev)) >> + vgxy61_power_off(&client->dev); >> + pm_runtime_set_suspended(&client->dev); >> + >> + return 0; >> +} >> + >> +static const struct of_device_id vgxy61_dt_ids[] = { >> + { .compatible = "st,st-vgxy61" }, >> + { /* sentinel */ } >> +}; >> +MODULE_DEVICE_TABLE(of, vgxy61_dt_ids); >> + >> +static const struct dev_pm_ops vgxy61_pm_ops = { >> + SET_RUNTIME_PM_OPS(vgxy61_power_off, vgxy61_power_on, NULL) >> +}; >> + >> +static struct i2c_driver vgxy61_i2c_driver = { >> + .driver = { >> + .name = "st-vgxy61", >> + .of_match_table = vgxy61_dt_ids, >> + .pm = &vgxy61_pm_ops, >> + }, >> + .probe_new = vgxy61_probe, >> + .remove = vgxy61_remove, >> +}; >> + >> +module_i2c_driver(vgxy61_i2c_driver); >> + >> +MODULE_AUTHOR("Benjamin Mugnier <benjamin.mugnier@foss.st.com>"); >> +MODULE_AUTHOR("Mickael Guene <mickael.guene@st.com>"); >> +MODULE_AUTHOR("Sylvain Petinot <sylvain.petinot@foss.st.com>"); >> +MODULE_DESCRIPTION("VGXY61 camera subdev driver"); >> +MODULE_LICENSE("GPL"); >
Hi Benjamin, On Fri, Aug 26, 2022 at 04:12:22PM +0200, Benjamin MUGNIER wrote: > > > + int rot_term; > > > > rot_term value is always the same. No need for a field here for that. > > > > It changes according to the sensor's model id and could be either > VGX761_SHORT_ROT_TERM or VGX661_SHORT_ROT_TERM. See the > vgxy61_fill_sensor_param function. Ah, thanks for the explanation. I somehow had missed that earlier. > > > + struct i2c_client *client = sensor->i2c_client; > > > + /* Correction factor for time required between 2 lines */ > > > + const int mipi_margin = 1056; > > > > What does this value signify? Is it really constant or somehow dependent by > > the configuration set by the driver? > > It is a simplification of time needed to send short packets and for the MIPI > to add transition times (EoT, LPS, and SoT packet delimiters) needed by the > protocol to go in low power between 2 packets of data. This is a mipi IP > constant for the sensor. Ok. A comment elaborating this as above would be nice. > > > + ctrl = v4l2_ctrl_new_int_menu(hdl, ops, V4L2_CID_LINK_FREQ, ARRAY_SIZE(link_freq) - 1, 0, > > > + link_freq); > > > + ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; > > > + /* Custom controls */ > > > + v4l2_ctrl_new_custom(hdl, &vgxy61_hdr_ctrl, NULL); > > > + /* Keep a pointer to these controls as we need to update them when setting the format */ > > > + sensor->pixel_rate_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_PIXEL_RATE, 1, INT_MAX, 1, > > > + get_pixel_rate(sensor)); > > > + sensor->pixel_rate_ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; > > > + sensor->expo_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_EXPOSURE, sensor->expo_min, > > > + sensor->expo_max, 1, sensor->expo_long); > > > + sensor->vblank_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VBLANK, sensor->vblank_min, > > > + 0xffff - sensor->current_mode->crop.height, 1, > > > + sensor->vblank); > > > + sensor->vflip_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VFLIP, 0, 1, 1, sensor->vflip); > > > + sensor->hflip_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HFLIP, 0, 1, 1, sensor->hflip); > > > > You don't seem to have the link frequency control. I suppose you should, as > > well as have it in DT. > > > > <URL:https://hverkuil.home.xs4all.nl/spec/driver-api/camera-sensor.html#handling-clocks> > > > > Are you referring to V4L2_CID_LINK_FREQ? I defined it above as a read only > control as it is fixed for this sensor. Ah, it was of course the first control. Anyway, in general case, the frequencies that can be used on the board should come from DT, as using other frequencies may interfere with other devices in the same system. ... > > > + sensor->slave_mode = of_property_read_bool(dev->of_node, "slave-mode"); > > > > What does this actually do? > > > > On parallel bus the slave mode tells the synchronisation signals originate > > from the receiver. On CSI-2 there are no specific synchronisation signals. > > > > We discussed about this on v1 a while ago: > > https://lore.kernel.org/all/c610a2c9-31b1-1950-00fa-a6b3fd3517a1@foss.st.com/ > > Tell me if there is any changes to be made, or maybe some documentation to > clarify? Ah, yes, I hadn't had time to read that reply yet. :-( I think we should have a generic solution for this. You may not have two similar kinds of sensors you're synchronising this way. One option could be to drop this functionality from the driver now to get it in sooner. Something to consider: - how to convey where the signal comes from (phandle), - capture start signal polarity, - whether it's input/output and - pull-down / pull-up configuration? These are similar to what GPIOs have. Hardware support may vary of course.
Hi Sakari, Thanks a lot for your answer. On 9/25/22 13:32, Sakari Ailus wrote: > Hi Benjamin, > > On Fri, Aug 26, 2022 at 04:12:22PM +0200, Benjamin MUGNIER wrote: >>>> + int rot_term; >>> >>> rot_term value is always the same. No need for a field here for that. >>> >> >> It changes according to the sensor's model id and could be either >> VGX761_SHORT_ROT_TERM or VGX661_SHORT_ROT_TERM. See the >> vgxy61_fill_sensor_param function. > > Ah, thanks for the explanation. I somehow had missed that earlier. > >>>> + struct i2c_client *client = sensor->i2c_client; >>>> + /* Correction factor for time required between 2 lines */ >>>> + const int mipi_margin = 1056; >>> >>> What does this value signify? Is it really constant or somehow dependent by >>> the configuration set by the driver? >> >> It is a simplification of time needed to send short packets and for the MIPI >> to add transition times (EoT, LPS, and SoT packet delimiters) needed by the >> protocol to go in low power between 2 packets of data. This is a mipi IP >> constant for the sensor. > > Ok. A comment elaborating this as above would be nice. > No problem, queued for v6. >>>> + ctrl = v4l2_ctrl_new_int_menu(hdl, ops, V4L2_CID_LINK_FREQ, ARRAY_SIZE(link_freq) - 1, 0, >>>> + link_freq); >>>> + ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; >>>> + /* Custom controls */ >>>> + v4l2_ctrl_new_custom(hdl, &vgxy61_hdr_ctrl, NULL); >>>> + /* Keep a pointer to these controls as we need to update them when setting the format */ >>>> + sensor->pixel_rate_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_PIXEL_RATE, 1, INT_MAX, 1, >>>> + get_pixel_rate(sensor)); >>>> + sensor->pixel_rate_ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; >>>> + sensor->expo_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_EXPOSURE, sensor->expo_min, >>>> + sensor->expo_max, 1, sensor->expo_long); >>>> + sensor->vblank_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VBLANK, sensor->vblank_min, >>>> + 0xffff - sensor->current_mode->crop.height, 1, >>>> + sensor->vblank); >>>> + sensor->vflip_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VFLIP, 0, 1, 1, sensor->vflip); >>>> + sensor->hflip_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HFLIP, 0, 1, 1, sensor->hflip); >>> >>> You don't seem to have the link frequency control. I suppose you should, as >>> well as have it in DT. >>> >>> <URL:https://hverkuil.home.xs4all.nl/spec/driver-api/camera-sensor.html#handling-clocks> >>> >> >> Are you referring to V4L2_CID_LINK_FREQ? I defined it above as a read only >> control as it is fixed for this sensor. > > Ah, it was of course the first control. > > Anyway, in general case, the frequencies that can be used on the board > should come from DT, as using other frequencies may interfere with other > devices in the same system. Ah I just remembered we talked about this with Laurent on v2: https://lore.kernel.org/linux-media/YmA5eTOoGNEbMyKB@pendragon.ideasonboard.com/ Here is the relevant part: >>> The link frequencies should come from DT. >> >> This is a hard requirement for this sensor. This value can not be >> modified and must be 402MHz. >> >> I see the ov9734.c does something similar. Is adding this value to the >> device tree really mandatory? > > I suppose it's fine then. Sakari, what do you think ? With this extra information, should I move the link frequency to the device tree or is it fine this way? > > ... > >>>> + sensor->slave_mode = of_property_read_bool(dev->of_node, "slave-mode"); >>> >>> What does this actually do? >>> >>> On parallel bus the slave mode tells the synchronisation signals originate >>> from the receiver. On CSI-2 there are no specific synchronisation signals. >>> >> >> We discussed about this on v1 a while ago: >> >> https://lore.kernel.org/all/c610a2c9-31b1-1950-00fa-a6b3fd3517a1@foss.st.com/ >> >> Tell me if there is any changes to be made, or maybe some documentation to >> clarify? > > Ah, yes, I hadn't had time to read that reply yet. :-( > > I think we should have a generic solution for this. You may not have two > similar kinds of sensors you're synchronising this way. One option could be > to drop this functionality from the driver now to get it in sooner. > > Something to consider: > > - how to convey where the signal comes from (phandle), > > - capture start signal polarity, > > - whether it's input/output and > > - pull-down / pull-up configuration? > > These are similar to what GPIOs have. Hardware support may vary of course. > Yes, we discussed this in the media summit. I'll drop this for now. I'll come back to this once the first version of the driver is accepted, and if Dave is not faster than me on this topic ;)
Hi Benjamin, On Mon, Sep 26, 2022 at 11:28:59AM +0200, Benjamin MUGNIER wrote: > Hi Sakari, > > Thanks a lot for your answer. > > On 9/25/22 13:32, Sakari Ailus wrote: > > Hi Benjamin, > > > > On Fri, Aug 26, 2022 at 04:12:22PM +0200, Benjamin MUGNIER wrote: > >>>> + int rot_term; > >>> > >>> rot_term value is always the same. No need for a field here for that. > >>> > >> > >> It changes according to the sensor's model id and could be either > >> VGX761_SHORT_ROT_TERM or VGX661_SHORT_ROT_TERM. See the > >> vgxy61_fill_sensor_param function. > > > > Ah, thanks for the explanation. I somehow had missed that earlier. > > > >>>> + struct i2c_client *client = sensor->i2c_client; > >>>> + /* Correction factor for time required between 2 lines */ > >>>> + const int mipi_margin = 1056; > >>> > >>> What does this value signify? Is it really constant or somehow dependent by > >>> the configuration set by the driver? > >> > >> It is a simplification of time needed to send short packets and for the MIPI > >> to add transition times (EoT, LPS, and SoT packet delimiters) needed by the > >> protocol to go in low power between 2 packets of data. This is a mipi IP > >> constant for the sensor. > > > > Ok. A comment elaborating this as above would be nice. > > > > No problem, queued for v6. > > >>>> + ctrl = v4l2_ctrl_new_int_menu(hdl, ops, V4L2_CID_LINK_FREQ, ARRAY_SIZE(link_freq) - 1, 0, > >>>> + link_freq); > >>>> + ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; > >>>> + /* Custom controls */ > >>>> + v4l2_ctrl_new_custom(hdl, &vgxy61_hdr_ctrl, NULL); > >>>> + /* Keep a pointer to these controls as we need to update them when setting the format */ > >>>> + sensor->pixel_rate_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_PIXEL_RATE, 1, INT_MAX, 1, > >>>> + get_pixel_rate(sensor)); > >>>> + sensor->pixel_rate_ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; > >>>> + sensor->expo_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_EXPOSURE, sensor->expo_min, > >>>> + sensor->expo_max, 1, sensor->expo_long); > >>>> + sensor->vblank_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VBLANK, sensor->vblank_min, > >>>> + 0xffff - sensor->current_mode->crop.height, 1, > >>>> + sensor->vblank); > >>>> + sensor->vflip_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VFLIP, 0, 1, 1, sensor->vflip); > >>>> + sensor->hflip_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HFLIP, 0, 1, 1, sensor->hflip); > >>> > >>> You don't seem to have the link frequency control. I suppose you should, as > >>> well as have it in DT. > >>> > >>> <URL:https://hverkuil.home.xs4all.nl/spec/driver-api/camera-sensor.html#handling-clocks> > >>> > >> > >> Are you referring to V4L2_CID_LINK_FREQ? I defined it above as a read only > >> control as it is fixed for this sensor. > > > > Ah, it was of course the first control. > > > > Anyway, in general case, the frequencies that can be used on the board > > should come from DT, as using other frequencies may interfere with other > > devices in the same system. > > Ah I just remembered we talked about this with Laurent on v2: > > https://lore.kernel.org/linux-media/YmA5eTOoGNEbMyKB@pendragon.ideasonboard.com/ > > Here is the relevant part: > >>> The link frequencies should come from DT. > >> > >> This is a hard requirement for this sensor. This value can not be > >> modified and must be 402MHz. > >> > >> I see the ov9734.c does something similar. Is adding this value to the > >> device tree really mandatory? > > > > I suppose it's fine then. Sakari, what do you think ? > > With this extra information, should I move the link frequency to the device tree or is it fine this way? If the hardware can only use a single frequency (i.e. it's not a driver limitation), then sure, no need to put this in DT. > > > > > ... > > > >>>> + sensor->slave_mode = of_property_read_bool(dev->of_node, "slave-mode"); > >>> > >>> What does this actually do? > >>> > >>> On parallel bus the slave mode tells the synchronisation signals originate > >>> from the receiver. On CSI-2 there are no specific synchronisation signals. > >>> > >> > >> We discussed about this on v1 a while ago: > >> > >> https://lore.kernel.org/all/c610a2c9-31b1-1950-00fa-a6b3fd3517a1@foss.st.com/ > >> > >> Tell me if there is any changes to be made, or maybe some documentation to > >> clarify? > > > > Ah, yes, I hadn't had time to read that reply yet. :-( > > > > I think we should have a generic solution for this. You may not have two > > similar kinds of sensors you're synchronising this way. One option could be > > to drop this functionality from the driver now to get it in sooner. > > > > Something to consider: > > > > - how to convey where the signal comes from (phandle), > > > > - capture start signal polarity, > > > > - whether it's input/output and > > > > - pull-down / pull-up configuration? > > > > These are similar to what GPIOs have. Hardware support may vary of course. > > > > Yes, we discussed this in the media summit. > I'll drop this for now. > I'll come back to this once the first version of the driver is accepted, and if Dave is not faster than me on this topic ;) Sounds good.
Hi Sakari, On 9/26/22 11:42, Sakari Ailus wrote: [...] >>>>>> + ctrl = v4l2_ctrl_new_int_menu(hdl, ops, V4L2_CID_LINK_FREQ, ARRAY_SIZE(link_freq) - 1, 0, >>>>>> + link_freq); >>>>>> + ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; >>>>>> + /* Custom controls */ >>>>>> + v4l2_ctrl_new_custom(hdl, &vgxy61_hdr_ctrl, NULL); >>>>>> + /* Keep a pointer to these controls as we need to update them when setting the format */ >>>>>> + sensor->pixel_rate_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_PIXEL_RATE, 1, INT_MAX, 1, >>>>>> + get_pixel_rate(sensor)); >>>>>> + sensor->pixel_rate_ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; >>>>>> + sensor->expo_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_EXPOSURE, sensor->expo_min, >>>>>> + sensor->expo_max, 1, sensor->expo_long); >>>>>> + sensor->vblank_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VBLANK, sensor->vblank_min, >>>>>> + 0xffff - sensor->current_mode->crop.height, 1, >>>>>> + sensor->vblank); >>>>>> + sensor->vflip_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VFLIP, 0, 1, 1, sensor->vflip); >>>>>> + sensor->hflip_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HFLIP, 0, 1, 1, sensor->hflip); >>>>> >>>>> You don't seem to have the link frequency control. I suppose you should, as >>>>> well as have it in DT. >>>>> >>>>> <URL:https://hverkuil.home.xs4all.nl/spec/driver-api/camera-sensor.html#handling-clocks> >>>>> >>>> >>>> Are you referring to V4L2_CID_LINK_FREQ? I defined it above as a read only >>>> control as it is fixed for this sensor. >>> >>> Ah, it was of course the first control. >>> >>> Anyway, in general case, the frequencies that can be used on the board >>> should come from DT, as using other frequencies may interfere with other >>> devices in the same system. >> >> Ah I just remembered we talked about this with Laurent on v2: >> >> https://lore.kernel.org/linux-media/YmA5eTOoGNEbMyKB@pendragon.ideasonboard.com/ >> >> Here is the relevant part: >>>>> The link frequencies should come from DT. >>>> >>>> This is a hard requirement for this sensor. This value can not be >>>> modified and must be 402MHz. >>>> >>>> I see the ov9734.c does something similar. Is adding this value to the >>>> device tree really mandatory? >>> >>> I suppose it's fine then. Sakari, what do you think ? >> >> With this extra information, should I move the link frequency to the device tree or is it fine this way? > > If the hardware can only use a single frequency (i.e. it's not a driver > limitation), then sure, no need to put this in DT. > Yes this is hardware limited. I'll send v6 soon. Thanks a ton. >> >>> >>> ... >>> >>>>>> + sensor->slave_mode = of_property_read_bool(dev->of_node, "slave-mode"); >>>>> >>>>> What does this actually do? >>>>> >>>>> On parallel bus the slave mode tells the synchronisation signals originate >>>>> from the receiver. On CSI-2 there are no specific synchronisation signals. >>>>> >>>> >>>> We discussed about this on v1 a while ago: >>>> >>>> https://lore.kernel.org/all/c610a2c9-31b1-1950-00fa-a6b3fd3517a1@foss.st.com/ >>>> >>>> Tell me if there is any changes to be made, or maybe some documentation to >>>> clarify? >>> >>> Ah, yes, I hadn't had time to read that reply yet. :-( >>> >>> I think we should have a generic solution for this. You may not have two >>> similar kinds of sensors you're synchronising this way. One option could be >>> to drop this functionality from the driver now to get it in sooner. >>> >>> Something to consider: >>> >>> - how to convey where the signal comes from (phandle), >>> >>> - capture start signal polarity, >>> >>> - whether it's input/output and >>> >>> - pull-down / pull-up configuration? >>> >>> These are similar to what GPIOs have. Hardware support may vary of course. >>> >> >> Yes, we discussed this in the media summit. >> I'll drop this for now. >> I'll come back to this once the first version of the driver is accepted, and if Dave is not faster than me on this topic ;) > > Sounds good. >
Hi Sakari, On 9/26/22 11:42, Sakari Ailus wrote: [...] >>>>>> + ctrl = v4l2_ctrl_new_int_menu(hdl, ops, V4L2_CID_LINK_FREQ, ARRAY_SIZE(link_freq) - 1, 0, >>>>>> + link_freq); >>>>>> + ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; >>>>>> + /* Custom controls */ >>>>>> + v4l2_ctrl_new_custom(hdl, &vgxy61_hdr_ctrl, NULL); >>>>>> + /* Keep a pointer to these controls as we need to update them when setting the format */ >>>>>> + sensor->pixel_rate_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_PIXEL_RATE, 1, INT_MAX, 1, >>>>>> + get_pixel_rate(sensor)); >>>>>> + sensor->pixel_rate_ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; >>>>>> + sensor->expo_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_EXPOSURE, sensor->expo_min, >>>>>> + sensor->expo_max, 1, sensor->expo_long); >>>>>> + sensor->vblank_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VBLANK, sensor->vblank_min, >>>>>> + 0xffff - sensor->current_mode->crop.height, 1, >>>>>> + sensor->vblank); >>>>>> + sensor->vflip_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VFLIP, 0, 1, 1, sensor->vflip); >>>>>> + sensor->hflip_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HFLIP, 0, 1, 1, sensor->hflip); >>>>> >>>>> You don't seem to have the link frequency control. I suppose you should, as >>>>> well as have it in DT. >>>>> >>>>> <URL:https://hverkuil.home.xs4all.nl/spec/driver-api/camera-sensor.html#handling-clocks> >>>>> >>>> >>>> Are you referring to V4L2_CID_LINK_FREQ? I defined it above as a read only >>>> control as it is fixed for this sensor. >>> >>> Ah, it was of course the first control. >>> >>> Anyway, in general case, the frequencies that can be used on the board >>> should come from DT, as using other frequencies may interfere with other >>> devices in the same system. >> >> Ah I just remembered we talked about this with Laurent on v2: >> >> https://lore.kernel.org/linux-media/YmA5eTOoGNEbMyKB@pendragon.ideasonboard.com/ >> >> Here is the relevant part: >>>>> The link frequencies should come from DT. >>>> >>>> This is a hard requirement for this sensor. This value can not be >>>> modified and must be 402MHz. >>>> >>>> I see the ov9734.c does something similar. Is adding this value to the >>>> device tree really mandatory? >>> >>> I suppose it's fine then. Sakari, what do you think ? >> >> With this extra information, should I move the link frequency to the device tree or is it fine this way? > > If the hardware can only use a single frequency (i.e. it's not a driver > limitation), then sure, no need to put this in DT. > Yes this is hardware limited. I'll send v6 soon. Thanks a ton. >> >>> >>> ... >>> >>>>>> + sensor->slave_mode = of_property_read_bool(dev->of_node, "slave-mode"); >>>>> >>>>> What does this actually do? >>>>> >>>>> On parallel bus the slave mode tells the synchronisation signals originate >>>>> from the receiver. On CSI-2 there are no specific synchronisation signals. >>>>> >>>> >>>> We discussed about this on v1 a while ago: >>>> >>>> https://lore.kernel.org/all/c610a2c9-31b1-1950-00fa-a6b3fd3517a1@foss.st.com/ >>>> >>>> Tell me if there is any changes to be made, or maybe some documentation to >>>> clarify? >>> >>> Ah, yes, I hadn't had time to read that reply yet. :-( >>> >>> I think we should have a generic solution for this. You may not have two >>> similar kinds of sensors you're synchronising this way. One option could be >>> to drop this functionality from the driver now to get it in sooner. >>> >>> Something to consider: >>> >>> - how to convey where the signal comes from (phandle), >>> >>> - capture start signal polarity, >>> >>> - whether it's input/output and >>> >>> - pull-down / pull-up configuration? >>> >>> These are similar to what GPIOs have. Hardware support may vary of course. >>> >> >> Yes, we discussed this in the media summit. >> I'll drop this for now. >> I'll come back to this once the first version of the driver is accepted, and if Dave is not faster than me on this topic ;) > > Sounds good. >
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index fae2baabb773..ee7e8e921783 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -753,6 +753,16 @@ config VIDEO_SR030PC30 help This driver supports SR030PC30 VGA camera from Siliconfile +config VIDEO_ST_VGXY61 + tristate "ST VGXY61 sensor support" + depends on OF && GPIOLIB && VIDEO_DEV && I2C + select MEDIA_CONTROLLER + select VIDEO_V4L2_SUBDEV_API + select V4L2_FWNODE + help + This is a Video4Linux2 sensor driver for the ST VGXY61 + camera sensor. + config VIDEO_VS6624 tristate "ST VS6624 sensor support" depends on VIDEO_DEV && I2C diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index 3e1696963e7f..7f2362228484 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -116,6 +116,7 @@ obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o obj-$(CONFIG_VIDEO_SR030PC30) += sr030pc30.o obj-$(CONFIG_VIDEO_ST_MIPID02) += st-mipid02.o +obj-$(CONFIG_VIDEO_ST_VGXY61) += st-vgxy61.o obj-$(CONFIG_VIDEO_TC358743) += tc358743.o obj-$(CONFIG_VIDEO_TDA1997X) += tda1997x.o obj-$(CONFIG_VIDEO_TDA7432) += tda7432.o diff --git a/drivers/media/i2c/st-vgxy61.c b/drivers/media/i2c/st-vgxy61.c new file mode 100644 index 000000000000..bad8f10f88fe --- /dev/null +++ b/drivers/media/i2c/st-vgxy61.c @@ -0,0 +1,1940 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Driver for VGXY61 global shutter sensor family driver + * + * Copyright (C) 2022 STMicroelectronics SA + */ + +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/gpio/consumer.h> +#include <linux/i2c.h> +#include <linux/iopoll.h> +#include <linux/module.h> +#include <linux/pm_runtime.h> +#include <linux/regulator/consumer.h> +#include <linux/units.h> +#include <media/mipi-csi2.h> +#include <media/v4l2-async.h> +#include <media/v4l2-ctrls.h> +#include <media/v4l2-device.h> +#include <media/v4l2-fwnode.h> +#include <media/v4l2-subdev.h> +#include <uapi/linux/st-vgxy61.h> + +#define VGXY61_REG_8BIT(n) ((1 << 16) | (n)) +#define VGXY61_REG_16BIT(n) ((2 << 16) | (n)) +#define VGXY61_REG_32BIT(n) ((4 << 16) | (n)) +#define VGXY61_REG_SIZE_SHIFT 16 +#define VGXY61_REG_ADDR_MASK 0xffff + +#define VGXY61_REG_MODEL_ID VGXY61_REG_16BIT(0x0000) +#define VG5661_MODEL_ID 0x5661 +#define VG5761_MODEL_ID 0x5761 +#define VGXY61_REG_REVISION VGXY61_REG_16BIT(0x0002) +#define VGXY61_REG_FWPATCH_REVISION VGXY61_REG_16BIT(0x0014) +#define VGXY61_REG_FWPATCH_START_ADDR VGXY61_REG_8BIT(0x2000) +#define VGXY61_REG_SYSTEM_FSM VGXY61_REG_8BIT(0x0020) +#define VGXY61_SYSTEM_FSM_SW_STBY 0x03 +#define VGXY61_SYSTEM_FSM_STREAMING 0x04 +#define VGXY61_REG_NVM VGXY61_REG_8BIT(0x0023) +#define VGXY61_NVM_OK 0x04 +#define VGXY61_REG_STBY VGXY61_REG_8BIT(0x0201) +#define VGXY61_STBY_NO_REQ 0 +#define VGXY61_STBY_REQ_TMP_READ BIT(2) +#define VGXY61_REG_STREAMING VGXY61_REG_8BIT(0x0202) +#define VGXY61_STREAMING_NO_REQ 0 +#define VGXY61_STREAMING_REQ_STOP BIT(0) +#define VGXY61_STREAMING_REQ_START BIT(1) +#define VGXY61_REG_EXT_CLOCK VGXY61_REG_32BIT(0x0220) +#define VGXY61_REG_CLK_PLL_PREDIV VGXY61_REG_8BIT(0x0224) +#define VGXY61_REG_CLK_SYS_PLL_MULT VGXY61_REG_8BIT(0x0225) +#define VGXY61_REG_GPIO_0_CTRL VGXY61_REG_8BIT(0x0236) +#define VGXY61_REG_GPIO_1_CTRL VGXY61_REG_8BIT(0x0237) +#define VGXY61_REG_GPIO_2_CTRL VGXY61_REG_8BIT(0x0238) +#define VGXY61_REG_GPIO_3_CTRL VGXY61_REG_8BIT(0x0239) +#define VGXY61_REG_SIGNALS_POLARITY_CTRL VGXY61_REG_8BIT(0x023b) +#define VGXY61_REG_LINE_LENGTH VGXY61_REG_16BIT(0x0300) +#define VGXY61_REG_ORIENTATION VGXY61_REG_8BIT(0x0302) +#define VGXY61_REG_VT_CTRL VGXY61_REG_8BIT(0x0304) +#define VGXY61_REG_FORMAT_CTRL VGXY61_REG_8BIT(0x0305) +#define VGXY61_REG_OIF_CTRL VGXY61_REG_16BIT(0x0306) +#define VGXY61_REG_OIF_ROI0_CTRL VGXY61_REG_8BIT(0x030a) +#define VGXY61_REG_ROI0_START_H VGXY61_REG_16BIT(0x0400) +#define VGXY61_REG_ROI0_START_V VGXY61_REG_16BIT(0x0402) +#define VGXY61_REG_ROI0_END_H VGXY61_REG_16BIT(0x0404) +#define VGXY61_REG_ROI0_END_V VGXY61_REG_16BIT(0x0406) +#define VGXY61_REG_PATGEN_CTRL VGXY61_REG_32BIT(0x0440) +#define VGXY61_PATGEN_LONG_ENABLE BIT(16) +#define VGXY61_PATGEN_SHORT_ENABLE BIT(0) +#define VGXY61_PATGEN_LONG_TYPE_SHIFT 18 +#define VGXY61_PATGEN_SHORT_TYPE_SHIFT 4 +#define VGXY61_REG_FRAME_CONTENT_CTRL VGXY61_REG_8BIT(0x0478) +#define VGXY61_REG_COARSE_EXPOSURE_LONG VGXY61_REG_16BIT(0x0500) +#define VGXY61_REG_COARSE_EXPOSURE_SHORT VGXY61_REG_16BIT(0x0504) +#define VGXY61_REG_ANALOG_GAIN VGXY61_REG_8BIT(0x0508) +#define VGXY61_REG_DIGITAL_GAIN_LONG VGXY61_REG_16BIT(0x050a) +#define VGXY61_REG_DIGITAL_GAIN_SHORT VGXY61_REG_16BIT(0x0512) +#define VGXY61_REG_FRAME_LENGTH VGXY61_REG_16BIT(0x051a) +#define VGXY61_REG_SIGNALS_CTRL VGXY61_REG_16BIT(0x0522) +#define VGXY61_SIGNALS_GPIO_ID_SHIFT 4 +#define VGXY61_REG_READOUT_CTRL VGXY61_REG_8BIT(0x0530) +#define VGXY61_REG_HDR_CTRL VGXY61_REG_8BIT(0x0532) +#define VGXY61_REG_PATGEN_LONG_DATA_GR VGXY61_REG_16BIT(0x092c) +#define VGXY61_REG_PATGEN_LONG_DATA_R VGXY61_REG_16BIT(0x092e) +#define VGXY61_REG_PATGEN_LONG_DATA_B VGXY61_REG_16BIT(0x0930) +#define VGXY61_REG_PATGEN_LONG_DATA_GB VGXY61_REG_16BIT(0x0932) +#define VGXY61_REG_PATGEN_SHORT_DATA_GR VGXY61_REG_16BIT(0x0950) +#define VGXY61_REG_PATGEN_SHORT_DATA_R VGXY61_REG_16BIT(0x0952) +#define VGXY61_REG_PATGEN_SHORT_DATA_B VGXY61_REG_16BIT(0x0954) +#define VGXY61_REG_PATGEN_SHORT_DATA_GB VGXY61_REG_16BIT(0x0956) +#define VGXY61_REG_BYPASS_CTRL VGXY61_REG_8BIT(0x0a60) + +#define VGX661_WIDTH 1464 +#define VGX661_HEIGHT 1104 +#define VGX761_WIDTH 1944 +#define VGX761_HEIGHT 1204 +#define VGX661_DEFAULT_MODE 1 +#define VGX761_DEFAULT_MODE 1 +#define VGX661_SHORT_ROT_TERM 93 +#define VGX761_SHORT_ROT_TERM 90 +#define VGXY61_EXPOS_ROT_TERM 66 +#define VGXY61_WRITE_MULTIPLE_CHUNK_MAX 16 +#define VGXY61_NB_GPIOS 4 +#define VGXY61_NB_POLARITIES 5 +#define VGXY61_FRAME_LENGTH_DEF 1313 +#define VGXY61_MIN_FRAME_LENGTH 1288 +#define VGXY61_MIN_EXPOSURE 10 +#define VGXY61_HDR_LINEAR_RATIO 10 +#define VGXY61_TIMEOUT_MS 500 +#define VGXY61_MEDIA_BUS_FMT_DEF MEDIA_BUS_FMT_Y8_1X8 + +#define VGXY61_FWPATCH_REVISION_MAJOR 2 +#define VGXY61_FWPATCH_REVISION_MINOR 0 +#define VGXY61_FWPATCH_REVISION_MICRO 5 + +static const u8 patch_array[] = { + 0xbf, 0x00, 0x05, 0x20, 0x06, 0x01, 0xe0, 0xe0, 0x04, 0x80, 0xe6, 0x45, + 0xed, 0x6f, 0xfe, 0xff, 0x14, 0x80, 0x1f, 0x84, 0x10, 0x42, 0x05, 0x7c, + 0x01, 0xc4, 0x1e, 0x80, 0xb6, 0x42, 0x00, 0xe0, 0x1e, 0x82, 0x1e, 0xc0, + 0x93, 0xdd, 0xc3, 0xc1, 0x0c, 0x04, 0x00, 0xfa, 0x86, 0x0d, 0x70, 0xe1, + 0x04, 0x98, 0x15, 0x00, 0x28, 0xe0, 0x14, 0x02, 0x08, 0xfc, 0x15, 0x40, + 0x28, 0xe0, 0x98, 0x58, 0xe0, 0xef, 0x04, 0x98, 0x0e, 0x04, 0x00, 0xf0, + 0x15, 0x00, 0x28, 0xe0, 0x19, 0xc8, 0x15, 0x40, 0x28, 0xe0, 0xc6, 0x41, + 0xfc, 0xe0, 0x14, 0x80, 0x1f, 0x84, 0x14, 0x02, 0xa0, 0xfc, 0x1e, 0x80, + 0x14, 0x80, 0x14, 0x02, 0x80, 0xfb, 0x14, 0x02, 0xe0, 0xfc, 0x1e, 0x80, + 0x14, 0xc0, 0x1f, 0x84, 0x14, 0x02, 0xa4, 0xfc, 0x1e, 0xc0, 0x14, 0xc0, + 0x14, 0x02, 0x80, 0xfb, 0x14, 0x02, 0xe4, 0xfc, 0x1e, 0xc0, 0x0c, 0x0c, + 0x00, 0xf2, 0x93, 0xdd, 0x86, 0x00, 0xf8, 0xe0, 0x04, 0x80, 0xc6, 0x03, + 0x70, 0xe1, 0x0e, 0x84, 0x93, 0xdd, 0xc3, 0xc1, 0x0c, 0x04, 0x00, 0xfa, + 0x6b, 0x80, 0x06, 0x40, 0x6c, 0xe1, 0x04, 0x80, 0x09, 0x00, 0xe0, 0xe0, + 0x0b, 0xa1, 0x95, 0x84, 0x05, 0x0c, 0x1c, 0xe0, 0x86, 0x02, 0xf9, 0x60, + 0xe0, 0xcf, 0x78, 0x6e, 0x80, 0xef, 0x25, 0x0c, 0x18, 0xe0, 0x05, 0x4c, + 0x1c, 0xe0, 0x86, 0x02, 0xf9, 0x60, 0xe0, 0xcf, 0x0b, 0x84, 0xd8, 0x6d, + 0x80, 0xef, 0x05, 0x4c, 0x18, 0xe0, 0x04, 0xd8, 0x0b, 0xa5, 0x95, 0x84, + 0x05, 0x0c, 0x2c, 0xe0, 0x06, 0x02, 0x01, 0x60, 0xe0, 0xce, 0x18, 0x6d, + 0x80, 0xef, 0x25, 0x0c, 0x30, 0xe0, 0x05, 0x4c, 0x2c, 0xe0, 0x06, 0x02, + 0x01, 0x60, 0xe0, 0xce, 0x0b, 0x84, 0x78, 0x6c, 0x80, 0xef, 0x05, 0x4c, + 0x30, 0xe0, 0x0c, 0x0c, 0x00, 0xf2, 0x93, 0xdd, 0x46, 0x01, 0x70, 0xe1, + 0x08, 0x80, 0x0b, 0xa1, 0x08, 0x5c, 0x00, 0xda, 0x06, 0x01, 0x68, 0xe1, + 0x04, 0x80, 0x4a, 0x40, 0x84, 0xe0, 0x08, 0x5c, 0x00, 0x9a, 0x06, 0x01, + 0xe0, 0xe0, 0x04, 0x80, 0x15, 0x00, 0x60, 0xe0, 0x19, 0xc4, 0x15, 0x40, + 0x60, 0xe0, 0x15, 0x00, 0x78, 0xe0, 0x19, 0xc4, 0x15, 0x40, 0x78, 0xe0, + 0x93, 0xdd, 0xc3, 0xc1, 0x46, 0x01, 0x70, 0xe1, 0x08, 0x80, 0x0b, 0xa1, + 0x08, 0x5c, 0x00, 0xda, 0x06, 0x01, 0x68, 0xe1, 0x04, 0x80, 0x4a, 0x40, + 0x84, 0xe0, 0x08, 0x5c, 0x00, 0x9a, 0x06, 0x01, 0xe0, 0xe0, 0x14, 0x80, + 0x25, 0x02, 0x54, 0xe0, 0x29, 0xc4, 0x25, 0x42, 0x54, 0xe0, 0x24, 0x80, + 0x35, 0x04, 0x6c, 0xe0, 0x39, 0xc4, 0x35, 0x44, 0x6c, 0xe0, 0x25, 0x02, + 0x64, 0xe0, 0x29, 0xc4, 0x25, 0x42, 0x64, 0xe0, 0x04, 0x80, 0x15, 0x00, + 0x7c, 0xe0, 0x19, 0xc4, 0x15, 0x40, 0x7c, 0xe0, 0x93, 0xdd, 0xc3, 0xc1, + 0x4c, 0x04, 0x7c, 0xfa, 0x86, 0x40, 0x98, 0xe0, 0x14, 0x80, 0x1b, 0xa1, + 0x06, 0x00, 0x00, 0xc0, 0x08, 0x42, 0x38, 0xdc, 0x08, 0x64, 0xa0, 0xef, + 0x86, 0x42, 0x3c, 0xe0, 0x68, 0x49, 0x80, 0xef, 0x6b, 0x80, 0x78, 0x53, + 0xc8, 0xef, 0xc6, 0x54, 0x6c, 0xe1, 0x7b, 0x80, 0xb5, 0x14, 0x0c, 0xf8, + 0x05, 0x14, 0x14, 0xf8, 0x1a, 0xac, 0x8a, 0x80, 0x0b, 0x90, 0x38, 0x55, + 0x80, 0xef, 0x1a, 0xae, 0x17, 0xc2, 0x03, 0x82, 0x88, 0x65, 0x80, 0xef, + 0x1b, 0x80, 0x0b, 0x8e, 0x68, 0x65, 0x80, 0xef, 0x9b, 0x80, 0x0b, 0x8c, + 0x08, 0x65, 0x80, 0xef, 0x6b, 0x80, 0x0b, 0x92, 0x1b, 0x8c, 0x98, 0x64, + 0x80, 0xef, 0x1a, 0xec, 0x9b, 0x80, 0x0b, 0x90, 0x95, 0x54, 0x10, 0xe0, + 0xa8, 0x53, 0x80, 0xef, 0x1a, 0xee, 0x17, 0xc2, 0x03, 0x82, 0xf8, 0x63, + 0x80, 0xef, 0x1b, 0x80, 0x0b, 0x8e, 0xd8, 0x63, 0x80, 0xef, 0x1b, 0x8c, + 0x68, 0x63, 0x80, 0xef, 0x6b, 0x80, 0x0b, 0x92, 0x65, 0x54, 0x14, 0xe0, + 0x08, 0x65, 0x84, 0xef, 0x68, 0x63, 0x80, 0xef, 0x7b, 0x80, 0x0b, 0x8c, + 0xa8, 0x64, 0x84, 0xef, 0x08, 0x63, 0x80, 0xef, 0x14, 0xe8, 0x46, 0x44, + 0x94, 0xe1, 0x24, 0x88, 0x4a, 0x4e, 0x04, 0xe0, 0x14, 0xea, 0x1a, 0x04, + 0x08, 0xe0, 0x0a, 0x40, 0x84, 0xed, 0x0c, 0x04, 0x00, 0xe2, 0x4a, 0x40, + 0x04, 0xe0, 0x19, 0x16, 0xc0, 0xe0, 0x0a, 0x40, 0x84, 0xed, 0x21, 0x54, + 0x60, 0xe0, 0x0c, 0x04, 0x00, 0xe2, 0x1b, 0xa5, 0x0e, 0xea, 0x01, 0x89, + 0x21, 0x54, 0x64, 0xe0, 0x7e, 0xe8, 0x65, 0x82, 0x1b, 0xa7, 0x26, 0x00, + 0x00, 0x80, 0xa5, 0x82, 0x1b, 0xa9, 0x65, 0x82, 0x1b, 0xa3, 0x01, 0x85, + 0x16, 0x00, 0x00, 0xc0, 0x01, 0x54, 0x04, 0xf8, 0x06, 0xaa, 0x01, 0x83, + 0x06, 0xa8, 0x65, 0x81, 0x06, 0xa8, 0x01, 0x54, 0x04, 0xf8, 0x01, 0x83, + 0x06, 0xaa, 0x09, 0x14, 0x18, 0xf8, 0x0b, 0xa1, 0x05, 0x84, 0xc6, 0x42, + 0xd4, 0xe0, 0x14, 0x84, 0x01, 0x83, 0x01, 0x54, 0x60, 0xe0, 0x01, 0x54, + 0x64, 0xe0, 0x0b, 0x02, 0x90, 0xe0, 0x10, 0x02, 0x90, 0xe5, 0x01, 0x54, + 0x88, 0xe0, 0xb5, 0x81, 0xc6, 0x40, 0xd4, 0xe0, 0x14, 0x80, 0x0b, 0x02, + 0xe0, 0xe4, 0x10, 0x02, 0x31, 0x66, 0x02, 0xc0, 0x01, 0x54, 0x88, 0xe0, + 0x1a, 0x84, 0x29, 0x14, 0x10, 0xe0, 0x1c, 0xaa, 0x2b, 0xa1, 0xf5, 0x82, + 0x25, 0x14, 0x10, 0xf8, 0x2b, 0x04, 0xa8, 0xe0, 0x20, 0x44, 0x0d, 0x70, + 0x03, 0xc0, 0x2b, 0xa1, 0x04, 0x00, 0x80, 0x9a, 0x02, 0x40, 0x84, 0x90, + 0x03, 0x54, 0x04, 0x80, 0x4c, 0x0c, 0x7c, 0xf2, 0x93, 0xdd, 0x00, 0x00, + 0x02, 0xa9, 0x00, 0x00, 0x64, 0x4a, 0x40, 0x00, 0x08, 0x2d, 0x58, 0xe0, + 0xa8, 0x98, 0x40, 0x00, 0x28, 0x07, 0x34, 0xe0, 0x05, 0xb9, 0x00, 0x00, + 0x28, 0x00, 0x41, 0x05, 0x88, 0x00, 0x41, 0x3c, 0x98, 0x00, 0x41, 0x52, + 0x04, 0x01, 0x41, 0x79, 0x3c, 0x01, 0x41, 0x6a, 0x3d, 0xfe, 0x00, 0x00, +}; + +static const char * const vgxy61_test_pattern_menu[] = { + "Disabled", + "Solid", + "Colorbar", + "Gradbar", + "Hgrey", + "Vgrey", + "Dgrey", + "PN28", +}; + +static const char * const vgxy61_hdr_mode_menu[] = { + "HDR linearize", + "HDR substraction", + "no HDR", +}; + +/* Regulator supplies */ +static const char * const vgxy61_supply_name[] = { + "VCORE", + "VDDIO", + "VANA", +}; + +static const s64 link_freq[] = { + /* + * MIPI output freq is 804Mhz / 2, as it uses both rising edge and falling edges to send + * data + */ + 402000000ULL +}; + +#define VGXY61_NUM_SUPPLIES ARRAY_SIZE(vgxy61_supply_name) + +enum vgxy61_bin_mode { + VGXY61_BIN_MODE_NORMAL, + VGXY61_BIN_MODE_DIGITAL_X2, + VGXY61_BIN_MODE_DIGITAL_X4, +}; + +enum vgxy61_hdr_mode { + VGXY61_HDR_LINEAR, + VGXY61_HDR_SUB, + VGXY61_NO_HDR, +}; + +enum vgxy61_strobe_mode { + VGXY61_STROBE_DISABLED, + VGXY61_STROBE_LONG, + VGXY61_STROBE_ENABLED, +}; + +struct vgxy61_mode_info { + u32 width; + u32 height; + enum vgxy61_bin_mode bin_mode; + struct v4l2_rect crop; +}; + +struct vgxy61_fmt_desc { + u32 code; + u8 bpp; + u8 data_type; +}; + +static const struct vgxy61_fmt_desc vgxy61_supported_codes[] = { + { + .code = MEDIA_BUS_FMT_Y8_1X8, + .bpp = 8, + .data_type = MIPI_CSI2_DT_RAW8, + }, + { + .code = MEDIA_BUS_FMT_Y10_1X10, + .bpp = 10, + .data_type = MIPI_CSI2_DT_RAW10, + }, + { + .code = MEDIA_BUS_FMT_Y12_1X12, + .bpp = 12, + .data_type = MIPI_CSI2_DT_RAW12, + }, + { + .code = MEDIA_BUS_FMT_Y14_1X14, + .bpp = 14, + .data_type = MIPI_CSI2_DT_RAW14, + }, + { + .code = MEDIA_BUS_FMT_Y16_1X16, + .bpp = 16, + .data_type = MIPI_CSI2_DT_RAW16, + }, +}; + +static const struct vgxy61_mode_info vgx661_mode_data[] = { + { + .width = VGX661_WIDTH, + .height = VGX661_HEIGHT, + .bin_mode = VGXY61_BIN_MODE_NORMAL, + .crop = { + .left = 0, + .top = 0, + .width = VGX661_WIDTH, + .height = VGX661_HEIGHT, + }, + }, + { + .width = 1280, + .height = 720, + .bin_mode = VGXY61_BIN_MODE_NORMAL, + .crop = { + .left = 92, + .top = 192, + .width = 1280, + .height = 720, + }, + }, + { + .width = 640, + .height = 480, + .bin_mode = VGXY61_BIN_MODE_DIGITAL_X2, + .crop = { + .left = 92, + .top = 72, + .width = 1280, + .height = 960, + }, + }, + { + .width = 320, + .height = 240, + .bin_mode = VGXY61_BIN_MODE_DIGITAL_X4, + .crop = { + .left = 92, + .top = 72, + .width = 1280, + .height = 960, + }, + }, +}; + +static const struct vgxy61_mode_info vgx761_mode_data[] = { + { + .width = VGX761_WIDTH, + .height = VGX761_HEIGHT, + .bin_mode = VGXY61_BIN_MODE_NORMAL, + .crop = { + .left = 0, + .top = 0, + .width = VGX761_WIDTH, + .height = VGX761_HEIGHT, + }, + }, + { + .width = 1920, + .height = 1080, + .bin_mode = VGXY61_BIN_MODE_NORMAL, + .crop = { + .left = 12, + .top = 62, + .width = 1920, + .height = 1080, + }, + }, + { + .width = 1280, + .height = 720, + .bin_mode = VGXY61_BIN_MODE_NORMAL, + .crop = { + .left = 332, + .top = 242, + .width = 1280, + .height = 720, + }, + }, + { + .width = 640, + .height = 480, + .bin_mode = VGXY61_BIN_MODE_DIGITAL_X2, + .crop = { + .left = 332, + .top = 122, + .width = 1280, + .height = 960, + }, + }, + { + .width = 320, + .height = 240, + .bin_mode = VGXY61_BIN_MODE_DIGITAL_X4, + .crop = { + .left = 332, + .top = 122, + .width = 1280, + .height = 960, + }, + }, +}; + +struct vgxy61_dev { + struct i2c_client *i2c_client; + struct v4l2_subdev sd; + struct media_pad pad; + struct regulator_bulk_data supplies[VGXY61_NUM_SUPPLIES]; + struct gpio_desc *reset_gpio; + struct clk *xclk; + u32 clk_freq; + int id; + int sensor_width; + int sensor_height; + u16 oif_ctrl; + int nb_of_lane; + int data_rate_in_mbps; + int pclk; + u16 line_length; + int rot_term; + bool gpios_polarity; + bool slave_mode; + /* Lock to protect all members below */ + struct mutex lock; + struct v4l2_ctrl_handler ctrl_handler; + struct v4l2_ctrl *pixel_rate_ctrl; + struct v4l2_ctrl *expo_ctrl; + struct v4l2_ctrl *vblank_ctrl; + struct v4l2_ctrl *vflip_ctrl; + struct v4l2_ctrl *hflip_ctrl; + bool streaming; + struct v4l2_mbus_framefmt fmt; + const struct vgxy61_mode_info *sensor_modes; + int sensor_modes_nb; + const struct vgxy61_mode_info *default_mode; + const struct vgxy61_mode_info *current_mode; + bool hflip; + bool vflip; + enum vgxy61_hdr_mode hdr; + int expo_long; + int expo_short; + int expo_max; + int expo_min; + int vblank; + int vblank_min; + u16 frame_length; + int digital_gain; + int analog_gain; + enum vgxy61_strobe_mode strobe_mode; + int pattern; +}; + +static u8 get_bpp_by_code(__u32 code) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(vgxy61_supported_codes); i++) { + if (vgxy61_supported_codes[i].code == code) + return vgxy61_supported_codes[i].bpp; + } + /* Should never happen */ + WARN(1, "Unsupported code %d. default to 8 bpp", code); + return 8; +} + +static u8 get_data_type_by_code(__u32 code) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(vgxy61_supported_codes); i++) { + if (vgxy61_supported_codes[i].code == code) + return vgxy61_supported_codes[i].data_type; + } + /* Should never happen */ + WARN(1, "Unsupported code %d. default to MIPI_CSI2_DT_RAW8 data type", code); + return MIPI_CSI2_DT_RAW8; +} + +static void compute_pll_parameters_by_freq(u32 freq, unsigned int *prediv, unsigned int *mult) +{ + const unsigned int predivs[] = {1, 2, 4}; + int i; + + /* + * Freq range is [6Mhz-27Mhz] already checked. + * Output of divider should be in [6Mhz-12Mhz[. + */ + for (i = 0; i < ARRAY_SIZE(predivs); i++) { + *prediv = predivs[i]; + if (freq / *prediv < 12 * HZ_PER_MHZ) + break; + } + WARN_ON(i == ARRAY_SIZE(predivs)); + + /* + * Target freq is 804Mhz. Don't change this as it will impact image quality. + */ + *mult = ((804 * HZ_PER_MHZ) * (*prediv) + freq / 2) / freq; +} + +static s32 get_pixel_rate(struct vgxy61_dev *sensor) +{ + return div64_u64((u64)sensor->data_rate_in_mbps * sensor->nb_of_lane, + get_bpp_by_code(sensor->fmt.code)); +} + +static inline struct vgxy61_dev *to_vgxy61_dev(struct v4l2_subdev *sd) +{ + return container_of(sd, struct vgxy61_dev, sd); +} + +static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl) +{ + return &container_of(ctrl->handler, struct vgxy61_dev, ctrl_handler)->sd; +} + +static int get_chunk_size(struct vgxy61_dev *sensor) +{ + struct i2c_adapter *adapter = sensor->i2c_client->adapter; + int max_write_len = VGXY61_WRITE_MULTIPLE_CHUNK_MAX; + + if (adapter->quirks && adapter->quirks->max_write_len) + max_write_len = adapter->quirks->max_write_len - 2; + + max_write_len = min(max_write_len, VGXY61_WRITE_MULTIPLE_CHUNK_MAX); + + return max(max_write_len, 1); +} + +static int vgxy61_read_multiple(struct vgxy61_dev *sensor, u32 reg, int len) +{ + struct i2c_client *client = sensor->i2c_client; + struct i2c_msg msg[2]; + u8 buf[2]; + u8 val[sizeof(int)] = {0}; + int ret; + + if (len > sizeof(int)) + return -EINVAL; + buf[0] = reg >> 8; + buf[1] = reg & 0xff; + + msg[0].addr = client->addr; + msg[0].flags = client->flags; + msg[0].buf = buf; + msg[0].len = sizeof(buf); + + msg[1].addr = client->addr; + msg[1].flags = client->flags | I2C_M_RD; + msg[1].buf = val; + msg[1].len = len; + + ret = i2c_transfer(client->adapter, msg, 2); + if (ret < 0) { + dev_dbg(&client->dev, "%s: %x i2c_transfer, reg: %x => %d\n", __func__, + client->addr, reg, ret); + return ret; + } + + return (val[3] << 24) | (val[2] << 16) | (val[1] << 8) | val[0]; +} + +static inline int vgxy61_read_reg(struct vgxy61_dev *sensor, u32 reg) +{ + return vgxy61_read_multiple(sensor, reg & VGXY61_REG_ADDR_MASK, + (reg >> VGXY61_REG_SIZE_SHIFT) & 7); +} + +static int vgxy61_write_multiple(struct vgxy61_dev *sensor, u32 reg, const u8 *data, int len, + int *err) +{ + struct i2c_client *client = sensor->i2c_client; + struct i2c_msg msg; + u8 buf[VGXY61_WRITE_MULTIPLE_CHUNK_MAX + 2]; + int i; + int ret; + + if (err && *err) + return *err; + + if (len > VGXY61_WRITE_MULTIPLE_CHUNK_MAX) + return -EINVAL; + buf[0] = reg >> 8; + buf[1] = reg & 0xff; + for (i = 0; i < len; i++) + buf[i + 2] = data[i]; + + msg.addr = client->addr; + msg.flags = client->flags; + msg.buf = buf; + msg.len = len + 2; + + ret = i2c_transfer(client->adapter, &msg, 1); + if (ret < 0) { + dev_dbg(&client->dev, "%s: i2c_transfer, reg: %x => %d\n", __func__, reg, ret); + if (err) + *err = ret; + return ret; + } + + return 0; +} + +static int vgxy61_write_array(struct vgxy61_dev *sensor, u32 reg, int nb, const u8 *array) +{ + const int chunk_size = get_chunk_size(sensor); + int ret; + int sz; + + while (nb) { + sz = min(nb, chunk_size); + ret = vgxy61_write_multiple(sensor, reg, array, sz, NULL); + if (ret < 0) + return ret; + nb -= sz; + reg += sz; + array += sz; + } + + return 0; +} + +static inline int vgxy61_write_reg(struct vgxy61_dev *sensor, u32 reg, u32 val, int *err) +{ + return vgxy61_write_multiple(sensor, reg & VGXY61_REG_ADDR_MASK, (u8 *)&val, + (reg >> VGXY61_REG_SIZE_SHIFT) & 7, err); +} + +static int vgxy61_poll_reg(struct vgxy61_dev *sensor, u32 reg, u8 poll_val, int timeout_ms) +{ + const int loop_delay_ms = 10; + int ret, timeout; + + timeout = read_poll_timeout(vgxy61_read_reg, ret, ((ret < 0) || (ret == poll_val)), + loop_delay_ms * 1000, timeout_ms * 1000, false, + sensor, reg); + if (timeout) + return timeout; + + return 0; +} + +static int vgxy61_wait_state(struct vgxy61_dev *sensor, int state, int timeout_ms) +{ + return vgxy61_poll_reg(sensor, VGXY61_REG_SYSTEM_FSM, state, timeout_ms); +} + +static int vgxy61_check_bw(struct vgxy61_dev *sensor) +{ + struct i2c_client *client = sensor->i2c_client; + /* Correction factor for time required between 2 lines */ + const int mipi_margin = 1056; + int binning_scale = sensor->current_mode->crop.height / sensor->current_mode->height; + int bpp = get_bpp_by_code(sensor->fmt.code); + int max_bit_per_line; + int bit_per_line; + u64 line_rate; + + line_rate = sensor->nb_of_lane * (u64)sensor->data_rate_in_mbps * sensor->line_length; + max_bit_per_line = div64_u64(line_rate, sensor->pclk) - mipi_margin; + bit_per_line = (bpp * sensor->current_mode->width) / binning_scale; + + dev_dbg(&client->dev, "max_bit_per_line = %d\n", max_bit_per_line); + dev_dbg(&client->dev, "required bit_per_line = %d\n", bit_per_line); + + return bit_per_line > max_bit_per_line ? -EINVAL : 0; +} + +static int vgxy61_apply_exposure(struct vgxy61_dev *sensor) +{ + struct i2c_client *client = sensor->i2c_client; + int ret = 0; + + /* We first set expo to zero to avoid forbidden parameters couple */ + ret = vgxy61_write_reg(sensor, VGXY61_REG_COARSE_EXPOSURE_SHORT, 0, &ret); + ret = vgxy61_write_reg(sensor, VGXY61_REG_COARSE_EXPOSURE_LONG, sensor->expo_long, &ret); + ret = vgxy61_write_reg(sensor, VGXY61_REG_COARSE_EXPOSURE_SHORT, sensor->expo_short, &ret); + if (ret) + return ret; + + dev_dbg(&client->dev, "%s applied expo %d (short: %d)\n", __func__, + sensor->expo_long, sensor->expo_short); + + return 0; +} + +static int vgxy61_get_regulators(struct vgxy61_dev *sensor) +{ + int i; + + for (i = 0; i < VGXY61_NUM_SUPPLIES; i++) + sensor->supplies[i].supply = vgxy61_supply_name[i]; + + return devm_regulator_bulk_get(&sensor->i2c_client->dev, VGXY61_NUM_SUPPLIES, + sensor->supplies); +} + +static int vgxy61_apply_reset(struct vgxy61_dev *sensor) +{ + struct i2c_client *client = sensor->i2c_client; + + dev_dbg(&client->dev, "%s applied reset\n", __func__); + gpiod_set_value_cansleep(sensor->reset_gpio, 0); + usleep_range(5000, 10000); + gpiod_set_value_cansleep(sensor->reset_gpio, 1); + usleep_range(5000, 10000); + gpiod_set_value_cansleep(sensor->reset_gpio, 0); + usleep_range(40000, 100000); + return vgxy61_wait_state(sensor, VGXY61_SYSTEM_FSM_SW_STBY, VGXY61_TIMEOUT_MS); +} + +static void vgxy61_fill_framefmt(struct vgxy61_dev *sensor, const struct vgxy61_mode_info *mode, + struct v4l2_mbus_framefmt *fmt, u32 code) +{ + fmt->code = code; + fmt->width = mode->width; + fmt->height = mode->height; + fmt->colorspace = V4L2_COLORSPACE_RAW; + fmt->field = V4L2_FIELD_NONE; + fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; + fmt->quantization = V4L2_QUANTIZATION_DEFAULT; + fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT; +} + +static int vgxy61_try_fmt_internal(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *fmt, + const struct vgxy61_mode_info **new_mode) +{ + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); + const struct vgxy61_mode_info *mode = sensor->sensor_modes; + unsigned int index; + + /* Select code */ + for (index = 0; index < ARRAY_SIZE(vgxy61_supported_codes); index++) { + if (vgxy61_supported_codes[index].code == fmt->code) + break; + } + if (index == ARRAY_SIZE(vgxy61_supported_codes)) + index = 0; + + /* Select size */ + mode = v4l2_find_nearest_size(sensor->sensor_modes, sensor->sensor_modes_nb, + width, height, fmt->width, fmt->height); + if (new_mode) + *new_mode = mode; + + vgxy61_fill_framefmt(sensor, mode, fmt, vgxy61_supported_codes[index].code); + + return 0; +} + +static int vgxy61_get_selection(struct v4l2_subdev *sd, + struct v4l2_subdev_state *sd_state, + struct v4l2_subdev_selection *sel) +{ + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); + + switch (sel->target) { + case V4L2_SEL_TGT_CROP: + sel->r = sensor->current_mode->crop; + return 0; + case V4L2_SEL_TGT_NATIVE_SIZE: + case V4L2_SEL_TGT_CROP_DEFAULT: + case V4L2_SEL_TGT_CROP_BOUNDS: + sel->r.top = 0; + sel->r.left = 0; + sel->r.width = sensor->sensor_width; + sel->r.height = sensor->sensor_height; + return 0; + } + + return -EINVAL; +} + +static int vgxy61_enum_mbus_code(struct v4l2_subdev *sd, + struct v4l2_subdev_state *sd_state, + struct v4l2_subdev_mbus_code_enum *code) +{ + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); + struct i2c_client *client = sensor->i2c_client; + + dev_dbg(&client->dev, "%s probe index %d\n", __func__, code->index); + if (code->index >= ARRAY_SIZE(vgxy61_supported_codes)) + return -EINVAL; + + code->code = vgxy61_supported_codes[code->index].code; + + return 0; +} + +static int vgxy61_get_fmt(struct v4l2_subdev *sd, + struct v4l2_subdev_state *sd_state, + struct v4l2_subdev_format *format) +{ + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); + struct i2c_client *client = sensor->i2c_client; + struct v4l2_mbus_framefmt *fmt; + + dev_dbg(&client->dev, "%s probe %d\n", __func__, format->pad); + dev_dbg(&client->dev, "%s %dx%d\n", __func__, format->format.width, format->format.height); + + mutex_lock(&sensor->lock); + + if (format->which == V4L2_SUBDEV_FORMAT_TRY) + fmt = v4l2_subdev_get_try_format(&sensor->sd, sd_state, format->pad); + else + fmt = &sensor->fmt; + + format->format = *fmt; + + mutex_unlock(&sensor->lock); + + return 0; +} + +static int vgxy61_get_vblank_min(struct vgxy61_dev *sensor, enum vgxy61_hdr_mode hdr) +{ + int min_vblank = VGXY61_MIN_FRAME_LENGTH - sensor->current_mode->crop.height; + /* Ensure the first rule of thumb can't be negative */ + int min_vblank_hdr = VGXY61_MIN_EXPOSURE + sensor->rot_term + 1; + + if (hdr != VGXY61_NO_HDR) + return max(min_vblank, min_vblank_hdr); + return min_vblank; +} + +static int vgxy61_enum_frame_size(struct v4l2_subdev *sd, struct v4l2_subdev_state *sd_state, + struct v4l2_subdev_frame_size_enum *fse) +{ + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); + struct i2c_client *client = sensor->i2c_client; + + dev_dbg(&client->dev, "%s for index %d\n", __func__, fse->index); + if (fse->index >= sensor->sensor_modes_nb) + return -EINVAL; + + fse->min_width = sensor->sensor_modes[fse->index].width; + fse->max_width = fse->min_width; + fse->min_height = sensor->sensor_modes[fse->index].height; + fse->max_height = fse->min_height; + + return 0; +} + +static int vgxy61_update_analog_gain(struct vgxy61_dev *sensor, u32 target) +{ + struct i2c_client *client = sensor->i2c_client; + + dev_dbg(&client->dev, "Set analog gain: 0x%04x\n", target); + sensor->analog_gain = target; + + /* Apply gain */ + if (sensor->streaming) + return vgxy61_write_reg(sensor, VGXY61_REG_ANALOG_GAIN, target, NULL); + return 0; +} + +static int vgxy61_apply_digital_gain(struct vgxy61_dev *sensor, u32 digital_gain) +{ + int ret = 0; + + /* + * For a monochrome version, configuring DIGITAL_GAIN_LONG_CH0 and + * DIGITAL_GAIN_SHORT_CH0 is enough to configure the gain of all + * four sub pixels. + */ + vgxy61_write_reg(sensor, VGXY61_REG_DIGITAL_GAIN_LONG, digital_gain, &ret); + vgxy61_write_reg(sensor, VGXY61_REG_DIGITAL_GAIN_SHORT, digital_gain, &ret); + + return ret; +} + +static int vgxy61_update_digital_gain(struct vgxy61_dev *sensor, u32 target) +{ + struct i2c_client *client = sensor->i2c_client; + + dev_dbg(&client->dev, "Set digital gain: 0x%04x\n", target); + sensor->digital_gain = target; + + if (sensor->streaming) + return vgxy61_apply_digital_gain(sensor, sensor->digital_gain); + return 0; +} + +static int vgxy61_apply_patgen(struct vgxy61_dev *sensor, u32 index) +{ + static const u8 index2val[] = {0x0, 0x1, 0x2, 0x3, 0x10, 0x11, 0x12, 0x13}; + u32 pattern = index2val[index]; + u32 reg = (pattern << VGXY61_PATGEN_LONG_TYPE_SHIFT) | + (pattern << VGXY61_PATGEN_SHORT_TYPE_SHIFT); + + if (pattern) + reg |= VGXY61_PATGEN_LONG_ENABLE | VGXY61_PATGEN_SHORT_ENABLE; + return vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_CTRL, reg, NULL); +} + +static int vgxy61_update_patgen(struct vgxy61_dev *sensor, int pattern) +{ + sensor->pattern = pattern; + + if (sensor->streaming) + return vgxy61_apply_patgen(sensor, sensor->pattern); + return 0; +} + +static int vgxy61_apply_gpiox_strobe_mode(struct vgxy61_dev *sensor, enum vgxy61_strobe_mode mode, + int idx) +{ + static const u8 index2val[] = {0x0, 0x1, 0x3}; + u16 reg; + + reg = vgxy61_read_reg(sensor, VGXY61_REG_SIGNALS_CTRL); + if (reg < 0) + return reg; + reg &= ~(0xf << (idx * VGXY61_SIGNALS_GPIO_ID_SHIFT)); + reg |= index2val[mode] << (idx * VGXY61_SIGNALS_GPIO_ID_SHIFT); + + return vgxy61_write_reg(sensor, VGXY61_REG_SIGNALS_CTRL, reg, NULL); +} + +static int vgxy61_update_gpios_strobe_mode(struct vgxy61_dev *sensor, enum vgxy61_hdr_mode hdr) +{ + int i, ret; + + switch (hdr) { + case VGXY61_HDR_LINEAR: + sensor->strobe_mode = VGXY61_STROBE_ENABLED; + break; + case VGXY61_HDR_SUB: + case VGXY61_NO_HDR: + sensor->strobe_mode = VGXY61_STROBE_LONG; + break; + default: + /* Should never happen */ + WARN_ON(true); + break; + } + + if (sensor->streaming) { + for (i = 0; i < VGXY61_NB_GPIOS; i++) { + ret = vgxy61_apply_gpiox_strobe_mode(sensor, sensor->strobe_mode, i); + if (ret) + return ret; + } + } + + return 0; +} + +static int vgxy61_update_gpios_strobe_polarity(struct vgxy61_dev *sensor, int polarity) +{ + struct i2c_client *client = sensor->i2c_client; + int ret = 0; + + if (sensor->streaming) + return -EBUSY; + + dev_dbg(&client->dev, "setting gpios polarity: %d\n", polarity); + + vgxy61_write_reg(sensor, VGXY61_REG_GPIO_0_CTRL, polarity << 1, &ret); + vgxy61_write_reg(sensor, VGXY61_REG_GPIO_1_CTRL, polarity << 1, &ret); + vgxy61_write_reg(sensor, VGXY61_REG_GPIO_2_CTRL, polarity << 1, &ret); + vgxy61_write_reg(sensor, VGXY61_REG_GPIO_3_CTRL, polarity << 1, &ret); + vgxy61_write_reg(sensor, VGXY61_REG_SIGNALS_POLARITY_CTRL, polarity, &ret); + + return ret; +} + +static int vgxy61_get_expo_long_max(struct vgxy61_dev *sensor, int short_expo_ratio) +{ + int first_rot_max_expo, second_rot_max_expo, third_rot_max_expo; + + /* Apply sensor's rules of thumb */ + /* + * Short exposure + height must be less than frame length to avoid bad pixel line at the + * botom of the image + */ + first_rot_max_expo = + ((sensor->frame_length - sensor->current_mode->crop.height - sensor->rot_term) * + short_expo_ratio) - 1; + + /* Total exposition time must be less than frame length to avoid sensor crash */ + second_rot_max_expo = + (((sensor->frame_length - VGXY61_EXPOS_ROT_TERM) * short_expo_ratio) / + (short_expo_ratio + 1)) - 1; + + /* Short exposure times 71 must be less than frame length to avoid sensor crash */ + third_rot_max_expo = (sensor->frame_length / 71) * short_expo_ratio; + + /* Take the minimum from all rules */ + return min(min(first_rot_max_expo, second_rot_max_expo), third_rot_max_expo); +} + +static int vgxy61_update_exposure(struct vgxy61_dev *sensor, int new_expo_long, + enum vgxy61_hdr_mode hdr) +{ + struct i2c_client *client = sensor->i2c_client; + int new_expo_short = 0; + int expo_short_max = 0; + int expo_long_min = VGXY61_MIN_EXPOSURE; + int expo_long_max; + + /* Compute short exposure according to hdr mode and long exposure */ + switch (hdr) { + case VGXY61_HDR_LINEAR: + /* Take ratio into account for minimal exposures in VGXY61_HDR_LINEAR */ + expo_long_min = VGXY61_MIN_EXPOSURE * VGXY61_HDR_LINEAR_RATIO; + new_expo_long = max(expo_long_min, new_expo_long); + + expo_long_max = vgxy61_get_expo_long_max(sensor, VGXY61_HDR_LINEAR_RATIO); + expo_short_max = + (expo_long_max + (VGXY61_HDR_LINEAR_RATIO / 2)) / VGXY61_HDR_LINEAR_RATIO; + new_expo_short = + (new_expo_long + (VGXY61_HDR_LINEAR_RATIO / 2)) / VGXY61_HDR_LINEAR_RATIO; + break; + case VGXY61_HDR_SUB: + new_expo_long = max(expo_long_min, new_expo_long); + + expo_long_max = vgxy61_get_expo_long_max(sensor, 1); + /* Short and long are the same in VGXY61_HDR_SUB */ + expo_short_max = expo_long_max; + new_expo_short = new_expo_long; + break; + case VGXY61_NO_HDR: + new_expo_long = max(expo_long_min, new_expo_long); + + /* + * As short expo is 0 here, only the second rule of thumb applies, see + * vgxy61_get_expo_long_max for more + */ + expo_long_max = sensor->frame_length - VGXY61_EXPOS_ROT_TERM; + break; + default: + /* Should never happen */ + WARN_ON(true); + break; + } + + /* If this happens, something is wrong with formulas */ + WARN_ON(expo_long_min > expo_long_max); + + if (new_expo_long > expo_long_max) { + dev_warn(&client->dev, "Exposure %d too high, clamping to %d\n", + new_expo_long, expo_long_max); + new_expo_long = expo_long_max; + new_expo_short = expo_short_max; + } + + dev_dbg(&client->dev, "frame_length %d, new_expo_long %d, new_expo_short %d\n", + sensor->frame_length, new_expo_long, new_expo_short); + dev_dbg(&client->dev, "expo_long_min %d, expo_long_max %d\n", + expo_long_min, expo_long_max); + + /* Apply exposure */ + sensor->expo_long = new_expo_long; + sensor->expo_short = new_expo_short; + sensor->expo_max = expo_long_max; + sensor->expo_min = expo_long_min; + + if (sensor->streaming) + return vgxy61_apply_exposure(sensor); + return 0; +} + +static int vgxy61_apply_framelength(struct vgxy61_dev *sensor) +{ + return vgxy61_write_reg(sensor, VGXY61_REG_FRAME_LENGTH, sensor->frame_length, NULL); +} + +static int vgxy61_update_vblank(struct vgxy61_dev *sensor, int vblank, enum vgxy61_hdr_mode hdr) +{ + int ret; + + sensor->vblank_min = vgxy61_get_vblank_min(sensor, hdr); + sensor->vblank = max(sensor->vblank_min, vblank); + sensor->frame_length = sensor->current_mode->crop.height + sensor->vblank; + + /* Update exposure according to vblank */ + ret = vgxy61_update_exposure(sensor, sensor->expo_long, hdr); + if (ret) + return ret; + + if (sensor->streaming) + return vgxy61_apply_framelength(sensor); + return 0; +} + +static int vgxy61_apply_hdr(struct vgxy61_dev *sensor, enum vgxy61_hdr_mode index) +{ + static const u8 index2val[] = {0x1, 0x4, 0xa}; + + return vgxy61_write_reg(sensor, VGXY61_REG_HDR_CTRL, index2val[index], NULL); +} + +static int vgxy61_update_hdr(struct vgxy61_dev *sensor, enum vgxy61_hdr_mode index) +{ + int ret; + + /* + * vblank and short exposure change according to HDR mode, do it first as it can + * violate sensors 'rule of thumbs' and therefore will require to change the long exposure + */ + ret = vgxy61_update_vblank(sensor, sensor->vblank, index); + if (ret) + return ret; + + /* Update strobe mode according to HDR */ + ret = vgxy61_update_gpios_strobe_mode(sensor, index); + if (ret) + return ret; + + sensor->hdr = index; + + if (sensor->streaming) + return vgxy61_apply_hdr(sensor, sensor->hdr); + return 0; +} + +static int vgxy61_apply_settings(struct vgxy61_dev *sensor) +{ + int ret, i; + + ret = vgxy61_apply_hdr(sensor, sensor->hdr); + if (ret) + return ret; + + ret = vgxy61_apply_framelength(sensor); + if (ret) + return ret; + + ret = vgxy61_apply_exposure(sensor); + if (ret) + return ret; + + ret = vgxy61_write_reg(sensor, VGXY61_REG_ANALOG_GAIN, sensor->analog_gain, NULL); + if (ret) + return ret; + ret = vgxy61_apply_digital_gain(sensor, sensor->digital_gain); + if (ret) + return ret; + + ret = vgxy61_write_reg(sensor, VGXY61_REG_ORIENTATION, + sensor->hflip | (sensor->vflip << 1), NULL); + if (ret) + return ret; + + ret = vgxy61_apply_patgen(sensor, sensor->pattern); + if (ret) + return ret; + + for (i = 0; i < VGXY61_NB_GPIOS; i++) { + ret = vgxy61_apply_gpiox_strobe_mode(sensor, sensor->strobe_mode, i); + if (ret) + return ret; + } + + return 0; +} + +static int vgxy61_stream_enable(struct vgxy61_dev *sensor) +{ + struct i2c_client *client = v4l2_get_subdevdata(&sensor->sd); + const struct v4l2_rect *crop = &sensor->current_mode->crop; + int ret = 0; + + ret = vgxy61_check_bw(sensor); + if (ret) + return ret; + + ret = pm_runtime_get_sync(&client->dev); + if (ret < 0) { + pm_runtime_put_noidle(&client->dev); + return ret; + } + + /* Configure sensor */ + vgxy61_write_reg(sensor, VGXY61_REG_FORMAT_CTRL, get_bpp_by_code(sensor->fmt.code), &ret); + vgxy61_write_reg(sensor, VGXY61_REG_OIF_ROI0_CTRL, + get_data_type_by_code(sensor->fmt.code), &ret); + + vgxy61_write_reg(sensor, VGXY61_REG_READOUT_CTRL, sensor->current_mode->bin_mode, &ret); + vgxy61_write_reg(sensor, VGXY61_REG_ROI0_START_H, crop->left, &ret); + vgxy61_write_reg(sensor, VGXY61_REG_ROI0_END_H, crop->left + crop->width - 1, &ret); + vgxy61_write_reg(sensor, VGXY61_REG_ROI0_START_V, crop->top, &ret); + vgxy61_write_reg(sensor, VGXY61_REG_ROI0_END_V, crop->top + crop->height - 1, &ret); + if (ret) + goto err_rpm_put; + + /* Apply controls cached values */ + ret = vgxy61_apply_settings(sensor); + if (ret) + goto err_rpm_put; + + /* Start streaming */ + ret = vgxy61_write_reg(sensor, VGXY61_REG_STREAMING, VGXY61_STREAMING_REQ_START, NULL); + if (ret) + goto err_rpm_put; + + ret = vgxy61_poll_reg(sensor, VGXY61_REG_STREAMING, VGXY61_STREAMING_NO_REQ, + VGXY61_TIMEOUT_MS); + if (ret) + goto err_rpm_put; + + ret = vgxy61_wait_state(sensor, VGXY61_SYSTEM_FSM_STREAMING, VGXY61_TIMEOUT_MS); + if (ret) + goto err_rpm_put; + + /* vflip and hflip cannot change during streaming */ + __v4l2_ctrl_grab(sensor->vflip_ctrl, true); + __v4l2_ctrl_grab(sensor->hflip_ctrl, true); + + return 0; + +err_rpm_put: + pm_runtime_put(&client->dev); + return ret; +} + +static int vgxy61_stream_disable(struct vgxy61_dev *sensor) +{ + struct i2c_client *client = v4l2_get_subdevdata(&sensor->sd); + int ret; + + ret = vgxy61_write_reg(sensor, VGXY61_REG_STREAMING, VGXY61_STREAMING_REQ_STOP, NULL); + if (ret) + return ret; + + ret = vgxy61_poll_reg(sensor, VGXY61_REG_STREAMING, VGXY61_STREAMING_NO_REQ, 2000); + if (ret) + return ret; + + ret = vgxy61_wait_state(sensor, VGXY61_SYSTEM_FSM_SW_STBY, VGXY61_TIMEOUT_MS); + if (ret) + return ret; + + __v4l2_ctrl_grab(sensor->vflip_ctrl, false); + __v4l2_ctrl_grab(sensor->hflip_ctrl, false); + + pm_runtime_put(&client->dev); + + return 0; +} + +static int vgxy61_s_stream(struct v4l2_subdev *sd, int enable) +{ + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); + struct i2c_client *client = sensor->i2c_client; + int ret = 0; + + mutex_lock(&sensor->lock); + dev_dbg(&client->dev, "%s : requested %d / current = %d\n", __func__, enable, + sensor->streaming); + + ret = enable ? vgxy61_stream_enable(sensor) : vgxy61_stream_disable(sensor); + if (!ret) + sensor->streaming = enable; + + dev_dbg(&client->dev, "%s current now = %d / %d\n", __func__, sensor->streaming, ret); + mutex_unlock(&sensor->lock); + + return ret; +} + +static int vgxy61_set_fmt(struct v4l2_subdev *sd, + struct v4l2_subdev_state *sd_state, + struct v4l2_subdev_format *format) +{ + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); + struct i2c_client *client = sensor->i2c_client; + const struct vgxy61_mode_info *new_mode; + struct v4l2_mbus_framefmt *fmt; + int ret; + + dev_dbg(&client->dev, "%s probe %d\n", __func__, format->pad); + dev_dbg(&client->dev, "%s %dx%d\n", __func__, format->format.width, format->format.height); + + mutex_lock(&sensor->lock); + + if (sensor->streaming) { + ret = -EBUSY; + goto out; + } + + /* Find best format */ + ret = vgxy61_try_fmt_internal(sd, &format->format, &new_mode); + if (ret) + goto out; + + if (format->which == V4L2_SUBDEV_FORMAT_TRY) { + fmt = v4l2_subdev_get_try_format(sd, sd_state, 0); + *fmt = format->format; + } else if (sensor->current_mode != new_mode || sensor->fmt.code != format->format.code) { + fmt = &sensor->fmt; + *fmt = format->format; + + sensor->current_mode = new_mode; + + /* Reset vblank and framelength to default */ + ret = vgxy61_update_vblank(sensor, + VGXY61_FRAME_LENGTH_DEF - + sensor->current_mode->crop.height, + sensor->hdr); + + /* Update controls to reflect new mode */ + __v4l2_ctrl_s_ctrl_int64(sensor->pixel_rate_ctrl, get_pixel_rate(sensor)); + __v4l2_ctrl_modify_range(sensor->vblank_ctrl, sensor->vblank_min, + 0xffff - sensor->current_mode->crop.height, 1, + sensor->vblank); + __v4l2_ctrl_s_ctrl(sensor->vblank_ctrl, sensor->vblank); + __v4l2_ctrl_modify_range(sensor->expo_ctrl, sensor->expo_min, + sensor->expo_max, 1, + sensor->expo_long); + } + +out: + mutex_unlock(&sensor->lock); + + return ret; +} + +static int vgxy61_init_cfg(struct v4l2_subdev *sd, struct v4l2_subdev_state *sd_state) +{ + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); + struct v4l2_subdev_format fmt = { 0 }; + + sensor->current_mode = sensor->default_mode; + vgxy61_fill_framefmt(sensor, sensor->current_mode, &fmt.format, VGXY61_MEDIA_BUS_FMT_DEF); + + return vgxy61_set_fmt(sd, sd_state, &fmt); +} + +static int vgxy61_s_ctrl(struct v4l2_ctrl *ctrl) +{ + struct v4l2_subdev *sd = ctrl_to_sd(ctrl); + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); + int ret; + + switch (ctrl->id) { + case V4L2_CID_EXPOSURE: + ret = vgxy61_update_exposure(sensor, ctrl->val, sensor->hdr); + ctrl->val = sensor->expo_long; + break; + case V4L2_CID_ANALOGUE_GAIN: + ret = vgxy61_update_analog_gain(sensor, ctrl->val); + break; + case V4L2_CID_DIGITAL_GAIN: + ret = vgxy61_update_digital_gain(sensor, ctrl->val); + break; + case V4L2_CID_VFLIP: + case V4L2_CID_HFLIP: + if (sensor->streaming) { + ret = -EBUSY; + break; + } + if (ctrl->id == V4L2_CID_VFLIP) + sensor->vflip = ctrl->val; + if (ctrl->id == V4L2_CID_HFLIP) + sensor->hflip = ctrl->val; + ret = 0; + break; + case V4L2_CID_TEST_PATTERN: + ret = vgxy61_update_patgen(sensor, ctrl->val); + break; + case V4L2_CID_STVGXY61_HDR: + ret = vgxy61_update_hdr(sensor, ctrl->val); + /* Update vblank and exposure controls to match new hdr */ + __v4l2_ctrl_modify_range(sensor->vblank_ctrl, sensor->vblank_min, + 0xffff - sensor->current_mode->crop.height, 1, + sensor->vblank); + __v4l2_ctrl_modify_range(sensor->expo_ctrl, sensor->expo_min, + sensor->expo_max, 1, sensor->expo_long); + break; + case V4L2_CID_VBLANK: + ret = vgxy61_update_vblank(sensor, ctrl->val, sensor->hdr); + /* Update exposure control to match new vblank */ + __v4l2_ctrl_modify_range(sensor->expo_ctrl, sensor->expo_min, + sensor->expo_max, 1, sensor->expo_long); + break; + default: + ret = -EINVAL; + break; + } + + return ret; +} + +static const struct v4l2_ctrl_ops vgxy61_ctrl_ops = { + .s_ctrl = vgxy61_s_ctrl, +}; + +static const struct v4l2_ctrl_config vgxy61_hdr_ctrl = { + .ops = &vgxy61_ctrl_ops, + .id = V4L2_CID_STVGXY61_HDR, + .name = "HDR mode", + .type = V4L2_CTRL_TYPE_MENU, + .min = 0, + .max = ARRAY_SIZE(vgxy61_hdr_mode_menu) - 1, + .def = VGXY61_NO_HDR, + .qmenu = vgxy61_hdr_mode_menu, +}; + +static int vgxy61_init_controls(struct vgxy61_dev *sensor) +{ + const struct v4l2_ctrl_ops *ops = &vgxy61_ctrl_ops; + struct v4l2_ctrl_handler *hdl = &sensor->ctrl_handler; + struct v4l2_ctrl *ctrl; + int ret; + + v4l2_ctrl_handler_init(hdl, 16); + /* We can use our own mutex for the ctrl lock */ + hdl->lock = &sensor->lock; + /* Standard controls */ + v4l2_ctrl_new_std(hdl, ops, V4L2_CID_ANALOGUE_GAIN, 0, 0x1c, 1, sensor->analog_gain); + v4l2_ctrl_new_std(hdl, ops, V4L2_CID_DIGITAL_GAIN, 0, 0xfff, 1, sensor->digital_gain); + v4l2_ctrl_new_std_menu_items(hdl, ops, V4L2_CID_TEST_PATTERN, + ARRAY_SIZE(vgxy61_test_pattern_menu) - 1, 0, 0, + vgxy61_test_pattern_menu); + ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HBLANK, 0, sensor->line_length, 1, + sensor->line_length - sensor->current_mode->width); + ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; + ctrl = v4l2_ctrl_new_int_menu(hdl, ops, V4L2_CID_LINK_FREQ, ARRAY_SIZE(link_freq) - 1, 0, + link_freq); + ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; + /* Custom controls */ + v4l2_ctrl_new_custom(hdl, &vgxy61_hdr_ctrl, NULL); + /* Keep a pointer to these controls as we need to update them when setting the format */ + sensor->pixel_rate_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_PIXEL_RATE, 1, INT_MAX, 1, + get_pixel_rate(sensor)); + sensor->pixel_rate_ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; + sensor->expo_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_EXPOSURE, sensor->expo_min, + sensor->expo_max, 1, sensor->expo_long); + sensor->vblank_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VBLANK, sensor->vblank_min, + 0xffff - sensor->current_mode->crop.height, 1, + sensor->vblank); + sensor->vflip_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VFLIP, 0, 1, 1, sensor->vflip); + sensor->hflip_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HFLIP, 0, 1, 1, sensor->hflip); + + if (hdl->error) { + ret = hdl->error; + goto free_ctrls; + } + + sensor->sd.ctrl_handler = hdl; + return 0; + +free_ctrls: + v4l2_ctrl_handler_free(hdl); + return ret; +} + +static const struct v4l2_subdev_video_ops vgxy61_video_ops = { + .s_stream = vgxy61_s_stream, +}; + +static const struct v4l2_subdev_pad_ops vgxy61_pad_ops = { + .init_cfg = vgxy61_init_cfg, + .enum_mbus_code = vgxy61_enum_mbus_code, + .get_fmt = vgxy61_get_fmt, + .set_fmt = vgxy61_set_fmt, + .get_selection = vgxy61_get_selection, + .enum_frame_size = vgxy61_enum_frame_size, +}; + +static const struct v4l2_subdev_ops vgxy61_subdev_ops = { + .video = &vgxy61_video_ops, + .pad = &vgxy61_pad_ops, +}; + +static const struct media_entity_operations vgxy61_subdev_entity_ops = { + .link_validate = v4l2_subdev_link_validate, +}; + +/* Set phy polarities */ +static int vgxy61_tx_from_ep(struct vgxy61_dev *sensor, struct fwnode_handle *endpoint) +{ + struct v4l2_fwnode_endpoint ep = { .bus_type = V4L2_MBUS_CSI2_DPHY }; + struct i2c_client *client = sensor->i2c_client; + u32 log2phy[VGXY61_NB_POLARITIES] = {~0, ~0, ~0, ~0, ~0}; + u32 phy2log[VGXY61_NB_POLARITIES] = {~0, ~0, ~0, ~0, ~0}; + int polarities[VGXY61_NB_POLARITIES] = {0, 0, 0, 0, 0}; + int l_nb; + int p, l; + int ret; + int i; + + ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &ep); + if (ret) + goto error_alloc; + + l_nb = ep.bus.mipi_csi2.num_data_lanes; + if (l_nb != 1 && l_nb != 2 && l_nb != 4) { + dev_err(&client->dev, "invalid data lane number %d\n", l_nb); + goto error_ep; + } + + /* Build log2phy, phy2log and polarities from ep info */ + log2phy[0] = ep.bus.mipi_csi2.clock_lane; + phy2log[log2phy[0]] = 0; + for (l = 1; l < l_nb + 1; l++) { + log2phy[l] = ep.bus.mipi_csi2.data_lanes[l - 1]; + phy2log[log2phy[l]] = l; + } + /* + * Then fill remaining slots for every physical slot to have something valid for hardware + * stuff. + */ + for (p = 0; p < VGXY61_NB_POLARITIES; p++) { + if (phy2log[p] != ~0) + continue; + phy2log[p] = l; + log2phy[l] = p; + l++; + } + for (l = 0; l < l_nb + 1; l++) + polarities[l] = ep.bus.mipi_csi2.lane_polarities[l]; + + if (log2phy[0] != 0) { + dev_err(&client->dev, "clk lane must be map to physical lane 0\n"); + goto error_ep; + } + sensor->oif_ctrl = (polarities[4] << 15) + ((phy2log[4] - 1) << 13) + + (polarities[3] << 12) + ((phy2log[3] - 1) << 10) + + (polarities[2] << 9) + ((phy2log[2] - 1) << 7) + + (polarities[1] << 6) + ((phy2log[1] - 1) << 4) + + (polarities[0] << 3) + + l_nb; + sensor->nb_of_lane = l_nb; + + dev_dbg(&client->dev, "tx uses %d lanes", l_nb); + for (i = 0; i < 5; i++) { + dev_dbg(&client->dev, "log2phy[%d] = %d\n", i, log2phy[i]); + dev_dbg(&client->dev, "phy2log[%d] = %d\n", i, phy2log[i]); + dev_dbg(&client->dev, "polarity[%d] = %d\n", i, polarities[i]); + } + dev_dbg(&client->dev, "oif_ctrl = 0x%04x\n", sensor->oif_ctrl); + + v4l2_fwnode_endpoint_free(&ep); + + return 0; + +error_ep: + v4l2_fwnode_endpoint_free(&ep); +error_alloc: + + return -EINVAL; +} + +static int vgxy61_configure(struct vgxy61_dev *sensor) +{ + struct i2c_client *client = sensor->i2c_client; + int sensor_freq; + unsigned int prediv; + unsigned int mult; + int line_length; + int ret = 0; + + compute_pll_parameters_by_freq(sensor->clk_freq, &prediv, &mult); + sensor_freq = (mult * sensor->clk_freq) / prediv; + /* Frequency to data rate is 1:1 ratio for MIPI */ + sensor->data_rate_in_mbps = sensor_freq; + /* Video timing ISP path (pixel clock) requires 804/5 mhz = 160 mhz */ + sensor->pclk = sensor_freq / 5; + + /* Cache line_length value */ + line_length = vgxy61_read_reg(sensor, VGXY61_REG_LINE_LENGTH); + if (line_length < 0) + return ret; + sensor->line_length = line_length; + /* Configure clocks */ + vgxy61_write_reg(sensor, VGXY61_REG_EXT_CLOCK, sensor->clk_freq, &ret); + vgxy61_write_reg(sensor, VGXY61_REG_CLK_PLL_PREDIV, prediv, &ret); + vgxy61_write_reg(sensor, VGXY61_REG_CLK_SYS_PLL_MULT, mult, &ret); + /* Configure interface */ + vgxy61_write_reg(sensor, VGXY61_REG_OIF_CTRL, sensor->oif_ctrl, &ret); + /* Disable pwm compression */ + vgxy61_write_reg(sensor, VGXY61_REG_FRAME_CONTENT_CTRL, 0, &ret); + /* Disable asil lines */ + vgxy61_write_reg(sensor, VGXY61_REG_BYPASS_CTRL, 4, &ret); + if (ret) + return ret; + /* Set gpios polarity according to device tree value */ + vgxy61_update_gpios_strobe_polarity(sensor, sensor->gpios_polarity); + /* Slave mode */ + ret = vgxy61_write_reg(sensor, VGXY61_REG_VT_CTRL, sensor->slave_mode, &ret); + /* Set pattern generator solid to middle value */ + vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_LONG_DATA_GR, 0x800, &ret); + vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_LONG_DATA_R, 0x800, &ret); + vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_LONG_DATA_B, 0x800, &ret); + vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_LONG_DATA_GB, 0x800, &ret); + vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_SHORT_DATA_GR, 0x800, &ret); + vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_SHORT_DATA_R, 0x800, &ret); + vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_SHORT_DATA_B, 0x800, &ret); + vgxy61_write_reg(sensor, VGXY61_REG_PATGEN_SHORT_DATA_GB, 0x800, &ret); + if (ret) + return ret; + + dev_dbg(&client->dev, "clock prediv = %d\n", prediv); + dev_dbg(&client->dev, "clock mult = %d\n", mult); + dev_dbg(&client->dev, "data rate = %d mbps\n", sensor->data_rate_in_mbps); + + return 0; +} + +static int vgxy61_patch(struct vgxy61_dev *sensor) +{ + struct i2c_client *client = sensor->i2c_client; + u16 patch; + int ret; + + ret = vgxy61_write_array(sensor, VGXY61_REG_FWPATCH_START_ADDR, sizeof(patch_array), + patch_array); + if (ret) + return ret; + + ret = vgxy61_write_reg(sensor, VGXY61_REG_STBY, 0x10, NULL); + if (ret) + return ret; + + ret = vgxy61_poll_reg(sensor, VGXY61_REG_STBY, 0, VGXY61_TIMEOUT_MS); + if (ret) + return ret; + + patch = vgxy61_read_reg(sensor, VGXY61_REG_FWPATCH_REVISION); + if (patch < 0) + return patch; + + if (patch != (VGXY61_FWPATCH_REVISION_MAJOR << 12) + + (VGXY61_FWPATCH_REVISION_MINOR << 8) + + VGXY61_FWPATCH_REVISION_MICRO) { + dev_err(&client->dev, "bad patch version expected %d.%d.%d got %d.%d.%d\n", + VGXY61_FWPATCH_REVISION_MAJOR, + VGXY61_FWPATCH_REVISION_MINOR, + VGXY61_FWPATCH_REVISION_MICRO, + patch >> 12, (patch >> 8) & 0x0f, patch & 0xff); + return -ENODEV; + } + dev_dbg(&client->dev, "patch %d.%d.%d applied\n", + patch >> 12, (patch >> 8) & 0x0f, patch & 0xff); + + return 0; +} + +static int vgxy61_detect_cut_version(struct vgxy61_dev *sensor) +{ + struct i2c_client *client = sensor->i2c_client; + u16 device_rev; + + device_rev = vgxy61_read_reg(sensor, VGXY61_REG_REVISION); + if (device_rev < 0) + return device_rev; + + switch (device_rev >> 8) { + case 0xA: + dev_dbg(&client->dev, "Cut1 detected\n"); + dev_err(&client->dev, "Cut1 not supported by this driver\n"); + return -ENODEV; + case 0xB: + dev_dbg(&client->dev, "Cut2 detected\n"); + return 0; + case 0xC: + dev_dbg(&client->dev, "Cut3 detected\n"); + return 0; + default: + dev_err(&client->dev, "Unable to detect cut version\n"); + return -ENODEV; + } +} + +static int vgxy61_detect(struct vgxy61_dev *sensor) +{ + struct i2c_client *client = sensor->i2c_client; + u16 id = 0; + int ret; + u8 st; + + id = vgxy61_read_reg(sensor, VGXY61_REG_MODEL_ID); + if (id < 0) + return id; + if (id != VG5661_MODEL_ID && id != VG5761_MODEL_ID) { + dev_warn(&client->dev, "Unsupported sensor id %x\n", id); + return -ENODEV; + } + dev_dbg(&client->dev, "detected sensor id = 0x%04x\n", id); + sensor->id = id; + + ret = vgxy61_wait_state(sensor, VGXY61_SYSTEM_FSM_SW_STBY, VGXY61_TIMEOUT_MS); + if (ret) + return ret; + + st = vgxy61_read_reg(sensor, VGXY61_REG_NVM); + if (st < 0) + return st; + if (st != VGXY61_NVM_OK) + dev_warn(&client->dev, "Bad nvm state got %d\n", st); + + /* Detect cut version */ + ret = vgxy61_detect_cut_version(sensor); + if (ret) + return ret; + + return 0; +} + +/* Power/clock management functions */ +static int vgxy61_power_on(struct device *dev) +{ + struct i2c_client *client = to_i2c_client(dev); + struct v4l2_subdev *sd = i2c_get_clientdata(client); + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); + int ret; + + ret = vgxy61_get_regulators(sensor); + if (ret) { + dev_err(&client->dev, "failed to get regulators %d\n", ret); + return ret; + } + + ret = regulator_bulk_enable(VGXY61_NUM_SUPPLIES, sensor->supplies); + if (ret) { + dev_err(&client->dev, "failed to enable regulators %d\n", ret); + return ret; + } + + ret = clk_prepare_enable(sensor->xclk); + if (ret) { + dev_err(&client->dev, "failed to enable clock %d\n", ret); + goto disable_bulk; + } + + /* Apply reset sequence */ + if (sensor->reset_gpio) { + ret = vgxy61_apply_reset(sensor); + if (ret) { + dev_err(&client->dev, "sensor reset failed %d\n", ret); + goto disable_clock; + } + } + + ret = vgxy61_patch(sensor); + if (ret) { + dev_err(&client->dev, "sensor patch failed %d\n", ret); + goto disable_clock; + } + + ret = vgxy61_configure(sensor); + if (ret) { + dev_err(&client->dev, "sensor configuration failed %d\n", ret); + goto disable_clock; + } + + return 0; + +disable_clock: + clk_disable_unprepare(sensor->xclk); +disable_bulk: + regulator_bulk_disable(VGXY61_NUM_SUPPLIES, sensor->supplies); + + return ret; +} + +static int vgxy61_power_off(struct device *dev) +{ + struct i2c_client *client = to_i2c_client(dev); + struct v4l2_subdev *sd = i2c_get_clientdata(client); + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); + + clk_disable_unprepare(sensor->xclk); + regulator_bulk_disable(VGXY61_NUM_SUPPLIES, sensor->supplies); + return 0; +} + +static void vgxy61_fill_sensor_param(struct vgxy61_dev *sensor) +{ + if (sensor->id == VG5761_MODEL_ID) { + sensor->sensor_width = VGX761_WIDTH; + sensor->sensor_height = VGX761_HEIGHT; + sensor->sensor_modes = vgx761_mode_data; + sensor->sensor_modes_nb = ARRAY_SIZE(vgx761_mode_data); + sensor->default_mode = &vgx761_mode_data[VGX761_DEFAULT_MODE]; + sensor->rot_term = VGX761_SHORT_ROT_TERM; + } else if (sensor->id == VG5661_MODEL_ID) { + sensor->sensor_width = VGX661_WIDTH; + sensor->sensor_height = VGX661_HEIGHT; + sensor->sensor_modes = vgx661_mode_data; + sensor->sensor_modes_nb = ARRAY_SIZE(vgx661_mode_data); + sensor->default_mode = &vgx661_mode_data[VGX661_DEFAULT_MODE]; + sensor->rot_term = VGX661_SHORT_ROT_TERM; + } else { + /* Should never happen */ + WARN_ON(true); + } + sensor->current_mode = sensor->default_mode; + sensor->vblank = sensor->frame_length - sensor->current_mode->crop.height; +} + +static int vgxy61_probe(struct i2c_client *client) +{ + struct device *dev = &client->dev; + struct fwnode_handle *endpoint; + struct vgxy61_dev *sensor; + int ret; + + sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL); + if (!sensor) + return -ENOMEM; + + sensor->i2c_client = client; + sensor->streaming = false; + sensor->hdr = VGXY61_NO_HDR; + sensor->expo_long = 200; + sensor->expo_short = 0; + sensor->hflip = false; + sensor->vflip = false; + sensor->analog_gain = 0; + sensor->digital_gain = 256; + + endpoint = fwnode_graph_get_next_endpoint(of_fwnode_handle(dev->of_node), NULL); + if (!endpoint) { + dev_err(dev, "endpoint node not found\n"); + return -EINVAL; + } + + ret = vgxy61_tx_from_ep(sensor, endpoint); + fwnode_handle_put(endpoint); + if (ret) { + dev_err(dev, "Failed to parse endpoint %d\n", ret); + return ret; + } + + sensor->xclk = devm_clk_get(dev, NULL); + if (IS_ERR(sensor->xclk)) { + dev_err(dev, "failed to get xclk\n"); + return PTR_ERR(sensor->xclk); + } + sensor->clk_freq = clk_get_rate(sensor->xclk); + if (sensor->clk_freq < 6 * HZ_PER_MHZ || sensor->clk_freq > 27 * HZ_PER_MHZ) { + dev_err(dev, "Only 6Mhz-27Mhz clock range supported. provide %lu MHz\n", + sensor->clk_freq / HZ_PER_MHZ); + return -EINVAL; + } + sensor->gpios_polarity = of_property_read_bool(dev->of_node, "invert-gpios-polarity"); + sensor->slave_mode = of_property_read_bool(dev->of_node, "slave-mode"); + + v4l2_i2c_subdev_init(&sensor->sd, client, &vgxy61_subdev_ops); + sensor->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; + sensor->pad.flags = MEDIA_PAD_FL_SOURCE; + sensor->sd.entity.ops = &vgxy61_subdev_entity_ops; + sensor->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR; + + /* Request optional reset pin */ + sensor->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); + + ret = vgxy61_power_on(dev); + if (ret) + return ret; + + ret = vgxy61_detect(sensor); + if (ret) { + dev_err(&client->dev, "sensor detect failed %d\n", ret); + return ret; + } + + vgxy61_fill_sensor_param(sensor); + vgxy61_fill_framefmt(sensor, sensor->current_mode, &sensor->fmt, VGXY61_MEDIA_BUS_FMT_DEF); + + /* HDR mode */ + ret = vgxy61_update_hdr(sensor, sensor->hdr); + if (ret) + return ret; + + mutex_init(&sensor->lock); + + ret = vgxy61_init_controls(sensor); + if (ret) { + dev_err(&client->dev, "controls initialization failed %d\n", ret); + goto error_power_off; + } + + ret = media_entity_pads_init(&sensor->sd.entity, 1, &sensor->pad); + if (ret) { + dev_err(&client->dev, "pads init failed %d\n", ret); + goto error_handler_free; + } + + /* Enable runtime PM and turn off the device */ + pm_runtime_set_active(dev); + pm_runtime_enable(dev); + pm_runtime_idle(dev); + + ret = v4l2_async_register_subdev(&sensor->sd); + if (ret) { + dev_err(&client->dev, "async subdev register failed %d\n", ret); + goto error_pm_runtime; + } + + pm_runtime_set_autosuspend_delay(&client->dev, 1000); + pm_runtime_use_autosuspend(&client->dev); + pm_runtime_put_autosuspend(&client->dev); + + dev_dbg(&client->dev, "vgxy61 probe successfully\n"); + + return 0; + +error_pm_runtime: + pm_runtime_disable(&client->dev); + pm_runtime_set_suspended(&client->dev); + media_entity_cleanup(&sensor->sd.entity); +error_handler_free: + v4l2_ctrl_handler_free(sensor->sd.ctrl_handler); + mutex_destroy(&sensor->lock); +error_power_off: + vgxy61_power_off(dev); + + return ret; +} + +static int vgxy61_remove(struct i2c_client *client) +{ + struct v4l2_subdev *sd = i2c_get_clientdata(client); + struct vgxy61_dev *sensor = to_vgxy61_dev(sd); + + v4l2_async_unregister_subdev(&sensor->sd); + mutex_destroy(&sensor->lock); + media_entity_cleanup(&sensor->sd.entity); + + pm_runtime_disable(&client->dev); + if (!pm_runtime_status_suspended(&client->dev)) + vgxy61_power_off(&client->dev); + pm_runtime_set_suspended(&client->dev); + + return 0; +} + +static const struct of_device_id vgxy61_dt_ids[] = { + { .compatible = "st,st-vgxy61" }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, vgxy61_dt_ids); + +static const struct dev_pm_ops vgxy61_pm_ops = { + SET_RUNTIME_PM_OPS(vgxy61_power_off, vgxy61_power_on, NULL) +}; + +static struct i2c_driver vgxy61_i2c_driver = { + .driver = { + .name = "st-vgxy61", + .of_match_table = vgxy61_dt_ids, + .pm = &vgxy61_pm_ops, + }, + .probe_new = vgxy61_probe, + .remove = vgxy61_remove, +}; + +module_i2c_driver(vgxy61_i2c_driver); + +MODULE_AUTHOR("Benjamin Mugnier <benjamin.mugnier@foss.st.com>"); +MODULE_AUTHOR("Mickael Guene <mickael.guene@st.com>"); +MODULE_AUTHOR("Sylvain Petinot <sylvain.petinot@foss.st.com>"); +MODULE_DESCRIPTION("VGXY61 camera subdev driver"); +MODULE_LICENSE("GPL");
The VGXY61 has a quad lanes CSI-2 output port running at 800mbps per lane, and supports RAW8, RAW10, RAW12, RAW14 and RAW16 formats. The driver handles both sensor types: - VG5661 and VG6661: 1.6 Mpx (1464 x 1104) 75fps. - VG5761 and VG6761: 2.3 Mpx (1944 x 1204) 60 fps. The driver supports: - HDR linearize mode, HDR substraction mode, and no HDR - GPIOs LEDs strobing - Digital binning and analog subsampling - Horizontal and vertical flip - Manual exposure - Analog and digital gains - Test patterns Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com> --- drivers/media/i2c/Kconfig | 10 + drivers/media/i2c/Makefile | 1 + drivers/media/i2c/st-vgxy61.c | 1940 +++++++++++++++++++++++++++++++++ 3 files changed, 1951 insertions(+) create mode 100644 drivers/media/i2c/st-vgxy61.c