Hi Sakari, Thanks for your review. On Thu, Oct 26, 2023 at 01:18:21PM +0000, Sakari Ailus wrote: > Hi Tommaso, > > Thanks for the update. > > There's still quite a bit to do in this driver. Feel free to ask further > questions regarding the comments. > > On Fri, Oct 20, 2023 at 04:13:51PM +0200, Tommaso Merciai wrote: > > The Alvium camera is shipped with sensor + isp in the same housing. > > The camera can be equipped with one out of various sensor and abstract > > the user from this. Camera is connected via MIPI CSI-2. > > > > Most of the camera module features are supported, with the main exception > > being fw update. > > > > The driver provides all mandatory, optional and recommended V4L2 controls > > for maximum compatibility with libcamera > > > > References: > > - https://www.alliedvision.com/en/products/embedded-vision-solutions > > > > Signed-off-by: Tommaso Merciai <tomm.merciai@xxxxxxxxx> > > --- > > Changes since v2: > > - Removed gpios/clock handling as suggested by LPinchart > > - Added vcc-ext-in supply support as suggested by LPinchart > > - Fixed alvium_setup_mipi_fmt funct as suggested by CJAILLET > > - Removed upside_down/hshake_bit priv data as suggested by CJAILLET > > - Fixed commit body as suggested by LPinchart > > - Mv alvium_set_streamon_delay to yalvium_set_lp2hs_delay > > - Fixed comment on lp2hs prop as suggested by LPinchart > > - Added pm resume/suspend functs as suggested by LPinchart > > - Dropped alvium_link_setup/alvium_s_power as suggested by LPinchart > > - Fixed regs defines as suggested by LPinchart > > - Fixed typedef as suggested by LPinchart > > - Dropped bcrm_v/fw_v from priv data as suggested by LPinchart > > - Now driver use the subdev active state to store the active format and crop > > as suggested by LPinchart > > - Dropped alvium_is_csi2/i2c_to_alvium as suggested by LPinchart > > > > Changes since v3: > > - Fixed warnings Reported-by: kernel test robot <lkp@xxxxxxxxx> > > > > Changes since v4: > > - Removed print into alvium_get_dt_data for alliedvision,lp2hs-delay-us as > > suggested by CDooley > > > > Changes since v5: > > - Used tab instead of space in .h as suggested by SAilus > > - Added support for new CCI API from HDeGoede as suggested by SAilus > > - Fixed alvium_write/alvium_read, functions now using the new CCI api, suggested by LPinchart > > - Fixed alvium_get_feat_inq func as suggested by SAilus > > - Fixed indentation/var-order/includes-order as suggested by SAilus > > - Fixed alvium_csi2_fmts with MIPI_CSI2_DT_ defines as suggested by SAilus > > - Fixed alvium_is_alive as suggested by SAilus > > - Fixed alvium_code_to_pixfmt funct as suggested by SAilus > > - Fixed alvium_get_dt_data function, now use only fwnode as suggested by SAilus > > - Fixed autosuspend into the probe, is disable as default as suggested by SAilus > > - Fixed alvium_get_dt_data function, assigned bus type before parsing the ep > > as suggested by SAilus > > - Fixed alvium_power_off, removed wrong print as suggested by SAilus > > > > Changes since v6: > > - Fixed .h indentation > > - Fixed function params indentation > > - Added int *err params for alvium_read/alvium_write as suggested by LPinchart > > - Removed dbg print from the driver, driver is now using dbg/err prints that comes from > > new cci API as suggested by LPinchart. This, fits SAilus suggestion on common pattern function. > > - Fixed alvium_write_hshake, now use read_poll_timeout as suggested by LPinchart > > - Removed useless includes > > - Added maintainers file entries > > > > Changes since v7: > > - Fix company legal entity from Inc. to GmbH > > - Fix warnings given from HVerkuil build-scripts in alvium_get_bcrm_vers, > > alvium_get_fw_version and probe functions using __le16/__le32. Fixed also > > probe function warning alvium-csi2.c:2665 alvium_probe() warn: missing error code? 'ret' > > > > Changes since v8: > > - Fixed alvium_i2c_driver struct, use probe istead of probe_new > > - Fixed Kconfig description taking as reference new mt9m114 driver > > - Fixed Kconfig just select V4L2_CCI_I2C taking as reference new mt9m114 driver > > > > Changes since v9: > > - Fixed Y8_1X8 mipi_fmt_regval > > - Removed alliedvision,lp2hs-delay-us property we set now a default safe value as discussed with SAilus > > - Added dft property for ctrls initialization, we first read dft values from the camera and set this into ctrls > > - Fixed indentation as suggested by SAilus > > - Fixed bit field definitions alignment into .h as suggested by SAilus > > - Fixed Heartbeat reg from R -> RW > > - Fixed adjusting values in format/crop changes as suggested by SAilus > > - Removed unnecessary brcm_addr checks as suggested by SAilus > > - Merged poweron/poweroff functions as suggested by SAilus > > - Added poweroff path during probe as suggested by SAilus > > - Fixed module license type as suggested by SAilus > > - Removed unnecessary MODULE_DEVICE_TABLE as suggested by SAilus > > - Fixed pm support in s_ctrl and s_stream functions > > - Removed unnecessary local variables as suggested by SAilus > > - Added ret values checks as suggested by SAilus > > > > MAINTAINERS | 9 + > > drivers/media/i2c/Kconfig | 10 + > > drivers/media/i2c/Makefile | 1 + > > drivers/media/i2c/alvium-csi2.c | 2666 +++++++++++++++++++++++++++++++ > > drivers/media/i2c/alvium-csi2.h | 489 ++++++ > > 5 files changed, 3175 insertions(+) > > create mode 100644 drivers/media/i2c/alvium-csi2.c > > create mode 100644 drivers/media/i2c/alvium-csi2.h > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index f3e6dbbbbccb..98d322880c96 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -709,6 +709,15 @@ S: Maintained > > F: Documentation/devicetree/bindings/media/allegro,al5e.yaml > > F: drivers/media/platform/allegro-dvt/ > > > > +ALLIED VISION ALVIUM CAMERA DRIVER > > +M: Tommaso Merciai <tomm.merciai@xxxxxxxxx> > > +M: Martin Hecht <martin.hecht@xxxxxxxx> > > +L: linux-media@xxxxxxxxxxxxxxx > > +S: Maintained > > +F: Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml > > +F: drivers/media/i2c/alvium-csi2.c > > +F: drivers/media/i2c/alvium-csi2.h > > + > > ALLWINNER A10 CSI DRIVER > > M: Maxime Ripard <mripard@xxxxxxxxxx> > > L: linux-media@xxxxxxxxxxxxxxx > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig > > index d182c3514fb5..8525cfde26ba 100644 > > --- a/drivers/media/i2c/Kconfig > > +++ b/drivers/media/i2c/Kconfig > > @@ -41,6 +41,16 @@ config VIDEO_APTINA_PLL > > config VIDEO_CCS_PLL > > tristate > > > > +config VIDEO_ALVIUM_CSI2 > > + tristate "Allied Vision ALVIUM MIPI CSI-2 camera support" > > + select V4L2_CCI_I2C > > + help > > + This is a Video4Linux2 sensor-level driver for the Allied Vision > > + ALVIUM camera connected via MIPI CSI-2 interface. > > + > > + To compile this driver as a module, choose M here: the > > + module will be called alvium-csi2. > > + > > config VIDEO_AR0521 > > tristate "ON Semiconductor AR0521 sensor support" > > help > > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile > > index f5010f80a21f..d75aa7f74315 100644 > > --- a/drivers/media/i2c/Makefile > > +++ b/drivers/media/i2c/Makefile > > @@ -17,6 +17,7 @@ obj-$(CONFIG_VIDEO_ADV7604) += adv7604.o > > obj-$(CONFIG_VIDEO_ADV7842) += adv7842.o > > obj-$(CONFIG_VIDEO_AK7375) += ak7375.o > > obj-$(CONFIG_VIDEO_AK881X) += ak881x.o > > +obj-$(CONFIG_VIDEO_ALVIUM_CSI2) += alvium-csi2.o > > obj-$(CONFIG_VIDEO_APTINA_PLL) += aptina-pll.o > > obj-$(CONFIG_VIDEO_AR0521) += ar0521.o > > obj-$(CONFIG_VIDEO_BT819) += bt819.o > > diff --git a/drivers/media/i2c/alvium-csi2.c b/drivers/media/i2c/alvium-csi2.c > > new file mode 100644 > > index 000000000000..2c40804655cd > > --- /dev/null > > +++ b/drivers/media/i2c/alvium-csi2.c > > @@ -0,0 +1,2666 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Allied Vision Technologies GmbH Alvium camera driver > > + * > > + * Copyright (C) 2023 Tommaso Merciai > > + * Copyright (C) 2023 Martin Hecht > > + * Copyright (C) 2023 Avnet EMG GmbH > > + */ > > + > > +#include <linux/i2c.h> > > +#include <linux/module.h> > > +#include <linux/pm_runtime.h> > > +#include <linux/regmap.h> > > +#include <linux/regulator/consumer.h> > > +#include <media/mipi-csi2.h> > > +#include <media/v4l2-async.h> > > +#include <media/v4l2-ctrls.h> > > +#include <media/v4l2-device.h> > > +#include <media/v4l2-event.h> > > +#include <media/v4l2-fwnode.h> > > +#include <media/v4l2-subdev.h> > > + > > +#include "alvium-csi2.h" > > + > > +static const struct v4l2_mbus_framefmt alvium_csi2_default_fmt = { > > + .code = MEDIA_BUS_FMT_UYVY8_1X16, > > + .width = 640, > > + .height = 480, > > + .colorspace = V4L2_COLORSPACE_SRGB, > > + .ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(V4L2_COLORSPACE_SRGB), > > + .quantization = V4L2_QUANTIZATION_FULL_RANGE, > > + .xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(V4L2_COLORSPACE_SRGB), > > + .field = V4L2_FIELD_NONE, > > +}; > > + > > +static const struct alvium_pixfmt alvium_csi2_fmts[] = { > > + { > > + /* UYVY8_2X8 */ > > + .id = ALVIUM_FMT_UYVY8_2X8, > > + .code = MEDIA_BUS_FMT_UYVY8_2X8, > > + .colorspace = V4L2_COLORSPACE_SRGB, > > + .fmt_av_bit = ALVIUM_BIT_YUV422_8, > > + .bay_av_bit = ALVIUM_BIT_BAY_NONE, > > + .mipi_fmt_regval = MIPI_CSI2_DT_YUV422_8B, > > + .bay_fmt_regval = -1, > > + .is_raw = 0, > > + }, { > > + /* UYVY8_1X16 */ > > + .id = ALVIUM_FMT_UYVY8_1X16, > > + .code = MEDIA_BUS_FMT_UYVY8_1X16, > > + .colorspace = V4L2_COLORSPACE_SRGB, > > + .fmt_av_bit = ALVIUM_BIT_YUV422_8, > > + .bay_av_bit = ALVIUM_BIT_BAY_NONE, > > + .mipi_fmt_regval = MIPI_CSI2_DT_YUV422_8B, > > + .bay_fmt_regval = -1, > > + .is_raw = 0, > > + }, { > > + /* YUYV8_1X16 */ > > + .id = ALVIUM_FMT_YUYV8_1X16, > > + .code = MEDIA_BUS_FMT_YUYV8_1X16, > > + .colorspace = V4L2_COLORSPACE_SRGB, > > + .fmt_av_bit = ALVIUM_BIT_YUV422_8, > > + .bay_av_bit = ALVIUM_BIT_BAY_NONE, > > + .mipi_fmt_regval = MIPI_CSI2_DT_YUV422_8B, > > + .bay_fmt_regval = -1, > > + .is_raw = 0, > > + }, { > > + /* YUYV8_2X8 */ > > + .id = ALVIUM_FMT_YUYV8_2X8, > > + .code = MEDIA_BUS_FMT_YUYV8_2X8, > > + .colorspace = V4L2_COLORSPACE_SRGB, > > + .fmt_av_bit = ALVIUM_BIT_YUV422_8, > > + .bay_av_bit = ALVIUM_BIT_BAY_NONE, > > + .mipi_fmt_regval = MIPI_CSI2_DT_YUV422_8B, > > + .bay_fmt_regval = -1, > > + .is_raw = 0, > > + }, { > > + /* YUYV10_1X20 */ > > + .id = ALVIUM_FMT_YUYV10_1X20, > > + .code = MEDIA_BUS_FMT_YUYV10_1X20, > > + .colorspace = V4L2_COLORSPACE_SRGB, > > + .fmt_av_bit = ALVIUM_BIT_YUV422_10, > > + .bay_av_bit = ALVIUM_BIT_BAY_NONE, > > + .mipi_fmt_regval = MIPI_CSI2_DT_YUV422_10B, > > + .bay_fmt_regval = -1, > > + .is_raw = 0, > > + }, { > > + /* RGB888_1X24 */ > > + .id = ALVIUM_FMT_RGB888_1X24, > > + .code = MEDIA_BUS_FMT_RGB888_1X24, > > + .colorspace = V4L2_COLORSPACE_SRGB, > > + .fmt_av_bit = ALVIUM_BIT_RGB888, > > + .bay_av_bit = ALVIUM_BIT_BAY_NONE, > > + .mipi_fmt_regval = MIPI_CSI2_DT_RGB888, > > + .bay_fmt_regval = -1, > > + .is_raw = 0, > > + }, { > > + /* RBG888_1X24 */ > > + .id = ALVIUM_FMT_RBG888_1X24, > > + .code = MEDIA_BUS_FMT_RBG888_1X24, > > + .colorspace = V4L2_COLORSPACE_SRGB, > > + .fmt_av_bit = ALVIUM_BIT_RGB888, > > + .bay_av_bit = ALVIUM_BIT_BAY_NONE, > > + .mipi_fmt_regval = MIPI_CSI2_DT_RGB888, > > + .bay_fmt_regval = -1, > > + .is_raw = 0, > > + }, { > > + /* BGR888_1X24 */ > > + .id = ALVIUM_FMT_BGR888_1X24, > > + .code = MEDIA_BUS_FMT_BGR888_1X24, > > + .colorspace = V4L2_COLORSPACE_SRGB, > > + .fmt_av_bit = ALVIUM_BIT_RGB888, > > + .bay_av_bit = ALVIUM_BIT_BAY_NONE, > > + .mipi_fmt_regval = MIPI_CSI2_DT_RGB888, > > + .bay_fmt_regval = -1, > > + .is_raw = 0, > > + }, { > > + /* RGB888_3X8 */ > > + .id = ALVIUM_FMT_RGB888_3X8, > > + .code = MEDIA_BUS_FMT_RGB888_3X8, > > + .colorspace = V4L2_COLORSPACE_SRGB, > > + .fmt_av_bit = ALVIUM_BIT_RGB888, > > + .bay_av_bit = ALVIUM_BIT_BAY_NONE, > > + .mipi_fmt_regval = MIPI_CSI2_DT_RGB888, > > + .bay_fmt_regval = -1, > > + .is_raw = 0, > > + }, { > > + /* Y8_1X8 */ > > + .id = ALVIUM_FMT_Y8_1X8, > > + .code = MEDIA_BUS_FMT_Y8_1X8, > > + .colorspace = V4L2_COLORSPACE_RAW, > > + .fmt_av_bit = ALVIUM_BIT_RAW8, > > + .bay_av_bit = ALVIUM_BIT_BAY_MONO, > > + .mipi_fmt_regval = MIPI_CSI2_DT_RAW8, > > + .bay_fmt_regval = 0x00, > > + .is_raw = 1, > > + }, { > > + /* SGRBG8_1X8 */ > > + .id = ALVIUM_FMT_SGRBG8_1X8, > > + .code = MEDIA_BUS_FMT_SGRBG8_1X8, > > + .colorspace = V4L2_COLORSPACE_RAW, > > + .fmt_av_bit = ALVIUM_BIT_RAW8, > > + .bay_av_bit = ALVIUM_BIT_BAY_GR, > > + .mipi_fmt_regval = MIPI_CSI2_DT_RAW8, > > + .bay_fmt_regval = 0x01, > > + .is_raw = 1, > > + }, { > > + /* SRGGB8_1X8 */ > > + .id = ALVIUM_FMT_SRGGB8_1X8, > > + .code = MEDIA_BUS_FMT_SRGGB8_1X8, > > + .colorspace = V4L2_COLORSPACE_RAW, > > + .fmt_av_bit = ALVIUM_BIT_RAW8, > > + .bay_av_bit = ALVIUM_BIT_BAY_RG, > > + .mipi_fmt_regval = MIPI_CSI2_DT_RAW8, > > + .bay_fmt_regval = 0x02, > > + .is_raw = 1, > > + }, { > > + /* SGBRG8_1X8 */ > > + .id = ALVIUM_FMT_SGBRG8_1X8, > > + .code = MEDIA_BUS_FMT_SGBRG8_1X8, > > + .colorspace = V4L2_COLORSPACE_RAW, > > + .fmt_av_bit = ALVIUM_BIT_RAW8, > > + .bay_av_bit = ALVIUM_BIT_BAY_GB, > > + .mipi_fmt_regval = MIPI_CSI2_DT_RAW8, > > + .bay_fmt_regval = 0x03, > > + .is_raw = 1, > > + }, { > > + /* SBGGR8_1X8 */ > > + .id = ALVIUM_FMT_SBGGR8_1X8, > > + .code = MEDIA_BUS_FMT_SBGGR8_1X8, > > + .colorspace = V4L2_COLORSPACE_RAW, > > + .fmt_av_bit = ALVIUM_BIT_RAW8, > > + .bay_av_bit = ALVIUM_BIT_BAY_BG, > > + .mipi_fmt_regval = MIPI_CSI2_DT_RAW8, > > + .bay_fmt_regval = 0x04, > > + .is_raw = 1, > > + }, { > > + /* Y10_1X10 */ > > + .id = ALVIUM_FMT_Y10_1X10, > > + .code = MEDIA_BUS_FMT_Y10_1X10, > > + .colorspace = V4L2_COLORSPACE_RAW, > > + .fmt_av_bit = ALVIUM_BIT_RAW10, > > + .bay_av_bit = ALVIUM_BIT_BAY_MONO, > > + .mipi_fmt_regval = MIPI_CSI2_DT_RAW10, > > + .bay_fmt_regval = 0x00, > > + .is_raw = 1, > > + }, { > > + /* SGRBG10_1X10 */ > > + .id = ALVIUM_FMT_SGRBG10_1X10, > > + .code = MEDIA_BUS_FMT_SGRBG10_1X10, > > + .colorspace = V4L2_COLORSPACE_RAW, > > + .fmt_av_bit = ALVIUM_BIT_RAW10, > > + .bay_av_bit = ALVIUM_BIT_BAY_GR, > > + .mipi_fmt_regval = MIPI_CSI2_DT_RAW10, > > + .bay_fmt_regval = 0x01, > > + .is_raw = 1, > > + }, { > > + /* SRGGB10_1X10 */ > > + .id = ALVIUM_FMT_SRGGB10_1X10, > > + .code = MEDIA_BUS_FMT_SRGGB10_1X10, > > + .colorspace = V4L2_COLORSPACE_RAW, > > + .fmt_av_bit = ALVIUM_BIT_RAW10, > > + .bay_av_bit = ALVIUM_BIT_BAY_RG, > > + .mipi_fmt_regval = MIPI_CSI2_DT_RAW10, > > + .bay_fmt_regval = 0x02, > > + .is_raw = 1, > > + }, { > > + /* SGBRG10_1X10 */ > > + .id = ALVIUM_FMT_SGBRG10_1X10, > > + .code = MEDIA_BUS_FMT_SGBRG10_1X10, > > + .colorspace = V4L2_COLORSPACE_RAW, > > + .fmt_av_bit = ALVIUM_BIT_RAW10, > > + .bay_av_bit = ALVIUM_BIT_BAY_GB, > > + .mipi_fmt_regval = MIPI_CSI2_DT_RAW10, > > + .bay_fmt_regval = 0x03, > > + .is_raw = 1, > > + }, { > > + /* SBGGR10_1X10 */ > > + .id = ALVIUM_FMT_SBGGR10_1X10, > > + .code = MEDIA_BUS_FMT_SBGGR10_1X10, > > + .colorspace = V4L2_COLORSPACE_RAW, > > + .fmt_av_bit = ALVIUM_BIT_RAW10, > > + .bay_av_bit = ALVIUM_BIT_BAY_BG, > > + .mipi_fmt_regval = MIPI_CSI2_DT_RAW10, > > + .bay_fmt_regval = 0x04, > > + .is_raw = 1, > > + }, { > > + /* Y12_1X12 */ > > + .id = ALVIUM_FMT_Y12_1X12, > > + .code = MEDIA_BUS_FMT_Y12_1X12, > > + .colorspace = V4L2_COLORSPACE_RAW, > > + .fmt_av_bit = ALVIUM_BIT_RAW12, > > + .bay_av_bit = ALVIUM_BIT_BAY_MONO, > > + .mipi_fmt_regval = MIPI_CSI2_DT_RAW12, > > + .bay_fmt_regval = 0x00, > > + .is_raw = 1, > > + }, { > > + /* SGRBG12_1X12 */ > > + .id = ALVIUM_FMT_SGRBG12_1X12, > > + .code = MEDIA_BUS_FMT_SGRBG12_1X12, > > + .colorspace = V4L2_COLORSPACE_RAW, > > + .fmt_av_bit = ALVIUM_BIT_RAW12, > > + .bay_av_bit = ALVIUM_BIT_BAY_GR, > > + .mipi_fmt_regval = MIPI_CSI2_DT_RAW12, > > + .bay_fmt_regval = 0x01, > > + .is_raw = 1, > > + }, { > > + /* SRGGB12_1X12 */ > > + .id = ALVIUM_FMT_SRGGB12_1X12, > > + .code = MEDIA_BUS_FMT_SRGGB12_1X12, > > + .colorspace = V4L2_COLORSPACE_RAW, > > + .fmt_av_bit = ALVIUM_BIT_RAW12, > > + .bay_av_bit = ALVIUM_BIT_BAY_RG, > > + .mipi_fmt_regval = MIPI_CSI2_DT_RAW12, > > + .bay_fmt_regval = 0x02, > > + .is_raw = 1, > > + }, { > > + /* SGBRG12_1X12 */ > > + .id = ALVIUM_FMT_SGBRG12_1X12, > > + .code = MEDIA_BUS_FMT_SGBRG12_1X12, > > + .colorspace = V4L2_COLORSPACE_RAW, > > + .fmt_av_bit = ALVIUM_BIT_RAW12, > > + .bay_av_bit = ALVIUM_BIT_BAY_GB, > > + .mipi_fmt_regval = MIPI_CSI2_DT_RAW12, > > + .bay_fmt_regval = 0x03, > > + .is_raw = 1, > > + }, { > > + /* SBGGR12_1X12 */ > > + .id = ALVIUM_FMT_SBGGR12_1X12, > > + .code = MEDIA_BUS_FMT_SBGGR12_1X12, > > + .colorspace = V4L2_COLORSPACE_RAW, > > + .fmt_av_bit = ALVIUM_BIT_RAW12, > > + .bay_av_bit = ALVIUM_BIT_BAY_BG, > > + .mipi_fmt_regval = MIPI_CSI2_DT_RAW12, > > + .bay_fmt_regval = 0x04, > > + .is_raw = 1, > > + }, { > > + /* SBGGR14_1X14 */ > > + .id = ALVIUM_FMT_SBGGR14_1X14, > > + .code = MEDIA_BUS_FMT_SBGGR14_1X14, > > + .colorspace = V4L2_COLORSPACE_RAW, > > + .fmt_av_bit = ALVIUM_BIT_RAW14, > > + .bay_av_bit = ALVIUM_BIT_BAY_GR, > > + .mipi_fmt_regval = MIPI_CSI2_DT_RAW14, > > + .bay_fmt_regval = 0x01, > > + .is_raw = 1, > > + }, { > > + /* SGBRG14_1X14 */ > > + .id = ALVIUM_FMT_SGBRG14_1X14, > > + .code = MEDIA_BUS_FMT_SGBRG14_1X14, > > + .colorspace = V4L2_COLORSPACE_RAW, > > + .fmt_av_bit = ALVIUM_BIT_RAW14, > > + .bay_av_bit = ALVIUM_BIT_BAY_RG, > > + .mipi_fmt_regval = MIPI_CSI2_DT_RAW14, > > + .bay_fmt_regval = 0x02, > > + .is_raw = 1, > > + }, { > > + /* SRGGB14_1X14 */ > > + .id = ALVIUM_FMT_SRGGB14_1X14, > > + .code = MEDIA_BUS_FMT_SRGGB14_1X14, > > + .colorspace = V4L2_COLORSPACE_RAW, > > + .fmt_av_bit = ALVIUM_BIT_RAW14, > > + .bay_av_bit = ALVIUM_BIT_BAY_GB, > > + .mipi_fmt_regval = MIPI_CSI2_DT_RAW14, > > + .bay_fmt_regval = 0x03, > > + .is_raw = 1, > > + }, { > > + /* SGRBG14_1X14 */ > > + .id = ALVIUM_FMT_SGRBG14_1X14, > > + .code = MEDIA_BUS_FMT_SGRBG14_1X14, > > + .colorspace = V4L2_COLORSPACE_RAW, > > + .fmt_av_bit = ALVIUM_BIT_RAW14, > > + .bay_av_bit = ALVIUM_BIT_BAY_BG, > > + .mipi_fmt_regval = MIPI_CSI2_DT_RAW14, > > + .bay_fmt_regval = 0x04, > > + .is_raw = 1, > > + }, > > + { /* sentinel */ } > > +}; > > + > > +static int alvium_read(struct alvium_dev *alvium, u32 reg, u64 *val, int *err) > > +{ > > + if (reg & REG_BCRM_V4L2) { > > + reg &= ~REG_BCRM_V4L2; > > + reg += alvium->bcrm_addr; > > + } > > + > > + return cci_read(alvium->regmap, reg, val, err); > > +} > > + > > +static int alvium_write(struct alvium_dev *alvium, u32 reg, u64 val, int *err) > > +{ > > + if (reg & REG_BCRM_V4L2) { > > + reg &= ~REG_BCRM_V4L2; > > + reg += alvium->bcrm_addr; > > + } > > + > > + return cci_write(alvium->regmap, reg, val, err); > > +} > > + > > +static int alvium_write_hshake(struct alvium_dev *alvium, u32 reg, u64 val) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + u64 hshake_bit; > > + int ret = 0; > > + > > + /* reset handshake bit and write alvium reg */ > > + alvium_write(alvium, REG_BCRM_WRITE_HANDSHAKE_RW, 0, &ret); > > + alvium_write(alvium, reg, val, &ret); > > + if (ret) { > > + dev_err(dev, "Fail to write reg\n"); > > + return ret; > > + } > > + > > + /* poll handshake bit since bit0 = 1 */ > > + read_poll_timeout(alvium_read, hshake_bit, > > + ((hshake_bit & BCRM_HANDSHAKE_W_DONE_EN_BIT) == 1), > > + 15000, 45000, true, > > + alvium, REG_BCRM_WRITE_HANDSHAKE_RW, &hshake_bit, &ret); > > + if (ret) { > > + dev_err(dev, "poll bit[0] = 1, hshake reg fail\n"); > > + return ret; > > + } > > + > > + /* reset handshake bit, write 0 to bit0 */ > > + alvium_write(alvium, REG_BCRM_WRITE_HANDSHAKE_RW, 0, &ret); > > + if (ret) { > > + dev_err(dev, "Fail to reset hshake reg\n"); > > + return ret; > > + } > > + > > + /* poll handshake bit since bit0 = 0 */ > > + read_poll_timeout(alvium_read, hshake_bit, > > + ((hshake_bit & BCRM_HANDSHAKE_W_DONE_EN_BIT) == 0), > > + 15000, 45000, true, > > + alvium, REG_BCRM_WRITE_HANDSHAKE_RW, &hshake_bit, &ret); > > + if (ret) { > > + dev_err(dev, "poll bit[0] = 0, hshake reg fail\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_get_bcrm_vers(struct alvium_dev *alvium) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + struct alvium_bcrm_vers *v; > > + u64 val; > > + int ret = 0; > > + > > + alvium_read(alvium, REG_BCRM_VERSION_R, &val, &ret); > > + if (ret) { > > + dev_err(dev, "fail to read bcrm version reg\n"); > > + return ret; > > + } > > + > > + v = (struct alvium_bcrm_vers *) &val; > > + dev_info(dev, "bcrm version: %u.%u\n", v->minor, v->major); > > + > > + return ret; > > +} > > + > > +static int alvium_get_fw_version(struct alvium_dev *alvium) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + struct alvium_fw_vers *fw_v; > > + u64 val; > > + int ret = 0; > > + > > + alvium_read(alvium, REG_BCRM_DEVICE_FIRMWARE_VERSION_R, &val, &ret); > > + if (ret) { > > + dev_err(dev, "fail to read bcrm device fw version reg\n"); > > + return ret; > > + } > > + > > + fw_v = (struct alvium_fw_vers *) &val; > > + dev_info(dev, "fw version: %u.%u.%u.%u\n", fw_v->special, fw_v->major, > > + fw_v->minor, fw_v->patch); > > + > > + return ret; > > +} > > + > > +static int alvium_get_bcrm_addr(struct alvium_dev *alvium) > > +{ > > + u64 val; > > + int ret = 0; > > + > > + alvium_read(alvium, REG_BCRM_REG_ADDR_R, &val, &ret); > > + if (ret) > > + return ret; > > + > > + alvium->bcrm_addr = val; > > + > > + return ret; > > +} > > + > > +static int alvium_is_alive(struct alvium_dev *alvium) > > +{ > > + u64 val = 0; > > + u8 hbeat_val = 0; > > + int ret = 0; > > + > > + alvium_read(alvium, REG_BCRM_VERSION_R, &val, &ret); > > + alvium_read(alvium, REG_BCRM_HEARTBEAT_RW, &val, &ret); > > + > > + if (ret) > > + return ret; > > + > > + hbeat_val = val; > > + > > + return hbeat_val; > > +} > > + > > +static void alvium_print_avail_mipi_fmt(struct alvium_dev *alvium) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + > > + dev_dbg(dev, "avail mipi_fmt yuv420_8_leg: %u\n", > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_YUV420_8_LEG]); > > + dev_dbg(dev, "avail mipi_fmt yuv420_8: %u\n", > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_YUV420_8]); > > + dev_dbg(dev, "avail mipi_fmt yuv420_10: %u\n", > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_YUV420_10]); > > + dev_dbg(dev, "avail mipi_fmt yuv420_8_csps: %u\n", > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_YUV420_8_CSPS]); > > + dev_dbg(dev, "avail mipi_fmt yuv420_10_csps: %u\n", > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_YUV420_10_CSPS]); > > + dev_dbg(dev, "avail mipi_fmt yuv422_8: %u\n", > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_YUV422_8]); > > + dev_dbg(dev, "avail mipi_fmt yuv422_10: %u\n", > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_YUV422_10]); > > + dev_dbg(dev, "avail mipi_fmt rgb888: %u\n", > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_RGB888]); > > + dev_dbg(dev, "avail mipi_fmt rgb666: %u\n", > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_RGB666]); > > + dev_dbg(dev, "avail mipi_fmt rgb565: %u\n", > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_RGB565]); > > + dev_dbg(dev, "avail mipi_fmt rgb555: %u\n", > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_RGB555]); > > + dev_dbg(dev, "avail mipi_fmt rgb444: %u\n", > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_RGB444]); > > + dev_dbg(dev, "avail mipi_fmt raw6: %u\n", > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_RAW6]); > > + dev_dbg(dev, "avail mipi_fmt raw7: %u\n", > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_RAW7]); > > + dev_dbg(dev, "avail mipi_fmt raw8: %u\n", > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_RAW8]); > > + dev_dbg(dev, "avail mipi_fmt raw10: %u\n", > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_RAW10]); > > + dev_dbg(dev, "avail mipi_fmt raw12: %u\n", > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_RAW12]); > > + dev_dbg(dev, "avail mipi_fmt raw14: %u\n", > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_RAW14]); > > + dev_dbg(dev, "avail mipi_fmt jpeg: %u\n", > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_JPEG]); > > +} > > + > > +static void alvium_print_avail_feat(struct alvium_dev *alvium) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + > > + dev_dbg(dev, "feature rev_x: %u\n", alvium->avail_ft.rev_x); > > + dev_dbg(dev, "feature rev_y: %u\n", alvium->avail_ft.rev_y); > > + dev_dbg(dev, "feature int_autop: %u\n", alvium->avail_ft.int_autop); > > + dev_dbg(dev, "feature black_lvl: %u\n", alvium->avail_ft.black_lvl); > > + dev_dbg(dev, "feature gain: %u\n", alvium->avail_ft.gain); > > + dev_dbg(dev, "feature gamma: %u\n", alvium->avail_ft.gamma); > > + dev_dbg(dev, "feature contrast: %u\n", alvium->avail_ft.contrast); > > + dev_dbg(dev, "feature sat: %u\n", alvium->avail_ft.sat); > > + dev_dbg(dev, "feature hue: %u\n", alvium->avail_ft.hue); > > + dev_dbg(dev, "feature whiteb: %u\n", alvium->avail_ft.whiteb); > > + dev_dbg(dev, "feature sharp: %u\n", alvium->avail_ft.sharp); > > + dev_dbg(dev, "feature auto_exp: %u\n", alvium->avail_ft.auto_exp); > > + dev_dbg(dev, "feature auto_gain: %u\n", alvium->avail_ft.auto_gain); > > + dev_dbg(dev, "feature auto_whiteb: %u\n", alvium->avail_ft.auto_whiteb); > > + dev_dbg(dev, "feature dev_temp: %u\n", alvium->avail_ft.dev_temp); > > + dev_dbg(dev, "feature acq_abort: %u\n", alvium->avail_ft.acq_abort); > > + dev_dbg(dev, "feature acq_fr: %u\n", alvium->avail_ft.acq_fr); > > + dev_dbg(dev, "feature fr_trigger: %u\n", alvium->avail_ft.fr_trigger); > > + dev_dbg(dev, "feature exp_acq_line: %u\n", alvium->avail_ft.exp_acq_line); > > +} > > + > > +static void alvium_print_avail_bayer(struct alvium_dev *alvium) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + > > + dev_dbg(dev, "avail bayer mono: %u\n", > > + alvium->is_bay_avail[ALVIUM_BIT_BAY_MONO]); > > + dev_dbg(dev, "avail bayer gr: %u\n", > > + alvium->is_bay_avail[ALVIUM_BIT_BAY_GR]); > > + dev_dbg(dev, "avail bayer rg: %u\n", > > + alvium->is_bay_avail[ALVIUM_BIT_BAY_RG]); > > + dev_dbg(dev, "avail bayer gb: %u\n", > > + alvium->is_bay_avail[ALVIUM_BIT_BAY_GB]); > > + dev_dbg(dev, "avail bayer bg: %u\n", > > + alvium->is_bay_avail[ALVIUM_BIT_BAY_BG]); > > +} > > + > > +static int alvium_get_feat_inq(struct alvium_dev *alvium) > > +{ > > + struct alvium_avail_feat *f; > > + u64 val; > > + int ret = 0; > > + > > + ret = alvium_read(alvium, REG_BCRM_FEATURE_INQUIRY_R, &val, &ret); > > + f = (struct alvium_avail_feat *) &val; > > + > > + alvium->avail_ft = *f; > > + alvium_print_avail_feat(alvium); > > + > > + return ret; > > +} > > + > > +static int alvium_get_host_supp_csi_lanes(struct alvium_dev *alvium) > > +{ > > + u64 val; > > + int ret = 0; > > + > > + alvium_read(alvium, REG_BCRM_CSI2_LANE_COUNT_RW, &val, &ret); > > Missing error checking before the use of the value. The same pattern > remains prevalent throughout the driver. > > I think it'd be easier if you didn't use a temporary variable for reading, > but instead had a register width specific access function. You could even > introduce a helper macro to read this information as I suggested in an > earlier review. oks. We are moving to use the following macros: #define alvium_read_check(alvium, reg, value) \ { \ int ret = alvium_read(alvium, reg, value, NULL); \ if (ret) \ return ret; \ } > > > + alvium->h_sup_csi_lanes = val; > > + > > + return ret; > > +} > > + > > +static int alvium_set_csi_lanes(struct alvium_dev *alvium) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + u64 num_lanes; > > + int ret; > > + > > + num_lanes = alvium->ep.bus.mipi_csi2.num_data_lanes; > > + > > + if (num_lanes > alvium->h_sup_csi_lanes) > > + return -EINVAL; > > + > > + ret = alvium_write_hshake(alvium, REG_BCRM_CSI2_LANE_COUNT_RW, > > + num_lanes); > > + if (ret) { > > + dev_err(dev, "Fail to set csi lanes reg\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_set_lp2hs_delay(struct alvium_dev *alvium) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + int ret = 0; > > + > > + /* > > + * The purpose of this reg is force a DPhy reset > > + * for the period described by the millisecond on > > + * the reg, before it starts streaming. > > + * > > + * To be clear, with that value bigger than 0 the > > + * Alvium forces a dphy-reset on all lanes for that period. > > + * That means all lanes go up into low power state. > > + * > > + */ > > + alvium_write(alvium, > > + REG_BCRM_LP2HS_DELAY_RW, ALVIUM_LP2HS_DELAY_MS, &ret); > > + if (ret) { > > + dev_err(dev, "Fail to set lp2hs delay reg\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_get_csi_clk_params(struct alvium_dev *alvium) > > +{ > > + u64 val; > > + int ret = 0; > > + > > + alvium_read(alvium, REG_BCRM_CSI2_CLOCK_MIN_R, &val, &ret); > > + alvium->min_csi_clk = val; > > + > > + alvium_read(alvium, REG_BCRM_CSI2_CLOCK_MAX_R, &val, &ret); > > + alvium->max_csi_clk = val; > > + > > + return ret; > > +} > > + > > +static int alvium_set_csi_clk(struct alvium_dev *alvium) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + u64 csi_clk; > > + int ret; > > + > > + csi_clk = (u32)alvium->ep.link_frequencies[0]; > > Why casting to u32? Shouldn't csi_clk be u32 instead? Ok we fix this in v11. Change to use u64 for calculation because type of ep.link_frequencies[0] Plan is to clamp csi_clk between min/max instead of returning error. > > > + > > + if (csi_clk < alvium->min_csi_clk || csi_clk > alvium->max_csi_clk) > > + return -EINVAL; > > + > > + ret = alvium_write_hshake(alvium, REG_BCRM_CSI2_CLOCK_RW, csi_clk); > > + if (ret) { > > + dev_err(dev, "Fail to set csi lanes reg\n"); > > + return ret; > > + } > > + > > + alvium->link_freq = alvium->ep.link_frequencies[0]; > > + > > + return 0; > > +} > > + > > +static int alvium_get_img_width_params(struct alvium_dev *alvium) > > +{ > > + u64 val; > > + int ret = 0; > > + > > + alvium_read(alvium, REG_BCRM_IMG_WIDTH_RW, &val, &ret); > > + alvium->dft_img_width = val; > > + > > + alvium_read(alvium, REG_BCRM_IMG_WIDTH_MIN_R, &val, &ret); > > + alvium->img_min_width = val; > > + > > + alvium_read(alvium, REG_BCRM_IMG_WIDTH_MAX_R, &val, NULL); > > + alvium->img_max_width = val; > > + > > + alvium_read(alvium, REG_BCRM_IMG_WIDTH_INC_R, &val, &ret); > > + alvium->img_inc_width = val; > > + > > + return ret; > > +} > > + > > +static int alvium_get_img_height_params(struct alvium_dev *alvium) > > +{ > > + u64 val; > > + int ret = 0; > > + > > + alvium_read(alvium, REG_BCRM_IMG_HEIGHT_RW, &val, &ret); > > + alvium->dft_img_height = val; > > + > > + alvium_read(alvium, REG_BCRM_IMG_HEIGHT_MIN_R, &val, &ret); > > + alvium->img_min_height = val; > > + > > + alvium_read(alvium, REG_BCRM_IMG_HEIGHT_MAX_R, &val, &ret); > > + alvium->img_max_height = val; > > + > > + alvium_read(alvium, REG_BCRM_IMG_HEIGHT_INC_R, &val, &ret); > > + alvium->img_inc_height = val; > > + > > + return ret; > > +} > > + > > +static int alvium_set_img_width(struct alvium_dev *alvium, u32 width) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + int ret; > > + > > + ret = alvium_write_hshake(alvium, REG_BCRM_IMG_WIDTH_RW, width); > > + if (ret) { > > + dev_err(dev, "Fail to set img width\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_set_img_height(struct alvium_dev *alvium, u32 height) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + int ret; > > + > > + ret = alvium_write_hshake(alvium, REG_BCRM_IMG_HEIGHT_RW, height); > > + if (ret) { > > + dev_err(dev, "Fail to set img height\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_set_img_offx(struct alvium_dev *alvium, u32 offx) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + int ret; > > + > > + ret = alvium_write_hshake(alvium, REG_BCRM_IMG_OFFSET_X_RW, offx); > > + if (ret) { > > + dev_err(dev, "Fail to set img offx\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_set_img_offy(struct alvium_dev *alvium, u32 offy) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + int ret; > > + > > + ret = alvium_write_hshake(alvium, REG_BCRM_IMG_OFFSET_Y_RW, offy); > > + if (ret) { > > + dev_err(dev, "Fail to set img offy\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_get_offx_params(struct alvium_dev *alvium) > > +{ > > + u64 val; > > + int ret = 0; > > + > > + alvium_read(alvium, REG_BCRM_IMG_OFFSET_X_MIN_R, &val, &ret); > > + alvium->min_offx = val; > > + > > + alvium_read(alvium, REG_BCRM_IMG_OFFSET_X_MAX_R, &val, &ret); > > + alvium->max_offx = val; > > + > > + alvium_read(alvium, REG_BCRM_IMG_OFFSET_X_INC_R, &val, &ret); > > + alvium->inc_offx = val; > > + > > + return ret; > > +} > > + > > +static int alvium_get_offy_params(struct alvium_dev *alvium) > > +{ > > + u64 val; > > + int ret = 0; > > + > > + ret = alvium_read(alvium, REG_BCRM_IMG_OFFSET_Y_MIN_R, &val, &ret); > > + alvium->min_offy = val; > > + > > + alvium_read(alvium, REG_BCRM_IMG_OFFSET_Y_MAX_R, &val, &ret); > > + alvium->max_offy = val; > > + > > + alvium_read(alvium, REG_BCRM_IMG_OFFSET_Y_INC_R, &val, &ret); > > + alvium->inc_offy = val; > > + > > + return ret; > > +} > > + > > +static int alvium_get_gain_params(struct alvium_dev *alvium) > > +{ > > + u64 val; > > + int ret = 0; > > + > > + alvium_read(alvium, REG_BCRM_GAIN_RW, &val, &ret); > > + alvium->dft_gain = val; > > + > > + alvium_read(alvium, REG_BCRM_GAIN_MIN_R, &val, &ret); > > + alvium->min_gain = val; > > + > > + alvium_read(alvium, REG_BCRM_GAIN_MAX_R, &val, &ret); > > + alvium->max_gain = val; > > + > > + alvium_read(alvium, REG_BCRM_GAIN_INC_R, &val, &ret); > > + alvium->inc_gain = val; > > + > > + return ret; > > +} > > + > > +static int alvium_get_exposure_params(struct alvium_dev *alvium) > > +{ > > + u64 val; > > + int ret = 0; > > + > > + alvium_read(alvium, REG_BCRM_EXPOSURE_TIME_RW, &val, &ret); > > + alvium->dft_exp = val; > > + > > + alvium_read(alvium, REG_BCRM_EXPOSURE_TIME_MIN_R, &val, &ret); > > + alvium->min_exp = val; > > + > > + alvium_read(alvium, REG_BCRM_EXPOSURE_TIME_MAX_R, &val, &ret); > > + alvium->max_exp = val; > > + > > + alvium_read(alvium, REG_BCRM_EXPOSURE_TIME_INC_R, &val, &ret); > > + alvium->inc_exp = val; > > + > > + return ret; > > +} > > + > > +static int alvium_get_red_balance_ratio_params(struct alvium_dev *alvium) > > +{ > > + u64 val; > > + int ret = 0; > > + > > + alvium_read(alvium, REG_BCRM_RED_BALANCE_RATIO_RW, &val, &ret); > > + alvium->dft_rbalance = val; > > + > > + alvium_read(alvium, REG_BCRM_RED_BALANCE_RATIO_MIN_R, &val, &ret); > > + alvium->min_rbalance = val; > > + > > + alvium_read(alvium, REG_BCRM_RED_BALANCE_RATIO_MAX_R, &val, &ret); > > + alvium->max_rbalance = val; > > + > > + alvium_read(alvium, REG_BCRM_RED_BALANCE_RATIO_INC_R, &val, &ret); > > + alvium->inc_rbalance = val; > > + > > + return ret; > > +} > > + > > +static int alvium_get_blue_balance_ratio_params(struct alvium_dev *alvium) > > +{ > > + u64 val; > > + int ret = 0; > > + > > + alvium_read(alvium, REG_BCRM_BLUE_BALANCE_RATIO_RW, &val, &ret); > > + alvium->dft_bbalance = val; > > + > > + alvium_read(alvium, REG_BCRM_BLUE_BALANCE_RATIO_MIN_R, &val, &ret); > > + alvium->min_bbalance = val; > > + > > + alvium_read(alvium, REG_BCRM_BLUE_BALANCE_RATIO_MAX_R, &val, &ret); > > + alvium->max_bbalance = val; > > + > > + alvium_read(alvium, REG_BCRM_BLUE_BALANCE_RATIO_INC_R, &val, &ret); > > + alvium->inc_bbalance = val; > > + > > + return ret; > > +} > > + > > +static int alvium_get_hue_params(struct alvium_dev *alvium) > > +{ > > + u64 val; > > + int ret = 0; > > + > > + alvium_read(alvium, REG_BCRM_HUE_RW, &val, &ret); > > + alvium->dft_hue = (s32)val; > > + > > + alvium_read(alvium, REG_BCRM_HUE_MIN_R, &val, &ret); > > + alvium->min_hue = (s32)val; > > + > > + alvium_read(alvium, REG_BCRM_HUE_MAX_R, &val, &ret); > > + alvium->max_hue = (s32)val; > > + > > + alvium_read(alvium, REG_BCRM_HUE_INC_R, &val, &ret); > > + alvium->inc_hue = (s32)val; > > + > > + return ret; > > +} > > + > > +static int alvium_get_black_lvl_params(struct alvium_dev *alvium) > > +{ > > + u64 val; > > + int ret = 0; > > + > > + alvium_read(alvium, REG_BCRM_BLACK_LEVEL_RW, &val, &ret); > > + alvium->dft_black_lvl = (s32)val; > > + > > + alvium_read(alvium, REG_BCRM_BLACK_LEVEL_MIN_R, &val, &ret); > > + alvium->min_black_lvl = (s32)val; > > + > > + alvium_read(alvium, REG_BCRM_BLACK_LEVEL_MAX_R, &val, &ret); > > + alvium->max_black_lvl = (s32)val; > > + > > + alvium_read(alvium, REG_BCRM_BLACK_LEVEL_INC_R, &val, &ret); > > + alvium->inc_black_lvl = (s32)val; > > + > > + return ret; > > +} > > + > > +static int alvium_get_gamma_params(struct alvium_dev *alvium) > > +{ > > + u64 val; > > + int ret = 0; > > + > > + alvium_read(alvium, REG_BCRM_GAMMA_RW, &val, &ret); > > + alvium->dft_gamma = val; > > + > > + alvium_read(alvium, REG_BCRM_GAMMA_MIN_R, &val, &ret); > > + alvium->min_gamma = val; > > + > > + alvium_read(alvium, REG_BCRM_GAMMA_MAX_R, &val, &ret); > > + alvium->max_gamma = val; > > + > > + alvium_read(alvium, REG_BCRM_GAMMA_INC_R, &val, &ret); > > + alvium->inc_gamma = val; > > + > > + return ret; > > +} > > + > > +static int alvium_get_sharpness_params(struct alvium_dev *alvium) > > +{ > > + u64 val; > > + int ret = 0; > > + > > + alvium_read(alvium, REG_BCRM_SHARPNESS_RW, &val, &ret); > > + alvium->dft_sharp = (s32)val; > > + > > + alvium_read(alvium, REG_BCRM_SHARPNESS_MIN_R, &val, &ret); > > + alvium->min_sharp = (s32)val; > > + > > + alvium_read(alvium, REG_BCRM_BLACK_LEVEL_MAX_R, &val, &ret); > > + alvium->max_sharp = (s32)val; > > + > > + alvium_read(alvium, REG_BCRM_SHARPNESS_INC_R, &val, &ret); > > + alvium->inc_sharp = (s32)val; > > + > > + return ret; > > +} > > + > > +static int alvium_get_contrast_params(struct alvium_dev *alvium) > > +{ > > + u64 val; > > + int ret = 0; > > + > > + alvium_read(alvium, REG_BCRM_CONTRAST_VALUE_RW, &val, &ret); > > + alvium->dft_contrast = val; > > + > > + alvium_read(alvium, REG_BCRM_CONTRAST_VALUE_MIN_R, &val, &ret); > > + alvium->min_contrast = val; > > + > > + alvium_read(alvium, REG_BCRM_CONTRAST_VALUE_MAX_R, &val, &ret); > > + alvium->max_contrast = val; > > + > > + alvium_read(alvium, REG_BCRM_CONTRAST_VALUE_INC_R, &val, &ret); > > + alvium->inc_contrast = val; > > + > > + return ret; > > +} > > + > > +static int alvium_get_saturation_params(struct alvium_dev *alvium) > > +{ > > + u64 val; > > + int ret = 0; > > + > > + alvium_read(alvium, REG_BCRM_SATURATION_RW, &val, &ret); > > + alvium->dft_sat = val; > > + > > + alvium_read(alvium, REG_BCRM_SATURATION_MIN_R, &val, &ret); > > + alvium->min_sat = val; > > + > > + alvium_read(alvium, REG_BCRM_SATURATION_MAX_R, &val, &ret); > > + alvium->max_sat = val; > > + > > + alvium_read(alvium, REG_BCRM_SATURATION_INC_R, &val, &ret); > > + alvium->inc_sat = val; > > + > > + return ret; > > +} > > + > > +static int alvium_set_bcm_mode(struct alvium_dev *alvium) > > +{ > > + int ret = 0; > > + > > + alvium_write(alvium, REG_GENCP_CHANGEMODE_W, ALVIUM_BCM_MODE, &ret); > > + alvium->bcrm_mode = ALVIUM_BCM_MODE; > > + > > + return ret; > > +} > > + > > +static int alvium_get_mode(struct alvium_dev *alvium) > > +{ > > + u64 bcrm_mode; > > + int ret = 0; > > + > > + alvium_read(alvium, REG_GENCP_CURRENTMODE_R, &bcrm_mode, &ret); > > + if (ret) > > + return ret; > > + > > + switch (bcrm_mode) { > > + case ALVIUM_BCM_MODE: > > + alvium->bcrm_mode = ALVIUM_BCM_MODE; > > + break; > > + case ALVIUM_GENCP_MODE: > > + alvium->bcrm_mode = ALVIUM_GENCP_MODE; > > + break; > > + } > > + > > + return ret; > > +} > > + > > +static int alvium_get_avail_mipi_data_format(struct alvium_dev *alvium) > > +{ > > + struct alvium_avail_mipi_fmt *avail_fmt; > > + u64 val; > > + int ret = 0; > > + > > + alvium_read(alvium, > > + REG_BCRM_IMG_AVAILABLE_MIPI_DATA_FORMATS_R, &val, &ret); > > + if (ret) > > + return ret; > > + > > + avail_fmt = (struct alvium_avail_mipi_fmt *) &val; > > + > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_YUV420_8_LEG] > > + = avail_fmt->yuv420_8_leg; > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_YUV420_8] > > + = avail_fmt->yuv420_8; > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_YUV420_10] > > + = avail_fmt->yuv420_10; > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_YUV420_8_CSPS] > > + = avail_fmt->yuv420_8_csps; > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_YUV420_10_CSPS] > > + = avail_fmt->yuv420_10_csps; > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_YUV422_8] > > + = avail_fmt->yuv422_8; > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_YUV422_10] > > + = avail_fmt->yuv422_10; > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_RGB888] > > + = avail_fmt->rgb888; > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_RGB666] > > + = avail_fmt->rgb666; > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_RGB565] > > + = avail_fmt->rgb565; > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_RGB555] > > + = avail_fmt->rgb555; > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_RGB444] > > + = avail_fmt->rgb444; > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_RAW6] > > + = avail_fmt->raw6; > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_RAW7] > > + = avail_fmt->raw7; > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_RAW8] > > + = avail_fmt->raw8; > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_RAW10] > > + = avail_fmt->raw10; > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_RAW12] > > + = avail_fmt->raw12; > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_RAW14] > > + = avail_fmt->raw14; > > + alvium->is_mipi_fmt_avail[ALVIUM_BIT_JPEG] > > + = avail_fmt->jpeg; > > + > > + alvium_print_avail_mipi_fmt(alvium); > > + > > + return ret; > > +} > > + > > +static int alvium_setup_mipi_fmt(struct alvium_dev *alvium) > > +{ > > + int avail_fmt_cnt = 0; > > + int sz = 0; > > + int fmt = 0; > > unsigned int? I'll fix this on v11. > > > + > > + alvium->alvium_csi2_fmt = NULL; > > + > > + /* calculate fmt array size */ > > + for (fmt = 0; fmt < ALVIUM_NUM_SUPP_MIPI_DATA_FMT; fmt++) { > > + if (alvium->is_mipi_fmt_avail[alvium_csi2_fmts[fmt].fmt_av_bit]) > > + if ((!alvium_csi2_fmts[fmt].is_raw) || > > + (alvium->is_bay_avail[alvium_csi2_fmts[fmt].bay_av_bit])) > > Alignment. > > > + sz++; > > + } > > + > > + /* init alvium_csi2_fmt array */ > > + alvium->alvium_csi2_fmt_n = sz; > > + alvium->alvium_csi2_fmt = kmalloc_array(sz, > > + sizeof(struct alvium_pixfmt), > > Alignment. > > > + GFP_KERNEL); > > + > > + /* Create the alvium_csi2 fmt array from formats available */ > > + for (fmt = 0; fmt < ALVIUM_NUM_SUPP_MIPI_DATA_FMT; fmt++) { > > + if (!alvium->is_mipi_fmt_avail[alvium_csi2_fmts[fmt].fmt_av_bit]) > > + continue; > > + > > + if ((!alvium_csi2_fmts[fmt].is_raw) || > > + (alvium->is_bay_avail[alvium_csi2_fmts[fmt].bay_av_bit])) { > > Here, too... > > Generally the practice is to align the following lines just right of the > opening parenthesis. Same here, thanks. > > > + alvium->alvium_csi2_fmt[avail_fmt_cnt] = > > + alvium_csi2_fmts[fmt]; > > + avail_fmt_cnt++; > > + } > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_set_mipi_fmt(struct alvium_dev *alvium, > > + const struct alvium_pixfmt *pixfmt) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + u64 reg_val; > > + int ret; > > + > > + reg_val = pixfmt->mipi_fmt_regval; > > + > > + ret = alvium_write_hshake(alvium, REG_BCRM_IMG_MIPI_DATA_FORMAT_RW, > > + reg_val); > > + if (ret) { > > + dev_err(dev, "Fail to set mipi fmt\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_get_avail_bayer(struct alvium_dev *alvium) > > +{ > > + struct alvium_avail_bayer *avail_bay; > > + u64 val; > > + int ret = 0; > > + > > + alvium_read(alvium, REG_BCRM_IMG_BAYER_PATTERN_INQUIRY_R, &val, &ret); > > + if (ret) > > + return ret; > > + > > + avail_bay = (struct alvium_avail_bayer *) &val; > > + > > + alvium->is_bay_avail[ALVIUM_BIT_BAY_MONO] = avail_bay->mono; > > + alvium->is_bay_avail[ALVIUM_BIT_BAY_GR] = avail_bay->gr; > > + alvium->is_bay_avail[ALVIUM_BIT_BAY_RG] = avail_bay->rg; > > + alvium->is_bay_avail[ALVIUM_BIT_BAY_GB] = avail_bay->gb; > > + alvium->is_bay_avail[ALVIUM_BIT_BAY_BG] = avail_bay->bg; > > + > > + alvium_print_avail_bayer(alvium); > > + > > + return ret; > > +} > > + > > +static int alvium_set_bayer_pattern(struct alvium_dev *alvium, > > + const struct alvium_pixfmt *pixfmt) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + u64 reg_val; > > + int ret; > > + > > + reg_val = pixfmt->bay_fmt_regval; > > + > > + ret = alvium_write_hshake(alvium, REG_BCRM_IMG_BAYER_PATTERN_RW, reg_val); > > + if (ret) { > > + dev_err(dev, "Fail to set bayer pattern\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_get_frame_interval(struct alvium_dev *alvium) > > +{ > > + u64 val; > > + int ret = 0; > > + > > + alvium_read(alvium, REG_BCRM_ACQUISITION_FRAME_RATE_MIN_R, &val, &ret); > > + if (ret) > > + return ret; > > + > > + alvium->min_fr = val; > > + > > + alvium_read(alvium, REG_BCRM_ACQUISITION_FRAME_RATE_MAX_R, &val, &ret); > > + if (ret) > > + return ret; > > + > > + alvium->max_fr = val; > > + > > + return 0; > > +} > > + > > +static int alvium_set_frame_rate(struct alvium_dev *alvium) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + u64 fr; > > + int ret; > > + > > + fr = alvium->fr; > > + > > No need for a temporary variable. I'll fix this on v11. > > > + ret = alvium_write_hshake(alvium, REG_BCRM_ACQUISITION_FRAME_RATE_RW, fr); > > + if (ret) { > > + dev_err(dev, "Fail to set frame rate lanes reg\n"); > > + return ret; > > + } > > + > > + dev_dbg(dev, "set frame rate: %llu us\n", fr); > > + > > + return 0; > > +} > > + > > +static int alvium_set_stream_mipi(struct alvium_dev *alvium, bool on) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + int ret; > > + > > + if (on) > > + ret = alvium_write_hshake(alvium, REG_BCRM_ACQUISITION_START_RW, 0x01); > > + else > > + ret = alvium_write_hshake(alvium, REG_BCRM_ACQUISITION_STOP_RW, 0x01); > > + > > + if (ret) { > > + dev_err(dev, "Fail set_stream_mipi\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_get_gain(struct alvium_dev *alvium) > > +{ > > + u64 gain; > > + int ret = 0; > > + > > + /* The unit is millibel (1 mB = 0.01 dB) */ > > + alvium_read(alvium, REG_BCRM_GAIN_RW, &gain, &ret); > > + if (ret) > > + return ret; > > + > > + return gain; > > +} > > + > > +static int alvium_set_gain(struct alvium_dev *alvium, int gain) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + int ret; > > + > > + /* The unit is millibel (1 mB = 0.01 dB) */ > > + ret = alvium_write_hshake(alvium, REG_BCRM_GAIN_RW, (u64)gain); > > + if (ret) { > > + dev_err(dev, "Fail to set gain value reg\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_set_autogain(struct alvium_dev *alvium, bool on) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + int ret; > > + > > + if (!alvium->avail_ft.auto_gain) > > + on = false; > > + > > + if (on) > > + ret = alvium_write_hshake(alvium, REG_BCRM_GAIN_AUTO_RW, 0x02); > > + else > > + ret = alvium_write_hshake(alvium, REG_BCRM_GAIN_AUTO_RW, 0x00); > > + > > + if (ret) { > > + dev_err(dev, "Fail to set autogain reg\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_get_exposure(struct alvium_dev *alvium) > > +{ > > + u64 exp; > > + int ret = 0; > > + > > + /* Exposure time in ns */ > > + alvium_read(alvium, REG_BCRM_EXPOSURE_TIME_RW, &exp, &ret); > > + if (ret) > > + return ret; > > + > > + return exp; > > +} > > + > > +static int alvium_set_autoexposure(struct alvium_dev *alvium, bool on) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + int ret; > > + > > + if (!alvium->avail_ft.auto_exp) > > + on = false; > > + > > + if (on) > > + ret = alvium_write_hshake(alvium, REG_BCRM_EXPOSURE_AUTO_RW, 0x02); > > + else > > + ret = alvium_write_hshake(alvium, REG_BCRM_EXPOSURE_AUTO_RW, 0x00); > > + > > + if (ret) { > > + dev_err(dev, "Fail to set autoexposure reg\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_set_exposure(struct alvium_dev *alvium, int exposure_ns) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + int ret; > > + > > + ret = alvium_write_hshake(alvium, REG_BCRM_EXPOSURE_TIME_RW, > > + (u64)exposure_ns); > > + if (ret) { > > + dev_err(dev, "Fail to set exposure value reg\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_set_ctrl_gain(struct alvium_dev *alvium, bool auto_gain) > > +{ > > + struct alvium_ctrls *ctrls = &alvium->ctrls; > > + int ret = 0; > > + > > + if (ctrls->auto_gain->is_new) { > > + ret = alvium_set_autogain(alvium, auto_gain); > > + if (ret) > > + return ret; > > + } > > + > > + if (!auto_gain && ctrls->gain->is_new) { > > + > > + ret = alvium_get_gain_params(alvium); > > + if (ret) > > + return ret; > > + > > + if (ctrls->gain->val < alvium->min_gain) > > + ctrls->gain->val = alvium->min_gain; > > + > > + if (ctrls->gain->val > alvium->max_gain) > > + ctrls->gain->val = alvium->max_gain; > > Can you set the control range accordingly? Same for exposure at least. Thanks, in v11 we fix also the ctrls min/max values > > > + > > + ret = alvium_set_gain(alvium, ctrls->gain->val); > > + } > > + > > + return ret; > > +} > > + > > +static int alvium_set_blue_balance_ratio(struct alvium_dev *alvium, int blue) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + int ret; > > + > > + ret = alvium_write_hshake(alvium, REG_BCRM_BLUE_BALANCE_RATIO_RW, > > + (u64)blue); > > + if (ret) { > > + dev_err(dev, "Fail to set blue ratio value reg\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_set_red_balance_ratio(struct alvium_dev *alvium, int red) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + int ret; > > + > > + ret = alvium_write_hshake(alvium, > > + REG_BCRM_RED_BALANCE_RATIO_RW, (u64)red); > > + if (ret) { > > + dev_err(dev, "Fail to set red ratio value reg\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_set_awb(struct alvium_dev *alvium, bool on) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + int ret; > > + > > + if (!alvium->avail_ft.auto_whiteb) > > + on = false; > > + > > + if (on) > > + ret = alvium_write_hshake(alvium, > > + REG_BCRM_WHITE_BALANCE_AUTO_RW, 0x02); > > + else > > + ret = alvium_write_hshake(alvium, > > + REG_BCRM_WHITE_BALANCE_AUTO_RW, 0x00); > > You could write this and other similar ones as: > > ret = alvium_write_hshake(alvium, REG_BCRM_WHITE_BALANCE_AUTO_RW, > on ? 0x02 : 0x00); Fix in v11, thanks. > > > + > > + if (ret) { > > + dev_err(dev, "Fail to set awb reg\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_set_ctrl_exposure(struct alvium_dev *alvium, > > + enum v4l2_exposure_auto_type auto_exposure) > > +{ > > + struct alvium_ctrls *ctrls = &alvium->ctrls; > > + bool auto_exp = (auto_exposure == V4L2_EXPOSURE_AUTO); > > + int ret = 0; > > + > > + if (ctrls->auto_exp->is_new) { > > + ret = alvium_set_autoexposure(alvium, auto_exp); > > + if (ret) > > + return ret; > > + } > > + > > + if (!auto_exp && ctrls->exposure->is_new) { > > + > > + ret = alvium_get_exposure_params(alvium); > > What can cause the exposure limits to change? Certainly not setting the > exposure value? > > You should re-obtain these limits when they change and update the range of > the exposure control. > > Same comment on other similar controls. Good catch. I remove get_**params function in v11 it's redundant. > > > + if (ret) > > + return ret; > > + > > + if (ctrls->exposure->val < alvium->min_exp) > > + ctrls->exposure->val = alvium->min_exp; > > + > > + if (ctrls->exposure->val > alvium->max_exp) > > + ctrls->exposure->val = alvium->max_exp; > > + > > + ret = alvium_set_exposure(alvium, ctrls->exposure->val); > > + if (ret) > > + return ret; > > + } > > + > > + return ret; > > +} > > + > > +static int alvium_set_ctrl_white_balance(struct alvium_dev *alvium, bool awb) > > +{ > > + struct alvium_ctrls *ctrls = &alvium->ctrls; > > + int ret = 0; > > + > > + if (ctrls->auto_wb->is_new) { > > + ret = alvium_set_awb(alvium, awb); > > + if (ret) > > + return ret; > > + } > > You could return 0 here if awb is true. Fix in v11 > > > + > > + if (!awb && (ctrls->blue_balance->is_new)) { > > + u64 blue = (u64)ctrls->blue_balance->val; > > + > > + ret = alvium_get_blue_balance_ratio_params(alvium); > > + if (ret) > > + return ret; > > + > > + if (blue < alvium->min_bbalance) > > + blue = alvium->min_bbalance; > > + > > + if (blue > alvium->max_bbalance) > > + blue = alvium->min_bbalance; > > + > > + ret = alvium_set_blue_balance_ratio(alvium, blue); > > + if (ret) > > + return ret; > > + } > > + > > + if (!awb && (ctrls->red_balance->is_new)) { > > + u64 red = (u64)ctrls->red_balance->val; > > + > > + ret = alvium_get_red_balance_ratio_params(alvium); > > + if (ret) > > + return ret; > > + > > + if (red < alvium->min_rbalance) > > + red = alvium->min_rbalance; > > + > > + if (red > alvium->max_rbalance) > > + red = alvium->max_rbalance; > > + > > + ret = alvium_set_red_balance_ratio(alvium, red); > > + } > > + > > + return ret; > > +} > > + > > +static int alvium_set_ctrl_hue(struct alvium_dev *alvium, int val) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + int ret; > > + > > + ret = alvium_write_hshake(alvium, REG_BCRM_HUE_RW, (u64)val); > > + if (ret) { > > + dev_err(dev, "Fail to set hue value reg\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_set_ctrl_contrast(struct alvium_dev *alvium, int val) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + int ret; > > + > > + ret = alvium_write_hshake(alvium, REG_BCRM_CONTRAST_VALUE_RW, (u64)val); > > + if (ret) { > > + dev_err(dev, "Fail to set contrast value reg\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_set_ctrl_saturation(struct alvium_dev *alvium, int val) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + int ret; > > + > > + ret = alvium_write_hshake(alvium, REG_BCRM_SATURATION_RW, (u64)val); > > + if (ret) { > > + dev_err(dev, "Fail to set contrast value reg\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_set_ctrl_gamma(struct alvium_dev *alvium, int val) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + int ret; > > + > > + ret = alvium_write_hshake(alvium, REG_BCRM_GAMMA_RW, (u64)val); > > + if (ret) { > > + dev_err(dev, "Fail to set gamma value reg\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_set_ctrl_sharpness(struct alvium_dev *alvium, int val) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + int ret; > > + > > + ret = alvium_write_hshake(alvium, REG_BCRM_SHARPNESS_RW, (u64)val); > > + if (ret) { > > + dev_err(dev, "Fail to set sharpness value reg\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_set_ctrl_hflip(struct alvium_dev *alvium, int val) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + int ret; > > + > > + ret = alvium_write_hshake(alvium, REG_BCRM_IMG_REVERSE_X_RW, (u64)val); > > + if (ret) { > > + dev_err(dev, "Fail to set reverse_x value reg\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_set_ctrl_vflip(struct alvium_dev *alvium, int val) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + int ret; > > + > > + ret = alvium_write_hshake(alvium, REG_BCRM_IMG_REVERSE_Y_RW, (u64)val); > > + if (ret) { > > + dev_err(dev, "Fail to set reverse_y value reg\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_get_hw_features_params(struct alvium_dev *alvium) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + int ret; > > + > > + ret = alvium_get_csi_clk_params(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to read min/max csi clock regs\n"); > > + return ret; > > + } > > + > > + ret = alvium_get_img_width_params(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to read img width regs\n"); > > + return ret; > > + } > > + > > + ret = alvium_get_img_height_params(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to read img heigth regs\n"); > > + return ret; > > + } > > + > > + ret = alvium_get_offx_params(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to read offx regs\n"); > > + return ret; > > + } > > + > > + ret = alvium_get_offy_params(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to read offy regs\n"); > > + return ret; > > + } > > + > > + ret = alvium_get_gain_params(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to read gain regs\n"); > > + return ret; > > + } > > + > > + ret = alvium_get_exposure_params(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to read min/max exp regs\n"); > > + return ret; > > + } > > + > > + ret = alvium_get_red_balance_ratio_params(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to read red balance ratio regs\n"); > > + return ret; > > + } > > + > > + ret = alvium_get_blue_balance_ratio_params(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to read blue balance ratio regs\n"); > > + return ret; > > + } > > + > > + ret = alvium_get_hue_params(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to read hue regs\n"); > > + return ret; > > + } > > + > > + ret = alvium_get_contrast_params(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to read contrast regs\n"); > > + return ret; > > + } > > + > > + ret = alvium_get_saturation_params(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to read saturation regs\n"); > > + return ret; > > + } > > + > > + ret = alvium_get_black_lvl_params(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to read black lvl regs\n"); > > + return ret; > > + } > > + > > + ret = alvium_get_gamma_params(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to read gamma regs\n"); > > + return ret; > > + } > > + > > + ret = alvium_get_sharpness_params(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to read sharpness regs\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_get_hw_info(struct alvium_dev *alvium) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + int ret; > > + > > + ret = alvium_get_bcrm_vers(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to read bcrm version reg\n"); > > + return ret; > > + } > > + > > + ret = alvium_get_bcrm_addr(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to bcrm address reg\n"); > > + return ret; > > + } > > + > > + ret = alvium_get_fw_version(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to read fw version reg\n"); > > + return ret; > > + } > > + > > + ret = alvium_get_host_supp_csi_lanes(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to read host supported csi lanes reg\n"); > > + return ret; > > + } > > + > > + ret = alvium_get_feat_inq(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to read bcrm feature inquiry reg\n"); > > + return ret; > > + } > > + > > + ret = alvium_get_hw_features_params(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to read features params regs\n"); > > + return ret; > > + } > > + > > + ret = alvium_get_avail_mipi_data_format(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to read available mipi data formats reg\n"); > > + return ret; > > + } > > + > > + ret = alvium_get_avail_bayer(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to read available Bayer patterns reg\n"); > > + return ret; > > + } > > + > > + ret = alvium_get_mode(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to get current mode reg\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_hw_init(struct alvium_dev *alvium) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + int ret; > > + > > + /* Set Alvium BCM mode*/ > > + ret = alvium_set_bcm_mode(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to set BCM mode\n"); > > + return ret; > > + } > > + > > + ret = alvium_set_csi_lanes(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to set csi lanes\n"); > > + return ret; > > + } > > + > > + ret = alvium_set_csi_clk(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to set csi clk\n"); > > + return ret; > > + } > > + > > + ret = alvium_set_lp2hs_delay(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to set lp2hs reg\n"); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +/* --------------- Subdev Operations --------------- */ > > + > > +static int alvium_g_frame_interval(struct v4l2_subdev *sd, > > + struct v4l2_subdev_frame_interval *fi) > > +{ > > + struct alvium_dev *alvium = sd_to_alvium(sd); > > + > > + fi->interval = alvium->frame_interval; > > + > > + return 0; > > +} > > + > > +static int alvium_set_frame_interval(struct alvium_dev *alvium, > > + struct v4l2_subdev_frame_interval *fi) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + u64 req_fr, min_fr, max_fr; > > + int ret; > > + > > + if (fi->interval.denominator == 0) > > + return -EINVAL; > > + > > + ret = alvium_get_frame_interval(alvium); > > + if (ret) { > > + dev_err(dev, "Fail to get frame interval\n"); > > + return ret; > > + } > > + > > + min_fr = alvium->min_fr; > > + max_fr = alvium->max_fr; > > + > > + dev_dbg(dev, "fi->interval.numerator = %d\n", fi->interval.numerator); > > + dev_dbg(dev, "fi->interval.denominator = %d\n", fi->interval.denominator); > > + > > + req_fr = (fi->interval.denominator * USEC_PER_SEC) / > > + fi->interval.numerator; > > + > > + if (req_fr >= max_fr && req_fr <= min_fr) > > + return -EINVAL; > > + > > + alvium->fr = req_fr; > > + alvium->frame_interval.numerator = fi->interval.numerator; > > + alvium->frame_interval.denominator = fi->interval.denominator; > > + > > + return 0; > > +} > > + > > +static int alvium_s_frame_interval(struct v4l2_subdev *sd, > > + struct v4l2_subdev_frame_interval *fi) > > +{ > > + struct alvium_dev *alvium = sd_to_alvium(sd); > > + int ret; > > + > > + if (alvium->streaming) > > + return -EBUSY; > > + > > + ret = alvium_set_frame_interval(alvium, fi); > > + if (!ret) { > > + ret = alvium_set_frame_rate(alvium); > > + if (ret) > > + return -EIO; > > + } > > + > > + return ret; > > +} > > + > > +static int alvium_enum_mbus_code(struct v4l2_subdev *sd, > > + struct v4l2_subdev_state *sd_state, > > + struct v4l2_subdev_mbus_code_enum *code) > > +{ > > + struct alvium_dev *alvium = sd_to_alvium(sd); > > + > > + if (code->index >= alvium->alvium_csi2_fmt_n) > > + return -EINVAL; > > + > > + code->code = alvium->alvium_csi2_fmt[code->index].code; > > + > > + return 0; > > +} > > + > > +static const struct alvium_pixfmt * > > +alvium_code_to_pixfmt(struct alvium_dev *alvium, u32 code) > > +{ > > + const struct alvium_pixfmt *formats = alvium->alvium_csi2_fmt; > > + unsigned int i; > > + > > + for (i = 0; formats[i].code; ++i) > > + if (formats[i].code == code) > > + return &formats[i]; > > + > > + return &formats[0]; > > +} > > + > > +static int alvium_set_mode(struct alvium_dev *alvium, > > + struct v4l2_subdev_state *state) > > +{ > > + struct v4l2_subdev *sd = &alvium->sd; > > + struct v4l2_mbus_framefmt *fmt; > > + struct v4l2_rect *crop; > > + int ret; > > + > > + crop = v4l2_subdev_get_pad_crop(sd, state, 0); > > + fmt = v4l2_subdev_get_pad_format(sd, state, 0); > > You should validate the values before assigning them to the subdev state, > not here. > > Please also use clamp() where appropriate. I'll fix this in v11 > > > + > > + if (fmt->width > alvium->img_max_width) > > + fmt->width = alvium->img_max_width; > > + > > + if (fmt->width < alvium->img_min_width) > > + fmt->width = alvium->img_min_width; > > + > > + ret = alvium_set_img_width(alvium, fmt->width); > > + if (ret) > > + return ret; > > + > > + if (fmt->height > alvium->img_max_height) > > + fmt->height = alvium->img_max_height; > > + > > + if (fmt->height < alvium->img_min_height) > > + fmt->height = alvium->img_min_height; > > + > > + ret = alvium_set_img_height(alvium, fmt->height); > > + if (ret) > > + return ret; > > + > > + ret = alvium_set_img_offx(alvium, crop->left); > > + if (ret) > > + return ret; > > + > > + ret = alvium_set_img_offy(alvium, crop->top); > > + if (ret) > > + return ret; > > + > > + return 0; > > +} > > + > > +static int alvium_set_framefmt(struct alvium_dev *alvium, > > + struct v4l2_mbus_framefmt *format) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + const struct alvium_pixfmt *alvium_csi2_fmt; > > + int ret = 0; > > + > > + alvium_csi2_fmt = alvium_code_to_pixfmt(alvium, format->code); > > + > > + ret = alvium_set_mipi_fmt(alvium, alvium_csi2_fmt); > > + if (ret) > > + return ret; > > + > > + if (alvium_csi2_fmt->is_raw) { > > + ret = alvium_set_bayer_pattern(alvium, alvium_csi2_fmt); > > + if (ret) > > + return ret; > > + } > > + > > + dev_dbg(dev, "start: %s, mipi_fmt_regval regval = 0x%llx", > > + __func__, alvium_csi2_fmt->mipi_fmt_regval); > > + > > + return ret; > > +} > > + > > +static int alvium_s_stream(struct v4l2_subdev *sd, int enable) > > +{ > > + struct alvium_dev *alvium = sd_to_alvium(sd); > > + struct i2c_client *client = v4l2_get_subdevdata(&alvium->sd); > > + struct v4l2_mbus_framefmt *fmt; > > + struct v4l2_subdev_state *state; > > + int ret = 0; > > + > > + state = v4l2_subdev_lock_and_get_active_state(sd); > > + > > + if (enable) { > > + ret = pm_runtime_resume_and_get(&client->dev); > > + if (ret < 0) > > + goto out; > > + > > + ret = __v4l2_ctrl_handler_setup(&alvium->ctrls.handler); > > + if (ret) > > From now on you'll need pm_runtime_put(). In v11 we will introduce pm_runtime_put() into out path. Thanks. > > > + goto out; > > + > > + ret = alvium_set_mode(alvium, state); > > + if (ret) > > + goto out; > > + > > + fmt = v4l2_subdev_get_pad_format(sd, state, 0); > > + ret = alvium_set_framefmt(alvium, fmt); > > + if (ret) > > + goto out; > > + > > + ret = alvium_set_stream_mipi(alvium, enable); > > + if (ret) > > + goto out; > > + > > + } else { > > + alvium_set_stream_mipi(alvium, enable); > > + pm_runtime_mark_last_busy(&client->dev); > > + pm_runtime_put_autosuspend(&client->dev); > > pm_runtime_put() here, too. Here is not needed we already have pm_runtime_put_autosuspend. I'm missing something? > > > + } > > + > > + alvium->streaming = !!enable; > > + v4l2_subdev_unlock_state(state); > > + > > + return 0; > > + > > +out: > > + v4l2_subdev_unlock_state(state); > > + return ret; > > +} > > + > > +static int alvium_init_cfg(struct v4l2_subdev *sd, > > + struct v4l2_subdev_state *state) > > +{ > > + struct alvium_dev *alvium = sd_to_alvium(sd); > > + struct alvium_mode *mode = &alvium->mode; > > Init_cfg() is expected to be configuration independent (as much as > possible). Therefore you should use defaults here, not current mode. Defaults alvium mode already used here. > > > + struct v4l2_subdev_format sd_fmt = { > > + .which = V4L2_SUBDEV_FORMAT_TRY, > > + .format = alvium_csi2_default_fmt, > > + }; > > + struct v4l2_subdev_crop sd_crop = { > > + .which = V4L2_SUBDEV_FORMAT_TRY, > > + .rect = { > > + .left = mode->crop.left, > > + .top = mode->crop.top, > > + .width = mode->crop.width, > > + .height = mode->crop.height, > > + }, > > + }; > > + > > + *v4l2_subdev_get_pad_crop(sd, state, 0) = sd_crop.rect; > > + *v4l2_subdev_get_pad_format(sd, state, 0) = sd_fmt.format; > > + > > + return 0; > > +} > > + > > +static int alvium_set_fmt(struct v4l2_subdev *sd, > > + struct v4l2_subdev_state *sd_state, > > + struct v4l2_subdev_format *format) > > +{ > > + struct alvium_dev *alvium = sd_to_alvium(sd); > > + const struct alvium_pixfmt *alvium_csi2_fmt; > > + struct v4l2_mbus_framefmt *fmt; > > + struct v4l2_rect *crop; > > + > > + fmt = v4l2_subdev_get_pad_format(sd, sd_state, 0); > > + crop = v4l2_subdev_get_pad_crop(sd, sd_state, 0); > > + > > + if (format->format.width > alvium->img_max_width) > > + fmt->width = alvium->img_max_width; > > + else > > + fmt->width = format->format.width; > > + > > + if (format->format.width < alvium->img_min_width) > > + fmt->width = alvium->img_min_width; > > + else > > + fmt->width = format->format.width; > > + > > + if (format->format.height > alvium->img_max_height) > > + fmt->height = alvium->img_max_height; > > + else > > + fmt->height = format->format.height; > > + > > + if (format->format.height < alvium->img_min_height) > > + fmt->height = alvium->img_min_height; > > + else > > + fmt->height = format->format.height; > > + > > + /* Adjust top and left to prevent roll over sensor area */ > > + if (crop->left + format->format.width > alvium->img_max_width) > > + crop->left = alvium->img_max_width - format->format.width; > > + > > + if (crop->top + format->format.height > alvium->img_max_height) > > + crop->top = alvium->img_max_height - format->format.height; > > v4l_bound_align_image() perhaps? I'll fix this in v11, thanks. > > > + > > + /* Set also the crop width and height when set a new fmt */ > > + crop->width = fmt->width; > > + crop->height = fmt->height; > > + > > + alvium_csi2_fmt = alvium_code_to_pixfmt(alvium, format->format.code); > > + fmt->code = alvium_csi2_fmt->code; > > + > > + *fmt = format->format; > > + > > + return 0; > > +} > > + > > +static int alvium_set_selection(struct v4l2_subdev *sd, > > + struct v4l2_subdev_state *sd_state, > > + struct v4l2_subdev_selection *sel) > > +{ > > + struct alvium_dev *alvium = sd_to_alvium(sd); > > + struct v4l2_mbus_framefmt *fmt; > > + struct v4l2_rect *crop; > > + > > + crop = v4l2_subdev_get_pad_crop(sd, sd_state, 0); > > + fmt = v4l2_subdev_get_pad_format(sd, sd_state, 0); > > + > > + /* > > + * Alvium can only shift the origin of the img > > + * then we accept only value with the same value of the actual fmt > > + */ > > + if (sel->r.width != fmt->width || > > + sel->r.height != fmt->height) > > + return -EINVAL; > > Please adjust, don't return an error in set_selection. Ops. Thanks. > > > + > > + if (sel->target != V4L2_SEL_TGT_CROP) > > + return -EINVAL; > > + > > + if ((sel->r.left + sel->r.width) > alvium->img_max_width) > > + crop->left = alvium->img_max_width - sel->r.width; > > + else > > + crop->left = sel->r.left; > > + > > + if ((sel->r.top + sel->r.height) > alvium->img_max_height) > > + crop->top = alvium->img_max_height - sel->r.height; > > + else > > + crop->top = sel->r.top; > > v4l_bound_align_image() perhaps? Or clamp()? Same. I'll fix this in v11. > > > + > > + sel->r = *crop; > > + > > + return 0; > > +} > > + > > +static int alvium_get_selection(struct v4l2_subdev *sd, > > + struct v4l2_subdev_state *sd_state, > > + struct v4l2_subdev_selection *sel) > > +{ > > + struct alvium_dev *alvium = sd_to_alvium(sd); > > + > > + switch (sel->target) { > > + /* Current cropping area */ > > + case V4L2_SEL_TGT_CROP: > > + sel->r = *v4l2_subdev_get_pad_crop(sd, sd_state, 0); > > + break; > > + /* Cropping bounds */ > > + case V4L2_SEL_TGT_NATIVE_SIZE: > > + sel->r.top = 0; > > + sel->r.left = 0; > > + sel->r.width = alvium->img_max_width; > > + sel->r.height = alvium->img_max_height; > > + break; > > + /* Default cropping area */ > > + case V4L2_SEL_TGT_CROP_BOUNDS: > > + case V4L2_SEL_TGT_CROP_DEFAULT: > > + sel->r.top = alvium->min_offy; > > + sel->r.left = alvium->min_offx; > > + sel->r.width = alvium->img_max_width; > > + sel->r.height = alvium->img_max_height; > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_g_volatile_ctrl(struct v4l2_ctrl *ctrl) > > +{ > > + struct v4l2_subdev *sd = ctrl_to_sd(ctrl); > > + struct alvium_dev *alvium = sd_to_alvium(sd); > > + int val; > > + > > + switch (ctrl->id) { > > + case V4L2_CID_AUTOGAIN: > > This should be V4L2_CID_GAIN. > > > + val = alvium_get_gain(alvium); > > + if (val < 0) > > + return val; > > + alvium->ctrls.gain->val = val; > > + break; > > + case V4L2_CID_EXPOSURE_AUTO: > > This should be V4L2_CID_EXPOSURE. Thanks. > > > + val = alvium_get_exposure(alvium); > > + if (val < 0) > > + return val; > > + alvium->ctrls.exposure->val = val; > > + break; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_s_ctrl(struct v4l2_ctrl *ctrl) > > +{ > > + struct v4l2_subdev *sd = ctrl_to_sd(ctrl); > > + struct alvium_dev *alvium = sd_to_alvium(sd); > > + struct i2c_client *client = v4l2_get_subdevdata(&alvium->sd); > > + int ret; > > + > > + /* > > + * Applying V4L2 control value only happens > > + * when power is up for streaming > > + */ > > + if (!pm_runtime_get_if_in_use(&client->dev)) > > + return 0; > > + > > + switch (ctrl->id) { > > + case V4L2_CID_AUTOGAIN: > > + ret = alvium_set_ctrl_gain(alvium, ctrl->val); > > + break; > > + case V4L2_CID_EXPOSURE_AUTO: > > + ret = alvium_set_ctrl_exposure(alvium, ctrl->val); > > + break; > > + case V4L2_CID_AUTO_WHITE_BALANCE: > > + ret = alvium_set_ctrl_white_balance(alvium, ctrl->val); > > + break; > > + case V4L2_CID_HUE: > > + ret = alvium_set_ctrl_hue(alvium, ctrl->val); > > + break; > > + case V4L2_CID_CONTRAST: > > + ret = alvium_set_ctrl_contrast(alvium, ctrl->val); > > + break; > > + case V4L2_CID_SATURATION: > > + ret = alvium_set_ctrl_saturation(alvium, ctrl->val); > > + break; > > + case V4L2_CID_GAMMA: > > + ret = alvium_set_ctrl_gamma(alvium, ctrl->val); > > + break; > > + case V4L2_CID_SHARPNESS: > > + ret = alvium_set_ctrl_sharpness(alvium, ctrl->val); > > + break; > > + case V4L2_CID_HFLIP: > > + ret = alvium_set_ctrl_hflip(alvium, ctrl->val); > > + break; > > + case V4L2_CID_VFLIP: > > + ret = alvium_set_ctrl_vflip(alvium, ctrl->val); > > + break; > > + default: > > + ret = -EINVAL; > > + break; > > + } > > + > > + pm_runtime_put(&client->dev); > > + > > + return ret; > > +} > > + > > +static const struct v4l2_ctrl_ops alvium_ctrl_ops = { > > + .g_volatile_ctrl = alvium_g_volatile_ctrl, > > + .s_ctrl = alvium_s_ctrl, > > +}; > > + > > +static int alvium_ctrl_init(struct alvium_dev *alvium) > > +{ > > + const struct v4l2_ctrl_ops *ops = &alvium_ctrl_ops; > > + struct alvium_ctrls *ctrls = &alvium->ctrls; > > + struct v4l2_ctrl_handler *hdl = &ctrls->handler; > > + struct v4l2_fwnode_device_properties props; > > + int ret; > > + > > + v4l2_ctrl_handler_init(hdl, 32); > > + > > + /* Pixel rate is fixed */ > > + ctrls->pixel_rate = v4l2_ctrl_new_std(hdl, ops, > > + V4L2_CID_PIXEL_RATE, 0, > > + ALVIUM_DEFAULT_PIXEL_RATE_MHZ, 1, > > + ALVIUM_DEFAULT_PIXEL_RATE_MHZ); > > + > > + /* Link freq is fixed */ > > + ctrls->link_freq = v4l2_ctrl_new_int_menu(hdl, ops, > > + V4L2_CID_LINK_FREQ, > > + 0, 0, &alvium->link_freq); > > + > > + /* Auto/manual white balance */ > > + ctrls->auto_wb = v4l2_ctrl_new_std(hdl, ops, > > + V4L2_CID_AUTO_WHITE_BALANCE, > > + 0, 1, 1, > > + alvium->avail_ft.auto_whiteb ? 1 : 0); > > + > > + ctrls->blue_balance = v4l2_ctrl_new_std(hdl, ops, > > + V4L2_CID_BLUE_BALANCE, > > + alvium->min_bbalance, > > + alvium->max_bbalance, > > + alvium->inc_bbalance, > > + alvium->dft_bbalance); > > + ctrls->red_balance = v4l2_ctrl_new_std(hdl, ops, > > + V4L2_CID_RED_BALANCE, > > + alvium->min_rbalance, > > + alvium->max_rbalance, > > + alvium->inc_rbalance, > > + alvium->dft_rbalance); > > + > > + /* Auto/manual exposure */ > > + ctrls->auto_exp = v4l2_ctrl_new_std_menu(hdl, ops, > > + V4L2_CID_EXPOSURE_AUTO, > > + V4L2_EXPOSURE_MANUAL, 0, > > + alvium->avail_ft.auto_exp ? > > + V4L2_EXPOSURE_AUTO : V4L2_EXPOSURE_MANUAL); > > + > > + ctrls->exposure = v4l2_ctrl_new_std(hdl, ops, > > + V4L2_CID_EXPOSURE, > > + alvium->min_exp, > > + alvium->max_exp, > > + alvium->inc_exp, > > + alvium->dft_exp); > > + > > + /* Auto/manual gain */ > > + ctrls->auto_gain = v4l2_ctrl_new_std(hdl, ops, > > + V4L2_CID_AUTOGAIN, > > + 0, 1, 1, > > + alvium->avail_ft.auto_gain ? 1 : 0); > > + > > + if (alvium->avail_ft.gain) > > + ctrls->gain = v4l2_ctrl_new_std(hdl, ops, > > + V4L2_CID_GAIN, > > + alvium->min_gain, > > + alvium->max_gain, > > + alvium->inc_gain, > > + alvium->dft_gain); > > + > > + if (alvium->avail_ft.sat) > > + ctrls->saturation = v4l2_ctrl_new_std(hdl, ops, > > + V4L2_CID_SATURATION, > > + alvium->min_sat, > > + alvium->max_sat, > > + alvium->inc_sat, > > + alvium->dft_sat); > > + > > + if (alvium->avail_ft.hue) > > + ctrls->hue = v4l2_ctrl_new_std(hdl, ops, > > + V4L2_CID_HUE, > > + alvium->min_hue, > > + alvium->max_hue, > > + alvium->inc_hue, > > + alvium->dft_hue); > > + > > + if (alvium->avail_ft.contrast) > > + ctrls->contrast = v4l2_ctrl_new_std(hdl, ops, > > + V4L2_CID_CONTRAST, > > + alvium->min_contrast, > > + alvium->max_contrast, > > + alvium->inc_contrast, > > + alvium->dft_contrast); > > + > > + if (alvium->avail_ft.gamma) > > + ctrls->gamma = v4l2_ctrl_new_std(hdl, ops, > > + V4L2_CID_GAMMA, > > + alvium->min_gamma, > > + alvium->max_gamma, > > + alvium->inc_gamma, > > + alvium->dft_gamma); > > + > > + if (alvium->avail_ft.sharp) > > + ctrls->sharpness = v4l2_ctrl_new_std(hdl, ops, > > + V4L2_CID_SHARPNESS, > > + alvium->min_sharp, > > + alvium->max_sharp, > > + alvium->inc_sharp, > > + alvium->dft_sharp); > > + > > + if (alvium->avail_ft.rev_x) > > + ctrls->hflip = v4l2_ctrl_new_std(hdl, ops, > > + V4L2_CID_HFLIP, > > + 0, 1, 1, 0); > > + > > + if (alvium->avail_ft.rev_y) > > + ctrls->vflip = v4l2_ctrl_new_std(hdl, ops, > > + V4L2_CID_VFLIP, > > + 0, 1, 1, 0); > > + > > + if (hdl->error) { > > + ret = hdl->error; > > + goto free_ctrls; > > + } > > + > > + ret = v4l2_fwnode_device_parse(&alvium->i2c_client->dev, &props); > > + if (ret) > > + goto free_ctrls; > > + > > + ret = v4l2_ctrl_new_fwnode_properties(hdl, ops, &props); > > + if (ret) > > + goto free_ctrls; > > + > > + ctrls->pixel_rate->flags |= V4L2_CTRL_FLAG_READ_ONLY; > > + ctrls->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY; > > + ctrls->gain->flags |= V4L2_CTRL_FLAG_VOLATILE; > > + ctrls->exposure->flags |= V4L2_CTRL_FLAG_VOLATILE; > > + > > + v4l2_ctrl_auto_cluster(3, &ctrls->auto_wb, 0, false); > > + v4l2_ctrl_auto_cluster(2, &ctrls->auto_gain, 0, true); > > + v4l2_ctrl_auto_cluster(2, &ctrls->auto_exp, 1, true); > > + > > + alvium->sd.ctrl_handler = hdl; > > + return 0; > > + > > +free_ctrls: > > + v4l2_ctrl_handler_free(hdl); > > + return ret; > > +} > > + > > +static const struct v4l2_subdev_core_ops alvium_core_ops = { > > + .log_status = v4l2_ctrl_subdev_log_status, > > + .subscribe_event = v4l2_ctrl_subdev_subscribe_event, > > + .unsubscribe_event = v4l2_event_subdev_unsubscribe, > > +}; > > + > > +static const struct v4l2_subdev_video_ops alvium_video_ops = { > > + .g_frame_interval = alvium_g_frame_interval, > > + .s_frame_interval = alvium_s_frame_interval, > > + .s_stream = alvium_s_stream, > > +}; > > + > > +static const struct v4l2_subdev_pad_ops alvium_pad_ops = { > > + .init_cfg = alvium_init_cfg, > > + .enum_mbus_code = alvium_enum_mbus_code, > > + .get_fmt = v4l2_subdev_get_fmt, > > + .set_fmt = alvium_set_fmt, > > + .get_selection = alvium_get_selection, > > + .set_selection = alvium_set_selection, > > +}; > > + > > +static const struct v4l2_subdev_ops alvium_subdev_ops = { > > + .core = &alvium_core_ops, > > + .pad = &alvium_pad_ops, > > + .video = &alvium_video_ops, > > +}; > > + > > +static int alvium_subdev_init(struct alvium_dev *alvium) > > +{ > > + struct i2c_client *client = alvium->i2c_client; > > + struct device *dev = &alvium->i2c_client->dev; > > + struct v4l2_subdev *sd = &alvium->sd; > > + int ret; > > + > > + /* Setup initial frame interval*/ > > + alvium->frame_interval.numerator = 1; > > + alvium->frame_interval.denominator = ALVIUM_DEFAULT_FR_HZ; > > + alvium->fr = ALVIUM_DEFAULT_FR_HZ; > > + > > + /* Setup the initial mode */ > > + alvium->mode.fmt = alvium_csi2_default_fmt; > > + alvium->mode.width = alvium_csi2_default_fmt.width; > > + alvium->mode.height = alvium_csi2_default_fmt.height; > > + alvium->mode.crop.left = alvium->min_offx; > > + alvium->mode.crop.top = alvium->min_offy; > > + alvium->mode.crop.width = alvium_csi2_default_fmt.width; > > + alvium->mode.crop.height = alvium_csi2_default_fmt.height; > > + > > + /* init alvium sd */ > > + v4l2_i2c_subdev_init(sd, client, &alvium_subdev_ops); > > + > > + sd->flags |= V4L2_SUBDEV_FL_HAS_EVENTS | V4L2_SUBDEV_FL_HAS_DEVNODE; > > + alvium->pad.flags = MEDIA_PAD_FL_SOURCE; > > + sd->entity.function = MEDIA_ENT_F_CAM_SENSOR; > > + > > + ret = media_entity_pads_init(&sd->entity, 1, &alvium->pad); > > + if (ret) { > > + dev_err(dev, "Could not register media entity\n"); > > + return ret; > > + } > > + > > + ret = alvium_ctrl_init(alvium); > > + if (ret) { > > + dev_err(dev, "Control initialization error %d\n", ret); > > + goto entity_cleanup; > > + } > > + > > + alvium->sd.state_lock = alvium->ctrls.handler.lock; > > + > > + ret = v4l2_subdev_init_finalize(sd); > > + if (ret < 0) { > > + dev_err(dev, "subdev initialization error %d\n", ret); > > + goto err_ctrls; > > + } > > + > > + return 0; > > + > > +err_ctrls: > > + v4l2_ctrl_handler_free(&alvium->ctrls.handler); > > +entity_cleanup: > > + media_entity_cleanup(&alvium->sd.entity); > > + return ret; > > +} > > + > > +static void alvium_subdev_cleanup(struct alvium_dev *alvium) > > +{ > > + v4l2_subdev_cleanup(&alvium->sd); > > + media_entity_cleanup(&alvium->sd.entity); > > + v4l2_ctrl_handler_free(&alvium->ctrls.handler); > > +} > > + > > +static int alvium_get_dt_data(struct alvium_dev *alvium) > > +{ > > + struct device *dev = &alvium->i2c_client->dev; > > + struct fwnode_handle *fwnode = dev_fwnode(dev); > > + struct fwnode_handle *endpoint; > > + int ret = -EINVAL; > > ret is never assigned to another value: you could simply use -EINVAL > instead of ret and drop ret. Thanks. > > > + > > + if (!fwnode) > > + return -EINVAL; > > + > > + /* Only CSI2 is supported for now: */ > > + alvium->ep.bus_type = V4L2_MBUS_CSI2_DPHY; > > + > > + endpoint = fwnode_graph_get_endpoint_by_id(fwnode, 0, 0, 0); > > + if (!endpoint) { > > + dev_err(dev, "endpoint node not found\n"); > > + return -EINVAL; > > + } > > + > > + if (v4l2_fwnode_endpoint_alloc_parse(endpoint, &alvium->ep)) { > > + dev_err(dev, "could not parse endpoint\n"); > > + goto error_out; > > + } > > + > > + if (!alvium->ep.nr_of_link_frequencies) { > > + dev_err(dev, "no link frequencies defined"); > > + goto error_out; > > + } > > + > > + return 0; > > + > > +error_out: > > + v4l2_fwnode_endpoint_free(&alvium->ep); > > + fwnode_handle_put(endpoint); > > + > > + return ret; > > +} > > + > > +static int alvium_power_on(struct alvium_dev *alvium, bool on) > > +{ > > + int ret = 0; > > + > > + if (!on) > > + return regulator_disable(alvium->reg_vcc); > > + > > + ret = regulator_enable(alvium->reg_vcc); > > + if (ret) > > + return ret; > > + > > + /* alvium boot time 7s*/ > > + msleep(7000); > > + return 0; > > +} > > + > > +static int alvium_runtime_resume(struct device *dev) > > +{ > > + struct v4l2_subdev *sd = dev_get_drvdata(dev); > > + struct alvium_dev *alvium = sd_to_alvium(sd); > > + int ret; > > + > > + ret = alvium_power_on(alvium, true); > > + if (ret) > > + return ret; > > + > > + ret = alvium_hw_init(alvium); > > + if (ret){ > > + regulator_disable(alvium->reg_vcc); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int alvium_runtime_suspend(struct device *dev) > > +{ > > + struct v4l2_subdev *sd = dev_get_drvdata(dev); > > + struct alvium_dev *alvium = sd_to_alvium(sd); > > + > > + alvium_power_on(alvium, false); > > + > > + return 0; > > +} > > + > > +static const struct dev_pm_ops alvium_pm_ops = { > > + RUNTIME_PM_OPS(alvium_runtime_suspend, alvium_runtime_resume, NULL) > > +}; > > + > > +static int alvium_probe(struct i2c_client *client) > > +{ > > + struct device *dev = &client->dev; > > + struct alvium_dev *alvium; > > + int ret; > > + > > + alvium = devm_kzalloc(dev, sizeof(*alvium), GFP_KERNEL); > > + if (!alvium) > > + return -ENOMEM; > > + > > + alvium->i2c_client = client; > > + > > + alvium->regmap = devm_cci_regmap_init_i2c(client, 16); > > + if (IS_ERR(alvium->regmap)) > > + return PTR_ERR(alvium->regmap); > > + > > + ret = alvium_get_dt_data(alvium); > > + if (ret) > > + return ret; > > + > > + alvium->reg_vcc = devm_regulator_get_optional(dev, "vcc-ext-in"); > > + if (IS_ERR(alvium->reg_vcc)) > > + return dev_err_probe(dev, PTR_ERR(alvium->reg_vcc), > > + "no vcc-ext-in regulator provided\n"); > > + > > + ret = alvium_power_on(alvium, true); > > + if (ret) > > + goto err_powerdown; > > + > > + if (!alvium_is_alive(alvium)) { > > + dev_err(dev, "Device detection failed: %d\n", ret); > > + ret = -ENODEV; > > + goto err_powerdown; > > + } > > + > > + ret = alvium_get_hw_info(alvium); > > + if (ret) { > > + dev_err(dev, "get_hw_info fail %d\n", ret); > > + goto err_powerdown; > > + } > > + > > + ret = alvium_hw_init(alvium); > > + if (ret) { > > + dev_err(dev, "hw_init fail %d\n", ret); > > + goto err_powerdown; > > + } > > + > > + ret = alvium_setup_mipi_fmt(alvium); > > + if (ret) { > > + dev_err(dev, "setup_mipi_fmt fail %d\n", ret); > > + goto err_powerdown; > > + } > > + > > + /* > > + * Enable runtime PM without autosuspend: > > + * > > + * Don't use pm autosuspend (alvium have ~7s boot time). > > + * Alvium has been powered manually: > > + * - mark it as active > > + * - increase the usage count without resuming the device. > > + */ > > + pm_runtime_set_active(dev); > > + pm_runtime_get_noresume(dev); > > + pm_runtime_enable(dev); > > + > > + /* Initialize the V4L2 subdev. */ > > + ret = alvium_subdev_init(alvium); > > + if (ret) > > + goto err_pm; > > + > > + ret = v4l2_async_register_subdev(&alvium->sd); > > + if (ret < 0) { > > + dev_err(dev, "Could not register v4l2 device\n"); > > + goto err_subdev; > > + } > > + > > + return 0; > > + > > +err_subdev: > > + alvium_subdev_cleanup(alvium); > > +err_pm: > > + pm_runtime_disable(dev); > > + pm_runtime_put_noidle(dev); > > +err_powerdown: > > + alvium_power_on(alvium, false); > > + > > + return ret; > > +} > > + > > +static void alvium_remove(struct i2c_client *client) > > +{ > > + struct v4l2_subdev *sd = i2c_get_clientdata(client); > > + struct alvium_dev *alvium = sd_to_alvium(sd); > > + struct device *dev = &alvium->i2c_client->dev; > > + > > + i2c_unregister_device(alvium->i2c_client); > > + > > + /* > > + * Disable runtime PM. In case runtime PM is disabled in the kernel, > > + * make sure to turn power off manually. > > + */ > > + pm_runtime_disable(dev); > > + if (!pm_runtime_status_suspended(dev)) > > + alvium_power_on(alvium, false); > > + pm_runtime_set_suspended(dev); > > +} > > + > > +static const struct of_device_id alvium_of_ids[] = { > > + { .compatible = "alliedvision,alvium-csi2", }, > > + { } > > +}; > > +MODULE_DEVICE_TABLE(of, alvium_of_ids); > > + > > +static struct i2c_driver alvium_i2c_driver = { > > + .driver = { > > + .name = "alvium-csi2", > > + .pm = pm_ptr(&alvium_pm_ops), > > + .of_match_table = alvium_of_ids, > > + }, > > + .probe = alvium_probe, > > + .remove = alvium_remove, > > +}; > > + > > +module_i2c_driver(alvium_i2c_driver); > > + > > +MODULE_DESCRIPTION("Allied Vision's Alvium Camera Driver"); > > +MODULE_AUTHOR("Tommaso Merciai <tomm.merciai@xxxxxxxxx>"); > > +MODULE_AUTHOR("Martin Hecht <martin.hecht@xxxxxxxx>"); > > +MODULE_AUTHOR("Avnet Silica Software & Services EMEA"); > > +MODULE_LICENSE("GPL"); > > diff --git a/drivers/media/i2c/alvium-csi2.h b/drivers/media/i2c/alvium-csi2.h > > new file mode 100644 > > index 000000000000..f9cb9716eebc > > --- /dev/null > > +++ b/drivers/media/i2c/alvium-csi2.h > > @@ -0,0 +1,489 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Allied Vision Technologies GmbH Alvium camera driver > > + * > > + * Copyright (C) 2023 Tommaso Merciai > > + * Copyright (C) 2023 Martin Hecht > > + * Copyright (C) 2023 Avnet EMG GmbH > > + */ > > + > > +#ifndef ALVIUM_CSI2_H_ > > +#define ALVIUM_CSI2_H_ > > + > > +#include <linux/kernel.h> > > +#include <linux/regulator/consumer.h> > > +#include <media/v4l2-cci.h> > > +#include <media/v4l2-common.h> > > +#include <media/v4l2-ctrls.h> > > +#include <media/v4l2-fwnode.h> > > +#include <media/v4l2-subdev.h> > > + > > +#define REG_BCRM_V4L2 BIT(31) > > + > > +#define REG_BCRM_V4L2_8BIT(n) (REG_BCRM_V4L2 | CCI_REG8(n)) > > +#define REG_BCRM_V4L2_16BIT(n) (REG_BCRM_V4L2 | CCI_REG16(n)) > > +#define REG_BCRM_V4L2_32BIT(n) (REG_BCRM_V4L2 | CCI_REG32(n)) > > +#define REG_BCRM_V4L2_64BIT(n) (REG_BCRM_V4L2 | CCI_REG64(n)) > > + > > +/* Basic Control Register Map register offsets (BCRM) */ > > +#define REG_BCRM_VERSION_R CCI_REG32(0x0000) > > +#define REG_BCRM_REG_ADDR_R CCI_REG16(0x0014) > > + > > +#define REG_BCRM_FEATURE_INQUIRY_R REG_BCRM_V4L2_64BIT(0x0008) > > +#define REG_BCRM_DEVICE_FIRMWARE_VERSION_R REG_BCRM_V4L2_64BIT(0x0010) > > +#define REG_BCRM_WRITE_HANDSHAKE_RW REG_BCRM_V4L2_8BIT(0x0018) > > + > > +/* Streaming Control Registers */ > > +#define REG_BCRM_SUPPORTED_CSI2_LANE_COUNTS_R REG_BCRM_V4L2_8BIT(0x0040) > > +#define REG_BCRM_CSI2_LANE_COUNT_RW REG_BCRM_V4L2_8BIT(0x0044) > > +#define REG_BCRM_CSI2_CLOCK_MIN_R REG_BCRM_V4L2_32BIT(0x0048) > > +#define REG_BCRM_CSI2_CLOCK_MAX_R REG_BCRM_V4L2_32BIT(0x004c) > > +#define REG_BCRM_CSI2_CLOCK_RW REG_BCRM_V4L2_32BIT(0x0050) > > +#define REG_BCRM_BUFFER_SIZE_R REG_BCRM_V4L2_32BIT(0x0054) > > + > > +#define REG_BCRM_IPU_X_MIN_W REG_BCRM_V4L2_32BIT(0x0058) > > +#define REG_BCRM_IPU_X_MAX_W REG_BCRM_V4L2_32BIT(0x005c) > > +#define REG_BCRM_IPU_X_INC_W REG_BCRM_V4L2_32BIT(0x0060) > > +#define REG_BCRM_IPU_Y_MIN_W REG_BCRM_V4L2_32BIT(0x0064) > > +#define REG_BCRM_IPU_Y_MAX_W REG_BCRM_V4L2_32BIT(0x0068) > > +#define REG_BCRM_IPU_Y_INC_W REG_BCRM_V4L2_32BIT(0x006c) > > +#define REG_BCRM_IPU_X_R REG_BCRM_V4L2_32BIT(0x0070) > > +#define REG_BCRM_IPU_Y_R REG_BCRM_V4L2_32BIT(0x0074) > > + > > +#define REG_BCRM_PHY_RESET_RW REG_BCRM_V4L2_8BIT(0x0078) > > +#define REG_BCRM_LP2HS_DELAY_RW REG_BCRM_V4L2_32BIT(0x007c) > > + > > +/* Acquisition Control Registers */ > > +#define REG_BCRM_ACQUISITION_START_RW REG_BCRM_V4L2_8BIT(0x0080) > > +#define REG_BCRM_ACQUISITION_STOP_RW REG_BCRM_V4L2_8BIT(0x0084) > > +#define REG_BCRM_ACQUISITION_ABORT_RW REG_BCRM_V4L2_8BIT(0x0088) > > +#define REG_BCRM_ACQUISITION_STATUS_R REG_BCRM_V4L2_8BIT(0x008c) > > +#define REG_BCRM_ACQUISITION_FRAME_RATE_RW REG_BCRM_V4L2_64BIT(0x0090) > > +#define REG_BCRM_ACQUISITION_FRAME_RATE_MIN_R REG_BCRM_V4L2_64BIT(0x0098) > > +#define REG_BCRM_ACQUISITION_FRAME_RATE_MAX_R REG_BCRM_V4L2_64BIT(0x00a0) > > +#define REG_BCRM_ACQUISITION_FRAME_RATE_INC_R REG_BCRM_V4L2_64BIT(0x00a8) > > +#define REG_BCRM_ACQUISITION_FRAME_RATE_ENABLE_RW REG_BCRM_V4L2_8BIT(0x00b0) > > + > > +#define REG_BCRM_FRAME_START_TRIGGER_MODE_RW REG_BCRM_V4L2_8BIT(0x00b4) > > +#define REG_BCRM_FRAME_START_TRIGGER_SOURCE_RW REG_BCRM_V4L2_8BIT(0x00b8) > > +#define REG_BCRM_FRAME_START_TRIGGER_ACTIVATION_RW REG_BCRM_V4L2_8BIT(0x00bc) > > +#define REG_BCRM_FRAME_START_TRIGGER_SOFTWARE_W REG_BCRM_V4L2_8BIT(0x00c0) > > +#define REG_BCRM_FRAME_START_TRIGGER_DELAY_RW REG_BCRM_V4L2_32BIT(0x00c4) > > +#define REG_BCRM_EXPOSURE_ACTIVE_LINE_MODE_RW REG_BCRM_V4L2_8BIT(0x00c8) > > +#define REG_BCRM_EXPOSURE_ACTIVE_LINE_SELECTOR_RW REG_BCRM_V4L2_8BIT(0x00cc) > > +#define REG_BCRM_LINE_CONFIGURATION_RW REG_BCRM_V4L2_32BIT(0x00d0) > > + > > +#define REG_BCRM_IMG_WIDTH_RW REG_BCRM_V4L2_32BIT(0x0100) > > +#define REG_BCRM_IMG_WIDTH_MIN_R REG_BCRM_V4L2_32BIT(0x0104) > > +#define REG_BCRM_IMG_WIDTH_MAX_R REG_BCRM_V4L2_32BIT(0x0108) > > +#define REG_BCRM_IMG_WIDTH_INC_R REG_BCRM_V4L2_32BIT(0x010c) > > + > > +#define REG_BCRM_IMG_HEIGHT_RW REG_BCRM_V4L2_32BIT(0x0110) > > +#define REG_BCRM_IMG_HEIGHT_MIN_R REG_BCRM_V4L2_32BIT(0x0114) > > +#define REG_BCRM_IMG_HEIGHT_MAX_R REG_BCRM_V4L2_32BIT(0x0118) > > +#define REG_BCRM_IMG_HEIGHT_INC_R REG_BCRM_V4L2_32BIT(0x011c) > > + > > +#define REG_BCRM_IMG_OFFSET_X_RW REG_BCRM_V4L2_32BIT(0x0120) > > +#define REG_BCRM_IMG_OFFSET_X_MIN_R REG_BCRM_V4L2_32BIT(0x0124) > > +#define REG_BCRM_IMG_OFFSET_X_MAX_R REG_BCRM_V4L2_32BIT(0x0128) > > +#define REG_BCRM_IMG_OFFSET_X_INC_R REG_BCRM_V4L2_32BIT(0x012c) > > + > > +#define REG_BCRM_IMG_OFFSET_Y_RW REG_BCRM_V4L2_32BIT(0x0130) > > +#define REG_BCRM_IMG_OFFSET_Y_MIN_R REG_BCRM_V4L2_32BIT(0x0134) > > +#define REG_BCRM_IMG_OFFSET_Y_MAX_R REG_BCRM_V4L2_32BIT(0x0138) > > +#define REG_BCRM_IMG_OFFSET_Y_INC_R REG_BCRM_V4L2_32BIT(0x013c) > > + > > +#define REG_BCRM_IMG_MIPI_DATA_FORMAT_RW REG_BCRM_V4L2_32BIT(0x0140) > > +#define REG_BCRM_IMG_AVAILABLE_MIPI_DATA_FORMATS_R REG_BCRM_V4L2_64BIT(0x0148) > > +#define REG_BCRM_IMG_BAYER_PATTERN_INQUIRY_R REG_BCRM_V4L2_8BIT(0x0150) > > +#define REG_BCRM_IMG_BAYER_PATTERN_RW REG_BCRM_V4L2_8BIT(0x0154) > > +#define REG_BCRM_IMG_REVERSE_X_RW REG_BCRM_V4L2_8BIT(0x0158) > > +#define REG_BCRM_IMG_REVERSE_Y_RW REG_BCRM_V4L2_8BIT(0x015c) > > + > > +#define REG_BCRM_SENSOR_WIDTH_R REG_BCRM_V4L2_32BIT(0x0160) > > +#define REG_BCRM_SENSOR_HEIGHT_R REG_BCRM_V4L2_32BIT(0x0164) > > +#define REG_BCRM_WIDTH_MAX_R REG_BCRM_V4L2_32BIT(0x0168) > > +#define REG_BCRM_HEIGHT_MAX_R REG_BCRM_V4L2_32BIT(0x016c) > > + > > +#define REG_BCRM_EXPOSURE_TIME_RW REG_BCRM_V4L2_64BIT(0x0180) > > +#define REG_BCRM_EXPOSURE_TIME_MIN_R REG_BCRM_V4L2_64BIT(0x0188) > > +#define REG_BCRM_EXPOSURE_TIME_MAX_R REG_BCRM_V4L2_64BIT(0x0190) > > +#define REG_BCRM_EXPOSURE_TIME_INC_R REG_BCRM_V4L2_64BIT(0x0198) > > +#define REG_BCRM_EXPOSURE_AUTO_RW REG_BCRM_V4L2_8BIT(0x01a0) > > + > > +#define REG_BCRM_INTENSITY_AUTO_PRECEDENCE_RW REG_BCRM_V4L2_8BIT(0x01a4) > > +#define REG_BCRM_INTENSITY_AUTO_PRECEDENCE_VALUE_RW REG_BCRM_V4L2_32BIT(0x01a8) > > +#define REG_BCRM_INTENSITY_AUTO_PRECEDENCE_MIN_R REG_BCRM_V4L2_32BIT(0x01ac) > > +#define REG_BCRM_INTENSITY_AUTO_PRECEDENCE_MAX_R REG_BCRM_V4L2_32BIT(0x01b0) > > +#define REG_BCRM_INTENSITY_AUTO_PRECEDENCE_INC_R REG_BCRM_V4L2_32BIT(0x01b4) > > + > > +#define REG_BCRM_BLACK_LEVEL_RW REG_BCRM_V4L2_32BIT(0x01b8) > > +#define REG_BCRM_BLACK_LEVEL_MIN_R REG_BCRM_V4L2_32BIT(0x01bc) > > +#define REG_BCRM_BLACK_LEVEL_MAX_R REG_BCRM_V4L2_32BIT(0x01c0) > > +#define REG_BCRM_BLACK_LEVEL_INC_R REG_BCRM_V4L2_32BIT(0x01c4) > > + > > +#define REG_BCRM_GAIN_RW REG_BCRM_V4L2_64BIT(0x01c8) > > +#define REG_BCRM_GAIN_MIN_R REG_BCRM_V4L2_64BIT(0x01d0) > > +#define REG_BCRM_GAIN_MAX_R REG_BCRM_V4L2_64BIT(0x01d8) > > +#define REG_BCRM_GAIN_INC_R REG_BCRM_V4L2_64BIT(0x01e0) > > +#define REG_BCRM_GAIN_AUTO_RW REG_BCRM_V4L2_8BIT(0x01e8) > > + > > +#define REG_BCRM_GAMMA_RW REG_BCRM_V4L2_64BIT(0x01f0) > > +#define REG_BCRM_GAMMA_MIN_R REG_BCRM_V4L2_64BIT(0x01f8) > > +#define REG_BCRM_GAMMA_MAX_R REG_BCRM_V4L2_64BIT(0x0200) > > +#define REG_BCRM_GAMMA_INC_R REG_BCRM_V4L2_64BIT(0x0208) > > + > > +#define REG_BCRM_CONTRAST_VALUE_RW REG_BCRM_V4L2_32BIT(0x0214) > > +#define REG_BCRM_CONTRAST_VALUE_MIN_R REG_BCRM_V4L2_32BIT(0x0218) > > +#define REG_BCRM_CONTRAST_VALUE_MAX_R REG_BCRM_V4L2_32BIT(0x021c) > > +#define REG_BCRM_CONTRAST_VALUE_INC_R REG_BCRM_V4L2_32BIT(0x0220) > > + > > +#define REG_BCRM_SATURATION_RW REG_BCRM_V4L2_32BIT(0x0240) > > +#define REG_BCRM_SATURATION_MIN_R REG_BCRM_V4L2_32BIT(0x0244) > > +#define REG_BCRM_SATURATION_MAX_R REG_BCRM_V4L2_32BIT(0x0248) > > +#define REG_BCRM_SATURATION_INC_R REG_BCRM_V4L2_32BIT(0x024c) > > + > > +#define REG_BCRM_HUE_RW REG_BCRM_V4L2_32BIT(0x0250) > > +#define REG_BCRM_HUE_MIN_R REG_BCRM_V4L2_32BIT(0x0254) > > +#define REG_BCRM_HUE_MAX_R REG_BCRM_V4L2_32BIT(0x0258) > > +#define REG_BCRM_HUE_INC_R REG_BCRM_V4L2_32BIT(0x025c) > > + > > +#define REG_BCRM_ALL_BALANCE_RATIO_RW REG_BCRM_V4L2_64BIT(0x0260) > > +#define REG_BCRM_ALL_BALANCE_RATIO_MIN_R REG_BCRM_V4L2_64BIT(0x0268) > > +#define REG_BCRM_ALL_BALANCE_RATIO_MAX_R REG_BCRM_V4L2_64BIT(0x0270) > > +#define REG_BCRM_ALL_BALANCE_RATIO_INC_R REG_BCRM_V4L2_64BIT(0x0278) > > + > > +#define REG_BCRM_RED_BALANCE_RATIO_RW REG_BCRM_V4L2_64BIT(0x0280) > > +#define REG_BCRM_RED_BALANCE_RATIO_MIN_R REG_BCRM_V4L2_64BIT(0x0288) > > +#define REG_BCRM_RED_BALANCE_RATIO_MAX_R REG_BCRM_V4L2_64BIT(0x0290) > > +#define REG_BCRM_RED_BALANCE_RATIO_INC_R REG_BCRM_V4L2_64BIT(0x0298) > > + > > +#define REG_BCRM_GREEN_BALANCE_RATIO_RW REG_BCRM_V4L2_64BIT(0x02a0) > > +#define REG_BCRM_GREEN_BALANCE_RATIO_MIN_R REG_BCRM_V4L2_64BIT(0x02a8) > > +#define REG_BCRM_GREEN_BALANCE_RATIO_MAX_R REG_BCRM_V4L2_64BIT(0x02b0) > > +#define REG_BCRM_GREEN_BALANCE_RATIO_INC_R REG_BCRM_V4L2_64BIT(0x02b8) > > + > > +#define REG_BCRM_BLUE_BALANCE_RATIO_RW REG_BCRM_V4L2_64BIT(0x02c0) > > +#define REG_BCRM_BLUE_BALANCE_RATIO_MIN_R REG_BCRM_V4L2_64BIT(0x02c8) > > +#define REG_BCRM_BLUE_BALANCE_RATIO_MAX_R REG_BCRM_V4L2_64BIT(0x02d0) > > +#define REG_BCRM_BLUE_BALANCE_RATIO_INC_R REG_BCRM_V4L2_64BIT(0x02d8) > > + > > +#define REG_BCRM_WHITE_BALANCE_AUTO_RW REG_BCRM_V4L2_8BIT(0x02e0) > > +#define REG_BCRM_SHARPNESS_RW REG_BCRM_V4L2_32BIT(0x0300) > > +#define REG_BCRM_SHARPNESS_MIN_R REG_BCRM_V4L2_32BIT(0x0304) > > +#define REG_BCRM_SHARPNESS_MAX_R REG_BCRM_V4L2_32BIT(0x0308) > > +#define REG_BCRM_SHARPNESS_INC_R REG_BCRM_V4L2_32BIT(0x030c) > > + > > +#define REG_BCRM_DEVICE_TEMPERATURE_R REG_BCRM_V4L2_32BIT(0x0310) > > +#define REG_BCRM_EXPOSURE_AUTO_MIN_RW REG_BCRM_V4L2_64BIT(0x0330) > > +#define REG_BCRM_EXPOSURE_AUTO_MAX_RW REG_BCRM_V4L2_64BIT(0x0338) > > +#define REG_BCRM_GAIN_AUTO_MIN_RW REG_BCRM_V4L2_64BIT(0x0340) > > +#define REG_BCRM_GAIN_AUTO_MAX_RW REG_BCRM_V4L2_64BIT(0x0348) > > + > > +/* Heartbeat reg*/ > > +#define REG_BCRM_HEARTBEAT_RW CCI_REG8(0x021f) > > + > > +/* GenCP Registers */ > > +#define REG_GENCP_CHANGEMODE_W CCI_REG8(0x021c) > > +#define REG_GENCP_CURRENTMODE_R CCI_REG8(0x021d) > > +#define REG_GENCP_IN_HANDSHAKE_RW CCI_REG8(0x001c) > > +#define REG_GENCP_OUT_SIZE_W CCI_REG16(0x0020) > > +#define REG_GENCP_IN_SIZE_R CCI_REG16(0x0024) > > + > > +/* defines */ > > +#define REG_BCRM_HANDSHAKE_STATUS_MASK 0x01 > > +#define REG_BCRM_HANDSHAKE_AVAILABLE_MASK 0x80 > > + > > +#define BCRM_HANDSHAKE_W_DONE_EN_BIT BIT(0) > > + > > +#define ALVIUM_DEFAULT_FR_HZ 10 > > +#define ALVIUM_DEFAULT_PIXEL_RATE_MHZ 148000000 > > + > > +#define ALVIUM_LP2HS_DELAY_MS 100 > > + > > +enum alvium_bcrm_mode { > > + ALVIUM_BCM_MODE, > > + ALVIUM_GENCP_MODE, > > + ALVIUM_NUM_MODE > > +}; > > + > > +enum alvium_mipi_fmt { > > + ALVIUM_FMT_UYVY8_2X8 = 0, > > + ALVIUM_FMT_UYVY8_1X16, > > + ALVIUM_FMT_YUYV8_1X16, > > + ALVIUM_FMT_YUYV8_2X8, > > + ALVIUM_FMT_YUYV10_1X20, > > + ALVIUM_FMT_RGB888_1X24, > > + ALVIUM_FMT_RBG888_1X24, > > + ALVIUM_FMT_BGR888_1X24, > > + ALVIUM_FMT_RGB888_3X8, > > + ALVIUM_FMT_Y8_1X8, > > + ALVIUM_FMT_SGRBG8_1X8, > > + ALVIUM_FMT_SRGGB8_1X8, > > + ALVIUM_FMT_SGBRG8_1X8, > > + ALVIUM_FMT_SBGGR8_1X8, > > + ALVIUM_FMT_Y10_1X10, > > + ALVIUM_FMT_SGRBG10_1X10, > > + ALVIUM_FMT_SRGGB10_1X10, > > + ALVIUM_FMT_SGBRG10_1X10, > > + ALVIUM_FMT_SBGGR10_1X10, > > + ALVIUM_FMT_Y12_1X12, > > + ALVIUM_FMT_SGRBG12_1X12, > > + ALVIUM_FMT_SRGGB12_1X12, > > + ALVIUM_FMT_SGBRG12_1X12, > > + ALVIUM_FMT_SBGGR12_1X12, > > + ALVIUM_FMT_SBGGR14_1X14, > > + ALVIUM_FMT_SGBRG14_1X14, > > + ALVIUM_FMT_SRGGB14_1X14, > > + ALVIUM_FMT_SGRBG14_1X14, > > + ALVIUM_NUM_SUPP_MIPI_DATA_FMT > > +}; > > + > > +enum alvium_av_bayer_bit { > > + ALVIUM_BIT_BAY_NONE = -1, > > + ALVIUM_BIT_BAY_MONO = 0, > > + ALVIUM_BIT_BAY_GR, > > + ALVIUM_BIT_BAY_RG, > > + ALVIUM_BIT_BAY_GB, > > + ALVIUM_BIT_BAY_BG, > > + ALVIUM_NUM_BAY_AV_BIT > > +}; > > + > > +enum alvium_av_mipi_bit { > > + ALVIUM_BIT_YUV420_8_LEG = 0, > > + ALVIUM_BIT_YUV420_8, > > + ALVIUM_BIT_YUV420_10, > > + ALVIUM_BIT_YUV420_8_CSPS, > > + ALVIUM_BIT_YUV420_10_CSPS, > > + ALVIUM_BIT_YUV422_8, > > + ALVIUM_BIT_YUV422_10, > > + ALVIUM_BIT_RGB888, > > + ALVIUM_BIT_RGB666, > > + ALVIUM_BIT_RGB565, > > + ALVIUM_BIT_RGB555, > > + ALVIUM_BIT_RGB444, > > + ALVIUM_BIT_RAW6, > > + ALVIUM_BIT_RAW7, > > + ALVIUM_BIT_RAW8, > > + ALVIUM_BIT_RAW10, > > + ALVIUM_BIT_RAW12, > > + ALVIUM_BIT_RAW14, > > + ALVIUM_BIT_JPEG, > > + ALVIUM_NUM_SUPP_MIPI_DATA_BIT > > +}; > > + > > +struct alvium_bcrm_vers { > > + __le16 minor; > > + __le16 major; > > +}; > > + > > +struct alvium_fw_vers { > > + u8 special; > > + u8 major; > > + __le16 minor; > > + __le32 patch; > > +}; > > + > > +struct alvium_avail_feat { > > + u64 rev_x:1; > > + u64 rev_y:1; > > + u64 int_autop:1; > > + u64 black_lvl:1; > > + u64 gain:1; > > + u64 gamma:1; > > + u64 contrast:1; > > + u64 sat:1; > > + u64 hue:1; > > + u64 whiteb:1; > > + u64 sharp:1; > > + u64 auto_exp:1; > > + u64 auto_gain:1; > > + u64 auto_whiteb:1; > > + u64 dev_temp:1; > > + u64 acq_abort:1; > > + u64 acq_fr:1; > > + u64 fr_trigger:1; > > + u64 exp_acq_line:1; > > + u64 reserved:45; > > +}; > > + > > +struct alvium_avail_mipi_fmt { > > + u64 yuv420_8_leg:1; > > + u64 yuv420_8:1; > > + u64 yuv420_10:1; > > + u64 yuv420_8_csps:1; > > + u64 yuv420_10_csps:1; > > + u64 yuv422_8:1; > > + u64 yuv422_10:1; > > + u64 rgb888:1; > > + u64 rgb666:1; > > + u64 rgb565:1; > > + u64 rgb555:1; > > + u64 rgb444:1; > > + u64 raw6:1; > > + u64 raw7:1; > > + u64 raw8:1; > > + u64 raw10:1; > > + u64 raw12:1; > > + u64 raw14:1; > > + u64 jpeg:1; > > + u64 reserved:45; > > +}; > > + > > +struct alvium_avail_bayer { > > + u8 mono:1; > > + u8 gr:1; > > + u8 rg:1; > > + u8 gb:1; > > + u8 bg:1; > > + u8 reserved:3; > > +}; > > + > > +struct alvium_mode { > > + struct v4l2_rect crop; > > + struct v4l2_mbus_framefmt fmt; > > + u32 width; > > + u32 height; > > +}; > > + > > +struct alvium_pixfmt { > > + u32 code; > > + u32 colorspace; > > + u64 mipi_fmt_regval; > > + u64 bay_fmt_regval; > > + u8 id; > > + u8 is_raw; > > + u8 fmt_av_bit; > > + u8 bay_av_bit; > > +}; > > + > > +struct alvium_ctrls { > > + struct v4l2_ctrl_handler handler; > > + struct v4l2_ctrl *pixel_rate; > > + struct v4l2_ctrl *link_freq; > > + struct { > > + struct v4l2_ctrl *auto_exp; > > + struct v4l2_ctrl *exposure; > > + }; > > + struct { > > + struct v4l2_ctrl *auto_wb; > > + struct v4l2_ctrl *blue_balance; > > + struct v4l2_ctrl *red_balance; > > + }; > > + struct { > > + struct v4l2_ctrl *auto_gain; > > + struct v4l2_ctrl *gain; > > + }; > > + struct v4l2_ctrl *saturation; > > + struct v4l2_ctrl *hue; > > + struct v4l2_ctrl *contrast; > > + struct v4l2_ctrl *gamma; > > + struct v4l2_ctrl *sharpness; > > + struct v4l2_ctrl *hflip; > > + struct v4l2_ctrl *vflip; > > +}; > > + > > +struct alvium_dev { > > + struct i2c_client *i2c_client; > > + struct v4l2_subdev sd; > > + struct v4l2_fwnode_endpoint ep; > > + struct media_pad pad; > > + struct regmap *regmap; > > + > > + struct regulator *reg_vcc; > > + > > + u16 bcrm_addr; > > + > > + struct alvium_avail_feat avail_ft; > > + u8 is_mipi_fmt_avail[ALVIUM_NUM_SUPP_MIPI_DATA_BIT]; > > + u8 is_bay_avail[ALVIUM_NUM_BAY_AV_BIT]; > > + > > + u32 min_csi_clk; > > + u32 max_csi_clk; > > + u32 dft_img_width; > > + u32 img_min_width; > > + u32 img_max_width; > > + u32 img_inc_width; > > + u32 dft_img_height; > > + u32 img_min_height; > > + u32 img_max_height; > > + u32 img_inc_height; > > + u32 min_offx; > > + u32 max_offx; > > + u32 inc_offx; > > + u32 min_offy; > > + u32 max_offy; > > + u32 inc_offy; > > + u64 dft_gain; > > + u64 min_gain; > > + u64 max_gain; > > + u64 inc_gain; > > + u64 dft_exp; > > + u64 min_exp; > > + u64 max_exp; > > + u64 inc_exp; > > + u64 dft_rbalance; > > + u64 min_rbalance; > > + u64 max_rbalance; > > + u64 inc_rbalance; > > + u64 dft_bbalance; > > + u64 min_bbalance; > > + u64 max_bbalance; > > + u64 inc_bbalance; > > + s32 dft_hue; > > + s32 min_hue; > > + s32 max_hue; > > + s32 inc_hue; > > + u32 dft_contrast; > > + u32 min_contrast; > > + u32 max_contrast; > > + u32 inc_contrast; > > + u32 dft_sat; > > + u32 min_sat; > > + u32 max_sat; > > + u32 inc_sat; > > + s32 dft_black_lvl; > > + s32 min_black_lvl; > > + s32 max_black_lvl; > > + s32 inc_black_lvl; > > + u64 dft_gamma; > > + u64 min_gamma; > > + u64 max_gamma; > > + u64 inc_gamma; > > + s32 dft_sharp; > > + s32 min_sharp; > > + s32 max_sharp; > > + s32 inc_sharp; > > + > > + struct alvium_mode mode; > > + struct v4l2_fract frame_interval; > > + u64 min_fr; > > + u64 max_fr; > > + u64 fr; > > + > > + u8 h_sup_csi_lanes; > > + u32 csi_clk; > > + u64 link_freq; > > + > > + struct alvium_ctrls ctrls; > > + > > + u8 bcrm_mode; > > + > > + struct alvium_pixfmt *alvium_csi2_fmt; > > + u8 alvium_csi2_fmt_n; > > + > > + u8 streaming; > > + u8 apply_fiv; > > +}; > > + > > +static inline struct alvium_dev *sd_to_alvium(struct v4l2_subdev *sd) > > +{ > > + return container_of(sd, struct alvium_dev, sd); > > +} > > + > > +static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl) > > +{ > > + return &container_of(ctrl->handler, struct alvium_dev, > > + ctrls.handler)->sd; > > +} > > +#endif /* ALVIUM_CSI2_H_ */ > > -- > Kind regards, > > Sakari Ailus Thanks & Regards, Tommaso