Re: [PATCH 05/24] media: v4l2-dev: convert VFL_TYPE_* into an enum

2017-12-18 Thread Mauro Carvalho Chehab 
Em Tue, 10 Oct 2017 21:47:04 +0100
Andrey Utkin  escreveu:

> On Mon, Oct 09, 2017 at 07:19:11AM -0300, Mauro Carvalho Chehab wrote:
> > Using enums makes easier to document, as it can use kernel-doc
> > markups. It also allows cross-referencing, with increases the
> > kAPI readability.
> >   
> 
> 
> All changes look legit.
> 
> But I'd expect cx88_querycap() return type change and such to be in
> separate commit.

It should be together, as the switch() now would generate a warning,
because some enum values aren't listed. On such case, it has to
return an error.

I added an explanation at the commit message.

> 
> > diff --git a/drivers/media/pci/cx88/cx88-blackbird.c 
> > b/drivers/media/pci/cx88/cx88-blackbird.c
> > index e3101f04941c..0e0952e60795 100644
> > --- a/drivers/media/pci/cx88/cx88-blackbird.c
> > +++ b/drivers/media/pci/cx88/cx88-blackbird.c
> > @@ -805,8 +805,7 @@ static int vidioc_querycap(struct file *file, void  
> > *priv,
> >  
> > strcpy(cap->driver, "cx88_blackbird");
> > sprintf(cap->bus_info, "PCI:%s", pci_name(dev->pci));
> > -   cx88_querycap(file, core, cap);
> > -   return 0;
> > +   return cx88_querycap(file, core, cap);
> >  }
> >  
> >  static int vidioc_enum_fmt_vid_cap(struct file *file, void  *priv,
> > diff --git a/drivers/media/pci/cx88/cx88-video.c 
> > b/drivers/media/pci/cx88/cx88-video.c
> > index 7d25ecd4404b..9be682cdb644 100644
> > --- a/drivers/media/pci/cx88/cx88-video.c
> > +++ b/drivers/media/pci/cx88/cx88-video.c
> > @@ -806,8 +806,8 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void 
> > *priv,
> > return 0;
> >  }
> >  
> > -void cx88_querycap(struct file *file, struct cx88_core *core,
> > -  struct v4l2_capability *cap)
> > +int cx88_querycap(struct file *file, struct cx88_core *core,
> > + struct v4l2_capability *cap)
> >  {
> > struct video_device *vdev = video_devdata(file);
> >



Thanks,
Mauro

Re: [PATCH 05/24] media: v4l2-dev: convert VFL_TYPE_* into an enum

2017-10-10 Thread Andrey Utkin 
On Mon, Oct 09, 2017 at 07:19:11AM -0300, Mauro Carvalho Chehab wrote:
> Using enums makes easier to document, as it can use kernel-doc
> markups. It also allows cross-referencing, with increases the
> kAPI readability.
> 


All changes look legit.

But I'd expect cx88_querycap() return type change and such to be in
separate commit.

> diff --git a/drivers/media/pci/cx88/cx88-blackbird.c 
> b/drivers/media/pci/cx88/cx88-blackbird.c
> index e3101f04941c..0e0952e60795 100644
> --- a/drivers/media/pci/cx88/cx88-blackbird.c
> +++ b/drivers/media/pci/cx88/cx88-blackbird.c
> @@ -805,8 +805,7 @@ static int vidioc_querycap(struct file *file, void  *priv,
>  
>   strcpy(cap->driver, "cx88_blackbird");
>   sprintf(cap->bus_info, "PCI:%s", pci_name(dev->pci));
> - cx88_querycap(file, core, cap);
> - return 0;
> + return cx88_querycap(file, core, cap);
>  }
>  
>  static int vidioc_enum_fmt_vid_cap(struct file *file, void  *priv,
> diff --git a/drivers/media/pci/cx88/cx88-video.c 
> b/drivers/media/pci/cx88/cx88-video.c
> index 7d25ecd4404b..9be682cdb644 100644
> --- a/drivers/media/pci/cx88/cx88-video.c
> +++ b/drivers/media/pci/cx88/cx88-video.c
> @@ -806,8 +806,8 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void 
> *priv,
>   return 0;
>  }
>  
> -void cx88_querycap(struct file *file, struct cx88_core *core,
> -struct v4l2_capability *cap)
> +int cx88_querycap(struct file *file, struct cx88_core *core,
> +   struct v4l2_capability *cap)
>  {
>   struct video_device *vdev = video_devdata(file);
>

Re: [PATCH 05/24] media: v4l2-dev: convert VFL_TYPE_* into an enum

2017-10-09 Thread Mike Isely 

Acked-By: Mike Isely 

On Mon, 9 Oct 2017, Mauro Carvalho Chehab wrote:

> Using enums makes easier to document, as it can use kernel-doc
> markups. It also allows cross-referencing, with increases the
> kAPI readability.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  Documentation/media/kapi/v4l2-dev.rst | 17 ++---
>  drivers/media/pci/cx88/cx88-blackbird.c   |  3 +-
>  drivers/media/pci/cx88/cx88-video.c   | 10 +++---
>  drivers/media/pci/cx88/cx88.h |  4 +--
>  drivers/media/pci/saa7134/saa7134-video.c |  2 ++
>  drivers/media/usb/cx231xx/cx231xx-video.c |  2 ++
>  drivers/media/usb/pvrusb2/pvrusb2-v4l2.c  |  2 ++
>  drivers/media/usb/tm6000/tm6000-video.c   |  2 ++
>  drivers/media/v4l2-core/v4l2-dev.c| 10 +++---
>  include/media/v4l2-dev.h  | 59 
> +--
>  include/media/v4l2-mediabus.h | 30 
>  11 files changed, 98 insertions(+), 43 deletions(-)
> 
> diff --git a/Documentation/media/kapi/v4l2-dev.rst 
> b/Documentation/media/kapi/v4l2-dev.rst
> index b29aa616c267..7bb0505b60f1 100644
> --- a/Documentation/media/kapi/v4l2-dev.rst
> +++ b/Documentation/media/kapi/v4l2-dev.rst
> @@ -196,11 +196,18 @@ device.
>  Which device is registered depends on the type argument. The following
>  types exist:
>  
> -- ``VFL_TYPE_GRABBER``: ``/dev/videoX`` for video input/output devices
> -- ``VFL_TYPE_VBI``: ``/dev/vbiX`` for vertical blank data (i.e. closed 
> captions, teletext)
> -- ``VFL_TYPE_RADIO``: ``/dev/radioX`` for radio tuners
> -- ``VFL_TYPE_SDR``: ``/dev/swradioX`` for Software Defined Radio tuners
> -- ``VFL_TYPE_TOUCH``: ``/dev/v4l-touchX`` for touch sensors
> +==    
> ==
> +:c:type:`vfl_devnode_type` Device nameUsage
> +==    
> ==
> +``VFL_TYPE_GRABBER``   ``/dev/videoX``   for video input/output 
> devices
> +``VFL_TYPE_VBI``   ``/dev/vbiX`` for vertical blank data 
> (i.e.
> +  closed captions, teletext)
> +``VFL_TYPE_RADIO`` ``/dev/radioX``   for radio tuners
> +``VFL_TYPE_SUBDEV````/dev/v4l-subdevX``  for V4L2 subdevices
> +``VFL_TYPE_SDR``   ``/dev/swradioX`` for Software Defined Radio
> +  (SDR) tuners
> +``VFL_TYPE_TOUCH`` ``/dev/v4l-touchX``   for touch sensors
> +==    
> ==
>  
>  The last argument gives you a certain amount of control over the device
>  device node number used (i.e. the X in ``videoX``). Normally you will pass -1
> diff --git a/drivers/media/pci/cx88/cx88-blackbird.c 
> b/drivers/media/pci/cx88/cx88-blackbird.c
> index e3101f04941c..0e0952e60795 100644
> --- a/drivers/media/pci/cx88/cx88-blackbird.c
> +++ b/drivers/media/pci/cx88/cx88-blackbird.c
> @@ -805,8 +805,7 @@ static int vidioc_querycap(struct file *file, void  *priv,
>  
>   strcpy(cap->driver, "cx88_blackbird");
>   sprintf(cap->bus_info, "PCI:%s", pci_name(dev->pci));
> - cx88_querycap(file, core, cap);
> - return 0;
> + return cx88_querycap(file, core, cap);
>  }
>  
>  static int vidioc_enum_fmt_vid_cap(struct file *file, void  *priv,
> diff --git a/drivers/media/pci/cx88/cx88-video.c 
> b/drivers/media/pci/cx88/cx88-video.c
> index 7d25ecd4404b..9be682cdb644 100644
> --- a/drivers/media/pci/cx88/cx88-video.c
> +++ b/drivers/media/pci/cx88/cx88-video.c
> @@ -806,8 +806,8 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void 
> *priv,
>   return 0;
>  }
>  
> -void cx88_querycap(struct file *file, struct cx88_core *core,
> -struct v4l2_capability *cap)
> +int cx88_querycap(struct file *file, struct cx88_core *core,
> +   struct v4l2_capability *cap)
>  {
>   struct video_device *vdev = video_devdata(file);
>  
> @@ -825,11 +825,14 @@ void cx88_querycap(struct file *file, struct cx88_core 
> *core,
>   case VFL_TYPE_VBI:
>   cap->device_caps |= V4L2_CAP_VBI_CAPTURE;
>   break;
> + default:
> + return -EINVAL;
>   }
>   cap->capabilities = cap->device_caps | V4L2_CAP_VIDEO_CAPTURE |
>   V4L2_CAP_VBI_CAPTURE | V4L2_CAP_DEVICE_CAPS;
>   if (core->board.radio.type == CX88_RADIO)
>   cap->capabilities |= V4L2_CAP_RADIO;
> + return 0;
>  }
>  EXPORT_SYMBOL(cx88_querycap);
>  
> @@ -841,8 +844,7 @@ static int vidioc_querycap(struct file *file, void  *priv,
>  
>   strcpy(cap->driver, "cx8800");
>   sprintf(cap->bus_info, "PCI:%s", pci_name(dev->pci));
> - cx88_querycap(file, core, cap);
> - return 0;
> + return cx88_querycap(file, core, cap);
>  }
>  
>  static int vidioc_enum_fmt_vid_cap(struct file *file, void

Re: [PATCH 05/24] media: v4l2-dev: convert VFL_TYPE_* into an enum

2017-10-09 Thread Hans Verkuil 
On 09/10/17 12:19, Mauro Carvalho Chehab wrote:
> Using enums makes easier to document, as it can use kernel-doc
> markups. It also allows cross-referencing, with increases the
> kAPI readability.
> 
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Hans Verkuil 

Regards,

Hans

> ---
>  Documentation/media/kapi/v4l2-dev.rst | 17 ++---
>  drivers/media/pci/cx88/cx88-blackbird.c   |  3 +-
>  drivers/media/pci/cx88/cx88-video.c   | 10 +++---
>  drivers/media/pci/cx88/cx88.h |  4 +--
>  drivers/media/pci/saa7134/saa7134-video.c |  2 ++
>  drivers/media/usb/cx231xx/cx231xx-video.c |  2 ++
>  drivers/media/usb/pvrusb2/pvrusb2-v4l2.c  |  2 ++
>  drivers/media/usb/tm6000/tm6000-video.c   |  2 ++
>  drivers/media/v4l2-core/v4l2-dev.c| 10 +++---
>  include/media/v4l2-dev.h  | 59 
> +--
>  include/media/v4l2-mediabus.h | 30 
>  11 files changed, 98 insertions(+), 43 deletions(-)
> 
> diff --git a/Documentation/media/kapi/v4l2-dev.rst 
> b/Documentation/media/kapi/v4l2-dev.rst
> index b29aa616c267..7bb0505b60f1 100644
> --- a/Documentation/media/kapi/v4l2-dev.rst
> +++ b/Documentation/media/kapi/v4l2-dev.rst
> @@ -196,11 +196,18 @@ device.
>  Which device is registered depends on the type argument. The following
>  types exist:
>  
> -- ``VFL_TYPE_GRABBER``: ``/dev/videoX`` for video input/output devices
> -- ``VFL_TYPE_VBI``: ``/dev/vbiX`` for vertical blank data (i.e. closed 
> captions, teletext)
> -- ``VFL_TYPE_RADIO``: ``/dev/radioX`` for radio tuners
> -- ``VFL_TYPE_SDR``: ``/dev/swradioX`` for Software Defined Radio tuners
> -- ``VFL_TYPE_TOUCH``: ``/dev/v4l-touchX`` for touch sensors
> +==    
> ==
> +:c:type:`vfl_devnode_type` Device nameUsage
> +==    
> ==
> +``VFL_TYPE_GRABBER``   ``/dev/videoX``   for video input/output 
> devices
> +``VFL_TYPE_VBI``   ``/dev/vbiX`` for vertical blank data 
> (i.e.
> +  closed captions, teletext)
> +``VFL_TYPE_RADIO`` ``/dev/radioX``   for radio tuners
> +``VFL_TYPE_SUBDEV````/dev/v4l-subdevX``  for V4L2 subdevices
> +``VFL_TYPE_SDR``   ``/dev/swradioX`` for Software Defined Radio
> +  (SDR) tuners
> +``VFL_TYPE_TOUCH`` ``/dev/v4l-touchX``   for touch sensors
> +==    
> ==
>  
>  The last argument gives you a certain amount of control over the device
>  device node number used (i.e. the X in ``videoX``). Normally you will pass -1
> diff --git a/drivers/media/pci/cx88/cx88-blackbird.c 
> b/drivers/media/pci/cx88/cx88-blackbird.c
> index e3101f04941c..0e0952e60795 100644
> --- a/drivers/media/pci/cx88/cx88-blackbird.c
> +++ b/drivers/media/pci/cx88/cx88-blackbird.c
> @@ -805,8 +805,7 @@ static int vidioc_querycap(struct file *file, void  *priv,
>  
>   strcpy(cap->driver, "cx88_blackbird");
>   sprintf(cap->bus_info, "PCI:%s", pci_name(dev->pci));
> - cx88_querycap(file, core, cap);
> - return 0;
> + return cx88_querycap(file, core, cap);
>  }
>  
>  static int vidioc_enum_fmt_vid_cap(struct file *file, void  *priv,
> diff --git a/drivers/media/pci/cx88/cx88-video.c 
> b/drivers/media/pci/cx88/cx88-video.c
> index 7d25ecd4404b..9be682cdb644 100644
> --- a/drivers/media/pci/cx88/cx88-video.c
> +++ b/drivers/media/pci/cx88/cx88-video.c
> @@ -806,8 +806,8 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void 
> *priv,
>   return 0;
>  }
>  
> -void cx88_querycap(struct file *file, struct cx88_core *core,
> -struct v4l2_capability *cap)
> +int cx88_querycap(struct file *file, struct cx88_core *core,
> +   struct v4l2_capability *cap)
>  {
>   struct video_device *vdev = video_devdata(file);
>  
> @@ -825,11 +825,14 @@ void cx88_querycap(struct file *file, struct cx88_core 
> *core,
>   case VFL_TYPE_VBI:
>   cap->device_caps |= V4L2_CAP_VBI_CAPTURE;
>   break;
> + default:
> + return -EINVAL;
>   }
>   cap->capabilities = cap->device_caps | V4L2_CAP_VIDEO_CAPTURE |
>   V4L2_CAP_VBI_CAPTURE | V4L2_CAP_DEVICE_CAPS;
>   if (core->board.radio.type == CX88_RADIO)
>   cap->capabilities |= V4L2_CAP_RADIO;
> + return 0;
>  }
>  EXPORT_SYMBOL(cx88_querycap);
>  
> @@ -841,8 +844,7 @@ static int vidioc_querycap(struct file *file, void  *priv,
>  
>   strcpy(cap->driver, "cx8800");
>   sprintf(cap->bus_info, "PCI:%s", pci_name(dev->pci));
> - cx88_querycap(file, core, cap);
> - return 0;
> + return cx88_querycap(file, core, cap);
>  }
>  
>  static int

[PATCH 05/24] media: v4l2-dev: convert VFL_TYPE_* into an enum

2017-10-09 Thread Mauro Carvalho Chehab 
Using enums makes easier to document, as it can use kernel-doc
markups. It also allows cross-referencing, with increases the
kAPI readability.

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/media/kapi/v4l2-dev.rst | 17 ++---
 drivers/media/pci/cx88/cx88-blackbird.c   |  3 +-
 drivers/media/pci/cx88/cx88-video.c   | 10 +++---
 drivers/media/pci/cx88/cx88.h |  4 +--
 drivers/media/pci/saa7134/saa7134-video.c |  2 ++
 drivers/media/usb/cx231xx/cx231xx-video.c |  2 ++
 drivers/media/usb/pvrusb2/pvrusb2-v4l2.c  |  2 ++
 drivers/media/usb/tm6000/tm6000-video.c   |  2 ++
 drivers/media/v4l2-core/v4l2-dev.c| 10 +++---
 include/media/v4l2-dev.h  | 59 +--
 include/media/v4l2-mediabus.h | 30 
 11 files changed, 98 insertions(+), 43 deletions(-)

diff --git a/Documentation/media/kapi/v4l2-dev.rst 
b/Documentation/media/kapi/v4l2-dev.rst
index b29aa616c267..7bb0505b60f1 100644
--- a/Documentation/media/kapi/v4l2-dev.rst
+++ b/Documentation/media/kapi/v4l2-dev.rst
@@ -196,11 +196,18 @@ device.
 Which device is registered depends on the type argument. The following
 types exist:
 
-- ``VFL_TYPE_GRABBER``: ``/dev/videoX`` for video input/output devices
-- ``VFL_TYPE_VBI``: ``/dev/vbiX`` for vertical blank data (i.e. closed 
captions, teletext)
-- ``VFL_TYPE_RADIO``: ``/dev/radioX`` for radio tuners
-- ``VFL_TYPE_SDR``: ``/dev/swradioX`` for Software Defined Radio tuners
-- ``VFL_TYPE_TOUCH``: ``/dev/v4l-touchX`` for touch sensors
+==  
==
+:c:type:`vfl_devnode_type` Device name  Usage
+==  
==
+``VFL_TYPE_GRABBER``   ``/dev/videoX``   for video input/output devices
+``VFL_TYPE_VBI``   ``/dev/vbiX`` for vertical blank data (i.e.
+closed captions, teletext)
+``VFL_TYPE_RADIO`` ``/dev/radioX``   for radio tuners
+``VFL_TYPE_SUBDEV````/dev/v4l-subdevX``  for V4L2 subdevices
+``VFL_TYPE_SDR``   ``/dev/swradioX`` for Software Defined Radio
+(SDR) tuners
+``VFL_TYPE_TOUCH`` ``/dev/v4l-touchX``   for touch sensors
+==  
==
 
 The last argument gives you a certain amount of control over the device
 device node number used (i.e. the X in ``videoX``). Normally you will pass -1
diff --git a/drivers/media/pci/cx88/cx88-blackbird.c 
b/drivers/media/pci/cx88/cx88-blackbird.c
index e3101f04941c..0e0952e60795 100644
--- a/drivers/media/pci/cx88/cx88-blackbird.c
+++ b/drivers/media/pci/cx88/cx88-blackbird.c
@@ -805,8 +805,7 @@ static int vidioc_querycap(struct file *file, void  *priv,
 
strcpy(cap->driver, "cx88_blackbird");
sprintf(cap->bus_info, "PCI:%s", pci_name(dev->pci));
-   cx88_querycap(file, core, cap);
-   return 0;
+   return cx88_querycap(file, core, cap);
 }
 
 static int vidioc_enum_fmt_vid_cap(struct file *file, void  *priv,
diff --git a/drivers/media/pci/cx88/cx88-video.c 
b/drivers/media/pci/cx88/cx88-video.c
index 7d25ecd4404b..9be682cdb644 100644
--- a/drivers/media/pci/cx88/cx88-video.c
+++ b/drivers/media/pci/cx88/cx88-video.c
@@ -806,8 +806,8 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void 
*priv,
return 0;
 }
 
-void cx88_querycap(struct file *file, struct cx88_core *core,
-  struct v4l2_capability *cap)
+int cx88_querycap(struct file *file, struct cx88_core *core,
+ struct v4l2_capability *cap)
 {
struct video_device *vdev = video_devdata(file);
 
@@ -825,11 +825,14 @@ void cx88_querycap(struct file *file, struct cx88_core 
*core,
case VFL_TYPE_VBI:
cap->device_caps |= V4L2_CAP_VBI_CAPTURE;
break;
+   default:
+   return -EINVAL;
}
cap->capabilities = cap->device_caps | V4L2_CAP_VIDEO_CAPTURE |
V4L2_CAP_VBI_CAPTURE | V4L2_CAP_DEVICE_CAPS;
if (core->board.radio.type == CX88_RADIO)
cap->capabilities |= V4L2_CAP_RADIO;
+   return 0;
 }
 EXPORT_SYMBOL(cx88_querycap);
 
@@ -841,8 +844,7 @@ static int vidioc_querycap(struct file *file, void  *priv,
 
strcpy(cap->driver, "cx8800");
sprintf(cap->bus_info, "PCI:%s", pci_name(dev->pci));
-   cx88_querycap(file, core, cap);
-   return 0;
+   return cx88_querycap(file, core, cap);
 }
 
 static int vidioc_enum_fmt_vid_cap(struct file *file, void  *priv,
diff --git a/drivers/media/pci/cx88/cx88.h b/drivers/media/pci/cx88/cx88.h
index 6777926f20f2..07a33f02fef4 100644
--- a/drivers/media/pci/cx88/cx88.h
+++ b/drivers/media/pci/cx88/cx88.h
@@ -734,7 +734,7 @@ int cx8802_start_dma(struct cx8802_dev*dev,
 int