Re: [PATCH v8] media: imx258: Add imx258 camera sensor driver
Hi Jason,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.16-rc5 next-20180316]
[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/Andy-Yeh/media-imx258-Add-imx258-camera-sensor-driver/20180316-201540
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/imx258.c: In function 'imx258_set_pad_format':
drivers/media/i2c/imx258.c:869: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(
^~
v4l2_find_nearest_format
drivers/media/i2c/imx258.c:870:200: error: 'width' undeclared (first use in
this function)
supported_modes, ARRAY_SIZE(supported_modes), width, height,
^
drivers/media/i2c/imx258.c:870:200: note: each undeclared identifier is
reported only once for each function it appears in
>> drivers/media/i2c/imx258.c:870:207: error: 'height' undeclared (first use in
>> this function); did you mean '__iget'?
supported_modes, ARRAY_SIZE(supported_modes), width, height,
^
__iget
drivers/media/i2c/imx258.c:871:3: error: 'fmr' undeclared (first use in this
function); did you mean 'fmt'?
fmr->format.width, fmt->format.height);
^~~
fmt
cc1: some warnings being treated as errors
vim +870 drivers/media/i2c/imx258.c
850
851 static int imx258_set_pad_format(struct v4l2_subdev *sd,
852 struct v4l2_subdev_pad_config *cfg,
853 struct v4l2_subdev_format *fmt)
854 {
855 struct imx258 *imx258 = to_imx258(sd);
856 const struct imx258_mode *mode;
857 struct v4l2_mbus_framefmt *framefmt;
858 s32 vblank_def;
859 s32 vblank_min;
860 s64 h_blank;
861 s64 pixel_rate;
862 s64 link_freq;
863
864 mutex_lock(&imx258->mutex);
865
866 /* Only one raw bayer(GBRG) order is supported */
867 fmt->format.code = MEDIA_BUS_FMT_SGRBG10_1X10;
868
> 869 mode = v4l2_find_nearest_size(
> 870 supported_modes, ARRAY_SIZE(supported_modes), width,
> height,
871 fmr->format.width, fmt->format.height);
872 imx258_update_pad_format(mode, fmt);
873 if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
874 framefmt = v4l2_subdev_get_try_format(sd, cfg,
fmt->pad);
875 *framefmt = fmt->format;
876 } else {
877 imx258->cur_mode = mode;
878 __v4l2_ctrl_s_ctrl(imx258->link_freq,
mode->link_freq_index);
879
880 link_freq = link_freq_menu_items[mode->link_freq_index];
881 pixel_rate = link_freq_to_pixel_rate(link_freq);
882 __v4l2_ctrl_s_ctrl_int64(imx258->pixel_rate,
pixel_rate);
883 /* Update limits and set FPS to default */
884 vblank_def = imx258->cur_mode->vts_def -
885 imx258->cur_mode->height;
886 vblank_min = imx258->cur_mode->vts_min -
887 imx258->cur_mode->height;
888 __v4l2_ctrl_modify_range(
889 imx258->vblank, vblank_min,
890 IMX258_VTS_MAX - imx258->cur_mode->height, 1,
891 vblank_def);
892 __v4l2_ctrl_s_ctrl(imx258->vblank, vblank_def);
893 h_blank =
894
link_freq_configs[mode->link_freq_index].pixels_per_line
895 - imx258->cur_mode->width;
896 __v4l2_ctrl_modify_range(imx258->hblank, h_blank,
897 h_blank, 1, h_blank);
898 }
899
900 mutex_unlock(&imx258->mutex);
901
902 return 0;
903 }
RE: [PATCH v8] media: imx258: Add imx258 camera sensor driver
Hi Ikp,
Per sync with Sakari, Mauro'd handle the pull request for
"v4l2_find_nearest_size" patch (https://patchwork.kernel.org/patch/10207087/)
early next week.
Hence only fixed a typo in v9.1. https://patchwork.linuxtv.org/patch/47976/
Thanks a lot!
Regards, Andy
-Original Message-
From: lkp
Sent: Friday, March 16, 2018 10:14 PM
To: Yeh, Andy
Cc: kbuild-...@01.org; linux-media@vger.kernel.org; tf...@chromium.org;
sakari.ai...@linux.intel.com; Yeh, Andy ; Chen, JasonX Z
; Chiang, AlanX ; Lai, Jim
Subject: Re: [PATCH v8] media: imx258: Add imx258 camera sensor driver
Hi Jason,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.16-rc5 next-20180316] [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/Andy-Yeh/media-imx258-Add-imx258-camera-sensor-driver/20180316-201540
config: i386-allmodconfig (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/imx258.c: In function 'imx258_set_pad_format':
>> drivers/media/i2c/imx258.c:869: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(
^~
v4l2_find_nearest_format
>> drivers/media/i2c/imx258.c:870:49: error: 'width' undeclared (first
>> use in this function)
supported_modes, ARRAY_SIZE(supported_modes), width, height,
^
drivers/media/i2c/imx258.c:870:49: note: each undeclared identifier is
reported only once for each function it appears in
>> drivers/media/i2c/imx258.c:870:56: error: 'height' undeclared (first use in
>> this function); did you mean 'hweight8'?
supported_modes, ARRAY_SIZE(supported_modes), width, height,
^~
hweight8
>> drivers/media/i2c/imx258.c:871:3: error: 'fmr' undeclared (first use in this
>> function); did you mean 'fmt'?
fmr->format.width, fmt->format.height);
^~~
fmt
cc1: some warnings being treated as errors
vim +869 drivers/media/i2c/imx258.c
850
851 static int imx258_set_pad_format(struct v4l2_subdev *sd,
852 struct v4l2_subdev_pad_config *cfg,
853 struct v4l2_subdev_format *fmt)
854 {
855 struct imx258 *imx258 = to_imx258(sd);
856 const struct imx258_mode *mode;
857 struct v4l2_mbus_framefmt *framefmt;
858 s32 vblank_def;
859 s32 vblank_min;
860 s64 h_blank;
861 s64 pixel_rate;
862 s64 link_freq;
863
864 mutex_lock(&imx258->mutex);
865
866 /* Only one raw bayer(GBRG) order is supported */
867 fmt->format.code = MEDIA_BUS_FMT_SGRBG10_1X10;
868
> 869 mode = v4l2_find_nearest_size(
> 870 supported_modes, ARRAY_SIZE(supported_modes), width,
> height,
> 871 fmr->format.width, fmt->format.height);
872 imx258_update_pad_format(mode, fmt);
873 if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
874 framefmt = v4l2_subdev_get_try_format(sd, cfg,
fmt->pad);
875 *framefmt = fmt->format;
876 } else {
877 imx258->cur_mode = mode;
878 __v4l2_ctrl_s_ctrl(imx258->link_freq,
mode->link_freq_index);
879
880 link_freq = link_freq_menu_items[mode->link_freq_index];
881 pixel_rate = link_freq_to_pixel_rate(link_freq);
882 __v4l2_ctrl_s_ctrl_int64(imx258->pixel_rate,
pixel_rate);
883 /* Update limits and set FPS to default */
884 vblank_def = imx258->cur_mode->vts_def -
885 imx258->cur_mode->height;
886 vblank_min = imx258->cur_mode->vts_min -
887 imx258->cur_mode->height;
888 __v4l2_ctrl_modify_range(
889 imx258->vblank, vblank_min,
890 IMX258_VTS_MAX - imx258->cur_mode->height, 1,
891 vblank_def);
892 __v4l2_ctrl_s_ctrl(imx258->vblank, vblank_def);
893
Re: [PATCH v8] media: imx258: Add imx258 camera sensor driver
Hi Jason,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.16-rc5 next-20180316]
[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/Andy-Yeh/media-imx258-Add-imx258-camera-sensor-driver/20180316-201540
config: i386-allmodconfig (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/imx258.c: In function 'imx258_set_pad_format':
>> drivers/media/i2c/imx258.c:869: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(
^~
v4l2_find_nearest_format
>> drivers/media/i2c/imx258.c:870:49: error: 'width' undeclared (first use in
>> this function)
supported_modes, ARRAY_SIZE(supported_modes), width, height,
^
drivers/media/i2c/imx258.c:870:49: note: each undeclared identifier is
reported only once for each function it appears in
>> drivers/media/i2c/imx258.c:870:56: error: 'height' undeclared (first use in
>> this function); did you mean 'hweight8'?
supported_modes, ARRAY_SIZE(supported_modes), width, height,
^~
hweight8
>> drivers/media/i2c/imx258.c:871:3: error: 'fmr' undeclared (first use in this
>> function); did you mean 'fmt'?
fmr->format.width, fmt->format.height);
^~~
fmt
cc1: some warnings being treated as errors
vim +869 drivers/media/i2c/imx258.c
850
851 static int imx258_set_pad_format(struct v4l2_subdev *sd,
852 struct v4l2_subdev_pad_config *cfg,
853 struct v4l2_subdev_format *fmt)
854 {
855 struct imx258 *imx258 = to_imx258(sd);
856 const struct imx258_mode *mode;
857 struct v4l2_mbus_framefmt *framefmt;
858 s32 vblank_def;
859 s32 vblank_min;
860 s64 h_blank;
861 s64 pixel_rate;
862 s64 link_freq;
863
864 mutex_lock(&imx258->mutex);
865
866 /* Only one raw bayer(GBRG) order is supported */
867 fmt->format.code = MEDIA_BUS_FMT_SGRBG10_1X10;
868
> 869 mode = v4l2_find_nearest_size(
> 870 supported_modes, ARRAY_SIZE(supported_modes), width,
> height,
> 871 fmr->format.width, fmt->format.height);
872 imx258_update_pad_format(mode, fmt);
873 if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
874 framefmt = v4l2_subdev_get_try_format(sd, cfg,
fmt->pad);
875 *framefmt = fmt->format;
876 } else {
877 imx258->cur_mode = mode;
878 __v4l2_ctrl_s_ctrl(imx258->link_freq,
mode->link_freq_index);
879
880 link_freq = link_freq_menu_items[mode->link_freq_index];
881 pixel_rate = link_freq_to_pixel_rate(link_freq);
882 __v4l2_ctrl_s_ctrl_int64(imx258->pixel_rate,
pixel_rate);
883 /* Update limits and set FPS to default */
884 vblank_def = imx258->cur_mode->vts_def -
885 imx258->cur_mode->height;
886 vblank_min = imx258->cur_mode->vts_min -
887 imx258->cur_mode->height;
888 __v4l2_ctrl_modify_range(
889 imx258->vblank, vblank_min,
890 IMX258_VTS_MAX - imx258->cur_mode->height, 1,
891 vblank_def);
892 __v4l2_ctrl_s_ctrl(imx258->vblank, vblank_def);
893 h_blank =
894
link_freq_configs[mode->link_freq_index].pixels_per_line
895 - imx258->cur_mode->width;
896 __v4l2_ctrl_modify_range(imx258->hblank, h_blank,
897 h_blank, 1, h_blank);
898 }
899
900 mutex_unlock(&imx258->mutex);
901
902 return 0;
903 }
904
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
RE: [PATCH v8] media: imx258: Add imx258 camera sensor driver
Both comments are OKAY.
Thanks.
-Original Message-
From: Sakari Ailus [mailto:sakari.ai...@linux.intel.com]
Sent: Thursday, March 15, 2018 6:31 AM
To: Yeh, Andy
Cc: linux-media@vger.kernel.org; tf...@chromium.org; Chen, JasonX Z
; Chiang, AlanX ; Lai, Jim
Subject: Re: [PATCH v8] media: imx258: Add imx258 camera sensor driver
Hi Andy,
Thanks for the update. Two minor comments below.
On Thu, Mar 15, 2018 at 12:24:19AM +0800, Andy Yeh wrote:
...
> +static int imx258_set_ctrl(struct v4l2_ctrl *ctrl) {
> + struct imx258 *imx258 =
> + container_of(ctrl->handler, struct imx258, ctrl_handler);
> + struct i2c_client *client = v4l2_get_subdevdata(&imx258->sd);
> + int ret = 0;
> +
> + /*
> + * Applying V4L2 control value only happens
> + * when power is up for streaming
> + */
> + if (pm_runtime_get_if_in_use(&client->dev) == 0)
> + return 0;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_ANALOGUE_GAIN:
> + ret = imx258_write_reg(imx258, IMX258_REG_ANALOG_GAIN,
> + IMX258_REG_VALUE_16BIT,
> + ctrl->val);
> + break;
> + case V4L2_CID_EXPOSURE:
> + ret = imx258_write_reg(imx258, IMX258_REG_EXPOSURE,
> + IMX258_REG_VALUE_16BIT,
> + ctrl->val);
> + break;
> + case V4L2_CID_DIGITAL_GAIN:
> + ret = imx258_update_digital_gain(imx258, IMX258_REG_VALUE_16BIT,
> + ctrl->val);
> + break;
> + case V4L2_CID_VBLANK:
> + /*
> + * Auto Frame Length Line Control is enabled by default.
> + * Not need control Vblank Register.
> + */
> + break;
> + default:
> + dev_info(&client->dev,
> + "ctrl(id:0x%x,val:0x%x) is not handled\n",
> + ctrl->id, ctrl->val);
As this is an error, I'd set ret to e.g. -EINVAL here.
> + break;
> + }
> +
> + pm_runtime_put(&client->dev);
> +
> + return ret;
> +}
...
> +/* Initialize control handlers */
> +static int imx258_init_controls(struct imx258 *imx258) {
> + struct i2c_client *client = v4l2_get_subdevdata(&imx258->sd);
> + struct v4l2_ctrl_handler *ctrl_hdlr;
> + s64 exposure_max;
> + s64 vblank_def;
> + s64 vblank_min;
> + s64 pixel_rate_min;
> + s64 pixel_rate_max;
> + int ret;
> +
> + ctrl_hdlr = &imx258->ctrl_handler;
> + ret = v4l2_ctrl_handler_init(ctrl_hdlr, 8);
> + if (ret)
> + return ret;
> +
> + mutex_init(&imx258->mutex);
> + ctrl_hdlr->lock = &imx258->mutex;
> + imx258->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr,
> + &imx258_ctrl_ops,
> + V4L2_CID_LINK_FREQ,
> + ARRAY_SIZE(link_freq_menu_items) - 1,
> + 0,
> + link_freq_menu_items);
> +
> + if (imx258->link_freq)
> + imx258->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> +
> + pixel_rate_max = link_freq_to_pixel_rate(link_freq_menu_items[0]);
> + pixel_rate_min = link_freq_to_pixel_rate(link_freq_menu_items[1]);
> + /* By default, PIXEL_RATE is read only */
> + imx258->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, &imx258_ctrl_ops,
> + V4L2_CID_PIXEL_RATE,
> + pixel_rate_min, pixel_rate_max,
> + 1, pixel_rate_max);
> +
> +
> + vblank_def = imx258->cur_mode->vts_def - imx258->cur_mode->height;
> + vblank_min = imx258->cur_mode->vts_min - imx258->cur_mode->height;
> + imx258->vblank = v4l2_ctrl_new_std(
> + ctrl_hdlr, &imx258_ctrl_ops, V4L2_CID_VBLANK,
> + vblank_min,
> + IMX258_VTS_MAX - imx258->cur_mode->height, 1,
> + vblank_def);
> +
> + imx258->hblank = v4l2_ctrl_new_std(
> + ctrl_hdlr, &imx258_ctrl_ops, V4L2_CID_HBLANK,
> + IMX258_PPL_DEFAULT - imx258->cur_mode->width,
> + IMX258_PPL_DEFAULT - imx258->cur_mode->width,
> + 1,
> + IMX258_PPL_DEFAULT - imx258->cur_mode->width);
> +
>
Re: [PATCH v8] media: imx258: Add imx258 camera sensor driver
Hi Andy, On Wed, Mar 14, 2018 at 04:29:52PM +, Yeh, Andy wrote: > Still wrong line break... > Please check the list instead. Thanks. > https://patchwork.linuxtv.org/patch/47936/ The patch you sent using git send-email seems fine; I bet it's your e-mail client that does the rewrapping. -- Regards, Sakari Ailus e-mail: sakari.ai...@iki.fi
RE: [PATCH v8] media: imx258: Add imx258 camera sensor driver
Sure will do. Thanks Sakari.
-Original Message-
From: Sakari Ailus [mailto:sakari.ai...@linux.intel.com]
Sent: Thursday, March 15, 2018 6:31 AM
To: Yeh, Andy
Cc: linux-media@vger.kernel.org; tf...@chromium.org; Chen, JasonX Z
; Chiang, AlanX ; Lai, Jim
Subject: Re: [PATCH v8] media: imx258: Add imx258 camera sensor driver
Hi Andy,
Thanks for the update. Two minor comments below.
On Thu, Mar 15, 2018 at 12:24:19AM +0800, Andy Yeh wrote:
...
> +static int imx258_set_ctrl(struct v4l2_ctrl *ctrl) {
> + struct imx258 *imx258 =
> + container_of(ctrl->handler, struct imx258, ctrl_handler);
> + struct i2c_client *client = v4l2_get_subdevdata(&imx258->sd);
> + int ret = 0;
> +
> + /*
> + * Applying V4L2 control value only happens
> + * when power is up for streaming
> + */
> + if (pm_runtime_get_if_in_use(&client->dev) == 0)
> + return 0;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_ANALOGUE_GAIN:
> + ret = imx258_write_reg(imx258, IMX258_REG_ANALOG_GAIN,
> + IMX258_REG_VALUE_16BIT,
> + ctrl->val);
> + break;
> + case V4L2_CID_EXPOSURE:
> + ret = imx258_write_reg(imx258, IMX258_REG_EXPOSURE,
> + IMX258_REG_VALUE_16BIT,
> + ctrl->val);
> + break;
> + case V4L2_CID_DIGITAL_GAIN:
> + ret = imx258_update_digital_gain(imx258, IMX258_REG_VALUE_16BIT,
> + ctrl->val);
> + break;
> + case V4L2_CID_VBLANK:
> + /*
> + * Auto Frame Length Line Control is enabled by default.
> + * Not need control Vblank Register.
> + */
> + break;
> + default:
> + dev_info(&client->dev,
> + "ctrl(id:0x%x,val:0x%x) is not handled\n",
> + ctrl->id, ctrl->val);
As this is an error, I'd set ret to e.g. -EINVAL here.
> + break;
> + }
> +
> + pm_runtime_put(&client->dev);
> +
> + return ret;
> +}
...
> +/* Initialize control handlers */
> +static int imx258_init_controls(struct imx258 *imx258) {
> + struct i2c_client *client = v4l2_get_subdevdata(&imx258->sd);
> + struct v4l2_ctrl_handler *ctrl_hdlr;
> + s64 exposure_max;
> + s64 vblank_def;
> + s64 vblank_min;
> + s64 pixel_rate_min;
> + s64 pixel_rate_max;
> + int ret;
> +
> + ctrl_hdlr = &imx258->ctrl_handler;
> + ret = v4l2_ctrl_handler_init(ctrl_hdlr, 8);
> + if (ret)
> + return ret;
> +
> + mutex_init(&imx258->mutex);
> + ctrl_hdlr->lock = &imx258->mutex;
> + imx258->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr,
> + &imx258_ctrl_ops,
> + V4L2_CID_LINK_FREQ,
> + ARRAY_SIZE(link_freq_menu_items) - 1,
> + 0,
> + link_freq_menu_items);
> +
> + if (imx258->link_freq)
> + imx258->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> +
> + pixel_rate_max = link_freq_to_pixel_rate(link_freq_menu_items[0]);
> + pixel_rate_min = link_freq_to_pixel_rate(link_freq_menu_items[1]);
> + /* By default, PIXEL_RATE is read only */
> + imx258->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, &imx258_ctrl_ops,
> + V4L2_CID_PIXEL_RATE,
> + pixel_rate_min, pixel_rate_max,
> + 1, pixel_rate_max);
> +
> +
> + vblank_def = imx258->cur_mode->vts_def - imx258->cur_mode->height;
> + vblank_min = imx258->cur_mode->vts_min - imx258->cur_mode->height;
> + imx258->vblank = v4l2_ctrl_new_std(
> + ctrl_hdlr, &imx258_ctrl_ops, V4L2_CID_VBLANK,
> + vblank_min,
> + IMX258_VTS_MAX - imx258->cur_mode->height, 1,
> + vblank_def);
> +
> + imx258->hblank = v4l2_ctrl_new_std(
> + ctrl_hdlr, &imx258_ctrl_ops, V4L2_CID_HBLANK,
> + IMX258_PPL_DEFAULT - imx258->cur_mode->width,
> + IMX258_PPL_DEFAULT - imx258->cur_mode->width,
> + 1,
> + IMX258_PPL_DEFAULT - imx258->cur_mode->width);
> +
>
Re: [PATCH v8] media: imx258: Add imx258 camera sensor driver
Hi Andy,
Thanks for the update. Two minor comments below.
On Thu, Mar 15, 2018 at 12:24:19AM +0800, Andy Yeh wrote:
...
> +static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct imx258 *imx258 =
> + container_of(ctrl->handler, struct imx258, ctrl_handler);
> + struct i2c_client *client = v4l2_get_subdevdata(&imx258->sd);
> + int ret = 0;
> +
> + /*
> + * Applying V4L2 control value only happens
> + * when power is up for streaming
> + */
> + if (pm_runtime_get_if_in_use(&client->dev) == 0)
> + return 0;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_ANALOGUE_GAIN:
> + ret = imx258_write_reg(imx258, IMX258_REG_ANALOG_GAIN,
> + IMX258_REG_VALUE_16BIT,
> + ctrl->val);
> + break;
> + case V4L2_CID_EXPOSURE:
> + ret = imx258_write_reg(imx258, IMX258_REG_EXPOSURE,
> + IMX258_REG_VALUE_16BIT,
> + ctrl->val);
> + break;
> + case V4L2_CID_DIGITAL_GAIN:
> + ret = imx258_update_digital_gain(imx258, IMX258_REG_VALUE_16BIT,
> + ctrl->val);
> + break;
> + case V4L2_CID_VBLANK:
> + /*
> + * Auto Frame Length Line Control is enabled by default.
> + * Not need control Vblank Register.
> + */
> + break;
> + default:
> + dev_info(&client->dev,
> + "ctrl(id:0x%x,val:0x%x) is not handled\n",
> + ctrl->id, ctrl->val);
As this is an error, I'd set ret to e.g. -EINVAL here.
> + break;
> + }
> +
> + pm_runtime_put(&client->dev);
> +
> + return ret;
> +}
...
> +/* Initialize control handlers */
> +static int imx258_init_controls(struct imx258 *imx258)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&imx258->sd);
> + struct v4l2_ctrl_handler *ctrl_hdlr;
> + s64 exposure_max;
> + s64 vblank_def;
> + s64 vblank_min;
> + s64 pixel_rate_min;
> + s64 pixel_rate_max;
> + int ret;
> +
> + ctrl_hdlr = &imx258->ctrl_handler;
> + ret = v4l2_ctrl_handler_init(ctrl_hdlr, 8);
> + if (ret)
> + return ret;
> +
> + mutex_init(&imx258->mutex);
> + ctrl_hdlr->lock = &imx258->mutex;
> + imx258->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr,
> + &imx258_ctrl_ops,
> + V4L2_CID_LINK_FREQ,
> + ARRAY_SIZE(link_freq_menu_items) - 1,
> + 0,
> + link_freq_menu_items);
> +
> + if (imx258->link_freq)
> + imx258->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> +
> + pixel_rate_max = link_freq_to_pixel_rate(link_freq_menu_items[0]);
> + pixel_rate_min = link_freq_to_pixel_rate(link_freq_menu_items[1]);
> + /* By default, PIXEL_RATE is read only */
> + imx258->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, &imx258_ctrl_ops,
> + V4L2_CID_PIXEL_RATE,
> + pixel_rate_min, pixel_rate_max,
> + 1, pixel_rate_max);
> +
> +
> + vblank_def = imx258->cur_mode->vts_def - imx258->cur_mode->height;
> + vblank_min = imx258->cur_mode->vts_min - imx258->cur_mode->height;
> + imx258->vblank = v4l2_ctrl_new_std(
> + ctrl_hdlr, &imx258_ctrl_ops, V4L2_CID_VBLANK,
> + vblank_min,
> + IMX258_VTS_MAX - imx258->cur_mode->height, 1,
> + vblank_def);
> +
> + imx258->hblank = v4l2_ctrl_new_std(
> + ctrl_hdlr, &imx258_ctrl_ops, V4L2_CID_HBLANK,
> + IMX258_PPL_DEFAULT - imx258->cur_mode->width,
> + IMX258_PPL_DEFAULT - imx258->cur_mode->width,
> + 1,
> + IMX258_PPL_DEFAULT - imx258->cur_mode->width);
> +
> + if (!imx258->hblank) {
Could you align handling for NULL hblank control with NULL link_freq above?
> + ret = -EINVAL;
> + goto error;
> + }
> +
> + imx258->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> +
> + exposure_max = imx258->cur_mode->vts_def - 8;
> + imx258->exposure = v4l2_ctrl_new_std(
> + ctrl_hdlr, &imx258_ctrl_ops,
> + V4L2_CID_EXPOSURE, IMX258_EXPOSURE_MIN,
> + IMX258_EXPOSURE_MAX, IMX258_EXPOSURE_STEP,
> + IMX258_EXPOSURE_DEFAULT);
> +
> + v4l2_ctrl_new_std(ctrl_hdlr, &imx258_ctrl_ops, V4L2_CID_ANALOGUE_GAIN,
> + IMX258_ANA_GAIN_MIN, IMX258_ANA_GAIN_MAX,
> + IMX258_ANA_GAIN_STEP, IMX258_ANA_GAIN_DEFAULT);
> +
>
RE: [PATCH v8] media: imx258: Add imx258 camera sensor driver
Still wrong line break... Please check the list instead. Thanks. https://patchwork.linuxtv.org/patch/47936/ Regards, Andy -Original Message- From: Yeh, Andy Sent: Thursday, March 15, 2018 12:24 AM To: linux-media@vger.kernel.org; tf...@chromium.org Cc: sakari.ai...@linux.intel.com; Yeh, Andy ; Chen, JasonX Z ; Chiang, AlanX ; Lai, Jim Subject: [PATCH v8] media: imx258: Add imx258 camera sensor driver From: Jason Chen Add a V4L2 sub-device driver for the Sony IMX258 image sensor. This is a camera sensor using the I2C bus for control and the CSI-2 bus for data. Signed-off-by: Andy Yeh Signed-off-by: Alan Chiang --- since v2: -- Update the streaming function to remove SW_STANDBY in the beginning. -- Adjust the delay time from 1ms to 12ms before set stream-on register. since v3: -- fix the sd.entity to make code be compiled on the mainline kernel. since v4: -- Enabled AG, DG, and Exposure time control correctly. since v5: -- Sensor vendor provided a new setting to fix different CLK issue -- Add one more resolution for 1048x780, used for VGA streaming since v6: -- improved i2c read/write function to support writing 2 registers -- modified i2c reg read/write function with a more portable way -- utilized v4l2_find_nearest_size instead of the local find_best_fit function -- defined an enum for the link freq entries for explicit indexing since v7: -- Removed usleep due to sufficient delay implemented in coreboot -- Added handling for VBLANK control that auto frame-line-control is enabled MAINTAINERS|7 + drivers/media/i2c/Kconfig | 11 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/imx258.c | 1299 4 files changed, 1318 insertions(+) create mode 100644 drivers/media/i2c/imx258.c diff --git a/MAINTAINERS b/MAINTAINERS index a339bb5..9f75510 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12646,6 +12646,13 @@ S: Maintained F: drivers/ssb/ F: include/linux/ssb/ +SONY IMX258 SENSOR DRIVER +M: Sakari Ailus +L: linux-media@vger.kernel.org +T: git git://linuxtv.org/media_tree.git +S: Maintained +F: drivers/media/i2c/imx258.c + SONY IMX274 SENSOR DRIVER M: Leon Luo L: linux-media@vger.kernel.org diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index fd01842..bcd4bf1 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -565,6 +565,17 @@ config VIDEO_APTINA_PLL config VIDEO_SMIAPP_PLL tristate +config VIDEO_IMX258 + tristate "Sony IMX258 sensor support" + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API + depends on MEDIA_CAMERA_SUPPORT + ---help--- + This is a Video4Linux2 sensor-level driver for the Sony + IMX258 camera. + + To compile this driver as a module, choose M here: the + module will be called imx258. + config VIDEO_IMX274 tristate "Sony IMX274 sensor support" depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index 1b62639..4bf7d00 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -94,6 +94,7 @@ obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o obj-$(CONFIG_VIDEO_ML86V7667) += ml86v7667.o obj-$(CONFIG_VIDEO_OV2659) += ov2659.o obj-$(CONFIG_VIDEO_TC358743) += tc358743.o +obj-$(CONFIG_VIDEO_IMX258) += imx258.o obj-$(CONFIG_VIDEO_IMX274) += imx274.o obj-$(CONFIG_SDR_MAX2175) += max2175.o diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c new file mode 100644 index 000..b671ee5 --- /dev/null +++ b/drivers/media/i2c/imx258.c @@ -0,0 +1,1299 @@ +// Copyright (C) 2018 Intel Corporation // SPDX-License-Identifier: +GPL-2.0 + +#include +#include +#include +#include +#include +#include +#include +#include + +#define IMX258_REG_VALUE_08BIT 1 +#define IMX258_REG_VALUE_16BIT 2 + +#define IMX258_REG_MODE_SELECT 0x0100 +#define IMX258_MODE_STANDBY0x00 +#define IMX258_MODE_STREAMING 0x01 + +/* Chip ID */ +#define IMX258_REG_CHIP_ID 0x0016 +#define IMX258_CHIP_ID 0x0258 + +/* V_TIMING internal */ +#define IMX258_REG_VTS 0x0340 +#define IMX258_VTS_30FPS 0x0c98 +#define IMX258_VTS_30FPS_2K0x0638 +#define IMX258_VTS_30FPS_VGA 0x034c +#define IMX258_VTS_MAX 0x + +/*Frame Length Line*/ +#define IMX258_FLL_MIN 0x08a6 +#define IMX258_FLL_MAX 0x +#define IMX258_FLL_STEP1 +#define IMX258_FLL_DEFAULT 0x0c98 + +/* HBLANK control - read only */ +#define IMX258_PPL_DEFAULT 5352 + +/* Exposure control */ +#define IMX258_REG_EXPOSURE0x0202 +#define IMX258_EXPOSURE_MIN4 +#define IMX258_EXPOSURE_STEP 1 +#define IMX258_EXPOSURE_DEFAULT0x640 +#define IMX2
RE: [PATCH v8] media: imx258: Add imx258 camera sensor driver
Understood. Thank you Tomasz too. -Original Message- From: Tomasz Figa [mailto:tf...@chromium.org] Sent: Friday, March 9, 2018 6:48 PM To: Yeh, Andy Cc: Sakari Ailus ; Linux Media Mailing List ; Chen, JasonX Z ; Chiang, AlanX ; Lai, Jim Subject: Re: [PATCH v8] media: imx258: Add imx258 camera sensor driver On Fri, Mar 9, 2018 at 7:46 PM, Yeh, Andy wrote: > Hi Tomasz, > > My apology. Actually I obsoleted this [V8] > https://patchwork.linuxtv.org/patch/47768/ just after submitted. > Since I found few comments as what you pointed below were not addressed yet. > Didn’t expect you to check this. I shall send an email to notify you two the > obsolete state as well. > > We are working on addressing all those outstanding comments. Almost done. > Will do reply v6 with OKAY, and send to list a new v7 with all fixed. > Thank you. Looking forward to next version then! (By the way, I still think that replying to review comments, even with a simple "Okay" is a good practice. :)) Best regards, Tomasz > > Regards, Andy > > -Original Message- > From: Tomasz Figa [mailto:tf...@chromium.org] > Sent: Friday, March 9, 2018 6:20 PM > To: Sakari Ailus > Cc: Yeh, Andy ; Linux Media Mailing List > ; Chen, JasonX Z > ; Chiang, AlanX > Subject: Re: [PATCH v8] media: imx258: Add imx258 camera sensor driver > > Hi Andy, Sakari, > > On Fri, Mar 9, 2018 at 5:54 PM, Sakari Ailus > wrote: >> Hi Andy, >> >> Thanks for the update. Please see my comments below. >> >> On Fri, Mar 09, 2018 at 12:15:54AM +0800, Andy Yeh wrote: >>> Add a V4L2 sub-device driver for the Sony IMX258 image sensor. >>> This is a camera sensor using the I2C bus for control and the >>> CSI-2 bus for data. >>> >>> Signed-off-by: Jason Chen >>> Signed-off-by: Alan Chiang >>> --- >>> since v2: >>> -- Update the streaming function to remove SW_STANDBY in the beginning. >>> -- Adjust the delay time from 1ms to 12ms before set stream-on register. >>> since v3: >>> -- fix the sd.entity to make code be compiled on the mainline kernel. >>> since v4: >>> -- Enabled AG, DG, and Exposure time control correctly. >>> since v5: >>> -- Sensor vendor provided a new setting to fix different CLK issue >>> -- Add one more resolution for 1048x780, used for VGA streaming >>> since >>> v6: >>> -- improve i2c write function to support writing 2 registers >>> -- Not Orring ret in update_digital_gain which lead to unintended >>> error since v7: >>> -- modified imx258_reg read / write function with a more portable >>> way >>> -- arranged some format per suggestions >>> >>> >>> MAINTAINERS|7 + >>> drivers/media/i2c/Kconfig | 11 + >>> drivers/media/i2c/Makefile |1 + >>> drivers/media/i2c/imx258.c | 1324 >>> >>> 4 files changed, 1343 insertions(+) create mode 100644 >>> drivers/media/i2c/imx258.c >>> >>> diff --git a/MAINTAINERS b/MAINTAINERS index a339bb5..9f75510 100644 >>> --- a/MAINTAINERS >>> +++ b/MAINTAINERS >>> @@ -12646,6 +12646,13 @@ S: Maintained >>> F: drivers/ssb/ >>> F: include/linux/ssb/ >>> >>> +SONY IMX258 SENSOR DRIVER >>> +M: Sakari Ailus >>> +L: linux-media@vger.kernel.org >>> +T: git git://linuxtv.org/media_tree.git >>> +S: Maintained >>> +F: drivers/media/i2c/imx258.c >>> + >>> SONY IMX274 SENSOR DRIVER >>> M: Leon Luo >>> L: linux-media@vger.kernel.org >>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig >>> index fd01842..bcd4bf1 100644 >>> --- a/drivers/media/i2c/Kconfig >>> +++ b/drivers/media/i2c/Kconfig >>> @@ -565,6 +565,17 @@ config VIDEO_APTINA_PLL config VIDEO_SMIAPP_PLL >>> tristate >>> >>> +config VIDEO_IMX258 >>> + tristate "Sony IMX258 sensor support" >>> + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API >>> + depends on MEDIA_CAMERA_SUPPORT >>> + ---help--- >>> + This is a Video4Linux2 sensor-level driver for the Sony >>> + IMX258 camera. >>> + >>> + To compile this driver as a module, choose M here: the >>> + module will be called imx258. >>> + >>> config VIDEO_IMX274 >>> tristate "Sony IMX274 sensor support" >>> depends on I2C &a
Re: [PATCH v8] media: imx258: Add imx258 camera sensor driver
On Fri, Mar 9, 2018 at 7:46 PM, Yeh, Andy wrote: > Hi Tomasz, > > My apology. Actually I obsoleted this [V8] > https://patchwork.linuxtv.org/patch/47768/ just after submitted. > Since I found few comments as what you pointed below were not addressed yet. > Didn’t expect you to check this. I shall send an email to notify you two the > obsolete state as well. > > We are working on addressing all those outstanding comments. Almost done. > Will do reply v6 with OKAY, and send to list a new v7 with all fixed. > Thank you. Looking forward to next version then! (By the way, I still think that replying to review comments, even with a simple "Okay" is a good practice. :)) Best regards, Tomasz > > Regards, Andy > > -Original Message- > From: Tomasz Figa [mailto:tf...@chromium.org] > Sent: Friday, March 9, 2018 6:20 PM > To: Sakari Ailus > Cc: Yeh, Andy ; Linux Media Mailing List > ; Chen, JasonX Z ; > Chiang, AlanX > Subject: Re: [PATCH v8] media: imx258: Add imx258 camera sensor driver > > Hi Andy, Sakari, > > On Fri, Mar 9, 2018 at 5:54 PM, Sakari Ailus > wrote: >> Hi Andy, >> >> Thanks for the update. Please see my comments below. >> >> On Fri, Mar 09, 2018 at 12:15:54AM +0800, Andy Yeh wrote: >>> Add a V4L2 sub-device driver for the Sony IMX258 image sensor. >>> This is a camera sensor using the I2C bus for control and the >>> CSI-2 bus for data. >>> >>> Signed-off-by: Jason Chen >>> Signed-off-by: Alan Chiang >>> --- >>> since v2: >>> -- Update the streaming function to remove SW_STANDBY in the beginning. >>> -- Adjust the delay time from 1ms to 12ms before set stream-on register. >>> since v3: >>> -- fix the sd.entity to make code be compiled on the mainline kernel. >>> since v4: >>> -- Enabled AG, DG, and Exposure time control correctly. >>> since v5: >>> -- Sensor vendor provided a new setting to fix different CLK issue >>> -- Add one more resolution for 1048x780, used for VGA streaming since >>> v6: >>> -- improve i2c write function to support writing 2 registers >>> -- Not Orring ret in update_digital_gain which lead to unintended >>> error since v7: >>> -- modified imx258_reg read / write function with a more portable way >>> -- arranged some format per suggestions >>> >>> >>> MAINTAINERS|7 + >>> drivers/media/i2c/Kconfig | 11 + >>> drivers/media/i2c/Makefile |1 + >>> drivers/media/i2c/imx258.c | 1324 >>> >>> 4 files changed, 1343 insertions(+) >>> create mode 100644 drivers/media/i2c/imx258.c >>> >>> diff --git a/MAINTAINERS b/MAINTAINERS index a339bb5..9f75510 100644 >>> --- a/MAINTAINERS >>> +++ b/MAINTAINERS >>> @@ -12646,6 +12646,13 @@ S: Maintained >>> F: drivers/ssb/ >>> F: include/linux/ssb/ >>> >>> +SONY IMX258 SENSOR DRIVER >>> +M: Sakari Ailus >>> +L: linux-media@vger.kernel.org >>> +T: git git://linuxtv.org/media_tree.git >>> +S: Maintained >>> +F: drivers/media/i2c/imx258.c >>> + >>> SONY IMX274 SENSOR DRIVER >>> M: Leon Luo >>> L: linux-media@vger.kernel.org >>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig >>> index fd01842..bcd4bf1 100644 >>> --- a/drivers/media/i2c/Kconfig >>> +++ b/drivers/media/i2c/Kconfig >>> @@ -565,6 +565,17 @@ config VIDEO_APTINA_PLL config VIDEO_SMIAPP_PLL >>> tristate >>> >>> +config VIDEO_IMX258 >>> + tristate "Sony IMX258 sensor support" >>> + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API >>> + depends on MEDIA_CAMERA_SUPPORT >>> + ---help--- >>> + This is a Video4Linux2 sensor-level driver for the Sony >>> + IMX258 camera. >>> + >>> + To compile this driver as a module, choose M here: the >>> + module will be called imx258. >>> + >>> config VIDEO_IMX274 >>> tristate "Sony IMX274 sensor support" >>> depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API diff >>> --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index >>> 1b62639..4bf7d00 100644 >>> --- a/drivers/media/i2c/Makefile >>> +++ b/drivers/media/i2c/Makefile >>> @@ -94,6 +94,7 @@ obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o >&g
RE: [PATCH v8] media: imx258: Add imx258 camera sensor driver
Hi Tomasz, My apology. Actually I obsoleted this [V8] https://patchwork.linuxtv.org/patch/47768/ just after submitted. Since I found few comments as what you pointed below were not addressed yet. Didn’t expect you to check this. I shall send an email to notify you two the obsolete state as well. We are working on addressing all those outstanding comments. Almost done. Will do reply v6 with OKAY, and send to list a new v7 with all fixed. Regards, Andy -Original Message- From: Tomasz Figa [mailto:tf...@chromium.org] Sent: Friday, March 9, 2018 6:20 PM To: Sakari Ailus Cc: Yeh, Andy ; Linux Media Mailing List ; Chen, JasonX Z ; Chiang, AlanX Subject: Re: [PATCH v8] media: imx258: Add imx258 camera sensor driver Hi Andy, Sakari, On Fri, Mar 9, 2018 at 5:54 PM, Sakari Ailus wrote: > Hi Andy, > > Thanks for the update. Please see my comments below. > > On Fri, Mar 09, 2018 at 12:15:54AM +0800, Andy Yeh wrote: >> Add a V4L2 sub-device driver for the Sony IMX258 image sensor. >> This is a camera sensor using the I2C bus for control and the >> CSI-2 bus for data. >> >> Signed-off-by: Jason Chen >> Signed-off-by: Alan Chiang >> --- >> since v2: >> -- Update the streaming function to remove SW_STANDBY in the beginning. >> -- Adjust the delay time from 1ms to 12ms before set stream-on register. >> since v3: >> -- fix the sd.entity to make code be compiled on the mainline kernel. >> since v4: >> -- Enabled AG, DG, and Exposure time control correctly. >> since v5: >> -- Sensor vendor provided a new setting to fix different CLK issue >> -- Add one more resolution for 1048x780, used for VGA streaming since >> v6: >> -- improve i2c write function to support writing 2 registers >> -- Not Orring ret in update_digital_gain which lead to unintended >> error since v7: >> -- modified imx258_reg read / write function with a more portable way >> -- arranged some format per suggestions >> >> >> MAINTAINERS|7 + >> drivers/media/i2c/Kconfig | 11 + >> drivers/media/i2c/Makefile |1 + >> drivers/media/i2c/imx258.c | 1324 >> >> 4 files changed, 1343 insertions(+) >> create mode 100644 drivers/media/i2c/imx258.c >> >> diff --git a/MAINTAINERS b/MAINTAINERS index a339bb5..9f75510 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -12646,6 +12646,13 @@ S: Maintained >> F: drivers/ssb/ >> F: include/linux/ssb/ >> >> +SONY IMX258 SENSOR DRIVER >> +M: Sakari Ailus >> +L: linux-media@vger.kernel.org >> +T: git git://linuxtv.org/media_tree.git >> +S: Maintained >> +F: drivers/media/i2c/imx258.c >> + >> SONY IMX274 SENSOR DRIVER >> M: Leon Luo >> L: linux-media@vger.kernel.org >> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig >> index fd01842..bcd4bf1 100644 >> --- a/drivers/media/i2c/Kconfig >> +++ b/drivers/media/i2c/Kconfig >> @@ -565,6 +565,17 @@ config VIDEO_APTINA_PLL config VIDEO_SMIAPP_PLL >> tristate >> >> +config VIDEO_IMX258 >> + tristate "Sony IMX258 sensor support" >> + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API >> + depends on MEDIA_CAMERA_SUPPORT >> + ---help--- >> + This is a Video4Linux2 sensor-level driver for the Sony >> + IMX258 camera. >> + >> + To compile this driver as a module, choose M here: the >> + module will be called imx258. >> + >> config VIDEO_IMX274 >> tristate "Sony IMX274 sensor support" >> depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API diff >> --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index >> 1b62639..4bf7d00 100644 >> --- a/drivers/media/i2c/Makefile >> +++ b/drivers/media/i2c/Makefile >> @@ -94,6 +94,7 @@ obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o >> obj-$(CONFIG_VIDEO_ML86V7667)+= ml86v7667.o >> obj-$(CONFIG_VIDEO_OV2659) += ov2659.o >> obj-$(CONFIG_VIDEO_TC358743) += tc358743.o >> +obj-$(CONFIG_VIDEO_IMX258) += imx258.o >> obj-$(CONFIG_VIDEO_IMX274) += imx274.o >> >> obj-$(CONFIG_SDR_MAX2175) += max2175.o diff --git >> a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c new file >> mode 100644 index 000..814520f >> --- /dev/null >> +++ b/drivers/media/i2c/imx258.c >> @@ -0,0 +1,1324 @@ >> +// Copyright (C) 2018 Intel Corporation // SPDX-License-Identifier: >> +GPL-2.0 >> + >> +#include >>
Re: [PATCH v8] media: imx258: Add imx258 camera sensor driver
Hi Andy, Sakari, On Fri, Mar 9, 2018 at 5:54 PM, Sakari Ailus wrote: > Hi Andy, > > Thanks for the update. Please see my comments below. > > On Fri, Mar 09, 2018 at 12:15:54AM +0800, Andy Yeh wrote: >> Add a V4L2 sub-device driver for the Sony IMX258 image sensor. >> This is a camera sensor using the I2C bus for control and the >> CSI-2 bus for data. >> >> Signed-off-by: Jason Chen >> Signed-off-by: Alan Chiang >> --- >> since v2: >> -- Update the streaming function to remove SW_STANDBY in the beginning. >> -- Adjust the delay time from 1ms to 12ms before set stream-on register. >> since v3: >> -- fix the sd.entity to make code be compiled on the mainline kernel. >> since v4: >> -- Enabled AG, DG, and Exposure time control correctly. >> since v5: >> -- Sensor vendor provided a new setting to fix different CLK issue >> -- Add one more resolution for 1048x780, used for VGA streaming >> since v6: >> -- improve i2c write function to support writing 2 registers >> -- Not Orring ret in update_digital_gain which lead to unintended error >> since v7: >> -- modified imx258_reg read / write function with a more portable way >> -- arranged some format per suggestions >> >> >> MAINTAINERS|7 + >> drivers/media/i2c/Kconfig | 11 + >> drivers/media/i2c/Makefile |1 + >> drivers/media/i2c/imx258.c | 1324 >> >> 4 files changed, 1343 insertions(+) >> create mode 100644 drivers/media/i2c/imx258.c >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index a339bb5..9f75510 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -12646,6 +12646,13 @@ S: Maintained >> F: drivers/ssb/ >> F: include/linux/ssb/ >> >> +SONY IMX258 SENSOR DRIVER >> +M: Sakari Ailus >> +L: linux-media@vger.kernel.org >> +T: git git://linuxtv.org/media_tree.git >> +S: Maintained >> +F: drivers/media/i2c/imx258.c >> + >> SONY IMX274 SENSOR DRIVER >> M: Leon Luo >> L: linux-media@vger.kernel.org >> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig >> index fd01842..bcd4bf1 100644 >> --- a/drivers/media/i2c/Kconfig >> +++ b/drivers/media/i2c/Kconfig >> @@ -565,6 +565,17 @@ config VIDEO_APTINA_PLL >> config VIDEO_SMIAPP_PLL >> tristate >> >> +config VIDEO_IMX258 >> + tristate "Sony IMX258 sensor support" >> + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API >> + depends on MEDIA_CAMERA_SUPPORT >> + ---help--- >> + This is a Video4Linux2 sensor-level driver for the Sony >> + IMX258 camera. >> + >> + To compile this driver as a module, choose M here: the >> + module will be called imx258. >> + >> config VIDEO_IMX274 >> tristate "Sony IMX274 sensor support" >> depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API >> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile >> index 1b62639..4bf7d00 100644 >> --- a/drivers/media/i2c/Makefile >> +++ b/drivers/media/i2c/Makefile >> @@ -94,6 +94,7 @@ obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o >> obj-$(CONFIG_VIDEO_ML86V7667)+= ml86v7667.o >> obj-$(CONFIG_VIDEO_OV2659) += ov2659.o >> obj-$(CONFIG_VIDEO_TC358743) += tc358743.o >> +obj-$(CONFIG_VIDEO_IMX258) += imx258.o >> obj-$(CONFIG_VIDEO_IMX274) += imx274.o >> >> obj-$(CONFIG_SDR_MAX2175) += max2175.o >> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c >> new file mode 100644 >> index 000..814520f >> --- /dev/null >> +++ b/drivers/media/i2c/imx258.c >> @@ -0,0 +1,1324 @@ >> +// Copyright (C) 2018 Intel Corporation >> +// SPDX-License-Identifier: GPL-2.0 >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#define IMX258_REG_VALUE_08BIT 1 >> +#define IMX258_REG_VALUE_16BIT 2 >> +#define IMX258_REG_VALUE_24BIT 3 > > The last one is not used. Perhaps because the sensor does not have any > 24-bit registers? :-) How about removing it? I pointed in my earlier review comments that we don't even need these macros anymore. They add as much value as defining ONE = 1 and TWO = 2. Andy, this is already a second re-spin of this patch, where my previous comments are left unaddressed. > >> + >> +#define IMX258_REG_MODE_SELECT 0x0100 >> +#define IMX258_MODE_STANDBY 0x00 >> +#define IMX258_MODE_STREAMING0x01 >> + >> +/* Chip ID */ >> +#define IMX258_REG_CHIP_ID 0x0016 >> +#define IMX258_CHIP_ID 0x0258 >> + >> +/* V_TIMING internal */ >> +#define IMX258_REG_VTS 0x0340 >> +#define IMX258_VTS_30FPS 0x0c98 >> +#define IMX258_VTS_MAX 0x >> + >> +/*Frame Length Line*/ >> +#define IMX258_FLL_MIN 0x08a6 >> +#define IMX258_FLL_MAX 0x >> +#define IMX258_FLL_STEP 1 >> +#define IMX258_FLL_DEFAULT 0x0c98 >> + >> +/* HBLANK control - re
Re: [PATCH v8] media: imx258: Add imx258 camera sensor driver
Hi Andy, Thanks for the update. Please see my comments below. On Fri, Mar 09, 2018 at 12:15:54AM +0800, Andy Yeh wrote: > Add a V4L2 sub-device driver for the Sony IMX258 image sensor. > This is a camera sensor using the I2C bus for control and the > CSI-2 bus for data. > > Signed-off-by: Jason Chen > Signed-off-by: Alan Chiang > --- > since v2: > -- Update the streaming function to remove SW_STANDBY in the beginning. > -- Adjust the delay time from 1ms to 12ms before set stream-on register. > since v3: > -- fix the sd.entity to make code be compiled on the mainline kernel. > since v4: > -- Enabled AG, DG, and Exposure time control correctly. > since v5: > -- Sensor vendor provided a new setting to fix different CLK issue > -- Add one more resolution for 1048x780, used for VGA streaming > since v6: > -- improve i2c write function to support writing 2 registers > -- Not Orring ret in update_digital_gain which lead to unintended error > since v7: > -- modified imx258_reg read / write function with a more portable way > -- arranged some format per suggestions > > > MAINTAINERS|7 + > drivers/media/i2c/Kconfig | 11 + > drivers/media/i2c/Makefile |1 + > drivers/media/i2c/imx258.c | 1324 > > 4 files changed, 1343 insertions(+) > create mode 100644 drivers/media/i2c/imx258.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index a339bb5..9f75510 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -12646,6 +12646,13 @@ S: Maintained > F: drivers/ssb/ > F: include/linux/ssb/ > > +SONY IMX258 SENSOR DRIVER > +M: Sakari Ailus > +L: linux-media@vger.kernel.org > +T: git git://linuxtv.org/media_tree.git > +S: Maintained > +F: drivers/media/i2c/imx258.c > + > SONY IMX274 SENSOR DRIVER > M: Leon Luo > L: linux-media@vger.kernel.org > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig > index fd01842..bcd4bf1 100644 > --- a/drivers/media/i2c/Kconfig > +++ b/drivers/media/i2c/Kconfig > @@ -565,6 +565,17 @@ config VIDEO_APTINA_PLL > config VIDEO_SMIAPP_PLL > tristate > > +config VIDEO_IMX258 > + tristate "Sony IMX258 sensor support" > + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API > + depends on MEDIA_CAMERA_SUPPORT > + ---help--- > + This is a Video4Linux2 sensor-level driver for the Sony > + IMX258 camera. > + > + To compile this driver as a module, choose M here: the > + module will be called imx258. > + > config VIDEO_IMX274 > tristate "Sony IMX274 sensor support" > depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile > index 1b62639..4bf7d00 100644 > --- a/drivers/media/i2c/Makefile > +++ b/drivers/media/i2c/Makefile > @@ -94,6 +94,7 @@ obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o > obj-$(CONFIG_VIDEO_ML86V7667)+= ml86v7667.o > obj-$(CONFIG_VIDEO_OV2659) += ov2659.o > obj-$(CONFIG_VIDEO_TC358743) += tc358743.o > +obj-$(CONFIG_VIDEO_IMX258) += imx258.o > obj-$(CONFIG_VIDEO_IMX274) += imx274.o > > obj-$(CONFIG_SDR_MAX2175) += max2175.o > diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c > new file mode 100644 > index 000..814520f > --- /dev/null > +++ b/drivers/media/i2c/imx258.c > @@ -0,0 +1,1324 @@ > +// Copyright (C) 2018 Intel Corporation > +// SPDX-License-Identifier: GPL-2.0 > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define IMX258_REG_VALUE_08BIT 1 > +#define IMX258_REG_VALUE_16BIT 2 > +#define IMX258_REG_VALUE_24BIT 3 The last one is not used. Perhaps because the sensor does not have any 24-bit registers? :-) How about removing it? > + > +#define IMX258_REG_MODE_SELECT 0x0100 > +#define IMX258_MODE_STANDBY 0x00 > +#define IMX258_MODE_STREAMING0x01 > + > +/* Chip ID */ > +#define IMX258_REG_CHIP_ID 0x0016 > +#define IMX258_CHIP_ID 0x0258 > + > +/* V_TIMING internal */ > +#define IMX258_REG_VTS 0x0340 > +#define IMX258_VTS_30FPS 0x0c98 > +#define IMX258_VTS_MAX 0x > + > +/*Frame Length Line*/ > +#define IMX258_FLL_MIN 0x08a6 > +#define IMX258_FLL_MAX 0x > +#define IMX258_FLL_STEP 1 > +#define IMX258_FLL_DEFAULT 0x0c98 > + > +/* HBLANK control - read only */ > +#define IMX258_PPL_DEFAULT 5352 > + > +/* Exposure control */ > +#define IMX258_REG_EXPOSURE 0x0202 > +#define IMX258_EXPOSURE_MIN 4 > +#define IMX258_EXPOSURE_STEP 1 > +#define IMX258_EXPOSURE_DEFAULT 0x640 > +#define IMX258_EXPOSURE_MAX 65535 > + > +/* Analog gain control */ > +#define IMX258_REG_ANALOG_GAIN 0x0204 > +#define IMX258_ANA_GAIN_MIN 0 > +
