Re: [PATCH v3 2/2] media: Add a driver for the ov7251 camera sensor

2018-04-25 Thread Todor Tomov
Hi,

On 25.04.2018 13:54, Sakari Ailus wrote:
> Hi Todor,
> 
> Thanks for the update. Just a few minor comments below.

Thanks for the review again, Sakari.
I'm preparing the next version.

Best regards,
Todor


Re: [PATCH v3 2/2] media: Add a driver for the ov7251 camera sensor

2018-04-25 Thread Sakari Ailus
Hi Todor,

Thanks for the update. Just a few minor comments below.

On Tue, Apr 24, 2018 at 06:01:07PM +0300, Todor Tomov wrote:
> The ov7251 sensor is a 1/7.5-Inch B VGA (640x480) CMOS Digital Image
> Sensor from Omnivision.
> 
> The driver supports the following modes:
> - 640x480 30fps
> - 640x480 60fps
> - 640x480 90fps
> 
> Output format is 10bit B RAW - MEDIA_BUS_FMT_Y10_1X10.
> 
> The driver supports configuration via user controls for:
> - exposure and gain;
> - horizontal and vertical flip;
> - test pattern.
> 
> Signed-off-by: Todor Tomov 
> Reviewed-by: Jacopo Mondi 
> ---
>  drivers/media/i2c/Kconfig  |   13 +
>  drivers/media/i2c/Makefile |1 +
>  drivers/media/i2c/ov7251.c | 1501 
> 
>  3 files changed, 1515 insertions(+)
>  create mode 100644 drivers/media/i2c/ov7251.c
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 541f0d28..1ff7aaf 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -688,6 +688,19 @@ config VIDEO_OV5695
> To compile this driver as a module, choose M here: the
> module will be called ov5695.
>  
> +config VIDEO_OV7251
> + tristate "OmniVision OV7251 sensor support"
> + depends on OF

I think you could drop OF dependency.

> + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> + depends on MEDIA_CAMERA_SUPPORT
> + select V4L2_FWNODE
> + help
> +   This is a Video4Linux2 sensor-level driver for the OmniVision
> +   OV7251 camera.
> +
> +   To compile this driver as a module, choose M here: the
> +   module will be called ov7251.
> +
>  config VIDEO_OV772X
>   tristate "OmniVision OV772x sensor support"
>   depends on I2C && VIDEO_V4L2
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index ea34aee..c8585b1 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -70,6 +70,7 @@ obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
>  obj-$(CONFIG_VIDEO_OV5670) += ov5670.o
>  obj-$(CONFIG_VIDEO_OV5695) += ov5695.o
>  obj-$(CONFIG_VIDEO_OV6650) += ov6650.o
> +obj-$(CONFIG_VIDEO_OV7251) += ov7251.o
>  obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
>  obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
>  obj-$(CONFIG_VIDEO_OV772X) += ov772x.o
> diff --git a/drivers/media/i2c/ov7251.c b/drivers/media/i2c/ov7251.c
> new file mode 100644
> index 000..69e70a0
> --- /dev/null
> +++ b/drivers/media/i2c/ov7251.c
> @@ -0,0 +1,1501 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for the OV7251 camera sensor.
> + *
> + * Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2017-2018, Linaro Ltd.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define OV7251_SC_MODE_SELECT0x0100
> +#define OV7251_SC_MODE_SELECT_SW_STANDBY 0x0
> +#define OV7251_SC_MODE_SELECT_STREAMING  0x1
> +
> +#define OV7251_CHIP_ID_HIGH  0x300a
> +#define OV7251_CHIP_ID_HIGH_BYTE 0x77
> +#define OV7251_CHIP_ID_LOW   0x300b
> +#define OV7251_CHIP_ID_LOW_BYTE  0x50
> +#define OV7251_SC_GP_IO_IN1  0x3029
> +#define OV7251_AEC_EXPO_00x3500
> +#define OV7251_AEC_EXPO_10x3501
> +#define OV7251_AEC_EXPO_20x3502
> +#define OV7251_AEC_AGC_ADJ_0 0x350a
> +#define OV7251_AEC_AGC_ADJ_1 0x350b
> +#define OV7251_TIMING_FORMAT10x3820
> +#define OV7251_TIMING_FORMAT1_VFLIP  BIT(2)
> +#define OV7251_TIMING_FORMAT20x3821
> +#define OV7251_TIMING_FORMAT2_MIRROR BIT(2)
> +#define OV7251_PRE_ISP_000x5e00
> +#define OV7251_PRE_ISP_00_TEST_PATTERN   BIT(7)
> +
> +struct reg_value {
> + u16 reg;
> + u8 val;
> +};
> +
> +struct ov7251_mode_info {
> + u32 width;
> + u32 height;
> + const struct reg_value *data;
> + u32 data_size;
> + u32 pixel_clock;
> + u32 link_freq;
> + u16 exposure_max;
> + u16 exposure_def;
> + struct v4l2_fract timeperframe;
> +};
> +
> +struct ov7251 {
> + struct i2c_client *i2c_client;
> + struct device *dev;
> + struct v4l2_subdev sd;
> + struct media_pad pad;
> + struct v4l2_fwnode_endpoint ep;
> + struct v4l2_mbus_framefmt fmt;
> + struct v4l2_rect crop;
> + struct clk *xclk;
> + u32 xclk_freq;
> +
> + struct regulator *io_regulator;
> + struct regulator *core_regulator;
> + struct regulator *analog_regulator;
> +
> + const struct ov7251_mode_info *current_mode;
> +
> + struct v4l2_ctrl_handler ctrls;
> + struct v4l2_ctrl *pixel_clock;
> + struct v4l2_ctrl *link_freq;
> + struct v4l2_ctrl *exposure;
> + struct v4l2_ctrl *gain;
> +
> + /* Cached register values */
> + u8 aec_pk_manual;
> + u8 

[PATCH v3 2/2] media: Add a driver for the ov7251 camera sensor

2018-04-24 Thread Todor Tomov
The ov7251 sensor is a 1/7.5-Inch B VGA (640x480) CMOS Digital Image
Sensor from Omnivision.

The driver supports the following modes:
- 640x480 30fps
- 640x480 60fps
- 640x480 90fps

Output format is 10bit B RAW - MEDIA_BUS_FMT_Y10_1X10.

The driver supports configuration via user controls for:
- exposure and gain;
- horizontal and vertical flip;
- test pattern.

Signed-off-by: Todor Tomov 
Reviewed-by: Jacopo Mondi 
---
 drivers/media/i2c/Kconfig  |   13 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov7251.c | 1501 
 3 files changed, 1515 insertions(+)
 create mode 100644 drivers/media/i2c/ov7251.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 541f0d28..1ff7aaf 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -688,6 +688,19 @@ config VIDEO_OV5695
  To compile this driver as a module, choose M here: the
  module will be called ov5695.
 
+config VIDEO_OV7251
+   tristate "OmniVision OV7251 sensor support"
+   depends on OF
+   depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+   depends on MEDIA_CAMERA_SUPPORT
+   select V4L2_FWNODE
+   help
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV7251 camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov7251.
+
 config VIDEO_OV772X
tristate "OmniVision OV772x sensor support"
depends on I2C && VIDEO_V4L2
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index ea34aee..c8585b1 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -70,6 +70,7 @@ obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
 obj-$(CONFIG_VIDEO_OV5670) += ov5670.o
 obj-$(CONFIG_VIDEO_OV5695) += ov5695.o
 obj-$(CONFIG_VIDEO_OV6650) += ov6650.o
+obj-$(CONFIG_VIDEO_OV7251) += ov7251.o
 obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
 obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
 obj-$(CONFIG_VIDEO_OV772X) += ov772x.o
diff --git a/drivers/media/i2c/ov7251.c b/drivers/media/i2c/ov7251.c
new file mode 100644
index 000..69e70a0
--- /dev/null
+++ b/drivers/media/i2c/ov7251.c
@@ -0,0 +1,1501 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for the OV7251 camera sensor.
+ *
+ * Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2017-2018, Linaro Ltd.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define OV7251_SC_MODE_SELECT  0x0100
+#define OV7251_SC_MODE_SELECT_SW_STANDBY   0x0
+#define OV7251_SC_MODE_SELECT_STREAMING0x1
+
+#define OV7251_CHIP_ID_HIGH0x300a
+#define OV7251_CHIP_ID_HIGH_BYTE   0x77
+#define OV7251_CHIP_ID_LOW 0x300b
+#define OV7251_CHIP_ID_LOW_BYTE0x50
+#define OV7251_SC_GP_IO_IN10x3029
+#define OV7251_AEC_EXPO_0  0x3500
+#define OV7251_AEC_EXPO_1  0x3501
+#define OV7251_AEC_EXPO_2  0x3502
+#define OV7251_AEC_AGC_ADJ_0   0x350a
+#define OV7251_AEC_AGC_ADJ_1   0x350b
+#define OV7251_TIMING_FORMAT1  0x3820
+#define OV7251_TIMING_FORMAT1_VFLIPBIT(2)
+#define OV7251_TIMING_FORMAT2  0x3821
+#define OV7251_TIMING_FORMAT2_MIRROR   BIT(2)
+#define OV7251_PRE_ISP_00  0x5e00
+#define OV7251_PRE_ISP_00_TEST_PATTERN BIT(7)
+
+struct reg_value {
+   u16 reg;
+   u8 val;
+};
+
+struct ov7251_mode_info {
+   u32 width;
+   u32 height;
+   const struct reg_value *data;
+   u32 data_size;
+   u32 pixel_clock;
+   u32 link_freq;
+   u16 exposure_max;
+   u16 exposure_def;
+   struct v4l2_fract timeperframe;
+};
+
+struct ov7251 {
+   struct i2c_client *i2c_client;
+   struct device *dev;
+   struct v4l2_subdev sd;
+   struct media_pad pad;
+   struct v4l2_fwnode_endpoint ep;
+   struct v4l2_mbus_framefmt fmt;
+   struct v4l2_rect crop;
+   struct clk *xclk;
+   u32 xclk_freq;
+
+   struct regulator *io_regulator;
+   struct regulator *core_regulator;
+   struct regulator *analog_regulator;
+
+   const struct ov7251_mode_info *current_mode;
+
+   struct v4l2_ctrl_handler ctrls;
+   struct v4l2_ctrl *pixel_clock;
+   struct v4l2_ctrl *link_freq;
+   struct v4l2_ctrl *exposure;
+   struct v4l2_ctrl *gain;
+
+   /* Cached register values */
+   u8 aec_pk_manual;
+   u8 pre_isp_00;
+   u8 timing_format1;
+   u8 timing_format2;
+
+   struct mutex lock; /* lock to protect power state, ctrls and mode */
+   bool power_on;
+
+   struct gpio_desc *enable_gpio;
+};
+
+static inline struct ov7251 *to_ov7251(struct v4l2_subdev *sd)
+{
+   return container_of(sd, struct ov7251, sd);
+}
+
+static const struct reg_value