Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Date:   Thu, 19 Oct 2023 14:58:42 +0000
From:   Sakari Ailus <sakari.ailus@linux.intel.com>
To:     Tommaso Merciai <tomm.merciai@gmail.com>
Cc:     laurent.pinchart@ideasonboard.com, linuxfancy@googlegroups.com,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Martin Hecht <martin.hecht@avnet.eu>,
        Liam Girdwood <lgirdwood@gmail.com>,
        Mark Brown <broonie@kernel.org>,
        Hans de Goede <hdegoede@redhat.com>,
        Andy Shevchenko <andy.shevchenko@gmail.com>,
        Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>,
        Marco Felsch <m.felsch@pengutronix.de>,
        Gerald Loacker <gerald.loacker@wolfvision.net>,
        Hans Verkuil <hverkuil-cisco@xs4all.nl>,
        Mikhail Rudenko <mike.rudenko@gmail.com>,
        Bingbu Cao <bingbu.cao@intel.com>,
        Linus Walleij <linus.walleij@linaro.org>,
        linux-kernel@vger.kernel.org, linux-media@vger.kernel.org
Subject: Re: [PATCH v9 3/3] media: i2c: Add support for alvium camera
Message-ID: <ZTFEIl5ovnBQm2gn@kekkonen.localdomain>
References: <20230928145638.2268331-1-tomm.merciai@gmail.com>
 <20230928145638.2268331-4-tomm.merciai@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230928145638.2268331-4-tomm.merciai@gmail.com>
Precedence: bulk

Hi Tommaso,

More comments below. Quite a few actually. Nothing grave though.

On Thu, Sep 28, 2023 at 04:56:34PM +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@gmail.com>
> ---
> 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@intel.com>
> 
> 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
> 
>  MAINTAINERS                     |    9 +
>  drivers/media/i2c/Kconfig       |   10 +
>  drivers/media/i2c/Makefile      |    1 +
>  drivers/media/i2c/alvium-csi2.c | 2761 +++++++++++++++++++++++++++++++
>  drivers/media/i2c/alvium-csi2.h |  474 ++++++
>  5 files changed, 3255 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 83a0c7f3826b..277f2d1e894b 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@gmail.com>
> +M:	Martin Hecht <martin.hecht@avnet.eu>
> +L:	linux-media@vger.kernel.org
> +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@kernel.org>
>  L:	linux-media@vger.kernel.org
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 59ee0ca2c978..6125f85e7c65 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..33c096ca3d93
> --- /dev/null
> +++ b/drivers/media/i2c/alvium-csi2.c
> @@ -0,0 +1,2761 @@
> +// 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_RGB888,
> +		.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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	/* 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),

Alignment.

> +			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),

Ditto.

> +			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);

Shouldn't you check ret here?

> +	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);

Ditto.

> +	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);

Please check for ret before you use the value. This happens elsewhere, too.

> +	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_R, &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]);

Please align further lines of the same function call just right of the
opening parenthesis.

> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	alvium_read(alvium, REG_BCRM_CSI2_LANE_COUNT_RW, &val, &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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	num_lanes = alvium->ep.bus.mipi_csi2.num_data_lanes;
> +
> +	if (num_lanes <= alvium->h_sup_csi_lanes) {

	if (numlanes > alvium->h_sup_csi_lanes)
		return -EINVAL;


And you can unindent the rest.

> +		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;
> +		}
> +	} else {
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int alvium_set_lp2hs_delay(struct alvium_dev *alvium)
> +{
> +	struct device *dev = &alvium->i2c_client->dev;
> +	u64 lp2hs_delay;
> +	int ret;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	/*
> +	 * The purpose of this reg is force a DPhy reset
> +	 * for the period described by the microseconds 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.
> +	 *
> +	 * This may help a csi2 rx ip to reset if that IP
> +	 * can't deal with a continous clock.
> +	 */
> +	lp2hs_delay = (u64)alvium->lp2hs_delay;
> +
> +	ret = alvium_write_hshake(alvium, REG_BCRM_LP2HS_DELAY_RW, lp2hs_delay);
> +	if (ret) {
> +		dev_err(dev, "Fail to set streamon delay reg\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int alvium_get_csi_clk_params(struct alvium_dev *alvium)
> +{
> +	u64 val;
> +	int ret = 0;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	csi_clk = (u32)alvium->ep.link_frequencies[0];
> +
> +	if (csi_clk > alvium->min_csi_clk ||

Similar comment on this one.

> +		csi_clk < alvium->max_csi_clk) {
> +		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;
> +		}
> +	} else {
> +		return -EINVAL;
> +	}
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	alvium_read(alvium, REG_BCRM_IMG_WIDTH_MIN_R, &val, &ret);
> +	alvium->img_min_width = val;

You have this same pattern in so so many places you could consider using
C11 _Generic for writing the value directly to where it's needed.

> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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);

Check ret here.

> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	alvium_read(alvium, REG_BCRM_IMG_AVAILABLE_MIPI_DATA_FORMATS_R, &val, &ret);

Same here.

> +	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;
> +
> +	alvium->alvium_csi2_fmt = NULL;
> +
> +	/* calculate fmt array size */
> +	for (int 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]))
> +				sz++;
> +	}
> +
> +	/* init alvium_csi2_fmt array */
> +	alvium->alvium_csi2_fmt_n = sz;
> +	alvium->alvium_csi2_fmt = kmalloc_array(sz,
> +						     sizeof(struct alvium_pixfmt),
> +						     GFP_KERNEL);
> +
> +	/* Create the alvium_csi2 fmt array from formats available */
> +	for (int fmt = 0; fmt < ALVIUM_NUM_SUPP_MIPI_DATA_FMT; fmt++) {

Not all compilers support this yet I believe --- please declare fmt with
the rest of the variables. Same elsewhere.

> +		if (alvium->is_mipi_fmt_avail[alvium_csi2_fmts[fmt].fmt_av_bit])

		if (!...)
			continue;

And you can unindnet the rest.

> +			if ((!alvium_csi2_fmts[fmt].is_raw) ||
> +				  (alvium->is_bay_avail[alvium_csi2_fmts[fmt].bay_av_bit])) {
> +				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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	alvium_read(alvium, REG_BCRM_IMG_BAYER_PATTERN_INQUIRY_R, &val, &ret);

Missing ret check.

> +	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);

Missing ret checks. Please check the rest of the calls.

> +	alvium->min_fr = val;
> +
> +	alvium_read(alvium, REG_BCRM_ACQUISITION_FRAME_RATE_MAX_R, &val, &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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;

You already check for alvium->bcrm_addr in alvium_write_hshake. Same below.

> +
> +	fr = alvium->fr;
> +
> +	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 (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	if (!alvium->avail_ft.auto_gain)
> +		return -EINVAL;
> +
> +	/* 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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	if (!alvium->avail_ft.auto_gain)
> +		return -EINVAL;
> +
> +	/* 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->bcrm_addr)
> +		return -EINVAL;
> +
> +	if (!alvium->avail_ft.auto_gain)
> +		return -EINVAL;
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	if (!alvium->avail_ft.auto_exp)
> +		return -EINVAL;
> +
> +	/* 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->bcrm_addr)
> +		return -EINVAL;
> +
> +	if (!alvium->avail_ft.auto_exp)
> +		return -EINVAL;
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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) {
> +		u64 max_gain;
> +		u64 min_gain;
> +
> +		ret = alvium_get_gain_params(alvium);
> +		if (ret)
> +			return ret;
> +
> +		max_gain = alvium->max_gain;
> +		min_gain = alvium->min_gain;
> +
> +		if (ctrls->gain->val >= min_gain &&
> +			  ctrls->gain->val <= max_gain) {
> +			ret = alvium_set_gain(alvium, ctrls->gain->val);
> +		} else {
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static int alvium_set_blue_balance_ratio(struct alvium_dev *alvium, int blue)
> +{
> +	struct device *dev = &alvium->i2c_client->dev;
> +	int ret;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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->bcrm_addr)
> +		return -EINVAL;
> +
> +	if (!alvium->avail_ft.auto_whiteb)
> +		return -EINVAL;
> +
> +	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);

Over 80 characters per line without an obvious reason. There are a few such
occurrences elsewhere, too.

> +
> +	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) {
> +		u64 max_exp;
> +		u64 min_exp;
> +
> +		ret = alvium_get_exposure_params(alvium);
> +		if (ret)
> +			return ret;
> +
> +		max_exp = alvium->max_exp;
> +		min_exp = alvium->min_exp;

Can the exposure limits change after setting up controls or at some other
point of time the driver is in control? If not, ensure the range is
correct, otherwise adjust the value. Don't return an error.

> +
> +		if (ctrls->exposure->val >= min_exp &&
> +			  ctrls->exposure->val <= max_exp) {
> +			ret = alvium_set_exposure(alvium, ctrls->exposure->val);
> +			if (ret)
> +				return ret;
> +		} else {
> +			return -EINVAL;
> +		}
> +	}
> +
> +	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;
> +	}
> +
> +	if (!awb && ctrls->auto_wb->is_new) {
> +		u64 red = (u64)ctrls->red_balance->val;
> +		u64 blue = (u64)ctrls->blue_balance->val;
> +		u64 red_max;
> +		u64 red_min;
> +		u64 blue_max;
> +		u64 blue_min;
> +
> +		ret = alvium_get_red_balance_ratio_params(alvium);
> +		if (ret)
> +			return ret;
> +
> +		ret = alvium_get_blue_balance_ratio_params(alvium);
> +		if (ret)
> +			return ret;
> +
> +		red_max = alvium->max_rbalance;
> +		red_min = alvium->min_rbalance;
> +		blue_max = alvium->max_bbalance;
> +		blue_min = alvium->min_rbalance;

Same comment on this one.

> +
> +		if (red >= red_min &&
> +			  red <= red_max) {
> +			ret = alvium_set_red_balance_ratio(alvium, red);
> +			if (ret)
> +				return ret;
> +		} else {
> +			return -EINVAL;
> +		}
> +
> +		if (blue >= blue_min &&
> +			  blue <= blue_max) {
> +			ret = alvium_set_blue_balance_ratio(alvium, blue);
> +			if (ret)
> +				return ret;
> +		} else {
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static int alvium_set_ctrl_hue(struct alvium_dev *alvium, int val)
> +{
> +	struct device *dev = &alvium->i2c_client->dev;
> +	int ret;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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;
> +
> +	if (!alvium->bcrm_addr)
> +		return -EINVAL;
> +
> +	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;

How likely is it obtaining an individual parameter fails? I'd suppose this
either works always (or fails always)? I'd drop the individual messages
here and in other similar places. Up to you though.

> +	}
> +
> +	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 lanes\n");
> +		return ret;
> +	}
> +
> +	ret = alvium_set_lp2hs_delay(alvium);
> +	if (ret) {
> +		dev_err(dev, "Fail to set streamon 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)

Fits on the same line.

> +		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);
> +	const struct alvium_pixfmt *formats;
> +	unsigned int num_formats;
> +
> +	formats = alvium->alvium_csi2_fmt;
> +	num_formats = alvium->alvium_csi2_fmt_n;

Both of these local variables are a bit redundant: you could just access
the information where it is stored.

> +
> +	if (code->index >= num_formats)
> +		return -EINVAL;
> +
> +	code->code = formats[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 device *dev = &alvium->i2c_client->dev;
> +	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);
> +
> +	if (fmt->width < alvium->img_max_width &&
> +		   fmt->width > alvium->img_min_width) {
> +		ret = alvium_set_img_width(alvium, fmt->width);
> +		if (ret)
> +			return ret;
> +	} else {
> +		dev_err(dev, "Unsupported width");
> +		return -EINVAL;

Same here than on selections: adjust the value, don't return an error.

Haven't you already validated the configuration previously when setting
format?

> +	}
> +
> +	if (fmt->height < alvium->img_max_height &&
> +		   fmt->height > alvium->img_min_height) {
> +		ret = alvium_set_img_height(alvium, fmt->height);
> +		if (ret)
> +			return ret;
> +	} else {
> +		dev_err(dev, "Unsupported height");
> +		return -EINVAL;
> +	}
> +
> +	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);

Alignment.

> +
> +	return ret;
> +}
> +
> +static int alvium_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> +	struct alvium_dev *alvium = sd_to_alvium(sd);
> +	struct v4l2_mbus_framefmt *fmt;
> +	struct v4l2_subdev_state *state;
> +	int ret = 0;
> +

Same comment on runtime PM as on s_ctrl callback.

> +	state = v4l2_subdev_lock_and_get_active_state(sd);
> +
> +	if (enable) {
> +		ret = __v4l2_ctrl_handler_setup(&alvium->ctrls.handler);
> +		if (ret)
> +			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 {
> +		ret = alvium_set_stream_mipi(alvium, enable);
> +		if (ret)
> +			goto out;
> +	}
> +
> +	v4l2_subdev_unlock_state(state);
> +	alvium->streaming = !!enable;

This goes within the section where you hold the lock.

> +
> +	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;
> +	struct v4l2_mbus_framefmt *fmt;
> +	struct v4l2_rect *crop;
> +

Extra newline.

> +	struct v4l2_subdev_format sd_fmt = {
> +		.which = V4L2_SUBDEV_FORMAT_TRY,
> +		.format = alvium_csi2_default_fmt,
> +	};
> +

Here, too.

> +	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,
> +		},
> +	};
> +
> +	crop = v4l2_subdev_get_pad_crop(sd, state, 0);
> +	fmt = v4l2_subdev_get_pad_format(sd, state, 0);
> +
> +	*crop = sd_crop.rect;
> +	*fmt = sd_fmt.format;

crop and fmt are redundant, too: just assign sd_crop.rect and sd_fmt.format
directly.

> +
> +	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 &&
> +		  format->format.width >= alvium->img_min_width) {
> +		fmt->width = format->format.width;
> +	} else {

Adjust, don't return an error.

> +		return -EINVAL;
> +	}
> +
> +	if (format->format.height <= alvium->img_max_height &&
> +		  format->format.height >= alvium->img_min_height) {
> +		fmt->height = format->format.height;
> +	} else {
> +		return -EINVAL;
> +	}
> +
> +	alvium_csi2_fmt = alvium_code_to_pixfmt(alvium, format->format.code);
> +	fmt->code = alvium_csi2_fmt->code;
> +
> +	/* Set also the crop width and height when set a new fmt */
> +	crop->width = fmt->width;
> +	crop->height = fmt->height;
> +
> +	*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;
> +
> +	if (sel->target != V4L2_SEL_TGT_CROP)
> +		return -EINVAL;
> +
> +	if ((sel->r.left + sel->r.width) <= alvium->img_max_width)
> +		crop->left = sel->r.left;
> +	else
> +		return -EINVAL;

Instead of failing on invalid user-provided sizes, you should adjust them
to make them valid.

> +
> +	if ((sel->r.top + sel->r.height) <= alvium->img_max_height)
> +		crop->top = sel->r.top;
> +	else
> +		return -EINVAL;

Same here.

> +
> +	crop->width = sel->r.width;
> +	crop->height = sel->r.height;
> +
> +	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:

Autogain is basically a boolean for enabling and disabling automatic gain
control. If you want analogue or digital gain value, use
V4L2_CID{_ANALOGUE,_DIGITAL,}_GAIN. Same for s_ctrl below.

> +		val = alvium_get_gain(alvium);
> +		if (val < 0)
> +			return val;
> +		alvium->ctrls.gain->val = val;
> +		break;
> +	case V4L2_CID_EXPOSURE_AUTO:
> +		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);
> +	int ret;
> +

As you do support runtime PM, you should prepare for the device being
suspended. Please see e.g. the imx319 driver. This is also relevant for
setting streaming (as well as for getting volatile controls).

> +	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;
> +	}
> +
> +	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 */
> +	if (alvium->avail_ft.auto_whiteb)
> +		ctrls->auto_wb = v4l2_ctrl_new_std(hdl, ops,
> +					     V4L2_CID_AUTO_WHITE_BALANCE,
> +					     0, 1, 1, 1);
> +
> +	ctrls->blue_balance = v4l2_ctrl_new_std(hdl, ops,
> +					     V4L2_CID_BLUE_BALANCE,
> +					     alvium->min_bbalance,
> +					     alvium->max_bbalance,
> +					     alvium->inc_bbalance,
> +					     alvium->min_bbalance);
> +	ctrls->red_balance = v4l2_ctrl_new_std(hdl, ops,
> +					     V4L2_CID_RED_BALANCE,
> +					     alvium->min_rbalance,
> +					     alvium->max_rbalance,
> +					     alvium->inc_rbalance,
> +					     alvium->min_rbalance);
> +
> +	/* Auto/manual exposure */
> +	if (alvium->avail_ft.auto_exp)
> +		ctrls->auto_exp = v4l2_ctrl_new_std_menu(hdl, ops,
> +					     V4L2_CID_EXPOSURE_AUTO,
> +					     V4L2_EXPOSURE_MANUAL, 0,
> +					     V4L2_EXPOSURE_AUTO);
> +
> +	ctrls->exposure = v4l2_ctrl_new_std(hdl, ops,
> +					     V4L2_CID_EXPOSURE,
> +					     alvium->min_exp,
> +					     alvium->max_exp,
> +					     alvium->inc_exp,
> +					     alvium->min_exp);
> +
> +	/* Auto/manual gain */
> +	if (alvium->avail_ft.auto_gain)
> +		ctrls->auto_gain = v4l2_ctrl_new_std(hdl, ops,
> +					     V4L2_CID_AUTOGAIN,
> +					     0, 1, 1, 1);
> +
> +	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->min_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->max_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->min_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->min_contrast);
> +
> +	if (alvium->avail_ft.gamma)
> +		ctrls->contrast = v4l2_ctrl_new_std(hdl, ops,
> +					     V4L2_CID_GAMMA,
> +					     alvium->min_gamma,
> +					     alvium->max_gamma,
> +					     alvium->inc_gamma,
> +					     alvium->min_gamma);
> +
> +	if (alvium->avail_ft.sharp)
> +		ctrls->contrast = v4l2_ctrl_new_std(hdl, ops,
> +					     V4L2_CID_SHARPNESS,
> +					     alvium->min_sharp,
> +					     alvium->max_sharp,
> +					     alvium->inc_sharp,
> +					     alvium->min_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;
> +
> +	if (!fwnode)
> +		return -EINVAL;
> +
> +	/* Only CSI2 is supported for now: */
> +	alvium->ep.bus_type = V4L2_MBUS_CSI2_DPHY;
> +
> +	fwnode_property_read_u32(fwnode, "alliedvision,lp2hs-delay-us",
> +							  &alvium->lp2hs_delay);
> +
> +	endpoint = fwnode_graph_get_next_endpoint(fwnode, NULL);

	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");

Put endpoint on error path. Same below. I'd introduce a new label for it.

> +		return 0;
> +	}
> +
> +	if (alvium->ep.bus_type != V4L2_MBUS_CSI2_DPHY) {

No need to check for this --- if you're asking for D-PHY (as you are),
you'll get it (or you'll get an error you already check above)

> +		dev_err(dev, "unsupported bus type\n");
> +		return -EINVAL;
> +	}
> +
> +	if (!alvium->ep.nr_of_link_frequencies) {
> +		dev_err(dev, "no link frequencies defined");
> +		return -EINVAL;
> +	}
> +
> +	dev_dbg(dev, "freq: %llu\n", alvium->ep.link_frequencies[0]);
> +	dev_dbg(dev, "lanes: %d\n", alvium->ep.bus.mipi_csi2.num_data_lanes);

The V4L2 fwnode framework already prints both, please remove.

> +
> +	return 0;
> +}
> +
> +static int alvium_power_on(struct alvium_dev *alvium)
> +{
> +	struct device *dev = &alvium->i2c_client->dev;
> +	int ret = 0;
> +
> +	if (regulator_is_enabled(alvium->reg_vcc)) {

No, you must use regulator_enable() unconditionally. It counts the number
of enables and disables.

> +		/* vcc-ext-in reg is already enabled */
> +		dev_dbg(dev, "vcc-ext-in reg already enabled");
> +	} else {
> +		ret = regulator_enable(alvium->reg_vcc);
> +		if (ret)
> +			return ret;
> +
> +		/* alvium boot time 7s*/
> +		msleep(7000);
> +	}
> +
> +	return 0;
> +}
> +
> +static int alvium_power_off(struct alvium_dev *alvium)
> +{
> +	int ret = 0;
> +
> +	ret = regulator_disable(alvium->reg_vcc);

You may return the return value of regulator_disable without a need of a
temporary variable.

> +	if (ret)
> +		return ret;
> +
> +	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);
> +	if (ret)
> +		return ret;
> +
> +	ret = alvium_hw_init(alvium);

Missing error handling i.e. you'll have to power off the device if this
fails.

> +	if (ret)
> +		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_off(alvium);

You could merge your runtime PM callback and alvium_power_{on,off}.

> +
> +	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);
> +	if (ret)
> +		return ret;

From here onwards, you'll need to call alvium_power_off() on the error
path.

> +
> +	if (!alvium_is_alive(alvium)) {
> +		dev_err(dev, "Device detection failed: %d\n", ret);
> +		return -ENODEV;
> +	}
> +
> +	ret = alvium_get_hw_info(alvium);
> +	if (ret) {
> +		dev_err(dev, "get_hw_info fail %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = alvium_hw_init(alvium);
> +	if (ret) {
> +		dev_err(dev, "hw_init fail %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = alvium_setup_mipi_fmt(alvium);
> +	if (ret) {
> +		dev_err(dev, "setup_mipi_fmt fail %d\n", ret);
> +		return ret;
> +	}
> +
> +	/*
> +	 * 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.
> +	 */

Well in this case there's no benefit from using runtime PM. But the support
could be added later on, but I think we'll need a way for the user space to
request resume latency somehow first. So I'm not requesting to remove
runtime PM support here.

> +	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);
> +	alvium_power_off(alvium);
> +	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_off(alvium);
> +	pm_runtime_set_suspended(dev);
> +}
> +
> +static const struct i2c_device_id alvium_id[] = {
> +	{"alvium-csi2", 0},

Spaces inside braces, please: { content }.

> +	{},
> +};
> +MODULE_DEVICE_TABLE(i2c, alvium_id);

Do you need this, or is the OF device ID enough? If not, this could be
removed.

> +
> +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,
> +	.id_table = alvium_id,
> +};
> +
> +module_i2c_driver(alvium_i2c_driver);
> +
> +MODULE_DESCRIPTION("Allied Vision's Alvium Camera Driver");
> +MODULE_AUTHOR("Tommaso Merciai <tomm.merciai@gmail.com>");
> +MODULE_AUTHOR("Martin Hecht <martin.hecht@avnet.eu>");
> +MODULE_AUTHOR("Avnet Silica Software & Services EMEA");
> +MODULE_LICENSE("GPL v2");

"GPL" --- the meaning is the same.

> diff --git a/drivers/media/i2c/alvium-csi2.h b/drivers/media/i2c/alvium-csi2.h
> new file mode 100644
> index 000000000000..fdfbdceda577
> --- /dev/null
> +++ b/drivers/media/i2c/alvium-csi2.h
> @@ -0,0 +1,474 @@
> +// 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_H_
> +#define ALVIUM_H_

Maybe "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_R				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
> +
> +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 {
> +	u8 rev_x:1;
> +	u8 rev_y:1;
> +	u8 int_autop:1;
> +	u8 black_lvl:1;
> +	u8 gain:1;
> +	u8 gamma:1;
> +	u8 contrast:1;
> +	u8 sat:1;
> +	u8 hue:1;
> +	u8 whiteb:1;
> +	u8 sharp:1;
> +	u8 auto_exp:1;
> +	u8 auto_gain:1;
> +	u8 auto_whiteb:1;
> +	u8 dev_temp:1;
> +	u8 acq_abort:1;
> +	u8 acq_fr:1;
> +	u8 fr_trigger:1;
> +	u8 exp_acq_line:1;
> +};
> +
> +struct alvium_avail_mipi_fmt {
> +	u8 yuv420_8_leg:1;
> +	u8 yuv420_8:1;
> +	u8 yuv420_10:1;
> +	u8 yuv420_8_csps:1;
> +	u8 yuv420_10_csps:1;
> +	u8 yuv422_8:1;
> +	u8 yuv422_10:1;
> +	u8 rgb888:1;
> +	u8 rgb666:1;
> +	u8 rgb565:1;
> +	u8 rgb555:1;
> +	u8 rgb444:1;
> +	u8 raw6:1;
> +	u8 raw7:1;
> +	u8 raw8:1;
> +	u8 raw10:1;
> +	u8 raw12:1;
> +	u8 raw14:1;
> +	u8 jpeg:1;

Could you add padding up to full u8, to ensure the alignment? My
understanding is not all ABIs on different archs may be aligned on this.

Same for the other such structs. It'd be nice to document which are unsed
on the firmware interface, too.

> +};
> +
> +struct alvium_avail_bayer {
> +	u8 mono:1;
> +	u8 gr:1;
> +	u8 rg:1;
> +	u8 gb:1;
> +	u8 bg:1;
> +};
> +
> +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 img_min_width;
> +	u32 img_max_width;
> +	u32 img_inc_width;
> +	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 min_gain;
> +	u64 max_gain;
> +	u64 inc_gain;
> +	u64 min_exp;
> +	u64 max_exp;
> +	u64 inc_exp;
> +	u64 min_rbalance;
> +	u64 max_rbalance;
> +	u64 inc_rbalance;
> +	u64 min_bbalance;
> +	u64 max_bbalance;
> +	u64 inc_bbalance;
> +	s32 min_hue;
> +	s32 max_hue;
> +	s32 inc_hue;
> +	u32 min_contrast;
> +	u32 max_contrast;
> +	u32 inc_contrast;
> +	u32 min_sat;
> +	u32 max_sat;
> +	u32 inc_sat;
> +	s32 min_black_lvl;
> +	s32 max_black_lvl;
> +	s32 inc_black_lvl;
> +	u64 min_gamma;
> +	u64 max_gamma;
> +	u64 inc_gamma;
> +	s32 min_sharp;
> +	s32 max_sharp;
> +	s32 inc_sharp;
> +
> +	u32 lp2hs_delay;
> +
> +	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:1;
> +	u8 apply_fiv:1;
> +};
> +
> +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_H_ */

-- 
Kind regards,

Sakari Ailus