[PATCH v4 0/7] document types of hardware control for V4L2

[PATCH v4 0/7] document types of hardware control for V4L2

[Mauro Carvalho Chehab]

On Kernel 2.6.39, the omap3 driver was introduced together with a new way
to control complex V4L2 devices used on embedded systems, but this was
never documented, as the original idea were to have "soon" support for
standard apps to use it as well, via libv4l, but that didn't happen so far.

Also, it is not possible for an userspace applicatin to detect the kind of
control a device supports.

This series fill the gap, by documenting the new type of hardware
control and adding a way for userspace to detect if the device can be
used or not by an standard V4L2 application.

Notes:
====

1) For the sake of better review, this series start with the addition of a
glossary, as requested by Laurent. Please notice, however, that
the glossary there references some new captions that will only be added
by subsequent patches. So, when this series get applied, the glossary
patch should actually be merged after the patches that introduce those
new captions, in order to avoid warnings for non-existing references.

2) This series doesn't contain patches that actually use the new flag.
This will be added after such patch gets reviewed.

v4:

- Addressed Hans comments for v2;
- Fixed broken references at the glossary.rst

v3:

- Add a glossary to be used by the new documentation about hardware control;
- Add a patch removing minor number range
- Use glossary terms at open.rst
- Split the notice about subdev-API on vdev-centric, as this change
   will require further discussions.

v2:

- added a patch at the beginning of the series better defining the
  device node naming rules;
- better defined the differenes between device hardware and V4L2 device node
  as suggested by Laurent and with changes proposed by Hans and Sakari
- changed the caps flag to indicate MC-centric devices
- removed the final patch that would use the new caps flag. I'll write it
  once we agree on the new caps flag.


Mauro Carvalho Chehab (7):
  media: add glossary.rst with a glossary of terms used at V4L2 spec
  media: open.rst: better document device node naming
  media: open.rst: remove the minor number range
  media: open.rst: document devnode-centric and mc-centric types
  media: open.rst: Adjust some terms to match the glossary
  media: videodev2: add a flag for MC-centric devices
  media: open.rst: add a notice about subdev-API on vdev-centric

 Documentation/media/uapi/v4l/glossary.rst        |  98 ++++++++++++++++++
 Documentation/media/uapi/v4l/open.rst            | 123 ++++++++++++++++++++---
 Documentation/media/uapi/v4l/v4l2.rst            |   1 +
 Documentation/media/uapi/v4l/vidioc-querycap.rst |   5 +
 Documentation/media/videodev2.h.rst.exceptions   |   1 +
 include/uapi/linux/videodev2.h                   |   2 +
 6 files changed, 216 insertions(+), 14 deletions(-)
 create mode 100644 Documentation/media/uapi/v4l/glossary.rst

-- 
2.13.3

[PATCH v4 1/7] media: add glossary.rst with a glossary of terms used at V4L2 spec

[Mauro Carvalho Chehab]

Add a glossary of terms for V4L2, as several concepts are complex
enough to cause misunderstandings.

Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
 Documentation/media/uapi/v4l/glossary.rst | 98 +++++++++++++++++++++++++++++++
 Documentation/media/uapi/v4l/v4l2.rst     |  1 +
 2 files changed, 99 insertions(+)
 create mode 100644 Documentation/media/uapi/v4l/glossary.rst

diff --git a/Documentation/media/uapi/v4l/glossary.rst b/Documentation/media/uapi/v4l/glossary.rst
new file mode 100644
index 000000000000..e55cd357dad3
--- /dev/null
+++ b/Documentation/media/uapi/v4l/glossary.rst
@@ -0,0 +1,98 @@
+========
+Glossary
+========
+
+.. note::
+
+   This goal of section is to standardize the terms used within the V4L2
+   documentation. It is written incrementally as they're standardized at
+   the V4L2 documentation. So, it is an incomplete Work In Progress.
+
+.. Please keep the glossary entries in alphabetical order
+
+.. glossary::
+
+    Bridge driver
+	 The same as V4L2 main driver.
+
+    Device Node
+	 A character device node at the file system used to control and do
+	 input/output data transfers to a Kernel driver.
+
+    Driver
+	 The part of the Linux Kernel that implements support
+	 for a hardware component.
+
+    Inter-Integrated Circuit - I²C
+	 A  multi-master, multi-slave, packet switched, single-ended,
+	 serial computer bus used to control V4L2 sub-devices
+
+    Hardware component
+	 a subset of the media hardware.
+
+    Hardware peripheral
+	 A group of hardware components that together make a larger
+	 user-facing functional peripheral. For instance the SoC ISP IP
+	 cores and external camera sensors together make a
+	 camera hardware peripheral.
+	 Also known as peripheral.
+
+    Hardware peripheral control
+	 Type of control that it is possible for a V4L2 hardware peripheral.
+
+	 See :ref:`v4l2_hardware_control`.
+
+    Peripheral
+	 The same as hardware peripheral.
+
+    Media Controller
+	 An API used to identify the hardware components.
+
+	 See :ref:`media_controller`.
+
+    MC-centric
+	 V4L2 hardware that requires a Media controller to be controlled.
+
+	 See :ref:`v4l2_hardware_control`.
+
+    SMBus
+	 A subset of I²C, with defines a stricter usage of the bus.
+
+    Serial Peripheral Interface Bus - SPI
+	 Synchronous serial communication interface specification used for
+	 short distance communication, primarily in embedded systems.
+
+    Sub-device hardware components
+	 hardware components that aren't controlled by the
+	 V4L2 main driver.
+
+    V4L2 device node
+	 A device node that it is associated to a V4L2 main driver,
+	 as specified at :ref:`v4l2_device_naming`.
+
+    V4L2 hardware
+	 A hardware used to on a media device supported by the V4L2
+	 subsystem.
+
+    V4L2 hardware control
+	 The type of hardware control that a device supports.
+
+	 See :ref:`v4l2_hardware_control`.
+
+    V4L2 main driver
+	 The V4L2 device driver that implements the main logic to talk with
+	 the V4L2 hardware.
+	 Also known as bridge driver.
+
+	 See :ref:`v4l2_hardware_control`.
+
+    V4L2 sub-device
+	 part of the media hardware that it is implemented by a device
+	 driver that is not part of the main V4L2 driver.
+
+	 See :ref:`subdev`.
+
+    Vdev-centric
+	 V4L2 hardware that it is controlled via V4L2 device nodes.
+
+	 See :ref:`v4l2_hardware_control`.
diff --git a/Documentation/media/uapi/v4l/v4l2.rst b/Documentation/media/uapi/v4l/v4l2.rst
index f52a11c949d3..1ee4b86d18e1 100644
--- a/Documentation/media/uapi/v4l/v4l2.rst
+++ b/Documentation/media/uapi/v4l/v4l2.rst
@@ -31,6 +31,7 @@ This part describes the Video for Linux API version 2 (V4L2 API) specification.
     videodev
     capture-example
     v4l2grab-example
+    glossary
     biblio
 
 
-- 
2.13.3

Re: [PATCH v4 1/7] media: add glossary.rst with a glossary of terms used at V4L2 spec

[Hans Verkuil]

This is a partial review. I think I need to review the other patches first
before continuing this review. But it doesn't hurt to post this now.

On 26/08/17 13:53, Mauro Carvalho Chehab wrote:
> Add a glossary of terms for V4L2, as several concepts are complex
> enough to cause misunderstandings.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---
>  Documentation/media/uapi/v4l/glossary.rst | 98 +++++++++++++++++++++++++++++++
>  Documentation/media/uapi/v4l/v4l2.rst     |  1 +
>  2 files changed, 99 insertions(+)
>  create mode 100644 Documentation/media/uapi/v4l/glossary.rst
> 
> diff --git a/Documentation/media/uapi/v4l/glossary.rst b/Documentation/media/uapi/v4l/glossary.rst
> new file mode 100644
> index 000000000000..e55cd357dad3
> --- /dev/null
> +++ b/Documentation/media/uapi/v4l/glossary.rst
> @@ -0,0 +1,98 @@
> +========
> +Glossary
> +========
> +
> +.. note::
> +
> +   This goal of section is to standardize the terms used within the V4L2
> +   documentation. It is written incrementally as they're standardized at

they're -> they are
at -> in

> +   the V4L2 documentation. So, it is an incomplete Work In Progress.

an incomplete Work -> a Work

(a work in progress is by definition incomplete :-) )

> +
> +.. Please keep the glossary entries in alphabetical order
> +
> +.. glossary::
> +
> +    Bridge driver
> +	 The same as V4L2 main driver.
> +
> +    Device Node
> +	 A character device node at the file system used to control and do

at -> in

> +	 input/output data transfers to a Kernel driver.

to -> from/to

> +
> +    Driver
> +	 The part of the Linux Kernel that implements support
> +	 for a hardware component.
> +
> +    Inter-Integrated Circuit - I²C
> +	 A  multi-master, multi-slave, packet switched, single-ended,
> +	 serial computer bus used to control V4L2 sub-devices

Missing . at the end. Perhaps add a link to the i2c spec or wikipedia page?

Also, this entry is not in alphabetical order.

> +
> +    Hardware component
> +	 a subset of the media hardware.

Could use some examples:

"For example an i2c or SPI device, or an IP block inside an SoC or FPGA
(https://en.wikipedia.org/wiki/Semiconductor_intellectual_property_core)."

> +
> +    Hardware peripheral
> +	 A group of hardware components that together make a larger
> +	 user-facing functional peripheral. For instance the SoC ISP IP
> +	 cores and external camera sensors together make a
> +	 camera hardware peripheral.
> +	 Also known as peripheral.

SoC, ISP and IP core need their own entries here.

> +
> +    Hardware peripheral control
> +	 Type of control that it is possible for a V4L2 hardware peripheral.

This sentence makes no sense.

> +
> +	 See :ref:`v4l2_hardware_control`.
> +
> +    Peripheral
> +	 The same as hardware peripheral.
> +
> +    Media Controller
> +	 An API used to identify the hardware components.

...components and (optionally) change the links between hardware components.

> +
> +	 See :ref:`media_controller`.
> +
> +    MC-centric
> +	 V4L2 hardware that requires a Media controller to be controlled.

I think I would just drop the 'to be controlled' bit.

> +
> +	 See :ref:`v4l2_hardware_control`.
> +
> +    SMBus
> +	 A subset of I²C, with defines a stricter usage of the bus.
> +
> +    Serial Peripheral Interface Bus - SPI
> +	 Synchronous serial communication interface specification used for
> +	 short distance communication, primarily in embedded systems.
> +
> +    Sub-device hardware components
> +	 hardware components that aren't controlled by the

hardware -> Hardware

> +	 V4L2 main driver.
> +
> +    V4L2 device node
> +	 A device node that it is associated to a V4L2 main driver,

it is -> is
to -> with

> +	 as specified at :ref:`v4l2_device_naming`.

at -> in

> +
> +    V4L2 hardware
> +	 A hardware used to on a media device supported by the V4L2
> +	 subsystem.
> +
> +    V4L2 hardware control
> +	 The type of hardware control that a device supports.
> +
> +	 See :ref:`v4l2_hardware_control`.
> +
> +    V4L2 main driver
> +	 The V4L2 device driver that implements the main logic to talk with
> +	 the V4L2 hardware.
> +	 Also known as bridge driver.
> +
> +	 See :ref:`v4l2_hardware_control`.
> +
> +    V4L2 sub-device
> +	 part of the media hardware that it is implemented by a device
> +	 driver that is not part of the main V4L2 driver.
> +
> +	 See :ref:`subdev`.
> +
> +    Vdev-centric
> +	 V4L2 hardware that it is controlled via V4L2 device nodes.
> +
> +	 See :ref:`v4l2_hardware_control`.
> diff --git a/Documentation/media/uapi/v4l/v4l2.rst b/Documentation/media/uapi/v4l/v4l2.rst
> index f52a11c949d3..1ee4b86d18e1 100644
> --- a/Documentation/media/uapi/v4l/v4l2.rst
> +++ b/Documentation/media/uapi/v4l/v4l2.rst
> @@ -31,6 +31,7 @@ This part describes the Video for Linux API version 2 (V4L2 API) specification.
>      videodev
>      capture-example
>      v4l2grab-example
> +    glossary
>      biblio
>  
>  
> 

[PATCH v4 2/7] media: open.rst: better document device node naming

[Mauro Carvalho Chehab]

Right now, only kAPI documentation describes the device naming.
However, such description is needed at the uAPI too. Add it,
and describe how to get an unique identify for a given device.

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
 Documentation/media/uapi/v4l/open.rst | 39 ++++++++++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
index afd116edb40d..fc0037091814 100644
--- a/Documentation/media/uapi/v4l/open.rst
+++ b/Documentation/media/uapi/v4l/open.rst
@@ -7,12 +7,14 @@ Opening and Closing Devices
 ***************************
 
 
-Device Naming
-=============
+.. _v4l2_device_naming:
+
+V4L2 Device Node Naming
+=======================
 
 V4L2 drivers are implemented as kernel modules, loaded manually by the
 system administrator or automatically when a device is first discovered.
-The driver modules plug into the "videodev" kernel module. It provides
+The driver modules plug into the ``videodev`` kernel module. It provides
 helper functions and a common application interface specified in this
 document.
 
@@ -23,6 +25,37 @@ option CONFIG_VIDEO_FIXED_MINOR_RANGES. In that case minor numbers
 are allocated in ranges depending on the device node type (video, radio,
 etc.).
 
+The existing V4L2 device node types are:
+
+======================== ======================================================
+Default device node name Usage
+======================== ======================================================
+``/dev/videoX``		 Video input/output devices
+``/dev/vbiX``		 Vertical blank data (i.e. closed captions, teletext)
+``/dev/radioX``		 Radio tuners and modulators
+``/dev/swradioX``	 Software Defined Radio tuners and modulators
+``/dev/v4l-touchX``	 Touch sensors
+======================== ======================================================
+
+Where ``X`` is a non-negative number.
+
+.. note::
+
+   1. The actual device node name is system-dependent, as udev rules may apply.
+   2. There is no warranty that ``X`` will remain the same for the same
+      device, as the number depends on the device driver's probe order.
+      If you need an unique name, udev default rules produce
+      ``/dev/v4l/by-id/`` and ``/dev/v4l/by-path/`` directoiries containing
+      links that can be used uniquely to identify a V4L2 device node::
+
+	$ tree /dev/v4l
+	/dev/v4l
+	├── by-id
+	│   └── usb-OmniVision._USB_Camera-B4.04.27.1-video-index0 -> ../../video0
+	└── by-path
+	    └── pci-0000:00:14.0-usb-0:2:1.0-video-index0 -> ../../video0
+
+
 Many drivers support "video_nr", "radio_nr" or "vbi_nr" module
 options to select specific video/radio/vbi node numbers. This allows the
 user to request that the device node is named e.g. /dev/video5 instead
-- 
2.13.3

[PATCH v4 3/7] media: open.rst: remove the minor number range

[Mauro Carvalho Chehab]

minor numbers use to range between 0 to 255, but that
was changed a long time ago. While it still applies when
CONFIG_VIDEO_FIXED_MINOR_RANGES, when the minor number is
dynamically allocated, this may not be true. In any case,
this is not relevant, as udev will take care of it.

So, remove this useless misinformation.

Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
 Documentation/media/uapi/v4l/open.rst | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
index fc0037091814..96ac972c1fa2 100644
--- a/Documentation/media/uapi/v4l/open.rst
+++ b/Documentation/media/uapi/v4l/open.rst
@@ -19,11 +19,10 @@ helper functions and a common application interface specified in this
 document.
 
 Each driver thus loaded registers one or more device nodes with major
-number 81 and a minor number between 0 and 255. Minor numbers are
-allocated dynamically unless the kernel is compiled with the kernel
-option CONFIG_VIDEO_FIXED_MINOR_RANGES. In that case minor numbers
-are allocated in ranges depending on the device node type (video, radio,
-etc.).
+number 81. Minor numbers are allocated dynamically unless the kernel
+is compiled with the kernel option CONFIG_VIDEO_FIXED_MINOR_RANGES.
+In that case minor numbers are allocated in ranges depending on the
+device node type.
 
 The existing V4L2 device node types are:
 
-- 
2.13.3

Re: [PATCH v4 3/7] media: open.rst: remove the minor number range

[Hans Verkuil]

On 26/08/17 13:53, Mauro Carvalho Chehab wrote:
> minor numbers use to range between 0 to 255, but that
> was changed a long time ago. While it still applies when
> CONFIG_VIDEO_FIXED_MINOR_RANGES, when the minor number is
> dynamically allocated, this may not be true. In any case,
> this is not relevant, as udev will take care of it.
> 
> So, remove this useless misinformation.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

> ---
>  Documentation/media/uapi/v4l/open.rst | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
> index fc0037091814..96ac972c1fa2 100644
> --- a/Documentation/media/uapi/v4l/open.rst
> +++ b/Documentation/media/uapi/v4l/open.rst
> @@ -19,11 +19,10 @@ helper functions and a common application interface specified in this
>  document.
>  
>  Each driver thus loaded registers one or more device nodes with major
> -number 81 and a minor number between 0 and 255. Minor numbers are
> -allocated dynamically unless the kernel is compiled with the kernel
> -option CONFIG_VIDEO_FIXED_MINOR_RANGES. In that case minor numbers
> -are allocated in ranges depending on the device node type (video, radio,
> -etc.).
> +number 81. Minor numbers are allocated dynamically unless the kernel
> +is compiled with the kernel option CONFIG_VIDEO_FIXED_MINOR_RANGES.

I wonder if we shouldn't remove this kernel option completely. Does it
make any sense to keep holding on to this?

Regards,

	Hans

> +In that case minor numbers are allocated in ranges depending on the
> +device node type.
>  
>  The existing V4L2 device node types are:
>  
> 

[PATCH v4 4/7] media: open.rst: document devnode-centric and mc-centric types

[Mauro Carvalho Chehab]

When we added support for omap3, back in 2010, we added a new
type of V4L2 devices that aren't fully controlled via the V4L2
device node.

Yet, we have never clearly documented in the V4L2 specification
the differences between the two types.

Let's document them based on the the current implementation.

Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
 Documentation/media/uapi/v4l/open.rst | 49 +++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
index 96ac972c1fa2..51acb8de8ba8 100644
--- a/Documentation/media/uapi/v4l/open.rst
+++ b/Documentation/media/uapi/v4l/open.rst
@@ -7,6 +7,55 @@ Opening and Closing Devices
 ***************************
 
 
+.. _v4l2_hardware_control:
+
+
+Types of V4L2 hardware peripheral control
+=========================================
+
+V4L2 hardware periferal is usually complex: support for it is
+implemented via a V4L2 main driver and often by several additional drivers.
+The main driver always exposes one or more **V4L2 device nodes**
+(see :ref:`v4l2_device_naming`).
+
+The other drivers are called **V4L2 sub-devices** and provide control to
+other hardware components usually connected via a serial bus (like
+I²C, SMBus or SPI). Depending on the main driver, they can be implicitly
+controlled directly by the main driver or explicitly via
+the **V4L2 sub-device API** (see :ref:`subdev`).
+
+When V4L2 was originally designed, there was only one type of
+peripheral control: via the **V4L2 device nodes**. We refer to this kind
+of control as **V4L2 device node centric** (or, simply, "**vdev-centric**").
+
+Later (kernel 2.6.39), a new type of periferal control was
+added in order to support complex peripherals that are common for embedded
+systems. This type of periferal is controlled mainly via the media
+controller and V4L2 sub-devices. So, it is called
+**Media controller centric** (or, simply, "**MC-centric**").
+
+For **vdev-centric** hardware peripheral control, the peripheral is
+controlled via the **V4L2 device nodes**. They may optionally support the
+:ref:`media controller API <media_controller>` as well, in order to let
+the application to know which device nodes are available
+(see :ref:`related`).
+
+For **MC-centric** hardware peripheral control it is required to configure
+the pipelines via the :ref:`media controller API <media_controller>` before
+the periferal can be used. For such devices, the sub-devices' configuration
+can be controlled via the :ref:`sub-device API <subdev>`, which creates one
+device node per sub-device.
+
+In summary, for **MC-centric** hardware peripheral control:
+
+- The **V4L2 device** node is responsible for controlling the streaming
+  features;
+- The **media controller device** is responsible to setup the pipelines
+  at the peripheral;
+- The **V4L2 sub-devices** are responsible for V4L2 sub-device
+  specific settings at the sub-device hardware components.
+
+
 .. _v4l2_device_naming:
 
 V4L2 Device Node Naming
-- 
2.13.3

Re: [PATCH v4 4/7] media: open.rst: document devnode-centric and mc-centric types

[Hans Verkuil]

On 26/08/17 13:53, Mauro Carvalho Chehab wrote:
> When we added support for omap3, back in 2010, we added a new
> type of V4L2 devices that aren't fully controlled via the V4L2
> device node.
> 
> Yet, we have never clearly documented in the V4L2 specification
> the differences between the two types.
> 
> Let's document them based on the the current implementation.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---
>  Documentation/media/uapi/v4l/open.rst | 49 +++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
> index 96ac972c1fa2..51acb8de8ba8 100644
> --- a/Documentation/media/uapi/v4l/open.rst
> +++ b/Documentation/media/uapi/v4l/open.rst
> @@ -7,6 +7,55 @@ Opening and Closing Devices
>  ***************************
>  
>  
> +.. _v4l2_hardware_control:
> +
> +
> +Types of V4L2 hardware peripheral control
> +=========================================
> +
> +V4L2 hardware periferal is usually complex: support for it is

peripheral

I *really* don't like the term "hardware peripheral". For me that means a
mouse, keyboard, printer, webcam, i.e. some external device that you connect
to a USB bus or similar, but it makes no sense as a definition of an
SoC + sensor(s) hardware design.

I would simple define "V4L2 hardware" as consisting of 1 or more "V4L2 hardware
components".

> +implemented via a V4L2 main driver and often by several additional drivers.
> +The main driver always exposes one or more **V4L2 device nodes**
> +(see :ref:`v4l2_device_naming`).

I think we should mention that the V4L2 device nodes are responsible for
implementing streaming (if applicable) of data.

> +
> +The other drivers are called **V4L2 sub-devices** and provide control to
> +other hardware components usually connected via a serial bus (like
> +I²C, SMBus or SPI). Depending on the main driver, they can be implicitly
> +controlled directly by the main driver or explicitly via
> +the **V4L2 sub-device API** (see :ref:`subdev`).
> +
> +When V4L2 was originally designed, there was only one type of
> +peripheral control: via the **V4L2 device nodes**. We refer to this kind

Again, I prefer the term "V4L2 hardware control".

> +of control as **V4L2 device node centric** (or, simply, "**vdev-centric**").
> +
> +Later (kernel 2.6.39), a new type of periferal control was

periferal -> V4L2 hardware

> +added in order to support complex peripherals that are common for embedded

complex V4L2 hardware

(repeat below where you use this 'peripheral' term)

> +systems. This type of periferal is controlled mainly via the media
> +controller and V4L2 sub-devices. So, it is called
> +**Media controller centric** (or, simply, "**MC-centric**").

add 'control' at the end.

> +
> +For **vdev-centric** hardware peripheral control, the peripheral is
> +controlled via the **V4L2 device nodes**. They may optionally support the
> +:ref:`media controller API <media_controller>` as well, in order to let
> +the application to know which device nodes are available

to know -> know

Actually, I would rephrase this to:

in order to inform the application which device nodes are available

> +(see :ref:`related`).
> +
> +For **MC-centric** hardware peripheral control it is required to configure
> +the pipelines via the :ref:`media controller API <media_controller>` before
> +the periferal can be used. For such devices, the sub-devices' configuration
> +can be controlled via the :ref:`sub-device API <subdev>`, which creates one
> +device node per sub-device.
> +
> +In summary, for **MC-centric** hardware peripheral control:
> +
> +- The **V4L2 device** node is responsible for controlling the streaming
> +  features;
> +- The **media controller device** is responsible to setup the pipelines
> +  at the peripheral;
> +- The **V4L2 sub-devices** are responsible for V4L2 sub-device
> +  specific settings at the sub-device hardware components.

... settings of the corresponding hardware components.

I agree with Laurent that I don't think this summary is needed. I would drop
it for v5 and we can look at the text again and see if it needs more work to
clarify things.

The main thing here is that the note about the V4L2 device node being responsible
for controlling the streaming features is mentioned when the V4L2 device node is
introduced above, since this is true for both MC and vdev-centric HW control.

> +
> +
>  .. _v4l2_device_naming:
>  
>  V4L2 Device Node Naming
> 

Regards,

	Hans

Re: [PATCH v4 4/7] media: open.rst: document devnode-centric and mc-centric types

[Mauro Carvalho Chehab]

Em Mon, 28 Aug 2017 11:36:13 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 26/08/17 13:53, Mauro Carvalho Chehab wrote:
> > When we added support for omap3, back in 2010, we added a new
> > type of V4L2 devices that aren't fully controlled via the V4L2
> > device node.
> > 
> > Yet, we have never clearly documented in the V4L2 specification
> > the differences between the two types.
> > 
> > Let's document them based on the the current implementation.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > ---
> >  Documentation/media/uapi/v4l/open.rst | 49 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 49 insertions(+)
> > 
> > diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
> > index 96ac972c1fa2..51acb8de8ba8 100644
> > --- a/Documentation/media/uapi/v4l/open.rst
> > +++ b/Documentation/media/uapi/v4l/open.rst
> > @@ -7,6 +7,55 @@ Opening and Closing Devices
> >  ***************************
> >  
> >  
> > +.. _v4l2_hardware_control:
> > +
> > +
> > +Types of V4L2 hardware peripheral control
> > +=========================================
> > +
> > +V4L2 hardware periferal is usually complex: support for it is  
> 
> peripheral
> 
> I *really* don't like the term "hardware peripheral". For me that means a
> mouse, keyboard, printer, webcam, i.e. some external device that you connect
> to a USB bus or similar, but it makes no sense as a definition of an
> SoC + sensor(s) hardware design.
> 
> I would simple define "V4L2 hardware" as consisting of 1 or more "V4L2 hardware
> components".

I don't have a strong preference here.

If I remember my computer architecture books, back on the old days, 
everything that does I/O is technically a peripheral to the CPU. That
seems, btw, the definition used by Wikipedia:

	https://en.wikipedia.org/wiki/Peripheral

So, calling it as peripheral works.

On the other hand, "V4L2 hardware" doesn't seem an adequate term,
as it associates a software API with a piece of hardware. It is
weirder when the hardware have an hybrid tuner, as the same hardware
is visible via both DVB and V4L2 APIs. Yet, we could find a similar
wording, like "media hardware".

I'm OK to replace it to something like "media hardware", but, I
prefer to do such change on a patch to be applied after this series,
in order to minimize the rebase needs[1].

[1] as you noticed on patch 6/7, with all those nomenclature changes,
one of the places were written as "v4l2-centric" instead of "vdev-centric"
as on the other patches. That's basically the problem with rebases: we
end by letting loose ends. So, IMHO, if we end by deciding to rename
	A->B
	C->D
Let's do it on a separate patch at the end of the series, as a simple
grep |sed -i could replace all occurrences at once without letting
lose ands and without needing to solve patch merge conflicts.

> > +implemented via a V4L2 main driver and often by several additional drivers.
> > +The main driver always exposes one or more **V4L2 device nodes**
> > +(see :ref:`v4l2_device_naming`).  
> 
> I think we should mention that the V4L2 device nodes are responsible for
> implementing streaming (if applicable) of data.

Ok.

> > +
> > +The other drivers are called **V4L2 sub-devices** and provide control to
> > +other hardware components usually connected via a serial bus (like
> > +I²C, SMBus or SPI). Depending on the main driver, they can be implicitly
> > +controlled directly by the main driver or explicitly via
> > +the **V4L2 sub-device API** (see :ref:`subdev`).
> > +
> > +When V4L2 was originally designed, there was only one type of
> > +peripheral control: via the **V4L2 device nodes**. We refer to this kind  
> 
> Again, I prefer the term "V4L2 hardware control".
> 
> > +of control as **V4L2 device node centric** (or, simply, "**vdev-centric**").
> > +
> > +Later (kernel 2.6.39), a new type of periferal control was  
> 
> periferal -> V4L2 hardware
> 
> > +added in order to support complex peripherals that are common for embedded  
> 
> complex V4L2 hardware
> 
> (repeat below where you use this 'peripheral' term)
> 
> > +systems. This type of periferal is controlled mainly via the media
> > +controller and V4L2 sub-devices. So, it is called
> > +**Media controller centric** (or, simply, "**MC-centric**").  
> 
> add 'control' at the end.

Ok.

> > +
> > +For **vdev-centric** hardware peripheral control, the peripheral is
> > +controlled via the **V4L2 device nodes**. They may optionally support the
> > +:ref:`media controller API <media_controller>` as well, in order to let
> > +the application to know which device nodes are available  
> 
> to know -> know
> 
> Actually, I would rephrase this to:
> 
> in order to inform the application which device nodes are available

Ok.

> > +(see :ref:`related`).
> > +
> > +For **MC-centric** hardware peripheral control it is required to configure
> > +the pipelines via the :ref:`media controller API <media_controller>` before
> > +the periferal can be used. For such devices, the sub-devices' configuration
> > +can be controlled via the :ref:`sub-device API <subdev>`, which creates one
> > +device node per sub-device.
> > +
> > +In summary, for **MC-centric** hardware peripheral control:
> > +
> > +- The **V4L2 device** node is responsible for controlling the streaming
> > +  features;
> > +- The **media controller device** is responsible to setup the pipelines
> > +  at the peripheral;
> > +- The **V4L2 sub-devices** are responsible for V4L2 sub-device
> > +  specific settings at the sub-device hardware components.  
> 
> ... settings of the corresponding hardware components.
> 
> I agree with Laurent that I don't think this summary is needed. I would drop
> it for v5 and we can look at the text again and see if it needs more work to
> clarify things.

Ok, I'll drop it.

> The main thing here is that the note about the V4L2 device node being responsible
> for controlling the streaming features is mentioned when the V4L2 device node is
> introduced above, since this is true for both MC and vdev-centric HW control.
> 
> > +
> > +
> >  .. _v4l2_device_naming:
> >  
> >  V4L2 Device Node Naming
> >   
> 
> Regards,
> 
> 	Hans

I'll submit a v5 soon, without the terms renaming. If we all agree
with renaming terms, I'll produce a separate patch fixing it where
used.

Thanks,
Mauro

[PATCH v4 5/7] media: open.rst: Adjust some terms to match the glossary

[Mauro Carvalho Chehab]

As we now have a glossary, some terms used on open.rst
require adjustments.

Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
 Documentation/media/uapi/v4l/open.rst | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
index 51acb8de8ba8..64b1de047b1b 100644
--- a/Documentation/media/uapi/v4l/open.rst
+++ b/Documentation/media/uapi/v4l/open.rst
@@ -147,7 +147,7 @@ Related Devices
 Devices can support several functions. For example video capturing, VBI
 capturing and radio support.
 
-The V4L2 API creates different nodes for each of these functions.
+The V4L2 API creates different V4L2 device nodes for each of these functions.
 
 The V4L2 API was designed with the idea that one device node could
 support all functions. However, in practice this never worked: this
@@ -157,17 +157,17 @@ switching a device node between different functions only works when
 using the streaming I/O API, not with the
 :ref:`read() <func-read>`/\ :ref:`write() <func-write>` API.
 
-Today each device node supports just one function.
+Today each V4L2 device node supports just one function.
 
 Besides video input or output the hardware may also support audio
 sampling or playback. If so, these functions are implemented as ALSA PCM
 devices with optional ALSA audio mixer devices.
 
 One problem with all these devices is that the V4L2 API makes no
-provisions to find these related devices. Some really complex devices
-use the Media Controller (see :ref:`media_controller`) which can be
-used for this purpose. But most drivers do not use it, and while some
-code exists that uses sysfs to discover related devices (see
+provisions to find these related V4L2 device nodes. Some really complex
+hardware use the Media Controller (see :ref:`media_controller`) which can
+be used for this purpose. But several drivers do not use it, and while some
+code exists that uses sysfs to discover related V4L2 device nodes (see
 libmedia_dev in the
 `v4l-utils <http://git.linuxtv.org/cgit.cgi/v4l-utils.git/>`__ git
 repository), there is no library yet that can provide a single API
-- 
2.13.3

Re: [PATCH v4 5/7] media: open.rst: Adjust some terms to match the glossary

[Hans Verkuil]

On 26/08/17 13:53, Mauro Carvalho Chehab wrote:
> As we now have a glossary, some terms used on open.rst
> require adjustments.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

> ---
>  Documentation/media/uapi/v4l/open.rst | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
> index 51acb8de8ba8..64b1de047b1b 100644
> --- a/Documentation/media/uapi/v4l/open.rst
> +++ b/Documentation/media/uapi/v4l/open.rst
> @@ -147,7 +147,7 @@ Related Devices
>  Devices can support several functions. For example video capturing, VBI
>  capturing and radio support.
>  
> -The V4L2 API creates different nodes for each of these functions.
> +The V4L2 API creates different V4L2 device nodes for each of these functions.
>  
>  The V4L2 API was designed with the idea that one device node could
>  support all functions. However, in practice this never worked: this
> @@ -157,17 +157,17 @@ switching a device node between different functions only works when
>  using the streaming I/O API, not with the
>  :ref:`read() <func-read>`/\ :ref:`write() <func-write>` API.
>  
> -Today each device node supports just one function.
> +Today each V4L2 device node supports just one function.
>  
>  Besides video input or output the hardware may also support audio
>  sampling or playback. If so, these functions are implemented as ALSA PCM
>  devices with optional ALSA audio mixer devices.
>  
>  One problem with all these devices is that the V4L2 API makes no
> -provisions to find these related devices. Some really complex devices
> -use the Media Controller (see :ref:`media_controller`) which can be
> -used for this purpose. But most drivers do not use it, and while some
> -code exists that uses sysfs to discover related devices (see
> +provisions to find these related V4L2 device nodes. Some really complex
> +hardware use the Media Controller (see :ref:`media_controller`) which can
> +be used for this purpose. But several drivers do not use it, and while some
> +code exists that uses sysfs to discover related V4L2 device nodes (see
>  libmedia_dev in the
>  `v4l-utils <http://git.linuxtv.org/cgit.cgi/v4l-utils.git/>`__ git
>  repository), there is no library yet that can provide a single API
> 

[PATCH v4 6/7] media: videodev2: add a flag for MC-centric devices

[Mauro Carvalho Chehab]

As both vdev-centric and MC-centric devices may implement the
same APIs, we need a flag to allow userspace to distinguish
between them.

Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
 Documentation/media/uapi/v4l/open.rst            | 7 +++++++
 Documentation/media/uapi/v4l/vidioc-querycap.rst | 5 +++++
 Documentation/media/videodev2.h.rst.exceptions   | 1 +
 include/uapi/linux/videodev2.h                   | 2 ++
 4 files changed, 15 insertions(+)

diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
index 64b1de047b1b..d0930fc170f0 100644
--- a/Documentation/media/uapi/v4l/open.rst
+++ b/Documentation/media/uapi/v4l/open.rst
@@ -46,6 +46,13 @@ the periferal can be used. For such devices, the sub-devices' configuration
 can be controlled via the :ref:`sub-device API <subdev>`, which creates one
 device node per sub-device.
 
+.. attention::
+
+   Devices that require **mc-centric** hardware peripheral control should
+   report a ``V4L2_MC_CENTRIC`` :c:type:`v4l2_capability` flag
+   (see :ref:`VIDIOC_QUERYCAP`).
+
+
 In summary, for **MC-centric** hardware peripheral control:
 
 - The **V4L2 device** node is responsible for controlling the streaming
diff --git a/Documentation/media/uapi/v4l/vidioc-querycap.rst b/Documentation/media/uapi/v4l/vidioc-querycap.rst
index 12e0d9a63cd8..2b08723375bc 100644
--- a/Documentation/media/uapi/v4l/vidioc-querycap.rst
+++ b/Documentation/media/uapi/v4l/vidioc-querycap.rst
@@ -252,6 +252,11 @@ specification the ioctl returns an ``EINVAL`` error code.
     * - ``V4L2_CAP_TOUCH``
       - 0x10000000
       - This is a touch device.
+    * - ``V4L2_MC_CENTRIC``
+      - 0x20000000
+      - Indicates that the device require **mc-centric** hardware
+        control, and thus can't be used by **v4l2-centric** applications.
+        See :ref:`v4l2_hardware_control` for more details.
     * - ``V4L2_CAP_DEVICE_CAPS``
       - 0x80000000
       - The driver fills the ``device_caps`` field. This capability can
diff --git a/Documentation/media/videodev2.h.rst.exceptions b/Documentation/media/videodev2.h.rst.exceptions
index a5cb0a8686ac..b51a575f9f75 100644
--- a/Documentation/media/videodev2.h.rst.exceptions
+++ b/Documentation/media/videodev2.h.rst.exceptions
@@ -157,6 +157,7 @@ replace define V4L2_CAP_META_CAPTURE device-capabilities
 replace define V4L2_CAP_READWRITE device-capabilities
 replace define V4L2_CAP_ASYNCIO device-capabilities
 replace define V4L2_CAP_STREAMING device-capabilities
+replace define V4L2_CAP_MC_CENTRIC device-capabilities
 replace define V4L2_CAP_DEVICE_CAPS device-capabilities
 replace define V4L2_CAP_TOUCH device-capabilities
 
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 45cf7359822c..7b490fe97980 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -460,6 +460,8 @@ struct v4l2_capability {
 
 #define V4L2_CAP_TOUCH                  0x10000000  /* Is a touch device */
 
+#define V4L2_CAP_MC_CENTRIC             0x20000000  /* Device require mc-centric hardware control */
+
 #define V4L2_CAP_DEVICE_CAPS            0x80000000  /* sets device capabilities field */
 
 /*
-- 
2.13.3

Re: [PATCH v4 6/7] media: videodev2: add a flag for MC-centric devices

[Hans Verkuil]

On 26/08/17 13:53, Mauro Carvalho Chehab wrote:
> As both vdev-centric and MC-centric devices may implement the
> same APIs, we need a flag to allow userspace to distinguish
> between them.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---
>  Documentation/media/uapi/v4l/open.rst            | 7 +++++++
>  Documentation/media/uapi/v4l/vidioc-querycap.rst | 5 +++++
>  Documentation/media/videodev2.h.rst.exceptions   | 1 +
>  include/uapi/linux/videodev2.h                   | 2 ++
>  4 files changed, 15 insertions(+)
> 
> diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
> index 64b1de047b1b..d0930fc170f0 100644
> --- a/Documentation/media/uapi/v4l/open.rst
> +++ b/Documentation/media/uapi/v4l/open.rst
> @@ -46,6 +46,13 @@ the periferal can be used. For such devices, the sub-devices' configuration
>  can be controlled via the :ref:`sub-device API <subdev>`, which creates one
>  device node per sub-device.
>  
> +.. attention::
> +
> +   Devices that require **mc-centric** hardware peripheral control should

MC-centric

As mentioned in my review of a previous patch I dislike the term "peripheral".
I think it should be dropped here.

> +   report a ``V4L2_MC_CENTRIC`` :c:type:`v4l2_capability` flag
> +   (see :ref:`VIDIOC_QUERYCAP`).
> +
> +
>  In summary, for **MC-centric** hardware peripheral control:
>  
>  - The **V4L2 device** node is responsible for controlling the streaming
> diff --git a/Documentation/media/uapi/v4l/vidioc-querycap.rst b/Documentation/media/uapi/v4l/vidioc-querycap.rst
> index 12e0d9a63cd8..2b08723375bc 100644
> --- a/Documentation/media/uapi/v4l/vidioc-querycap.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-querycap.rst
> @@ -252,6 +252,11 @@ specification the ioctl returns an ``EINVAL`` error code.
>      * - ``V4L2_CAP_TOUCH``
>        - 0x10000000
>        - This is a touch device.
> +    * - ``V4L2_MC_CENTRIC``
> +      - 0x20000000
> +      - Indicates that the device require **mc-centric** hardware

MC-centric

> +        control, and thus can't be used by **v4l2-centric** applications.

vdev-centric

TBD: I still think I prefer V4L2-centric over vdev-centric.

> +        See :ref:`v4l2_hardware_control` for more details.
>      * - ``V4L2_CAP_DEVICE_CAPS``
>        - 0x80000000
>        - The driver fills the ``device_caps`` field. This capability can
> diff --git a/Documentation/media/videodev2.h.rst.exceptions b/Documentation/media/videodev2.h.rst.exceptions
> index a5cb0a8686ac..b51a575f9f75 100644
> --- a/Documentation/media/videodev2.h.rst.exceptions
> +++ b/Documentation/media/videodev2.h.rst.exceptions
> @@ -157,6 +157,7 @@ replace define V4L2_CAP_META_CAPTURE device-capabilities
>  replace define V4L2_CAP_READWRITE device-capabilities
>  replace define V4L2_CAP_ASYNCIO device-capabilities
>  replace define V4L2_CAP_STREAMING device-capabilities
> +replace define V4L2_CAP_MC_CENTRIC device-capabilities
>  replace define V4L2_CAP_DEVICE_CAPS device-capabilities
>  replace define V4L2_CAP_TOUCH device-capabilities
>  
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 45cf7359822c..7b490fe97980 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -460,6 +460,8 @@ struct v4l2_capability {
>  
>  #define V4L2_CAP_TOUCH                  0x10000000  /* Is a touch device */
>  
> +#define V4L2_CAP_MC_CENTRIC             0x20000000  /* Device require mc-centric hardware control */

requires
MC-centric

> +
>  #define V4L2_CAP_DEVICE_CAPS            0x80000000  /* sets device capabilities field */
>  
>  /*
> 

Regards,

	Hans

V4L2 device node centric - Was: [PATCH v4 6/7] media: videodev2: add a flag for MC-centric devices

[Mauro Carvalho Chehab]

Em Mon, 28 Aug 2017 11:41:58 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> > +        control, and thus can't be used by **v4l2-centric** applications.  
> 
> vdev-centric
> 
> TBD: I still think I prefer V4L2-centric over vdev-centric.

I'm splitting it on a separate thread, to make easier for us to discuss.

For those that aren't tracking the patchset that are documenting those
terms, when MC was added, we got a hole new series of V4L2 devices that
are incompatible with standard V4L2 applications, as they require 
knowledge about the hardware sub-devices. We are referring to such
devices as MC-centric. We need another term to refer to the V4L2 devices
that can be used by a generic application, and are fully controlled via a 
V4L2 device (/dev/video*, /dev/radio*, /dev/swradio*, /dev/vbi*,
/dev/v4l-touch*).

The proposed documentation patch series solves this issue by
adding a glossary (patch 1) that defines what a "V4L2 device node"
as:

    V4L2 device node
	 A device node that it is associated to a V4L2 main driver,
	 as specified at :ref:`v4l2_device_naming`.

And, at the device naming chapter, at the spec (patch 2), it
explicitly lists all V4L2 device node names:

	.. _v4l2_device_naming:

	V4L2 Device Node Naming
	=======================

	... 
 
	The existing V4L2 device node types are:

	======================== ======================================================
	Default device node name Usage
	======================== ======================================================
	``/dev/videoX``		 Video input/output devices
	``/dev/vbiX``		 Vertical blank data (i.e. closed captions, teletext)
	``/dev/radioX``		 Radio tuners and modulators
	``/dev/swradioX``	 Software Defined Radio tuners and modulators
	``/dev/v4l-touchX``	 Touch sensors
	======================== ======================================================

So, the concept of "V4L2 Device Node" is now clear, and doesn't
include V4L2 sub-device device nodes (/dev/v4l-subdev*).

For devices controlled via media controller, everybody seems to be
comfortable of calling them as MC-centric.

There are currently two proposals to refer to the media hardware that
is controlled via a V4L2 Device Node:

	- vdev-centric
	- V4L2-centric

The last one sounds confusing to me, as subdev API is part of the V4L2
specification. "V4L2-centric" name sounds to include subdevs. 

That's why IMHO, vdev-centric is better.

We could go to some other naming for them, that would also be
an alias for "V4L2 Device Node":

	- VD-centric
	- VDN-centric

Comments?

Thanks,
Mauro

[PATCH v4 7/7] media: open.rst: add a notice about subdev-API on vdev-centric

[Mauro Carvalho Chehab]

The documentation doesn't mention if vdev-centric hardware
control would have subdev API or not.

Add a notice about that, reflecting the current status, where
three drivers use it, in order to support some subdev-specific
controls.

Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
 Documentation/media/uapi/v4l/open.rst | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
index d0930fc170f0..48f628bbabc7 100644
--- a/Documentation/media/uapi/v4l/open.rst
+++ b/Documentation/media/uapi/v4l/open.rst
@@ -46,6 +46,13 @@ the periferal can be used. For such devices, the sub-devices' configuration
 can be controlled via the :ref:`sub-device API <subdev>`, which creates one
 device node per sub-device.
 
+.. note::
+
+   A **vdev-centric** may also optionally expose V4L2 sub-devices via
+   :ref:`sub-device API <subdev>`. In that case, it has to implement
+   the :ref:`media controller API <media_controller>` as well.
+
+
 .. attention::
 
    Devices that require **mc-centric** hardware peripheral control should
-- 
2.13.3

Re: [PATCH v4 7/7] media: open.rst: add a notice about subdev-API on vdev-centric

[Hans Verkuil]

On 26/08/17 13:53, Mauro Carvalho Chehab wrote:
> The documentation doesn't mention if vdev-centric hardware
> control would have subdev API or not.
> 
> Add a notice about that, reflecting the current status, where
> three drivers use it, in order to support some subdev-specific
> controls.

I posted a patch removing v4l-subdevX support for cobalt. It's only used
within Cisco, so this is safe to do and won't break any userspace support.

atmel-isc is another driver that creates subdev nodes. Like cobalt, this
is unnecessary. There are no sensors that use private controls.

This driver is not referenced anywhere (dts or board file) in the kernel.
It is highly unlikely anyone would use v4l-subdevX nodes when there is no
need to do so. My suggestion is to add a kernel option for this driver
to enable v4l-subdevX support, but set it to 'default n'. Perhaps with
a note in the Kconfig description and a message in the kernel log that
this will be removed in the future.

The final driver is rcar_drif that uses this to set the "I2S Enable"
private control of the max2175 driver.

I remember that there was a long discussion over this control. I still
think that there is no need to mark this private. This is a recent
driver addition and we can get away with changing this, possibly using
a Kconfig option as well as discussed for the atmel-isc driver.

This is actually the only driver using a private control. I am of the
opinion that private controls were a mistake. I think it is the
bridge driver that has to decide whether or not to make subdev controls
available through a video node.

So in summary:

- drop is_private controls
- apply the cobalt patch (safe to do)
- add a Kconfig option for atmel-isc & rcar_drif that has to be explicitly
  enabled to support subdev nodes, and add logging that this is deprecated.
- by 4.17 or so remove this altogether.

If we agree to this, then this patch can be dropped.

Regards,

	Hans

> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---
>  Documentation/media/uapi/v4l/open.rst | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/media/uapi/v4l/open.rst b/Documentation/media/uapi/v4l/open.rst
> index d0930fc170f0..48f628bbabc7 100644
> --- a/Documentation/media/uapi/v4l/open.rst
> +++ b/Documentation/media/uapi/v4l/open.rst
> @@ -46,6 +46,13 @@ the periferal can be used. For such devices, the sub-devices' configuration
>  can be controlled via the :ref:`sub-device API <subdev>`, which creates one
>  device node per sub-device.
>  
> +.. note::
> +
> +   A **vdev-centric** may also optionally expose V4L2 sub-devices via
> +   :ref:`sub-device API <subdev>`. In that case, it has to implement
> +   the :ref:`media controller API <media_controller>` as well.
> +
> +
>  .. attention::
>  
>     Devices that require **mc-centric** hardware peripheral control should
> 

Re: [PATCH v4 7/7] media: open.rst: add a notice about subdev-API on vdev-centric

[Mauro Carvalho Chehab]

Em Mon, 28 Aug 2017 12:05:06 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 26/08/17 13:53, Mauro Carvalho Chehab wrote:
> > The documentation doesn't mention if vdev-centric hardware
> > control would have subdev API or not.
> > 
> > Add a notice about that, reflecting the current status, where
> > three drivers use it, in order to support some subdev-specific
> > controls.
> 
> I posted a patch removing v4l-subdevX support for cobalt. It's only used
> within Cisco, so this is safe to do and won't break any userspace support.

OK.

> atmel-isc is another driver that creates subdev nodes. Like cobalt, this
> is unnecessary. There are no sensors that use private controls.

The question is not if the driver has private controls. Private controls
can be V4L2 device node oriented.

The real question is if userspace applications use subdevs or not in
order to set something specific to a subdev, on a pipeline where
multiple subdevs could use the same control.

E. g. even on a simple case where the driver would have something like:

sensor -> processing -> DMA

both "sensor" and "processing" could provide the same control
(bright, contrast, gain, or whatever). Only by exposing such 
control via subdev is possible to pinpoint what part of the 
hardware pipeline would be affected when such control is changed.

> This driver is not referenced anywhere (dts or board file) in the kernel.
> It is highly unlikely anyone would use v4l-subdevX nodes when there is no
> need to do so. My suggestion is to add a kernel option for this driver
> to enable v4l-subdevX support, but set it to 'default n'. Perhaps with
> a note in the Kconfig description and a message in the kernel log that
> this will be removed in the future.
> 
> The final driver is rcar_drif that uses this to set the "I2S Enable"
> private control of the max2175 driver.
> 
> I remember that there was a long discussion over this control. I still
> think that there is no need to mark this private. 

The problem with I2S is that a device may have multiple places
where I2S could be used. I don't know how the rcar-drif driver uses
it, but there are several vdev-centric boards that use I2S for audio.

On several of the devices I worked with, the I2S can be enabled, in
runtime, if the audio signal would be directed to some digital output,
or it can be disabled if the audio signal would be directed to some
analog output. Thankfully, on those devices, I2S can be indirectly
controlled via either an ALSA mixer or via VIDIOC A/V routing
ioctls. Also, there's just one I2S bus on them.

However, on a device that have multiple I2S bus, userspace should
be able to control each of them individually, as some parts of the
pipeline may require it enabled while others may require it
disabled. So, I strongly believe that this should be a subdev
control on such hardware.

That's said, I don't know how rcar_drif uses it. If it has just
one I2S bus and it is used only for audio, then VIDIOC A/V routing
ioctls and/or an ALSA mixer could replace it. If not, then
it should be kept as-is and the driver would need to add support
for MC, in order for applications to identify the right
sub-devices that are associated with the pipelines where I2S
will be controlled.

Thanks,
Mauro

Re: [PATCH v4 7/7] media: open.rst: add a notice about subdev-API on vdev-centric

[Hans Verkuil]

On 28/08/17 12:30, Mauro Carvalho Chehab wrote:
> Em Mon, 28 Aug 2017 12:05:06 +0200
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
>> On 26/08/17 13:53, Mauro Carvalho Chehab wrote:
>>> The documentation doesn't mention if vdev-centric hardware
>>> control would have subdev API or not.
>>>
>>> Add a notice about that, reflecting the current status, where
>>> three drivers use it, in order to support some subdev-specific
>>> controls.
>>
>> I posted a patch removing v4l-subdevX support for cobalt. It's only used
>> within Cisco, so this is safe to do and won't break any userspace support.
> 
> OK.
> 
>> atmel-isc is another driver that creates subdev nodes. Like cobalt, this
>> is unnecessary. There are no sensors that use private controls.
> 
> The question is not if the driver has private controls. Private controls
> can be V4L2 device node oriented.
> 
> The real question is if userspace applications use subdevs or not in
> order to set something specific to a subdev, on a pipeline where
> multiple subdevs could use the same control.
> 
> E. g. even on a simple case where the driver would have something like:
> 
> sensor -> processing -> DMA
> 
> both "sensor" and "processing" could provide the same control
> (bright, contrast, gain, or whatever). Only by exposing such 
> control via subdev is possible to pinpoint what part of the 
> hardware pipeline would be affected when such control is changed.

In theory, yes. In practice this does not happen for any of the
V4L2-centric drivers. Including for the three drivers under discussion.

> 
>> This driver is not referenced anywhere (dts or board file) in the kernel.
>> It is highly unlikely anyone would use v4l-subdevX nodes when there is no
>> need to do so. My suggestion is to add a kernel option for this driver
>> to enable v4l-subdevX support, but set it to 'default n'. Perhaps with
>> a note in the Kconfig description and a message in the kernel log that
>> this will be removed in the future.
>>
>> The final driver is rcar_drif that uses this to set the "I2S Enable"
>> private control of the max2175 driver.
>>
>> I remember that there was a long discussion over this control. I still
>> think that there is no need to mark this private. 
> 
> The problem with I2S is that a device may have multiple places
> where I2S could be used. I don't know how the rcar-drif driver uses
> it, but there are several vdev-centric boards that use I2S for audio.
> 
> On several of the devices I worked with, the I2S can be enabled, in
> runtime, if the audio signal would be directed to some digital output,
> or it can be disabled if the audio signal would be directed to some
> analog output. Thankfully, on those devices, I2S can be indirectly
> controlled via either an ALSA mixer or via VIDIOC A/V routing
> ioctls. Also, there's just one I2S bus on them.
> 
> However, on a device that have multiple I2S bus, userspace should
> be able to control each of them individually, as some parts of the
> pipeline may require it enabled while others may require it
> disabled. So, I strongly believe that this should be a subdev
> control on such hardware.
> 
> That's said, I don't know how rcar_drif uses it. If it has just
> one I2S bus and it is used only for audio, then VIDIOC A/V routing
> ioctls and/or an ALSA mixer could replace it. If not, then
> it should be kept as-is and the driver would need to add support
> for MC, in order for applications to identify the right
> sub-devices that are associated with the pipelines where I2S
> will be controlled.

Ramesh, do applications using rcar_drif + max2175 have to manually
enable the i2s? Shouldn't this be part of the device tree description
instead?

Regards,

	Hans

RE: [PATCH v4 7/7] media: open.rst: add a notice about subdev-API on vdev-centric

[Ramesh Shanmugasundaram]

Hi Hans,

> On 28/08/17 12:30, Mauro Carvalho Chehab wrote:
> > Em Mon, 28 Aug 2017 12:05:06 +0200
> > Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> >
> >> On 26/08/17 13:53, Mauro Carvalho Chehab wrote:
> >>> The documentation doesn't mention if vdev-centric hardware control
> >>> would have subdev API or not.
> >>>
> >>> Add a notice about that, reflecting the current status, where three
> >>> drivers use it, in order to support some subdev-specific controls.
> >>
> >> I posted a patch removing v4l-subdevX support for cobalt. It's only
> >> used within Cisco, so this is safe to do and won't break any userspace
> support.
> >
> > OK.
> >
> >> atmel-isc is another driver that creates subdev nodes. Like cobalt,
> >> this is unnecessary. There are no sensors that use private controls.
> >
> > The question is not if the driver has private controls. Private
> > controls can be V4L2 device node oriented.
> >
> > The real question is if userspace applications use subdevs or not in
> > order to set something specific to a subdev, on a pipeline where
> > multiple subdevs could use the same control.
> >
> > E. g. even on a simple case where the driver would have something like:
> >
> > sensor -> processing -> DMA
> >
> > both "sensor" and "processing" could provide the same control (bright,
> > contrast, gain, or whatever). Only by exposing such control via subdev
> > is possible to pinpoint what part of the hardware pipeline would be
> > affected when such control is changed.
> 
> In theory, yes. In practice this does not happen for any of the V4L2-
> centric drivers. Including for the three drivers under discussion.
> 
> >
> >> This driver is not referenced anywhere (dts or board file) in the
> kernel.
> >> It is highly unlikely anyone would use v4l-subdevX nodes when there
> >> is no need to do so. My suggestion is to add a kernel option for this
> >> driver to enable v4l-subdevX support, but set it to 'default n'.
> >> Perhaps with a note in the Kconfig description and a message in the
> >> kernel log that this will be removed in the future.
> >>
> >> The final driver is rcar_drif that uses this to set the "I2S Enable"
> >> private control of the max2175 driver.
> >>
> >> I remember that there was a long discussion over this control. I
> >> still think that there is no need to mark this private.
> >
> > The problem with I2S is that a device may have multiple places where
> > I2S could be used. I don't know how the rcar-drif driver uses it, but
> > there are several vdev-centric boards that use I2S for audio.
> >
> > On several of the devices I worked with, the I2S can be enabled, in
> > runtime, if the audio signal would be directed to some digital output,
> > or it can be disabled if the audio signal would be directed to some
> > analog output. Thankfully, on those devices, I2S can be indirectly
> > controlled via either an ALSA mixer or via VIDIOC A/V routing ioctls.
> > Also, there's just one I2S bus on them.
> >
> > However, on a device that have multiple I2S bus, userspace should be
> > able to control each of them individually, as some parts of the
> > pipeline may require it enabled while others may require it disabled.
> > So, I strongly believe that this should be a subdev control on such
> > hardware.
> >
> > That's said, I don't know how rcar_drif uses it. If it has just one
> > I2S bus and it is used only for audio, then VIDIOC A/V routing ioctls
> > and/or an ALSA mixer could replace it. If not, then it should be kept
> > as-is and the driver would need to add support for MC, in order for
> > applications to identify the right sub-devices that are associated
> > with the pipelines where I2S will be controlled.
> 
> Ramesh, do applications using rcar_drif + max2175 have to manually enable
> the i2s? Shouldn't this be part of the device tree description instead?
> 

Yes, applications have to control this explicitly. It is not only enable but also disable control is used at run time and hence DT is not applicable. 

rcar_drif has two registers to write to enable rx on two data pins. It expects a sequence where the master stops output (in this max2175 i2s output - disable) - enable rcar_drif rx and then the master starts output (max2175 i2s output - enable). The application ensures this sequence today. It is one I2S bus and it is not used for audio but raw I/Q samples from max2175 tuner. 

The v4l2_subdev_tuner_ops does not have .s_stream api as in v4l2_subdev_video_ops and v4l2_subdev_audio_ops. If we plan to have one this functionality may be hidden inside it and no need for an explicit control. I too do not like a private control option.

Thanks,
Ramesh

Re: [PATCH v4 7/7] media: open.rst: add a notice about subdev-API on vdev-centric

[Hans Verkuil]

On 29/08/17 10:31, Ramesh Shanmugasundaram wrote:
> Hi Hans,
> 
>> On 28/08/17 12:30, Mauro Carvalho Chehab wrote:
>>> Em Mon, 28 Aug 2017 12:05:06 +0200
>>> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
>>>
>>>> On 26/08/17 13:53, Mauro Carvalho Chehab wrote:
>>>>> The documentation doesn't mention if vdev-centric hardware control
>>>>> would have subdev API or not.
>>>>>
>>>>> Add a notice about that, reflecting the current status, where three
>>>>> drivers use it, in order to support some subdev-specific controls.
>>>>
>>>> I posted a patch removing v4l-subdevX support for cobalt. It's only
>>>> used within Cisco, so this is safe to do and won't break any userspace
>> support.
>>>
>>> OK.
>>>
>>>> atmel-isc is another driver that creates subdev nodes. Like cobalt,
>>>> this is unnecessary. There are no sensors that use private controls.
>>>
>>> The question is not if the driver has private controls. Private
>>> controls can be V4L2 device node oriented.
>>>
>>> The real question is if userspace applications use subdevs or not in
>>> order to set something specific to a subdev, on a pipeline where
>>> multiple subdevs could use the same control.
>>>
>>> E. g. even on a simple case where the driver would have something like:
>>>
>>> sensor -> processing -> DMA
>>>
>>> both "sensor" and "processing" could provide the same control (bright,
>>> contrast, gain, or whatever). Only by exposing such control via subdev
>>> is possible to pinpoint what part of the hardware pipeline would be
>>> affected when such control is changed.
>>
>> In theory, yes. In practice this does not happen for any of the V4L2-
>> centric drivers. Including for the three drivers under discussion.
>>
>>>
>>>> This driver is not referenced anywhere (dts or board file) in the
>> kernel.
>>>> It is highly unlikely anyone would use v4l-subdevX nodes when there
>>>> is no need to do so. My suggestion is to add a kernel option for this
>>>> driver to enable v4l-subdevX support, but set it to 'default n'.
>>>> Perhaps with a note in the Kconfig description and a message in the
>>>> kernel log that this will be removed in the future.
>>>>
>>>> The final driver is rcar_drif that uses this to set the "I2S Enable"
>>>> private control of the max2175 driver.
>>>>
>>>> I remember that there was a long discussion over this control. I
>>>> still think that there is no need to mark this private.
>>>
>>> The problem with I2S is that a device may have multiple places where
>>> I2S could be used. I don't know how the rcar-drif driver uses it, but
>>> there are several vdev-centric boards that use I2S for audio.
>>>
>>> On several of the devices I worked with, the I2S can be enabled, in
>>> runtime, if the audio signal would be directed to some digital output,
>>> or it can be disabled if the audio signal would be directed to some
>>> analog output. Thankfully, on those devices, I2S can be indirectly
>>> controlled via either an ALSA mixer or via VIDIOC A/V routing ioctls.
>>> Also, there's just one I2S bus on them.
>>>
>>> However, on a device that have multiple I2S bus, userspace should be
>>> able to control each of them individually, as some parts of the
>>> pipeline may require it enabled while others may require it disabled.
>>> So, I strongly believe that this should be a subdev control on such
>>> hardware.
>>>
>>> That's said, I don't know how rcar_drif uses it. If it has just one
>>> I2S bus and it is used only for audio, then VIDIOC A/V routing ioctls
>>> and/or an ALSA mixer could replace it. If not, then it should be kept
>>> as-is and the driver would need to add support for MC, in order for
>>> applications to identify the right sub-devices that are associated
>>> with the pipelines where I2S will be controlled.
>>
>> Ramesh, do applications using rcar_drif + max2175 have to manually enable
>> the i2s? Shouldn't this be part of the device tree description instead?
>>
> 
> Yes, applications have to control this explicitly. It is not only enable but also disable control is used at run time and hence DT is not applicable. 
> 
> rcar_drif has two registers to write to enable rx on two data pins. It expects a sequence where the master stops output (in this max2175 i2s output - disable) - enable rcar_drif rx and then the master starts output (max2175 i2s output - enable). The application ensures this sequence today. It is one I2S bus and it is not used for audio but raw I/Q samples from max2175 tuner. 
> 
> The v4l2_subdev_tuner_ops does not have .s_stream api as in v4l2_subdev_video_ops and v4l2_subdev_audio_ops. If we plan to have one this functionality may be hidden inside it and no need for an explicit control. I too do not like a private control option.

I think it would be reasonable to use the audio ops s_stream for this. We're
streaming data after all. The audio ops most closely fits what we want to do.

All this is an internal API, so can be changed in the future if needed.

I like that a lot better than this weird control.

What do you think, Mauro?

Regards,

	Hans

Re: [PATCH v4 7/7] media: open.rst: add a notice about subdev-API on vdev-centric

[Mauro Carvalho Chehab]

Em Tue, 29 Aug 2017 10:39:52 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 29/08/17 10:31, Ramesh Shanmugasundaram wrote:
> > Hi Hans,
> >   
> >> On 28/08/17 12:30, Mauro Carvalho Chehab wrote:  
> >>> Em Mon, 28 Aug 2017 12:05:06 +0200
> >>> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> >>>  
> >>>> On 26/08/17 13:53, Mauro Carvalho Chehab wrote:  
> >>>>> The documentation doesn't mention if vdev-centric hardware control
> >>>>> would have subdev API or not.
> >>>>>
> >>>>> Add a notice about that, reflecting the current status, where three
> >>>>> drivers use it, in order to support some subdev-specific controls.  
> >>>>
> >>>> I posted a patch removing v4l-subdevX support for cobalt. It's only
> >>>> used within Cisco, so this is safe to do and won't break any userspace  
> >> support.  
> >>>
> >>> OK.
> >>>  
> >>>> atmel-isc is another driver that creates subdev nodes. Like cobalt,
> >>>> this is unnecessary. There are no sensors that use private controls.  
> >>>
> >>> The question is not if the driver has private controls. Private
> >>> controls can be V4L2 device node oriented.
> >>>
> >>> The real question is if userspace applications use subdevs or not in
> >>> order to set something specific to a subdev, on a pipeline where
> >>> multiple subdevs could use the same control.
> >>>
> >>> E. g. even on a simple case where the driver would have something like:
> >>>
> >>> sensor -> processing -> DMA
> >>>
> >>> both "sensor" and "processing" could provide the same control (bright,
> >>> contrast, gain, or whatever). Only by exposing such control via subdev
> >>> is possible to pinpoint what part of the hardware pipeline would be
> >>> affected when such control is changed.  
> >>
> >> In theory, yes. In practice this does not happen for any of the V4L2-
> >> centric drivers. Including for the three drivers under discussion.
> >>  
> >>>  
> >>>> This driver is not referenced anywhere (dts or board file) in the  
> >> kernel.  
> >>>> It is highly unlikely anyone would use v4l-subdevX nodes when there
> >>>> is no need to do so. My suggestion is to add a kernel option for this
> >>>> driver to enable v4l-subdevX support, but set it to 'default n'.
> >>>> Perhaps with a note in the Kconfig description and a message in the
> >>>> kernel log that this will be removed in the future.
> >>>>
> >>>> The final driver is rcar_drif that uses this to set the "I2S Enable"
> >>>> private control of the max2175 driver.
> >>>>
> >>>> I remember that there was a long discussion over this control. I
> >>>> still think that there is no need to mark this private.  
> >>>
> >>> The problem with I2S is that a device may have multiple places where
> >>> I2S could be used. I don't know how the rcar-drif driver uses it, but
> >>> there are several vdev-centric boards that use I2S for audio.
> >>>
> >>> On several of the devices I worked with, the I2S can be enabled, in
> >>> runtime, if the audio signal would be directed to some digital output,
> >>> or it can be disabled if the audio signal would be directed to some
> >>> analog output. Thankfully, on those devices, I2S can be indirectly
> >>> controlled via either an ALSA mixer or via VIDIOC A/V routing ioctls.
> >>> Also, there's just one I2S bus on them.
> >>>
> >>> However, on a device that have multiple I2S bus, userspace should be
> >>> able to control each of them individually, as some parts of the
> >>> pipeline may require it enabled while others may require it disabled.
> >>> So, I strongly believe that this should be a subdev control on such
> >>> hardware.
> >>>
> >>> That's said, I don't know how rcar_drif uses it. If it has just one
> >>> I2S bus and it is used only for audio, then VIDIOC A/V routing ioctls
> >>> and/or an ALSA mixer could replace it. If not, then it should be kept
> >>> as-is and the driver would need to add support for MC, in order for
> >>> applications to identify the right sub-devices that are associated
> >>> with the pipelines where I2S will be controlled.  
> >>
> >> Ramesh, do applications using rcar_drif + max2175 have to manually enable
> >> the i2s? Shouldn't this be part of the device tree description instead?
> >>  
> > 
> > Yes, applications have to control this explicitly. It is not only enable but also disable control is used at run time and hence DT is not applicable. 
> > 
> > rcar_drif has two registers to write to enable rx on two data pins. It expects a sequence where the master stops output (in this max2175 i2s output - disable) - enable rcar_drif rx and then the master starts output (max2175 i2s output - enable). The application ensures this sequence today. It is one I2S bus and it is not used for audio but raw I/Q samples from max2175 tuner. 
> > 
> > The v4l2_subdev_tuner_ops does not have .s_stream api as in v4l2_subdev_video_ops and v4l2_subdev_audio_ops. If we plan to have one this functionality may be hidden inside it and no need for an explicit control. I too do not like a private control option.  
> 
> I think it would be reasonable to use the audio ops s_stream for this. We're
> streaming data after all. The audio ops most closely fits what we want to do.
> 
> All this is an internal API, so can be changed in the future if needed.
> 
> I like that a lot better than this weird control.

Yeah, I'm all in favor of implicitly control it.

> 
> What do you think, Mauro?

Right now, we have .s_stream on both audio and video. I guess we could
just move it to v4l2_subdev_core_ops, as all subdevs may provide a
way to start streaming.

Are there any reason why we have separate s_stream callbacks
right now?

Thanks,
Mauro