diff mbox series

[v2,3/4] media: platform: Add Renesas RZ/G2L MIPI CSI-2 receiver driver

Message ID 20220905230406.30801-4-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive)
State New
Delegated to: Kieran Bingham
Headers show
Series Add driver for CSI2 and CRU modules found on Renesas RZ/G2L SoC | expand

Commit Message

Lad Prabhakar Sept. 5, 2022, 11:04 p.m. UTC
Add MIPI CSI-2 receiver driver for Renesas RZ/G2L. The MIPI
CSI-2 is part of the CRU module found on RZ/G2L family.

Based on a patch in the BSP by Hien Huynh
<hien.huynh.px@renesas.com>

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v1 -> v2
* Sorted Kconfig select
* Prefixed generic names for struct/variables with rzg2_csi2
* Dropped unnecessary checks for remote source
* Dropped exporting functions
* Moved lane validation to probe
* Split up rzg2l_csi2_dphy_setting() and rzg2l_csi2_mipi_link_setting()
* Used rzg2l_csi2_write() wherever possible
* Dropped stream_count/lock
* Used active subdev state instead of manually storing format in driver
* Implemented init_cfg/enum_frame_size/enum_mbus_code callbacks
* Dropped check for bus_type of remote source
* Switched to manually turn ON/OFF the clocks instead of pm_runtime so that
  the mipi/dhpy initialization happens as per the HW manual
* Hardcoded VC0 usage for now as streams API is under development

RFC v2 -> v1
* Fixed initialization sequence of DPHY and link
* Exported DPHY and link initialization functions so that the
  CRU core driver can initialize the CRU and CSI2 as per the HW manual.

RFC v1 -> RFC v2
* new patch (split up as new driver compared to v1)
---
 drivers/media/platform/renesas/Kconfig        |   1 +
 drivers/media/platform/renesas/Makefile       |   1 +
 .../media/platform/renesas/rzg2l-cru/Kconfig  |  17 +
 .../media/platform/renesas/rzg2l-cru/Makefile |   3 +
 .../platform/renesas/rzg2l-cru/rzg2l-csi2.c   | 761 ++++++++++++++++++
 .../platform/renesas/rzg2l-cru/rzg2l-csi2.h   |  46 ++
 6 files changed, 829 insertions(+)
 create mode 100644 drivers/media/platform/renesas/rzg2l-cru/Kconfig
 create mode 100644 drivers/media/platform/renesas/rzg2l-cru/Makefile
 create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
 create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h

Comments

Laurent Pinchart Sept. 21, 2022, 4:12 p.m. UTC | #1
Hi Prabhakar,

Thank you for the patch.

On Tue, Sep 06, 2022 at 12:04:05AM +0100, Lad Prabhakar wrote:
> Add MIPI CSI-2 receiver driver for Renesas RZ/G2L. The MIPI
> CSI-2 is part of the CRU module found on RZ/G2L family.
> 
> Based on a patch in the BSP by Hien Huynh
> <hien.huynh.px@renesas.com>
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v1 -> v2
> * Sorted Kconfig select
> * Prefixed generic names for struct/variables with rzg2_csi2
> * Dropped unnecessary checks for remote source
> * Dropped exporting functions
> * Moved lane validation to probe
> * Split up rzg2l_csi2_dphy_setting() and rzg2l_csi2_mipi_link_setting()
> * Used rzg2l_csi2_write() wherever possible
> * Dropped stream_count/lock
> * Used active subdev state instead of manually storing format in driver
> * Implemented init_cfg/enum_frame_size/enum_mbus_code callbacks
> * Dropped check for bus_type of remote source
> * Switched to manually turn ON/OFF the clocks instead of pm_runtime so that
>   the mipi/dhpy initialization happens as per the HW manual

That doesn't look right. The driver doesn't use runtime PM anymore, so
power domains may not be handled properly. What was the problem with
clock handling using runtime PM ?

> * Hardcoded VC0 usage for now as streams API is under development
> 
> RFC v2 -> v1
> * Fixed initialization sequence of DPHY and link
> * Exported DPHY and link initialization functions so that the
>   CRU core driver can initialize the CRU and CSI2 as per the HW manual.
> 
> RFC v1 -> RFC v2
> * new patch (split up as new driver compared to v1)
> ---
>  drivers/media/platform/renesas/Kconfig        |   1 +
>  drivers/media/platform/renesas/Makefile       |   1 +
>  .../media/platform/renesas/rzg2l-cru/Kconfig  |  17 +
>  .../media/platform/renesas/rzg2l-cru/Makefile |   3 +
>  .../platform/renesas/rzg2l-cru/rzg2l-csi2.c   | 761 ++++++++++++++++++
>  .../platform/renesas/rzg2l-cru/rzg2l-csi2.h   |  46 ++
>  6 files changed, 829 insertions(+)
>  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/Kconfig
>  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/Makefile
>  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
>  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h
> 
> diff --git a/drivers/media/platform/renesas/Kconfig b/drivers/media/platform/renesas/Kconfig
> index 9fd90672ea2d..ed788e991f74 100644
> --- a/drivers/media/platform/renesas/Kconfig
> +++ b/drivers/media/platform/renesas/Kconfig
> @@ -41,6 +41,7 @@ config VIDEO_SH_VOU
>  	  Support for the Video Output Unit (VOU) on SuperH SoCs.
>  
>  source "drivers/media/platform/renesas/rcar-vin/Kconfig"
> +source "drivers/media/platform/renesas/rzg2l-cru/Kconfig"
>  
>  # Mem2mem drivers
>  
> diff --git a/drivers/media/platform/renesas/Makefile b/drivers/media/platform/renesas/Makefile
> index 3ec226ef5fd2..55854e868887 100644
> --- a/drivers/media/platform/renesas/Makefile
> +++ b/drivers/media/platform/renesas/Makefile
> @@ -4,6 +4,7 @@
>  #
>  
>  obj-y += rcar-vin/
> +obj-y += rzg2l-cru/
>  obj-y += vsp1/
>  
>  obj-$(CONFIG_VIDEO_RCAR_DRIF) += rcar_drif.o
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/Kconfig b/drivers/media/platform/renesas/rzg2l-cru/Kconfig
> new file mode 100644
> index 000000000000..57c40bb499df
> --- /dev/null
> +++ b/drivers/media/platform/renesas/rzg2l-cru/Kconfig
> @@ -0,0 +1,17 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +config VIDEO_RZG2L_CSI2
> +	tristate "RZ/G2L MIPI CSI-2 Receiver"
> +	depends on ARCH_RENESAS || COMPILE_TEST
> +	depends on V4L_PLATFORM_DRIVERS
> +	depends on VIDEO_DEV && OF
> +	select MEDIA_CONTROLLER
> +	select RESET_CONTROLLER
> +	select V4L2_FWNODE
> +	select VIDEO_V4L2_SUBDEV_API
> +	help
> +	  Support for Renesas RZ/G2L (and alike SoC's) MIPI CSI-2
> +	  Receiver driver.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called rzg2l-csi2.
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/Makefile b/drivers/media/platform/renesas/rzg2l-cru/Makefile
> new file mode 100644
> index 000000000000..91ea97a944e6
> --- /dev/null
> +++ b/drivers/media/platform/renesas/rzg2l-cru/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-$(CONFIG_VIDEO_RZG2L_CSI2) += rzg2l-csi2.o
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> new file mode 100644
> index 000000000000..1f6838ed64fc
> --- /dev/null
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> @@ -0,0 +1,761 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for Renesas RZ/G2L MIPI CSI-2 Receiver
> + *
> + * Copyright (C) 2022 Renesas Electronics Corp.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_graph.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/reset.h>
> +#include <linux/sys_soc.h>
> +#include <linux/units.h>
> +
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-mc.h>
> +#include <media/v4l2-subdev.h>
> +
> +#include "rzg2l-csi2.h"
> +
> +/* LINK registers */
> +/* Module Configuration Register */
> +#define CSI2nMCG			0x0
> +#define CSI2nMCG_SDLN			GENMASK(11, 8)
> +
> +/* Module Control Register 0 */
> +#define CSI2nMCT0			0x10
> +#define CSI2nMCT0_VDLN(x)		((x) << 0)
> +
> +/* Module Control Register 2 */
> +#define CSI2nMCT2			0x18
> +#define CSI2nMCT2_FRRSKW(x)		((x) << 16)
> +#define CSI2nMCT2_FRRCLK(x)		((x) << 0)
> +
> +/* Module Control Register 3 */
> +#define CSI2nMCT3			0x1c
> +#define CSI2nMCT3_RXEN			BIT(0)
> +
> +/* Reset Control Register */
> +#define CSI2nRTCT			0x28
> +#define CSI2nRTCT_VSRST			BIT(0)
> +
> +/* Reset Status Register */
> +#define CSI2nRTST			0x2c
> +#define CSI2nRTST_VSRSTS		BIT(0)
> +
> +/* Receive Data Type Enable Low Register */
> +#define CSI2nDTEL			0x60
> +
> +/* Receive Data Type Enable High Register */
> +#define CSI2nDTEH			0x64
> +
> +/* Power Management Status Register */
> +#define CSI2nPMST			0x200
> +
> +/* Power Management Status Clear Register */
> +#define CSI2nPMSC			0x204
> +
> +/* DPHY registers */
> +/* D-PHY Control Register 0 */
> +#define CSIDPHYCTRL0			0x400
> +#define CSIDPHYCTRL0_EN_LDO1200		BIT(1)
> +#define CSIDPHYCTRL0_EN_BGR		BIT(0)
> +
> +/* D-PHY Timing Register 0 */
> +#define CSIDPHYTIM0			0x404
> +#define CSIDPHYTIM0_TCLK_MISS(x)	((x) << 24)
> +#define CSIDPHYTIM0_T_INIT(x)		((x) << 0)
> +
> +/* D-PHY Timing Register 1 */
> +#define CSIDPHYTIM1			0x408
> +#define CSIDPHYTIM1_THS_PREPARE(x)	((x) << 24)
> +#define CSIDPHYTIM1_TCLK_PREPARE(x)	((x) << 16)
> +#define CSIDPHYTIM1_THS_SETTLE(x)	((x) << 8)
> +#define CSIDPHYTIM1_TCLK_SETTLE(x)	((x) << 0)
> +
> +/* D-PHY Skew Adjustment Function */
> +#define CSIDPHYSKW0			0x460
> +#define CSIDPHYSKW0_UTIL_DL0_SKW_ADJ(x)	((x) & 0x3)
> +#define CSIDPHYSKW0_UTIL_DL1_SKW_ADJ(x)	(((x) & 0x3) << 4)
> +#define CSIDPHYSKW0_UTIL_DL2_SKW_ADJ(x)	(((x) & 0x3) << 8)
> +#define CSIDPHYSKW0_UTIL_DL3_SKW_ADJ(x)	(((x) & 0x3) << 12)
> +#define CSIDPHYSKW0_DEFAULT_SKW		CSIDPHYSKW0_UTIL_DL0_SKW_ADJ(1) | \
> +					CSIDPHYSKW0_UTIL_DL1_SKW_ADJ(1) | \
> +					CSIDPHYSKW0_UTIL_DL2_SKW_ADJ(1) | \
> +					CSIDPHYSKW0_UTIL_DL3_SKW_ADJ(1)
> +
> +#define VSRSTS_RETRIES			20
> +
> +#define RZG2L_CSI2_DEFAULT_WIDTH	800
> +#define RZG2L_CSI2_DEFAULT_HEIGHT	600
> +#define RZG2L_CSI2_DEFAULT_FMT		MEDIA_BUS_FMT_UYVY8_1X16
> +
> +struct rzg2l_csi2_timings {
> +	u32 t_init;
> +	u32 tclk_miss;
> +	u32 tclk_settle;
> +	u32 ths_settle;
> +	u32 tclk_prepare;
> +	u32 ths_prepare;
> +};
> +
> +enum rzg2l_dphy_timings {
> +	TRANSMISSION_RATE_80_MBPS = 0,
> +	TRANSMISSION_RATE_125_MBPS,
> +	TRANSMISSION_RATE_250_MBPS,
> +	TRANSMISSION_RATE_360_MBPS,
> +	TRANSMISSION_RATE_360_MBPS_PLUS,
> +};
> +
> +static const struct rzg2l_csi2_timings rzg2l_csi2_global_timings[] = {
> +	[TRANSMISSION_RATE_80_MBPS] = {
> +		.t_init = 79801,
> +		.tclk_miss = 4,
> +		.tclk_settle = 23,
> +		.ths_settle = 31,
> +		.tclk_prepare = 10,
> +		.ths_prepare = 19,
> +	},
> +	[TRANSMISSION_RATE_125_MBPS] = {
> +		.t_init = 79801,
> +		.tclk_miss = 4,
> +		.tclk_settle = 23,
> +		.ths_settle = 28,
> +		.tclk_prepare = 10,
> +		.ths_prepare = 19,
> +	},
> +	[TRANSMISSION_RATE_250_MBPS] = {
> +		.t_init = 79801,
> +		.tclk_miss = 4,
> +		.tclk_settle = 23,
> +		.ths_settle = 22,
> +		.tclk_prepare = 10,
> +		.ths_prepare = 16,
> +	},
> +	[TRANSMISSION_RATE_360_MBPS] = {
> +		.t_init = 79801,
> +		.tclk_miss = 4,
> +		.tclk_settle = 18,
> +		.ths_settle = 19,
> +		.tclk_prepare = 10,
> +		.ths_prepare = 10,
> +	},
> +	[TRANSMISSION_RATE_360_MBPS_PLUS] = {
> +		.t_init = 79801,
> +		.tclk_miss = 4,
> +		.tclk_settle = 18,
> +		.ths_settle = 18,
> +		.tclk_prepare = 10,
> +		.ths_prepare = 10,
> +	},
> +};
> +
> +struct rzg2l_csi2_format {
> +	u32 code;
> +	unsigned int datatype;
> +	unsigned int bpp;
> +};
> +
> +static const struct rzg2l_csi2_format rzg2l_csi2_formats[] = {
> +	{ .code = MEDIA_BUS_FMT_UYVY8_1X16,	.datatype = 0x1e, .bpp = 16 },
> +};
> +
> +static const struct rzg2l_csi2_format *rzg2l_csi2_code_to_fmt(unsigned int code)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(rzg2l_csi2_formats); i++)
> +		if (rzg2l_csi2_formats[i].code == code)
> +			return &rzg2l_csi2_formats[i];
> +
> +	return NULL;
> +}
> +
> +static inline struct rzg2l_csi2 *notifier_to_csi2(struct v4l2_async_notifier *n)
> +{
> +	return container_of(n, struct rzg2l_csi2, notifier);
> +}
> +
> +static u32 rzg2l_csi2_read(struct rzg2l_csi2 *csi2, unsigned int reg)
> +{
> +	return ioread32(csi2->base + reg);
> +}
> +
> +static void rzg2l_csi2_write(struct rzg2l_csi2 *csi2, unsigned int reg,
> +			     u32 data)
> +{
> +	iowrite32(data, csi2->base + reg);
> +}
> +
> +static void rzg2l_csi2_set(struct rzg2l_csi2 *csi2, unsigned int reg, u32 set)
> +{
> +	rzg2l_csi2_write(csi2, reg, rzg2l_csi2_read(csi2, reg) | set);
> +}
> +
> +static void rzg2l_csi2_clr(struct rzg2l_csi2 *csi2, unsigned int reg, u32 clr)
> +{
> +	rzg2l_csi2_write(csi2, reg, rzg2l_csi2_read(csi2, reg) & ~clr);
> +}
> +
> +static int rzg2l_csi2_calc_mbps(struct rzg2l_csi2 *csi2)
> +{
> +	struct v4l2_subdev *sd = csi2->remote_source;

Could you rename sd to source ? A local sd variable usually refers to
the subdev associated with the driver, it would be a bit confusing here.

> +	const struct rzg2l_csi2_format *format;
> +	const struct v4l2_mbus_framefmt *fmt;
> +	struct v4l2_subdev_state *state;
> +	struct v4l2_ctrl *ctrl;
> +	u64 mbps;
> +
> +	/* Read the pixel rate control from remote. */
> +	ctrl = v4l2_ctrl_find(sd->ctrl_handler, V4L2_CID_PIXEL_RATE);
> +	if (!ctrl) {
> +		dev_err(csi2->dev, "no pixel rate control in subdev %s\n",
> +			sd->name);
> +		return -EINVAL;
> +	}
> +
> +	state = v4l2_subdev_lock_and_get_active_state(&csi2->subdev);
> +	fmt = v4l2_subdev_get_pad_format(sd, state, RZG2L_CSI2_SINK);

This isn't right, the first argument is the remote subdev, and the state
is for the local subdev. I think you meant

	fmt = v4l2_subdev_get_pad_format(&csi2->subdev, state, RZG2L_CSI2_SINK);

> +	v4l2_subdev_unlock_state(state);
> +	format = rzg2l_csi2_code_to_fmt(fmt->code);
> +
> +	/*
> +	 * Calculate hsfreq in Mbps
> +	 * hsfreq = (pixel_rate * bits_per_sample) / number_of_lanes
> +	 */
> +	mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * format->bpp;
> +	do_div(mbps, csi2->lanes * 1000000);
> +
> +	return mbps;
> +}
> +
> +int rzg2l_csi2_cmn_rstb_deassert(struct rzg2l_csi2 *csi2)
> +{
> +	return reset_control_deassert(csi2->rstc);
> +}

This function is called by the CRU driver just before call .s_stream(1).
Let's drop it, and move the reset_control_deassert() call to
rzg2l_csi2_s_stream().

> +
> +/* -----------------------------------------------------------------------------
> + * DPHY setting
> + */
> +
> +static int rzg2l_csi2_dphy_disable(struct rzg2l_csi2 *csi2)
> +{
> +	int ret;
> +
> +	/* Reset the CRU (D-PHY) */
> +	ret = reset_control_assert(csi2->rstc);
> +	if (ret)
> +		return ret;
> +
> +	/* Stop the D-PHY clock */
> +	clk_disable_unprepare(csi2->sysclk);
> +
> +	/* Cancel the EN_LDO1200 register setting */
> +	rzg2l_csi2_clr(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_LDO1200);
> +
> +	/* Cancel the EN_BGR register setting */
> +	rzg2l_csi2_clr(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_BGR);
> +
> +	return 0;
> +}
> +
> +static int rzg2l_csi2_dphy_enable(struct rzg2l_csi2 *csi2)
> +{
> +	const struct rzg2l_csi2_timings *dphy_timing;
> +	u32 dphytim0, dphytim1;
> +	int mbps;
> +	int ret;
> +
> +	mbps = rzg2l_csi2_calc_mbps(csi2);
> +	if (mbps < 0)
> +		return mbps;
> +
> +	csi2->hsfreq = mbps;
> +
> +	/* Set DPHY timing parameters */
> +	if (csi2->hsfreq <= 80)
> +		dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_80_MBPS];
> +	else if (csi2->hsfreq <= 125)
> +		dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_125_MBPS];
> +	else if (csi2->hsfreq <= 250)
> +		dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_250_MBPS];
> +	else if (csi2->hsfreq <= 360)
> +		dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_360_MBPS];
> +	else if (csi2->hsfreq <= 1500)
> +		dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_360_MBPS_PLUS];
> +	else
> +		return -EINVAL;

Add a max_hsfreq field to rzg2l_csi2_timings, and write

	for (i = 0; i < ARRAY_SIZE(rzg2l_csi2_global_timings); ++i) {
		dphy_timing = &rzg2l_csi2_global_timings[i];

		if (csi2->hsfreq <= dphy_timing->max_hsfreq)
			break;
	}

You can then drop the rzg2l_dphy_timings enum.

> +
> +	/* Set D-PHY timing parameters */
> +	dphytim0 = CSIDPHYTIM0_TCLK_MISS(dphy_timing->tclk_miss) |
> +			CSIDPHYTIM0_T_INIT(dphy_timing->t_init);
> +	dphytim1 = CSIDPHYTIM1_THS_PREPARE(dphy_timing->ths_prepare) |
> +			CSIDPHYTIM1_TCLK_PREPARE(dphy_timing->tclk_prepare) |
> +			CSIDPHYTIM1_THS_SETTLE(dphy_timing->ths_settle) |
> +			CSIDPHYTIM1_TCLK_SETTLE(dphy_timing->tclk_settle);
> +	rzg2l_csi2_write(csi2, CSIDPHYTIM0, dphytim0);
> +	rzg2l_csi2_write(csi2, CSIDPHYTIM1, dphytim1);
> +
> +	/* Enable D-PHY power control 0 */
> +	rzg2l_csi2_write(csi2, CSIDPHYSKW0, CSIDPHYSKW0_DEFAULT_SKW);
> +
> +	/* Set the EN_BGR bit */
> +	rzg2l_csi2_set(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_BGR);
> +
> +	/* Delay 20us to be stable */
> +	usleep_range(20, 40);
> +
> +	/* Enable D-PHY power control 1 */
> +	rzg2l_csi2_set(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_LDO1200);
> +
> +	/* Delay 10us to be stable */
> +	usleep_range(10, 20);
> +
> +	/* Start supplying the internal clock for the D-PHY block */
> +	ret = clk_prepare_enable(csi2->sysclk);
> +	if (ret)
> +		rzg2l_csi2_dphy_disable(csi2);
> +
> +	return ret;
> +}
> +
> +int rzg2l_csi2_dphy_setting(struct rzg2l_csi2 *csi2, bool on)
> +{
> +	if (on)
> +		return rzg2l_csi2_dphy_enable(csi2);
> +
> +	return rzg2l_csi2_dphy_disable(csi2);
> +}

Does this work without exporting the symbol when the drivers are
compiled as modules ?

> +
> +static void rzg2l_csi2_mipi_link_enable(struct rzg2l_csi2 *csi2)
> +{
> +	unsigned long vclk_rate = clk_get_rate(csi2->vclk) / HZ_PER_MHZ;
> +	u32 frrskw, frrclk, frrskw_coeff, frrclk_coeff;
> +
> +	/* Select data lanes */
> +	rzg2l_csi2_write(csi2, CSI2nMCT0, CSI2nMCT0_VDLN(csi2->lanes));
> +
> +	frrskw_coeff = 3 * vclk_rate * 8;
> +	frrclk_coeff = frrskw_coeff / 2;
> +	frrskw = DIV_ROUND_UP(frrskw_coeff, csi2->hsfreq);
> +	frrclk = DIV_ROUND_UP(frrclk_coeff, csi2->hsfreq);
> +	rzg2l_csi2_write(csi2, CSI2nMCT2, CSI2nMCT2_FRRSKW(frrskw) |
> +			 CSI2nMCT2_FRRCLK(frrclk));
> +
> +	/*
> +	 * Select data type.
> +	 * FS, FE, LS, LE, Generic Short Packet Codes 1 to 8,
> +	 * Generic Long Packet Data Types 1 to 4 YUV422 8-bit,
> +	 * RGB565, RGB888, RAW8 to RAW20, User-defined 8-bit
> +	 * data types 1 to 8
> +	 */
> +	rzg2l_csi2_write(csi2, CSI2nDTEL, 0xf778ff0f);
> +	rzg2l_csi2_write(csi2, CSI2nDTEH, 0x00ffff1f);
> +
> +	/* Enable LINK reception */
> +	rzg2l_csi2_write(csi2, CSI2nMCT3, CSI2nMCT3_RXEN);
> +}
> +
> +static void rzg2l_csi2_mipi_link_disable(struct rzg2l_csi2 *csi2)
> +{
> +	unsigned int timeout = VSRSTS_RETRIES;
> +
> +	/* Stop LINK reception */
> +	rzg2l_csi2_clr(csi2, CSI2nMCT3, CSI2nMCT3_RXEN);
> +
> +	/* Request a software reset of the LINK Video Pixel Interface */
> +	rzg2l_csi2_write(csi2, CSI2nRTCT, CSI2nRTCT_VSRST);
> +
> +	/* Make sure CSI2nRTST.VSRSTS bit is cleared */
> +	while (timeout--) {
> +		if (!(rzg2l_csi2_read(csi2, CSI2nRTST) & CSI2nRTST_VSRSTS))
> +			break;
> +		usleep_range(100, 200);
> +	};
> +
> +	if (!timeout)
> +		dev_err(csi2->dev, "Clearing CSI2nRTST.VSRSTS timed out\n");
> +}
> +
> +void rzg2l_csi2_mipi_link_setting(struct rzg2l_csi2 *csi2, bool on)

Can this also be moved to rzg2l_csi2_s_stream() instead of being called
from the CRU driver ? I really dislike the manual calls between the two
drivers. This will mess up future reworks of the subdev infrastructure
:-( Please try to stick to the standard code flow as much as possible.

> +{
> +	if (on)
> +		rzg2l_csi2_mipi_link_enable(csi2);
> +	else
> +		rzg2l_csi2_mipi_link_disable(csi2);
> +}
> +
> +static int rzg2l_csi2_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> +	struct rzg2l_csi2 *csi2 = sd_to_csi2(sd);
> +
> +	return v4l2_subdev_call(csi2->remote_source, video, s_stream, enable);
> +}
> +
> +static int rzg2l_csi2_set_format(struct v4l2_subdev *sd,
> +				 struct v4l2_subdev_state *state,
> +				 struct v4l2_subdev_format *fmt)
> +{
> +	struct v4l2_mbus_framefmt *src_format;
> +	struct v4l2_mbus_framefmt *format;
> +
> +	if (fmt->pad > RZG2L_CSI2_SINK)
> +		return -EINVAL;

That's not right, set_format must not return an error on the source pad.
Have you run v4l2-compliance on this driver ? Please include the
v4l2-compliance report in the cover letter.

As the CSI-2 receiver has the same format on the input and output, you
can simply write

	src_format = v4l2_subdev_get_pad_format(sd, state, RZG2L_CSI2_SOURCE);

	if (fmt->pad == RZG2L_CSI2_SOURCE) {
		fmt->format = *src_format;
		return 0;
	}

here. I'd also rename format to sink_format and write

	sink_format = v4l2_subdev_get_pad_format(sd, state, RZG2L_CSI2_SINK);
	src_format = v4l2_subdev_get_pad_format(sd, state, RZG2L_CSI2_SOURCE);

at the beginning of the function.

> +
> +	format = v4l2_subdev_get_pad_format(sd, state, fmt->pad);
> +
> +	if (!rzg2l_csi2_code_to_fmt(fmt->format.code))
> +		format->code = rzg2l_csi2_formats[0].code;
> +	else
> +		format->code = fmt->format.code;
> +
> +	format->field = V4L2_FIELD_NONE;
> +	format->colorspace = V4L2_COLORSPACE_SRGB;

The CSI-2 receiver doesn't care about color spaces, so I would simply
copy all colorspace-related fields received from userspace (colorspace,
xfer_func, ycbcr_enc and quantization).

> +	format->width = fmt->format.width;
> +	format->height = fmt->format.height;
> +	fmt->format = *format;
> +
> +	/* propagate format to source pad */
> +	src_format = v4l2_subdev_get_pad_format(sd, state, RZG2L_CSI2_SOURCE_VC0);
> +	*src_format = *format;
> +
> +	return 0;
> +}
> +
> +static int rzg2l_csi2_init_config(struct v4l2_subdev *sd,
> +				  struct v4l2_subdev_state *sd_state)
> +{
> +	struct v4l2_subdev_format fmt = { 0 };
> +
> +	fmt.which = sd_state ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;

When using the subdev active state, the state pointer passed to this
function will never be null. That's fine, as rzg2l_csi2_set_format()
doesn't use the which field. I'd just drop this line.

Please set the pad to RZG2L_CSI2_SINK explicitly. It works as-is because
RZG2L_CSI2_SINK is equal to zero, and the structure is zeroed above, but
let's not depend on that.

> +	fmt.format.width = RZG2L_CSI2_DEFAULT_WIDTH;
> +	fmt.format.height = RZG2L_CSI2_DEFAULT_HEIGHT;
> +	fmt.format.field = V4L2_FIELD_NONE;
> +	fmt.format.code = RZG2L_CSI2_DEFAULT_FMT;
> +	fmt.format.colorspace = V4L2_COLORSPACE_SRGB;
> +
> +	return rzg2l_csi2_set_format(sd, sd_state, &fmt);
> +}
> +
> +static int rzg2l_csi2_enum_mbus_code(struct v4l2_subdev *sd,
> +				     struct v4l2_subdev_state *sd_state,
> +				     struct v4l2_subdev_mbus_code_enum *code)
> +{
> +	if (code->index >= ARRAY_SIZE(rzg2l_csi2_formats))
> +		return -EINVAL;
> +
> +	code->code = rzg2l_csi2_formats[code->index].code;
> +
> +	return 0;
> +}
> +
> +static int rzg2l_csi2_enum_frame_size(struct v4l2_subdev *sd,
> +				      struct v4l2_subdev_state *sd_state,
> +				      struct v4l2_subdev_frame_size_enum *fse)
> +{
> +	if (fse->index != 0)
> +		return -EINVAL;
> +
> +	fse->min_width = clamp_t(u32, 1, 320, 2800);
> +	fse->min_height = clamp_t(u32, 1, 240, 4095);
> +	fse->max_width = clamp_t(u32, -1, 320, 2800);
> +	fse->max_height = clamp_t(u32, -1, 240, 4095);

That's a weird way to write

	fse->min_width = 320;
	fse->min_height = 240;
	fse->max_width = 2800;
	fse->max_height = 4095;

Could you add macros for those values, the same way you already have
RZG2L_CSI2_DEFAULT_WIDTH and RZG2L_CSI2_DEFAULT_HEIGHT ?
rzg2l_csi2_set_format() should also clamp the width and height
accordingly.

> +
> +	return 0;
> +}
> +
> +static const struct v4l2_subdev_video_ops rzg2l_csi2_video_ops = {
> +	.s_stream = rzg2l_csi2_s_stream,
> +};
> +
> +static const struct v4l2_subdev_pad_ops rzg2l_csi2_pad_ops = {
> +	.enum_mbus_code = rzg2l_csi2_enum_mbus_code,
> +	.init_cfg = rzg2l_csi2_init_config,
> +	.enum_frame_size = rzg2l_csi2_enum_frame_size,
> +	.set_fmt = rzg2l_csi2_set_format,
> +	.get_fmt = v4l2_subdev_get_fmt,
> +};
> +
> +static const struct v4l2_subdev_ops rzg2l_csi2_subdev_ops = {
> +	.video	= &rzg2l_csi2_video_ops,
> +	.pad	= &rzg2l_csi2_pad_ops,
> +};
> +
> +/* -----------------------------------------------------------------------------
> + * Async handling and registration of subdevices and links.
> + */
> +
> +static int rzg2l_csi2_notify_bound(struct v4l2_async_notifier *notifier,
> +				   struct v4l2_subdev *subdev,
> +				   struct v4l2_async_subdev *asd)
> +{
> +	struct rzg2l_csi2 *csi2 = notifier_to_csi2(notifier);
> +
> +	csi2->remote_source = subdev;
> +
> +	dev_dbg(csi2->dev, "Bound subdev: %s pad\n", subdev->name);
> +
> +	return media_create_pad_link(&subdev->entity, RZG2L_CSI2_SINK,
> +				     &csi2->subdev.entity, 0,
> +				     MEDIA_LNK_FL_ENABLED |
> +				     MEDIA_LNK_FL_IMMUTABLE);
> +}
> +
> +static void rzg2l_csi2_notify_unbind(struct v4l2_async_notifier *notifier,
> +				     struct v4l2_subdev *subdev,
> +				     struct v4l2_async_subdev *asd)
> +{
> +	struct rzg2l_csi2 *csi2 = notifier_to_csi2(notifier);
> +
> +	csi2->remote_source = NULL;
> +
> +	dev_dbg(csi2->dev, "Unbind subdev %s\n", subdev->name);
> +}
> +
> +static const struct v4l2_async_notifier_operations rzg2l_csi2_notify_ops = {
> +	.bound = rzg2l_csi2_notify_bound,
> +	.unbind = rzg2l_csi2_notify_unbind,
> +};
> +
> +static int rzg2l_csi2_parse_v4l2(struct rzg2l_csi2 *csi2,
> +				 struct v4l2_fwnode_endpoint *vep)
> +{
> +	/* Only port 0 endpoint 0 is valid. */
> +	if (vep->base.port || vep->base.id)
> +		return -ENOTCONN;
> +
> +	csi2->lanes = vep->bus.mipi_csi2.num_data_lanes;
> +
> +	return 0;
> +}
> +
> +static int rzg2l_csi2_parse_dt(struct rzg2l_csi2 *csi2)
> +{
> +	struct v4l2_fwnode_endpoint v4l2_ep = {
> +		.bus_type = V4L2_MBUS_CSI2_DPHY
> +	};
> +	struct v4l2_async_subdev *asd;
> +	struct fwnode_handle *fwnode;
> +	struct fwnode_handle *ep;
> +	int ret;
> +
> +	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(csi2->dev), 0, 0, 0);
> +	if (!ep) {
> +		dev_err(csi2->dev, "Not connected to subdevice\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = v4l2_fwnode_endpoint_parse(ep, &v4l2_ep);
> +	if (ret) {
> +		dev_err(csi2->dev, "Could not parse v4l2 endpoint\n");
> +		fwnode_handle_put(ep);
> +		return -EINVAL;
> +	}
> +
> +	ret = rzg2l_csi2_parse_v4l2(csi2, &v4l2_ep);
> +	if (ret) {
> +		fwnode_handle_put(ep);
> +		return ret;
> +	}
> +
> +	fwnode = fwnode_graph_get_remote_endpoint(ep);
> +	fwnode_handle_put(ep);
> +
> +	dev_dbg(csi2->dev, "Found '%pOF'\n", to_of_node(fwnode));
> +
> +	v4l2_async_nf_init(&csi2->notifier);
> +	csi2->notifier.ops = &rzg2l_csi2_notify_ops;
> +
> +	asd = v4l2_async_nf_add_fwnode(&csi2->notifier, fwnode,
> +				       struct v4l2_async_subdev);
> +	fwnode_handle_put(fwnode);
> +	if (IS_ERR(asd))
> +		return PTR_ERR(asd);
> +
> +	ret = v4l2_async_subdev_nf_register(&csi2->subdev, &csi2->notifier);
> +	if (ret)
> +		v4l2_async_nf_cleanup(&csi2->notifier);
> +
> +	return ret;
> +}
> +
> +static int rzg2l_validate_csi2_lanes(struct rzg2l_csi2 *csi2)
> +{
> +	int ret = 0;
> +	int lanes;
> +
> +	if (csi2->lanes != 1 && csi2->lanes != 2 && csi2->lanes != 4) {
> +		dev_err(csi2->dev, "Unsupported number of data-lanes: %u\n",
> +			csi2->lanes);
> +		return -EINVAL;
> +	}
> +
> +	ret = clk_prepare_enable(csi2->pclk);
> +	if (ret)
> +		return ret;
> +
> +	/* Checking the maximum lanes support for CSI-2 module */
> +	lanes = (rzg2l_csi2_read(csi2, CSI2nMCG) & CSI2nMCG_SDLN) >> 8;
> +	if (lanes < csi2->lanes) {
> +		dev_err(csi2->dev,
> +			"Failed to support %d data lanes\n", csi2->lanes);
> +		ret = -EINVAL;
> +	}
> +
> +	clk_disable_unprepare(csi2->pclk);
> +
> +	return ret;
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * Platform Device Driver.
> + */
> +
> +static const struct media_entity_operations rzg2l_csi2_entity_ops = {
> +	.link_validate = v4l2_subdev_link_validate,
> +};
> +
> +static int rzg2l_csi2_probe(struct platform_device *pdev)
> +{
> +	struct rzg2l_csi2 *csi2;
> +	int ret;
> +
> +	csi2 = devm_kzalloc(&pdev->dev, sizeof(*csi2), GFP_KERNEL);
> +	if (!csi2)
> +		return -ENOMEM;
> +
> +	csi2->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(csi2->base))
> +		return PTR_ERR(csi2->base);
> +
> +	csi2->rstc = devm_reset_control_get(&pdev->dev, "cmn-rstb");
> +	if (IS_ERR(csi2->rstc))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(csi2->rstc),
> +				     "failed to get cpg cmn-rstb\n");
> +
> +	csi2->sysclk = devm_clk_get(&pdev->dev, "sysclk");
> +	if (IS_ERR(csi2->sysclk)) {
> +		dev_err(&pdev->dev, "Failed to get sysclk");
> +		return PTR_ERR(csi2->sysclk);
> +	}
> +
> +	csi2->vclk = devm_clk_get(&pdev->dev, "vclk");
> +	if (IS_ERR(csi2->vclk)) {
> +		dev_err(&pdev->dev, "Failed to get vclk");
> +		return PTR_ERR(csi2->vclk);
> +	}
> +
> +	csi2->pclk = devm_clk_get(&pdev->dev, "pclk");
> +	if (IS_ERR(csi2->pclk)) {
> +		dev_err(&pdev->dev, "Failed to get pclk");
> +		return PTR_ERR(csi2->pclk);
> +	}

Could devm_clk_bulk_get() help ?

> +
> +	csi2->dev = &pdev->dev;
> +
> +	platform_set_drvdata(pdev, csi2);
> +
> +	ret = rzg2l_csi2_parse_dt(csi2);
> +	if (ret)
> +		return ret;
> +
> +	ret = rzg2l_validate_csi2_lanes(csi2);
> +	if (ret)
> +		return ret;
> +
> +	csi2->subdev.dev = &pdev->dev;
> +	v4l2_subdev_init(&csi2->subdev, &rzg2l_csi2_subdev_ops);
> +	v4l2_set_subdevdata(&csi2->subdev, &pdev->dev);
> +	snprintf(csi2->subdev.name, sizeof(csi2->subdev.name),
> +		 "csi-%s", dev_name(&pdev->dev));
> +	csi2->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> +
> +	csi2->subdev.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
> +	csi2->subdev.entity.ops = &rzg2l_csi2_entity_ops;
> +
> +	csi2->pads[RZG2L_CSI2_SINK].flags = MEDIA_PAD_FL_SINK;
> +	/*
> +	 * TODO: RZ/G2L CSI2 supports 4 virtual channels, as virtual
> +	 * channels should be implemented by streams API which is under
> +	 * development lets hardcode to VC0 for now.
> +	 */
> +	csi2->pads[RZG2L_CSI2_SOURCE_VC0].flags = MEDIA_PAD_FL_SOURCE;
> +	ret = media_entity_pads_init(&csi2->subdev.entity, 2, csi2->pads);
> +	if (ret)
> +		goto error_async;
> +
> +	pm_runtime_enable(&pdev->dev);
> +
> +	ret = v4l2_subdev_init_finalize(&csi2->subdev);
> +	if (ret < 0)
> +		goto error_pm;
> +
> +	/* enable pclk for register access */
> +	ret = clk_prepare_enable(csi2->pclk);
> +	if (ret)
> +		goto error_pm;

Keeping the clock always on isn't great from a power management point of
view. Runtime PM would be much better.

> +
> +	ret = v4l2_async_register_subdev(&csi2->subdev);
> +	if (ret < 0)
> +		goto error_subdev;
> +
> +	dev_info(csi2->dev, "%d lanes found\n", csi2->lanes);
> +
> +	return 0;
> +
> +error_subdev:
> +	clk_disable_unprepare(csi2->pclk);
> +	v4l2_subdev_cleanup(&csi2->subdev);
> +error_pm:
> +	pm_runtime_disable(&pdev->dev);
> +error_async:
> +	v4l2_async_nf_unregister(&csi2->notifier);
> +	v4l2_async_nf_cleanup(&csi2->notifier);
> +
> +	return ret;
> +}
> +
> +static const struct of_device_id rzg2l_csi2_of_table[] = {
> +	{ .compatible = "renesas,rzg2l-csi2", },
> +	{ /* sentinel */ }
> +};
> +
> +static int rzg2l_csi2_remove(struct platform_device *pdev)
> +{
> +	struct rzg2l_csi2 *csi2 = platform_get_drvdata(pdev);
> +
> +	v4l2_async_nf_unregister(&csi2->notifier);
> +	v4l2_async_nf_cleanup(&csi2->notifier);
> +	v4l2_async_unregister_subdev(&csi2->subdev);
> +	clk_disable_unprepare(csi2->pclk);
> +	v4l2_subdev_cleanup(&csi2->subdev);
> +
> +	pm_runtime_disable(&pdev->dev);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver rzg2l_csi2_pdrv = {
> +	.remove	= rzg2l_csi2_remove,
> +	.probe	= rzg2l_csi2_probe,
> +	.driver	= {
> +		.name = "rzg2l-csi2",
> +		.of_match_table = rzg2l_csi2_of_table,
> +	},
> +};
> +
> +module_platform_driver(rzg2l_csi2_pdrv);
> +
> +MODULE_AUTHOR("Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>");
> +MODULE_DESCRIPTION("Renesas RZ/G2L MIPI CSI2 receiver driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h
> new file mode 100644
> index 000000000000..f85a1d44250f
> --- /dev/null
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h
> @@ -0,0 +1,46 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2022 Renesas Electronics Corp.
> + */
> +
> +#ifndef __RZG2L_CSI2__
> +#define __RZG2L_CSI2__
> +
> +enum rzg2l_csi2_pads {
> +	RZG2L_CSI2_SINK = 0,
> +	RZG2L_CSI2_SOURCE_VC0,
> +	RZG2L_CSI2_SOURCE_VC1,
> +	RZG2L_CSI2_SOURCE_VC2,
> +	RZG2L_CSI2_SOURCE_VC3,

Drop RZG2L_CSI2_SOURCE_VC[123] and rename RZG2L_CSI2_SOURCE_VC0 to
RZG2L_CSI2_SOURCE.

> +	NR_OF_RZG2L_CSI2_PAD,
> +};
> +
> +struct rzg2l_csi2 {
> +	struct device *dev;
> +	void __iomem *base;
> +	struct reset_control *rstc;
> +	struct clk *sysclk;
> +	struct clk *vclk;
> +	struct clk *pclk;
> +
> +	struct v4l2_subdev subdev;
> +	struct media_pad pads[NR_OF_RZG2L_CSI2_PAD];
> +
> +	struct v4l2_async_notifier notifier;
> +	struct v4l2_subdev *remote_source;
> +
> +	unsigned short lanes;
> +
> +	unsigned long hsfreq;
> +};
> +
> +static inline struct rzg2l_csi2 *sd_to_csi2(struct v4l2_subdev *sd)
> +{
> +	return container_of(sd, struct rzg2l_csi2, subdev);
> +}
> +
> +int rzg2l_csi2_cmn_rstb_deassert(struct rzg2l_csi2 *csi2);
> +int rzg2l_csi2_dphy_setting(struct rzg2l_csi2 *csi2, bool on);
> +void rzg2l_csi2_mipi_link_setting(struct rzg2l_csi2 *csi2, bool on);

Let's make the API as opaque as possible for the CRU driver. I'd move
the definition of the rzg2l_csi2 structure to the .c file, along with
the sd_to_csi2() function, and change these three functions to take a
v4l2_subdev pointer.

> +
> +#endif
Sakari Ailus Sept. 21, 2022, 9:35 p.m. UTC | #2
Hi Prabhakar,

Thanks for the set.

On Tue, Sep 06, 2022 at 12:04:05AM +0100, Lad Prabhakar wrote:
> Add MIPI CSI-2 receiver driver for Renesas RZ/G2L. The MIPI
> CSI-2 is part of the CRU module found on RZ/G2L family.
> 
> Based on a patch in the BSP by Hien Huynh
> <hien.huynh.px@renesas.com>
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v1 -> v2
> * Sorted Kconfig select
> * Prefixed generic names for struct/variables with rzg2_csi2
> * Dropped unnecessary checks for remote source
> * Dropped exporting functions
> * Moved lane validation to probe
> * Split up rzg2l_csi2_dphy_setting() and rzg2l_csi2_mipi_link_setting()
> * Used rzg2l_csi2_write() wherever possible
> * Dropped stream_count/lock
> * Used active subdev state instead of manually storing format in driver
> * Implemented init_cfg/enum_frame_size/enum_mbus_code callbacks
> * Dropped check for bus_type of remote source
> * Switched to manually turn ON/OFF the clocks instead of pm_runtime so that
>   the mipi/dhpy initialization happens as per the HW manual
> * Hardcoded VC0 usage for now as streams API is under development
> 
> RFC v2 -> v1
> * Fixed initialization sequence of DPHY and link
> * Exported DPHY and link initialization functions so that the
>   CRU core driver can initialize the CRU and CSI2 as per the HW manual.
> 
> RFC v1 -> RFC v2
> * new patch (split up as new driver compared to v1)
> ---
>  drivers/media/platform/renesas/Kconfig        |   1 +
>  drivers/media/platform/renesas/Makefile       |   1 +
>  .../media/platform/renesas/rzg2l-cru/Kconfig  |  17 +
>  .../media/platform/renesas/rzg2l-cru/Makefile |   3 +
>  .../platform/renesas/rzg2l-cru/rzg2l-csi2.c   | 761 ++++++++++++++++++
>  .../platform/renesas/rzg2l-cru/rzg2l-csi2.h   |  46 ++
>  6 files changed, 829 insertions(+)
>  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/Kconfig
>  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/Makefile
>  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
>  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h
> 
> diff --git a/drivers/media/platform/renesas/Kconfig b/drivers/media/platform/renesas/Kconfig
> index 9fd90672ea2d..ed788e991f74 100644
> --- a/drivers/media/platform/renesas/Kconfig
> +++ b/drivers/media/platform/renesas/Kconfig
> @@ -41,6 +41,7 @@ config VIDEO_SH_VOU
>  	  Support for the Video Output Unit (VOU) on SuperH SoCs.
>  
>  source "drivers/media/platform/renesas/rcar-vin/Kconfig"
> +source "drivers/media/platform/renesas/rzg2l-cru/Kconfig"
>  
>  # Mem2mem drivers
>  
> diff --git a/drivers/media/platform/renesas/Makefile b/drivers/media/platform/renesas/Makefile
> index 3ec226ef5fd2..55854e868887 100644
> --- a/drivers/media/platform/renesas/Makefile
> +++ b/drivers/media/platform/renesas/Makefile
> @@ -4,6 +4,7 @@
>  #
>  
>  obj-y += rcar-vin/
> +obj-y += rzg2l-cru/
>  obj-y += vsp1/
>  
>  obj-$(CONFIG_VIDEO_RCAR_DRIF) += rcar_drif.o
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/Kconfig b/drivers/media/platform/renesas/rzg2l-cru/Kconfig
> new file mode 100644
> index 000000000000..57c40bb499df
> --- /dev/null
> +++ b/drivers/media/platform/renesas/rzg2l-cru/Kconfig
> @@ -0,0 +1,17 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +config VIDEO_RZG2L_CSI2
> +	tristate "RZ/G2L MIPI CSI-2 Receiver"
> +	depends on ARCH_RENESAS || COMPILE_TEST
> +	depends on V4L_PLATFORM_DRIVERS
> +	depends on VIDEO_DEV && OF
> +	select MEDIA_CONTROLLER
> +	select RESET_CONTROLLER
> +	select V4L2_FWNODE
> +	select VIDEO_V4L2_SUBDEV_API
> +	help
> +	  Support for Renesas RZ/G2L (and alike SoC's) MIPI CSI-2
> +	  Receiver driver.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called rzg2l-csi2.
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/Makefile b/drivers/media/platform/renesas/rzg2l-cru/Makefile
> new file mode 100644
> index 000000000000..91ea97a944e6
> --- /dev/null
> +++ b/drivers/media/platform/renesas/rzg2l-cru/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-$(CONFIG_VIDEO_RZG2L_CSI2) += rzg2l-csi2.o
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> new file mode 100644
> index 000000000000..1f6838ed64fc
> --- /dev/null
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> @@ -0,0 +1,761 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for Renesas RZ/G2L MIPI CSI-2 Receiver
> + *
> + * Copyright (C) 2022 Renesas Electronics Corp.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_graph.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/reset.h>
> +#include <linux/sys_soc.h>
> +#include <linux/units.h>
> +
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-mc.h>
> +#include <media/v4l2-subdev.h>
> +
> +#include "rzg2l-csi2.h"
> +
> +/* LINK registers */
> +/* Module Configuration Register */
> +#define CSI2nMCG			0x0
> +#define CSI2nMCG_SDLN			GENMASK(11, 8)
> +
> +/* Module Control Register 0 */
> +#define CSI2nMCT0			0x10
> +#define CSI2nMCT0_VDLN(x)		((x) << 0)
> +
> +/* Module Control Register 2 */
> +#define CSI2nMCT2			0x18
> +#define CSI2nMCT2_FRRSKW(x)		((x) << 16)
> +#define CSI2nMCT2_FRRCLK(x)		((x) << 0)
> +
> +/* Module Control Register 3 */
> +#define CSI2nMCT3			0x1c
> +#define CSI2nMCT3_RXEN			BIT(0)
> +
> +/* Reset Control Register */
> +#define CSI2nRTCT			0x28
> +#define CSI2nRTCT_VSRST			BIT(0)
> +
> +/* Reset Status Register */
> +#define CSI2nRTST			0x2c
> +#define CSI2nRTST_VSRSTS		BIT(0)
> +
> +/* Receive Data Type Enable Low Register */
> +#define CSI2nDTEL			0x60
> +
> +/* Receive Data Type Enable High Register */
> +#define CSI2nDTEH			0x64
> +
> +/* Power Management Status Register */
> +#define CSI2nPMST			0x200
> +
> +/* Power Management Status Clear Register */
> +#define CSI2nPMSC			0x204
> +
> +/* DPHY registers */
> +/* D-PHY Control Register 0 */
> +#define CSIDPHYCTRL0			0x400
> +#define CSIDPHYCTRL0_EN_LDO1200		BIT(1)
> +#define CSIDPHYCTRL0_EN_BGR		BIT(0)
> +
> +/* D-PHY Timing Register 0 */
> +#define CSIDPHYTIM0			0x404
> +#define CSIDPHYTIM0_TCLK_MISS(x)	((x) << 24)
> +#define CSIDPHYTIM0_T_INIT(x)		((x) << 0)
> +
> +/* D-PHY Timing Register 1 */
> +#define CSIDPHYTIM1			0x408
> +#define CSIDPHYTIM1_THS_PREPARE(x)	((x) << 24)
> +#define CSIDPHYTIM1_TCLK_PREPARE(x)	((x) << 16)
> +#define CSIDPHYTIM1_THS_SETTLE(x)	((x) << 8)
> +#define CSIDPHYTIM1_TCLK_SETTLE(x)	((x) << 0)
> +
> +/* D-PHY Skew Adjustment Function */
> +#define CSIDPHYSKW0			0x460
> +#define CSIDPHYSKW0_UTIL_DL0_SKW_ADJ(x)	((x) & 0x3)
> +#define CSIDPHYSKW0_UTIL_DL1_SKW_ADJ(x)	(((x) & 0x3) << 4)
> +#define CSIDPHYSKW0_UTIL_DL2_SKW_ADJ(x)	(((x) & 0x3) << 8)
> +#define CSIDPHYSKW0_UTIL_DL3_SKW_ADJ(x)	(((x) & 0x3) << 12)
> +#define CSIDPHYSKW0_DEFAULT_SKW		CSIDPHYSKW0_UTIL_DL0_SKW_ADJ(1) | \
> +					CSIDPHYSKW0_UTIL_DL1_SKW_ADJ(1) | \
> +					CSIDPHYSKW0_UTIL_DL2_SKW_ADJ(1) | \
> +					CSIDPHYSKW0_UTIL_DL3_SKW_ADJ(1)
> +
> +#define VSRSTS_RETRIES			20
> +
> +#define RZG2L_CSI2_DEFAULT_WIDTH	800
> +#define RZG2L_CSI2_DEFAULT_HEIGHT	600
> +#define RZG2L_CSI2_DEFAULT_FMT		MEDIA_BUS_FMT_UYVY8_1X16
> +
> +struct rzg2l_csi2_timings {
> +	u32 t_init;
> +	u32 tclk_miss;
> +	u32 tclk_settle;
> +	u32 ths_settle;
> +	u32 tclk_prepare;
> +	u32 ths_prepare;
> +};
> +
> +enum rzg2l_dphy_timings {
> +	TRANSMISSION_RATE_80_MBPS = 0,
> +	TRANSMISSION_RATE_125_MBPS,
> +	TRANSMISSION_RATE_250_MBPS,
> +	TRANSMISSION_RATE_360_MBPS,
> +	TRANSMISSION_RATE_360_MBPS_PLUS,
> +};
> +
> +static const struct rzg2l_csi2_timings rzg2l_csi2_global_timings[] = {
> +	[TRANSMISSION_RATE_80_MBPS] = {
> +		.t_init = 79801,
> +		.tclk_miss = 4,
> +		.tclk_settle = 23,
> +		.ths_settle = 31,
> +		.tclk_prepare = 10,
> +		.ths_prepare = 19,
> +	},
> +	[TRANSMISSION_RATE_125_MBPS] = {
> +		.t_init = 79801,
> +		.tclk_miss = 4,
> +		.tclk_settle = 23,
> +		.ths_settle = 28,
> +		.tclk_prepare = 10,
> +		.ths_prepare = 19,
> +	},
> +	[TRANSMISSION_RATE_250_MBPS] = {
> +		.t_init = 79801,
> +		.tclk_miss = 4,
> +		.tclk_settle = 23,
> +		.ths_settle = 22,
> +		.tclk_prepare = 10,
> +		.ths_prepare = 16,
> +	},
> +	[TRANSMISSION_RATE_360_MBPS] = {
> +		.t_init = 79801,
> +		.tclk_miss = 4,
> +		.tclk_settle = 18,
> +		.ths_settle = 19,
> +		.tclk_prepare = 10,
> +		.ths_prepare = 10,
> +	},
> +	[TRANSMISSION_RATE_360_MBPS_PLUS] = {
> +		.t_init = 79801,
> +		.tclk_miss = 4,
> +		.tclk_settle = 18,
> +		.ths_settle = 18,
> +		.tclk_prepare = 10,
> +		.ths_prepare = 10,
> +	},
> +};
> +
> +struct rzg2l_csi2_format {
> +	u32 code;
> +	unsigned int datatype;
> +	unsigned int bpp;
> +};
> +
> +static const struct rzg2l_csi2_format rzg2l_csi2_formats[] = {
> +	{ .code = MEDIA_BUS_FMT_UYVY8_1X16,	.datatype = 0x1e, .bpp = 16 },
> +};
> +
> +static const struct rzg2l_csi2_format *rzg2l_csi2_code_to_fmt(unsigned int code)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(rzg2l_csi2_formats); i++)
> +		if (rzg2l_csi2_formats[i].code == code)
> +			return &rzg2l_csi2_formats[i];
> +
> +	return NULL;
> +}
> +
> +static inline struct rzg2l_csi2 *notifier_to_csi2(struct v4l2_async_notifier *n)
> +{
> +	return container_of(n, struct rzg2l_csi2, notifier);
> +}
> +
> +static u32 rzg2l_csi2_read(struct rzg2l_csi2 *csi2, unsigned int reg)
> +{
> +	return ioread32(csi2->base + reg);
> +}
> +
> +static void rzg2l_csi2_write(struct rzg2l_csi2 *csi2, unsigned int reg,
> +			     u32 data)
> +{
> +	iowrite32(data, csi2->base + reg);
> +}
> +
> +static void rzg2l_csi2_set(struct rzg2l_csi2 *csi2, unsigned int reg, u32 set)
> +{
> +	rzg2l_csi2_write(csi2, reg, rzg2l_csi2_read(csi2, reg) | set);
> +}
> +
> +static void rzg2l_csi2_clr(struct rzg2l_csi2 *csi2, unsigned int reg, u32 clr)
> +{
> +	rzg2l_csi2_write(csi2, reg, rzg2l_csi2_read(csi2, reg) & ~clr);
> +}
> +
> +static int rzg2l_csi2_calc_mbps(struct rzg2l_csi2 *csi2)
> +{
> +	struct v4l2_subdev *sd = csi2->remote_source;
> +	const struct rzg2l_csi2_format *format;
> +	const struct v4l2_mbus_framefmt *fmt;
> +	struct v4l2_subdev_state *state;
> +	struct v4l2_ctrl *ctrl;
> +	u64 mbps;
> +
> +	/* Read the pixel rate control from remote. */
> +	ctrl = v4l2_ctrl_find(sd->ctrl_handler, V4L2_CID_PIXEL_RATE);
> +	if (!ctrl) {
> +		dev_err(csi2->dev, "no pixel rate control in subdev %s\n",
> +			sd->name);
> +		return -EINVAL;
> +	}
> +
> +	state = v4l2_subdev_lock_and_get_active_state(&csi2->subdev);
> +	fmt = v4l2_subdev_get_pad_format(sd, state, RZG2L_CSI2_SINK);
> +	v4l2_subdev_unlock_state(state);
> +	format = rzg2l_csi2_code_to_fmt(fmt->code);

You shouldn't access fmt once you've unlocked subdev state.

> +
> +	/*
> +	 * Calculate hsfreq in Mbps
> +	 * hsfreq = (pixel_rate * bits_per_sample) / number_of_lanes
> +	 */
> +	mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * format->bpp;
> +	do_div(mbps, csi2->lanes * 1000000);
> +
> +	return mbps;
> +}
> +
> +int rzg2l_csi2_cmn_rstb_deassert(struct rzg2l_csi2 *csi2)
> +{
> +	return reset_control_deassert(csi2->rstc);
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * DPHY setting
> + */
> +
> +static int rzg2l_csi2_dphy_disable(struct rzg2l_csi2 *csi2)
> +{
> +	int ret;
> +
> +	/* Reset the CRU (D-PHY) */
> +	ret = reset_control_assert(csi2->rstc);
> +	if (ret)
> +		return ret;
> +
> +	/* Stop the D-PHY clock */
> +	clk_disable_unprepare(csi2->sysclk);
> +
> +	/* Cancel the EN_LDO1200 register setting */
> +	rzg2l_csi2_clr(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_LDO1200);
> +
> +	/* Cancel the EN_BGR register setting */
> +	rzg2l_csi2_clr(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_BGR);
> +
> +	return 0;
> +}
> +
> +static int rzg2l_csi2_dphy_enable(struct rzg2l_csi2 *csi2)
> +{
> +	const struct rzg2l_csi2_timings *dphy_timing;
> +	u32 dphytim0, dphytim1;
> +	int mbps;
> +	int ret;
> +
> +	mbps = rzg2l_csi2_calc_mbps(csi2);
> +	if (mbps < 0)
> +		return mbps;
> +
> +	csi2->hsfreq = mbps;
> +
> +	/* Set DPHY timing parameters */
> +	if (csi2->hsfreq <= 80)
> +		dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_80_MBPS];
> +	else if (csi2->hsfreq <= 125)
> +		dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_125_MBPS];
> +	else if (csi2->hsfreq <= 250)
> +		dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_250_MBPS];
> +	else if (csi2->hsfreq <= 360)
> +		dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_360_MBPS];
> +	else if (csi2->hsfreq <= 1500)
> +		dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_360_MBPS_PLUS];
> +	else
> +		return -EINVAL;
> +
> +	/* Set D-PHY timing parameters */
> +	dphytim0 = CSIDPHYTIM0_TCLK_MISS(dphy_timing->tclk_miss) |
> +			CSIDPHYTIM0_T_INIT(dphy_timing->t_init);
> +	dphytim1 = CSIDPHYTIM1_THS_PREPARE(dphy_timing->ths_prepare) |
> +			CSIDPHYTIM1_TCLK_PREPARE(dphy_timing->tclk_prepare) |
> +			CSIDPHYTIM1_THS_SETTLE(dphy_timing->ths_settle) |
> +			CSIDPHYTIM1_TCLK_SETTLE(dphy_timing->tclk_settle);
> +	rzg2l_csi2_write(csi2, CSIDPHYTIM0, dphytim0);
> +	rzg2l_csi2_write(csi2, CSIDPHYTIM1, dphytim1);
> +
> +	/* Enable D-PHY power control 0 */
> +	rzg2l_csi2_write(csi2, CSIDPHYSKW0, CSIDPHYSKW0_DEFAULT_SKW);
> +
> +	/* Set the EN_BGR bit */
> +	rzg2l_csi2_set(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_BGR);
> +
> +	/* Delay 20us to be stable */
> +	usleep_range(20, 40);
> +
> +	/* Enable D-PHY power control 1 */
> +	rzg2l_csi2_set(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_LDO1200);
> +
> +	/* Delay 10us to be stable */
> +	usleep_range(10, 20);
> +
> +	/* Start supplying the internal clock for the D-PHY block */
> +	ret = clk_prepare_enable(csi2->sysclk);
> +	if (ret)
> +		rzg2l_csi2_dphy_disable(csi2);
> +
> +	return ret;
> +}
> +
> +int rzg2l_csi2_dphy_setting(struct rzg2l_csi2 *csi2, bool on)
> +{
> +	if (on)
> +		return rzg2l_csi2_dphy_enable(csi2);
> +
> +	return rzg2l_csi2_dphy_disable(csi2);
> +}
> +
> +static void rzg2l_csi2_mipi_link_enable(struct rzg2l_csi2 *csi2)
> +{
> +	unsigned long vclk_rate = clk_get_rate(csi2->vclk) / HZ_PER_MHZ;
> +	u32 frrskw, frrclk, frrskw_coeff, frrclk_coeff;
> +
> +	/* Select data lanes */
> +	rzg2l_csi2_write(csi2, CSI2nMCT0, CSI2nMCT0_VDLN(csi2->lanes));
> +
> +	frrskw_coeff = 3 * vclk_rate * 8;
> +	frrclk_coeff = frrskw_coeff / 2;
> +	frrskw = DIV_ROUND_UP(frrskw_coeff, csi2->hsfreq);
> +	frrclk = DIV_ROUND_UP(frrclk_coeff, csi2->hsfreq);
> +	rzg2l_csi2_write(csi2, CSI2nMCT2, CSI2nMCT2_FRRSKW(frrskw) |
> +			 CSI2nMCT2_FRRCLK(frrclk));
> +
> +	/*
> +	 * Select data type.
> +	 * FS, FE, LS, LE, Generic Short Packet Codes 1 to 8,
> +	 * Generic Long Packet Data Types 1 to 4 YUV422 8-bit,
> +	 * RGB565, RGB888, RAW8 to RAW20, User-defined 8-bit
> +	 * data types 1 to 8
> +	 */
> +	rzg2l_csi2_write(csi2, CSI2nDTEL, 0xf778ff0f);
> +	rzg2l_csi2_write(csi2, CSI2nDTEH, 0x00ffff1f);
> +
> +	/* Enable LINK reception */
> +	rzg2l_csi2_write(csi2, CSI2nMCT3, CSI2nMCT3_RXEN);
> +}
> +
> +static void rzg2l_csi2_mipi_link_disable(struct rzg2l_csi2 *csi2)
> +{
> +	unsigned int timeout = VSRSTS_RETRIES;
> +
> +	/* Stop LINK reception */
> +	rzg2l_csi2_clr(csi2, CSI2nMCT3, CSI2nMCT3_RXEN);
> +
> +	/* Request a software reset of the LINK Video Pixel Interface */
> +	rzg2l_csi2_write(csi2, CSI2nRTCT, CSI2nRTCT_VSRST);
> +
> +	/* Make sure CSI2nRTST.VSRSTS bit is cleared */
> +	while (timeout--) {
> +		if (!(rzg2l_csi2_read(csi2, CSI2nRTST) & CSI2nRTST_VSRSTS))
> +			break;
> +		usleep_range(100, 200);
> +	};
> +
> +	if (!timeout)
> +		dev_err(csi2->dev, "Clearing CSI2nRTST.VSRSTS timed out\n");
> +}
> +
> +void rzg2l_csi2_mipi_link_setting(struct rzg2l_csi2 *csi2, bool on)
> +{
> +	if (on)
> +		rzg2l_csi2_mipi_link_enable(csi2);
> +	else
> +		rzg2l_csi2_mipi_link_disable(csi2);
> +}
> +
> +static int rzg2l_csi2_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> +	struct rzg2l_csi2 *csi2 = sd_to_csi2(sd);
> +
> +	return v4l2_subdev_call(csi2->remote_source, video, s_stream, enable);
> +}
> +
> +static int rzg2l_csi2_set_format(struct v4l2_subdev *sd,
> +				 struct v4l2_subdev_state *state,
> +				 struct v4l2_subdev_format *fmt)
> +{
> +	struct v4l2_mbus_framefmt *src_format;
> +	struct v4l2_mbus_framefmt *format;
> +
> +	if (fmt->pad > RZG2L_CSI2_SINK)
> +		return -EINVAL;
> +
> +	format = v4l2_subdev_get_pad_format(sd, state, fmt->pad);
> +
> +	if (!rzg2l_csi2_code_to_fmt(fmt->format.code))
> +		format->code = rzg2l_csi2_formats[0].code;
> +	else
> +		format->code = fmt->format.code;
> +
> +	format->field = V4L2_FIELD_NONE;
> +	format->colorspace = V4L2_COLORSPACE_SRGB;
> +	format->width = fmt->format.width;
> +	format->height = fmt->format.height;
> +	fmt->format = *format;
> +
> +	/* propagate format to source pad */
> +	src_format = v4l2_subdev_get_pad_format(sd, state, RZG2L_CSI2_SOURCE_VC0);
> +	*src_format = *format;
> +
> +	return 0;
> +}
> +
> +static int rzg2l_csi2_init_config(struct v4l2_subdev *sd,
> +				  struct v4l2_subdev_state *sd_state)
> +{
> +	struct v4l2_subdev_format fmt = { 0 };

You could do this in variable initialisation.

> +
> +	fmt.which = sd_state ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
> +	fmt.format.width = RZG2L_CSI2_DEFAULT_WIDTH;
> +	fmt.format.height = RZG2L_CSI2_DEFAULT_HEIGHT;
> +	fmt.format.field = V4L2_FIELD_NONE;
> +	fmt.format.code = RZG2L_CSI2_DEFAULT_FMT;
> +	fmt.format.colorspace = V4L2_COLORSPACE_SRGB;
> +
> +	return rzg2l_csi2_set_format(sd, sd_state, &fmt);
> +}
> +
> +static int rzg2l_csi2_enum_mbus_code(struct v4l2_subdev *sd,
> +				     struct v4l2_subdev_state *sd_state,
> +				     struct v4l2_subdev_mbus_code_enum *code)
> +{
> +	if (code->index >= ARRAY_SIZE(rzg2l_csi2_formats))
> +		return -EINVAL;
> +
> +	code->code = rzg2l_csi2_formats[code->index].code;
> +
> +	return 0;
> +}
> +
> +static int rzg2l_csi2_enum_frame_size(struct v4l2_subdev *sd,
> +				      struct v4l2_subdev_state *sd_state,
> +				      struct v4l2_subdev_frame_size_enum *fse)
> +{
> +	if (fse->index != 0)
> +		return -EINVAL;
> +
> +	fse->min_width = clamp_t(u32, 1, 320, 2800);
> +	fse->min_height = clamp_t(u32, 1, 240, 4095);
> +	fse->max_width = clamp_t(u32, -1, 320, 2800);
> +	fse->max_height = clamp_t(u32, -1, 240, 4095);
> +
> +	return 0;
> +}
> +
> +static const struct v4l2_subdev_video_ops rzg2l_csi2_video_ops = {
> +	.s_stream = rzg2l_csi2_s_stream,
> +};
> +
> +static const struct v4l2_subdev_pad_ops rzg2l_csi2_pad_ops = {
> +	.enum_mbus_code = rzg2l_csi2_enum_mbus_code,
> +	.init_cfg = rzg2l_csi2_init_config,
> +	.enum_frame_size = rzg2l_csi2_enum_frame_size,
> +	.set_fmt = rzg2l_csi2_set_format,
> +	.get_fmt = v4l2_subdev_get_fmt,
> +};
> +
> +static const struct v4l2_subdev_ops rzg2l_csi2_subdev_ops = {
> +	.video	= &rzg2l_csi2_video_ops,
> +	.pad	= &rzg2l_csi2_pad_ops,
> +};
> +
> +/* -----------------------------------------------------------------------------
> + * Async handling and registration of subdevices and links.
> + */
> +
> +static int rzg2l_csi2_notify_bound(struct v4l2_async_notifier *notifier,
> +				   struct v4l2_subdev *subdev,
> +				   struct v4l2_async_subdev *asd)
> +{
> +	struct rzg2l_csi2 *csi2 = notifier_to_csi2(notifier);
> +
> +	csi2->remote_source = subdev;
> +
> +	dev_dbg(csi2->dev, "Bound subdev: %s pad\n", subdev->name);
> +
> +	return media_create_pad_link(&subdev->entity, RZG2L_CSI2_SINK,
> +				     &csi2->subdev.entity, 0,
> +				     MEDIA_LNK_FL_ENABLED |
> +				     MEDIA_LNK_FL_IMMUTABLE);
> +}
> +
> +static void rzg2l_csi2_notify_unbind(struct v4l2_async_notifier *notifier,
> +				     struct v4l2_subdev *subdev,
> +				     struct v4l2_async_subdev *asd)
> +{
> +	struct rzg2l_csi2 *csi2 = notifier_to_csi2(notifier);
> +
> +	csi2->remote_source = NULL;
> +
> +	dev_dbg(csi2->dev, "Unbind subdev %s\n", subdev->name);
> +}
> +
> +static const struct v4l2_async_notifier_operations rzg2l_csi2_notify_ops = {
> +	.bound = rzg2l_csi2_notify_bound,
> +	.unbind = rzg2l_csi2_notify_unbind,
> +};
> +
> +static int rzg2l_csi2_parse_v4l2(struct rzg2l_csi2 *csi2,
> +				 struct v4l2_fwnode_endpoint *vep)
> +{
> +	/* Only port 0 endpoint 0 is valid. */
> +	if (vep->base.port || vep->base.id)
> +		return -ENOTCONN;
> +
> +	csi2->lanes = vep->bus.mipi_csi2.num_data_lanes;
> +
> +	return 0;
> +}
> +
> +static int rzg2l_csi2_parse_dt(struct rzg2l_csi2 *csi2)
> +{
> +	struct v4l2_fwnode_endpoint v4l2_ep = {
> +		.bus_type = V4L2_MBUS_CSI2_DPHY
> +	};
> +	struct v4l2_async_subdev *asd;
> +	struct fwnode_handle *fwnode;
> +	struct fwnode_handle *ep;
> +	int ret;
> +
> +	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(csi2->dev), 0, 0, 0);
> +	if (!ep) {
> +		dev_err(csi2->dev, "Not connected to subdevice\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = v4l2_fwnode_endpoint_parse(ep, &v4l2_ep);
> +	if (ret) {
> +		dev_err(csi2->dev, "Could not parse v4l2 endpoint\n");
> +		fwnode_handle_put(ep);
> +		return -EINVAL;
> +	}
> +
> +	ret = rzg2l_csi2_parse_v4l2(csi2, &v4l2_ep);
> +	if (ret) {
> +		fwnode_handle_put(ep);
> +		return ret;
> +	}
> +
> +	fwnode = fwnode_graph_get_remote_endpoint(ep);
> +	fwnode_handle_put(ep);
> +
> +	dev_dbg(csi2->dev, "Found '%pOF'\n", to_of_node(fwnode));

You can enable debug message printing in v4l2 fwnode, and sysfs now has
some, too. I'd suggest dropping this line .

> +
> +	v4l2_async_nf_init(&csi2->notifier);
> +	csi2->notifier.ops = &rzg2l_csi2_notify_ops;
> +
> +	asd = v4l2_async_nf_add_fwnode(&csi2->notifier, fwnode,
> +				       struct v4l2_async_subdev);
> +	fwnode_handle_put(fwnode);
> +	if (IS_ERR(asd))
> +		return PTR_ERR(asd);
> +
> +	ret = v4l2_async_subdev_nf_register(&csi2->subdev, &csi2->notifier);
> +	if (ret)
> +		v4l2_async_nf_cleanup(&csi2->notifier);
> +
> +	return ret;
> +}
> +
> +static int rzg2l_validate_csi2_lanes(struct rzg2l_csi2 *csi2)
> +{
> +	int ret = 0;
> +	int lanes;
> +
> +	if (csi2->lanes != 1 && csi2->lanes != 2 && csi2->lanes != 4) {
> +		dev_err(csi2->dev, "Unsupported number of data-lanes: %u\n",
> +			csi2->lanes);
> +		return -EINVAL;
> +	}
> +
> +	ret = clk_prepare_enable(csi2->pclk);
> +	if (ret)
> +		return ret;
> +
> +	/* Checking the maximum lanes support for CSI-2 module */
> +	lanes = (rzg2l_csi2_read(csi2, CSI2nMCG) & CSI2nMCG_SDLN) >> 8;
> +	if (lanes < csi2->lanes) {
> +		dev_err(csi2->dev,
> +			"Failed to support %d data lanes\n", csi2->lanes);
> +		ret = -EINVAL;
> +	}
> +
> +	clk_disable_unprepare(csi2->pclk);
> +
> +	return ret;
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * Platform Device Driver.
> + */
> +
> +static const struct media_entity_operations rzg2l_csi2_entity_ops = {
> +	.link_validate = v4l2_subdev_link_validate,
> +};
> +
> +static int rzg2l_csi2_probe(struct platform_device *pdev)
> +{
> +	struct rzg2l_csi2 *csi2;
> +	int ret;
> +
> +	csi2 = devm_kzalloc(&pdev->dev, sizeof(*csi2), GFP_KERNEL);
> +	if (!csi2)
> +		return -ENOMEM;
> +
> +	csi2->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(csi2->base))
> +		return PTR_ERR(csi2->base);
> +
> +	csi2->rstc = devm_reset_control_get(&pdev->dev, "cmn-rstb");
> +	if (IS_ERR(csi2->rstc))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(csi2->rstc),
> +				     "failed to get cpg cmn-rstb\n");
> +
> +	csi2->sysclk = devm_clk_get(&pdev->dev, "sysclk");
> +	if (IS_ERR(csi2->sysclk)) {
> +		dev_err(&pdev->dev, "Failed to get sysclk");
> +		return PTR_ERR(csi2->sysclk);
> +	}
> +
> +	csi2->vclk = devm_clk_get(&pdev->dev, "vclk");
> +	if (IS_ERR(csi2->vclk)) {
> +		dev_err(&pdev->dev, "Failed to get vclk");
> +		return PTR_ERR(csi2->vclk);
> +	}
> +
> +	csi2->pclk = devm_clk_get(&pdev->dev, "pclk");
> +	if (IS_ERR(csi2->pclk)) {
> +		dev_err(&pdev->dev, "Failed to get pclk");
> +		return PTR_ERR(csi2->pclk);
> +	}
> +
> +	csi2->dev = &pdev->dev;
> +
> +	platform_set_drvdata(pdev, csi2);
> +
> +	ret = rzg2l_csi2_parse_dt(csi2);
> +	if (ret)
> +		return ret;
> +
> +	ret = rzg2l_validate_csi2_lanes(csi2);
> +	if (ret)
> +		return ret;
> +
> +	csi2->subdev.dev = &pdev->dev;
> +	v4l2_subdev_init(&csi2->subdev, &rzg2l_csi2_subdev_ops);
> +	v4l2_set_subdevdata(&csi2->subdev, &pdev->dev);
> +	snprintf(csi2->subdev.name, sizeof(csi2->subdev.name),
> +		 "csi-%s", dev_name(&pdev->dev));
> +	csi2->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> +
> +	csi2->subdev.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
> +	csi2->subdev.entity.ops = &rzg2l_csi2_entity_ops;
> +
> +	csi2->pads[RZG2L_CSI2_SINK].flags = MEDIA_PAD_FL_SINK;
> +	/*
> +	 * TODO: RZ/G2L CSI2 supports 4 virtual channels, as virtual
> +	 * channels should be implemented by streams API which is under
> +	 * development lets hardcode to VC0 for now.
> +	 */
> +	csi2->pads[RZG2L_CSI2_SOURCE_VC0].flags = MEDIA_PAD_FL_SOURCE;
> +	ret = media_entity_pads_init(&csi2->subdev.entity, 2, csi2->pads);
> +	if (ret)
> +		goto error_async;
> +
> +	pm_runtime_enable(&pdev->dev);
> +
> +	ret = v4l2_subdev_init_finalize(&csi2->subdev);
> +	if (ret < 0)
> +		goto error_pm;
> +
> +	/* enable pclk for register access */
> +	ret = clk_prepare_enable(csi2->pclk);
> +	if (ret)
> +		goto error_pm;
> +
> +	ret = v4l2_async_register_subdev(&csi2->subdev);
> +	if (ret < 0)
> +		goto error_subdev;
> +
> +	dev_info(csi2->dev, "%d lanes found\n", csi2->lanes);

This is also printed if you enable debug level prints in v4l2 fwnode.

> +
> +	return 0;
> +
> +error_subdev:
> +	clk_disable_unprepare(csi2->pclk);
> +	v4l2_subdev_cleanup(&csi2->subdev);
> +error_pm:
> +	pm_runtime_disable(&pdev->dev);
> +error_async:
> +	v4l2_async_nf_unregister(&csi2->notifier);
> +	v4l2_async_nf_cleanup(&csi2->notifier);
> +
> +	return ret;
> +}
> +
> +static const struct of_device_id rzg2l_csi2_of_table[] = {
> +	{ .compatible = "renesas,rzg2l-csi2", },
> +	{ /* sentinel */ }
> +};
> +
> +static int rzg2l_csi2_remove(struct platform_device *pdev)
> +{
> +	struct rzg2l_csi2 *csi2 = platform_get_drvdata(pdev);
> +
> +	v4l2_async_nf_unregister(&csi2->notifier);
> +	v4l2_async_nf_cleanup(&csi2->notifier);
> +	v4l2_async_unregister_subdev(&csi2->subdev);
> +	clk_disable_unprepare(csi2->pclk);
> +	v4l2_subdev_cleanup(&csi2->subdev);
> +
> +	pm_runtime_disable(&pdev->dev);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver rzg2l_csi2_pdrv = {
> +	.remove	= rzg2l_csi2_remove,
> +	.probe	= rzg2l_csi2_probe,
> +	.driver	= {
> +		.name = "rzg2l-csi2",
> +		.of_match_table = rzg2l_csi2_of_table,
> +	},
> +};
> +
> +module_platform_driver(rzg2l_csi2_pdrv);
> +
> +MODULE_AUTHOR("Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>");
> +MODULE_DESCRIPTION("Renesas RZ/G2L MIPI CSI2 receiver driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h
> new file mode 100644
> index 000000000000..f85a1d44250f
> --- /dev/null
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h
> @@ -0,0 +1,46 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2022 Renesas Electronics Corp.
> + */
> +
> +#ifndef __RZG2L_CSI2__
> +#define __RZG2L_CSI2__
> +
> +enum rzg2l_csi2_pads {
> +	RZG2L_CSI2_SINK = 0,
> +	RZG2L_CSI2_SOURCE_VC0,
> +	RZG2L_CSI2_SOURCE_VC1,
> +	RZG2L_CSI2_SOURCE_VC2,
> +	RZG2L_CSI2_SOURCE_VC3,
> +	NR_OF_RZG2L_CSI2_PAD,
> +};
> +
> +struct rzg2l_csi2 {
> +	struct device *dev;
> +	void __iomem *base;
> +	struct reset_control *rstc;
> +	struct clk *sysclk;
> +	struct clk *vclk;
> +	struct clk *pclk;
> +
> +	struct v4l2_subdev subdev;
> +	struct media_pad pads[NR_OF_RZG2L_CSI2_PAD];
> +
> +	struct v4l2_async_notifier notifier;
> +	struct v4l2_subdev *remote_source;
> +
> +	unsigned short lanes;
> +
> +	unsigned long hsfreq;
> +};
> +
> +static inline struct rzg2l_csi2 *sd_to_csi2(struct v4l2_subdev *sd)
> +{
> +	return container_of(sd, struct rzg2l_csi2, subdev);
> +}
> +
> +int rzg2l_csi2_cmn_rstb_deassert(struct rzg2l_csi2 *csi2);
> +int rzg2l_csi2_dphy_setting(struct rzg2l_csi2 *csi2, bool on);
> +void rzg2l_csi2_mipi_link_setting(struct rzg2l_csi2 *csi2, bool on);

Are these something that could be achieved using the standard interfaces,
as I believe the other drivers are doing? The pre_streamon and
post_streamon callbacks could be relevant for this.

> +
> +#endif
Prabhakar Sept. 22, 2022, 12:08 p.m. UTC | #3
Hi Laurent,

Thank you for the review.

On Wed, Sep 21, 2022 at 5:13 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Prabhakar,
>
> Thank you for the patch.
>
> On Tue, Sep 06, 2022 at 12:04:05AM +0100, Lad Prabhakar wrote:
> > Add MIPI CSI-2 receiver driver for Renesas RZ/G2L. The MIPI
> > CSI-2 is part of the CRU module found on RZ/G2L family.
> >
> > Based on a patch in the BSP by Hien Huynh
> > <hien.huynh.px@renesas.com>
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > v1 -> v2
> > * Sorted Kconfig select
> > * Prefixed generic names for struct/variables with rzg2_csi2
> > * Dropped unnecessary checks for remote source
> > * Dropped exporting functions
> > * Moved lane validation to probe
> > * Split up rzg2l_csi2_dphy_setting() and rzg2l_csi2_mipi_link_setting()
> > * Used rzg2l_csi2_write() wherever possible
> > * Dropped stream_count/lock
> > * Used active subdev state instead of manually storing format in driver
> > * Implemented init_cfg/enum_frame_size/enum_mbus_code callbacks
> > * Dropped check for bus_type of remote source
> > * Switched to manually turn ON/OFF the clocks instead of pm_runtime so that
> >   the mipi/dhpy initialization happens as per the HW manual
>
> That doesn't look right. The driver doesn't use runtime PM anymore, so
> power domains may not be handled properly. What was the problem with
> clock handling using runtime PM ?
>
If we use the runtime PM all the clocks will be turned ON when we call
pm_runtime_resume_and_get() which I dont want to. As per the "Starting
reception for MIPI CSI-2 Input" section 35.3.1 for example we first
need to turn ON all the clocks and later further down the line we need
to just turn OFF VCLK -> Enable Link -> turn ON VCLK. Due to such
cases I have switched to individual clock handling.

> > * Hardcoded VC0 usage for now as streams API is under development
> >
> > RFC v2 -> v1
> > * Fixed initialization sequence of DPHY and link
> > * Exported DPHY and link initialization functions so that the
> >   CRU core driver can initialize the CRU and CSI2 as per the HW manual.
> >
> > RFC v1 -> RFC v2
> > * new patch (split up as new driver compared to v1)
> > ---
> >  drivers/media/platform/renesas/Kconfig        |   1 +
> >  drivers/media/platform/renesas/Makefile       |   1 +
> >  .../media/platform/renesas/rzg2l-cru/Kconfig  |  17 +
> >  .../media/platform/renesas/rzg2l-cru/Makefile |   3 +
> >  .../platform/renesas/rzg2l-cru/rzg2l-csi2.c   | 761 ++++++++++++++++++
> >  .../platform/renesas/rzg2l-cru/rzg2l-csi2.h   |  46 ++
> >  6 files changed, 829 insertions(+)
> >  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/Kconfig
> >  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/Makefile
> >  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> >  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h
> >
> > diff --git a/drivers/media/platform/renesas/Kconfig b/drivers/media/platform/renesas/Kconfig
> > index 9fd90672ea2d..ed788e991f74 100644
> > --- a/drivers/media/platform/renesas/Kconfig
> > +++ b/drivers/media/platform/renesas/Kconfig
> > @@ -41,6 +41,7 @@ config VIDEO_SH_VOU
> >         Support for the Video Output Unit (VOU) on SuperH SoCs.
> >
> >  source "drivers/media/platform/renesas/rcar-vin/Kconfig"
> > +source "drivers/media/platform/renesas/rzg2l-cru/Kconfig"
> >
> >  # Mem2mem drivers
> >
> > diff --git a/drivers/media/platform/renesas/Makefile b/drivers/media/platform/renesas/Makefile
> > index 3ec226ef5fd2..55854e868887 100644
> > --- a/drivers/media/platform/renesas/Makefile
> > +++ b/drivers/media/platform/renesas/Makefile
> > @@ -4,6 +4,7 @@
> >  #
> >
> >  obj-y += rcar-vin/
> > +obj-y += rzg2l-cru/
> >  obj-y += vsp1/
> >
> >  obj-$(CONFIG_VIDEO_RCAR_DRIF) += rcar_drif.o
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/Kconfig b/drivers/media/platform/renesas/rzg2l-cru/Kconfig
> > new file mode 100644
> > index 000000000000..57c40bb499df
> > --- /dev/null
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/Kconfig
> > @@ -0,0 +1,17 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +config VIDEO_RZG2L_CSI2
> > +     tristate "RZ/G2L MIPI CSI-2 Receiver"
> > +     depends on ARCH_RENESAS || COMPILE_TEST
> > +     depends on V4L_PLATFORM_DRIVERS
> > +     depends on VIDEO_DEV && OF
> > +     select MEDIA_CONTROLLER
> > +     select RESET_CONTROLLER
> > +     select V4L2_FWNODE
> > +     select VIDEO_V4L2_SUBDEV_API
> > +     help
> > +       Support for Renesas RZ/G2L (and alike SoC's) MIPI CSI-2
> > +       Receiver driver.
> > +
> > +       To compile this driver as a module, choose M here: the
> > +       module will be called rzg2l-csi2.
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/Makefile b/drivers/media/platform/renesas/rzg2l-cru/Makefile
> > new file mode 100644
> > index 000000000000..91ea97a944e6
> > --- /dev/null
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/Makefile
> > @@ -0,0 +1,3 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +obj-$(CONFIG_VIDEO_RZG2L_CSI2) += rzg2l-csi2.o
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > new file mode 100644
> > index 000000000000..1f6838ed64fc
> > --- /dev/null
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > @@ -0,0 +1,761 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Driver for Renesas RZ/G2L MIPI CSI-2 Receiver
> > + *
> > + * Copyright (C) 2022 Renesas Electronics Corp.
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_graph.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/reset.h>
> > +#include <linux/sys_soc.h>
> > +#include <linux/units.h>
> > +
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-device.h>
> > +#include <media/v4l2-fwnode.h>
> > +#include <media/v4l2-mc.h>
> > +#include <media/v4l2-subdev.h>
> > +
> > +#include "rzg2l-csi2.h"
> > +
> > +/* LINK registers */
> > +/* Module Configuration Register */
> > +#define CSI2nMCG                     0x0
> > +#define CSI2nMCG_SDLN                        GENMASK(11, 8)
> > +
> > +/* Module Control Register 0 */
> > +#define CSI2nMCT0                    0x10
> > +#define CSI2nMCT0_VDLN(x)            ((x) << 0)
> > +
> > +/* Module Control Register 2 */
> > +#define CSI2nMCT2                    0x18
> > +#define CSI2nMCT2_FRRSKW(x)          ((x) << 16)
> > +#define CSI2nMCT2_FRRCLK(x)          ((x) << 0)
> > +
> > +/* Module Control Register 3 */
> > +#define CSI2nMCT3                    0x1c
> > +#define CSI2nMCT3_RXEN                       BIT(0)
> > +
> > +/* Reset Control Register */
> > +#define CSI2nRTCT                    0x28
> > +#define CSI2nRTCT_VSRST                      BIT(0)
> > +
> > +/* Reset Status Register */
> > +#define CSI2nRTST                    0x2c
> > +#define CSI2nRTST_VSRSTS             BIT(0)
> > +
> > +/* Receive Data Type Enable Low Register */
> > +#define CSI2nDTEL                    0x60
> > +
> > +/* Receive Data Type Enable High Register */
> > +#define CSI2nDTEH                    0x64
> > +
> > +/* Power Management Status Register */
> > +#define CSI2nPMST                    0x200
> > +
> > +/* Power Management Status Clear Register */
> > +#define CSI2nPMSC                    0x204
> > +
> > +/* DPHY registers */
> > +/* D-PHY Control Register 0 */
> > +#define CSIDPHYCTRL0                 0x400
> > +#define CSIDPHYCTRL0_EN_LDO1200              BIT(1)
> > +#define CSIDPHYCTRL0_EN_BGR          BIT(0)
> > +
> > +/* D-PHY Timing Register 0 */
> > +#define CSIDPHYTIM0                  0x404
> > +#define CSIDPHYTIM0_TCLK_MISS(x)     ((x) << 24)
> > +#define CSIDPHYTIM0_T_INIT(x)                ((x) << 0)
> > +
> > +/* D-PHY Timing Register 1 */
> > +#define CSIDPHYTIM1                  0x408
> > +#define CSIDPHYTIM1_THS_PREPARE(x)   ((x) << 24)
> > +#define CSIDPHYTIM1_TCLK_PREPARE(x)  ((x) << 16)
> > +#define CSIDPHYTIM1_THS_SETTLE(x)    ((x) << 8)
> > +#define CSIDPHYTIM1_TCLK_SETTLE(x)   ((x) << 0)
> > +
> > +/* D-PHY Skew Adjustment Function */
> > +#define CSIDPHYSKW0                  0x460
> > +#define CSIDPHYSKW0_UTIL_DL0_SKW_ADJ(x)      ((x) & 0x3)
> > +#define CSIDPHYSKW0_UTIL_DL1_SKW_ADJ(x)      (((x) & 0x3) << 4)
> > +#define CSIDPHYSKW0_UTIL_DL2_SKW_ADJ(x)      (((x) & 0x3) << 8)
> > +#define CSIDPHYSKW0_UTIL_DL3_SKW_ADJ(x)      (((x) & 0x3) << 12)
> > +#define CSIDPHYSKW0_DEFAULT_SKW              CSIDPHYSKW0_UTIL_DL0_SKW_ADJ(1) | \
> > +                                     CSIDPHYSKW0_UTIL_DL1_SKW_ADJ(1) | \
> > +                                     CSIDPHYSKW0_UTIL_DL2_SKW_ADJ(1) | \
> > +                                     CSIDPHYSKW0_UTIL_DL3_SKW_ADJ(1)
> > +
> > +#define VSRSTS_RETRIES                       20
> > +
> > +#define RZG2L_CSI2_DEFAULT_WIDTH     800
> > +#define RZG2L_CSI2_DEFAULT_HEIGHT    600
> > +#define RZG2L_CSI2_DEFAULT_FMT               MEDIA_BUS_FMT_UYVY8_1X16
> > +
> > +struct rzg2l_csi2_timings {
> > +     u32 t_init;
> > +     u32 tclk_miss;
> > +     u32 tclk_settle;
> > +     u32 ths_settle;
> > +     u32 tclk_prepare;
> > +     u32 ths_prepare;
> > +};
> > +
> > +enum rzg2l_dphy_timings {
> > +     TRANSMISSION_RATE_80_MBPS = 0,
> > +     TRANSMISSION_RATE_125_MBPS,
> > +     TRANSMISSION_RATE_250_MBPS,
> > +     TRANSMISSION_RATE_360_MBPS,
> > +     TRANSMISSION_RATE_360_MBPS_PLUS,
> > +};
> > +
> > +static const struct rzg2l_csi2_timings rzg2l_csi2_global_timings[] = {
> > +     [TRANSMISSION_RATE_80_MBPS] = {
> > +             .t_init = 79801,
> > +             .tclk_miss = 4,
> > +             .tclk_settle = 23,
> > +             .ths_settle = 31,
> > +             .tclk_prepare = 10,
> > +             .ths_prepare = 19,
> > +     },
> > +     [TRANSMISSION_RATE_125_MBPS] = {
> > +             .t_init = 79801,
> > +             .tclk_miss = 4,
> > +             .tclk_settle = 23,
> > +             .ths_settle = 28,
> > +             .tclk_prepare = 10,
> > +             .ths_prepare = 19,
> > +     },
> > +     [TRANSMISSION_RATE_250_MBPS] = {
> > +             .t_init = 79801,
> > +             .tclk_miss = 4,
> > +             .tclk_settle = 23,
> > +             .ths_settle = 22,
> > +             .tclk_prepare = 10,
> > +             .ths_prepare = 16,
> > +     },
> > +     [TRANSMISSION_RATE_360_MBPS] = {
> > +             .t_init = 79801,
> > +             .tclk_miss = 4,
> > +             .tclk_settle = 18,
> > +             .ths_settle = 19,
> > +             .tclk_prepare = 10,
> > +             .ths_prepare = 10,
> > +     },
> > +     [TRANSMISSION_RATE_360_MBPS_PLUS] = {
> > +             .t_init = 79801,
> > +             .tclk_miss = 4,
> > +             .tclk_settle = 18,
> > +             .ths_settle = 18,
> > +             .tclk_prepare = 10,
> > +             .ths_prepare = 10,
> > +     },
> > +};
> > +
> > +struct rzg2l_csi2_format {
> > +     u32 code;
> > +     unsigned int datatype;
> > +     unsigned int bpp;
> > +};
> > +
> > +static const struct rzg2l_csi2_format rzg2l_csi2_formats[] = {
> > +     { .code = MEDIA_BUS_FMT_UYVY8_1X16,     .datatype = 0x1e, .bpp = 16 },
> > +};
> > +
> > +static const struct rzg2l_csi2_format *rzg2l_csi2_code_to_fmt(unsigned int code)
> > +{
> > +     unsigned int i;
> > +
> > +     for (i = 0; i < ARRAY_SIZE(rzg2l_csi2_formats); i++)
> > +             if (rzg2l_csi2_formats[i].code == code)
> > +                     return &rzg2l_csi2_formats[i];
> > +
> > +     return NULL;
> > +}
> > +
> > +static inline struct rzg2l_csi2 *notifier_to_csi2(struct v4l2_async_notifier *n)
> > +{
> > +     return container_of(n, struct rzg2l_csi2, notifier);
> > +}
> > +
> > +static u32 rzg2l_csi2_read(struct rzg2l_csi2 *csi2, unsigned int reg)
> > +{
> > +     return ioread32(csi2->base + reg);
> > +}
> > +
> > +static void rzg2l_csi2_write(struct rzg2l_csi2 *csi2, unsigned int reg,
> > +                          u32 data)
> > +{
> > +     iowrite32(data, csi2->base + reg);
> > +}
> > +
> > +static void rzg2l_csi2_set(struct rzg2l_csi2 *csi2, unsigned int reg, u32 set)
> > +{
> > +     rzg2l_csi2_write(csi2, reg, rzg2l_csi2_read(csi2, reg) | set);
> > +}
> > +
> > +static void rzg2l_csi2_clr(struct rzg2l_csi2 *csi2, unsigned int reg, u32 clr)
> > +{
> > +     rzg2l_csi2_write(csi2, reg, rzg2l_csi2_read(csi2, reg) & ~clr);
> > +}
> > +
> > +static int rzg2l_csi2_calc_mbps(struct rzg2l_csi2 *csi2)
> > +{
> > +     struct v4l2_subdev *sd = csi2->remote_source;
>
> Could you rename sd to source ? A local sd variable usually refers to
> the subdev associated with the driver, it would be a bit confusing here.
>
Agreed, will do.

> > +     const struct rzg2l_csi2_format *format;
> > +     const struct v4l2_mbus_framefmt *fmt;
> > +     struct v4l2_subdev_state *state;
> > +     struct v4l2_ctrl *ctrl;
> > +     u64 mbps;
> > +
> > +     /* Read the pixel rate control from remote. */
> > +     ctrl = v4l2_ctrl_find(sd->ctrl_handler, V4L2_CID_PIXEL_RATE);
> > +     if (!ctrl) {
> > +             dev_err(csi2->dev, "no pixel rate control in subdev %s\n",
> > +                     sd->name);
> > +             return -EINVAL;
> > +     }
> > +
> > +     state = v4l2_subdev_lock_and_get_active_state(&csi2->subdev);
> > +     fmt = v4l2_subdev_get_pad_format(sd, state, RZG2L_CSI2_SINK);
>
> This isn't right, the first argument is the remote subdev, and the state
> is for the local subdev. I think you meant
>
>         fmt = v4l2_subdev_get_pad_format(&csi2->subdev, state, RZG2L_CSI2_SINK);
>
Ouch, yes you are right.

> > +     v4l2_subdev_unlock_state(state);
> > +     format = rzg2l_csi2_code_to_fmt(fmt->code);
> > +
> > +     /*
> > +      * Calculate hsfreq in Mbps
> > +      * hsfreq = (pixel_rate * bits_per_sample) / number_of_lanes
> > +      */
> > +     mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * format->bpp;
> > +     do_div(mbps, csi2->lanes * 1000000);
> > +
> > +     return mbps;
> > +}
> > +
> > +int rzg2l_csi2_cmn_rstb_deassert(struct rzg2l_csi2 *csi2)
> > +{
> > +     return reset_control_deassert(csi2->rstc);
> > +}
>
> This function is called by the CRU driver just before call .s_stream(1).
> Let's drop it, and move the reset_control_deassert() call to
> rzg2l_csi2_s_stream().
>
Agreed, I have dropped this completely now.

> > +
> > +/* -----------------------------------------------------------------------------
> > + * DPHY setting
> > + */
> > +
> > +static int rzg2l_csi2_dphy_disable(struct rzg2l_csi2 *csi2)
> > +{
> > +     int ret;
> > +
> > +     /* Reset the CRU (D-PHY) */
> > +     ret = reset_control_assert(csi2->rstc);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /* Stop the D-PHY clock */
> > +     clk_disable_unprepare(csi2->sysclk);
> > +
> > +     /* Cancel the EN_LDO1200 register setting */
> > +     rzg2l_csi2_clr(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_LDO1200);
> > +
> > +     /* Cancel the EN_BGR register setting */
> > +     rzg2l_csi2_clr(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_BGR);
> > +
> > +     return 0;
> > +}
> > +
> > +static int rzg2l_csi2_dphy_enable(struct rzg2l_csi2 *csi2)
> > +{
> > +     const struct rzg2l_csi2_timings *dphy_timing;
> > +     u32 dphytim0, dphytim1;
> > +     int mbps;
> > +     int ret;
> > +
> > +     mbps = rzg2l_csi2_calc_mbps(csi2);
> > +     if (mbps < 0)
> > +             return mbps;
> > +
> > +     csi2->hsfreq = mbps;
> > +
> > +     /* Set DPHY timing parameters */
> > +     if (csi2->hsfreq <= 80)
> > +             dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_80_MBPS];
> > +     else if (csi2->hsfreq <= 125)
> > +             dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_125_MBPS];
> > +     else if (csi2->hsfreq <= 250)
> > +             dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_250_MBPS];
> > +     else if (csi2->hsfreq <= 360)
> > +             dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_360_MBPS];
> > +     else if (csi2->hsfreq <= 1500)
> > +             dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_360_MBPS_PLUS];
> > +     else
> > +             return -EINVAL;
>
> Add a max_hsfreq field to rzg2l_csi2_timings, and write
>
>         for (i = 0; i < ARRAY_SIZE(rzg2l_csi2_global_timings); ++i) {
>                 dphy_timing = &rzg2l_csi2_global_timings[i];
>
>                 if (csi2->hsfreq <= dphy_timing->max_hsfreq)
>                         break;
>         }
>
> You can then drop the rzg2l_dphy_timings enum.
>
Thanks, Ive now implemeted as per your suggestion.

> > +
> > +     /* Set D-PHY timing parameters */
> > +     dphytim0 = CSIDPHYTIM0_TCLK_MISS(dphy_timing->tclk_miss) |
> > +                     CSIDPHYTIM0_T_INIT(dphy_timing->t_init);
> > +     dphytim1 = CSIDPHYTIM1_THS_PREPARE(dphy_timing->ths_prepare) |
> > +                     CSIDPHYTIM1_TCLK_PREPARE(dphy_timing->tclk_prepare) |
> > +                     CSIDPHYTIM1_THS_SETTLE(dphy_timing->ths_settle) |
> > +                     CSIDPHYTIM1_TCLK_SETTLE(dphy_timing->tclk_settle);
> > +     rzg2l_csi2_write(csi2, CSIDPHYTIM0, dphytim0);
> > +     rzg2l_csi2_write(csi2, CSIDPHYTIM1, dphytim1);
> > +
> > +     /* Enable D-PHY power control 0 */
> > +     rzg2l_csi2_write(csi2, CSIDPHYSKW0, CSIDPHYSKW0_DEFAULT_SKW);
> > +
> > +     /* Set the EN_BGR bit */
> > +     rzg2l_csi2_set(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_BGR);
> > +
> > +     /* Delay 20us to be stable */
> > +     usleep_range(20, 40);
> > +
> > +     /* Enable D-PHY power control 1 */
> > +     rzg2l_csi2_set(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_LDO1200);
> > +
> > +     /* Delay 10us to be stable */
> > +     usleep_range(10, 20);
> > +
> > +     /* Start supplying the internal clock for the D-PHY block */
> > +     ret = clk_prepare_enable(csi2->sysclk);
> > +     if (ret)
> > +             rzg2l_csi2_dphy_disable(csi2);
> > +
> > +     return ret;
> > +}
> > +
> > +int rzg2l_csi2_dphy_setting(struct rzg2l_csi2 *csi2, bool on)
> > +{
> > +     if (on)
> > +             return rzg2l_csi2_dphy_enable(csi2);
> > +
> > +     return rzg2l_csi2_dphy_disable(csi2);
> > +}
>
> Does this work without exporting the symbol when the drivers are
> compiled as modules ?
>
No it needs an EXPORT_SYMBOL().

> > +
> > +static void rzg2l_csi2_mipi_link_enable(struct rzg2l_csi2 *csi2)
> > +{
> > +     unsigned long vclk_rate = clk_get_rate(csi2->vclk) / HZ_PER_MHZ;
> > +     u32 frrskw, frrclk, frrskw_coeff, frrclk_coeff;
> > +
> > +     /* Select data lanes */
> > +     rzg2l_csi2_write(csi2, CSI2nMCT0, CSI2nMCT0_VDLN(csi2->lanes));
> > +
> > +     frrskw_coeff = 3 * vclk_rate * 8;
> > +     frrclk_coeff = frrskw_coeff / 2;
> > +     frrskw = DIV_ROUND_UP(frrskw_coeff, csi2->hsfreq);
> > +     frrclk = DIV_ROUND_UP(frrclk_coeff, csi2->hsfreq);
> > +     rzg2l_csi2_write(csi2, CSI2nMCT2, CSI2nMCT2_FRRSKW(frrskw) |
> > +                      CSI2nMCT2_FRRCLK(frrclk));
> > +
> > +     /*
> > +      * Select data type.
> > +      * FS, FE, LS, LE, Generic Short Packet Codes 1 to 8,
> > +      * Generic Long Packet Data Types 1 to 4 YUV422 8-bit,
> > +      * RGB565, RGB888, RAW8 to RAW20, User-defined 8-bit
> > +      * data types 1 to 8
> > +      */
> > +     rzg2l_csi2_write(csi2, CSI2nDTEL, 0xf778ff0f);
> > +     rzg2l_csi2_write(csi2, CSI2nDTEH, 0x00ffff1f);
> > +
> > +     /* Enable LINK reception */
> > +     rzg2l_csi2_write(csi2, CSI2nMCT3, CSI2nMCT3_RXEN);
> > +}
> > +
> > +static void rzg2l_csi2_mipi_link_disable(struct rzg2l_csi2 *csi2)
> > +{
> > +     unsigned int timeout = VSRSTS_RETRIES;
> > +
> > +     /* Stop LINK reception */
> > +     rzg2l_csi2_clr(csi2, CSI2nMCT3, CSI2nMCT3_RXEN);
> > +
> > +     /* Request a software reset of the LINK Video Pixel Interface */
> > +     rzg2l_csi2_write(csi2, CSI2nRTCT, CSI2nRTCT_VSRST);
> > +
> > +     /* Make sure CSI2nRTST.VSRSTS bit is cleared */
> > +     while (timeout--) {
> > +             if (!(rzg2l_csi2_read(csi2, CSI2nRTST) & CSI2nRTST_VSRSTS))
> > +                     break;
> > +             usleep_range(100, 200);
> > +     };
> > +
> > +     if (!timeout)
> > +             dev_err(csi2->dev, "Clearing CSI2nRTST.VSRSTS timed out\n");
> > +}
> > +
> > +void rzg2l_csi2_mipi_link_setting(struct rzg2l_csi2 *csi2, bool on)
>
> Can this also be moved to rzg2l_csi2_s_stream() instead of being called
> from the CRU driver ? I really dislike the manual calls between the two
> drivers. This will mess up future reworks of the subdev infrastructure
> :-( Please try to stick to the standard code flow as much as possible.
>
I am trying to avoid this now based on the suggestion provided by Sakari.

> > +{
> > +     if (on)
> > +             rzg2l_csi2_mipi_link_enable(csi2);
> > +     else
> > +             rzg2l_csi2_mipi_link_disable(csi2);
> > +}
> > +
> > +static int rzg2l_csi2_s_stream(struct v4l2_subdev *sd, int enable)
> > +{
> > +     struct rzg2l_csi2 *csi2 = sd_to_csi2(sd);
> > +
> > +     return v4l2_subdev_call(csi2->remote_source, video, s_stream, enable);
> > +}
> > +
> > +static int rzg2l_csi2_set_format(struct v4l2_subdev *sd,
> > +                              struct v4l2_subdev_state *state,
> > +                              struct v4l2_subdev_format *fmt)
> > +{
> > +     struct v4l2_mbus_framefmt *src_format;
> > +     struct v4l2_mbus_framefmt *format;
> > +
> > +     if (fmt->pad > RZG2L_CSI2_SINK)
> > +             return -EINVAL;
>
> That's not right, set_format must not return an error on the source pad.
> Have you run v4l2-compliance on this driver ? Please include the
> v4l2-compliance report in the cover letter.
>
OK, I'll provide the v4l2-compliance report.

> As the CSI-2 receiver has the same format on the input and output, you
> can simply write
>
>         src_format = v4l2_subdev_get_pad_format(sd, state, RZG2L_CSI2_SOURCE);
>
>         if (fmt->pad == RZG2L_CSI2_SOURCE) {
>                 fmt->format = *src_format;
>                 return 0;
>         }
>
> here. I'd also rename format to sink_format and write
>
>         sink_format = v4l2_subdev_get_pad_format(sd, state, RZG2L_CSI2_SINK);
>         src_format = v4l2_subdev_get_pad_format(sd, state, RZG2L_CSI2_SOURCE);
>
> at the beginning of the function.
>
Agreed, will do.

> > +
> > +     format = v4l2_subdev_get_pad_format(sd, state, fmt->pad);
> > +
> > +     if (!rzg2l_csi2_code_to_fmt(fmt->format.code))
> > +             format->code = rzg2l_csi2_formats[0].code;
> > +     else
> > +             format->code = fmt->format.code;
> > +
> > +     format->field = V4L2_FIELD_NONE;
> > +     format->colorspace = V4L2_COLORSPACE_SRGB;
>
> The CSI-2 receiver doesn't care about color spaces, so I would simply
> copy all colorspace-related fields received from userspace (colorspace,
> xfer_func, ycbcr_enc and quantization).
>
OK.

> > +     format->width = fmt->format.width;
> > +     format->height = fmt->format.height;
> > +     fmt->format = *format;
> > +
> > +     /* propagate format to source pad */
> > +     src_format = v4l2_subdev_get_pad_format(sd, state, RZG2L_CSI2_SOURCE_VC0);
> > +     *src_format = *format;
> > +
> > +     return 0;
> > +}
> > +
> > +static int rzg2l_csi2_init_config(struct v4l2_subdev *sd,
> > +                               struct v4l2_subdev_state *sd_state)
> > +{
> > +     struct v4l2_subdev_format fmt = { 0 };
> > +
> > +     fmt.which = sd_state ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
>
> When using the subdev active state, the state pointer passed to this
> function will never be null. That's fine, as rzg2l_csi2_set_format()
> doesn't use the which field. I'd just drop this line.
>
Thanks, I'll drop this line.

> Please set the pad to RZG2L_CSI2_SINK explicitly. It works as-is because
> RZG2L_CSI2_SINK is equal to zero, and the structure is zeroed above, but
> let's not depend on that.
>
Agreed, will do.

> > +     fmt.format.width = RZG2L_CSI2_DEFAULT_WIDTH;
> > +     fmt.format.height = RZG2L_CSI2_DEFAULT_HEIGHT;
> > +     fmt.format.field = V4L2_FIELD_NONE;
> > +     fmt.format.code = RZG2L_CSI2_DEFAULT_FMT;
> > +     fmt.format.colorspace = V4L2_COLORSPACE_SRGB;
> > +
And I will set the defaults to colorspace, xfer_func, ycbcr_enc and
quantization.

> > +     return rzg2l_csi2_set_format(sd, sd_state, &fmt);
> > +}
> > +
> > +static int rzg2l_csi2_enum_mbus_code(struct v4l2_subdev *sd,
> > +                                  struct v4l2_subdev_state *sd_state,
> > +                                  struct v4l2_subdev_mbus_code_enum *code)
> > +{
> > +     if (code->index >= ARRAY_SIZE(rzg2l_csi2_formats))
> > +             return -EINVAL;
> > +
> > +     code->code = rzg2l_csi2_formats[code->index].code;
> > +
> > +     return 0;
> > +}
> > +
> > +static int rzg2l_csi2_enum_frame_size(struct v4l2_subdev *sd,
> > +                                   struct v4l2_subdev_state *sd_state,
> > +                                   struct v4l2_subdev_frame_size_enum *fse)
> > +{
> > +     if (fse->index != 0)
> > +             return -EINVAL;
> > +
> > +     fse->min_width = clamp_t(u32, 1, 320, 2800);
> > +     fse->min_height = clamp_t(u32, 1, 240, 4095);
> > +     fse->max_width = clamp_t(u32, -1, 320, 2800);
> > +     fse->max_height = clamp_t(u32, -1, 240, 4095);
>
> That's a weird way to write
>
>         fse->min_width = 320;
>         fse->min_height = 240;
>         fse->max_width = 2800;
>         fse->max_height = 4095;
>
> Could you add macros for those values, the same way you already have
> RZG2L_CSI2_DEFAULT_WIDTH and RZG2L_CSI2_DEFAULT_HEIGHT ?
> rzg2l_csi2_set_format() should also clamp the width and height
> accordingly.
>
Agreed, will do.

> > +
> > +     return 0;
> > +}
> > +
> > +static const struct v4l2_subdev_video_ops rzg2l_csi2_video_ops = {
> > +     .s_stream = rzg2l_csi2_s_stream,
> > +};
> > +
> > +static const struct v4l2_subdev_pad_ops rzg2l_csi2_pad_ops = {
> > +     .enum_mbus_code = rzg2l_csi2_enum_mbus_code,
> > +     .init_cfg = rzg2l_csi2_init_config,
> > +     .enum_frame_size = rzg2l_csi2_enum_frame_size,
> > +     .set_fmt = rzg2l_csi2_set_format,
> > +     .get_fmt = v4l2_subdev_get_fmt,
> > +};
> > +
> > +static const struct v4l2_subdev_ops rzg2l_csi2_subdev_ops = {
> > +     .video  = &rzg2l_csi2_video_ops,
> > +     .pad    = &rzg2l_csi2_pad_ops,
> > +};
> > +
> > +/* -----------------------------------------------------------------------------
> > + * Async handling and registration of subdevices and links.
> > + */
> > +
> > +static int rzg2l_csi2_notify_bound(struct v4l2_async_notifier *notifier,
> > +                                struct v4l2_subdev *subdev,
> > +                                struct v4l2_async_subdev *asd)
> > +{
> > +     struct rzg2l_csi2 *csi2 = notifier_to_csi2(notifier);
> > +
> > +     csi2->remote_source = subdev;
> > +
> > +     dev_dbg(csi2->dev, "Bound subdev: %s pad\n", subdev->name);
> > +
> > +     return media_create_pad_link(&subdev->entity, RZG2L_CSI2_SINK,
> > +                                  &csi2->subdev.entity, 0,
> > +                                  MEDIA_LNK_FL_ENABLED |
> > +                                  MEDIA_LNK_FL_IMMUTABLE);
> > +}
> > +
> > +static void rzg2l_csi2_notify_unbind(struct v4l2_async_notifier *notifier,
> > +                                  struct v4l2_subdev *subdev,
> > +                                  struct v4l2_async_subdev *asd)
> > +{
> > +     struct rzg2l_csi2 *csi2 = notifier_to_csi2(notifier);
> > +
> > +     csi2->remote_source = NULL;
> > +
> > +     dev_dbg(csi2->dev, "Unbind subdev %s\n", subdev->name);
> > +}
> > +
> > +static const struct v4l2_async_notifier_operations rzg2l_csi2_notify_ops = {
> > +     .bound = rzg2l_csi2_notify_bound,
> > +     .unbind = rzg2l_csi2_notify_unbind,
> > +};
> > +
> > +static int rzg2l_csi2_parse_v4l2(struct rzg2l_csi2 *csi2,
> > +                              struct v4l2_fwnode_endpoint *vep)
> > +{
> > +     /* Only port 0 endpoint 0 is valid. */
> > +     if (vep->base.port || vep->base.id)
> > +             return -ENOTCONN;
> > +
> > +     csi2->lanes = vep->bus.mipi_csi2.num_data_lanes;
> > +
> > +     return 0;
> > +}
> > +
> > +static int rzg2l_csi2_parse_dt(struct rzg2l_csi2 *csi2)
> > +{
> > +     struct v4l2_fwnode_endpoint v4l2_ep = {
> > +             .bus_type = V4L2_MBUS_CSI2_DPHY
> > +     };
> > +     struct v4l2_async_subdev *asd;
> > +     struct fwnode_handle *fwnode;
> > +     struct fwnode_handle *ep;
> > +     int ret;
> > +
> > +     ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(csi2->dev), 0, 0, 0);
> > +     if (!ep) {
> > +             dev_err(csi2->dev, "Not connected to subdevice\n");
> > +             return -EINVAL;
> > +     }
> > +
> > +     ret = v4l2_fwnode_endpoint_parse(ep, &v4l2_ep);
> > +     if (ret) {
> > +             dev_err(csi2->dev, "Could not parse v4l2 endpoint\n");
> > +             fwnode_handle_put(ep);
> > +             return -EINVAL;
> > +     }
> > +
> > +     ret = rzg2l_csi2_parse_v4l2(csi2, &v4l2_ep);
> > +     if (ret) {
> > +             fwnode_handle_put(ep);
> > +             return ret;
> > +     }
> > +
> > +     fwnode = fwnode_graph_get_remote_endpoint(ep);
> > +     fwnode_handle_put(ep);
> > +
> > +     dev_dbg(csi2->dev, "Found '%pOF'\n", to_of_node(fwnode));
> > +
> > +     v4l2_async_nf_init(&csi2->notifier);
> > +     csi2->notifier.ops = &rzg2l_csi2_notify_ops;
> > +
> > +     asd = v4l2_async_nf_add_fwnode(&csi2->notifier, fwnode,
> > +                                    struct v4l2_async_subdev);
> > +     fwnode_handle_put(fwnode);
> > +     if (IS_ERR(asd))
> > +             return PTR_ERR(asd);
> > +
> > +     ret = v4l2_async_subdev_nf_register(&csi2->subdev, &csi2->notifier);
> > +     if (ret)
> > +             v4l2_async_nf_cleanup(&csi2->notifier);
> > +
> > +     return ret;
> > +}
> > +
> > +static int rzg2l_validate_csi2_lanes(struct rzg2l_csi2 *csi2)
> > +{
> > +     int ret = 0;
> > +     int lanes;
> > +
> > +     if (csi2->lanes != 1 && csi2->lanes != 2 && csi2->lanes != 4) {
> > +             dev_err(csi2->dev, "Unsupported number of data-lanes: %u\n",
> > +                     csi2->lanes);
> > +             return -EINVAL;
> > +     }
> > +
> > +     ret = clk_prepare_enable(csi2->pclk);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /* Checking the maximum lanes support for CSI-2 module */
> > +     lanes = (rzg2l_csi2_read(csi2, CSI2nMCG) & CSI2nMCG_SDLN) >> 8;
> > +     if (lanes < csi2->lanes) {
> > +             dev_err(csi2->dev,
> > +                     "Failed to support %d data lanes\n", csi2->lanes);
> > +             ret = -EINVAL;
> > +     }
> > +
> > +     clk_disable_unprepare(csi2->pclk);
> > +
> > +     return ret;
> > +}
> > +
> > +/* -----------------------------------------------------------------------------
> > + * Platform Device Driver.
> > + */
> > +
> > +static const struct media_entity_operations rzg2l_csi2_entity_ops = {
> > +     .link_validate = v4l2_subdev_link_validate,
> > +};
> > +
> > +static int rzg2l_csi2_probe(struct platform_device *pdev)
> > +{
> > +     struct rzg2l_csi2 *csi2;
> > +     int ret;
> > +
> > +     csi2 = devm_kzalloc(&pdev->dev, sizeof(*csi2), GFP_KERNEL);
> > +     if (!csi2)
> > +             return -ENOMEM;
> > +
> > +     csi2->base = devm_platform_ioremap_resource(pdev, 0);
> > +     if (IS_ERR(csi2->base))
> > +             return PTR_ERR(csi2->base);
> > +
> > +     csi2->rstc = devm_reset_control_get(&pdev->dev, "cmn-rstb");
> > +     if (IS_ERR(csi2->rstc))
> > +             return dev_err_probe(&pdev->dev, PTR_ERR(csi2->rstc),
> > +                                  "failed to get cpg cmn-rstb\n");
> > +
> > +     csi2->sysclk = devm_clk_get(&pdev->dev, "sysclk");
> > +     if (IS_ERR(csi2->sysclk)) {
> > +             dev_err(&pdev->dev, "Failed to get sysclk");
> > +             return PTR_ERR(csi2->sysclk);
> > +     }
> > +
> > +     csi2->vclk = devm_clk_get(&pdev->dev, "vclk");
> > +     if (IS_ERR(csi2->vclk)) {
> > +             dev_err(&pdev->dev, "Failed to get vclk");
> > +             return PTR_ERR(csi2->vclk);
> > +     }
> > +
> > +     csi2->pclk = devm_clk_get(&pdev->dev, "pclk");
> > +     if (IS_ERR(csi2->pclk)) {
> > +             dev_err(&pdev->dev, "Failed to get pclk");
> > +             return PTR_ERR(csi2->pclk);
> > +     }
>
> Could devm_clk_bulk_get() help ?
>
Enabling/disabling specific clocks would be more code?

> > +
> > +     csi2->dev = &pdev->dev;
> > +
> > +     platform_set_drvdata(pdev, csi2);
> > +
> > +     ret = rzg2l_csi2_parse_dt(csi2);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = rzg2l_validate_csi2_lanes(csi2);
> > +     if (ret)
> > +             return ret;
> > +
> > +     csi2->subdev.dev = &pdev->dev;
> > +     v4l2_subdev_init(&csi2->subdev, &rzg2l_csi2_subdev_ops);
> > +     v4l2_set_subdevdata(&csi2->subdev, &pdev->dev);
> > +     snprintf(csi2->subdev.name, sizeof(csi2->subdev.name),
> > +              "csi-%s", dev_name(&pdev->dev));
> > +     csi2->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> > +
> > +     csi2->subdev.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
> > +     csi2->subdev.entity.ops = &rzg2l_csi2_entity_ops;
> > +
> > +     csi2->pads[RZG2L_CSI2_SINK].flags = MEDIA_PAD_FL_SINK;
> > +     /*
> > +      * TODO: RZ/G2L CSI2 supports 4 virtual channels, as virtual
> > +      * channels should be implemented by streams API which is under
> > +      * development lets hardcode to VC0 for now.
> > +      */
> > +     csi2->pads[RZG2L_CSI2_SOURCE_VC0].flags = MEDIA_PAD_FL_SOURCE;
> > +     ret = media_entity_pads_init(&csi2->subdev.entity, 2, csi2->pads);
> > +     if (ret)
> > +             goto error_async;
> > +
> > +     pm_runtime_enable(&pdev->dev);
> > +
> > +     ret = v4l2_subdev_init_finalize(&csi2->subdev);
> > +     if (ret < 0)
> > +             goto error_pm;
> > +
> > +     /* enable pclk for register access */
> > +     ret = clk_prepare_enable(csi2->pclk);
> > +     if (ret)
> > +             goto error_pm;
>
> Keeping the clock always on isn't great from a power management point of
> view. Runtime PM would be much better.
>
I have now moved clk_prepare_enable()/clk_disable_unprepare() this to
rzg2l_csi2_dphy_setting() and rzg2l_csi2_mipi_link_setting() thats
where the actual read/writes are happening.

> > +
> > +     ret = v4l2_async_register_subdev(&csi2->subdev);
> > +     if (ret < 0)
> > +             goto error_subdev;
> > +
> > +     dev_info(csi2->dev, "%d lanes found\n", csi2->lanes);
> > +
> > +     return 0;
> > +
> > +error_subdev:
> > +     clk_disable_unprepare(csi2->pclk);
> > +     v4l2_subdev_cleanup(&csi2->subdev);
> > +error_pm:
> > +     pm_runtime_disable(&pdev->dev);
> > +error_async:
> > +     v4l2_async_nf_unregister(&csi2->notifier);
> > +     v4l2_async_nf_cleanup(&csi2->notifier);
> > +
> > +     return ret;
> > +}
> > +
> > +static const struct of_device_id rzg2l_csi2_of_table[] = {
> > +     { .compatible = "renesas,rzg2l-csi2", },
> > +     { /* sentinel */ }
> > +};
> > +
> > +static int rzg2l_csi2_remove(struct platform_device *pdev)
> > +{
> > +     struct rzg2l_csi2 *csi2 = platform_get_drvdata(pdev);
> > +
> > +     v4l2_async_nf_unregister(&csi2->notifier);
> > +     v4l2_async_nf_cleanup(&csi2->notifier);
> > +     v4l2_async_unregister_subdev(&csi2->subdev);
> > +     clk_disable_unprepare(csi2->pclk);
> > +     v4l2_subdev_cleanup(&csi2->subdev);
> > +
> > +     pm_runtime_disable(&pdev->dev);
> > +
> > +     return 0;
> > +}
> > +
> > +static struct platform_driver rzg2l_csi2_pdrv = {
> > +     .remove = rzg2l_csi2_remove,
> > +     .probe  = rzg2l_csi2_probe,
> > +     .driver = {
> > +             .name = "rzg2l-csi2",
> > +             .of_match_table = rzg2l_csi2_of_table,
> > +     },
> > +};
> > +
> > +module_platform_driver(rzg2l_csi2_pdrv);
> > +
> > +MODULE_AUTHOR("Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>");
> > +MODULE_DESCRIPTION("Renesas RZ/G2L MIPI CSI2 receiver driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h
> > new file mode 100644
> > index 000000000000..f85a1d44250f
> > --- /dev/null
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h
> > @@ -0,0 +1,46 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Copyright (C) 2022 Renesas Electronics Corp.
> > + */
> > +
> > +#ifndef __RZG2L_CSI2__
> > +#define __RZG2L_CSI2__
> > +
> > +enum rzg2l_csi2_pads {
> > +     RZG2L_CSI2_SINK = 0,
> > +     RZG2L_CSI2_SOURCE_VC0,
> > +     RZG2L_CSI2_SOURCE_VC1,
> > +     RZG2L_CSI2_SOURCE_VC2,
> > +     RZG2L_CSI2_SOURCE_VC3,
>
> Drop RZG2L_CSI2_SOURCE_VC[123] and rename RZG2L_CSI2_SOURCE_VC0 to
> RZG2L_CSI2_SOURCE.
>
Done.

> > +     NR_OF_RZG2L_CSI2_PAD,
> > +};
> > +
> > +struct rzg2l_csi2 {
> > +     struct device *dev;
> > +     void __iomem *base;
> > +     struct reset_control *rstc;
> > +     struct clk *sysclk;
> > +     struct clk *vclk;
> > +     struct clk *pclk;
> > +
> > +     struct v4l2_subdev subdev;
> > +     struct media_pad pads[NR_OF_RZG2L_CSI2_PAD];
> > +
> > +     struct v4l2_async_notifier notifier;
> > +     struct v4l2_subdev *remote_source;
> > +
> > +     unsigned short lanes;
> > +
> > +     unsigned long hsfreq;
> > +};
> > +
> > +static inline struct rzg2l_csi2 *sd_to_csi2(struct v4l2_subdev *sd)
> > +{
> > +     return container_of(sd, struct rzg2l_csi2, subdev);
> > +}
> > +
> > +int rzg2l_csi2_cmn_rstb_deassert(struct rzg2l_csi2 *csi2);
> > +int rzg2l_csi2_dphy_setting(struct rzg2l_csi2 *csi2, bool on);
> > +void rzg2l_csi2_mipi_link_setting(struct rzg2l_csi2 *csi2, bool on);
>
> Let's make the API as opaque as possible for the CRU driver. I'd move
> the definition of the rzg2l_csi2 structure to the .c file, along with
> the sd_to_csi2() function, and change these three functions to take a
> v4l2_subdev pointer.
>
Agreed, I have got rid of the above three functions (infact the header
file itself is removed) and now using the pre_streamon callback.
(Initial testing looks good)

Cheers,
Prabhakar
Sakari Ailus Sept. 22, 2022, 12:34 p.m. UTC | #4
Hi Prabhakar,

On Thu, Sep 22, 2022 at 01:08:33PM +0100, Lad, Prabhakar wrote:
> > > * Switched to manually turn ON/OFF the clocks instead of pm_runtime so that
> > >   the mipi/dhpy initialization happens as per the HW manual
> >
> > That doesn't look right. The driver doesn't use runtime PM anymore, so
> > power domains may not be handled properly. What was the problem with
> > clock handling using runtime PM ?
> >
> If we use the runtime PM all the clocks will be turned ON when we call
> pm_runtime_resume_and_get() which I dont want to. As per the "Starting
> reception for MIPI CSI-2 Input" section 35.3.1 for example we first
> need to turn ON all the clocks and later further down the line we need
> to just turn OFF VCLK -> Enable Link -> turn ON VCLK. Due to such
> cases I have switched to individual clock handling.

If that is the case, then you should control just that clock directly,
outside runtime PM callbacks.

Runtime PM may be needed e.g. for resuming a parent device.
Geert Uytterhoeven Sept. 22, 2022, 12:51 p.m. UTC | #5
On Thu, Sep 22, 2022 at 2:34 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
> On Thu, Sep 22, 2022 at 01:08:33PM +0100, Lad, Prabhakar wrote:
> > > > * Switched to manually turn ON/OFF the clocks instead of pm_runtime so that
> > > >   the mipi/dhpy initialization happens as per the HW manual
> > >
> > > That doesn't look right. The driver doesn't use runtime PM anymore, so
> > > power domains may not be handled properly. What was the problem with
> > > clock handling using runtime PM ?
> > >
> > If we use the runtime PM all the clocks will be turned ON when we call
> > pm_runtime_resume_and_get() which I dont want to. As per the "Starting
> > reception for MIPI CSI-2 Input" section 35.3.1 for example we first
> > need to turn ON all the clocks and later further down the line we need
> > to just turn OFF VCLK -> Enable Link -> turn ON VCLK. Due to such
> > cases I have switched to individual clock handling.
>
> If that is the case, then you should control just that clock directly,
> outside runtime PM callbacks.
>
> Runtime PM may be needed e.g. for resuming a parent device.

Exactly.
So probably you should not consider R9A07G044_CRU_VCLK a PM clock,
i.e. you need changes to rzg2l_cpg_is_pm_clk() to exclude it.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Prabhakar Sept. 22, 2022, 12:53 p.m. UTC | #6
Hi Sakari,

Thank you for the review.

On Wed, Sep 21, 2022 at 10:35 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Prabhakar,
>
> Thanks for the set.
>
> On Tue, Sep 06, 2022 at 12:04:05AM +0100, Lad Prabhakar wrote:
> > Add MIPI CSI-2 receiver driver for Renesas RZ/G2L. The MIPI
> > CSI-2 is part of the CRU module found on RZ/G2L family.
> >
> > Based on a patch in the BSP by Hien Huynh
> > <hien.huynh.px@renesas.com>
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > v1 -> v2
> > * Sorted Kconfig select
> > * Prefixed generic names for struct/variables with rzg2_csi2
> > * Dropped unnecessary checks for remote source
> > * Dropped exporting functions
> > * Moved lane validation to probe
> > * Split up rzg2l_csi2_dphy_setting() and rzg2l_csi2_mipi_link_setting()
> > * Used rzg2l_csi2_write() wherever possible
> > * Dropped stream_count/lock
> > * Used active subdev state instead of manually storing format in driver
> > * Implemented init_cfg/enum_frame_size/enum_mbus_code callbacks
> > * Dropped check for bus_type of remote source
> > * Switched to manually turn ON/OFF the clocks instead of pm_runtime so that
> >   the mipi/dhpy initialization happens as per the HW manual
> > * Hardcoded VC0 usage for now as streams API is under development
> >
> > RFC v2 -> v1
> > * Fixed initialization sequence of DPHY and link
> > * Exported DPHY and link initialization functions so that the
> >   CRU core driver can initialize the CRU and CSI2 as per the HW manual.
> >
> > RFC v1 -> RFC v2
> > * new patch (split up as new driver compared to v1)
> > ---
> >  drivers/media/platform/renesas/Kconfig        |   1 +
> >  drivers/media/platform/renesas/Makefile       |   1 +
> >  .../media/platform/renesas/rzg2l-cru/Kconfig  |  17 +
> >  .../media/platform/renesas/rzg2l-cru/Makefile |   3 +
> >  .../platform/renesas/rzg2l-cru/rzg2l-csi2.c   | 761 ++++++++++++++++++
> >  .../platform/renesas/rzg2l-cru/rzg2l-csi2.h   |  46 ++
> >  6 files changed, 829 insertions(+)
> >  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/Kconfig
> >  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/Makefile
> >  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> >  create mode 100644 drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h
> >
> > diff --git a/drivers/media/platform/renesas/Kconfig b/drivers/media/platform/renesas/Kconfig
> > index 9fd90672ea2d..ed788e991f74 100644
> > --- a/drivers/media/platform/renesas/Kconfig
> > +++ b/drivers/media/platform/renesas/Kconfig
> > @@ -41,6 +41,7 @@ config VIDEO_SH_VOU
> >         Support for the Video Output Unit (VOU) on SuperH SoCs.
> >
> >  source "drivers/media/platform/renesas/rcar-vin/Kconfig"
> > +source "drivers/media/platform/renesas/rzg2l-cru/Kconfig"
> >
> >  # Mem2mem drivers
> >
> > diff --git a/drivers/media/platform/renesas/Makefile b/drivers/media/platform/renesas/Makefile
> > index 3ec226ef5fd2..55854e868887 100644
> > --- a/drivers/media/platform/renesas/Makefile
> > +++ b/drivers/media/platform/renesas/Makefile
> > @@ -4,6 +4,7 @@
> >  #
> >
> >  obj-y += rcar-vin/
> > +obj-y += rzg2l-cru/
> >  obj-y += vsp1/
> >
> >  obj-$(CONFIG_VIDEO_RCAR_DRIF) += rcar_drif.o
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/Kconfig b/drivers/media/platform/renesas/rzg2l-cru/Kconfig
> > new file mode 100644
> > index 000000000000..57c40bb499df
> > --- /dev/null
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/Kconfig
> > @@ -0,0 +1,17 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +config VIDEO_RZG2L_CSI2
> > +     tristate "RZ/G2L MIPI CSI-2 Receiver"
> > +     depends on ARCH_RENESAS || COMPILE_TEST
> > +     depends on V4L_PLATFORM_DRIVERS
> > +     depends on VIDEO_DEV && OF
> > +     select MEDIA_CONTROLLER
> > +     select RESET_CONTROLLER
> > +     select V4L2_FWNODE
> > +     select VIDEO_V4L2_SUBDEV_API
> > +     help
> > +       Support for Renesas RZ/G2L (and alike SoC's) MIPI CSI-2
> > +       Receiver driver.
> > +
> > +       To compile this driver as a module, choose M here: the
> > +       module will be called rzg2l-csi2.
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/Makefile b/drivers/media/platform/renesas/rzg2l-cru/Makefile
> > new file mode 100644
> > index 000000000000..91ea97a944e6
> > --- /dev/null
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/Makefile
> > @@ -0,0 +1,3 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +obj-$(CONFIG_VIDEO_RZG2L_CSI2) += rzg2l-csi2.o
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > new file mode 100644
> > index 000000000000..1f6838ed64fc
> > --- /dev/null
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > @@ -0,0 +1,761 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Driver for Renesas RZ/G2L MIPI CSI-2 Receiver
> > + *
> > + * Copyright (C) 2022 Renesas Electronics Corp.
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_graph.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/reset.h>
> > +#include <linux/sys_soc.h>
> > +#include <linux/units.h>
> > +
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-device.h>
> > +#include <media/v4l2-fwnode.h>
> > +#include <media/v4l2-mc.h>
> > +#include <media/v4l2-subdev.h>
> > +
> > +#include "rzg2l-csi2.h"
> > +
> > +/* LINK registers */
> > +/* Module Configuration Register */
> > +#define CSI2nMCG                     0x0
> > +#define CSI2nMCG_SDLN                        GENMASK(11, 8)
> > +
> > +/* Module Control Register 0 */
> > +#define CSI2nMCT0                    0x10
> > +#define CSI2nMCT0_VDLN(x)            ((x) << 0)
> > +
> > +/* Module Control Register 2 */
> > +#define CSI2nMCT2                    0x18
> > +#define CSI2nMCT2_FRRSKW(x)          ((x) << 16)
> > +#define CSI2nMCT2_FRRCLK(x)          ((x) << 0)
> > +
> > +/* Module Control Register 3 */
> > +#define CSI2nMCT3                    0x1c
> > +#define CSI2nMCT3_RXEN                       BIT(0)
> > +
> > +/* Reset Control Register */
> > +#define CSI2nRTCT                    0x28
> > +#define CSI2nRTCT_VSRST                      BIT(0)
> > +
> > +/* Reset Status Register */
> > +#define CSI2nRTST                    0x2c
> > +#define CSI2nRTST_VSRSTS             BIT(0)
> > +
> > +/* Receive Data Type Enable Low Register */
> > +#define CSI2nDTEL                    0x60
> > +
> > +/* Receive Data Type Enable High Register */
> > +#define CSI2nDTEH                    0x64
> > +
> > +/* Power Management Status Register */
> > +#define CSI2nPMST                    0x200
> > +
> > +/* Power Management Status Clear Register */
> > +#define CSI2nPMSC                    0x204
> > +
> > +/* DPHY registers */
> > +/* D-PHY Control Register 0 */
> > +#define CSIDPHYCTRL0                 0x400
> > +#define CSIDPHYCTRL0_EN_LDO1200              BIT(1)
> > +#define CSIDPHYCTRL0_EN_BGR          BIT(0)
> > +
> > +/* D-PHY Timing Register 0 */
> > +#define CSIDPHYTIM0                  0x404
> > +#define CSIDPHYTIM0_TCLK_MISS(x)     ((x) << 24)
> > +#define CSIDPHYTIM0_T_INIT(x)                ((x) << 0)
> > +
> > +/* D-PHY Timing Register 1 */
> > +#define CSIDPHYTIM1                  0x408
> > +#define CSIDPHYTIM1_THS_PREPARE(x)   ((x) << 24)
> > +#define CSIDPHYTIM1_TCLK_PREPARE(x)  ((x) << 16)
> > +#define CSIDPHYTIM1_THS_SETTLE(x)    ((x) << 8)
> > +#define CSIDPHYTIM1_TCLK_SETTLE(x)   ((x) << 0)
> > +
> > +/* D-PHY Skew Adjustment Function */
> > +#define CSIDPHYSKW0                  0x460
> > +#define CSIDPHYSKW0_UTIL_DL0_SKW_ADJ(x)      ((x) & 0x3)
> > +#define CSIDPHYSKW0_UTIL_DL1_SKW_ADJ(x)      (((x) & 0x3) << 4)
> > +#define CSIDPHYSKW0_UTIL_DL2_SKW_ADJ(x)      (((x) & 0x3) << 8)
> > +#define CSIDPHYSKW0_UTIL_DL3_SKW_ADJ(x)      (((x) & 0x3) << 12)
> > +#define CSIDPHYSKW0_DEFAULT_SKW              CSIDPHYSKW0_UTIL_DL0_SKW_ADJ(1) | \
> > +                                     CSIDPHYSKW0_UTIL_DL1_SKW_ADJ(1) | \
> > +                                     CSIDPHYSKW0_UTIL_DL2_SKW_ADJ(1) | \
> > +                                     CSIDPHYSKW0_UTIL_DL3_SKW_ADJ(1)
> > +
> > +#define VSRSTS_RETRIES                       20
> > +
> > +#define RZG2L_CSI2_DEFAULT_WIDTH     800
> > +#define RZG2L_CSI2_DEFAULT_HEIGHT    600
> > +#define RZG2L_CSI2_DEFAULT_FMT               MEDIA_BUS_FMT_UYVY8_1X16
> > +
> > +struct rzg2l_csi2_timings {
> > +     u32 t_init;
> > +     u32 tclk_miss;
> > +     u32 tclk_settle;
> > +     u32 ths_settle;
> > +     u32 tclk_prepare;
> > +     u32 ths_prepare;
> > +};
> > +
> > +enum rzg2l_dphy_timings {
> > +     TRANSMISSION_RATE_80_MBPS = 0,
> > +     TRANSMISSION_RATE_125_MBPS,
> > +     TRANSMISSION_RATE_250_MBPS,
> > +     TRANSMISSION_RATE_360_MBPS,
> > +     TRANSMISSION_RATE_360_MBPS_PLUS,
> > +};
> > +
> > +static const struct rzg2l_csi2_timings rzg2l_csi2_global_timings[] = {
> > +     [TRANSMISSION_RATE_80_MBPS] = {
> > +             .t_init = 79801,
> > +             .tclk_miss = 4,
> > +             .tclk_settle = 23,
> > +             .ths_settle = 31,
> > +             .tclk_prepare = 10,
> > +             .ths_prepare = 19,
> > +     },
> > +     [TRANSMISSION_RATE_125_MBPS] = {
> > +             .t_init = 79801,
> > +             .tclk_miss = 4,
> > +             .tclk_settle = 23,
> > +             .ths_settle = 28,
> > +             .tclk_prepare = 10,
> > +             .ths_prepare = 19,
> > +     },
> > +     [TRANSMISSION_RATE_250_MBPS] = {
> > +             .t_init = 79801,
> > +             .tclk_miss = 4,
> > +             .tclk_settle = 23,
> > +             .ths_settle = 22,
> > +             .tclk_prepare = 10,
> > +             .ths_prepare = 16,
> > +     },
> > +     [TRANSMISSION_RATE_360_MBPS] = {
> > +             .t_init = 79801,
> > +             .tclk_miss = 4,
> > +             .tclk_settle = 18,
> > +             .ths_settle = 19,
> > +             .tclk_prepare = 10,
> > +             .ths_prepare = 10,
> > +     },
> > +     [TRANSMISSION_RATE_360_MBPS_PLUS] = {
> > +             .t_init = 79801,
> > +             .tclk_miss = 4,
> > +             .tclk_settle = 18,
> > +             .ths_settle = 18,
> > +             .tclk_prepare = 10,
> > +             .ths_prepare = 10,
> > +     },
> > +};
> > +
> > +struct rzg2l_csi2_format {
> > +     u32 code;
> > +     unsigned int datatype;
> > +     unsigned int bpp;
> > +};
> > +
> > +static const struct rzg2l_csi2_format rzg2l_csi2_formats[] = {
> > +     { .code = MEDIA_BUS_FMT_UYVY8_1X16,     .datatype = 0x1e, .bpp = 16 },
> > +};
> > +
> > +static const struct rzg2l_csi2_format *rzg2l_csi2_code_to_fmt(unsigned int code)
> > +{
> > +     unsigned int i;
> > +
> > +     for (i = 0; i < ARRAY_SIZE(rzg2l_csi2_formats); i++)
> > +             if (rzg2l_csi2_formats[i].code == code)
> > +                     return &rzg2l_csi2_formats[i];
> > +
> > +     return NULL;
> > +}
> > +
> > +static inline struct rzg2l_csi2 *notifier_to_csi2(struct v4l2_async_notifier *n)
> > +{
> > +     return container_of(n, struct rzg2l_csi2, notifier);
> > +}
> > +
> > +static u32 rzg2l_csi2_read(struct rzg2l_csi2 *csi2, unsigned int reg)
> > +{
> > +     return ioread32(csi2->base + reg);
> > +}
> > +
> > +static void rzg2l_csi2_write(struct rzg2l_csi2 *csi2, unsigned int reg,
> > +                          u32 data)
> > +{
> > +     iowrite32(data, csi2->base + reg);
> > +}
> > +
> > +static void rzg2l_csi2_set(struct rzg2l_csi2 *csi2, unsigned int reg, u32 set)
> > +{
> > +     rzg2l_csi2_write(csi2, reg, rzg2l_csi2_read(csi2, reg) | set);
> > +}
> > +
> > +static void rzg2l_csi2_clr(struct rzg2l_csi2 *csi2, unsigned int reg, u32 clr)
> > +{
> > +     rzg2l_csi2_write(csi2, reg, rzg2l_csi2_read(csi2, reg) & ~clr);
> > +}
> > +
> > +static int rzg2l_csi2_calc_mbps(struct rzg2l_csi2 *csi2)
> > +{
> > +     struct v4l2_subdev *sd = csi2->remote_source;
> > +     const struct rzg2l_csi2_format *format;
> > +     const struct v4l2_mbus_framefmt *fmt;
> > +     struct v4l2_subdev_state *state;
> > +     struct v4l2_ctrl *ctrl;
> > +     u64 mbps;
> > +
> > +     /* Read the pixel rate control from remote. */
> > +     ctrl = v4l2_ctrl_find(sd->ctrl_handler, V4L2_CID_PIXEL_RATE);
> > +     if (!ctrl) {
> > +             dev_err(csi2->dev, "no pixel rate control in subdev %s\n",
> > +                     sd->name);
> > +             return -EINVAL;
> > +     }
> > +
> > +     state = v4l2_subdev_lock_and_get_active_state(&csi2->subdev);
> > +     fmt = v4l2_subdev_get_pad_format(sd, state, RZG2L_CSI2_SINK);
> > +     v4l2_subdev_unlock_state(state);
> > +     format = rzg2l_csi2_code_to_fmt(fmt->code);
>
> You shouldn't access fmt once you've unlocked subdev state.
>
OK, I'll move the line above the unlock.

> > +
> > +     /*
> > +      * Calculate hsfreq in Mbps
> > +      * hsfreq = (pixel_rate * bits_per_sample) / number_of_lanes
> > +      */
> > +     mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * format->bpp;
> > +     do_div(mbps, csi2->lanes * 1000000);
> > +
> > +     return mbps;
> > +}
> > +
> > +int rzg2l_csi2_cmn_rstb_deassert(struct rzg2l_csi2 *csi2)
> > +{
> > +     return reset_control_deassert(csi2->rstc);
> > +}
> > +
> > +/* -----------------------------------------------------------------------------
> > + * DPHY setting
> > + */
> > +
> > +static int rzg2l_csi2_dphy_disable(struct rzg2l_csi2 *csi2)
> > +{
> > +     int ret;
> > +
> > +     /* Reset the CRU (D-PHY) */
> > +     ret = reset_control_assert(csi2->rstc);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /* Stop the D-PHY clock */
> > +     clk_disable_unprepare(csi2->sysclk);
> > +
> > +     /* Cancel the EN_LDO1200 register setting */
> > +     rzg2l_csi2_clr(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_LDO1200);
> > +
> > +     /* Cancel the EN_BGR register setting */
> > +     rzg2l_csi2_clr(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_BGR);
> > +
> > +     return 0;
> > +}
> > +
> > +static int rzg2l_csi2_dphy_enable(struct rzg2l_csi2 *csi2)
> > +{
> > +     const struct rzg2l_csi2_timings *dphy_timing;
> > +     u32 dphytim0, dphytim1;
> > +     int mbps;
> > +     int ret;
> > +
> > +     mbps = rzg2l_csi2_calc_mbps(csi2);
> > +     if (mbps < 0)
> > +             return mbps;
> > +
> > +     csi2->hsfreq = mbps;
> > +
> > +     /* Set DPHY timing parameters */
> > +     if (csi2->hsfreq <= 80)
> > +             dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_80_MBPS];
> > +     else if (csi2->hsfreq <= 125)
> > +             dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_125_MBPS];
> > +     else if (csi2->hsfreq <= 250)
> > +             dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_250_MBPS];
> > +     else if (csi2->hsfreq <= 360)
> > +             dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_360_MBPS];
> > +     else if (csi2->hsfreq <= 1500)
> > +             dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_360_MBPS_PLUS];
> > +     else
> > +             return -EINVAL;
> > +
> > +     /* Set D-PHY timing parameters */
> > +     dphytim0 = CSIDPHYTIM0_TCLK_MISS(dphy_timing->tclk_miss) |
> > +                     CSIDPHYTIM0_T_INIT(dphy_timing->t_init);
> > +     dphytim1 = CSIDPHYTIM1_THS_PREPARE(dphy_timing->ths_prepare) |
> > +                     CSIDPHYTIM1_TCLK_PREPARE(dphy_timing->tclk_prepare) |
> > +                     CSIDPHYTIM1_THS_SETTLE(dphy_timing->ths_settle) |
> > +                     CSIDPHYTIM1_TCLK_SETTLE(dphy_timing->tclk_settle);
> > +     rzg2l_csi2_write(csi2, CSIDPHYTIM0, dphytim0);
> > +     rzg2l_csi2_write(csi2, CSIDPHYTIM1, dphytim1);
> > +
> > +     /* Enable D-PHY power control 0 */
> > +     rzg2l_csi2_write(csi2, CSIDPHYSKW0, CSIDPHYSKW0_DEFAULT_SKW);
> > +
> > +     /* Set the EN_BGR bit */
> > +     rzg2l_csi2_set(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_BGR);
> > +
> > +     /* Delay 20us to be stable */
> > +     usleep_range(20, 40);
> > +
> > +     /* Enable D-PHY power control 1 */
> > +     rzg2l_csi2_set(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_LDO1200);
> > +
> > +     /* Delay 10us to be stable */
> > +     usleep_range(10, 20);
> > +
> > +     /* Start supplying the internal clock for the D-PHY block */
> > +     ret = clk_prepare_enable(csi2->sysclk);
> > +     if (ret)
> > +             rzg2l_csi2_dphy_disable(csi2);
> > +
> > +     return ret;
> > +}
> > +
> > +int rzg2l_csi2_dphy_setting(struct rzg2l_csi2 *csi2, bool on)
> > +{
> > +     if (on)
> > +             return rzg2l_csi2_dphy_enable(csi2);
> > +
> > +     return rzg2l_csi2_dphy_disable(csi2);
> > +}
> > +
> > +static void rzg2l_csi2_mipi_link_enable(struct rzg2l_csi2 *csi2)
> > +{
> > +     unsigned long vclk_rate = clk_get_rate(csi2->vclk) / HZ_PER_MHZ;
> > +     u32 frrskw, frrclk, frrskw_coeff, frrclk_coeff;
> > +
> > +     /* Select data lanes */
> > +     rzg2l_csi2_write(csi2, CSI2nMCT0, CSI2nMCT0_VDLN(csi2->lanes));
> > +
> > +     frrskw_coeff = 3 * vclk_rate * 8;
> > +     frrclk_coeff = frrskw_coeff / 2;
> > +     frrskw = DIV_ROUND_UP(frrskw_coeff, csi2->hsfreq);
> > +     frrclk = DIV_ROUND_UP(frrclk_coeff, csi2->hsfreq);
> > +     rzg2l_csi2_write(csi2, CSI2nMCT2, CSI2nMCT2_FRRSKW(frrskw) |
> > +                      CSI2nMCT2_FRRCLK(frrclk));
> > +
> > +     /*
> > +      * Select data type.
> > +      * FS, FE, LS, LE, Generic Short Packet Codes 1 to 8,
> > +      * Generic Long Packet Data Types 1 to 4 YUV422 8-bit,
> > +      * RGB565, RGB888, RAW8 to RAW20, User-defined 8-bit
> > +      * data types 1 to 8
> > +      */
> > +     rzg2l_csi2_write(csi2, CSI2nDTEL, 0xf778ff0f);
> > +     rzg2l_csi2_write(csi2, CSI2nDTEH, 0x00ffff1f);
> > +
> > +     /* Enable LINK reception */
> > +     rzg2l_csi2_write(csi2, CSI2nMCT3, CSI2nMCT3_RXEN);
> > +}
> > +
> > +static void rzg2l_csi2_mipi_link_disable(struct rzg2l_csi2 *csi2)
> > +{
> > +     unsigned int timeout = VSRSTS_RETRIES;
> > +
> > +     /* Stop LINK reception */
> > +     rzg2l_csi2_clr(csi2, CSI2nMCT3, CSI2nMCT3_RXEN);
> > +
> > +     /* Request a software reset of the LINK Video Pixel Interface */
> > +     rzg2l_csi2_write(csi2, CSI2nRTCT, CSI2nRTCT_VSRST);
> > +
> > +     /* Make sure CSI2nRTST.VSRSTS bit is cleared */
> > +     while (timeout--) {
> > +             if (!(rzg2l_csi2_read(csi2, CSI2nRTST) & CSI2nRTST_VSRSTS))
> > +                     break;
> > +             usleep_range(100, 200);
> > +     };
> > +
> > +     if (!timeout)
> > +             dev_err(csi2->dev, "Clearing CSI2nRTST.VSRSTS timed out\n");
> > +}
> > +
> > +void rzg2l_csi2_mipi_link_setting(struct rzg2l_csi2 *csi2, bool on)
> > +{
> > +     if (on)
> > +             rzg2l_csi2_mipi_link_enable(csi2);
> > +     else
> > +             rzg2l_csi2_mipi_link_disable(csi2);
> > +}
> > +
> > +static int rzg2l_csi2_s_stream(struct v4l2_subdev *sd, int enable)
> > +{
> > +     struct rzg2l_csi2 *csi2 = sd_to_csi2(sd);
> > +
> > +     return v4l2_subdev_call(csi2->remote_source, video, s_stream, enable);
> > +}
> > +
> > +static int rzg2l_csi2_set_format(struct v4l2_subdev *sd,
> > +                              struct v4l2_subdev_state *state,
> > +                              struct v4l2_subdev_format *fmt)
> > +{
> > +     struct v4l2_mbus_framefmt *src_format;
> > +     struct v4l2_mbus_framefmt *format;
> > +
> > +     if (fmt->pad > RZG2L_CSI2_SINK)
> > +             return -EINVAL;
> > +
> > +     format = v4l2_subdev_get_pad_format(sd, state, fmt->pad);
> > +
> > +     if (!rzg2l_csi2_code_to_fmt(fmt->format.code))
> > +             format->code = rzg2l_csi2_formats[0].code;
> > +     else
> > +             format->code = fmt->format.code;
> > +
> > +     format->field = V4L2_FIELD_NONE;
> > +     format->colorspace = V4L2_COLORSPACE_SRGB;
> > +     format->width = fmt->format.width;
> > +     format->height = fmt->format.height;
> > +     fmt->format = *format;
> > +
> > +     /* propagate format to source pad */
> > +     src_format = v4l2_subdev_get_pad_format(sd, state, RZG2L_CSI2_SOURCE_VC0);
> > +     *src_format = *format;
> > +
> > +     return 0;
> > +}
> > +
> > +static int rzg2l_csi2_init_config(struct v4l2_subdev *sd,
> > +                               struct v4l2_subdev_state *sd_state)
> > +{
> > +     struct v4l2_subdev_format fmt = { 0 };
>
> You could do this in variable initialisation.
>
Agreed, I will set the pad here.

> > +
> > +     fmt.which = sd_state ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
> > +     fmt.format.width = RZG2L_CSI2_DEFAULT_WIDTH;
> > +     fmt.format.height = RZG2L_CSI2_DEFAULT_HEIGHT;
> > +     fmt.format.field = V4L2_FIELD_NONE;
> > +     fmt.format.code = RZG2L_CSI2_DEFAULT_FMT;
> > +     fmt.format.colorspace = V4L2_COLORSPACE_SRGB;
> > +
> > +     return rzg2l_csi2_set_format(sd, sd_state, &fmt);
> > +}
> > +
> > +static int rzg2l_csi2_enum_mbus_code(struct v4l2_subdev *sd,
> > +                                  struct v4l2_subdev_state *sd_state,
> > +                                  struct v4l2_subdev_mbus_code_enum *code)
> > +{
> > +     if (code->index >= ARRAY_SIZE(rzg2l_csi2_formats))
> > +             return -EINVAL;
> > +
> > +     code->code = rzg2l_csi2_formats[code->index].code;
> > +
> > +     return 0;
> > +}
> > +
> > +static int rzg2l_csi2_enum_frame_size(struct v4l2_subdev *sd,
> > +                                   struct v4l2_subdev_state *sd_state,
> > +                                   struct v4l2_subdev_frame_size_enum *fse)
> > +{
> > +     if (fse->index != 0)
> > +             return -EINVAL;
> > +
> > +     fse->min_width = clamp_t(u32, 1, 320, 2800);
> > +     fse->min_height = clamp_t(u32, 1, 240, 4095);
> > +     fse->max_width = clamp_t(u32, -1, 320, 2800);
> > +     fse->max_height = clamp_t(u32, -1, 240, 4095);
> > +
> > +     return 0;
> > +}
> > +
> > +static const struct v4l2_subdev_video_ops rzg2l_csi2_video_ops = {
> > +     .s_stream = rzg2l_csi2_s_stream,
> > +};
> > +
> > +static const struct v4l2_subdev_pad_ops rzg2l_csi2_pad_ops = {
> > +     .enum_mbus_code = rzg2l_csi2_enum_mbus_code,
> > +     .init_cfg = rzg2l_csi2_init_config,
> > +     .enum_frame_size = rzg2l_csi2_enum_frame_size,
> > +     .set_fmt = rzg2l_csi2_set_format,
> > +     .get_fmt = v4l2_subdev_get_fmt,
> > +};
> > +
> > +static const struct v4l2_subdev_ops rzg2l_csi2_subdev_ops = {
> > +     .video  = &rzg2l_csi2_video_ops,
> > +     .pad    = &rzg2l_csi2_pad_ops,
> > +};
> > +
> > +/* -----------------------------------------------------------------------------
> > + * Async handling and registration of subdevices and links.
> > + */
> > +
> > +static int rzg2l_csi2_notify_bound(struct v4l2_async_notifier *notifier,
> > +                                struct v4l2_subdev *subdev,
> > +                                struct v4l2_async_subdev *asd)
> > +{
> > +     struct rzg2l_csi2 *csi2 = notifier_to_csi2(notifier);
> > +
> > +     csi2->remote_source = subdev;
> > +
> > +     dev_dbg(csi2->dev, "Bound subdev: %s pad\n", subdev->name);
> > +
> > +     return media_create_pad_link(&subdev->entity, RZG2L_CSI2_SINK,
> > +                                  &csi2->subdev.entity, 0,
> > +                                  MEDIA_LNK_FL_ENABLED |
> > +                                  MEDIA_LNK_FL_IMMUTABLE);
> > +}
> > +
> > +static void rzg2l_csi2_notify_unbind(struct v4l2_async_notifier *notifier,
> > +                                  struct v4l2_subdev *subdev,
> > +                                  struct v4l2_async_subdev *asd)
> > +{
> > +     struct rzg2l_csi2 *csi2 = notifier_to_csi2(notifier);
> > +
> > +     csi2->remote_source = NULL;
> > +
> > +     dev_dbg(csi2->dev, "Unbind subdev %s\n", subdev->name);
> > +}
> > +
> > +static const struct v4l2_async_notifier_operations rzg2l_csi2_notify_ops = {
> > +     .bound = rzg2l_csi2_notify_bound,
> > +     .unbind = rzg2l_csi2_notify_unbind,
> > +};
> > +
> > +static int rzg2l_csi2_parse_v4l2(struct rzg2l_csi2 *csi2,
> > +                              struct v4l2_fwnode_endpoint *vep)
> > +{
> > +     /* Only port 0 endpoint 0 is valid. */
> > +     if (vep->base.port || vep->base.id)
> > +             return -ENOTCONN;
> > +
> > +     csi2->lanes = vep->bus.mipi_csi2.num_data_lanes;
> > +
> > +     return 0;
> > +}
> > +
> > +static int rzg2l_csi2_parse_dt(struct rzg2l_csi2 *csi2)
> > +{
> > +     struct v4l2_fwnode_endpoint v4l2_ep = {
> > +             .bus_type = V4L2_MBUS_CSI2_DPHY
> > +     };
> > +     struct v4l2_async_subdev *asd;
> > +     struct fwnode_handle *fwnode;
> > +     struct fwnode_handle *ep;
> > +     int ret;
> > +
> > +     ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(csi2->dev), 0, 0, 0);
> > +     if (!ep) {
> > +             dev_err(csi2->dev, "Not connected to subdevice\n");
> > +             return -EINVAL;
> > +     }
> > +
> > +     ret = v4l2_fwnode_endpoint_parse(ep, &v4l2_ep);
> > +     if (ret) {
> > +             dev_err(csi2->dev, "Could not parse v4l2 endpoint\n");
> > +             fwnode_handle_put(ep);
> > +             return -EINVAL;
> > +     }
> > +
> > +     ret = rzg2l_csi2_parse_v4l2(csi2, &v4l2_ep);
> > +     if (ret) {
> > +             fwnode_handle_put(ep);
> > +             return ret;
> > +     }
> > +
> > +     fwnode = fwnode_graph_get_remote_endpoint(ep);
> > +     fwnode_handle_put(ep);
> > +
> > +     dev_dbg(csi2->dev, "Found '%pOF'\n", to_of_node(fwnode));
>
> You can enable debug message printing in v4l2 fwnode, and sysfs now has
> some, too. I'd suggest dropping this line .
>
Agreed, I will drop it.

> > +
> > +     v4l2_async_nf_init(&csi2->notifier);
> > +     csi2->notifier.ops = &rzg2l_csi2_notify_ops;
> > +
> > +     asd = v4l2_async_nf_add_fwnode(&csi2->notifier, fwnode,
> > +                                    struct v4l2_async_subdev);
> > +     fwnode_handle_put(fwnode);
> > +     if (IS_ERR(asd))
> > +             return PTR_ERR(asd);
> > +
> > +     ret = v4l2_async_subdev_nf_register(&csi2->subdev, &csi2->notifier);
> > +     if (ret)
> > +             v4l2_async_nf_cleanup(&csi2->notifier);
> > +
> > +     return ret;
> > +}
> > +
> > +static int rzg2l_validate_csi2_lanes(struct rzg2l_csi2 *csi2)
> > +{
> > +     int ret = 0;
> > +     int lanes;
> > +
> > +     if (csi2->lanes != 1 && csi2->lanes != 2 && csi2->lanes != 4) {
> > +             dev_err(csi2->dev, "Unsupported number of data-lanes: %u\n",
> > +                     csi2->lanes);
> > +             return -EINVAL;
> > +     }
> > +
> > +     ret = clk_prepare_enable(csi2->pclk);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /* Checking the maximum lanes support for CSI-2 module */
> > +     lanes = (rzg2l_csi2_read(csi2, CSI2nMCG) & CSI2nMCG_SDLN) >> 8;
> > +     if (lanes < csi2->lanes) {
> > +             dev_err(csi2->dev,
> > +                     "Failed to support %d data lanes\n", csi2->lanes);
> > +             ret = -EINVAL;
> > +     }
> > +
> > +     clk_disable_unprepare(csi2->pclk);
> > +
> > +     return ret;
> > +}
> > +
> > +/* -----------------------------------------------------------------------------
> > + * Platform Device Driver.
> > + */
> > +
> > +static const struct media_entity_operations rzg2l_csi2_entity_ops = {
> > +     .link_validate = v4l2_subdev_link_validate,
> > +};
> > +
> > +static int rzg2l_csi2_probe(struct platform_device *pdev)
> > +{
> > +     struct rzg2l_csi2 *csi2;
> > +     int ret;
> > +
> > +     csi2 = devm_kzalloc(&pdev->dev, sizeof(*csi2), GFP_KERNEL);
> > +     if (!csi2)
> > +             return -ENOMEM;
> > +
> > +     csi2->base = devm_platform_ioremap_resource(pdev, 0);
> > +     if (IS_ERR(csi2->base))
> > +             return PTR_ERR(csi2->base);
> > +
> > +     csi2->rstc = devm_reset_control_get(&pdev->dev, "cmn-rstb");
> > +     if (IS_ERR(csi2->rstc))
> > +             return dev_err_probe(&pdev->dev, PTR_ERR(csi2->rstc),
> > +                                  "failed to get cpg cmn-rstb\n");
> > +
> > +     csi2->sysclk = devm_clk_get(&pdev->dev, "sysclk");
> > +     if (IS_ERR(csi2->sysclk)) {
> > +             dev_err(&pdev->dev, "Failed to get sysclk");
> > +             return PTR_ERR(csi2->sysclk);
> > +     }
> > +
> > +     csi2->vclk = devm_clk_get(&pdev->dev, "vclk");
> > +     if (IS_ERR(csi2->vclk)) {
> > +             dev_err(&pdev->dev, "Failed to get vclk");
> > +             return PTR_ERR(csi2->vclk);
> > +     }
> > +
> > +     csi2->pclk = devm_clk_get(&pdev->dev, "pclk");
> > +     if (IS_ERR(csi2->pclk)) {
> > +             dev_err(&pdev->dev, "Failed to get pclk");
> > +             return PTR_ERR(csi2->pclk);
> > +     }
> > +
> > +     csi2->dev = &pdev->dev;
> > +
> > +     platform_set_drvdata(pdev, csi2);
> > +
> > +     ret = rzg2l_csi2_parse_dt(csi2);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = rzg2l_validate_csi2_lanes(csi2);
> > +     if (ret)
> > +             return ret;
> > +
> > +     csi2->subdev.dev = &pdev->dev;
> > +     v4l2_subdev_init(&csi2->subdev, &rzg2l_csi2_subdev_ops);
> > +     v4l2_set_subdevdata(&csi2->subdev, &pdev->dev);
> > +     snprintf(csi2->subdev.name, sizeof(csi2->subdev.name),
> > +              "csi-%s", dev_name(&pdev->dev));
> > +     csi2->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> > +
> > +     csi2->subdev.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
> > +     csi2->subdev.entity.ops = &rzg2l_csi2_entity_ops;
> > +
> > +     csi2->pads[RZG2L_CSI2_SINK].flags = MEDIA_PAD_FL_SINK;
> > +     /*
> > +      * TODO: RZ/G2L CSI2 supports 4 virtual channels, as virtual
> > +      * channels should be implemented by streams API which is under
> > +      * development lets hardcode to VC0 for now.
> > +      */
> > +     csi2->pads[RZG2L_CSI2_SOURCE_VC0].flags = MEDIA_PAD_FL_SOURCE;
> > +     ret = media_entity_pads_init(&csi2->subdev.entity, 2, csi2->pads);
> > +     if (ret)
> > +             goto error_async;
> > +
> > +     pm_runtime_enable(&pdev->dev);
> > +
> > +     ret = v4l2_subdev_init_finalize(&csi2->subdev);
> > +     if (ret < 0)
> > +             goto error_pm;
> > +
> > +     /* enable pclk for register access */
> > +     ret = clk_prepare_enable(csi2->pclk);
> > +     if (ret)
> > +             goto error_pm;
> > +
> > +     ret = v4l2_async_register_subdev(&csi2->subdev);
> > +     if (ret < 0)
> > +             goto error_subdev;
> > +
> > +     dev_info(csi2->dev, "%d lanes found\n", csi2->lanes);
>
> This is also printed if you enable debug level prints in v4l2 fwnode.
>
Agreed, I will drop it.

> > +
> > +     return 0;
> > +
> > +error_subdev:
> > +     clk_disable_unprepare(csi2->pclk);
> > +     v4l2_subdev_cleanup(&csi2->subdev);
> > +error_pm:
> > +     pm_runtime_disable(&pdev->dev);
> > +error_async:
> > +     v4l2_async_nf_unregister(&csi2->notifier);
> > +     v4l2_async_nf_cleanup(&csi2->notifier);
> > +
> > +     return ret;
> > +}
> > +
> > +static const struct of_device_id rzg2l_csi2_of_table[] = {
> > +     { .compatible = "renesas,rzg2l-csi2", },
> > +     { /* sentinel */ }
> > +};
> > +
> > +static int rzg2l_csi2_remove(struct platform_device *pdev)
> > +{
> > +     struct rzg2l_csi2 *csi2 = platform_get_drvdata(pdev);
> > +
> > +     v4l2_async_nf_unregister(&csi2->notifier);
> > +     v4l2_async_nf_cleanup(&csi2->notifier);
> > +     v4l2_async_unregister_subdev(&csi2->subdev);
> > +     clk_disable_unprepare(csi2->pclk);
> > +     v4l2_subdev_cleanup(&csi2->subdev);
> > +
> > +     pm_runtime_disable(&pdev->dev);
> > +
> > +     return 0;
> > +}
> > +
> > +static struct platform_driver rzg2l_csi2_pdrv = {
> > +     .remove = rzg2l_csi2_remove,
> > +     .probe  = rzg2l_csi2_probe,
> > +     .driver = {
> > +             .name = "rzg2l-csi2",
> > +             .of_match_table = rzg2l_csi2_of_table,
> > +     },
> > +};
> > +
> > +module_platform_driver(rzg2l_csi2_pdrv);
> > +
> > +MODULE_AUTHOR("Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>");
> > +MODULE_DESCRIPTION("Renesas RZ/G2L MIPI CSI2 receiver driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h
> > new file mode 100644
> > index 000000000000..f85a1d44250f
> > --- /dev/null
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h
> > @@ -0,0 +1,46 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Copyright (C) 2022 Renesas Electronics Corp.
> > + */
> > +
> > +#ifndef __RZG2L_CSI2__
> > +#define __RZG2L_CSI2__
> > +
> > +enum rzg2l_csi2_pads {
> > +     RZG2L_CSI2_SINK = 0,
> > +     RZG2L_CSI2_SOURCE_VC0,
> > +     RZG2L_CSI2_SOURCE_VC1,
> > +     RZG2L_CSI2_SOURCE_VC2,
> > +     RZG2L_CSI2_SOURCE_VC3,
> > +     NR_OF_RZG2L_CSI2_PAD,
> > +};
> > +
> > +struct rzg2l_csi2 {
> > +     struct device *dev;
> > +     void __iomem *base;
> > +     struct reset_control *rstc;
> > +     struct clk *sysclk;
> > +     struct clk *vclk;
> > +     struct clk *pclk;
> > +
> > +     struct v4l2_subdev subdev;
> > +     struct media_pad pads[NR_OF_RZG2L_CSI2_PAD];
> > +
> > +     struct v4l2_async_notifier notifier;
> > +     struct v4l2_subdev *remote_source;
> > +
> > +     unsigned short lanes;
> > +
> > +     unsigned long hsfreq;
> > +};
> > +
> > +static inline struct rzg2l_csi2 *sd_to_csi2(struct v4l2_subdev *sd)
> > +{
> > +     return container_of(sd, struct rzg2l_csi2, subdev);
> > +}
> > +
> > +int rzg2l_csi2_cmn_rstb_deassert(struct rzg2l_csi2 *csi2);
> > +int rzg2l_csi2_dphy_setting(struct rzg2l_csi2 *csi2, bool on);
> > +void rzg2l_csi2_mipi_link_setting(struct rzg2l_csi2 *csi2, bool on);
>
> Are these something that could be achieved using the standard interfaces,
> as I believe the other drivers are doing? The pre_streamon and
> post_streamon callbacks could be relevant for this.
>
Thanks for the pointer. I have now moved some code to pre_streamon ()
and rest to s_stream(). Is there any mandatory rule to have both
implemented? (as I wont be needing post_streamoff(), nothing complains
so for)

Cheers,
Prabhakar
Sakari Ailus Sept. 22, 2022, 1:02 p.m. UTC | #7
Hi Prabhakar,

On Thu, Sep 22, 2022 at 01:53:49PM +0100, Lad, Prabhakar wrote:
> > > +int rzg2l_csi2_cmn_rstb_deassert(struct rzg2l_csi2 *csi2);
> > > +int rzg2l_csi2_dphy_setting(struct rzg2l_csi2 *csi2, bool on);
> > > +void rzg2l_csi2_mipi_link_setting(struct rzg2l_csi2 *csi2, bool on);
> >
> > Are these something that could be achieved using the standard interfaces,
> > as I believe the other drivers are doing? The pre_streamon and
> > post_streamon callbacks could be relevant for this.
> >
> Thanks for the pointer. I have now moved some code to pre_streamon ()
> and rest to s_stream(). Is there any mandatory rule to have both
> implemented? (as I wont be needing post_streamoff(), nothing complains
> so for)

In principle no.

But if you e.g. resume the device in pre_streamon, you'll need to suspend
it in post_streamon.
Prabhakar Sept. 22, 2022, 1:12 p.m. UTC | #8
Hi Sakari,

On Thu, Sep 22, 2022 at 1:34 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Prabhakar,
>
> On Thu, Sep 22, 2022 at 01:08:33PM +0100, Lad, Prabhakar wrote:
> > > > * Switched to manually turn ON/OFF the clocks instead of pm_runtime so that
> > > >   the mipi/dhpy initialization happens as per the HW manual
> > >
> > > That doesn't look right. The driver doesn't use runtime PM anymore, so
> > > power domains may not be handled properly. What was the problem with
> > > clock handling using runtime PM ?
> > >
> > If we use the runtime PM all the clocks will be turned ON when we call
> > pm_runtime_resume_and_get() which I dont want to. As per the "Starting
> > reception for MIPI CSI-2 Input" section 35.3.1 for example we first
> > need to turn ON all the clocks and later further down the line we need
> > to just turn OFF VCLK -> Enable Link -> turn ON VCLK. Due to such
> > cases I have switched to individual clock handling.
>
> If that is the case, then you should control just that clock directly,
> outside runtime PM callbacks.
>
OK, I'll move the control outside runtime PM.

> Runtime PM may be needed e.g. for resuming a parent device.
>
Agreed.

Cheers,
Prabhakar
Prabhakar Sept. 22, 2022, 1:16 p.m. UTC | #9
Hi Sakari,

On Thu, Sep 22, 2022 at 2:02 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Prabhakar,
>
> On Thu, Sep 22, 2022 at 01:53:49PM +0100, Lad, Prabhakar wrote:
> > > > +int rzg2l_csi2_cmn_rstb_deassert(struct rzg2l_csi2 *csi2);
> > > > +int rzg2l_csi2_dphy_setting(struct rzg2l_csi2 *csi2, bool on);
> > > > +void rzg2l_csi2_mipi_link_setting(struct rzg2l_csi2 *csi2, bool on);
> > >
> > > Are these something that could be achieved using the standard interfaces,
> > > as I believe the other drivers are doing? The pre_streamon and
> > > post_streamon callbacks could be relevant for this.
> > >
> > Thanks for the pointer. I have now moved some code to pre_streamon ()
> > and rest to s_stream(). Is there any mandatory rule to have both
> > implemented? (as I wont be needing post_streamoff(), nothing complains
> > so for)
>
> In principle no.
>
OK.

> But if you e.g. resume the device in pre_streamon, you'll need to suspend
> it in post_streamon.
>
Agreed!

Cheers,
Prabhakar
Prabhakar Sept. 22, 2022, 1:27 p.m. UTC | #10
Hi Geert,

On Thu, Sep 22, 2022 at 1:51 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> On Thu, Sep 22, 2022 at 2:34 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> > On Thu, Sep 22, 2022 at 01:08:33PM +0100, Lad, Prabhakar wrote:
> > > > > * Switched to manually turn ON/OFF the clocks instead of pm_runtime so that
> > > > >   the mipi/dhpy initialization happens as per the HW manual
> > > >
> > > > That doesn't look right. The driver doesn't use runtime PM anymore, so
> > > > power domains may not be handled properly. What was the problem with
> > > > clock handling using runtime PM ?
> > > >
> > > If we use the runtime PM all the clocks will be turned ON when we call
> > > pm_runtime_resume_and_get() which I dont want to. As per the "Starting
> > > reception for MIPI CSI-2 Input" section 35.3.1 for example we first
> > > need to turn ON all the clocks and later further down the line we need
> > > to just turn OFF VCLK -> Enable Link -> turn ON VCLK. Due to such
> > > cases I have switched to individual clock handling.
> >
> > If that is the case, then you should control just that clock directly,
> > outside runtime PM callbacks.
> >
> > Runtime PM may be needed e.g. for resuming a parent device.
>
> Exactly.
> So probably you should not consider R9A07G044_CRU_VCLK a PM clock,
> i.e. you need changes to rzg2l_cpg_is_pm_clk() to exclude it.
>
Thanks for the pointer. In that case we will have to consider
R9A07G044_CRU_VCLK and R9A07G044_CRU_SYSCLK as not PM clocks.

Does the below sound good?
- DEF_NO_PM() macro
- bool is_pm_clk in struct rzg2l_mod_clk.

I still have to implement it, just wanted your opinion beforehand.

Cheers,
Prabhakar
Laurent Pinchart Sept. 22, 2022, 1:51 p.m. UTC | #11
On Thu, Sep 22, 2022 at 02:27:15PM +0100, Lad, Prabhakar wrote:
> On Thu, Sep 22, 2022 at 1:51 PM Geert Uytterhoeven wrote:
> > On Thu, Sep 22, 2022 at 2:34 PM Sakari Ailus wrote:
> > > On Thu, Sep 22, 2022 at 01:08:33PM +0100, Lad, Prabhakar wrote:
> > > > > > * Switched to manually turn ON/OFF the clocks instead of pm_runtime so that
> > > > > >   the mipi/dhpy initialization happens as per the HW manual
> > > > >
> > > > > That doesn't look right. The driver doesn't use runtime PM anymore, so
> > > > > power domains may not be handled properly. What was the problem with
> > > > > clock handling using runtime PM ?
> > > > >
> > > > If we use the runtime PM all the clocks will be turned ON when we call
> > > > pm_runtime_resume_and_get() which I dont want to. As per the "Starting
> > > > reception for MIPI CSI-2 Input" section 35.3.1 for example we first
> > > > need to turn ON all the clocks and later further down the line we need
> > > > to just turn OFF VCLK -> Enable Link -> turn ON VCLK. Due to such
> > > > cases I have switched to individual clock handling.
> > >
> > > If that is the case, then you should control just that clock directly,
> > > outside runtime PM callbacks.
> > >
> > > Runtime PM may be needed e.g. for resuming a parent device.
> >
> > Exactly.
> > So probably you should not consider R9A07G044_CRU_VCLK a PM clock,
> > i.e. you need changes to rzg2l_cpg_is_pm_clk() to exclude it.
> 
> Thanks for the pointer. In that case we will have to consider
> R9A07G044_CRU_VCLK and R9A07G044_CRU_SYSCLK as not PM clocks.

I like when patch review discussions continue on the list without my
involvement, and the end result is exactly what I would have advised :-)

> Does the below sound good?
> - DEF_NO_PM() macro
> - bool is_pm_clk in struct rzg2l_mod_clk.
> 
> I still have to implement it, just wanted your opinion beforehand.

Looks good to me, but I'm no expert in this area. I trust Geert's
advice.
Geert Uytterhoeven Sept. 22, 2022, 7:13 p.m. UTC | #12
Hi Prabhakar,

On Thu, Sep 22, 2022 at 3:27 PM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
> On Thu, Sep 22, 2022 at 1:51 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Thu, Sep 22, 2022 at 2:34 PM Sakari Ailus
> > <sakari.ailus@linux.intel.com> wrote:
> > > On Thu, Sep 22, 2022 at 01:08:33PM +0100, Lad, Prabhakar wrote:
> > > > > > * Switched to manually turn ON/OFF the clocks instead of pm_runtime so that
> > > > > >   the mipi/dhpy initialization happens as per the HW manual
> > > > >
> > > > > That doesn't look right. The driver doesn't use runtime PM anymore, so
> > > > > power domains may not be handled properly. What was the problem with
> > > > > clock handling using runtime PM ?
> > > > >
> > > > If we use the runtime PM all the clocks will be turned ON when we call
> > > > pm_runtime_resume_and_get() which I dont want to. As per the "Starting
> > > > reception for MIPI CSI-2 Input" section 35.3.1 for example we first
> > > > need to turn ON all the clocks and later further down the line we need
> > > > to just turn OFF VCLK -> Enable Link -> turn ON VCLK. Due to such
> > > > cases I have switched to individual clock handling.
> > >
> > > If that is the case, then you should control just that clock directly,
> > > outside runtime PM callbacks.
> > >
> > > Runtime PM may be needed e.g. for resuming a parent device.
> >
> > Exactly.
> > So probably you should not consider R9A07G044_CRU_VCLK a PM clock,
> > i.e. you need changes to rzg2l_cpg_is_pm_clk() to exclude it.
> >
> Thanks for the pointer. In that case we will have to consider
> R9A07G044_CRU_VCLK and R9A07G044_CRU_SYSCLK as not PM clocks.
>
> Does the below sound good?
> - DEF_NO_PM() macro
> - bool is_pm_clk in struct rzg2l_mod_clk.
>
> I still have to implement it, just wanted your opinion beforehand.

LGTM. Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
diff mbox series

Patch

diff --git a/drivers/media/platform/renesas/Kconfig b/drivers/media/platform/renesas/Kconfig
index 9fd90672ea2d..ed788e991f74 100644
--- a/drivers/media/platform/renesas/Kconfig
+++ b/drivers/media/platform/renesas/Kconfig
@@ -41,6 +41,7 @@  config VIDEO_SH_VOU
 	  Support for the Video Output Unit (VOU) on SuperH SoCs.
 
 source "drivers/media/platform/renesas/rcar-vin/Kconfig"
+source "drivers/media/platform/renesas/rzg2l-cru/Kconfig"
 
 # Mem2mem drivers
 
diff --git a/drivers/media/platform/renesas/Makefile b/drivers/media/platform/renesas/Makefile
index 3ec226ef5fd2..55854e868887 100644
--- a/drivers/media/platform/renesas/Makefile
+++ b/drivers/media/platform/renesas/Makefile
@@ -4,6 +4,7 @@ 
 #
 
 obj-y += rcar-vin/
+obj-y += rzg2l-cru/
 obj-y += vsp1/
 
 obj-$(CONFIG_VIDEO_RCAR_DRIF) += rcar_drif.o
diff --git a/drivers/media/platform/renesas/rzg2l-cru/Kconfig b/drivers/media/platform/renesas/rzg2l-cru/Kconfig
new file mode 100644
index 000000000000..57c40bb499df
--- /dev/null
+++ b/drivers/media/platform/renesas/rzg2l-cru/Kconfig
@@ -0,0 +1,17 @@ 
+# SPDX-License-Identifier: GPL-2.0
+
+config VIDEO_RZG2L_CSI2
+	tristate "RZ/G2L MIPI CSI-2 Receiver"
+	depends on ARCH_RENESAS || COMPILE_TEST
+	depends on V4L_PLATFORM_DRIVERS
+	depends on VIDEO_DEV && OF
+	select MEDIA_CONTROLLER
+	select RESET_CONTROLLER
+	select V4L2_FWNODE
+	select VIDEO_V4L2_SUBDEV_API
+	help
+	  Support for Renesas RZ/G2L (and alike SoC's) MIPI CSI-2
+	  Receiver driver.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called rzg2l-csi2.
diff --git a/drivers/media/platform/renesas/rzg2l-cru/Makefile b/drivers/media/platform/renesas/rzg2l-cru/Makefile
new file mode 100644
index 000000000000..91ea97a944e6
--- /dev/null
+++ b/drivers/media/platform/renesas/rzg2l-cru/Makefile
@@ -0,0 +1,3 @@ 
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_VIDEO_RZG2L_CSI2) += rzg2l-csi2.o
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
new file mode 100644
index 000000000000..1f6838ed64fc
--- /dev/null
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
@@ -0,0 +1,761 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Renesas RZ/G2L MIPI CSI-2 Receiver
+ *
+ * Copyright (C) 2022 Renesas Electronics Corp.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_graph.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+#include <linux/sys_soc.h>
+#include <linux/units.h>
+
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-mc.h>
+#include <media/v4l2-subdev.h>
+
+#include "rzg2l-csi2.h"
+
+/* LINK registers */
+/* Module Configuration Register */
+#define CSI2nMCG			0x0
+#define CSI2nMCG_SDLN			GENMASK(11, 8)
+
+/* Module Control Register 0 */
+#define CSI2nMCT0			0x10
+#define CSI2nMCT0_VDLN(x)		((x) << 0)
+
+/* Module Control Register 2 */
+#define CSI2nMCT2			0x18
+#define CSI2nMCT2_FRRSKW(x)		((x) << 16)
+#define CSI2nMCT2_FRRCLK(x)		((x) << 0)
+
+/* Module Control Register 3 */
+#define CSI2nMCT3			0x1c
+#define CSI2nMCT3_RXEN			BIT(0)
+
+/* Reset Control Register */
+#define CSI2nRTCT			0x28
+#define CSI2nRTCT_VSRST			BIT(0)
+
+/* Reset Status Register */
+#define CSI2nRTST			0x2c
+#define CSI2nRTST_VSRSTS		BIT(0)
+
+/* Receive Data Type Enable Low Register */
+#define CSI2nDTEL			0x60
+
+/* Receive Data Type Enable High Register */
+#define CSI2nDTEH			0x64
+
+/* Power Management Status Register */
+#define CSI2nPMST			0x200
+
+/* Power Management Status Clear Register */
+#define CSI2nPMSC			0x204
+
+/* DPHY registers */
+/* D-PHY Control Register 0 */
+#define CSIDPHYCTRL0			0x400
+#define CSIDPHYCTRL0_EN_LDO1200		BIT(1)
+#define CSIDPHYCTRL0_EN_BGR		BIT(0)
+
+/* D-PHY Timing Register 0 */
+#define CSIDPHYTIM0			0x404
+#define CSIDPHYTIM0_TCLK_MISS(x)	((x) << 24)
+#define CSIDPHYTIM0_T_INIT(x)		((x) << 0)
+
+/* D-PHY Timing Register 1 */
+#define CSIDPHYTIM1			0x408
+#define CSIDPHYTIM1_THS_PREPARE(x)	((x) << 24)
+#define CSIDPHYTIM1_TCLK_PREPARE(x)	((x) << 16)
+#define CSIDPHYTIM1_THS_SETTLE(x)	((x) << 8)
+#define CSIDPHYTIM1_TCLK_SETTLE(x)	((x) << 0)
+
+/* D-PHY Skew Adjustment Function */
+#define CSIDPHYSKW0			0x460
+#define CSIDPHYSKW0_UTIL_DL0_SKW_ADJ(x)	((x) & 0x3)
+#define CSIDPHYSKW0_UTIL_DL1_SKW_ADJ(x)	(((x) & 0x3) << 4)
+#define CSIDPHYSKW0_UTIL_DL2_SKW_ADJ(x)	(((x) & 0x3) << 8)
+#define CSIDPHYSKW0_UTIL_DL3_SKW_ADJ(x)	(((x) & 0x3) << 12)
+#define CSIDPHYSKW0_DEFAULT_SKW		CSIDPHYSKW0_UTIL_DL0_SKW_ADJ(1) | \
+					CSIDPHYSKW0_UTIL_DL1_SKW_ADJ(1) | \
+					CSIDPHYSKW0_UTIL_DL2_SKW_ADJ(1) | \
+					CSIDPHYSKW0_UTIL_DL3_SKW_ADJ(1)
+
+#define VSRSTS_RETRIES			20
+
+#define RZG2L_CSI2_DEFAULT_WIDTH	800
+#define RZG2L_CSI2_DEFAULT_HEIGHT	600
+#define RZG2L_CSI2_DEFAULT_FMT		MEDIA_BUS_FMT_UYVY8_1X16
+
+struct rzg2l_csi2_timings {
+	u32 t_init;
+	u32 tclk_miss;
+	u32 tclk_settle;
+	u32 ths_settle;
+	u32 tclk_prepare;
+	u32 ths_prepare;
+};
+
+enum rzg2l_dphy_timings {
+	TRANSMISSION_RATE_80_MBPS = 0,
+	TRANSMISSION_RATE_125_MBPS,
+	TRANSMISSION_RATE_250_MBPS,
+	TRANSMISSION_RATE_360_MBPS,
+	TRANSMISSION_RATE_360_MBPS_PLUS,
+};
+
+static const struct rzg2l_csi2_timings rzg2l_csi2_global_timings[] = {
+	[TRANSMISSION_RATE_80_MBPS] = {
+		.t_init = 79801,
+		.tclk_miss = 4,
+		.tclk_settle = 23,
+		.ths_settle = 31,
+		.tclk_prepare = 10,
+		.ths_prepare = 19,
+	},
+	[TRANSMISSION_RATE_125_MBPS] = {
+		.t_init = 79801,
+		.tclk_miss = 4,
+		.tclk_settle = 23,
+		.ths_settle = 28,
+		.tclk_prepare = 10,
+		.ths_prepare = 19,
+	},
+	[TRANSMISSION_RATE_250_MBPS] = {
+		.t_init = 79801,
+		.tclk_miss = 4,
+		.tclk_settle = 23,
+		.ths_settle = 22,
+		.tclk_prepare = 10,
+		.ths_prepare = 16,
+	},
+	[TRANSMISSION_RATE_360_MBPS] = {
+		.t_init = 79801,
+		.tclk_miss = 4,
+		.tclk_settle = 18,
+		.ths_settle = 19,
+		.tclk_prepare = 10,
+		.ths_prepare = 10,
+	},
+	[TRANSMISSION_RATE_360_MBPS_PLUS] = {
+		.t_init = 79801,
+		.tclk_miss = 4,
+		.tclk_settle = 18,
+		.ths_settle = 18,
+		.tclk_prepare = 10,
+		.ths_prepare = 10,
+	},
+};
+
+struct rzg2l_csi2_format {
+	u32 code;
+	unsigned int datatype;
+	unsigned int bpp;
+};
+
+static const struct rzg2l_csi2_format rzg2l_csi2_formats[] = {
+	{ .code = MEDIA_BUS_FMT_UYVY8_1X16,	.datatype = 0x1e, .bpp = 16 },
+};
+
+static const struct rzg2l_csi2_format *rzg2l_csi2_code_to_fmt(unsigned int code)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(rzg2l_csi2_formats); i++)
+		if (rzg2l_csi2_formats[i].code == code)
+			return &rzg2l_csi2_formats[i];
+
+	return NULL;
+}
+
+static inline struct rzg2l_csi2 *notifier_to_csi2(struct v4l2_async_notifier *n)
+{
+	return container_of(n, struct rzg2l_csi2, notifier);
+}
+
+static u32 rzg2l_csi2_read(struct rzg2l_csi2 *csi2, unsigned int reg)
+{
+	return ioread32(csi2->base + reg);
+}
+
+static void rzg2l_csi2_write(struct rzg2l_csi2 *csi2, unsigned int reg,
+			     u32 data)
+{
+	iowrite32(data, csi2->base + reg);
+}
+
+static void rzg2l_csi2_set(struct rzg2l_csi2 *csi2, unsigned int reg, u32 set)
+{
+	rzg2l_csi2_write(csi2, reg, rzg2l_csi2_read(csi2, reg) | set);
+}
+
+static void rzg2l_csi2_clr(struct rzg2l_csi2 *csi2, unsigned int reg, u32 clr)
+{
+	rzg2l_csi2_write(csi2, reg, rzg2l_csi2_read(csi2, reg) & ~clr);
+}
+
+static int rzg2l_csi2_calc_mbps(struct rzg2l_csi2 *csi2)
+{
+	struct v4l2_subdev *sd = csi2->remote_source;
+	const struct rzg2l_csi2_format *format;
+	const struct v4l2_mbus_framefmt *fmt;
+	struct v4l2_subdev_state *state;
+	struct v4l2_ctrl *ctrl;
+	u64 mbps;
+
+	/* Read the pixel rate control from remote. */
+	ctrl = v4l2_ctrl_find(sd->ctrl_handler, V4L2_CID_PIXEL_RATE);
+	if (!ctrl) {
+		dev_err(csi2->dev, "no pixel rate control in subdev %s\n",
+			sd->name);
+		return -EINVAL;
+	}
+
+	state = v4l2_subdev_lock_and_get_active_state(&csi2->subdev);
+	fmt = v4l2_subdev_get_pad_format(sd, state, RZG2L_CSI2_SINK);
+	v4l2_subdev_unlock_state(state);
+	format = rzg2l_csi2_code_to_fmt(fmt->code);
+
+	/*
+	 * Calculate hsfreq in Mbps
+	 * hsfreq = (pixel_rate * bits_per_sample) / number_of_lanes
+	 */
+	mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * format->bpp;
+	do_div(mbps, csi2->lanes * 1000000);
+
+	return mbps;
+}
+
+int rzg2l_csi2_cmn_rstb_deassert(struct rzg2l_csi2 *csi2)
+{
+	return reset_control_deassert(csi2->rstc);
+}
+
+/* -----------------------------------------------------------------------------
+ * DPHY setting
+ */
+
+static int rzg2l_csi2_dphy_disable(struct rzg2l_csi2 *csi2)
+{
+	int ret;
+
+	/* Reset the CRU (D-PHY) */
+	ret = reset_control_assert(csi2->rstc);
+	if (ret)
+		return ret;
+
+	/* Stop the D-PHY clock */
+	clk_disable_unprepare(csi2->sysclk);
+
+	/* Cancel the EN_LDO1200 register setting */
+	rzg2l_csi2_clr(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_LDO1200);
+
+	/* Cancel the EN_BGR register setting */
+	rzg2l_csi2_clr(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_BGR);
+
+	return 0;
+}
+
+static int rzg2l_csi2_dphy_enable(struct rzg2l_csi2 *csi2)
+{
+	const struct rzg2l_csi2_timings *dphy_timing;
+	u32 dphytim0, dphytim1;
+	int mbps;
+	int ret;
+
+	mbps = rzg2l_csi2_calc_mbps(csi2);
+	if (mbps < 0)
+		return mbps;
+
+	csi2->hsfreq = mbps;
+
+	/* Set DPHY timing parameters */
+	if (csi2->hsfreq <= 80)
+		dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_80_MBPS];
+	else if (csi2->hsfreq <= 125)
+		dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_125_MBPS];
+	else if (csi2->hsfreq <= 250)
+		dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_250_MBPS];
+	else if (csi2->hsfreq <= 360)
+		dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_360_MBPS];
+	else if (csi2->hsfreq <= 1500)
+		dphy_timing = &rzg2l_csi2_global_timings[TRANSMISSION_RATE_360_MBPS_PLUS];
+	else
+		return -EINVAL;
+
+	/* Set D-PHY timing parameters */
+	dphytim0 = CSIDPHYTIM0_TCLK_MISS(dphy_timing->tclk_miss) |
+			CSIDPHYTIM0_T_INIT(dphy_timing->t_init);
+	dphytim1 = CSIDPHYTIM1_THS_PREPARE(dphy_timing->ths_prepare) |
+			CSIDPHYTIM1_TCLK_PREPARE(dphy_timing->tclk_prepare) |
+			CSIDPHYTIM1_THS_SETTLE(dphy_timing->ths_settle) |
+			CSIDPHYTIM1_TCLK_SETTLE(dphy_timing->tclk_settle);
+	rzg2l_csi2_write(csi2, CSIDPHYTIM0, dphytim0);
+	rzg2l_csi2_write(csi2, CSIDPHYTIM1, dphytim1);
+
+	/* Enable D-PHY power control 0 */
+	rzg2l_csi2_write(csi2, CSIDPHYSKW0, CSIDPHYSKW0_DEFAULT_SKW);
+
+	/* Set the EN_BGR bit */
+	rzg2l_csi2_set(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_BGR);
+
+	/* Delay 20us to be stable */
+	usleep_range(20, 40);
+
+	/* Enable D-PHY power control 1 */
+	rzg2l_csi2_set(csi2, CSIDPHYCTRL0, CSIDPHYCTRL0_EN_LDO1200);
+
+	/* Delay 10us to be stable */
+	usleep_range(10, 20);
+
+	/* Start supplying the internal clock for the D-PHY block */
+	ret = clk_prepare_enable(csi2->sysclk);
+	if (ret)
+		rzg2l_csi2_dphy_disable(csi2);
+
+	return ret;
+}
+
+int rzg2l_csi2_dphy_setting(struct rzg2l_csi2 *csi2, bool on)
+{
+	if (on)
+		return rzg2l_csi2_dphy_enable(csi2);
+
+	return rzg2l_csi2_dphy_disable(csi2);
+}
+
+static void rzg2l_csi2_mipi_link_enable(struct rzg2l_csi2 *csi2)
+{
+	unsigned long vclk_rate = clk_get_rate(csi2->vclk) / HZ_PER_MHZ;
+	u32 frrskw, frrclk, frrskw_coeff, frrclk_coeff;
+
+	/* Select data lanes */
+	rzg2l_csi2_write(csi2, CSI2nMCT0, CSI2nMCT0_VDLN(csi2->lanes));
+
+	frrskw_coeff = 3 * vclk_rate * 8;
+	frrclk_coeff = frrskw_coeff / 2;
+	frrskw = DIV_ROUND_UP(frrskw_coeff, csi2->hsfreq);
+	frrclk = DIV_ROUND_UP(frrclk_coeff, csi2->hsfreq);
+	rzg2l_csi2_write(csi2, CSI2nMCT2, CSI2nMCT2_FRRSKW(frrskw) |
+			 CSI2nMCT2_FRRCLK(frrclk));
+
+	/*
+	 * Select data type.
+	 * FS, FE, LS, LE, Generic Short Packet Codes 1 to 8,
+	 * Generic Long Packet Data Types 1 to 4 YUV422 8-bit,
+	 * RGB565, RGB888, RAW8 to RAW20, User-defined 8-bit
+	 * data types 1 to 8
+	 */
+	rzg2l_csi2_write(csi2, CSI2nDTEL, 0xf778ff0f);
+	rzg2l_csi2_write(csi2, CSI2nDTEH, 0x00ffff1f);
+
+	/* Enable LINK reception */
+	rzg2l_csi2_write(csi2, CSI2nMCT3, CSI2nMCT3_RXEN);
+}
+
+static void rzg2l_csi2_mipi_link_disable(struct rzg2l_csi2 *csi2)
+{
+	unsigned int timeout = VSRSTS_RETRIES;
+
+	/* Stop LINK reception */
+	rzg2l_csi2_clr(csi2, CSI2nMCT3, CSI2nMCT3_RXEN);
+
+	/* Request a software reset of the LINK Video Pixel Interface */
+	rzg2l_csi2_write(csi2, CSI2nRTCT, CSI2nRTCT_VSRST);
+
+	/* Make sure CSI2nRTST.VSRSTS bit is cleared */
+	while (timeout--) {
+		if (!(rzg2l_csi2_read(csi2, CSI2nRTST) & CSI2nRTST_VSRSTS))
+			break;
+		usleep_range(100, 200);
+	};
+
+	if (!timeout)
+		dev_err(csi2->dev, "Clearing CSI2nRTST.VSRSTS timed out\n");
+}
+
+void rzg2l_csi2_mipi_link_setting(struct rzg2l_csi2 *csi2, bool on)
+{
+	if (on)
+		rzg2l_csi2_mipi_link_enable(csi2);
+	else
+		rzg2l_csi2_mipi_link_disable(csi2);
+}
+
+static int rzg2l_csi2_s_stream(struct v4l2_subdev *sd, int enable)
+{
+	struct rzg2l_csi2 *csi2 = sd_to_csi2(sd);
+
+	return v4l2_subdev_call(csi2->remote_source, video, s_stream, enable);
+}
+
+static int rzg2l_csi2_set_format(struct v4l2_subdev *sd,
+				 struct v4l2_subdev_state *state,
+				 struct v4l2_subdev_format *fmt)
+{
+	struct v4l2_mbus_framefmt *src_format;
+	struct v4l2_mbus_framefmt *format;
+
+	if (fmt->pad > RZG2L_CSI2_SINK)
+		return -EINVAL;
+
+	format = v4l2_subdev_get_pad_format(sd, state, fmt->pad);
+
+	if (!rzg2l_csi2_code_to_fmt(fmt->format.code))
+		format->code = rzg2l_csi2_formats[0].code;
+	else
+		format->code = fmt->format.code;
+
+	format->field = V4L2_FIELD_NONE;
+	format->colorspace = V4L2_COLORSPACE_SRGB;
+	format->width = fmt->format.width;
+	format->height = fmt->format.height;
+	fmt->format = *format;
+
+	/* propagate format to source pad */
+	src_format = v4l2_subdev_get_pad_format(sd, state, RZG2L_CSI2_SOURCE_VC0);
+	*src_format = *format;
+
+	return 0;
+}
+
+static int rzg2l_csi2_init_config(struct v4l2_subdev *sd,
+				  struct v4l2_subdev_state *sd_state)
+{
+	struct v4l2_subdev_format fmt = { 0 };
+
+	fmt.which = sd_state ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
+	fmt.format.width = RZG2L_CSI2_DEFAULT_WIDTH;
+	fmt.format.height = RZG2L_CSI2_DEFAULT_HEIGHT;
+	fmt.format.field = V4L2_FIELD_NONE;
+	fmt.format.code = RZG2L_CSI2_DEFAULT_FMT;
+	fmt.format.colorspace = V4L2_COLORSPACE_SRGB;
+
+	return rzg2l_csi2_set_format(sd, sd_state, &fmt);
+}
+
+static int rzg2l_csi2_enum_mbus_code(struct v4l2_subdev *sd,
+				     struct v4l2_subdev_state *sd_state,
+				     struct v4l2_subdev_mbus_code_enum *code)
+{
+	if (code->index >= ARRAY_SIZE(rzg2l_csi2_formats))
+		return -EINVAL;
+
+	code->code = rzg2l_csi2_formats[code->index].code;
+
+	return 0;
+}
+
+static int rzg2l_csi2_enum_frame_size(struct v4l2_subdev *sd,
+				      struct v4l2_subdev_state *sd_state,
+				      struct v4l2_subdev_frame_size_enum *fse)
+{
+	if (fse->index != 0)
+		return -EINVAL;
+
+	fse->min_width = clamp_t(u32, 1, 320, 2800);
+	fse->min_height = clamp_t(u32, 1, 240, 4095);
+	fse->max_width = clamp_t(u32, -1, 320, 2800);
+	fse->max_height = clamp_t(u32, -1, 240, 4095);
+
+	return 0;
+}
+
+static const struct v4l2_subdev_video_ops rzg2l_csi2_video_ops = {
+	.s_stream = rzg2l_csi2_s_stream,
+};
+
+static const struct v4l2_subdev_pad_ops rzg2l_csi2_pad_ops = {
+	.enum_mbus_code = rzg2l_csi2_enum_mbus_code,
+	.init_cfg = rzg2l_csi2_init_config,
+	.enum_frame_size = rzg2l_csi2_enum_frame_size,
+	.set_fmt = rzg2l_csi2_set_format,
+	.get_fmt = v4l2_subdev_get_fmt,
+};
+
+static const struct v4l2_subdev_ops rzg2l_csi2_subdev_ops = {
+	.video	= &rzg2l_csi2_video_ops,
+	.pad	= &rzg2l_csi2_pad_ops,
+};
+
+/* -----------------------------------------------------------------------------
+ * Async handling and registration of subdevices and links.
+ */
+
+static int rzg2l_csi2_notify_bound(struct v4l2_async_notifier *notifier,
+				   struct v4l2_subdev *subdev,
+				   struct v4l2_async_subdev *asd)
+{
+	struct rzg2l_csi2 *csi2 = notifier_to_csi2(notifier);
+
+	csi2->remote_source = subdev;
+
+	dev_dbg(csi2->dev, "Bound subdev: %s pad\n", subdev->name);
+
+	return media_create_pad_link(&subdev->entity, RZG2L_CSI2_SINK,
+				     &csi2->subdev.entity, 0,
+				     MEDIA_LNK_FL_ENABLED |
+				     MEDIA_LNK_FL_IMMUTABLE);
+}
+
+static void rzg2l_csi2_notify_unbind(struct v4l2_async_notifier *notifier,
+				     struct v4l2_subdev *subdev,
+				     struct v4l2_async_subdev *asd)
+{
+	struct rzg2l_csi2 *csi2 = notifier_to_csi2(notifier);
+
+	csi2->remote_source = NULL;
+
+	dev_dbg(csi2->dev, "Unbind subdev %s\n", subdev->name);
+}
+
+static const struct v4l2_async_notifier_operations rzg2l_csi2_notify_ops = {
+	.bound = rzg2l_csi2_notify_bound,
+	.unbind = rzg2l_csi2_notify_unbind,
+};
+
+static int rzg2l_csi2_parse_v4l2(struct rzg2l_csi2 *csi2,
+				 struct v4l2_fwnode_endpoint *vep)
+{
+	/* Only port 0 endpoint 0 is valid. */
+	if (vep->base.port || vep->base.id)
+		return -ENOTCONN;
+
+	csi2->lanes = vep->bus.mipi_csi2.num_data_lanes;
+
+	return 0;
+}
+
+static int rzg2l_csi2_parse_dt(struct rzg2l_csi2 *csi2)
+{
+	struct v4l2_fwnode_endpoint v4l2_ep = {
+		.bus_type = V4L2_MBUS_CSI2_DPHY
+	};
+	struct v4l2_async_subdev *asd;
+	struct fwnode_handle *fwnode;
+	struct fwnode_handle *ep;
+	int ret;
+
+	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(csi2->dev), 0, 0, 0);
+	if (!ep) {
+		dev_err(csi2->dev, "Not connected to subdevice\n");
+		return -EINVAL;
+	}
+
+	ret = v4l2_fwnode_endpoint_parse(ep, &v4l2_ep);
+	if (ret) {
+		dev_err(csi2->dev, "Could not parse v4l2 endpoint\n");
+		fwnode_handle_put(ep);
+		return -EINVAL;
+	}
+
+	ret = rzg2l_csi2_parse_v4l2(csi2, &v4l2_ep);
+	if (ret) {
+		fwnode_handle_put(ep);
+		return ret;
+	}
+
+	fwnode = fwnode_graph_get_remote_endpoint(ep);
+	fwnode_handle_put(ep);
+
+	dev_dbg(csi2->dev, "Found '%pOF'\n", to_of_node(fwnode));
+
+	v4l2_async_nf_init(&csi2->notifier);
+	csi2->notifier.ops = &rzg2l_csi2_notify_ops;
+
+	asd = v4l2_async_nf_add_fwnode(&csi2->notifier, fwnode,
+				       struct v4l2_async_subdev);
+	fwnode_handle_put(fwnode);
+	if (IS_ERR(asd))
+		return PTR_ERR(asd);
+
+	ret = v4l2_async_subdev_nf_register(&csi2->subdev, &csi2->notifier);
+	if (ret)
+		v4l2_async_nf_cleanup(&csi2->notifier);
+
+	return ret;
+}
+
+static int rzg2l_validate_csi2_lanes(struct rzg2l_csi2 *csi2)
+{
+	int ret = 0;
+	int lanes;
+
+	if (csi2->lanes != 1 && csi2->lanes != 2 && csi2->lanes != 4) {
+		dev_err(csi2->dev, "Unsupported number of data-lanes: %u\n",
+			csi2->lanes);
+		return -EINVAL;
+	}
+
+	ret = clk_prepare_enable(csi2->pclk);
+	if (ret)
+		return ret;
+
+	/* Checking the maximum lanes support for CSI-2 module */
+	lanes = (rzg2l_csi2_read(csi2, CSI2nMCG) & CSI2nMCG_SDLN) >> 8;
+	if (lanes < csi2->lanes) {
+		dev_err(csi2->dev,
+			"Failed to support %d data lanes\n", csi2->lanes);
+		ret = -EINVAL;
+	}
+
+	clk_disable_unprepare(csi2->pclk);
+
+	return ret;
+}
+
+/* -----------------------------------------------------------------------------
+ * Platform Device Driver.
+ */
+
+static const struct media_entity_operations rzg2l_csi2_entity_ops = {
+	.link_validate = v4l2_subdev_link_validate,
+};
+
+static int rzg2l_csi2_probe(struct platform_device *pdev)
+{
+	struct rzg2l_csi2 *csi2;
+	int ret;
+
+	csi2 = devm_kzalloc(&pdev->dev, sizeof(*csi2), GFP_KERNEL);
+	if (!csi2)
+		return -ENOMEM;
+
+	csi2->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(csi2->base))
+		return PTR_ERR(csi2->base);
+
+	csi2->rstc = devm_reset_control_get(&pdev->dev, "cmn-rstb");
+	if (IS_ERR(csi2->rstc))
+		return dev_err_probe(&pdev->dev, PTR_ERR(csi2->rstc),
+				     "failed to get cpg cmn-rstb\n");
+
+	csi2->sysclk = devm_clk_get(&pdev->dev, "sysclk");
+	if (IS_ERR(csi2->sysclk)) {
+		dev_err(&pdev->dev, "Failed to get sysclk");
+		return PTR_ERR(csi2->sysclk);
+	}
+
+	csi2->vclk = devm_clk_get(&pdev->dev, "vclk");
+	if (IS_ERR(csi2->vclk)) {
+		dev_err(&pdev->dev, "Failed to get vclk");
+		return PTR_ERR(csi2->vclk);
+	}
+
+	csi2->pclk = devm_clk_get(&pdev->dev, "pclk");
+	if (IS_ERR(csi2->pclk)) {
+		dev_err(&pdev->dev, "Failed to get pclk");
+		return PTR_ERR(csi2->pclk);
+	}
+
+	csi2->dev = &pdev->dev;
+
+	platform_set_drvdata(pdev, csi2);
+
+	ret = rzg2l_csi2_parse_dt(csi2);
+	if (ret)
+		return ret;
+
+	ret = rzg2l_validate_csi2_lanes(csi2);
+	if (ret)
+		return ret;
+
+	csi2->subdev.dev = &pdev->dev;
+	v4l2_subdev_init(&csi2->subdev, &rzg2l_csi2_subdev_ops);
+	v4l2_set_subdevdata(&csi2->subdev, &pdev->dev);
+	snprintf(csi2->subdev.name, sizeof(csi2->subdev.name),
+		 "csi-%s", dev_name(&pdev->dev));
+	csi2->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
+
+	csi2->subdev.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
+	csi2->subdev.entity.ops = &rzg2l_csi2_entity_ops;
+
+	csi2->pads[RZG2L_CSI2_SINK].flags = MEDIA_PAD_FL_SINK;
+	/*
+	 * TODO: RZ/G2L CSI2 supports 4 virtual channels, as virtual
+	 * channels should be implemented by streams API which is under
+	 * development lets hardcode to VC0 for now.
+	 */
+	csi2->pads[RZG2L_CSI2_SOURCE_VC0].flags = MEDIA_PAD_FL_SOURCE;
+	ret = media_entity_pads_init(&csi2->subdev.entity, 2, csi2->pads);
+	if (ret)
+		goto error_async;
+
+	pm_runtime_enable(&pdev->dev);
+
+	ret = v4l2_subdev_init_finalize(&csi2->subdev);
+	if (ret < 0)
+		goto error_pm;
+
+	/* enable pclk for register access */
+	ret = clk_prepare_enable(csi2->pclk);
+	if (ret)
+		goto error_pm;
+
+	ret = v4l2_async_register_subdev(&csi2->subdev);
+	if (ret < 0)
+		goto error_subdev;
+
+	dev_info(csi2->dev, "%d lanes found\n", csi2->lanes);
+
+	return 0;
+
+error_subdev:
+	clk_disable_unprepare(csi2->pclk);
+	v4l2_subdev_cleanup(&csi2->subdev);
+error_pm:
+	pm_runtime_disable(&pdev->dev);
+error_async:
+	v4l2_async_nf_unregister(&csi2->notifier);
+	v4l2_async_nf_cleanup(&csi2->notifier);
+
+	return ret;
+}
+
+static const struct of_device_id rzg2l_csi2_of_table[] = {
+	{ .compatible = "renesas,rzg2l-csi2", },
+	{ /* sentinel */ }
+};
+
+static int rzg2l_csi2_remove(struct platform_device *pdev)
+{
+	struct rzg2l_csi2 *csi2 = platform_get_drvdata(pdev);
+
+	v4l2_async_nf_unregister(&csi2->notifier);
+	v4l2_async_nf_cleanup(&csi2->notifier);
+	v4l2_async_unregister_subdev(&csi2->subdev);
+	clk_disable_unprepare(csi2->pclk);
+	v4l2_subdev_cleanup(&csi2->subdev);
+
+	pm_runtime_disable(&pdev->dev);
+
+	return 0;
+}
+
+static struct platform_driver rzg2l_csi2_pdrv = {
+	.remove	= rzg2l_csi2_remove,
+	.probe	= rzg2l_csi2_probe,
+	.driver	= {
+		.name = "rzg2l-csi2",
+		.of_match_table = rzg2l_csi2_of_table,
+	},
+};
+
+module_platform_driver(rzg2l_csi2_pdrv);
+
+MODULE_AUTHOR("Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>");
+MODULE_DESCRIPTION("Renesas RZ/G2L MIPI CSI2 receiver driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h
new file mode 100644
index 000000000000..f85a1d44250f
--- /dev/null
+++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.h
@@ -0,0 +1,46 @@ 
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) 2022 Renesas Electronics Corp.
+ */
+
+#ifndef __RZG2L_CSI2__
+#define __RZG2L_CSI2__
+
+enum rzg2l_csi2_pads {
+	RZG2L_CSI2_SINK = 0,
+	RZG2L_CSI2_SOURCE_VC0,
+	RZG2L_CSI2_SOURCE_VC1,
+	RZG2L_CSI2_SOURCE_VC2,
+	RZG2L_CSI2_SOURCE_VC3,
+	NR_OF_RZG2L_CSI2_PAD,
+};
+
+struct rzg2l_csi2 {
+	struct device *dev;
+	void __iomem *base;
+	struct reset_control *rstc;
+	struct clk *sysclk;
+	struct clk *vclk;
+	struct clk *pclk;
+
+	struct v4l2_subdev subdev;
+	struct media_pad pads[NR_OF_RZG2L_CSI2_PAD];
+
+	struct v4l2_async_notifier notifier;
+	struct v4l2_subdev *remote_source;
+
+	unsigned short lanes;
+
+	unsigned long hsfreq;
+};
+
+static inline struct rzg2l_csi2 *sd_to_csi2(struct v4l2_subdev *sd)
+{
+	return container_of(sd, struct rzg2l_csi2, subdev);
+}
+
+int rzg2l_csi2_cmn_rstb_deassert(struct rzg2l_csi2 *csi2);
+int rzg2l_csi2_dphy_setting(struct rzg2l_csi2 *csi2, bool on);
+void rzg2l_csi2_mipi_link_setting(struct rzg2l_csi2 *csi2, bool on);
+
+#endif