* [bug report] media: i2c: Add driver for Aptina MT9V111
@ 2018-07-31 18:35 Dan Carpenter
2018-08-03 8:58 ` jacopo mondi
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2018-07-31 18:35 UTC (permalink / raw)
To: jacopo+renesas; +Cc: linux-media
Hello Jacopo Mondi,
The patch aab7ed1c3927: "media: i2c: Add driver for Aptina MT9V111"
from Jul 25, 2018, leads to the following static checker warning:
drivers/media/i2c/mt9v111.c:1163 mt9v111_probe() warn: passing zero to 'PTR_ERR'
drivers/media/i2c/mt9v111.c:1173 mt9v111_probe() warn: passing zero to 'PTR_ERR'
drivers/media/i2c/mt9v111.c:1184 mt9v111_probe() warn: passing zero to 'PTR_ERR'
drivers/media/i2c/mt9v111.c:1194 mt9v111_probe() warn: passing zero to 'PTR_ERR'
drivers/media/i2c/mt9v111.c
1155 v4l2_ctrl_handler_init(&mt9v111->ctrls, 5);
1156
1157 mt9v111->auto_awb = v4l2_ctrl_new_std(&mt9v111->ctrls,
1158 &mt9v111_ctrl_ops,
1159 V4L2_CID_AUTO_WHITE_BALANCE,
1160 0, 1, 1,
1161 V4L2_WHITE_BALANCE_AUTO);
1162 if (IS_ERR_OR_NULL(mt9v111->auto_awb)) {
1163 ret = PTR_ERR(mt9v111->auto_awb);
This just returns success because v4l2_ctrl_new_std() only return NULL
on error, it never returns error pointers. I guess we should set ret to
EINVAL?
if (!mt9v111->auto_awb) {
ret = -EINVAL;
goto error_free_ctrls;
}
1164 goto error_free_ctrls;
1165 }
1166
1167 mt9v111->auto_exp = v4l2_ctrl_new_std_menu(&mt9v111->ctrls,
1168 &mt9v111_ctrl_ops,
1169 V4L2_CID_EXPOSURE_AUTO,
1170 V4L2_EXPOSURE_MANUAL,
1171 0, V4L2_EXPOSURE_AUTO);
1172 if (IS_ERR_OR_NULL(mt9v111->auto_exp)) {
1173 ret = PTR_ERR(mt9v111->auto_exp);
1174 goto error_free_ctrls;
1175 }
1176
1177 /* Initialize timings */
1178 mt9v111->hblank = v4l2_ctrl_new_std(&mt9v111->ctrls, &mt9v111_ctrl_ops,
1179 V4L2_CID_HBLANK,
1180 MT9V111_CORE_R05_MIN_HBLANK,
1181 MT9V111_CORE_R05_MAX_HBLANK, 1,
1182 MT9V111_CORE_R05_DEF_HBLANK);
1183 if (IS_ERR_OR_NULL(mt9v111->hblank)) {
1184 ret = PTR_ERR(mt9v111->hblank);
1185 goto error_free_ctrls;
1186 }
1187
1188 mt9v111->vblank = v4l2_ctrl_new_std(&mt9v111->ctrls, &mt9v111_ctrl_ops,
1189 V4L2_CID_VBLANK,
1190 MT9V111_CORE_R06_MIN_VBLANK,
1191 MT9V111_CORE_R06_MAX_VBLANK, 1,
1192 MT9V111_CORE_R06_DEF_VBLANK);
1193 if (IS_ERR_OR_NULL(mt9v111->vblank)) {
1194 ret = PTR_ERR(mt9v111->vblank);
1195 goto error_free_ctrls;
1196 }
1197
1198 /* PIXEL_RATE is fixed: just expose it to user space. */
1199 v4l2_ctrl_new_std(&mt9v111->ctrls, &mt9v111_ctrl_ops,
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] media: i2c: Add driver for Aptina MT9V111
2018-07-31 18:35 [bug report] media: i2c: Add driver for Aptina MT9V111 Dan Carpenter
@ 2018-08-03 8:58 ` jacopo mondi
0 siblings, 0 replies; 2+ messages in thread
From: jacopo mondi @ 2018-08-03 8:58 UTC (permalink / raw)
To: Dan Carpenter; +Cc: jacopo+renesas, linux-media
[-- Attachment #1: Type: text/plain, Size: 3835 bytes --]
Hi Dan,
thanks for noticing this,
On Tue, Jul 31, 2018 at 09:35:54PM +0300, Dan Carpenter wrote:
> Hello Jacopo Mondi,
>
> The patch aab7ed1c3927: "media: i2c: Add driver for Aptina MT9V111"
> from Jul 25, 2018, leads to the following static checker warning:
>
> drivers/media/i2c/mt9v111.c:1163 mt9v111_probe() warn: passing zero to 'PTR_ERR'
> drivers/media/i2c/mt9v111.c:1173 mt9v111_probe() warn: passing zero to 'PTR_ERR'
> drivers/media/i2c/mt9v111.c:1184 mt9v111_probe() warn: passing zero to 'PTR_ERR'
> drivers/media/i2c/mt9v111.c:1194 mt9v111_probe() warn: passing zero to 'PTR_ERR'
>
> drivers/media/i2c/mt9v111.c
> 1155 v4l2_ctrl_handler_init(&mt9v111->ctrls, 5);
> 1156
> 1157 mt9v111->auto_awb = v4l2_ctrl_new_std(&mt9v111->ctrls,
> 1158 &mt9v111_ctrl_ops,
> 1159 V4L2_CID_AUTO_WHITE_BALANCE,
> 1160 0, 1, 1,
> 1161 V4L2_WHITE_BALANCE_AUTO);
> 1162 if (IS_ERR_OR_NULL(mt9v111->auto_awb)) {
> 1163 ret = PTR_ERR(mt9v111->auto_awb);
>
> This just returns success because v4l2_ctrl_new_std() only return NULL
Correct, sorry, I didn't notice that.
> on error, it never returns error pointers. I guess we should set ret to
> EINVAL?
>
> if (!mt9v111->auto_awb) {
> ret = -EINVAL;
> goto error_free_ctrls;
> }
>
We can do even better than that.
The v4l2 control handler retains errors in a flag I can inspect after
having added/created all controls here and here below.
I can return that error flag if something goes wrong.
Thanks
j
> 1164 goto error_free_ctrls;
> 1165 }
> 1166
> 1167 mt9v111->auto_exp = v4l2_ctrl_new_std_menu(&mt9v111->ctrls,
> 1168 &mt9v111_ctrl_ops,
> 1169 V4L2_CID_EXPOSURE_AUTO,
> 1170 V4L2_EXPOSURE_MANUAL,
> 1171 0, V4L2_EXPOSURE_AUTO);
> 1172 if (IS_ERR_OR_NULL(mt9v111->auto_exp)) {
> 1173 ret = PTR_ERR(mt9v111->auto_exp);
> 1174 goto error_free_ctrls;
> 1175 }
> 1176
> 1177 /* Initialize timings */
> 1178 mt9v111->hblank = v4l2_ctrl_new_std(&mt9v111->ctrls, &mt9v111_ctrl_ops,
> 1179 V4L2_CID_HBLANK,
> 1180 MT9V111_CORE_R05_MIN_HBLANK,
> 1181 MT9V111_CORE_R05_MAX_HBLANK, 1,
> 1182 MT9V111_CORE_R05_DEF_HBLANK);
> 1183 if (IS_ERR_OR_NULL(mt9v111->hblank)) {
> 1184 ret = PTR_ERR(mt9v111->hblank);
> 1185 goto error_free_ctrls;
> 1186 }
> 1187
> 1188 mt9v111->vblank = v4l2_ctrl_new_std(&mt9v111->ctrls, &mt9v111_ctrl_ops,
> 1189 V4L2_CID_VBLANK,
> 1190 MT9V111_CORE_R06_MIN_VBLANK,
> 1191 MT9V111_CORE_R06_MAX_VBLANK, 1,
> 1192 MT9V111_CORE_R06_DEF_VBLANK);
> 1193 if (IS_ERR_OR_NULL(mt9v111->vblank)) {
> 1194 ret = PTR_ERR(mt9v111->vblank);
> 1195 goto error_free_ctrls;
> 1196 }
> 1197
> 1198 /* PIXEL_RATE is fixed: just expose it to user space. */
> 1199 v4l2_ctrl_new_std(&mt9v111->ctrls, &mt9v111_ctrl_ops,
>
> regards,
> dan carpenter
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-08-03 10:54 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-31 18:35 [bug report] media: i2c: Add driver for Aptina MT9V111 Dan Carpenter
2018-08-03 8:58 ` jacopo mondi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox