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