From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dan Carpenter Subject: Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool Date: Fri, 14 Jul 2017 15:41:32 +0300 Message-ID: <20170714124132.u3fzpetbh3b7gj7f@mwanda> References: <20170714092540.1217397-1-arnd@arndb.de> <20170714093938.1469319-1-arnd@arndb.de> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <20170714093938.1469319-1-arnd@arndb.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Arnd Bergmann Cc: devel@driverdev.osuosl.org, linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org, Greg Kroah-Hartman , Daeseok Youn , Linus Torvalds , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, adi-buildroot-devel@lists.sourceforge.net, Hans Verkuil , linux-ide@vger.kernel.org, Guenter Roeck , Niklas =?iso-8859-1?Q?S=F6derlund?= , Tejun Heo , akpm@linux-foundation.org, Mauro Carvalho Chehab , Robert Jarzmik , linux-arm-kernel@lists.infradead.org, Alan Cox List-Id: linux-ide@vger.kernel.org T24gRnJpLCBKdWwgMTQsIDIwMTcgYXQgMTE6MzY6NTZBTSArMDIwMCwgQXJuZCBCZXJnbWFubiB3 cm90ZToKPiBAQCAtMTE1OCw3ICsxMTU4LDggQEAgc3RhdGljIHZvaWQgY2NkY19jb25maWd1cmUo c3RydWN0IGlzcF9jY2RjX2RldmljZSAqY2NkYykKPiAgCSAqLwo+ICAJZm10X3NyYy5wYWQgPSBw YWQtPmluZGV4Owo+ICAJZm10X3NyYy53aGljaCA9IFY0TDJfU1VCREVWX0ZPUk1BVF9BQ1RJVkU7 Cj4gLQlpZiAoIXY0bDJfc3ViZGV2X2NhbGwoc2Vuc29yLCBwYWQsIGdldF9mbXQsIE5VTEwsICZm bXRfc3JjKSkgewo+ICsJcmV0ID0gdjRsMl9zdWJkZXZfY2FsbChzZW5zb3IsIHBhZCwgZ2V0X2Zt dCwgTlVMTCwgJmZtdF9zcmMpOwo+ICsJaWYgKCFyZXQpIHsKPiAgCQlmbXRfaW5mbyA9IG9tYXAz aXNwX3ZpZGVvX2Zvcm1hdF9pbmZvKGZtdF9zcmMuZm9ybWF0LmNvZGUpOwo+ICAJCWRlcHRoX2lu ID0gZm10X2luZm8tPndpZHRoOwo+ICAJfQoKSXMgdGhlIG9yaWdpbmFsIGNvZGUgYnVnZ3k/Cgpt ZWRpYS9wbGF0Zm9ybS9vbWFwM2lzcC9pc3BjY2RjLmMKICAxMTU2ICAgICAgICAgIC8qIENvbXB1 dGUgdGhlIGxhbmUgc2hpZnRlciBzaGlmdCB2YWx1ZSBhbmQgZW5hYmxlIHRoZSBicmlkZ2Ugd2hl biB0aGUKICAxMTU3ICAgICAgICAgICAqIGlucHV0IGZvcm1hdCBpcyBhIG5vbi1CVC42NTYgWVVW IHZhcmlhbnQuCiAgMTE1OCAgICAgICAgICAgKi8KICAxMTU5ICAgICAgICAgIGZtdF9zcmMucGFk ID0gcGFkLT5pbmRleDsKICAxMTYwICAgICAgICAgIGZtdF9zcmMud2hpY2ggPSBWNEwyX1NVQkRF Vl9GT1JNQVRfQUNUSVZFOwogIDExNjEgICAgICAgICAgaWYgKCF2NGwyX3N1YmRldl9jYWxsKHNl bnNvciwgcGFkLCBnZXRfZm10LCBOVUxMLCAmZm10X3NyYykpIHsKICAxMTYyICAgICAgICAgICAg ICAgICAgZm10X2luZm8gPSBvbWFwM2lzcF92aWRlb19mb3JtYXRfaW5mbyhmbXRfc3JjLmZvcm1h dC5jb2RlKTsKICAxMTYzICAgICAgICAgICAgICAgICAgZGVwdGhfaW4gPSBmbXRfaW5mby0+d2lk dGg7CiAgMTE2NCAgICAgICAgICB9CgpJZiB2NGwyX3N1YmRldl9jYWxsKCkgdGhlbiBkZXB0aF9p biBpcyB6ZXJvLgoKICAxMTY1ICAKICAxMTY2ICAgICAgICAgIGZtdF9pbmZvID0gb21hcDNpc3Bf dmlkZW9fZm9ybWF0X2luZm8oZm9ybWF0LT5jb2RlKTsKICAxMTY3ICAgICAgICAgIGRlcHRoX291 dCA9IGZtdF9pbmZvLT53aWR0aDsKICAxMTY4ICAgICAgICAgIHNoaWZ0ID0gZGVwdGhfaW4gLSBk ZXB0aF9vdXQ7CiAgICAgICAgICAgICAgICBeXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eCgpI b3cgZG8gd2Uga25vdyB0aGF0IHRoaXMgc3VidHJhY3Rpb24gZG9lc24ndCBzZXQgInNoaWZ0IiB0 byBhIHZlcnkgaGlnaApwb3NpdGl2ZT8KCiAgMTE2OSAgCiAgMTE3MCAgICAgICAgICBpZiAoY2Nk Yy0+YnQ2NTYpCiAgMTE3MSAgICAgICAgICAgICAgICAgIGJyaWRnZSA9IElTUENUUkxfUEFSX0JS SURHRV9ESVNBQkxFOwogIDExNzIgICAgICAgICAgZWxzZSBpZiAoZm10X2luZm8tPmNvZGUgPT0g TUVESUFfQlVTX0ZNVF9ZVVlWOF8yWDgpCiAgMTE3MyAgICAgICAgICAgICAgICAgIGJyaWRnZSA9 IElTUENUUkxfUEFSX0JSSURHRV9MRU5ESUFOOwogIDExNzQgICAgICAgICAgZWxzZSBpZiAoZm10 X2luZm8tPmNvZGUgPT0gTUVESUFfQlVTX0ZNVF9VWVZZOF8yWDgpCiAgMTE3NSAgICAgICAgICAg ICAgICAgIGJyaWRnZSA9IElTUENUUkxfUEFSX0JSSURHRV9CRU5ESUFOOwogIDExNzYgICAgICAg ICAgZWxzZQogIDExNzcgICAgICAgICAgICAgICAgICBicmlkZ2UgPSBJU1BDVFJMX1BBUl9CUklE R0VfRElTQUJMRTsKICAxMTc4ICAKICAxMTc5ICAgICAgICAgIG9tYXAzaXNwX2NvbmZpZ3VyZV9i cmlkZ2UoaXNwLCBjY2RjLT5pbnB1dCwgcGFyY2ZnLCBzaGlmdCwgYnJpZGdlKTsKCnJlZ2FyZHMs CmRhbiBjYXJwZW50ZXIKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9w Lm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1k ZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753937AbdGNMnT (ORCPT ); Fri, 14 Jul 2017 08:43:19 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:26628 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753519AbdGNMnR (ORCPT ); Fri, 14 Jul 2017 08:43:17 -0400 Date: Fri, 14 Jul 2017 15:41:32 +0300 From: Dan Carpenter To: Arnd Bergmann Cc: linux-kernel@vger.kernel.org, Mauro Carvalho Chehab , Hans Verkuil , devel@driverdev.osuosl.org, Niklas =?iso-8859-1?Q?S=F6derlund?= , Greg Kroah-Hartman , Robert Jarzmik , adi-buildroot-devel@lists.sourceforge.net, dri-devel@lists.freedesktop.org, linux-renesas-soc@vger.kernel.org, linux-ide@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Tejun Heo , akpm@linux-foundation.org, Alan Cox , Linus Torvalds , Daeseok Youn , Guenter Roeck , linux-media@vger.kernel.org Subject: Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool Message-ID: <20170714124132.u3fzpetbh3b7gj7f@mwanda> References: <20170714092540.1217397-1-arnd@arndb.de> <20170714093938.1469319-1-arnd@arndb.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170714093938.1469319-1-arnd@arndb.de> User-Agent: NeoMutt/20170113 (1.7.2) X-Source-IP: userv0021.oracle.com [156.151.31.71] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jul 14, 2017 at 11:36:56AM +0200, Arnd Bergmann wrote: > @@ -1158,7 +1158,8 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc) > */ > fmt_src.pad = pad->index; > fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE; > - if (!v4l2_subdev_call(sensor, pad, get_fmt, NULL, &fmt_src)) { > + ret = v4l2_subdev_call(sensor, pad, get_fmt, NULL, &fmt_src); > + if (!ret) { > fmt_info = omap3isp_video_format_info(fmt_src.format.code); > depth_in = fmt_info->width; > } Is the original code buggy? media/platform/omap3isp/ispccdc.c 1156 /* Compute the lane shifter shift value and enable the bridge when the 1157 * input format is a non-BT.656 YUV variant. 1158 */ 1159 fmt_src.pad = pad->index; 1160 fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE; 1161 if (!v4l2_subdev_call(sensor, pad, get_fmt, NULL, &fmt_src)) { 1162 fmt_info = omap3isp_video_format_info(fmt_src.format.code); 1163 depth_in = fmt_info->width; 1164 } If v4l2_subdev_call() then depth_in is zero. 1165 1166 fmt_info = omap3isp_video_format_info(format->code); 1167 depth_out = fmt_info->width; 1168 shift = depth_in - depth_out; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ How do we know that this subtraction doesn't set "shift" to a very high positive? 1169 1170 if (ccdc->bt656) 1171 bridge = ISPCTRL_PAR_BRIDGE_DISABLE; 1172 else if (fmt_info->code == MEDIA_BUS_FMT_YUYV8_2X8) 1173 bridge = ISPCTRL_PAR_BRIDGE_LENDIAN; 1174 else if (fmt_info->code == MEDIA_BUS_FMT_UYVY8_2X8) 1175 bridge = ISPCTRL_PAR_BRIDGE_BENDIAN; 1176 else 1177 bridge = ISPCTRL_PAR_BRIDGE_DISABLE; 1178 1179 omap3isp_configure_bridge(isp, ccdc->input, parcfg, shift, bridge); regards, dan carpenter From mboxrd@z Thu Jan 1 00:00:00 1970 From: dan.carpenter@oracle.com (Dan Carpenter) Date: Fri, 14 Jul 2017 15:41:32 +0300 Subject: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool In-Reply-To: <20170714093938.1469319-1-arnd@arndb.de> References: <20170714092540.1217397-1-arnd@arndb.de> <20170714093938.1469319-1-arnd@arndb.de> Message-ID: <20170714124132.u3fzpetbh3b7gj7f@mwanda> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Fri, Jul 14, 2017 at 11:36:56AM +0200, Arnd Bergmann wrote: > @@ -1158,7 +1158,8 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc) > */ > fmt_src.pad = pad->index; > fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE; > - if (!v4l2_subdev_call(sensor, pad, get_fmt, NULL, &fmt_src)) { > + ret = v4l2_subdev_call(sensor, pad, get_fmt, NULL, &fmt_src); > + if (!ret) { > fmt_info = omap3isp_video_format_info(fmt_src.format.code); > depth_in = fmt_info->width; > } Is the original code buggy? media/platform/omap3isp/ispccdc.c 1156 /* Compute the lane shifter shift value and enable the bridge when the 1157 * input format is a non-BT.656 YUV variant. 1158 */ 1159 fmt_src.pad = pad->index; 1160 fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE; 1161 if (!v4l2_subdev_call(sensor, pad, get_fmt, NULL, &fmt_src)) { 1162 fmt_info = omap3isp_video_format_info(fmt_src.format.code); 1163 depth_in = fmt_info->width; 1164 } If v4l2_subdev_call() then depth_in is zero. 1165 1166 fmt_info = omap3isp_video_format_info(format->code); 1167 depth_out = fmt_info->width; 1168 shift = depth_in - depth_out; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ How do we know that this subtraction doesn't set "shift" to a very high positive? 1169 1170 if (ccdc->bt656) 1171 bridge = ISPCTRL_PAR_BRIDGE_DISABLE; 1172 else if (fmt_info->code == MEDIA_BUS_FMT_YUYV8_2X8) 1173 bridge = ISPCTRL_PAR_BRIDGE_LENDIAN; 1174 else if (fmt_info->code == MEDIA_BUS_FMT_UYVY8_2X8) 1175 bridge = ISPCTRL_PAR_BRIDGE_BENDIAN; 1176 else 1177 bridge = ISPCTRL_PAR_BRIDGE_DISABLE; 1178 1179 omap3isp_configure_bridge(isp, ccdc->input, parcfg, shift, bridge); regards, dan carpenter