diff mbox series

[RFC,5/8] media: v4l2-mediabus: Use structures to describe bus configuration

Message ID 20220103162414.27723-6-laurent.pinchart+renesas@ideasonboard.com (mailing list archive)
State New
Delegated to: Kieran Bingham
Headers show
Series media: Drop .set_mbus_config(), improve .get_mbus_config() | expand

Commit Message

Laurent Pinchart Jan. 3, 2022, 4:24 p.m. UTC 
The media bus configuration is specified through a set of flags, some of
which being mutually exclusive. This doesn't scale to express more
complex configurations. Improve the API by replacing the single flags
field in v4l2_mbus_config by a union of v4l2_mbus_config_* structures.
The flags themselves are still used in those structures, so they are
kept here. Drivers are however updated to use structure fields instead
of flags when already possible.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/ipu-v3/ipu-csi.c                |  6 ++---
 drivers/media/i2c/adv7180.c                 |  8 ++++---
 drivers/media/i2c/adv748x/adv748x-csi2.c    | 18 +--------------
 drivers/media/i2c/ml86v7667.c               |  5 +++--
 drivers/media/i2c/mt9m001.c                 |  8 ++++---
 drivers/media/i2c/mt9m111.c                 | 16 +++++++------
 drivers/media/i2c/ov6650.c                  | 14 ++++++------
 drivers/media/i2c/ov9640.c                  |  8 ++++---
 drivers/media/i2c/tc358743.c                | 20 ++---------------
 drivers/media/i2c/tvp5150.c                 |  6 +++--
 drivers/media/platform/pxa_camera.c         | 12 +++++-----
 drivers/media/platform/rcar-vin/rcar-csi2.c | 16 +++----------
 drivers/staging/media/imx/imx-media-csi.c   |  7 +++---
 drivers/staging/media/imx/imx6-mipi-csi2.c  | 25 +++------------------
 include/media/v4l2-mediabus.h               | 20 ++++++++++++++---
 15 files changed, 78 insertions(+), 111 deletions(-)

Comments

Jacopo Mondi Feb. 15, 2022, 11:13 a.m. UTC | #1 
Hi Laurent

On Mon, Jan 03, 2022 at 06:24:11PM +0200, Laurent Pinchart wrote:
> The media bus configuration is specified through a set of flags, some of
> which being mutually exclusive. This doesn't scale to express more
> complex configurations. Improve the API by replacing the single flags
> field in v4l2_mbus_config by a union of v4l2_mbus_config_* structures.
> The flags themselves are still used in those structures, so they are
> kept here. Drivers are however updated to use structure fields instead
> of flags when already possible.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/ipu-v3/ipu-csi.c                |  6 ++---
>  drivers/media/i2c/adv7180.c                 |  8 ++++---
>  drivers/media/i2c/adv748x/adv748x-csi2.c    | 18 +--------------
>  drivers/media/i2c/ml86v7667.c               |  5 +++--
>  drivers/media/i2c/mt9m001.c                 |  8 ++++---
>  drivers/media/i2c/mt9m111.c                 | 16 +++++++------
>  drivers/media/i2c/ov6650.c                  | 14 ++++++------
>  drivers/media/i2c/ov9640.c                  |  8 ++++---
>  drivers/media/i2c/tc358743.c                | 20 ++---------------
>  drivers/media/i2c/tvp5150.c                 |  6 +++--
>  drivers/media/platform/pxa_camera.c         | 12 +++++-----
>  drivers/media/platform/rcar-vin/rcar-csi2.c | 16 +++----------
>  drivers/staging/media/imx/imx-media-csi.c   |  7 +++---
>  drivers/staging/media/imx/imx6-mipi-csi2.c  | 25 +++------------------

drivers conversions looks good to me

>  include/media/v4l2-mediabus.h               | 20 ++++++++++++++---
>  15 files changed, 78 insertions(+), 111 deletions(-)
>
> diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
> index a9639d098893..778bc26d3ba5 100644
> --- a/drivers/gpu/ipu-v3/ipu-csi.c
> +++ b/drivers/gpu/ipu-v3/ipu-csi.c
> @@ -357,11 +357,11 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config *csicfg,
>  	switch (mbus_cfg->type) {
>  	case V4L2_MBUS_PARALLEL:
>  		csicfg->ext_vsync = 1;
> -		csicfg->vsync_pol = (mbus_cfg->flags &
> +		csicfg->vsync_pol = (mbus_cfg->bus.parallel.flags &
>  				     V4L2_MBUS_VSYNC_ACTIVE_LOW) ? 1 : 0;
> -		csicfg->hsync_pol = (mbus_cfg->flags &
> +		csicfg->hsync_pol = (mbus_cfg->bus.parallel.flags &
>  				     V4L2_MBUS_HSYNC_ACTIVE_LOW) ? 1 : 0;
> -		csicfg->pixclk_pol = (mbus_cfg->flags &
> +		csicfg->pixclk_pol = (mbus_cfg->bus.parallel.flags &
>  				      V4L2_MBUS_PCLK_SAMPLE_FALLING) ? 1 : 0;
>  		csicfg->clk_mode = IPU_CSI_CLK_MODE_GATED_CLK;
>  		break;
> diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
> index d9a99fcfacb1..286f5017d9c3 100644
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -784,7 +784,8 @@ static int adv7180_get_mbus_config(struct v4l2_subdev *sd,
>
>  	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
>  		cfg->type = V4L2_MBUS_CSI2_DPHY;
> -		cfg->flags = V4L2_MBUS_CSI2_1_LANE |
> +		cfg->bus.mipi_csi2.num_data_lanes = 1;
> +		cfg->bus.mipi_csi2.flags =
>  				V4L2_MBUS_CSI2_CHANNEL_0 |
>  				V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
>  	} else {
> @@ -792,8 +793,9 @@ static int adv7180_get_mbus_config(struct v4l2_subdev *sd,
>  		 * The ADV7180 sensor supports BT.601/656 output modes.
>  		 * The BT.656 is default and not yet configurable by s/w.
>  		 */
> -		cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING |
> -				 V4L2_MBUS_DATA_ACTIVE_HIGH;
> +		cfg->bus.parallel.flags = V4L2_MBUS_MASTER |
> +					  V4L2_MBUS_PCLK_SAMPLE_RISING |
> +					  V4L2_MBUS_DATA_ACTIVE_HIGH;
>  		cfg->type = V4L2_MBUS_BT656;
>  	}
>
> diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c
> index 589e9644fcdc..bd4f3fe0e309 100644
> --- a/drivers/media/i2c/adv748x/adv748x-csi2.c
> +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
> @@ -222,23 +222,7 @@ static int adv748x_csi2_get_mbus_config(struct v4l2_subdev *sd, unsigned int pad
>  		return -EINVAL;
>
>  	config->type = V4L2_MBUS_CSI2_DPHY;
> -	switch (tx->active_lanes) {
> -	case 1:
> -		config->flags = V4L2_MBUS_CSI2_1_LANE;
> -		break;
> -
> -	case 2:
> -		config->flags = V4L2_MBUS_CSI2_2_LANE;
> -		break;
> -
> -	case 3:
> -		config->flags = V4L2_MBUS_CSI2_3_LANE;
> -		break;
> -
> -	case 4:
> -		config->flags = V4L2_MBUS_CSI2_4_LANE;
> -		break;
> -	}
> +	config->bus.mipi_csi2.num_data_lanes = tx->active_lanes;
>
>  	return 0;
>  }
> diff --git a/drivers/media/i2c/ml86v7667.c b/drivers/media/i2c/ml86v7667.c
> index 4a1410ebb4c8..48cc0b0922f4 100644
> --- a/drivers/media/i2c/ml86v7667.c
> +++ b/drivers/media/i2c/ml86v7667.c
> @@ -223,9 +223,10 @@ static int ml86v7667_get_mbus_config(struct v4l2_subdev *sd,
>  				     unsigned int pad,
>  				     struct v4l2_mbus_config *cfg)
>  {
> -	cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING |
> -		     V4L2_MBUS_DATA_ACTIVE_HIGH;
>  	cfg->type = V4L2_MBUS_BT656;
> +	cfg->bus.parallel.flags = V4L2_MBUS_MASTER |
> +				  V4L2_MBUS_PCLK_SAMPLE_RISING |
> +				  V4L2_MBUS_DATA_ACTIVE_HIGH;
>
>  	return 0;
>  }
> diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
> index c9f0bd997ea7..ad13b0c890c0 100644
> --- a/drivers/media/i2c/mt9m001.c
> +++ b/drivers/media/i2c/mt9m001.c
> @@ -695,10 +695,12 @@ static int mt9m001_get_mbus_config(struct v4l2_subdev *sd,
>  				   struct v4l2_mbus_config *cfg)
>  {
>  	/* MT9M001 has all capture_format parameters fixed */
> -	cfg->flags = V4L2_MBUS_PCLK_SAMPLE_FALLING |
> -		V4L2_MBUS_HSYNC_ACTIVE_HIGH | V4L2_MBUS_VSYNC_ACTIVE_HIGH |
> -		V4L2_MBUS_DATA_ACTIVE_HIGH | V4L2_MBUS_MASTER;
>  	cfg->type = V4L2_MBUS_PARALLEL;
> +	cfg->bus.parallel.flags = V4L2_MBUS_PCLK_SAMPLE_FALLING |
> +				  V4L2_MBUS_HSYNC_ACTIVE_HIGH |
> +				  V4L2_MBUS_VSYNC_ACTIVE_HIGH |
> +				  V4L2_MBUS_DATA_ACTIVE_HIGH |
> +				  V4L2_MBUS_MASTER;
>
>  	return 0;
>  }
> diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
> index 91a44359bcd3..6cf3ccf85d27 100644
> --- a/drivers/media/i2c/mt9m111.c
> +++ b/drivers/media/i2c/mt9m111.c
> @@ -1143,15 +1143,17 @@ static int mt9m111_get_mbus_config(struct v4l2_subdev *sd,
>  {
>  	struct mt9m111 *mt9m111 = container_of(sd, struct mt9m111, subdev);
>
> -	cfg->flags = V4L2_MBUS_MASTER |
> -		V4L2_MBUS_HSYNC_ACTIVE_HIGH | V4L2_MBUS_VSYNC_ACTIVE_HIGH |
> -		V4L2_MBUS_DATA_ACTIVE_HIGH;
> -
> -	cfg->flags |= mt9m111->pclk_sample ? V4L2_MBUS_PCLK_SAMPLE_RISING :
> -		V4L2_MBUS_PCLK_SAMPLE_FALLING;
> -
>  	cfg->type = V4L2_MBUS_PARALLEL;
>
> +	cfg->bus.parallel.flags = V4L2_MBUS_MASTER |
> +				  V4L2_MBUS_HSYNC_ACTIVE_HIGH |
> +				  V4L2_MBUS_VSYNC_ACTIVE_HIGH |
> +				  V4L2_MBUS_DATA_ACTIVE_HIGH;
> +
> +	cfg->bus.parallel.flags |= mt9m111->pclk_sample ?
> +				   V4L2_MBUS_PCLK_SAMPLE_RISING :
> +				   V4L2_MBUS_PCLK_SAMPLE_FALLING;
> +
>  	return 0;
>  }
>
> diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c
> index 455a627e35a0..b2654b32bc62 100644
> --- a/drivers/media/i2c/ov6650.c
> +++ b/drivers/media/i2c/ov6650.c
> @@ -932,15 +932,15 @@ static int ov6650_get_mbus_config(struct v4l2_subdev *sd,
>  	if (ret)
>  		return ret;
>
> -	cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_DATA_ACTIVE_HIGH
> -		   | ((comj & COMJ_VSYNC_HIGH)  ? V4L2_MBUS_VSYNC_ACTIVE_HIGH
> -						: V4L2_MBUS_VSYNC_ACTIVE_LOW)
> -		   | ((comf & COMF_HREF_LOW)    ? V4L2_MBUS_HSYNC_ACTIVE_LOW
> -						: V4L2_MBUS_HSYNC_ACTIVE_HIGH)
> -		   | ((comj & COMJ_PCLK_RISING) ? V4L2_MBUS_PCLK_SAMPLE_RISING
> -						: V4L2_MBUS_PCLK_SAMPLE_FALLING);
>  	cfg->type = V4L2_MBUS_PARALLEL;
>
> +	cfg->bus.parallel.flags = V4L2_MBUS_MASTER | V4L2_MBUS_DATA_ACTIVE_HIGH
> +		| ((comj & COMJ_VSYNC_HIGH)  ? V4L2_MBUS_VSYNC_ACTIVE_HIGH
> +					     : V4L2_MBUS_VSYNC_ACTIVE_LOW)
> +		| ((comf & COMF_HREF_LOW)    ? V4L2_MBUS_HSYNC_ACTIVE_LOW
> +					     : V4L2_MBUS_HSYNC_ACTIVE_HIGH)
> +		| ((comj & COMJ_PCLK_RISING) ? V4L2_MBUS_PCLK_SAMPLE_RISING
> +					     : V4L2_MBUS_PCLK_SAMPLE_FALLING);
>  	return 0;
>  }
>
> diff --git a/drivers/media/i2c/ov9640.c b/drivers/media/i2c/ov9640.c
> index 0bab8c2cf160..9f44ed52d164 100644
> --- a/drivers/media/i2c/ov9640.c
> +++ b/drivers/media/i2c/ov9640.c
> @@ -652,10 +652,12 @@ static int ov9640_get_mbus_config(struct v4l2_subdev *sd,
>  				  unsigned int pad,
>  				  struct v4l2_mbus_config *cfg)
>  {
> -	cfg->flags = V4L2_MBUS_PCLK_SAMPLE_RISING | V4L2_MBUS_MASTER |
> -		V4L2_MBUS_VSYNC_ACTIVE_HIGH | V4L2_MBUS_HSYNC_ACTIVE_HIGH |
> -		V4L2_MBUS_DATA_ACTIVE_HIGH;
>  	cfg->type = V4L2_MBUS_PARALLEL;
> +	cfg->bus.parallel.flags = V4L2_MBUS_PCLK_SAMPLE_RISING |
> +				  V4L2_MBUS_MASTER |
> +				  V4L2_MBUS_VSYNC_ACTIVE_HIGH |
> +				  V4L2_MBUS_HSYNC_ACTIVE_HIGH |
> +				  V4L2_MBUS_DATA_ACTIVE_HIGH;
>
>  	return 0;
>  }
> diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
> index 40512004afba..dfbc42675143 100644
> --- a/drivers/media/i2c/tc358743.c
> +++ b/drivers/media/i2c/tc358743.c
> @@ -1613,24 +1613,8 @@ static int tc358743_get_mbus_config(struct v4l2_subdev *sd,
>  	cfg->type = V4L2_MBUS_CSI2_DPHY;
>
>  	/* Support for non-continuous CSI-2 clock is missing in the driver */
> -	cfg->flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
> -
> -	switch (state->csi_lanes_in_use) {
> -	case 1:
> -		cfg->flags |= V4L2_MBUS_CSI2_1_LANE;
> -		break;
> -	case 2:
> -		cfg->flags |= V4L2_MBUS_CSI2_2_LANE;
> -		break;
> -	case 3:
> -		cfg->flags |= V4L2_MBUS_CSI2_3_LANE;
> -		break;
> -	case 4:
> -		cfg->flags |= V4L2_MBUS_CSI2_4_LANE;
> -		break;
> -	default:
> -		return -EINVAL;
> -	}
> +	cfg->bus.mipi_csi2.flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
> +	cfg->bus.mipi_csi2.num_data_lanes = state->csi_lanes_in_use;
>
>  	return 0;
>  }
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index 4b16ffcaef98..65472438444b 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -1198,8 +1198,10 @@ static int tvp5150_get_mbus_config(struct v4l2_subdev *sd,
>  	struct tvp5150 *decoder = to_tvp5150(sd);
>
>  	cfg->type = decoder->mbus_type;
> -	cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING
> -		   | V4L2_MBUS_FIELD_EVEN_LOW | V4L2_MBUS_DATA_ACTIVE_HIGH;
> +	cfg->bus.parallel.flags = V4L2_MBUS_MASTER
> +				| V4L2_MBUS_PCLK_SAMPLE_RISING
> +				| V4L2_MBUS_FIELD_EVEN_LOW
> +				| V4L2_MBUS_DATA_ACTIVE_HIGH;
>
>  	return 0;
>  }
> diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c
> index b5644cf37fe9..35145e3348f0 100644
> --- a/drivers/media/platform/pxa_camera.c
> +++ b/drivers/media/platform/pxa_camera.c
> @@ -1587,24 +1587,26 @@ static int pxa_camera_set_bus_param(struct pxa_camera_dev *pcdev)
>  	 * PXA does not support V4L2_MBUS_DATA_ACTIVE_LOW and the bus mastering
>  	 * roles should match.
>  	 */
> -	if (cfg.flags != mbus_config) {
> +	if (cfg.bus.parallel.flags != mbus_config) {
>  		unsigned int pxa_mbus_role = mbus_config & (V4L2_MBUS_MASTER |
>  							    V4L2_MBUS_SLAVE);
> -		if (pxa_mbus_role != (cfg.flags & (V4L2_MBUS_MASTER |
> -						   V4L2_MBUS_SLAVE))) {
> +		unsigned int flags = cfg.bus.parallel.flags;
> +
> +		if (pxa_mbus_role != (flags & (V4L2_MBUS_MASTER |
> +					       V4L2_MBUS_SLAVE))) {
>  			dev_err(pcdev_to_dev(pcdev),
>  				"Unsupported mbus configuration: bus mastering\n");
>  			return -EINVAL;
>  		}
>
> -		if (cfg.flags & V4L2_MBUS_DATA_ACTIVE_LOW) {
> +		if (flags & V4L2_MBUS_DATA_ACTIVE_LOW) {
>  			dev_err(pcdev_to_dev(pcdev),
>  				"Unsupported mbus configuration: DATA_ACTIVE_LOW\n");
>  			return -EINVAL;
>  		}
>  	}
>
> -	pxa_camera_setup_cicr(pcdev, cfg.flags, pixfmt);
> +	pxa_camera_setup_cicr(pcdev, cfg.bus.parallel.flags, pixfmt);
>
>  	return 0;
>  }
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index 8c939cb3073d..cbac5801720b 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -603,7 +603,6 @@ static int rcsi2_get_active_lanes(struct rcar_csi2 *priv,
>  				  unsigned int *lanes)
>  {
>  	struct v4l2_mbus_config mbus_config = { 0 };
> -	unsigned int num_lanes = UINT_MAX;
>  	int ret;
>
>  	*lanes = priv->lanes;
> @@ -626,23 +625,14 @@ static int rcsi2_get_active_lanes(struct rcar_csi2 *priv,
>  		return -EINVAL;
>  	}
>
> -	if (mbus_config.flags & V4L2_MBUS_CSI2_1_LANE)
> -		num_lanes = 1;
> -	else if (mbus_config.flags & V4L2_MBUS_CSI2_2_LANE)
> -		num_lanes = 2;
> -	else if (mbus_config.flags & V4L2_MBUS_CSI2_3_LANE)
> -		num_lanes = 3;
> -	else if (mbus_config.flags & V4L2_MBUS_CSI2_4_LANE)
> -		num_lanes = 4;
> -
> -	if (num_lanes > priv->lanes) {
> +	if (mbus_config.bus.mipi_csi2.num_data_lanes > priv->lanes) {
>  		dev_err(priv->dev,
>  			"Unsupported mbus config: too many data lanes %u\n",
> -			num_lanes);
> +			mbus_config.bus.mipi_csi2.num_data_lanes);
>  		return -EINVAL;
>  	}
>
> -	*lanes = num_lanes;
> +	*lanes = mbus_config.bus.mipi_csi2.num_data_lanes;
>
>  	return 0;
>  }
> diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> index bd7f156f2d52..b2b1f4dd41d7 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -718,9 +718,10 @@ static int csi_setup(struct csi_priv *priv)
>
>  	/* compose mbus_config from the upstream endpoint */
>  	mbus_cfg.type = priv->upstream_ep.bus_type;
> -	mbus_cfg.flags = is_parallel_bus(&priv->upstream_ep) ?
> -		priv->upstream_ep.bus.parallel.flags :
> -		priv->upstream_ep.bus.mipi_csi2.flags;
> +	if (is_parallel_bus(&priv->upstream_ep))
> +		mbus_cfg.bus.parallel = priv->upstream_ep.bus.parallel;
> +	else
> +		mbus_cfg.bus.mipi_csi2 = priv->upstream_ep.bus.mipi_csi2;
>
>  	if_fmt = *infmt;
>  	crop = priv->crop;
> diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
> index 558b256ac935..c4cb558a85c6 100644
> --- a/drivers/staging/media/imx/imx6-mipi-csi2.c
> +++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
> @@ -303,7 +303,6 @@ static void csi2ipu_gasket_init(struct csi2_dev *csi2)
>  static int csi2_get_active_lanes(struct csi2_dev *csi2, unsigned int *lanes)
>  {
>  	struct v4l2_mbus_config mbus_config = { 0 };
> -	unsigned int num_lanes = UINT_MAX;
>  	int ret;
>
>  	*lanes = csi2->data_lanes;
> @@ -326,32 +325,14 @@ static int csi2_get_active_lanes(struct csi2_dev *csi2, unsigned int *lanes)
>  		return -EINVAL;
>  	}
>
> -	switch (mbus_config.flags & V4L2_MBUS_CSI2_LANES) {
> -	case V4L2_MBUS_CSI2_1_LANE:
> -		num_lanes = 1;
> -		break;
> -	case V4L2_MBUS_CSI2_2_LANE:
> -		num_lanes = 2;
> -		break;
> -	case V4L2_MBUS_CSI2_3_LANE:
> -		num_lanes = 3;
> -		break;
> -	case V4L2_MBUS_CSI2_4_LANE:
> -		num_lanes = 4;
> -		break;
> -	default:
> -		num_lanes = csi2->data_lanes;
> -		break;
> -	}
> -
> -	if (num_lanes > csi2->data_lanes) {
> +	if (mbus_config.bus.mipi_csi2.num_data_lanes > csi2->data_lanes) {
>  		dev_err(csi2->dev,
>  			"Unsupported mbus config: too many data lanes %u\n",
> -			num_lanes);
> +			mbus_config.bus.mipi_csi2.num_data_lanes);
>  		return -EINVAL;
>  	}
>
> -	*lanes = num_lanes;
> +	*lanes = mbus_config.bus.mipi_csi2.num_data_lanes;
>
>  	return 0;
>  }
> diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
> index 9c4970fbd8ea..269aaf57ba32 100644
> --- a/include/media/v4l2-mediabus.h
> +++ b/include/media/v4l2-mediabus.h
> @@ -166,12 +166,26 @@ enum v4l2_mbus_type {
>
>  /**
>   * struct v4l2_mbus_config - media bus configuration
> - * @type:	in: interface type
> - * @flags:	in / out: configuration flags, depending on @type
> + * @type: interface type
> + * @bus: bus configuration data structure
> + * @bus.parallel: embedded &struct v4l2_mbus_config_parallel.
> + *		  Used if the bus is parallel or BT.656.
> + * @bus.mipi_csi1: embedded &struct v4l2_mbus_config_mipi_csi1.
> + *		   Used if the bus is MIPI Alliance's Camera Serial
> + *		   Interface version 1 (MIPI CSI1) or Standard
> + *		   Mobile Imaging Architecture's Compact Camera Port 2
> + *		   (SMIA CCP2).
> + * @bus.mipi_csi2: embedded &struct v4l2_mbus_config_mipi_csi2.
> + *		   Used if the bus is MIPI Alliance's Camera Serial
> + *		   Interface version 2 (MIPI CSI2).
>   */
>  struct v4l2_mbus_config {
>  	enum v4l2_mbus_type type;
> -	unsigned int flags;
> +	union {
> +		struct v4l2_mbus_config_parallel parallel;
> +		struct v4l2_mbus_config_mipi_csi1 mipi_csi1;
> +		struct v4l2_mbus_config_mipi_csi2 mipi_csi2;

I wonder if we would be able to use a single structure for D-PHY and
C-PHY. The clock lane index does not make much sense for the latter,
but it was already there, so it can be changed on top in case it is
required

Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Thanks
  j

> +	} bus;
>  };
>
>  /**
> --
> Regards,
>
> Laurent Pinchart
>
Laurent Pinchart Feb. 15, 2022, 11:15 a.m. UTC | #2 
Hi Jacopo,

On Tue, Feb 15, 2022 at 12:13:16PM +0100, Jacopo Mondi wrote:
> On Mon, Jan 03, 2022 at 06:24:11PM +0200, Laurent Pinchart wrote:
> > The media bus configuration is specified through a set of flags, some of
> > which being mutually exclusive. This doesn't scale to express more
> > complex configurations. Improve the API by replacing the single flags
> > field in v4l2_mbus_config by a union of v4l2_mbus_config_* structures.
> > The flags themselves are still used in those structures, so they are
> > kept here. Drivers are however updated to use structure fields instead
> > of flags when already possible.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >  drivers/gpu/ipu-v3/ipu-csi.c                |  6 ++---
> >  drivers/media/i2c/adv7180.c                 |  8 ++++---
> >  drivers/media/i2c/adv748x/adv748x-csi2.c    | 18 +--------------
> >  drivers/media/i2c/ml86v7667.c               |  5 +++--
> >  drivers/media/i2c/mt9m001.c                 |  8 ++++---
> >  drivers/media/i2c/mt9m111.c                 | 16 +++++++------
> >  drivers/media/i2c/ov6650.c                  | 14 ++++++------
> >  drivers/media/i2c/ov9640.c                  |  8 ++++---
> >  drivers/media/i2c/tc358743.c                | 20 ++---------------
> >  drivers/media/i2c/tvp5150.c                 |  6 +++--
> >  drivers/media/platform/pxa_camera.c         | 12 +++++-----
> >  drivers/media/platform/rcar-vin/rcar-csi2.c | 16 +++----------
> >  drivers/staging/media/imx/imx-media-csi.c   |  7 +++---
> >  drivers/staging/media/imx/imx6-mipi-csi2.c  | 25 +++------------------
> 
> drivers conversions looks good to me
> 
> >  include/media/v4l2-mediabus.h               | 20 ++++++++++++++---
> >  15 files changed, 78 insertions(+), 111 deletions(-)
> >
> > diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
> > index a9639d098893..778bc26d3ba5 100644
> > --- a/drivers/gpu/ipu-v3/ipu-csi.c
> > +++ b/drivers/gpu/ipu-v3/ipu-csi.c
> > @@ -357,11 +357,11 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config *csicfg,
> >  	switch (mbus_cfg->type) {
> >  	case V4L2_MBUS_PARALLEL:
> >  		csicfg->ext_vsync = 1;
> > -		csicfg->vsync_pol = (mbus_cfg->flags &
> > +		csicfg->vsync_pol = (mbus_cfg->bus.parallel.flags &
> >  				     V4L2_MBUS_VSYNC_ACTIVE_LOW) ? 1 : 0;
> > -		csicfg->hsync_pol = (mbus_cfg->flags &
> > +		csicfg->hsync_pol = (mbus_cfg->bus.parallel.flags &
> >  				     V4L2_MBUS_HSYNC_ACTIVE_LOW) ? 1 : 0;
> > -		csicfg->pixclk_pol = (mbus_cfg->flags &
> > +		csicfg->pixclk_pol = (mbus_cfg->bus.parallel.flags &
> >  				      V4L2_MBUS_PCLK_SAMPLE_FALLING) ? 1 : 0;
> >  		csicfg->clk_mode = IPU_CSI_CLK_MODE_GATED_CLK;
> >  		break;
> > diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
> > index d9a99fcfacb1..286f5017d9c3 100644
> > --- a/drivers/media/i2c/adv7180.c
> > +++ b/drivers/media/i2c/adv7180.c
> > @@ -784,7 +784,8 @@ static int adv7180_get_mbus_config(struct v4l2_subdev *sd,
> >
> >  	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
> >  		cfg->type = V4L2_MBUS_CSI2_DPHY;
> > -		cfg->flags = V4L2_MBUS_CSI2_1_LANE |
> > +		cfg->bus.mipi_csi2.num_data_lanes = 1;
> > +		cfg->bus.mipi_csi2.flags =
> >  				V4L2_MBUS_CSI2_CHANNEL_0 |
> >  				V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
> >  	} else {
> > @@ -792,8 +793,9 @@ static int adv7180_get_mbus_config(struct v4l2_subdev *sd,
> >  		 * The ADV7180 sensor supports BT.601/656 output modes.
> >  		 * The BT.656 is default and not yet configurable by s/w.
> >  		 */
> > -		cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING |
> > -				 V4L2_MBUS_DATA_ACTIVE_HIGH;
> > +		cfg->bus.parallel.flags = V4L2_MBUS_MASTER |
> > +					  V4L2_MBUS_PCLK_SAMPLE_RISING |
> > +					  V4L2_MBUS_DATA_ACTIVE_HIGH;
> >  		cfg->type = V4L2_MBUS_BT656;
> >  	}
> >
> > diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c
> > index 589e9644fcdc..bd4f3fe0e309 100644
> > --- a/drivers/media/i2c/adv748x/adv748x-csi2.c
> > +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
> > @@ -222,23 +222,7 @@ static int adv748x_csi2_get_mbus_config(struct v4l2_subdev *sd, unsigned int pad
> >  		return -EINVAL;
> >
> >  	config->type = V4L2_MBUS_CSI2_DPHY;
> > -	switch (tx->active_lanes) {
> > -	case 1:
> > -		config->flags = V4L2_MBUS_CSI2_1_LANE;
> > -		break;
> > -
> > -	case 2:
> > -		config->flags = V4L2_MBUS_CSI2_2_LANE;
> > -		break;
> > -
> > -	case 3:
> > -		config->flags = V4L2_MBUS_CSI2_3_LANE;
> > -		break;
> > -
> > -	case 4:
> > -		config->flags = V4L2_MBUS_CSI2_4_LANE;
> > -		break;
> > -	}
> > +	config->bus.mipi_csi2.num_data_lanes = tx->active_lanes;
> >
> >  	return 0;
> >  }
> > diff --git a/drivers/media/i2c/ml86v7667.c b/drivers/media/i2c/ml86v7667.c
> > index 4a1410ebb4c8..48cc0b0922f4 100644
> > --- a/drivers/media/i2c/ml86v7667.c
> > +++ b/drivers/media/i2c/ml86v7667.c
> > @@ -223,9 +223,10 @@ static int ml86v7667_get_mbus_config(struct v4l2_subdev *sd,
> >  				     unsigned int pad,
> >  				     struct v4l2_mbus_config *cfg)
> >  {
> > -	cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING |
> > -		     V4L2_MBUS_DATA_ACTIVE_HIGH;
> >  	cfg->type = V4L2_MBUS_BT656;
> > +	cfg->bus.parallel.flags = V4L2_MBUS_MASTER |
> > +				  V4L2_MBUS_PCLK_SAMPLE_RISING |
> > +				  V4L2_MBUS_DATA_ACTIVE_HIGH;
> >
> >  	return 0;
> >  }
> > diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
> > index c9f0bd997ea7..ad13b0c890c0 100644
> > --- a/drivers/media/i2c/mt9m001.c
> > +++ b/drivers/media/i2c/mt9m001.c
> > @@ -695,10 +695,12 @@ static int mt9m001_get_mbus_config(struct v4l2_subdev *sd,
> >  				   struct v4l2_mbus_config *cfg)
> >  {
> >  	/* MT9M001 has all capture_format parameters fixed */
> > -	cfg->flags = V4L2_MBUS_PCLK_SAMPLE_FALLING |
> > -		V4L2_MBUS_HSYNC_ACTIVE_HIGH | V4L2_MBUS_VSYNC_ACTIVE_HIGH |
> > -		V4L2_MBUS_DATA_ACTIVE_HIGH | V4L2_MBUS_MASTER;
> >  	cfg->type = V4L2_MBUS_PARALLEL;
> > +	cfg->bus.parallel.flags = V4L2_MBUS_PCLK_SAMPLE_FALLING |
> > +				  V4L2_MBUS_HSYNC_ACTIVE_HIGH |
> > +				  V4L2_MBUS_VSYNC_ACTIVE_HIGH |
> > +				  V4L2_MBUS_DATA_ACTIVE_HIGH |
> > +				  V4L2_MBUS_MASTER;
> >
> >  	return 0;
> >  }
> > diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
> > index 91a44359bcd3..6cf3ccf85d27 100644
> > --- a/drivers/media/i2c/mt9m111.c
> > +++ b/drivers/media/i2c/mt9m111.c
> > @@ -1143,15 +1143,17 @@ static int mt9m111_get_mbus_config(struct v4l2_subdev *sd,
> >  {
> >  	struct mt9m111 *mt9m111 = container_of(sd, struct mt9m111, subdev);
> >
> > -	cfg->flags = V4L2_MBUS_MASTER |
> > -		V4L2_MBUS_HSYNC_ACTIVE_HIGH | V4L2_MBUS_VSYNC_ACTIVE_HIGH |
> > -		V4L2_MBUS_DATA_ACTIVE_HIGH;
> > -
> > -	cfg->flags |= mt9m111->pclk_sample ? V4L2_MBUS_PCLK_SAMPLE_RISING :
> > -		V4L2_MBUS_PCLK_SAMPLE_FALLING;
> > -
> >  	cfg->type = V4L2_MBUS_PARALLEL;
> >
> > +	cfg->bus.parallel.flags = V4L2_MBUS_MASTER |
> > +				  V4L2_MBUS_HSYNC_ACTIVE_HIGH |
> > +				  V4L2_MBUS_VSYNC_ACTIVE_HIGH |
> > +				  V4L2_MBUS_DATA_ACTIVE_HIGH;
> > +
> > +	cfg->bus.parallel.flags |= mt9m111->pclk_sample ?
> > +				   V4L2_MBUS_PCLK_SAMPLE_RISING :
> > +				   V4L2_MBUS_PCLK_SAMPLE_FALLING;
> > +
> >  	return 0;
> >  }
> >
> > diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c
> > index 455a627e35a0..b2654b32bc62 100644
> > --- a/drivers/media/i2c/ov6650.c
> > +++ b/drivers/media/i2c/ov6650.c
> > @@ -932,15 +932,15 @@ static int ov6650_get_mbus_config(struct v4l2_subdev *sd,
> >  	if (ret)
> >  		return ret;
> >
> > -	cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_DATA_ACTIVE_HIGH
> > -		   | ((comj & COMJ_VSYNC_HIGH)  ? V4L2_MBUS_VSYNC_ACTIVE_HIGH
> > -						: V4L2_MBUS_VSYNC_ACTIVE_LOW)
> > -		   | ((comf & COMF_HREF_LOW)    ? V4L2_MBUS_HSYNC_ACTIVE_LOW
> > -						: V4L2_MBUS_HSYNC_ACTIVE_HIGH)
> > -		   | ((comj & COMJ_PCLK_RISING) ? V4L2_MBUS_PCLK_SAMPLE_RISING
> > -						: V4L2_MBUS_PCLK_SAMPLE_FALLING);
> >  	cfg->type = V4L2_MBUS_PARALLEL;
> >
> > +	cfg->bus.parallel.flags = V4L2_MBUS_MASTER | V4L2_MBUS_DATA_ACTIVE_HIGH
> > +		| ((comj & COMJ_VSYNC_HIGH)  ? V4L2_MBUS_VSYNC_ACTIVE_HIGH
> > +					     : V4L2_MBUS_VSYNC_ACTIVE_LOW)
> > +		| ((comf & COMF_HREF_LOW)    ? V4L2_MBUS_HSYNC_ACTIVE_LOW
> > +					     : V4L2_MBUS_HSYNC_ACTIVE_HIGH)
> > +		| ((comj & COMJ_PCLK_RISING) ? V4L2_MBUS_PCLK_SAMPLE_RISING
> > +					     : V4L2_MBUS_PCLK_SAMPLE_FALLING);
> >  	return 0;
> >  }
> >
> > diff --git a/drivers/media/i2c/ov9640.c b/drivers/media/i2c/ov9640.c
> > index 0bab8c2cf160..9f44ed52d164 100644
> > --- a/drivers/media/i2c/ov9640.c
> > +++ b/drivers/media/i2c/ov9640.c
> > @@ -652,10 +652,12 @@ static int ov9640_get_mbus_config(struct v4l2_subdev *sd,
> >  				  unsigned int pad,
> >  				  struct v4l2_mbus_config *cfg)
> >  {
> > -	cfg->flags = V4L2_MBUS_PCLK_SAMPLE_RISING | V4L2_MBUS_MASTER |
> > -		V4L2_MBUS_VSYNC_ACTIVE_HIGH | V4L2_MBUS_HSYNC_ACTIVE_HIGH |
> > -		V4L2_MBUS_DATA_ACTIVE_HIGH;
> >  	cfg->type = V4L2_MBUS_PARALLEL;
> > +	cfg->bus.parallel.flags = V4L2_MBUS_PCLK_SAMPLE_RISING |
> > +				  V4L2_MBUS_MASTER |
> > +				  V4L2_MBUS_VSYNC_ACTIVE_HIGH |
> > +				  V4L2_MBUS_HSYNC_ACTIVE_HIGH |
> > +				  V4L2_MBUS_DATA_ACTIVE_HIGH;
> >
> >  	return 0;
> >  }
> > diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
> > index 40512004afba..dfbc42675143 100644
> > --- a/drivers/media/i2c/tc358743.c
> > +++ b/drivers/media/i2c/tc358743.c
> > @@ -1613,24 +1613,8 @@ static int tc358743_get_mbus_config(struct v4l2_subdev *sd,
> >  	cfg->type = V4L2_MBUS_CSI2_DPHY;
> >
> >  	/* Support for non-continuous CSI-2 clock is missing in the driver */
> > -	cfg->flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
> > -
> > -	switch (state->csi_lanes_in_use) {
> > -	case 1:
> > -		cfg->flags |= V4L2_MBUS_CSI2_1_LANE;
> > -		break;
> > -	case 2:
> > -		cfg->flags |= V4L2_MBUS_CSI2_2_LANE;
> > -		break;
> > -	case 3:
> > -		cfg->flags |= V4L2_MBUS_CSI2_3_LANE;
> > -		break;
> > -	case 4:
> > -		cfg->flags |= V4L2_MBUS_CSI2_4_LANE;
> > -		break;
> > -	default:
> > -		return -EINVAL;
> > -	}
> > +	cfg->bus.mipi_csi2.flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
> > +	cfg->bus.mipi_csi2.num_data_lanes = state->csi_lanes_in_use;
> >
> >  	return 0;
> >  }
> > diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> > index 4b16ffcaef98..65472438444b 100644
> > --- a/drivers/media/i2c/tvp5150.c
> > +++ b/drivers/media/i2c/tvp5150.c
> > @@ -1198,8 +1198,10 @@ static int tvp5150_get_mbus_config(struct v4l2_subdev *sd,
> >  	struct tvp5150 *decoder = to_tvp5150(sd);
> >
> >  	cfg->type = decoder->mbus_type;
> > -	cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING
> > -		   | V4L2_MBUS_FIELD_EVEN_LOW | V4L2_MBUS_DATA_ACTIVE_HIGH;
> > +	cfg->bus.parallel.flags = V4L2_MBUS_MASTER
> > +				| V4L2_MBUS_PCLK_SAMPLE_RISING
> > +				| V4L2_MBUS_FIELD_EVEN_LOW
> > +				| V4L2_MBUS_DATA_ACTIVE_HIGH;
> >
> >  	return 0;
> >  }
> > diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c
> > index b5644cf37fe9..35145e3348f0 100644
> > --- a/drivers/media/platform/pxa_camera.c
> > +++ b/drivers/media/platform/pxa_camera.c
> > @@ -1587,24 +1587,26 @@ static int pxa_camera_set_bus_param(struct pxa_camera_dev *pcdev)
> >  	 * PXA does not support V4L2_MBUS_DATA_ACTIVE_LOW and the bus mastering
> >  	 * roles should match.
> >  	 */
> > -	if (cfg.flags != mbus_config) {
> > +	if (cfg.bus.parallel.flags != mbus_config) {
> >  		unsigned int pxa_mbus_role = mbus_config & (V4L2_MBUS_MASTER |
> >  							    V4L2_MBUS_SLAVE);
> > -		if (pxa_mbus_role != (cfg.flags & (V4L2_MBUS_MASTER |
> > -						   V4L2_MBUS_SLAVE))) {
> > +		unsigned int flags = cfg.bus.parallel.flags;
> > +
> > +		if (pxa_mbus_role != (flags & (V4L2_MBUS_MASTER |
> > +					       V4L2_MBUS_SLAVE))) {
> >  			dev_err(pcdev_to_dev(pcdev),
> >  				"Unsupported mbus configuration: bus mastering\n");
> >  			return -EINVAL;
> >  		}
> >
> > -		if (cfg.flags & V4L2_MBUS_DATA_ACTIVE_LOW) {
> > +		if (flags & V4L2_MBUS_DATA_ACTIVE_LOW) {
> >  			dev_err(pcdev_to_dev(pcdev),
> >  				"Unsupported mbus configuration: DATA_ACTIVE_LOW\n");
> >  			return -EINVAL;
> >  		}
> >  	}
> >
> > -	pxa_camera_setup_cicr(pcdev, cfg.flags, pixfmt);
> > +	pxa_camera_setup_cicr(pcdev, cfg.bus.parallel.flags, pixfmt);
> >
> >  	return 0;
> >  }
> > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > index 8c939cb3073d..cbac5801720b 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > @@ -603,7 +603,6 @@ static int rcsi2_get_active_lanes(struct rcar_csi2 *priv,
> >  				  unsigned int *lanes)
> >  {
> >  	struct v4l2_mbus_config mbus_config = { 0 };
> > -	unsigned int num_lanes = UINT_MAX;
> >  	int ret;
> >
> >  	*lanes = priv->lanes;
> > @@ -626,23 +625,14 @@ static int rcsi2_get_active_lanes(struct rcar_csi2 *priv,
> >  		return -EINVAL;
> >  	}
> >
> > -	if (mbus_config.flags & V4L2_MBUS_CSI2_1_LANE)
> > -		num_lanes = 1;
> > -	else if (mbus_config.flags & V4L2_MBUS_CSI2_2_LANE)
> > -		num_lanes = 2;
> > -	else if (mbus_config.flags & V4L2_MBUS_CSI2_3_LANE)
> > -		num_lanes = 3;
> > -	else if (mbus_config.flags & V4L2_MBUS_CSI2_4_LANE)
> > -		num_lanes = 4;
> > -
> > -	if (num_lanes > priv->lanes) {
> > +	if (mbus_config.bus.mipi_csi2.num_data_lanes > priv->lanes) {
> >  		dev_err(priv->dev,
> >  			"Unsupported mbus config: too many data lanes %u\n",
> > -			num_lanes);
> > +			mbus_config.bus.mipi_csi2.num_data_lanes);
> >  		return -EINVAL;
> >  	}
> >
> > -	*lanes = num_lanes;
> > +	*lanes = mbus_config.bus.mipi_csi2.num_data_lanes;
> >
> >  	return 0;
> >  }
> > diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> > index bd7f156f2d52..b2b1f4dd41d7 100644
> > --- a/drivers/staging/media/imx/imx-media-csi.c
> > +++ b/drivers/staging/media/imx/imx-media-csi.c
> > @@ -718,9 +718,10 @@ static int csi_setup(struct csi_priv *priv)
> >
> >  	/* compose mbus_config from the upstream endpoint */
> >  	mbus_cfg.type = priv->upstream_ep.bus_type;
> > -	mbus_cfg.flags = is_parallel_bus(&priv->upstream_ep) ?
> > -		priv->upstream_ep.bus.parallel.flags :
> > -		priv->upstream_ep.bus.mipi_csi2.flags;
> > +	if (is_parallel_bus(&priv->upstream_ep))
> > +		mbus_cfg.bus.parallel = priv->upstream_ep.bus.parallel;
> > +	else
> > +		mbus_cfg.bus.mipi_csi2 = priv->upstream_ep.bus.mipi_csi2;
> >
> >  	if_fmt = *infmt;
> >  	crop = priv->crop;
> > diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
> > index 558b256ac935..c4cb558a85c6 100644
> > --- a/drivers/staging/media/imx/imx6-mipi-csi2.c
> > +++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
> > @@ -303,7 +303,6 @@ static void csi2ipu_gasket_init(struct csi2_dev *csi2)
> >  static int csi2_get_active_lanes(struct csi2_dev *csi2, unsigned int *lanes)
> >  {
> >  	struct v4l2_mbus_config mbus_config = { 0 };
> > -	unsigned int num_lanes = UINT_MAX;
> >  	int ret;
> >
> >  	*lanes = csi2->data_lanes;
> > @@ -326,32 +325,14 @@ static int csi2_get_active_lanes(struct csi2_dev *csi2, unsigned int *lanes)
> >  		return -EINVAL;
> >  	}
> >
> > -	switch (mbus_config.flags & V4L2_MBUS_CSI2_LANES) {
> > -	case V4L2_MBUS_CSI2_1_LANE:
> > -		num_lanes = 1;
> > -		break;
> > -	case V4L2_MBUS_CSI2_2_LANE:
> > -		num_lanes = 2;
> > -		break;
> > -	case V4L2_MBUS_CSI2_3_LANE:
> > -		num_lanes = 3;
> > -		break;
> > -	case V4L2_MBUS_CSI2_4_LANE:
> > -		num_lanes = 4;
> > -		break;
> > -	default:
> > -		num_lanes = csi2->data_lanes;
> > -		break;
> > -	}
> > -
> > -	if (num_lanes > csi2->data_lanes) {
> > +	if (mbus_config.bus.mipi_csi2.num_data_lanes > csi2->data_lanes) {
> >  		dev_err(csi2->dev,
> >  			"Unsupported mbus config: too many data lanes %u\n",
> > -			num_lanes);
> > +			mbus_config.bus.mipi_csi2.num_data_lanes);
> >  		return -EINVAL;
> >  	}
> >
> > -	*lanes = num_lanes;
> > +	*lanes = mbus_config.bus.mipi_csi2.num_data_lanes;
> >
> >  	return 0;
> >  }
> > diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
> > index 9c4970fbd8ea..269aaf57ba32 100644
> > --- a/include/media/v4l2-mediabus.h
> > +++ b/include/media/v4l2-mediabus.h
> > @@ -166,12 +166,26 @@ enum v4l2_mbus_type {
> >
> >  /**
> >   * struct v4l2_mbus_config - media bus configuration
> > - * @type:	in: interface type
> > - * @flags:	in / out: configuration flags, depending on @type
> > + * @type: interface type
> > + * @bus: bus configuration data structure
> > + * @bus.parallel: embedded &struct v4l2_mbus_config_parallel.
> > + *		  Used if the bus is parallel or BT.656.
> > + * @bus.mipi_csi1: embedded &struct v4l2_mbus_config_mipi_csi1.
> > + *		   Used if the bus is MIPI Alliance's Camera Serial
> > + *		   Interface version 1 (MIPI CSI1) or Standard
> > + *		   Mobile Imaging Architecture's Compact Camera Port 2
> > + *		   (SMIA CCP2).
> > + * @bus.mipi_csi2: embedded &struct v4l2_mbus_config_mipi_csi2.
> > + *		   Used if the bus is MIPI Alliance's Camera Serial
> > + *		   Interface version 2 (MIPI CSI2).
> >   */
> >  struct v4l2_mbus_config {
> >  	enum v4l2_mbus_type type;
> > -	unsigned int flags;
> > +	union {
> > +		struct v4l2_mbus_config_parallel parallel;
> > +		struct v4l2_mbus_config_mipi_csi1 mipi_csi1;
> > +		struct v4l2_mbus_config_mipi_csi2 mipi_csi2;
> 
> I wonder if we would be able to use a single structure for D-PHY and
> C-PHY. The clock lane index does not make much sense for the latter,
> but it was already there, so it can be changed on top in case it is
> required

I'm not sure to follow you here. CSI1 and CSI2 are not D-PHY versus
C-PHY.

> Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> 
> > +	} bus;
> >  };
> >
> >  /**
Jacopo Mondi Feb. 15, 2022, 11:28 a.m. UTC | #3 
On Tue, Feb 15, 2022 at 01:15:40PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Tue, Feb 15, 2022 at 12:13:16PM +0100, Jacopo Mondi wrote:
> > On Mon, Jan 03, 2022 at 06:24:11PM +0200, Laurent Pinchart wrote:
> > > The media bus configuration is specified through a set of flags, some of
> > > which being mutually exclusive. This doesn't scale to express more
> > > complex configurations. Improve the API by replacing the single flags
> > > field in v4l2_mbus_config by a union of v4l2_mbus_config_* structures.
> > > The flags themselves are still used in those structures, so they are
> > > kept here. Drivers are however updated to use structure fields instead
> > > of flags when already possible.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > ---
> > >  drivers/gpu/ipu-v3/ipu-csi.c                |  6 ++---
> > >  drivers/media/i2c/adv7180.c                 |  8 ++++---
> > >  drivers/media/i2c/adv748x/adv748x-csi2.c    | 18 +--------------
> > >  drivers/media/i2c/ml86v7667.c               |  5 +++--
> > >  drivers/media/i2c/mt9m001.c                 |  8 ++++---
> > >  drivers/media/i2c/mt9m111.c                 | 16 +++++++------
> > >  drivers/media/i2c/ov6650.c                  | 14 ++++++------
> > >  drivers/media/i2c/ov9640.c                  |  8 ++++---
> > >  drivers/media/i2c/tc358743.c                | 20 ++---------------
> > >  drivers/media/i2c/tvp5150.c                 |  6 +++--
> > >  drivers/media/platform/pxa_camera.c         | 12 +++++-----
> > >  drivers/media/platform/rcar-vin/rcar-csi2.c | 16 +++----------
> > >  drivers/staging/media/imx/imx-media-csi.c   |  7 +++---
> > >  drivers/staging/media/imx/imx6-mipi-csi2.c  | 25 +++------------------
> >
> > drivers conversions looks good to me
> >
> > >  include/media/v4l2-mediabus.h               | 20 ++++++++++++++---
> > >  15 files changed, 78 insertions(+), 111 deletions(-)
> > >
> > > diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
> > > index a9639d098893..778bc26d3ba5 100644
> > > --- a/drivers/gpu/ipu-v3/ipu-csi.c
> > > +++ b/drivers/gpu/ipu-v3/ipu-csi.c
> > > @@ -357,11 +357,11 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config *csicfg,
> > >  	switch (mbus_cfg->type) {
> > >  	case V4L2_MBUS_PARALLEL:
> > >  		csicfg->ext_vsync = 1;
> > > -		csicfg->vsync_pol = (mbus_cfg->flags &
> > > +		csicfg->vsync_pol = (mbus_cfg->bus.parallel.flags &
> > >  				     V4L2_MBUS_VSYNC_ACTIVE_LOW) ? 1 : 0;
> > > -		csicfg->hsync_pol = (mbus_cfg->flags &
> > > +		csicfg->hsync_pol = (mbus_cfg->bus.parallel.flags &
> > >  				     V4L2_MBUS_HSYNC_ACTIVE_LOW) ? 1 : 0;
> > > -		csicfg->pixclk_pol = (mbus_cfg->flags &
> > > +		csicfg->pixclk_pol = (mbus_cfg->bus.parallel.flags &
> > >  				      V4L2_MBUS_PCLK_SAMPLE_FALLING) ? 1 : 0;
> > >  		csicfg->clk_mode = IPU_CSI_CLK_MODE_GATED_CLK;
> > >  		break;
> > > diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
> > > index d9a99fcfacb1..286f5017d9c3 100644
> > > --- a/drivers/media/i2c/adv7180.c
> > > +++ b/drivers/media/i2c/adv7180.c
> > > @@ -784,7 +784,8 @@ static int adv7180_get_mbus_config(struct v4l2_subdev *sd,
> > >
> > >  	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
> > >  		cfg->type = V4L2_MBUS_CSI2_DPHY;
> > > -		cfg->flags = V4L2_MBUS_CSI2_1_LANE |
> > > +		cfg->bus.mipi_csi2.num_data_lanes = 1;
> > > +		cfg->bus.mipi_csi2.flags =
> > >  				V4L2_MBUS_CSI2_CHANNEL_0 |
> > >  				V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
> > >  	} else {
> > > @@ -792,8 +793,9 @@ static int adv7180_get_mbus_config(struct v4l2_subdev *sd,
> > >  		 * The ADV7180 sensor supports BT.601/656 output modes.
> > >  		 * The BT.656 is default and not yet configurable by s/w.
> > >  		 */
> > > -		cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING |
> > > -				 V4L2_MBUS_DATA_ACTIVE_HIGH;
> > > +		cfg->bus.parallel.flags = V4L2_MBUS_MASTER |
> > > +					  V4L2_MBUS_PCLK_SAMPLE_RISING |
> > > +					  V4L2_MBUS_DATA_ACTIVE_HIGH;
> > >  		cfg->type = V4L2_MBUS_BT656;
> > >  	}
> > >
> > > diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c
> > > index 589e9644fcdc..bd4f3fe0e309 100644
> > > --- a/drivers/media/i2c/adv748x/adv748x-csi2.c
> > > +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
> > > @@ -222,23 +222,7 @@ static int adv748x_csi2_get_mbus_config(struct v4l2_subdev *sd, unsigned int pad
> > >  		return -EINVAL;
> > >
> > >  	config->type = V4L2_MBUS_CSI2_DPHY;
> > > -	switch (tx->active_lanes) {
> > > -	case 1:
> > > -		config->flags = V4L2_MBUS_CSI2_1_LANE;
> > > -		break;
> > > -
> > > -	case 2:
> > > -		config->flags = V4L2_MBUS_CSI2_2_LANE;
> > > -		break;
> > > -
> > > -	case 3:
> > > -		config->flags = V4L2_MBUS_CSI2_3_LANE;
> > > -		break;
> > > -
> > > -	case 4:
> > > -		config->flags = V4L2_MBUS_CSI2_4_LANE;
> > > -		break;
> > > -	}
> > > +	config->bus.mipi_csi2.num_data_lanes = tx->active_lanes;
> > >
> > >  	return 0;
> > >  }
> > > diff --git a/drivers/media/i2c/ml86v7667.c b/drivers/media/i2c/ml86v7667.c
> > > index 4a1410ebb4c8..48cc0b0922f4 100644
> > > --- a/drivers/media/i2c/ml86v7667.c
> > > +++ b/drivers/media/i2c/ml86v7667.c
> > > @@ -223,9 +223,10 @@ static int ml86v7667_get_mbus_config(struct v4l2_subdev *sd,
> > >  				     unsigned int pad,
> > >  				     struct v4l2_mbus_config *cfg)
> > >  {
> > > -	cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING |
> > > -		     V4L2_MBUS_DATA_ACTIVE_HIGH;
> > >  	cfg->type = V4L2_MBUS_BT656;
> > > +	cfg->bus.parallel.flags = V4L2_MBUS_MASTER |
> > > +				  V4L2_MBUS_PCLK_SAMPLE_RISING |
> > > +				  V4L2_MBUS_DATA_ACTIVE_HIGH;
> > >
> > >  	return 0;
> > >  }
> > > diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
> > > index c9f0bd997ea7..ad13b0c890c0 100644
> > > --- a/drivers/media/i2c/mt9m001.c
> > > +++ b/drivers/media/i2c/mt9m001.c
> > > @@ -695,10 +695,12 @@ static int mt9m001_get_mbus_config(struct v4l2_subdev *sd,
> > >  				   struct v4l2_mbus_config *cfg)
> > >  {
> > >  	/* MT9M001 has all capture_format parameters fixed */
> > > -	cfg->flags = V4L2_MBUS_PCLK_SAMPLE_FALLING |
> > > -		V4L2_MBUS_HSYNC_ACTIVE_HIGH | V4L2_MBUS_VSYNC_ACTIVE_HIGH |
> > > -		V4L2_MBUS_DATA_ACTIVE_HIGH | V4L2_MBUS_MASTER;
> > >  	cfg->type = V4L2_MBUS_PARALLEL;
> > > +	cfg->bus.parallel.flags = V4L2_MBUS_PCLK_SAMPLE_FALLING |
> > > +				  V4L2_MBUS_HSYNC_ACTIVE_HIGH |
> > > +				  V4L2_MBUS_VSYNC_ACTIVE_HIGH |
> > > +				  V4L2_MBUS_DATA_ACTIVE_HIGH |
> > > +				  V4L2_MBUS_MASTER;
> > >
> > >  	return 0;
> > >  }
> > > diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
> > > index 91a44359bcd3..6cf3ccf85d27 100644
> > > --- a/drivers/media/i2c/mt9m111.c
> > > +++ b/drivers/media/i2c/mt9m111.c
> > > @@ -1143,15 +1143,17 @@ static int mt9m111_get_mbus_config(struct v4l2_subdev *sd,
> > >  {
> > >  	struct mt9m111 *mt9m111 = container_of(sd, struct mt9m111, subdev);
> > >
> > > -	cfg->flags = V4L2_MBUS_MASTER |
> > > -		V4L2_MBUS_HSYNC_ACTIVE_HIGH | V4L2_MBUS_VSYNC_ACTIVE_HIGH |
> > > -		V4L2_MBUS_DATA_ACTIVE_HIGH;
> > > -
> > > -	cfg->flags |= mt9m111->pclk_sample ? V4L2_MBUS_PCLK_SAMPLE_RISING :
> > > -		V4L2_MBUS_PCLK_SAMPLE_FALLING;
> > > -
> > >  	cfg->type = V4L2_MBUS_PARALLEL;
> > >
> > > +	cfg->bus.parallel.flags = V4L2_MBUS_MASTER |
> > > +				  V4L2_MBUS_HSYNC_ACTIVE_HIGH |
> > > +				  V4L2_MBUS_VSYNC_ACTIVE_HIGH |
> > > +				  V4L2_MBUS_DATA_ACTIVE_HIGH;
> > > +
> > > +	cfg->bus.parallel.flags |= mt9m111->pclk_sample ?
> > > +				   V4L2_MBUS_PCLK_SAMPLE_RISING :
> > > +				   V4L2_MBUS_PCLK_SAMPLE_FALLING;
> > > +
> > >  	return 0;
> > >  }
> > >
> > > diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c
> > > index 455a627e35a0..b2654b32bc62 100644
> > > --- a/drivers/media/i2c/ov6650.c
> > > +++ b/drivers/media/i2c/ov6650.c
> > > @@ -932,15 +932,15 @@ static int ov6650_get_mbus_config(struct v4l2_subdev *sd,
> > >  	if (ret)
> > >  		return ret;
> > >
> > > -	cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_DATA_ACTIVE_HIGH
> > > -		   | ((comj & COMJ_VSYNC_HIGH)  ? V4L2_MBUS_VSYNC_ACTIVE_HIGH
> > > -						: V4L2_MBUS_VSYNC_ACTIVE_LOW)
> > > -		   | ((comf & COMF_HREF_LOW)    ? V4L2_MBUS_HSYNC_ACTIVE_LOW
> > > -						: V4L2_MBUS_HSYNC_ACTIVE_HIGH)
> > > -		   | ((comj & COMJ_PCLK_RISING) ? V4L2_MBUS_PCLK_SAMPLE_RISING
> > > -						: V4L2_MBUS_PCLK_SAMPLE_FALLING);
> > >  	cfg->type = V4L2_MBUS_PARALLEL;
> > >
> > > +	cfg->bus.parallel.flags = V4L2_MBUS_MASTER | V4L2_MBUS_DATA_ACTIVE_HIGH
> > > +		| ((comj & COMJ_VSYNC_HIGH)  ? V4L2_MBUS_VSYNC_ACTIVE_HIGH
> > > +					     : V4L2_MBUS_VSYNC_ACTIVE_LOW)
> > > +		| ((comf & COMF_HREF_LOW)    ? V4L2_MBUS_HSYNC_ACTIVE_LOW
> > > +					     : V4L2_MBUS_HSYNC_ACTIVE_HIGH)
> > > +		| ((comj & COMJ_PCLK_RISING) ? V4L2_MBUS_PCLK_SAMPLE_RISING
> > > +					     : V4L2_MBUS_PCLK_SAMPLE_FALLING);
> > >  	return 0;
> > >  }
> > >
> > > diff --git a/drivers/media/i2c/ov9640.c b/drivers/media/i2c/ov9640.c
> > > index 0bab8c2cf160..9f44ed52d164 100644
> > > --- a/drivers/media/i2c/ov9640.c
> > > +++ b/drivers/media/i2c/ov9640.c
> > > @@ -652,10 +652,12 @@ static int ov9640_get_mbus_config(struct v4l2_subdev *sd,
> > >  				  unsigned int pad,
> > >  				  struct v4l2_mbus_config *cfg)
> > >  {
> > > -	cfg->flags = V4L2_MBUS_PCLK_SAMPLE_RISING | V4L2_MBUS_MASTER |
> > > -		V4L2_MBUS_VSYNC_ACTIVE_HIGH | V4L2_MBUS_HSYNC_ACTIVE_HIGH |
> > > -		V4L2_MBUS_DATA_ACTIVE_HIGH;
> > >  	cfg->type = V4L2_MBUS_PARALLEL;
> > > +	cfg->bus.parallel.flags = V4L2_MBUS_PCLK_SAMPLE_RISING |
> > > +				  V4L2_MBUS_MASTER |
> > > +				  V4L2_MBUS_VSYNC_ACTIVE_HIGH |
> > > +				  V4L2_MBUS_HSYNC_ACTIVE_HIGH |
> > > +				  V4L2_MBUS_DATA_ACTIVE_HIGH;
> > >
> > >  	return 0;
> > >  }
> > > diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
> > > index 40512004afba..dfbc42675143 100644
> > > --- a/drivers/media/i2c/tc358743.c
> > > +++ b/drivers/media/i2c/tc358743.c
> > > @@ -1613,24 +1613,8 @@ static int tc358743_get_mbus_config(struct v4l2_subdev *sd,
> > >  	cfg->type = V4L2_MBUS_CSI2_DPHY;
> > >
> > >  	/* Support for non-continuous CSI-2 clock is missing in the driver */
> > > -	cfg->flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
> > > -
> > > -	switch (state->csi_lanes_in_use) {
> > > -	case 1:
> > > -		cfg->flags |= V4L2_MBUS_CSI2_1_LANE;
> > > -		break;
> > > -	case 2:
> > > -		cfg->flags |= V4L2_MBUS_CSI2_2_LANE;
> > > -		break;
> > > -	case 3:
> > > -		cfg->flags |= V4L2_MBUS_CSI2_3_LANE;
> > > -		break;
> > > -	case 4:
> > > -		cfg->flags |= V4L2_MBUS_CSI2_4_LANE;
> > > -		break;
> > > -	default:
> > > -		return -EINVAL;
> > > -	}
> > > +	cfg->bus.mipi_csi2.flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
> > > +	cfg->bus.mipi_csi2.num_data_lanes = state->csi_lanes_in_use;
> > >
> > >  	return 0;
> > >  }
> > > diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> > > index 4b16ffcaef98..65472438444b 100644
> > > --- a/drivers/media/i2c/tvp5150.c
> > > +++ b/drivers/media/i2c/tvp5150.c
> > > @@ -1198,8 +1198,10 @@ static int tvp5150_get_mbus_config(struct v4l2_subdev *sd,
> > >  	struct tvp5150 *decoder = to_tvp5150(sd);
> > >
> > >  	cfg->type = decoder->mbus_type;
> > > -	cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING
> > > -		   | V4L2_MBUS_FIELD_EVEN_LOW | V4L2_MBUS_DATA_ACTIVE_HIGH;
> > > +	cfg->bus.parallel.flags = V4L2_MBUS_MASTER
> > > +				| V4L2_MBUS_PCLK_SAMPLE_RISING
> > > +				| V4L2_MBUS_FIELD_EVEN_LOW
> > > +				| V4L2_MBUS_DATA_ACTIVE_HIGH;
> > >
> > >  	return 0;
> > >  }
> > > diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c
> > > index b5644cf37fe9..35145e3348f0 100644
> > > --- a/drivers/media/platform/pxa_camera.c
> > > +++ b/drivers/media/platform/pxa_camera.c
> > > @@ -1587,24 +1587,26 @@ static int pxa_camera_set_bus_param(struct pxa_camera_dev *pcdev)
> > >  	 * PXA does not support V4L2_MBUS_DATA_ACTIVE_LOW and the bus mastering
> > >  	 * roles should match.
> > >  	 */
> > > -	if (cfg.flags != mbus_config) {
> > > +	if (cfg.bus.parallel.flags != mbus_config) {
> > >  		unsigned int pxa_mbus_role = mbus_config & (V4L2_MBUS_MASTER |
> > >  							    V4L2_MBUS_SLAVE);
> > > -		if (pxa_mbus_role != (cfg.flags & (V4L2_MBUS_MASTER |
> > > -						   V4L2_MBUS_SLAVE))) {
> > > +		unsigned int flags = cfg.bus.parallel.flags;
> > > +
> > > +		if (pxa_mbus_role != (flags & (V4L2_MBUS_MASTER |
> > > +					       V4L2_MBUS_SLAVE))) {
> > >  			dev_err(pcdev_to_dev(pcdev),
> > >  				"Unsupported mbus configuration: bus mastering\n");
> > >  			return -EINVAL;
> > >  		}
> > >
> > > -		if (cfg.flags & V4L2_MBUS_DATA_ACTIVE_LOW) {
> > > +		if (flags & V4L2_MBUS_DATA_ACTIVE_LOW) {
> > >  			dev_err(pcdev_to_dev(pcdev),
> > >  				"Unsupported mbus configuration: DATA_ACTIVE_LOW\n");
> > >  			return -EINVAL;
> > >  		}
> > >  	}
> > >
> > > -	pxa_camera_setup_cicr(pcdev, cfg.flags, pixfmt);
> > > +	pxa_camera_setup_cicr(pcdev, cfg.bus.parallel.flags, pixfmt);
> > >
> > >  	return 0;
> > >  }
> > > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > > index 8c939cb3073d..cbac5801720b 100644
> > > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> > > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > > @@ -603,7 +603,6 @@ static int rcsi2_get_active_lanes(struct rcar_csi2 *priv,
> > >  				  unsigned int *lanes)
> > >  {
> > >  	struct v4l2_mbus_config mbus_config = { 0 };
> > > -	unsigned int num_lanes = UINT_MAX;
> > >  	int ret;
> > >
> > >  	*lanes = priv->lanes;
> > > @@ -626,23 +625,14 @@ static int rcsi2_get_active_lanes(struct rcar_csi2 *priv,
> > >  		return -EINVAL;
> > >  	}
> > >
> > > -	if (mbus_config.flags & V4L2_MBUS_CSI2_1_LANE)
> > > -		num_lanes = 1;
> > > -	else if (mbus_config.flags & V4L2_MBUS_CSI2_2_LANE)
> > > -		num_lanes = 2;
> > > -	else if (mbus_config.flags & V4L2_MBUS_CSI2_3_LANE)
> > > -		num_lanes = 3;
> > > -	else if (mbus_config.flags & V4L2_MBUS_CSI2_4_LANE)
> > > -		num_lanes = 4;
> > > -
> > > -	if (num_lanes > priv->lanes) {
> > > +	if (mbus_config.bus.mipi_csi2.num_data_lanes > priv->lanes) {
> > >  		dev_err(priv->dev,
> > >  			"Unsupported mbus config: too many data lanes %u\n",
> > > -			num_lanes);
> > > +			mbus_config.bus.mipi_csi2.num_data_lanes);
> > >  		return -EINVAL;
> > >  	}
> > >
> > > -	*lanes = num_lanes;
> > > +	*lanes = mbus_config.bus.mipi_csi2.num_data_lanes;
> > >
> > >  	return 0;
> > >  }
> > > diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> > > index bd7f156f2d52..b2b1f4dd41d7 100644
> > > --- a/drivers/staging/media/imx/imx-media-csi.c
> > > +++ b/drivers/staging/media/imx/imx-media-csi.c
> > > @@ -718,9 +718,10 @@ static int csi_setup(struct csi_priv *priv)
> > >
> > >  	/* compose mbus_config from the upstream endpoint */
> > >  	mbus_cfg.type = priv->upstream_ep.bus_type;
> > > -	mbus_cfg.flags = is_parallel_bus(&priv->upstream_ep) ?
> > > -		priv->upstream_ep.bus.parallel.flags :
> > > -		priv->upstream_ep.bus.mipi_csi2.flags;
> > > +	if (is_parallel_bus(&priv->upstream_ep))
> > > +		mbus_cfg.bus.parallel = priv->upstream_ep.bus.parallel;
> > > +	else
> > > +		mbus_cfg.bus.mipi_csi2 = priv->upstream_ep.bus.mipi_csi2;
> > >
> > >  	if_fmt = *infmt;
> > >  	crop = priv->crop;
> > > diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
> > > index 558b256ac935..c4cb558a85c6 100644
> > > --- a/drivers/staging/media/imx/imx6-mipi-csi2.c
> > > +++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
> > > @@ -303,7 +303,6 @@ static void csi2ipu_gasket_init(struct csi2_dev *csi2)
> > >  static int csi2_get_active_lanes(struct csi2_dev *csi2, unsigned int *lanes)
> > >  {
> > >  	struct v4l2_mbus_config mbus_config = { 0 };
> > > -	unsigned int num_lanes = UINT_MAX;
> > >  	int ret;
> > >
> > >  	*lanes = csi2->data_lanes;
> > > @@ -326,32 +325,14 @@ static int csi2_get_active_lanes(struct csi2_dev *csi2, unsigned int *lanes)
> > >  		return -EINVAL;
> > >  	}
> > >
> > > -	switch (mbus_config.flags & V4L2_MBUS_CSI2_LANES) {
> > > -	case V4L2_MBUS_CSI2_1_LANE:
> > > -		num_lanes = 1;
> > > -		break;
> > > -	case V4L2_MBUS_CSI2_2_LANE:
> > > -		num_lanes = 2;
> > > -		break;
> > > -	case V4L2_MBUS_CSI2_3_LANE:
> > > -		num_lanes = 3;
> > > -		break;
> > > -	case V4L2_MBUS_CSI2_4_LANE:
> > > -		num_lanes = 4;
> > > -		break;
> > > -	default:
> > > -		num_lanes = csi2->data_lanes;
> > > -		break;
> > > -	}
> > > -
> > > -	if (num_lanes > csi2->data_lanes) {
> > > +	if (mbus_config.bus.mipi_csi2.num_data_lanes > csi2->data_lanes) {
> > >  		dev_err(csi2->dev,
> > >  			"Unsupported mbus config: too many data lanes %u\n",
> > > -			num_lanes);
> > > +			mbus_config.bus.mipi_csi2.num_data_lanes);
> > >  		return -EINVAL;
> > >  	}
> > >
> > > -	*lanes = num_lanes;
> > > +	*lanes = mbus_config.bus.mipi_csi2.num_data_lanes;
> > >
> > >  	return 0;
> > >  }
> > > diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
> > > index 9c4970fbd8ea..269aaf57ba32 100644
> > > --- a/include/media/v4l2-mediabus.h
> > > +++ b/include/media/v4l2-mediabus.h
> > > @@ -166,12 +166,26 @@ enum v4l2_mbus_type {
> > >
> > >  /**
> > >   * struct v4l2_mbus_config - media bus configuration
> > > - * @type:	in: interface type
> > > - * @flags:	in / out: configuration flags, depending on @type
> > > + * @type: interface type
> > > + * @bus: bus configuration data structure
> > > + * @bus.parallel: embedded &struct v4l2_mbus_config_parallel.
> > > + *		  Used if the bus is parallel or BT.656.
> > > + * @bus.mipi_csi1: embedded &struct v4l2_mbus_config_mipi_csi1.
> > > + *		   Used if the bus is MIPI Alliance's Camera Serial
> > > + *		   Interface version 1 (MIPI CSI1) or Standard
> > > + *		   Mobile Imaging Architecture's Compact Camera Port 2
> > > + *		   (SMIA CCP2).
> > > + * @bus.mipi_csi2: embedded &struct v4l2_mbus_config_mipi_csi2.
> > > + *		   Used if the bus is MIPI Alliance's Camera Serial
> > > + *		   Interface version 2 (MIPI CSI2).
> > >   */
> > >  struct v4l2_mbus_config {
> > >  	enum v4l2_mbus_type type;
> > > -	unsigned int flags;
> > > +	union {
> > > +		struct v4l2_mbus_config_parallel parallel;
> > > +		struct v4l2_mbus_config_mipi_csi1 mipi_csi1;
> > > +		struct v4l2_mbus_config_mipi_csi2 mipi_csi2;
> >
> > I wonder if we would be able to use a single structure for D-PHY and
> > C-PHY. The clock lane index does not make much sense for the latter,
> > but it was already there, so it can be changed on top in case it is
> > required
>
> I'm not sure to follow you here. CSI1 and CSI2 are not D-PHY versus
> C-PHY.
>

I meant if we will be able to correctly represent CSI-2 on C-PHY with
struct v4l2_mbus_config_mipi_csi2.

> > Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> >
> > > +	} bus;
> > >  };
> > >
> > >  /**
>
> --
> Regards,
>
> Laurent Pinchart
Laurent Pinchart Feb. 15, 2022, 11:32 a.m. UTC | #4 
Hi Jacopo,

On Tue, Feb 15, 2022 at 12:28:28PM +0100, Jacopo Mondi wrote:
> On Tue, Feb 15, 2022 at 01:15:40PM +0200, Laurent Pinchart wrote:
> > On Tue, Feb 15, 2022 at 12:13:16PM +0100, Jacopo Mondi wrote:
> > > On Mon, Jan 03, 2022 at 06:24:11PM +0200, Laurent Pinchart wrote:
> > > > The media bus configuration is specified through a set of flags, some of
> > > > which being mutually exclusive. This doesn't scale to express more
> > > > complex configurations. Improve the API by replacing the single flags
> > > > field in v4l2_mbus_config by a union of v4l2_mbus_config_* structures.
> > > > The flags themselves are still used in those structures, so they are
> > > > kept here. Drivers are however updated to use structure fields instead
> > > > of flags when already possible.
> > > >
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > > ---
> > > >  drivers/gpu/ipu-v3/ipu-csi.c                |  6 ++---
> > > >  drivers/media/i2c/adv7180.c                 |  8 ++++---
> > > >  drivers/media/i2c/adv748x/adv748x-csi2.c    | 18 +--------------
> > > >  drivers/media/i2c/ml86v7667.c               |  5 +++--
> > > >  drivers/media/i2c/mt9m001.c                 |  8 ++++---
> > > >  drivers/media/i2c/mt9m111.c                 | 16 +++++++------
> > > >  drivers/media/i2c/ov6650.c                  | 14 ++++++------
> > > >  drivers/media/i2c/ov9640.c                  |  8 ++++---
> > > >  drivers/media/i2c/tc358743.c                | 20 ++---------------
> > > >  drivers/media/i2c/tvp5150.c                 |  6 +++--
> > > >  drivers/media/platform/pxa_camera.c         | 12 +++++-----
> > > >  drivers/media/platform/rcar-vin/rcar-csi2.c | 16 +++----------
> > > >  drivers/staging/media/imx/imx-media-csi.c   |  7 +++---
> > > >  drivers/staging/media/imx/imx6-mipi-csi2.c  | 25 +++------------------
> > >
> > > drivers conversions looks good to me
> > >
> > > >  include/media/v4l2-mediabus.h               | 20 ++++++++++++++---
> > > >  15 files changed, 78 insertions(+), 111 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
> > > > index a9639d098893..778bc26d3ba5 100644
> > > > --- a/drivers/gpu/ipu-v3/ipu-csi.c
> > > > +++ b/drivers/gpu/ipu-v3/ipu-csi.c
> > > > @@ -357,11 +357,11 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config *csicfg,
> > > >  	switch (mbus_cfg->type) {
> > > >  	case V4L2_MBUS_PARALLEL:
> > > >  		csicfg->ext_vsync = 1;
> > > > -		csicfg->vsync_pol = (mbus_cfg->flags &
> > > > +		csicfg->vsync_pol = (mbus_cfg->bus.parallel.flags &
> > > >  				     V4L2_MBUS_VSYNC_ACTIVE_LOW) ? 1 : 0;
> > > > -		csicfg->hsync_pol = (mbus_cfg->flags &
> > > > +		csicfg->hsync_pol = (mbus_cfg->bus.parallel.flags &
> > > >  				     V4L2_MBUS_HSYNC_ACTIVE_LOW) ? 1 : 0;
> > > > -		csicfg->pixclk_pol = (mbus_cfg->flags &
> > > > +		csicfg->pixclk_pol = (mbus_cfg->bus.parallel.flags &
> > > >  				      V4L2_MBUS_PCLK_SAMPLE_FALLING) ? 1 : 0;
> > > >  		csicfg->clk_mode = IPU_CSI_CLK_MODE_GATED_CLK;
> > > >  		break;
> > > > diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
> > > > index d9a99fcfacb1..286f5017d9c3 100644
> > > > --- a/drivers/media/i2c/adv7180.c
> > > > +++ b/drivers/media/i2c/adv7180.c
> > > > @@ -784,7 +784,8 @@ static int adv7180_get_mbus_config(struct v4l2_subdev *sd,
> > > >
> > > >  	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
> > > >  		cfg->type = V4L2_MBUS_CSI2_DPHY;
> > > > -		cfg->flags = V4L2_MBUS_CSI2_1_LANE |
> > > > +		cfg->bus.mipi_csi2.num_data_lanes = 1;
> > > > +		cfg->bus.mipi_csi2.flags =
> > > >  				V4L2_MBUS_CSI2_CHANNEL_0 |
> > > >  				V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
> > > >  	} else {
> > > > @@ -792,8 +793,9 @@ static int adv7180_get_mbus_config(struct v4l2_subdev *sd,
> > > >  		 * The ADV7180 sensor supports BT.601/656 output modes.
> > > >  		 * The BT.656 is default and not yet configurable by s/w.
> > > >  		 */
> > > > -		cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING |
> > > > -				 V4L2_MBUS_DATA_ACTIVE_HIGH;
> > > > +		cfg->bus.parallel.flags = V4L2_MBUS_MASTER |
> > > > +					  V4L2_MBUS_PCLK_SAMPLE_RISING |
> > > > +					  V4L2_MBUS_DATA_ACTIVE_HIGH;
> > > >  		cfg->type = V4L2_MBUS_BT656;
> > > >  	}
> > > >
> > > > diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c
> > > > index 589e9644fcdc..bd4f3fe0e309 100644
> > > > --- a/drivers/media/i2c/adv748x/adv748x-csi2.c
> > > > +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
> > > > @@ -222,23 +222,7 @@ static int adv748x_csi2_get_mbus_config(struct v4l2_subdev *sd, unsigned int pad
> > > >  		return -EINVAL;
> > > >
> > > >  	config->type = V4L2_MBUS_CSI2_DPHY;
> > > > -	switch (tx->active_lanes) {
> > > > -	case 1:
> > > > -		config->flags = V4L2_MBUS_CSI2_1_LANE;
> > > > -		break;
> > > > -
> > > > -	case 2:
> > > > -		config->flags = V4L2_MBUS_CSI2_2_LANE;
> > > > -		break;
> > > > -
> > > > -	case 3:
> > > > -		config->flags = V4L2_MBUS_CSI2_3_LANE;
> > > > -		break;
> > > > -
> > > > -	case 4:
> > > > -		config->flags = V4L2_MBUS_CSI2_4_LANE;
> > > > -		break;
> > > > -	}
> > > > +	config->bus.mipi_csi2.num_data_lanes = tx->active_lanes;
> > > >
> > > >  	return 0;
> > > >  }
> > > > diff --git a/drivers/media/i2c/ml86v7667.c b/drivers/media/i2c/ml86v7667.c
> > > > index 4a1410ebb4c8..48cc0b0922f4 100644
> > > > --- a/drivers/media/i2c/ml86v7667.c
> > > > +++ b/drivers/media/i2c/ml86v7667.c
> > > > @@ -223,9 +223,10 @@ static int ml86v7667_get_mbus_config(struct v4l2_subdev *sd,
> > > >  				     unsigned int pad,
> > > >  				     struct v4l2_mbus_config *cfg)
> > > >  {
> > > > -	cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING |
> > > > -		     V4L2_MBUS_DATA_ACTIVE_HIGH;
> > > >  	cfg->type = V4L2_MBUS_BT656;
> > > > +	cfg->bus.parallel.flags = V4L2_MBUS_MASTER |
> > > > +				  V4L2_MBUS_PCLK_SAMPLE_RISING |
> > > > +				  V4L2_MBUS_DATA_ACTIVE_HIGH;
> > > >
> > > >  	return 0;
> > > >  }
> > > > diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
> > > > index c9f0bd997ea7..ad13b0c890c0 100644
> > > > --- a/drivers/media/i2c/mt9m001.c
> > > > +++ b/drivers/media/i2c/mt9m001.c
> > > > @@ -695,10 +695,12 @@ static int mt9m001_get_mbus_config(struct v4l2_subdev *sd,
> > > >  				   struct v4l2_mbus_config *cfg)
> > > >  {
> > > >  	/* MT9M001 has all capture_format parameters fixed */
> > > > -	cfg->flags = V4L2_MBUS_PCLK_SAMPLE_FALLING |
> > > > -		V4L2_MBUS_HSYNC_ACTIVE_HIGH | V4L2_MBUS_VSYNC_ACTIVE_HIGH |
> > > > -		V4L2_MBUS_DATA_ACTIVE_HIGH | V4L2_MBUS_MASTER;
> > > >  	cfg->type = V4L2_MBUS_PARALLEL;
> > > > +	cfg->bus.parallel.flags = V4L2_MBUS_PCLK_SAMPLE_FALLING |
> > > > +				  V4L2_MBUS_HSYNC_ACTIVE_HIGH |
> > > > +				  V4L2_MBUS_VSYNC_ACTIVE_HIGH |
> > > > +				  V4L2_MBUS_DATA_ACTIVE_HIGH |
> > > > +				  V4L2_MBUS_MASTER;
> > > >
> > > >  	return 0;
> > > >  }
> > > > diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
> > > > index 91a44359bcd3..6cf3ccf85d27 100644
> > > > --- a/drivers/media/i2c/mt9m111.c
> > > > +++ b/drivers/media/i2c/mt9m111.c
> > > > @@ -1143,15 +1143,17 @@ static int mt9m111_get_mbus_config(struct v4l2_subdev *sd,
> > > >  {
> > > >  	struct mt9m111 *mt9m111 = container_of(sd, struct mt9m111, subdev);
> > > >
> > > > -	cfg->flags = V4L2_MBUS_MASTER |
> > > > -		V4L2_MBUS_HSYNC_ACTIVE_HIGH | V4L2_MBUS_VSYNC_ACTIVE_HIGH |
> > > > -		V4L2_MBUS_DATA_ACTIVE_HIGH;
> > > > -
> > > > -	cfg->flags |= mt9m111->pclk_sample ? V4L2_MBUS_PCLK_SAMPLE_RISING :
> > > > -		V4L2_MBUS_PCLK_SAMPLE_FALLING;
> > > > -
> > > >  	cfg->type = V4L2_MBUS_PARALLEL;
> > > >
> > > > +	cfg->bus.parallel.flags = V4L2_MBUS_MASTER |
> > > > +				  V4L2_MBUS_HSYNC_ACTIVE_HIGH |
> > > > +				  V4L2_MBUS_VSYNC_ACTIVE_HIGH |
> > > > +				  V4L2_MBUS_DATA_ACTIVE_HIGH;
> > > > +
> > > > +	cfg->bus.parallel.flags |= mt9m111->pclk_sample ?
> > > > +				   V4L2_MBUS_PCLK_SAMPLE_RISING :
> > > > +				   V4L2_MBUS_PCLK_SAMPLE_FALLING;
> > > > +
> > > >  	return 0;
> > > >  }
> > > >
> > > > diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c
> > > > index 455a627e35a0..b2654b32bc62 100644
> > > > --- a/drivers/media/i2c/ov6650.c
> > > > +++ b/drivers/media/i2c/ov6650.c
> > > > @@ -932,15 +932,15 @@ static int ov6650_get_mbus_config(struct v4l2_subdev *sd,
> > > >  	if (ret)
> > > >  		return ret;
> > > >
> > > > -	cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_DATA_ACTIVE_HIGH
> > > > -		   | ((comj & COMJ_VSYNC_HIGH)  ? V4L2_MBUS_VSYNC_ACTIVE_HIGH
> > > > -						: V4L2_MBUS_VSYNC_ACTIVE_LOW)
> > > > -		   | ((comf & COMF_HREF_LOW)    ? V4L2_MBUS_HSYNC_ACTIVE_LOW
> > > > -						: V4L2_MBUS_HSYNC_ACTIVE_HIGH)
> > > > -		   | ((comj & COMJ_PCLK_RISING) ? V4L2_MBUS_PCLK_SAMPLE_RISING
> > > > -						: V4L2_MBUS_PCLK_SAMPLE_FALLING);
> > > >  	cfg->type = V4L2_MBUS_PARALLEL;
> > > >
> > > > +	cfg->bus.parallel.flags = V4L2_MBUS_MASTER | V4L2_MBUS_DATA_ACTIVE_HIGH
> > > > +		| ((comj & COMJ_VSYNC_HIGH)  ? V4L2_MBUS_VSYNC_ACTIVE_HIGH
> > > > +					     : V4L2_MBUS_VSYNC_ACTIVE_LOW)
> > > > +		| ((comf & COMF_HREF_LOW)    ? V4L2_MBUS_HSYNC_ACTIVE_LOW
> > > > +					     : V4L2_MBUS_HSYNC_ACTIVE_HIGH)
> > > > +		| ((comj & COMJ_PCLK_RISING) ? V4L2_MBUS_PCLK_SAMPLE_RISING
> > > > +					     : V4L2_MBUS_PCLK_SAMPLE_FALLING);
> > > >  	return 0;
> > > >  }
> > > >
> > > > diff --git a/drivers/media/i2c/ov9640.c b/drivers/media/i2c/ov9640.c
> > > > index 0bab8c2cf160..9f44ed52d164 100644
> > > > --- a/drivers/media/i2c/ov9640.c
> > > > +++ b/drivers/media/i2c/ov9640.c
> > > > @@ -652,10 +652,12 @@ static int ov9640_get_mbus_config(struct v4l2_subdev *sd,
> > > >  				  unsigned int pad,
> > > >  				  struct v4l2_mbus_config *cfg)
> > > >  {
> > > > -	cfg->flags = V4L2_MBUS_PCLK_SAMPLE_RISING | V4L2_MBUS_MASTER |
> > > > -		V4L2_MBUS_VSYNC_ACTIVE_HIGH | V4L2_MBUS_HSYNC_ACTIVE_HIGH |
> > > > -		V4L2_MBUS_DATA_ACTIVE_HIGH;
> > > >  	cfg->type = V4L2_MBUS_PARALLEL;
> > > > +	cfg->bus.parallel.flags = V4L2_MBUS_PCLK_SAMPLE_RISING |
> > > > +				  V4L2_MBUS_MASTER |
> > > > +				  V4L2_MBUS_VSYNC_ACTIVE_HIGH |
> > > > +				  V4L2_MBUS_HSYNC_ACTIVE_HIGH |
> > > > +				  V4L2_MBUS_DATA_ACTIVE_HIGH;
> > > >
> > > >  	return 0;
> > > >  }
> > > > diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
> > > > index 40512004afba..dfbc42675143 100644
> > > > --- a/drivers/media/i2c/tc358743.c
> > > > +++ b/drivers/media/i2c/tc358743.c
> > > > @@ -1613,24 +1613,8 @@ static int tc358743_get_mbus_config(struct v4l2_subdev *sd,
> > > >  	cfg->type = V4L2_MBUS_CSI2_DPHY;
> > > >
> > > >  	/* Support for non-continuous CSI-2 clock is missing in the driver */
> > > > -	cfg->flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
> > > > -
> > > > -	switch (state->csi_lanes_in_use) {
> > > > -	case 1:
> > > > -		cfg->flags |= V4L2_MBUS_CSI2_1_LANE;
> > > > -		break;
> > > > -	case 2:
> > > > -		cfg->flags |= V4L2_MBUS_CSI2_2_LANE;
> > > > -		break;
> > > > -	case 3:
> > > > -		cfg->flags |= V4L2_MBUS_CSI2_3_LANE;
> > > > -		break;
> > > > -	case 4:
> > > > -		cfg->flags |= V4L2_MBUS_CSI2_4_LANE;
> > > > -		break;
> > > > -	default:
> > > > -		return -EINVAL;
> > > > -	}
> > > > +	cfg->bus.mipi_csi2.flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
> > > > +	cfg->bus.mipi_csi2.num_data_lanes = state->csi_lanes_in_use;
> > > >
> > > >  	return 0;
> > > >  }
> > > > diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> > > > index 4b16ffcaef98..65472438444b 100644
> > > > --- a/drivers/media/i2c/tvp5150.c
> > > > +++ b/drivers/media/i2c/tvp5150.c
> > > > @@ -1198,8 +1198,10 @@ static int tvp5150_get_mbus_config(struct v4l2_subdev *sd,
> > > >  	struct tvp5150 *decoder = to_tvp5150(sd);
> > > >
> > > >  	cfg->type = decoder->mbus_type;
> > > > -	cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING
> > > > -		   | V4L2_MBUS_FIELD_EVEN_LOW | V4L2_MBUS_DATA_ACTIVE_HIGH;
> > > > +	cfg->bus.parallel.flags = V4L2_MBUS_MASTER
> > > > +				| V4L2_MBUS_PCLK_SAMPLE_RISING
> > > > +				| V4L2_MBUS_FIELD_EVEN_LOW
> > > > +				| V4L2_MBUS_DATA_ACTIVE_HIGH;
> > > >
> > > >  	return 0;
> > > >  }
> > > > diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c
> > > > index b5644cf37fe9..35145e3348f0 100644
> > > > --- a/drivers/media/platform/pxa_camera.c
> > > > +++ b/drivers/media/platform/pxa_camera.c
> > > > @@ -1587,24 +1587,26 @@ static int pxa_camera_set_bus_param(struct pxa_camera_dev *pcdev)
> > > >  	 * PXA does not support V4L2_MBUS_DATA_ACTIVE_LOW and the bus mastering
> > > >  	 * roles should match.
> > > >  	 */
> > > > -	if (cfg.flags != mbus_config) {
> > > > +	if (cfg.bus.parallel.flags != mbus_config) {
> > > >  		unsigned int pxa_mbus_role = mbus_config & (V4L2_MBUS_MASTER |
> > > >  							    V4L2_MBUS_SLAVE);
> > > > -		if (pxa_mbus_role != (cfg.flags & (V4L2_MBUS_MASTER |
> > > > -						   V4L2_MBUS_SLAVE))) {
> > > > +		unsigned int flags = cfg.bus.parallel.flags;
> > > > +
> > > > +		if (pxa_mbus_role != (flags & (V4L2_MBUS_MASTER |
> > > > +					       V4L2_MBUS_SLAVE))) {
> > > >  			dev_err(pcdev_to_dev(pcdev),
> > > >  				"Unsupported mbus configuration: bus mastering\n");
> > > >  			return -EINVAL;
> > > >  		}
> > > >
> > > > -		if (cfg.flags & V4L2_MBUS_DATA_ACTIVE_LOW) {
> > > > +		if (flags & V4L2_MBUS_DATA_ACTIVE_LOW) {
> > > >  			dev_err(pcdev_to_dev(pcdev),
> > > >  				"Unsupported mbus configuration: DATA_ACTIVE_LOW\n");
> > > >  			return -EINVAL;
> > > >  		}
> > > >  	}
> > > >
> > > > -	pxa_camera_setup_cicr(pcdev, cfg.flags, pixfmt);
> > > > +	pxa_camera_setup_cicr(pcdev, cfg.bus.parallel.flags, pixfmt);
> > > >
> > > >  	return 0;
> > > >  }
> > > > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > > > index 8c939cb3073d..cbac5801720b 100644
> > > > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> > > > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > > > @@ -603,7 +603,6 @@ static int rcsi2_get_active_lanes(struct rcar_csi2 *priv,
> > > >  				  unsigned int *lanes)
> > > >  {
> > > >  	struct v4l2_mbus_config mbus_config = { 0 };
> > > > -	unsigned int num_lanes = UINT_MAX;
> > > >  	int ret;
> > > >
> > > >  	*lanes = priv->lanes;
> > > > @@ -626,23 +625,14 @@ static int rcsi2_get_active_lanes(struct rcar_csi2 *priv,
> > > >  		return -EINVAL;
> > > >  	}
> > > >
> > > > -	if (mbus_config.flags & V4L2_MBUS_CSI2_1_LANE)
> > > > -		num_lanes = 1;
> > > > -	else if (mbus_config.flags & V4L2_MBUS_CSI2_2_LANE)
> > > > -		num_lanes = 2;
> > > > -	else if (mbus_config.flags & V4L2_MBUS_CSI2_3_LANE)
> > > > -		num_lanes = 3;
> > > > -	else if (mbus_config.flags & V4L2_MBUS_CSI2_4_LANE)
> > > > -		num_lanes = 4;
> > > > -
> > > > -	if (num_lanes > priv->lanes) {
> > > > +	if (mbus_config.bus.mipi_csi2.num_data_lanes > priv->lanes) {
> > > >  		dev_err(priv->dev,
> > > >  			"Unsupported mbus config: too many data lanes %u\n",
> > > > -			num_lanes);
> > > > +			mbus_config.bus.mipi_csi2.num_data_lanes);
> > > >  		return -EINVAL;
> > > >  	}
> > > >
> > > > -	*lanes = num_lanes;
> > > > +	*lanes = mbus_config.bus.mipi_csi2.num_data_lanes;
> > > >
> > > >  	return 0;
> > > >  }
> > > > diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> > > > index bd7f156f2d52..b2b1f4dd41d7 100644
> > > > --- a/drivers/staging/media/imx/imx-media-csi.c
> > > > +++ b/drivers/staging/media/imx/imx-media-csi.c
> > > > @@ -718,9 +718,10 @@ static int csi_setup(struct csi_priv *priv)
> > > >
> > > >  	/* compose mbus_config from the upstream endpoint */
> > > >  	mbus_cfg.type = priv->upstream_ep.bus_type;
> > > > -	mbus_cfg.flags = is_parallel_bus(&priv->upstream_ep) ?
> > > > -		priv->upstream_ep.bus.parallel.flags :
> > > > -		priv->upstream_ep.bus.mipi_csi2.flags;
> > > > +	if (is_parallel_bus(&priv->upstream_ep))
> > > > +		mbus_cfg.bus.parallel = priv->upstream_ep.bus.parallel;
> > > > +	else
> > > > +		mbus_cfg.bus.mipi_csi2 = priv->upstream_ep.bus.mipi_csi2;
> > > >
> > > >  	if_fmt = *infmt;
> > > >  	crop = priv->crop;
> > > > diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
> > > > index 558b256ac935..c4cb558a85c6 100644
> > > > --- a/drivers/staging/media/imx/imx6-mipi-csi2.c
> > > > +++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
> > > > @@ -303,7 +303,6 @@ static void csi2ipu_gasket_init(struct csi2_dev *csi2)
> > > >  static int csi2_get_active_lanes(struct csi2_dev *csi2, unsigned int *lanes)
> > > >  {
> > > >  	struct v4l2_mbus_config mbus_config = { 0 };
> > > > -	unsigned int num_lanes = UINT_MAX;
> > > >  	int ret;
> > > >
> > > >  	*lanes = csi2->data_lanes;
> > > > @@ -326,32 +325,14 @@ static int csi2_get_active_lanes(struct csi2_dev *csi2, unsigned int *lanes)
> > > >  		return -EINVAL;
> > > >  	}
> > > >
> > > > -	switch (mbus_config.flags & V4L2_MBUS_CSI2_LANES) {
> > > > -	case V4L2_MBUS_CSI2_1_LANE:
> > > > -		num_lanes = 1;
> > > > -		break;
> > > > -	case V4L2_MBUS_CSI2_2_LANE:
> > > > -		num_lanes = 2;
> > > > -		break;
> > > > -	case V4L2_MBUS_CSI2_3_LANE:
> > > > -		num_lanes = 3;
> > > > -		break;
> > > > -	case V4L2_MBUS_CSI2_4_LANE:
> > > > -		num_lanes = 4;
> > > > -		break;
> > > > -	default:
> > > > -		num_lanes = csi2->data_lanes;
> > > > -		break;
> > > > -	}
> > > > -
> > > > -	if (num_lanes > csi2->data_lanes) {
> > > > +	if (mbus_config.bus.mipi_csi2.num_data_lanes > csi2->data_lanes) {
> > > >  		dev_err(csi2->dev,
> > > >  			"Unsupported mbus config: too many data lanes %u\n",
> > > > -			num_lanes);
> > > > +			mbus_config.bus.mipi_csi2.num_data_lanes);
> > > >  		return -EINVAL;
> > > >  	}
> > > >
> > > > -	*lanes = num_lanes;
> > > > +	*lanes = mbus_config.bus.mipi_csi2.num_data_lanes;
> > > >
> > > >  	return 0;
> > > >  }
> > > > diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
> > > > index 9c4970fbd8ea..269aaf57ba32 100644
> > > > --- a/include/media/v4l2-mediabus.h
> > > > +++ b/include/media/v4l2-mediabus.h
> > > > @@ -166,12 +166,26 @@ enum v4l2_mbus_type {
> > > >
> > > >  /**
> > > >   * struct v4l2_mbus_config - media bus configuration
> > > > - * @type:	in: interface type
> > > > - * @flags:	in / out: configuration flags, depending on @type
> > > > + * @type: interface type
> > > > + * @bus: bus configuration data structure
> > > > + * @bus.parallel: embedded &struct v4l2_mbus_config_parallel.
> > > > + *		  Used if the bus is parallel or BT.656.
> > > > + * @bus.mipi_csi1: embedded &struct v4l2_mbus_config_mipi_csi1.
> > > > + *		   Used if the bus is MIPI Alliance's Camera Serial
> > > > + *		   Interface version 1 (MIPI CSI1) or Standard
> > > > + *		   Mobile Imaging Architecture's Compact Camera Port 2
> > > > + *		   (SMIA CCP2).
> > > > + * @bus.mipi_csi2: embedded &struct v4l2_mbus_config_mipi_csi2.
> > > > + *		   Used if the bus is MIPI Alliance's Camera Serial
> > > > + *		   Interface version 2 (MIPI CSI2).
> > > >   */
> > > >  struct v4l2_mbus_config {
> > > >  	enum v4l2_mbus_type type;
> > > > -	unsigned int flags;
> > > > +	union {
> > > > +		struct v4l2_mbus_config_parallel parallel;
> > > > +		struct v4l2_mbus_config_mipi_csi1 mipi_csi1;
> > > > +		struct v4l2_mbus_config_mipi_csi2 mipi_csi2;
> > >
> > > I wonder if we would be able to use a single structure for D-PHY and
> > > C-PHY. The clock lane index does not make much sense for the latter,
> > > but it was already there, so it can be changed on top in case it is
> > > required
> >
> > I'm not sure to follow you here. CSI1 and CSI2 are not D-PHY versus
> > C-PHY.
> 
> I meant if we will be able to correctly represent CSI-2 on C-PHY with
> struct v4l2_mbus_config_mipi_csi2.

I suppose we can split it into two structures later if the need arises.

> > > Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > >
> > > > +	} bus;
> > > >  };
> > > >
> > > >  /**
diff mbox series

Patch

diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
index a9639d098893..778bc26d3ba5 100644
--- a/drivers/gpu/ipu-v3/ipu-csi.c
+++ b/drivers/gpu/ipu-v3/ipu-csi.c
@@ -357,11 +357,11 @@  static int fill_csi_bus_cfg(struct ipu_csi_bus_config *csicfg,
 	switch (mbus_cfg->type) {
 	case V4L2_MBUS_PARALLEL:
 		csicfg->ext_vsync = 1;
-		csicfg->vsync_pol = (mbus_cfg->flags &
+		csicfg->vsync_pol = (mbus_cfg->bus.parallel.flags &
 				     V4L2_MBUS_VSYNC_ACTIVE_LOW) ? 1 : 0;
-		csicfg->hsync_pol = (mbus_cfg->flags &
+		csicfg->hsync_pol = (mbus_cfg->bus.parallel.flags &
 				     V4L2_MBUS_HSYNC_ACTIVE_LOW) ? 1 : 0;
-		csicfg->pixclk_pol = (mbus_cfg->flags &
+		csicfg->pixclk_pol = (mbus_cfg->bus.parallel.flags &
 				      V4L2_MBUS_PCLK_SAMPLE_FALLING) ? 1 : 0;
 		csicfg->clk_mode = IPU_CSI_CLK_MODE_GATED_CLK;
 		break;
diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index d9a99fcfacb1..286f5017d9c3 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -784,7 +784,8 @@  static int adv7180_get_mbus_config(struct v4l2_subdev *sd,
 
 	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
 		cfg->type = V4L2_MBUS_CSI2_DPHY;
-		cfg->flags = V4L2_MBUS_CSI2_1_LANE |
+		cfg->bus.mipi_csi2.num_data_lanes = 1;
+		cfg->bus.mipi_csi2.flags =
 				V4L2_MBUS_CSI2_CHANNEL_0 |
 				V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
 	} else {
@@ -792,8 +793,9 @@  static int adv7180_get_mbus_config(struct v4l2_subdev *sd,
 		 * The ADV7180 sensor supports BT.601/656 output modes.
 		 * The BT.656 is default and not yet configurable by s/w.
 		 */
-		cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING |
-				 V4L2_MBUS_DATA_ACTIVE_HIGH;
+		cfg->bus.parallel.flags = V4L2_MBUS_MASTER |
+					  V4L2_MBUS_PCLK_SAMPLE_RISING |
+					  V4L2_MBUS_DATA_ACTIVE_HIGH;
 		cfg->type = V4L2_MBUS_BT656;
 	}
 
diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c
index 589e9644fcdc..bd4f3fe0e309 100644
--- a/drivers/media/i2c/adv748x/adv748x-csi2.c
+++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
@@ -222,23 +222,7 @@  static int adv748x_csi2_get_mbus_config(struct v4l2_subdev *sd, unsigned int pad
 		return -EINVAL;
 
 	config->type = V4L2_MBUS_CSI2_DPHY;
-	switch (tx->active_lanes) {
-	case 1:
-		config->flags = V4L2_MBUS_CSI2_1_LANE;
-		break;
-
-	case 2:
-		config->flags = V4L2_MBUS_CSI2_2_LANE;
-		break;
-
-	case 3:
-		config->flags = V4L2_MBUS_CSI2_3_LANE;
-		break;
-
-	case 4:
-		config->flags = V4L2_MBUS_CSI2_4_LANE;
-		break;
-	}
+	config->bus.mipi_csi2.num_data_lanes = tx->active_lanes;
 
 	return 0;
 }
diff --git a/drivers/media/i2c/ml86v7667.c b/drivers/media/i2c/ml86v7667.c
index 4a1410ebb4c8..48cc0b0922f4 100644
--- a/drivers/media/i2c/ml86v7667.c
+++ b/drivers/media/i2c/ml86v7667.c
@@ -223,9 +223,10 @@  static int ml86v7667_get_mbus_config(struct v4l2_subdev *sd,
 				     unsigned int pad,
 				     struct v4l2_mbus_config *cfg)
 {
-	cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING |
-		     V4L2_MBUS_DATA_ACTIVE_HIGH;
 	cfg->type = V4L2_MBUS_BT656;
+	cfg->bus.parallel.flags = V4L2_MBUS_MASTER |
+				  V4L2_MBUS_PCLK_SAMPLE_RISING |
+				  V4L2_MBUS_DATA_ACTIVE_HIGH;
 
 	return 0;
 }
diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
index c9f0bd997ea7..ad13b0c890c0 100644
--- a/drivers/media/i2c/mt9m001.c
+++ b/drivers/media/i2c/mt9m001.c
@@ -695,10 +695,12 @@  static int mt9m001_get_mbus_config(struct v4l2_subdev *sd,
 				   struct v4l2_mbus_config *cfg)
 {
 	/* MT9M001 has all capture_format parameters fixed */
-	cfg->flags = V4L2_MBUS_PCLK_SAMPLE_FALLING |
-		V4L2_MBUS_HSYNC_ACTIVE_HIGH | V4L2_MBUS_VSYNC_ACTIVE_HIGH |
-		V4L2_MBUS_DATA_ACTIVE_HIGH | V4L2_MBUS_MASTER;
 	cfg->type = V4L2_MBUS_PARALLEL;
+	cfg->bus.parallel.flags = V4L2_MBUS_PCLK_SAMPLE_FALLING |
+				  V4L2_MBUS_HSYNC_ACTIVE_HIGH |
+				  V4L2_MBUS_VSYNC_ACTIVE_HIGH |
+				  V4L2_MBUS_DATA_ACTIVE_HIGH |
+				  V4L2_MBUS_MASTER;
 
 	return 0;
 }
diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
index 91a44359bcd3..6cf3ccf85d27 100644
--- a/drivers/media/i2c/mt9m111.c
+++ b/drivers/media/i2c/mt9m111.c
@@ -1143,15 +1143,17 @@  static int mt9m111_get_mbus_config(struct v4l2_subdev *sd,
 {
 	struct mt9m111 *mt9m111 = container_of(sd, struct mt9m111, subdev);
 
-	cfg->flags = V4L2_MBUS_MASTER |
-		V4L2_MBUS_HSYNC_ACTIVE_HIGH | V4L2_MBUS_VSYNC_ACTIVE_HIGH |
-		V4L2_MBUS_DATA_ACTIVE_HIGH;
-
-	cfg->flags |= mt9m111->pclk_sample ? V4L2_MBUS_PCLK_SAMPLE_RISING :
-		V4L2_MBUS_PCLK_SAMPLE_FALLING;
-
 	cfg->type = V4L2_MBUS_PARALLEL;
 
+	cfg->bus.parallel.flags = V4L2_MBUS_MASTER |
+				  V4L2_MBUS_HSYNC_ACTIVE_HIGH |
+				  V4L2_MBUS_VSYNC_ACTIVE_HIGH |
+				  V4L2_MBUS_DATA_ACTIVE_HIGH;
+
+	cfg->bus.parallel.flags |= mt9m111->pclk_sample ?
+				   V4L2_MBUS_PCLK_SAMPLE_RISING :
+				   V4L2_MBUS_PCLK_SAMPLE_FALLING;
+
 	return 0;
 }
 
diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c
index 455a627e35a0..b2654b32bc62 100644
--- a/drivers/media/i2c/ov6650.c
+++ b/drivers/media/i2c/ov6650.c
@@ -932,15 +932,15 @@  static int ov6650_get_mbus_config(struct v4l2_subdev *sd,
 	if (ret)
 		return ret;
 
-	cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_DATA_ACTIVE_HIGH
-		   | ((comj & COMJ_VSYNC_HIGH)  ? V4L2_MBUS_VSYNC_ACTIVE_HIGH
-						: V4L2_MBUS_VSYNC_ACTIVE_LOW)
-		   | ((comf & COMF_HREF_LOW)    ? V4L2_MBUS_HSYNC_ACTIVE_LOW
-						: V4L2_MBUS_HSYNC_ACTIVE_HIGH)
-		   | ((comj & COMJ_PCLK_RISING) ? V4L2_MBUS_PCLK_SAMPLE_RISING
-						: V4L2_MBUS_PCLK_SAMPLE_FALLING);
 	cfg->type = V4L2_MBUS_PARALLEL;
 
+	cfg->bus.parallel.flags = V4L2_MBUS_MASTER | V4L2_MBUS_DATA_ACTIVE_HIGH
+		| ((comj & COMJ_VSYNC_HIGH)  ? V4L2_MBUS_VSYNC_ACTIVE_HIGH
+					     : V4L2_MBUS_VSYNC_ACTIVE_LOW)
+		| ((comf & COMF_HREF_LOW)    ? V4L2_MBUS_HSYNC_ACTIVE_LOW
+					     : V4L2_MBUS_HSYNC_ACTIVE_HIGH)
+		| ((comj & COMJ_PCLK_RISING) ? V4L2_MBUS_PCLK_SAMPLE_RISING
+					     : V4L2_MBUS_PCLK_SAMPLE_FALLING);
 	return 0;
 }
 
diff --git a/drivers/media/i2c/ov9640.c b/drivers/media/i2c/ov9640.c
index 0bab8c2cf160..9f44ed52d164 100644
--- a/drivers/media/i2c/ov9640.c
+++ b/drivers/media/i2c/ov9640.c
@@ -652,10 +652,12 @@  static int ov9640_get_mbus_config(struct v4l2_subdev *sd,
 				  unsigned int pad,
 				  struct v4l2_mbus_config *cfg)
 {
-	cfg->flags = V4L2_MBUS_PCLK_SAMPLE_RISING | V4L2_MBUS_MASTER |
-		V4L2_MBUS_VSYNC_ACTIVE_HIGH | V4L2_MBUS_HSYNC_ACTIVE_HIGH |
-		V4L2_MBUS_DATA_ACTIVE_HIGH;
 	cfg->type = V4L2_MBUS_PARALLEL;
+	cfg->bus.parallel.flags = V4L2_MBUS_PCLK_SAMPLE_RISING |
+				  V4L2_MBUS_MASTER |
+				  V4L2_MBUS_VSYNC_ACTIVE_HIGH |
+				  V4L2_MBUS_HSYNC_ACTIVE_HIGH |
+				  V4L2_MBUS_DATA_ACTIVE_HIGH;
 
 	return 0;
 }
diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 40512004afba..dfbc42675143 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1613,24 +1613,8 @@  static int tc358743_get_mbus_config(struct v4l2_subdev *sd,
 	cfg->type = V4L2_MBUS_CSI2_DPHY;
 
 	/* Support for non-continuous CSI-2 clock is missing in the driver */
-	cfg->flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
-
-	switch (state->csi_lanes_in_use) {
-	case 1:
-		cfg->flags |= V4L2_MBUS_CSI2_1_LANE;
-		break;
-	case 2:
-		cfg->flags |= V4L2_MBUS_CSI2_2_LANE;
-		break;
-	case 3:
-		cfg->flags |= V4L2_MBUS_CSI2_3_LANE;
-		break;
-	case 4:
-		cfg->flags |= V4L2_MBUS_CSI2_4_LANE;
-		break;
-	default:
-		return -EINVAL;
-	}
+	cfg->bus.mipi_csi2.flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+	cfg->bus.mipi_csi2.num_data_lanes = state->csi_lanes_in_use;
 
 	return 0;
 }
diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 4b16ffcaef98..65472438444b 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -1198,8 +1198,10 @@  static int tvp5150_get_mbus_config(struct v4l2_subdev *sd,
 	struct tvp5150 *decoder = to_tvp5150(sd);
 
 	cfg->type = decoder->mbus_type;
-	cfg->flags = V4L2_MBUS_MASTER | V4L2_MBUS_PCLK_SAMPLE_RISING
-		   | V4L2_MBUS_FIELD_EVEN_LOW | V4L2_MBUS_DATA_ACTIVE_HIGH;
+	cfg->bus.parallel.flags = V4L2_MBUS_MASTER
+				| V4L2_MBUS_PCLK_SAMPLE_RISING
+				| V4L2_MBUS_FIELD_EVEN_LOW
+				| V4L2_MBUS_DATA_ACTIVE_HIGH;
 
 	return 0;
 }
diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c
index b5644cf37fe9..35145e3348f0 100644
--- a/drivers/media/platform/pxa_camera.c
+++ b/drivers/media/platform/pxa_camera.c
@@ -1587,24 +1587,26 @@  static int pxa_camera_set_bus_param(struct pxa_camera_dev *pcdev)
 	 * PXA does not support V4L2_MBUS_DATA_ACTIVE_LOW and the bus mastering
 	 * roles should match.
 	 */
-	if (cfg.flags != mbus_config) {
+	if (cfg.bus.parallel.flags != mbus_config) {
 		unsigned int pxa_mbus_role = mbus_config & (V4L2_MBUS_MASTER |
 							    V4L2_MBUS_SLAVE);
-		if (pxa_mbus_role != (cfg.flags & (V4L2_MBUS_MASTER |
-						   V4L2_MBUS_SLAVE))) {
+		unsigned int flags = cfg.bus.parallel.flags;
+
+		if (pxa_mbus_role != (flags & (V4L2_MBUS_MASTER |
+					       V4L2_MBUS_SLAVE))) {
 			dev_err(pcdev_to_dev(pcdev),
 				"Unsupported mbus configuration: bus mastering\n");
 			return -EINVAL;
 		}
 
-		if (cfg.flags & V4L2_MBUS_DATA_ACTIVE_LOW) {
+		if (flags & V4L2_MBUS_DATA_ACTIVE_LOW) {
 			dev_err(pcdev_to_dev(pcdev),
 				"Unsupported mbus configuration: DATA_ACTIVE_LOW\n");
 			return -EINVAL;
 		}
 	}
 
-	pxa_camera_setup_cicr(pcdev, cfg.flags, pixfmt);
+	pxa_camera_setup_cicr(pcdev, cfg.bus.parallel.flags, pixfmt);
 
 	return 0;
 }
diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
index 8c939cb3073d..cbac5801720b 100644
--- a/drivers/media/platform/rcar-vin/rcar-csi2.c
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -603,7 +603,6 @@  static int rcsi2_get_active_lanes(struct rcar_csi2 *priv,
 				  unsigned int *lanes)
 {
 	struct v4l2_mbus_config mbus_config = { 0 };
-	unsigned int num_lanes = UINT_MAX;
 	int ret;
 
 	*lanes = priv->lanes;
@@ -626,23 +625,14 @@  static int rcsi2_get_active_lanes(struct rcar_csi2 *priv,
 		return -EINVAL;
 	}
 
-	if (mbus_config.flags & V4L2_MBUS_CSI2_1_LANE)
-		num_lanes = 1;
-	else if (mbus_config.flags & V4L2_MBUS_CSI2_2_LANE)
-		num_lanes = 2;
-	else if (mbus_config.flags & V4L2_MBUS_CSI2_3_LANE)
-		num_lanes = 3;
-	else if (mbus_config.flags & V4L2_MBUS_CSI2_4_LANE)
-		num_lanes = 4;
-
-	if (num_lanes > priv->lanes) {
+	if (mbus_config.bus.mipi_csi2.num_data_lanes > priv->lanes) {
 		dev_err(priv->dev,
 			"Unsupported mbus config: too many data lanes %u\n",
-			num_lanes);
+			mbus_config.bus.mipi_csi2.num_data_lanes);
 		return -EINVAL;
 	}
 
-	*lanes = num_lanes;
+	*lanes = mbus_config.bus.mipi_csi2.num_data_lanes;
 
 	return 0;
 }
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index bd7f156f2d52..b2b1f4dd41d7 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -718,9 +718,10 @@  static int csi_setup(struct csi_priv *priv)
 
 	/* compose mbus_config from the upstream endpoint */
 	mbus_cfg.type = priv->upstream_ep.bus_type;
-	mbus_cfg.flags = is_parallel_bus(&priv->upstream_ep) ?
-		priv->upstream_ep.bus.parallel.flags :
-		priv->upstream_ep.bus.mipi_csi2.flags;
+	if (is_parallel_bus(&priv->upstream_ep))
+		mbus_cfg.bus.parallel = priv->upstream_ep.bus.parallel;
+	else
+		mbus_cfg.bus.mipi_csi2 = priv->upstream_ep.bus.mipi_csi2;
 
 	if_fmt = *infmt;
 	crop = priv->crop;
diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
index 558b256ac935..c4cb558a85c6 100644
--- a/drivers/staging/media/imx/imx6-mipi-csi2.c
+++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
@@ -303,7 +303,6 @@  static void csi2ipu_gasket_init(struct csi2_dev *csi2)
 static int csi2_get_active_lanes(struct csi2_dev *csi2, unsigned int *lanes)
 {
 	struct v4l2_mbus_config mbus_config = { 0 };
-	unsigned int num_lanes = UINT_MAX;
 	int ret;
 
 	*lanes = csi2->data_lanes;
@@ -326,32 +325,14 @@  static int csi2_get_active_lanes(struct csi2_dev *csi2, unsigned int *lanes)
 		return -EINVAL;
 	}
 
-	switch (mbus_config.flags & V4L2_MBUS_CSI2_LANES) {
-	case V4L2_MBUS_CSI2_1_LANE:
-		num_lanes = 1;
-		break;
-	case V4L2_MBUS_CSI2_2_LANE:
-		num_lanes = 2;
-		break;
-	case V4L2_MBUS_CSI2_3_LANE:
-		num_lanes = 3;
-		break;
-	case V4L2_MBUS_CSI2_4_LANE:
-		num_lanes = 4;
-		break;
-	default:
-		num_lanes = csi2->data_lanes;
-		break;
-	}
-
-	if (num_lanes > csi2->data_lanes) {
+	if (mbus_config.bus.mipi_csi2.num_data_lanes > csi2->data_lanes) {
 		dev_err(csi2->dev,
 			"Unsupported mbus config: too many data lanes %u\n",
-			num_lanes);
+			mbus_config.bus.mipi_csi2.num_data_lanes);
 		return -EINVAL;
 	}
 
-	*lanes = num_lanes;
+	*lanes = mbus_config.bus.mipi_csi2.num_data_lanes;
 
 	return 0;
 }
diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
index 9c4970fbd8ea..269aaf57ba32 100644
--- a/include/media/v4l2-mediabus.h
+++ b/include/media/v4l2-mediabus.h
@@ -166,12 +166,26 @@  enum v4l2_mbus_type {
 
 /**
  * struct v4l2_mbus_config - media bus configuration
- * @type:	in: interface type
- * @flags:	in / out: configuration flags, depending on @type
+ * @type: interface type
+ * @bus: bus configuration data structure
+ * @bus.parallel: embedded &struct v4l2_mbus_config_parallel.
+ *		  Used if the bus is parallel or BT.656.
+ * @bus.mipi_csi1: embedded &struct v4l2_mbus_config_mipi_csi1.
+ *		   Used if the bus is MIPI Alliance's Camera Serial
+ *		   Interface version 1 (MIPI CSI1) or Standard
+ *		   Mobile Imaging Architecture's Compact Camera Port 2
+ *		   (SMIA CCP2).
+ * @bus.mipi_csi2: embedded &struct v4l2_mbus_config_mipi_csi2.
+ *		   Used if the bus is MIPI Alliance's Camera Serial
+ *		   Interface version 2 (MIPI CSI2).
  */
 struct v4l2_mbus_config {
 	enum v4l2_mbus_type type;
-	unsigned int flags;
+	union {
+		struct v4l2_mbus_config_parallel parallel;
+		struct v4l2_mbus_config_mipi_csi1 mipi_csi1;
+		struct v4l2_mbus_config_mipi_csi2 mipi_csi2;
+	} bus;
 };
 
 /**