Re: [PATCH v3 2/2] media: ov2680: Add Omnivision OV2680 sensor driver
Hi Sakari, On Sat 24 Mar 2018 at 10:57, Sakari Ailus wrote: Hi Rui, I wanted to go through the code the last time and I'd have a few review comments below... Thanks for the review. On Tue, Mar 13, 2018 at 11:33:11AM +, Rui Miguel Silva wrote: ... +static int ov2680_gain_set(struct ov2680_dev *sensor, bool auto_gain) +{ + struct ov2680_ctrls *ctrls = &sensor->ctrls; + u32 gain; + int ret; + + ret = ov2680_mod_reg(sensor, OV2680_REG_R_MANUAL, BIT(1), +auto_gain ? 0 : BIT(1)); + if (ret < 0) + return ret; + + if (auto_gain || !ctrls->gain->is_new) + return 0; + + gain = ctrls->gain->val; + + ret = ov2680_write_reg16(sensor, OV2680_REG_GAIN_PK, gain); + + return 0; +} + +static int ov2680_gain_get(struct ov2680_dev *sensor) +{ + u32 gain; + int ret; + + ret = ov2680_read_reg16(sensor, OV2680_REG_GAIN_PK, &gain); + if (ret) + return ret; + + return gain; +} + +static int ov2680_auto_gain_enable(struct ov2680_dev *sensor) +{ + return ov2680_gain_set(sensor, true); +} + +static int ov2680_auto_gain_disable(struct ov2680_dev *sensor) +{ + return ov2680_gain_set(sensor, false); +} + +static int ov2680_exposure_set(struct ov2680_dev *sensor, bool auto_exp) +{ + struct ov2680_ctrls *ctrls = &sensor->ctrls; + u32 exp; + int ret; + + ret = ov2680_mod_reg(sensor, OV2680_REG_R_MANUAL, BIT(0), +auto_exp ? 0 : BIT(0)); + if (ret < 0) + return ret; + + if (auto_exp || !ctrls->exposure->is_new) + return 0; + + exp = (u32)ctrls->exposure->val; + exp <<= 4; + + return ov2680_write_reg24(sensor, OV2680_REG_EXPOSURE_PK_HIGH, exp); +} + +static int ov2680_exposure_get(struct ov2680_dev *sensor) +{ + int ret; + u32 exp; + + ret = ov2680_read_reg24(sensor, OV2680_REG_EXPOSURE_PK_HIGH, &exp); + if (ret) + return ret; + + return exp >> 4; +} + +static int ov2680_auto_exposure_enable(struct ov2680_dev *sensor) +{ + return ov2680_exposure_set(sensor, true); +} + +static int ov2680_auto_exposure_disable(struct ov2680_dev *sensor) +{ + return ov2680_exposure_set(sensor, false); +} I still think you could call the function actually doing the work, and pass the bool parameter. That'd be much clearer. Please see the comments below, too; they're related. Same for gain. Ok, no problem, will change that in v4. ... +static int ov2680_g_volatile_ctrl(struct v4l2_ctrl *ctrl) +{ + struct v4l2_subdev *sd = ctrl_to_sd(ctrl); + struct ov2680_dev *sensor = to_ov2680_dev(sd); + int val; + + if (!sensor->is_enabled) + return 0; + + switch (ctrl->id) { + case V4L2_CID_AUTOGAIN: + if (!ctrl->val) + return 0; + val = ov2680_gain_get(sensor); + if (val < 0) + return val; + sensor->ctrls.gain->val = val; + break; + case V4L2_CID_EXPOSURE_AUTO: I reckon the purpose of implementing volatile controls here would be to provide the exposure and gain values to the user. But the controls here are the ones enabling or disabling the automatic behaviour, not the value of those settings themselves. + if (ctrl->val == V4L2_EXPOSURE_MANUAL) + return 0; + val = ov2680_exposure_get(sensor); + if (val < 0) + return val; + sensor->ctrls.exposure->val = val; + break; + } + + return 0; +} + +static int ov2680_s_ctrl(struct v4l2_ctrl *ctrl) +{ + struct v4l2_subdev *sd = ctrl_to_sd(ctrl); + struct ov2680_dev *sensor = to_ov2680_dev(sd); + + if (!sensor->is_enabled) + return 0; + + switch (ctrl->id) { + case V4L2_CID_AUTOGAIN: + return ov2680_gain_set(sensor, !!ctrl->val); + case V4L2_CID_EXPOSURE_AUTO: + return ov2680_exposure_set(sensor, !!ctrl->val); With this, you can enable or disable automatic exposure and gain, but you cannot manually set the values. Are such controls useful? Unless I'm mistaken, exposure or gain are not settable, so you should make them read-only controls. Or better, allow setting them if automatic control is disabled. Yeah, I could definitely change the values of exposure and gain also, but that may came how the ctrl group work internally. But I will change and add them. + case V4L2_CID_VFLIP: + if (sensor->is_streaming) + return -EBUSY; + if (ctrl->val) + return ov2680_vflip_enable(sensor); + else + return ov2680_vflip_disable(sensor); + case V4L2_CID_HFLIP: + if (ctrl->val) +
Re: [PATCH v3 2/2] media: ov2680: Add Omnivision OV2680 sensor driver
Hi Rui, I wanted to go through the code the last time and I'd have a few review comments below... On Tue, Mar 13, 2018 at 11:33:11AM +, Rui Miguel Silva wrote: ... > +static int ov2680_gain_set(struct ov2680_dev *sensor, bool auto_gain) > +{ > + struct ov2680_ctrls *ctrls = &sensor->ctrls; > + u32 gain; > + int ret; > + > + ret = ov2680_mod_reg(sensor, OV2680_REG_R_MANUAL, BIT(1), > + auto_gain ? 0 : BIT(1)); > + if (ret < 0) > + return ret; > + > + if (auto_gain || !ctrls->gain->is_new) > + return 0; > + > + gain = ctrls->gain->val; > + > + ret = ov2680_write_reg16(sensor, OV2680_REG_GAIN_PK, gain); > + > + return 0; > +} > + > +static int ov2680_gain_get(struct ov2680_dev *sensor) > +{ > + u32 gain; > + int ret; > + > + ret = ov2680_read_reg16(sensor, OV2680_REG_GAIN_PK, &gain); > + if (ret) > + return ret; > + > + return gain; > +} > + > +static int ov2680_auto_gain_enable(struct ov2680_dev *sensor) > +{ > + return ov2680_gain_set(sensor, true); > +} > + > +static int ov2680_auto_gain_disable(struct ov2680_dev *sensor) > +{ > + return ov2680_gain_set(sensor, false); > +} > + > +static int ov2680_exposure_set(struct ov2680_dev *sensor, bool auto_exp) > +{ > + struct ov2680_ctrls *ctrls = &sensor->ctrls; > + u32 exp; > + int ret; > + > + ret = ov2680_mod_reg(sensor, OV2680_REG_R_MANUAL, BIT(0), > + auto_exp ? 0 : BIT(0)); > + if (ret < 0) > + return ret; > + > + if (auto_exp || !ctrls->exposure->is_new) > + return 0; > + > + exp = (u32)ctrls->exposure->val; > + exp <<= 4; > + > + return ov2680_write_reg24(sensor, OV2680_REG_EXPOSURE_PK_HIGH, exp); > +} > + > +static int ov2680_exposure_get(struct ov2680_dev *sensor) > +{ > + int ret; > + u32 exp; > + > + ret = ov2680_read_reg24(sensor, OV2680_REG_EXPOSURE_PK_HIGH, &exp); > + if (ret) > + return ret; > + > + return exp >> 4; > +} > + > +static int ov2680_auto_exposure_enable(struct ov2680_dev *sensor) > +{ > + return ov2680_exposure_set(sensor, true); > +} > + > +static int ov2680_auto_exposure_disable(struct ov2680_dev *sensor) > +{ > + return ov2680_exposure_set(sensor, false); > +} I still think you could call the function actually doing the work, and pass the bool parameter. That'd be much clearer. Please see the comments below, too; they're related. Same for gain. ... > +static int ov2680_g_volatile_ctrl(struct v4l2_ctrl *ctrl) > +{ > + struct v4l2_subdev *sd = ctrl_to_sd(ctrl); > + struct ov2680_dev *sensor = to_ov2680_dev(sd); > + int val; > + > + if (!sensor->is_enabled) > + return 0; > + > + switch (ctrl->id) { > + case V4L2_CID_AUTOGAIN: > + if (!ctrl->val) > + return 0; > + val = ov2680_gain_get(sensor); > + if (val < 0) > + return val; > + sensor->ctrls.gain->val = val; > + break; > + case V4L2_CID_EXPOSURE_AUTO: I reckon the purpose of implementing volatile controls here would be to provide the exposure and gain values to the user. But the controls here are the ones enabling or disabling the automatic behaviour, not the value of those settings themselves. > + if (ctrl->val == V4L2_EXPOSURE_MANUAL) > + return 0; > + val = ov2680_exposure_get(sensor); > + if (val < 0) > + return val; > + sensor->ctrls.exposure->val = val; > + break; > + } > + > + return 0; > +} > + > +static int ov2680_s_ctrl(struct v4l2_ctrl *ctrl) > +{ > + struct v4l2_subdev *sd = ctrl_to_sd(ctrl); > + struct ov2680_dev *sensor = to_ov2680_dev(sd); > + > + if (!sensor->is_enabled) > + return 0; > + > + switch (ctrl->id) { > + case V4L2_CID_AUTOGAIN: > + return ov2680_gain_set(sensor, !!ctrl->val); > + case V4L2_CID_EXPOSURE_AUTO: > + return ov2680_exposure_set(sensor, !!ctrl->val); With this, you can enable or disable automatic exposure and gain, but you cannot manually set the values. Are such controls useful? Unless I'm mistaken, exposure or gain are not settable, so you should make them read-only controls. Or better, allow setting them if automatic control is disabled. > + case V4L2_CID_VFLIP: > + if (sensor->is_streaming) > + return -EBUSY; > + if (ctrl->val) > + return ov2680_vflip_enable(sensor); > + else > + return ov2680_vflip_disable(sensor); > + case V4L2_CID_HFLIP: > + if (ctrl->val) > + return ov2680_hflip_enable(sensor); > + else > + return ov2680_hflip_disable(sensor); > + case V4L2_CID_TEST_PATTERN: > + return ov2680_tes
Re: [PATCH v3 2/2] media: ov2680: Add Omnivision OV2680 sensor driver
Hi Sakari, On Fri 16 Mar 2018 at 16:10, Sakari Ailus wrote: On Thu, Mar 15, 2018 at 09:29:33AM +, Rui Miguel Silva wrote: Hi, On Wed 14 Mar 2018 at 19:39, kbuild test robot wrote: > Hi Rui, > > I love your patch! Yet something to improve: > > [auto build test ERROR on v4.16-rc4] > [cannot apply to next-20180314] > [if your patch is applied to the wrong git tree, please drop > us a note > to help improve the system] > > url: > https://github.com/0day-ci/linux/commits/Rui-Miguel-Silva/media-Introduce-Omnivision-OV2680-driver/20180315-020617 > config: sh-allmodconfig (attached as .config) > compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 > reproduce: > wget > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross > -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > make.cross ARCH=sh > > All errors (new ones prefixed by >>): > >drivers/media/i2c/ov2680.c: In function 'ov2680_set_fmt': > > > drivers/media/i2c/ov2680.c:713:9: error: implicit > > > declaration of > > > function 'v4l2_find_nearest_size'; did you mean > > > 'v4l2_find_nearest_format'? > > > [-Werror=implicit-function-declaration] > mode = v4l2_find_nearest_size(ov2680_mode_data, > ^~ > v4l2_find_nearest_format As requested by maintainer this series depend on this patch [0], which introduce this macro. I am not sure of the status of that patch though. No need to worry about that, the sensor driver will just be merged after the dependencies are in. Mauro said he'd handle the pull request early next week. Great, Many thanks for everything. --- Cheers, Rui
Re: [PATCH v3 2/2] media: ov2680: Add Omnivision OV2680 sensor driver
On Thu, Mar 15, 2018 at 09:29:33AM +, Rui Miguel Silva wrote: > Hi, > On Wed 14 Mar 2018 at 19:39, kbuild test robot wrote: > > Hi Rui, > > > > I love your patch! Yet something to improve: > > > > [auto build test ERROR on v4.16-rc4] > > [cannot apply to next-20180314] > > [if your patch is applied to the wrong git tree, please drop us a note > > to help improve the system] > > > > url: > > https://github.com/0day-ci/linux/commits/Rui-Miguel-Silva/media-Introduce-Omnivision-OV2680-driver/20180315-020617 > > config: sh-allmodconfig (attached as .config) > > compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 > > reproduce: > > wget > > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross > > -O ~/bin/make.cross > > chmod +x ~/bin/make.cross > > # save the attached .config to linux build tree > > make.cross ARCH=sh > > > > All errors (new ones prefixed by >>): > > > >drivers/media/i2c/ov2680.c: In function 'ov2680_set_fmt': > > > > drivers/media/i2c/ov2680.c:713:9: error: implicit declaration of > > > > function 'v4l2_find_nearest_size'; did you mean > > > > 'v4l2_find_nearest_format'? > > > > [-Werror=implicit-function-declaration] > > mode = v4l2_find_nearest_size(ov2680_mode_data, > > ^~ > > v4l2_find_nearest_format > > As requested by maintainer this series depend on this patch [0], which > introduce this macro. I am not sure of the status of that patch though. No need to worry about that, the sensor driver will just be merged after the dependencies are in. Mauro said he'd handle the pull request early next week. -- Sakari Ailus sakari.ai...@linux.intel.com
Re: [PATCH v3 2/2] media: ov2680: Add Omnivision OV2680 sensor driver
Hi, On Wed 14 Mar 2018 at 19:39, kbuild test robot wrote: Hi Rui, I love your patch! Yet something to improve: [auto build test ERROR on v4.16-rc4] [cannot apply to next-20180314] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Rui-Miguel-Silva/media-Introduce-Omnivision-OV2680-driver/20180315-020617 config: sh-allmodconfig (attached as .config) compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=sh All errors (new ones prefixed by >>): drivers/media/i2c/ov2680.c: In function 'ov2680_set_fmt': drivers/media/i2c/ov2680.c:713:9: error: implicit declaration of function 'v4l2_find_nearest_size'; did you mean 'v4l2_find_nearest_format'? [-Werror=implicit-function-declaration] mode = v4l2_find_nearest_size(ov2680_mode_data, ^~ v4l2_find_nearest_format As requested by maintainer this series depend on this patch [0], which introduce this macro. I am not sure of the status of that patch though. --- Cheers, Rui [0] https://patchwork.kernel.org/patch/10207087/ drivers/media/i2c/ov2680.c:714:41: error: 'width' undeclared (first use in this function) ARRAY_SIZE(ov2680_mode_data), width, ^ drivers/media/i2c/ov2680.c:714:41: note: each undeclared identifier is reported only once for each function it appears in drivers/media/i2c/ov2680.c:715:11: error: 'height' undeclared (first use in this function); did you mean 'hweight8'? height, fmt->width, fmt->height); ^~ hweight8 cc1: some warnings being treated as errors vim +713 drivers/media/i2c/ov2680.c 693 694 static int ov2680_set_fmt(struct v4l2_subdev *sd, 695 struct v4l2_subdev_pad_config *cfg, 696 struct v4l2_subdev_format *format) 697 { 698 struct ov2680_dev *sensor = to_ov2680_dev(sd); 699 struct v4l2_mbus_framefmt *fmt = &format->format; 700 const struct ov2680_mode_info *mode; 701 int ret = 0; 702 703 if (format->pad != 0) 704 return -EINVAL; 705 706 mutex_lock(&sensor->lock); 707 708 if (sensor->is_streaming) { 709 ret = -EBUSY; 710 goto unlock; 711 } 712 > 713 mode = > v4l2_find_nearest_size(ov2680_mode_data, > 714 > ARRAY_SIZE(ov2680_mode_data), width, > 715 height, > fmt->width, fmt->height); 716 if (!mode) { 717 ret = -EINVAL; 718 goto unlock; 719 } 720 721 if (format->which == V4L2_SUBDEV_FORMAT_TRY) { 722 fmt = v4l2_subdev_get_try_format(sd, cfg, 0); 723 724 *fmt = format->format; 725 goto unlock; 726 } 727 728 fmt->width = mode->width; 729 fmt->height = mode->height; 730 fmt->code = sensor->fmt.code; 731 fmt->colorspace = sensor->fmt.colorspace; 732 733 sensor->current_mode = mode; 734 sensor->fmt = format->format; 735 sensor->mode_pending_changes = true; 736 737 unlock: 738 mutex_unlock(&sensor->lock); 739 740 return ret; 741 } 742 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: [PATCH v3 2/2] media: ov2680: Add Omnivision OV2680 sensor driver
Hi Rui, I love your patch! Yet something to improve: [auto build test ERROR on v4.16-rc4] [cannot apply to next-20180314] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Rui-Miguel-Silva/media-Introduce-Omnivision-OV2680-driver/20180315-020617 config: i386-allyesconfig (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): drivers/media/i2c/ov2680.c: In function 'ov2680_set_fmt': drivers/media/i2c/ov2680.c:713:9: error: implicit declaration of function 'v4l2_find_nearest_size'; did you mean 'v4l2_find_nearest_format'? [-Werror=implicit-function-declaration] mode = v4l2_find_nearest_size(ov2680_mode_data, ^~ v4l2_find_nearest_format drivers/media/i2c/ov2680.c:714:195: error: 'width' undeclared (first use in this function) ARRAY_SIZE(ov2680_mode_data), width, ^ drivers/media/i2c/ov2680.c:714:195: note: each undeclared identifier is reported only once for each function it appears in >> drivers/media/i2c/ov2680.c:715:11: error: 'height' undeclared (first use in >> this function); did you mean '__iget'? height, fmt->width, fmt->height); ^~ __iget cc1: some warnings being treated as errors vim +715 drivers/media/i2c/ov2680.c 693 694 static int ov2680_set_fmt(struct v4l2_subdev *sd, 695struct v4l2_subdev_pad_config *cfg, 696struct v4l2_subdev_format *format) 697 { 698 struct ov2680_dev *sensor = to_ov2680_dev(sd); 699 struct v4l2_mbus_framefmt *fmt = &format->format; 700 const struct ov2680_mode_info *mode; 701 int ret = 0; 702 703 if (format->pad != 0) 704 return -EINVAL; 705 706 mutex_lock(&sensor->lock); 707 708 if (sensor->is_streaming) { 709 ret = -EBUSY; 710 goto unlock; 711 } 712 > 713 mode = v4l2_find_nearest_size(ov2680_mode_data, 714ARRAY_SIZE(ov2680_mode_data), width, > 715height, fmt->width, fmt->height); 716 if (!mode) { 717 ret = -EINVAL; 718 goto unlock; 719 } 720 721 if (format->which == V4L2_SUBDEV_FORMAT_TRY) { 722 fmt = v4l2_subdev_get_try_format(sd, cfg, 0); 723 724 *fmt = format->format; 725 goto unlock; 726 } 727 728 fmt->width = mode->width; 729 fmt->height = mode->height; 730 fmt->code = sensor->fmt.code; 731 fmt->colorspace = sensor->fmt.colorspace; 732 733 sensor->current_mode = mode; 734 sensor->fmt = format->format; 735 sensor->mode_pending_changes = true; 736 737 unlock: 738 mutex_unlock(&sensor->lock); 739 740 return ret; 741 } 742 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH v3 2/2] media: ov2680: Add Omnivision OV2680 sensor driver
Hi Rui, I love your patch! Yet something to improve: [auto build test ERROR on v4.16-rc4] [cannot apply to next-20180314] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Rui-Miguel-Silva/media-Introduce-Omnivision-OV2680-driver/20180315-020617 config: sh-allmodconfig (attached as .config) compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=sh All errors (new ones prefixed by >>): drivers/media/i2c/ov2680.c: In function 'ov2680_set_fmt': >> drivers/media/i2c/ov2680.c:713:9: error: implicit declaration of function >> 'v4l2_find_nearest_size'; did you mean 'v4l2_find_nearest_format'? >> [-Werror=implicit-function-declaration] mode = v4l2_find_nearest_size(ov2680_mode_data, ^~ v4l2_find_nearest_format >> drivers/media/i2c/ov2680.c:714:41: error: 'width' undeclared (first use in >> this function) ARRAY_SIZE(ov2680_mode_data), width, ^ drivers/media/i2c/ov2680.c:714:41: note: each undeclared identifier is reported only once for each function it appears in >> drivers/media/i2c/ov2680.c:715:11: error: 'height' undeclared (first use in >> this function); did you mean 'hweight8'? height, fmt->width, fmt->height); ^~ hweight8 cc1: some warnings being treated as errors vim +713 drivers/media/i2c/ov2680.c 693 694 static int ov2680_set_fmt(struct v4l2_subdev *sd, 695struct v4l2_subdev_pad_config *cfg, 696struct v4l2_subdev_format *format) 697 { 698 struct ov2680_dev *sensor = to_ov2680_dev(sd); 699 struct v4l2_mbus_framefmt *fmt = &format->format; 700 const struct ov2680_mode_info *mode; 701 int ret = 0; 702 703 if (format->pad != 0) 704 return -EINVAL; 705 706 mutex_lock(&sensor->lock); 707 708 if (sensor->is_streaming) { 709 ret = -EBUSY; 710 goto unlock; 711 } 712 > 713 mode = v4l2_find_nearest_size(ov2680_mode_data, > 714ARRAY_SIZE(ov2680_mode_data), > width, > 715height, fmt->width, fmt->height); 716 if (!mode) { 717 ret = -EINVAL; 718 goto unlock; 719 } 720 721 if (format->which == V4L2_SUBDEV_FORMAT_TRY) { 722 fmt = v4l2_subdev_get_try_format(sd, cfg, 0); 723 724 *fmt = format->format; 725 goto unlock; 726 } 727 728 fmt->width = mode->width; 729 fmt->height = mode->height; 730 fmt->code = sensor->fmt.code; 731 fmt->colorspace = sensor->fmt.colorspace; 732 733 sensor->current_mode = mode; 734 sensor->fmt = format->format; 735 sensor->mode_pending_changes = true; 736 737 unlock: 738 mutex_unlock(&sensor->lock); 739 740 return ret; 741 } 742 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
[PATCH v3 2/2] media: ov2680: Add Omnivision OV2680 sensor driver
This patch adds V4L2 sub-device driver for OV2680 image sensor. The OV2680 is a 1/5" CMOS color sensor from Omnivision. Supports output format: 10-bit Raw RGB. The OV2680 has a single lane MIPI interface. The driver exposes following V4L2 controls: - auto/manual exposure, - exposure, - auto/manual gain, - gain, - horizontal/vertical flip, - test pattern menu. Supported resolution are only: QUXGA, 720P, UXGA. Signed-off-by: Rui Miguel Silva --- drivers/media/i2c/Kconfig | 12 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/ov2680.c | 1130 3 files changed, 1143 insertions(+) create mode 100644 drivers/media/i2c/ov2680.c diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index 9f18cd296841..39dc9f236ffa 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -586,6 +586,18 @@ config VIDEO_OV2659 To compile this driver as a module, choose M here: the module will be called ov2659. +config VIDEO_OV2680 + tristate "OmniVision OV2680 sensor support" + depends on GPIOLIB && VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API + depends on MEDIA_CAMERA_SUPPORT + select V4L2_FWNODE + ---help--- + This is a Video4Linux2 sensor-level driver for the OmniVision + OV2680 camera sensor with a MIPI CSI-2 interface. + + To compile this driver as a module, choose M here: the + module will be called ov2680. + config VIDEO_OV5640 tristate "OmniVision OV5640 sensor support" depends on OF diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index c0f94cd8d56d..d0aba4d37b8d 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -61,6 +61,7 @@ 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_OV2680) += ov2680.o obj-$(CONFIG_VIDEO_OV5640) += ov5640.o obj-$(CONFIG_VIDEO_OV5645) += ov5645.o obj-$(CONFIG_VIDEO_OV5647) += ov5647.o diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c new file mode 100644 index ..a17971ce71bc --- /dev/null +++ b/drivers/media/i2c/ov2680.c @@ -0,0 +1,1130 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Omnivision OV2680 CMOS Image Sensor driver + * + * Copyright (C) 2018 Linaro Ltd + * + * Based on OV5640 Sensor Driver + * Copyright (C) 2011-2013 Freescale Semiconductor, Inc. All Rights Reserved. + * Copyright (C) 2014-2017 Mentor Graphics Inc. + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +#define OV2680_XVCLK_MIN 600 +#define OV2680_XVCLK_MAX 2400 + +#define OV2680_CHIP_ID 0x2680 + +#define OV2680_REG_STREAM_CTRL 0x0100 +#define OV2680_REG_SOFT_RESET 0x0103 + +#define OV2680_REG_CHIP_ID_HIGH0x300a +#define OV2680_REG_CHIP_ID_LOW 0x300b + +#define OV2680_REG_R_MANUAL0x3503 +#define OV2680_REG_GAIN_PK 0x350a +#define OV2680_REG_EXPOSURE_PK_HIGH0x3500 +#define OV2680_REG_TIMING_HTS 0x380c +#define OV2680_REG_TIMING_VTS 0x380e +#define OV2680_REG_FORMAT1 0x3820 +#define OV2680_REG_FORMAT2 0x3821 + +#define OV2680_REG_ISP_CTRL00 0x5080 + +#define OV2680_FRAME_RATE 30 + +#define OV2680_REG_VALUE_8BIT 1 +#define OV2680_REG_VALUE_16BIT 2 +#define OV2680_REG_VALUE_24BIT 3 + +enum ov2680_mode_id { + OV2680_MODE_QUXGA_800_600, + OV2680_MODE_720P_1280_720, + OV2680_MODE_UXGA_1600_1200, + OV2680_MODE_MAX, +}; + +struct reg_value { + u16 reg_addr; + u8 val; +}; + +struct ov2680_mode_info { + const char *name; + enum ov2680_mode_id id; + u32 width; + u32 height; + const struct reg_value *reg_data; + u32 reg_data_size; +}; + +struct ov2680_ctrls { + struct v4l2_ctrl_handler handler; + struct { + struct v4l2_ctrl *auto_exp; + struct v4l2_ctrl *exposure; + }; + struct { + struct v4l2_ctrl *auto_gain; + struct v4l2_ctrl *gain; + }; + + struct v4l2_ctrl *hflip; + struct v4l2_ctrl *vflip; + struct v4l2_ctrl *test_pattern; +}; + +struct ov2680_dev { + struct i2c_client *i2c_client; + struct v4l2_subdev sd; + + struct media_padpad; + struct clk *xvclk; + u32 xvclk_freq; + + struct gpio_desc*pwdn_gpio; + struct mutexlock; /* protect members */ + + boolmode_pending_changes; + boolis_enabled; + boolis_stream