Re: [RFC PATCH 2/8] media: i2c: ov6650: Drop implementation of .set_mbus_config()

From: Sakari Ailus <sakari.ailus@iki.fi>
To: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Cc: linux-media@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
	"Hans Verkuil" <hverkuil-cisco@xs4all.nl>,
	"Kieran Bingham" <kieran.bingham@ideasonboard.com>,
	"Jacopo Mondi" <jacopo@jmondi.org>,
	"Niklas Söderlund" <niklas.soderlund@ragnatech.se>,
	"Tomi Valkeinen" <tomi.valkeinen@ideasonboard.com>,
	"Janusz Krzysztofik" <jmkrzyszt@gmail.com>
Subject: Re: [RFC PATCH 2/8] media: i2c: ov6650: Drop implementation of .set_mbus_config()
Date: Wed, 5 Jan 2022 20:04:24 +0200	[thread overview]
Message-ID: <YdXdqJLKzDduVdex@valkosipuli.retiisi.eu> (raw)
In-Reply-To: <20220103162414.27723-3-laurent.pinchart+renesas@ideasonboard.com>

Hi Laurent,

Thanks for the set.

On Mon, Jan 03, 2022 at 06:24:08PM +0200, Laurent Pinchart wrote:
> The subdev .set_mbus_config() operation is deprecated. No code in the
> kernel calls it, so drop its implementation from the ov6650 driver.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/media/i2c/ov6650.c | 37 -------------------------------------
>  1 file changed, 37 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c
> index f67412150b16..455a627e35a0 100644
> --- a/drivers/media/i2c/ov6650.c
> +++ b/drivers/media/i2c/ov6650.c
> @@ -944,42 +944,6 @@ static int ov6650_get_mbus_config(struct v4l2_subdev *sd,
>  	return 0;
>  }
>  
> -/* Alter bus settings on camera side */
> -static int ov6650_set_mbus_config(struct v4l2_subdev *sd,
> -				  unsigned int pad,
> -				  struct v4l2_mbus_config *cfg)
> -{
> -	struct i2c_client *client = v4l2_get_subdevdata(sd);
> -	int ret = 0;
> -
> -	if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_RISING)
> -		ret = ov6650_reg_rmw(client, REG_COMJ, COMJ_PCLK_RISING, 0);
> -	else if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_FALLING)
> -		ret = ov6650_reg_rmw(client, REG_COMJ, 0, COMJ_PCLK_RISING);

I think this configuration should come from the endpoint which the driver
currently does not parse. In fact, there are no even DT bindings for the
device.

I wonder what kind of environment it is used in --- assuming it works
somewhere.

> -	if (ret)
> -		return ret;
> -
> -	if (cfg->flags & V4L2_MBUS_HSYNC_ACTIVE_LOW)
> -		ret = ov6650_reg_rmw(client, REG_COMF, COMF_HREF_LOW, 0);
> -	else if (cfg->flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
> -		ret = ov6650_reg_rmw(client, REG_COMF, 0, COMF_HREF_LOW);
> -	if (ret)
> -		return ret;
> -
> -	if (cfg->flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
> -		ret = ov6650_reg_rmw(client, REG_COMJ, COMJ_VSYNC_HIGH, 0);
> -	else if (cfg->flags & V4L2_MBUS_VSYNC_ACTIVE_LOW)
> -		ret = ov6650_reg_rmw(client, REG_COMJ, 0, COMJ_VSYNC_HIGH);
> -	if (ret)
> -		return ret;
> -
> -	/*
> -	 * Update the configuration to report what is actually applied to
> -	 * the hardware.
> -	 */
> -	return ov6650_get_mbus_config(sd, pad, cfg);
> -}
> -
>  static const struct v4l2_subdev_video_ops ov6650_video_ops = {
>  	.s_stream	= ov6650_s_stream,
>  	.g_frame_interval = ov6650_g_frame_interval,
> @@ -993,7 +957,6 @@ static const struct v4l2_subdev_pad_ops ov6650_pad_ops = {
>  	.get_fmt	= ov6650_get_fmt,
>  	.set_fmt	= ov6650_set_fmt,
>  	.get_mbus_config = ov6650_get_mbus_config,
> -	.set_mbus_config = ov6650_set_mbus_config,
>  };
>  
>  static const struct v4l2_subdev_ops ov6650_subdev_ops = {

-- 
Kind regards,

Sakari Ailus