From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mga12.intel.com ([192.55.52.136]:64510 "EHLO mga12.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726436AbeIZOQa (ORCPT ); Wed, 26 Sep 2018 10:16:30 -0400 Subject: Re: [PATCH v7] media: add imx319 camera sensor driver To: Sakari Ailus , bingbu.cao@intel.com Cc: linux-media@vger.kernel.org, tfiga@google.com, rajmohan.mani@intel.com, tian.shu.qiu@intel.com, jian.xu.zheng@intel.com, chiranjeevi.rapolu@intel.com References: <1537929738-27745-1-git-send-email-bingbu.cao@intel.com> <20180926075716.zjyt6kn2hb6nx7mp@kekkonen.localdomain> From: Bing Bu Cao Message-ID: Date: Wed, 26 Sep 2018 16:08:50 +0800 MIME-Version: 1.0 In-Reply-To: <20180926075716.zjyt6kn2hb6nx7mp@kekkonen.localdomain> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Content-Language: en-US Sender: linux-media-owner@vger.kernel.org List-ID: On 09/26/2018 03:57 PM, Sakari Ailus wrote: > Hi Bingbu, > > On Wed, Sep 26, 2018 at 10:42:18AM +0800, bingbu.cao@intel.com wrote: >> From: Bingbu Cao >> >> Add a v4l2 sub-device driver for the Sony imx319 image sensor. >> This is a camera sensor using the i2c bus for control and the >> csi-2 bus for data. >> >> This driver supports following features: >> - manual exposure and analog/digital gain control support >> - vblank/hblank control support >> - 4 test patterns control support >> - vflip/hflip control support (will impact the output bayer order) >> - support following resolutions: >> - 3264x2448, 3280x2464 @ 30fps >> - 1936x1096, 1920x1080 @ 60fps >> - 1640x1232, 1640x922, 1296x736, 1280x720 @ 120fps >> - support 4 bayer orders output (via change v/hflip) >> - SRGGB10(default), SGRBG10, SGBRG10, SBGGR10 >> >> Cc: Tomasz Figa >> Cc: Sakari Ailus >> Signed-off-by: Bingbu Cao >> Signed-off-by: Tianshu Qiu >> >> --- >> >> This patch is based on sakari's media-tree git: >> https://git.linuxtv.org/sailus/media_tree.git/log/?h=for-4.20-1 >> >> Changes from v5: >> - add some comments for gain calculation >> - use lock to protect the format >> - fix some style issues > Thanks for the update! > > I've applied the patch with the following diff. Dividing a 64-bit number > generally requires do_div() which was missed in the review: Thanks! > > diff --git a/drivers/media/i2c/imx319.c b/drivers/media/i2c/imx319.c > index e10d60f500dd..37c31d17ecf0 100644 > --- a/drivers/media/i2c/imx319.c > +++ b/drivers/media/i2c/imx319.c > @@ -2038,7 +2038,7 @@ imx319_set_pad_format(struct v4l2_subdev *sd, > s32 vblank_def; > s32 vblank_min; > s64 h_blank; > - s64 pixel_rate; > + u64 pixel_rate; > u32 height; > > mutex_lock(&imx319->mutex); > @@ -2059,7 +2059,8 @@ imx319_set_pad_format(struct v4l2_subdev *sd, > *framefmt = fmt->format; > } else { > imx319->cur_mode = mode; > - pixel_rate = (imx319->link_def_freq * 2 * 4) / 10; > + pixel_rate = imx319->link_def_freq * 2 * 4; > + do_div(pixel_rate, 10); > __v4l2_ctrl_s_ctrl_int64(imx319->pixel_rate, pixel_rate); > /* Update limits and set FPS to default */ > height = imx319->cur_mode->height; > @@ -2268,7 +2269,7 @@ static int imx319_init_controls(struct imx319 *imx319) > s64 vblank_def; > s64 vblank_min; > s64 hblank; > - s64 pixel_rate; > + u64 pixel_rate; > const struct imx319_mode *mode; > u32 max; > int ret; > @@ -2287,7 +2288,8 @@ static int imx319_init_controls(struct imx319 *imx319) > imx319->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY; > > /* pixel_rate = link_freq * 2 * nr_of_lanes / bits_per_sample */ > - pixel_rate = (imx319->link_def_freq * 2 * 4) / 10; > + pixel_rate = imx319->link_def_freq * 2 * 4; > + do_div(pixel_rate, 10); > /* By default, PIXEL_RATE is read only */ > imx319->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, &imx319_ctrl_ops, > V4L2_CID_PIXEL_RATE, pixel_rate, >