Re: [PATCH] [media] v4l2-ioctl: Give more information when device_caps are missing

2015-06-05 Thread Hans Verkuil
On 06/02/2015 09:07 PM, Laura Abbott wrote:
> Currently, the warning for missing device_caps gives a backtrace like so:
> 
> [] dump_stack+0x45/0x57
> [] warn_slowpath_common+0x8a/0xc0
> [] warn_slowpath_null+0x1a/0x20
> [] v4l_querycap+0x43/0x80 [videodev]
> [] __video_do_ioctl+0x2a4/0x320 [videodev]
> [] ? do_last+0x195/0x1210
> [] video_usercopy+0x22e/0x5b0 [videodev]
> [] ? v4l_querycap+0x80/0x80 [videodev]
> [] video_ioctl2+0x15/0x20 [videodev]
> [] v4l2_ioctl+0x113/0x150 [videodev]
> [] do_vfs_ioctl+0x2f8/0x4f0
> [] ? __audit_syscall_entry+0xb4/0x110
> [] ? do_audit_syscall_entry+0x6c/0x70
> [] SyS_ioctl+0x81/0xa0
> [] ? __audit_syscall_exit+0x1f6/0x2a0
> [] system_call_fastpath+0x12/0x17
> 
> This indicates that device_caps are missing but doesn't give
> much of a clue which driver is actually at fault. Improve
> the warning output by showing the capabilities and which
> operations set the capabilities.
> 
> Signed-off-by: Laura Abbott 
> ---
>  drivers/media/v4l2-core/v4l2-ioctl.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
> b/drivers/media/v4l2-core/v4l2-ioctl.c
> index aa407cb..e509608 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1023,8 +1023,9 @@ static int v4l_querycap(const struct v4l2_ioctl_ops 
> *ops,
>* Drivers MUST fill in device_caps, so check for this and
>* warn if it was forgotten.
>*/
> - WARN_ON(!(cap->capabilities & V4L2_CAP_DEVICE_CAPS) ||
> - !cap->device_caps);
> + WARN(!(cap->capabilities & V4L2_CAP_DEVICE_CAPS) ||
> + !cap->device_caps, "Bad caps for ops %pS, %x %x",
> + ops, cap->capabilities, cap->device_caps);

Why not just print cap->driver? Much simpler and it always works!

Regards,

Hans

>   cap->device_caps |= V4L2_CAP_EXT_PIX_FORMAT;
>  
>   return ret;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] [media] v4l2-ioctl: Give more information when device_caps are missing

2015-06-02 Thread Laura Abbott
Currently, the warning for missing device_caps gives a backtrace like so:

[] dump_stack+0x45/0x57
[] warn_slowpath_common+0x8a/0xc0
[] warn_slowpath_null+0x1a/0x20
[] v4l_querycap+0x43/0x80 [videodev]
[] __video_do_ioctl+0x2a4/0x320 [videodev]
[] ? do_last+0x195/0x1210
[] video_usercopy+0x22e/0x5b0 [videodev]
[] ? v4l_querycap+0x80/0x80 [videodev]
[] video_ioctl2+0x15/0x20 [videodev]
[] v4l2_ioctl+0x113/0x150 [videodev]
[] do_vfs_ioctl+0x2f8/0x4f0
[] ? __audit_syscall_entry+0xb4/0x110
[] ? do_audit_syscall_entry+0x6c/0x70
[] SyS_ioctl+0x81/0xa0
[] ? __audit_syscall_exit+0x1f6/0x2a0
[] system_call_fastpath+0x12/0x17

This indicates that device_caps are missing but doesn't give
much of a clue which driver is actually at fault. Improve
the warning output by showing the capabilities and which
operations set the capabilities.

Signed-off-by: Laura Abbott 
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
b/drivers/media/v4l2-core/v4l2-ioctl.c
index aa407cb..e509608 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1023,8 +1023,9 @@ static int v4l_querycap(const struct v4l2_ioctl_ops *ops,
 * Drivers MUST fill in device_caps, so check for this and
 * warn if it was forgotten.
 */
-   WARN_ON(!(cap->capabilities & V4L2_CAP_DEVICE_CAPS) ||
-   !cap->device_caps);
+   WARN(!(cap->capabilities & V4L2_CAP_DEVICE_CAPS) ||
+   !cap->device_caps, "Bad caps for ops %pS, %x %x",
+   ops, cap->capabilities, cap->device_caps);
cap->device_caps |= V4L2_CAP_EXT_PIX_FORMAT;
 
return ret;
-- 
2.4.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html