Hi Hans,

Thanks for the patch!

On Fri, May 01, 2015 at 01:33:48PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verk...@cisco.com>
> 
> While normal video/radio/vbi/swradio nodes have a proper QUERYCAP ioctl
> that apps can call to determine that it is indeed a V4L2 device, there
> is currently no equivalent for v4l-subdev nodes. Adding this ioctl will
> solve that, and it will allow utilities like v4l2-compliance to be used
> with these devices as well.
> 
> Signed-off-by: Hans Verkuil <hans.verk...@cisco.com>
> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 19 +++++++++++++++++++
>  include/uapi/linux/v4l2-subdev.h      | 12 ++++++++++++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c 
> b/drivers/media/v4l2-core/v4l2-subdev.c
> index 6359606..2ab1f7d 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -25,6 +25,7 @@
>  #include <linux/types.h>
>  #include <linux/videodev2.h>
>  #include <linux/export.h>
> +#include <linux/version.h>
>  
>  #include <media/v4l2-ctrls.h>
>  #include <media/v4l2-device.h>
> @@ -187,6 +188,24 @@ static long subdev_do_ioctl(struct file *file, unsigned 
> int cmd, void *arg)
>  #endif
>  
>       switch (cmd) {
> +     case VIDIOC_SUBDEV_QUERYCAP: {
> +             struct v4l2_subdev_capability *cap = arg;
> +
> +             cap->version = LINUX_VERSION_CODE;
> +             cap->device_caps = 0;
> +             cap->pads = 0;
> +             cap->entity_id = 0;
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +             if (sd->entity.parent) {
> +                     cap->device_caps = V4L2_SUBDEV_CAP_ENTITY;
> +                     cap->pads = sd->entity.num_pads;
> +                     cap->entity_id = sd->entity.id;
> +             }
> +#endif
> +             memset(cap->reserved, 0, sizeof(cap->reserved));
> +             break;
> +     }
> +
>       case VIDIOC_QUERYCTRL:
>               return v4l2_queryctrl(vfh->ctrl_handler, arg);
>  
> diff --git a/include/uapi/linux/v4l2-subdev.h 
> b/include/uapi/linux/v4l2-subdev.h
> index dbce2b5..e48b9fd 100644
> --- a/include/uapi/linux/v4l2-subdev.h
> +++ b/include/uapi/linux/v4l2-subdev.h
> @@ -154,9 +154,21 @@ struct v4l2_subdev_selection {
>       __u32 reserved[8];
>  };
>  
> +struct v4l2_subdev_capability {
> +     __u32 version;
> +     __u32 device_caps;

This is called capabilities in struct v4l2_capability. I'd follow the same
pattern.

> +     __u32 pads;
> +     __u32 entity_id;

What's the use case for the entity_id field btw.? Supposing that the user
wouldn't be using the MC interface to obtain it, is the entity_id relevant
in this context? Or is your intent first open the sub-device, and then find
out more information on the entity?

> +     __u32 reserved[48];

Why 48?

As memory is typically allocated in powers of two (or n^2 + (n-1)^2), how
about aligning it accordingly? I don't think we lose anything by making this
e.g. 60. Although 28 would probably suffice as well (or 29 with the pads
field removed as discussed). Even that much sounds like a lot.

> +};
> +
> +/* This v4l2_subdev is also a media entity and the entity_id field is valid 
> */
> +#define V4L2_SUBDEV_CAP_ENTITY               (1 << 0)
> +
>  /* Backwards compatibility define --- to be removed */
>  #define v4l2_subdev_edid v4l2_edid
>  
> +#define VIDIOC_SUBDEV_QUERYCAP                        _IOR('V',  0, struct 
> v4l2_subdev_capability)
>  #define VIDIOC_SUBDEV_G_FMT                  _IOWR('V',  4, struct 
> v4l2_subdev_format)
>  #define VIDIOC_SUBDEV_S_FMT                  _IOWR('V',  5, struct 
> v4l2_subdev_format)
>  #define VIDIOC_SUBDEV_G_FRAME_INTERVAL               _IOWR('V', 21, struct 
> v4l2_subdev_frame_interval)

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi     XMPP: sai...@retiisi.org.uk
--
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

Reply via email to