Re: [PATCHv6 11/14] ov2640: convert from soc-camera to a standard subdev sensor driver.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Y're welcome, thanks Hans for this work, this allows us to drop our 
camera interface driver directly based on subdev.

BR,
Hugues.

On 03/29/2017 03:44 PM, Hans Verkuil wrote:
> On 29/03/17 15:42, Hugues FRUCHET wrote:
>> Acked-by: Hugues Fruchet <hugues.fruchet@xxxxxx>
>>
>> Tested successfully on STM324x9I-EVAL evaluation board embedding
>> an OV2640 camera sensor.
>>
>> I don't understand the comment around s_power op that has been dropped
>> (it is there in code), and no problem is observed doing several
>> open/close, tell me if I miss something.
>
> Darn, I forgot to remove that comment in the commit log. It's a leftover from
> an earlier version.
>
> Regards,
>
> 	Hans
>
>>
>> BR,
>> Hugues.
>>
>> On 03/28/2017 10:23 AM, Hans Verkuil wrote:
>>> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>>>
>>> Convert ov2640 to a standard subdev driver. The soc-camera driver no longer
>>> uses this driver, so it can safely be converted.
>>>
>>> Note: the s_power op has been dropped: this never worked. When the last open()
>>> is closed, then the power is turned off, and when it is opened again the power
>>> is turned on again, but the old state isn't restored.
>>>
>>> Someone else can figure out in the future how to get this working correctly,
>>> but I don't want to spend more time on this.
>>>
>>> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>>> Acked-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
>>> ---
>>>  drivers/media/i2c/Kconfig                   | 11 ++++
>>>  drivers/media/i2c/Makefile                  |  1 +
>>>  drivers/media/i2c/{soc_camera => }/ov2640.c | 89 +++++------------------------
>>>  drivers/media/i2c/soc_camera/Kconfig        |  6 --
>>>  drivers/media/i2c/soc_camera/Makefile       |  1 -
>>>  5 files changed, 27 insertions(+), 81 deletions(-)
>>>  rename drivers/media/i2c/{soc_camera => }/ov2640.c (94%)
>>>
>>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>>> index cee1dae6e014..db2c63f592c5 100644
>>> --- a/drivers/media/i2c/Kconfig
>>> +++ b/drivers/media/i2c/Kconfig
>>> @@ -520,6 +520,17 @@ config VIDEO_APTINA_PLL
>>>  config VIDEO_SMIAPP_PLL
>>>  	tristate
>>>
>>> +config VIDEO_OV2640
>>> +	tristate "OmniVision OV2640 sensor support"
>>> +	depends on VIDEO_V4L2 && I2C && GPIOLIB
>>> +	depends on MEDIA_CAMERA_SUPPORT
>>> +	help
>>> +	  This is a Video4Linux2 sensor-level driver for the OmniVision
>>> +	  OV2640 camera.
>>> +
>>> +	  To compile this driver as a module, choose M here: the
>>> +	  module will be called ov2640.
>>> +
>>>  config VIDEO_OV2659
>>>  	tristate "OmniVision OV2659 sensor support"
>>>  	depends on VIDEO_V4L2 && I2C
>>> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
>>> index 5bc7bbeb5499..50af1e11c85a 100644
>>> --- a/drivers/media/i2c/Makefile
>>> +++ b/drivers/media/i2c/Makefile
>>> @@ -57,6 +57,7 @@ obj-$(CONFIG_VIDEO_VP27SMPX) += vp27smpx.o
>>>  obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
>>>  obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
>>>  obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
>>> +obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
>>>  obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
>>>  obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
>>>  obj-$(CONFIG_VIDEO_OV9650) += ov9650.o
>>> diff --git a/drivers/media/i2c/soc_camera/ov2640.c b/drivers/media/i2c/ov2640.c
>>> similarity index 94%
>>> rename from drivers/media/i2c/soc_camera/ov2640.c
>>> rename to drivers/media/i2c/ov2640.c
>>> index b9a0069f5b33..83f88efbce69 100644
>>> --- a/drivers/media/i2c/soc_camera/ov2640.c
>>> +++ b/drivers/media/i2c/ov2640.c
>>> @@ -24,8 +24,8 @@
>>>  #include <linux/v4l2-mediabus.h>
>>>  #include <linux/videodev2.h>
>>>
>>> -#include <media/soc_camera.h>
>>>  #include <media/v4l2-clk.h>
>>> +#include <media/v4l2-device.h>
>>>  #include <media/v4l2-subdev.h>
>>>  #include <media/v4l2-ctrls.h>
>>>  #include <media/v4l2-image-sizes.h>
>>> @@ -287,7 +287,6 @@ struct ov2640_priv {
>>>  	struct v4l2_clk			*clk;
>>>  	const struct ov2640_win_size	*win;
>>>
>>> -	struct soc_camera_subdev_desc	ssdd_dt;
>>>  	struct gpio_desc *resetb_gpio;
>>>  	struct gpio_desc *pwdn_gpio;
>>>  };
>>> @@ -677,13 +676,8 @@ static int ov2640_reset(struct i2c_client *client)
>>>  }
>>>
>>>  /*
>>> - * soc_camera_ops functions
>>> + * functions
>>>   */
>>> -static int ov2640_s_stream(struct v4l2_subdev *sd, int enable)
>>> -{
>>> -	return 0;
>>> -}
>>> -
>>>  static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
>>>  {
>>>  	struct v4l2_subdev *sd =
>>> @@ -744,10 +738,16 @@ static int ov2640_s_register(struct v4l2_subdev *sd,
>>>  static int ov2640_s_power(struct v4l2_subdev *sd, int on)
>>>  {
>>>  	struct i2c_client *client = v4l2_get_subdevdata(sd);
>>> -	struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
>>>  	struct ov2640_priv *priv = to_ov2640(client);
>>>
>>> -	return soc_camera_set_power(&client->dev, ssdd, priv->clk, on);
>>> +	gpiod_direction_output(priv->pwdn_gpio, !on);
>>> +	if (on && priv->resetb_gpio) {
>>> +		/* Active the resetb pin to perform a reset pulse */
>>> +		gpiod_direction_output(priv->resetb_gpio, 1);
>>> +		usleep_range(3000, 5000);
>>> +		gpiod_direction_output(priv->resetb_gpio, 0);
>>> +	}
>>> +	return 0;
>>>  }
>>>
>>>  /* Select the nearest higher resolution for capture */
>>> @@ -994,26 +994,6 @@ static struct v4l2_subdev_core_ops ov2640_subdev_core_ops = {
>>>  	.s_power	= ov2640_s_power,
>>>  };
>>>
>>> -static int ov2640_g_mbus_config(struct v4l2_subdev *sd,
>>> -				struct v4l2_mbus_config *cfg)
>>> -{
>>> -	struct i2c_client *client = v4l2_get_subdevdata(sd);
>>> -	struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
>>> -
>>> -	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->flags = soc_camera_apply_board_flags(ssdd, cfg);
>>> -
>>> -	return 0;
>>> -}
>>> -
>>> -static struct v4l2_subdev_video_ops ov2640_subdev_video_ops = {
>>> -	.s_stream	= ov2640_s_stream,
>>> -	.g_mbus_config	= ov2640_g_mbus_config,
>>> -};
>>> -
>>>  static const struct v4l2_subdev_pad_ops ov2640_subdev_pad_ops = {
>>>  	.enum_mbus_code = ov2640_enum_mbus_code,
>>>  	.get_selection	= ov2640_get_selection,
>>> @@ -1023,40 +1003,9 @@ static const struct v4l2_subdev_pad_ops ov2640_subdev_pad_ops = {
>>>
>>>  static struct v4l2_subdev_ops ov2640_subdev_ops = {
>>>  	.core	= &ov2640_subdev_core_ops,
>>> -	.video	= &ov2640_subdev_video_ops,
>>>  	.pad	= &ov2640_subdev_pad_ops,
>>>  };
>>>
>>> -/* OF probe functions */
>>> -static int ov2640_hw_power(struct device *dev, int on)
>>> -{
>>> -	struct i2c_client *client = to_i2c_client(dev);
>>> -	struct ov2640_priv *priv = to_ov2640(client);
>>> -
>>> -	dev_dbg(&client->dev, "%s: %s the camera\n",
>>> -			__func__, on ? "ENABLE" : "DISABLE");
>>> -
>>> -	if (priv->pwdn_gpio)
>>> -		gpiod_direction_output(priv->pwdn_gpio, !on);
>>> -
>>> -	return 0;
>>> -}
>>> -
>>> -static int ov2640_hw_reset(struct device *dev)
>>> -{
>>> -	struct i2c_client *client = to_i2c_client(dev);
>>> -	struct ov2640_priv *priv = to_ov2640(client);
>>> -
>>> -	if (priv->resetb_gpio) {
>>> -		/* Active the resetb pin to perform a reset pulse */
>>> -		gpiod_direction_output(priv->resetb_gpio, 1);
>>> -		usleep_range(3000, 5000);
>>> -		gpiod_direction_output(priv->resetb_gpio, 0);
>>> -	}
>>> -
>>> -	return 0;
>>> -}
>>> -
>>>  static int ov2640_probe_dt(struct i2c_client *client,
>>>  		struct ov2640_priv *priv)
>>>  {
>>> @@ -1076,11 +1025,6 @@ static int ov2640_probe_dt(struct i2c_client *client,
>>>  	else if (IS_ERR(priv->pwdn_gpio))
>>>  		return PTR_ERR(priv->pwdn_gpio);
>>>
>>> -	/* Initialize the soc_camera_subdev_desc */
>>> -	priv->ssdd_dt.power = ov2640_hw_power;
>>> -	priv->ssdd_dt.reset = ov2640_hw_reset;
>>> -	client->dev.platform_data = &priv->ssdd_dt;
>>> -
>>>  	return 0;
>>>  }
>>>
>>> @@ -1091,7 +1035,6 @@ static int ov2640_probe(struct i2c_client *client,
>>>  			const struct i2c_device_id *did)
>>>  {
>>>  	struct ov2640_priv	*priv;
>>> -	struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
>>>  	struct i2c_adapter	*adapter = to_i2c_adapter(client->dev.parent);
>>>  	int			ret;
>>>
>>> @@ -1112,17 +1055,15 @@ static int ov2640_probe(struct i2c_client *client,
>>>  	if (IS_ERR(priv->clk))
>>>  		return -EPROBE_DEFER;
>>>
>>> -	if (!ssdd && !client->dev.of_node) {
>>> +	if (!client->dev.of_node) {
>>>  		dev_err(&client->dev, "Missing platform_data for driver\n");
>>>  		ret = -EINVAL;
>>>  		goto err_clk;
>>>  	}
>>>
>>> -	if (!ssdd) {
>>> -		ret = ov2640_probe_dt(client, priv);
>>> -		if (ret)
>>> -			goto err_clk;
>>> -	}
>>> +	ret = ov2640_probe_dt(client, priv);
>>> +	if (ret)
>>> +		goto err_clk;
>>>
>>>  	v4l2_i2c_subdev_init(&priv->subdev, client, &ov2640_subdev_ops);
>>>  	v4l2_ctrl_handler_init(&priv->hdl, 2);
>>> @@ -1190,6 +1131,6 @@ static struct i2c_driver ov2640_i2c_driver = {
>>>
>>>  module_i2c_driver(ov2640_i2c_driver);
>>>
>>> -MODULE_DESCRIPTION("SoC Camera driver for Omni Vision 2640 sensor");
>>> +MODULE_DESCRIPTION("Driver for Omni Vision 2640 sensor");
>>>  MODULE_AUTHOR("Alberto Panizzo");
>>>  MODULE_LICENSE("GPL v2");
>>> diff --git a/drivers/media/i2c/soc_camera/Kconfig b/drivers/media/i2c/soc_camera/Kconfig
>>> index 7704bcf5cc25..96859f37cb1c 100644
>>> --- a/drivers/media/i2c/soc_camera/Kconfig
>>> +++ b/drivers/media/i2c/soc_camera/Kconfig
>>> @@ -41,12 +41,6 @@ config SOC_CAMERA_MT9V022
>>>  	help
>>>  	  This driver supports MT9V022 cameras from Micron
>>>
>>> -config SOC_CAMERA_OV2640
>>> -	tristate "ov2640 camera support"
>>> -	depends on SOC_CAMERA && I2C
>>> -	help
>>> -	  This is a ov2640 camera driver
>>> -
>>>  config SOC_CAMERA_OV5642
>>>  	tristate "ov5642 camera support"
>>>  	depends on SOC_CAMERA && I2C
>>> diff --git a/drivers/media/i2c/soc_camera/Makefile b/drivers/media/i2c/soc_camera/Makefile
>>> index 6f994f9353a0..974bdb721dbe 100644
>>> --- a/drivers/media/i2c/soc_camera/Makefile
>>> +++ b/drivers/media/i2c/soc_camera/Makefile
>>> @@ -3,7 +3,6 @@ obj-$(CONFIG_SOC_CAMERA_MT9M001)	+= mt9m001.o
>>>  obj-$(CONFIG_SOC_CAMERA_MT9T031)	+= mt9t031.o
>>>  obj-$(CONFIG_SOC_CAMERA_MT9T112)	+= mt9t112.o
>>>  obj-$(CONFIG_SOC_CAMERA_MT9V022)	+= mt9v022.o
>>> -obj-$(CONFIG_SOC_CAMERA_OV2640)		+= ov2640.o
>>>  obj-$(CONFIG_SOC_CAMERA_OV5642)		+= ov5642.o
>>>  obj-$(CONFIG_SOC_CAMERA_OV6650)		+= ov6650.o
>>>  obj-$(CONFIG_SOC_CAMERA_OV772X)		+= ov772x.o
>>>
>--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux