[PATCH 4/4] [media] zr364xx: Fix a typo in a comment line of the file header

2017-08-28 Thread SF Markus Elfring
From: Markus Elfring 
Date: Mon, 28 Aug 2017 22:46:30 +0200

Fix a word in this description.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/zr364xx/zr364xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/zr364xx/zr364xx.c 
b/drivers/media/usb/zr364xx/zr364xx.c
index 4cc6d2a9d91f..4ccf71d8b608 100644
--- a/drivers/media/usb/zr364xx/zr364xx.c
+++ b/drivers/media/usb/zr364xx/zr364xx.c
@@ -2,7 +2,7 @@
  * Zoran 364xx based USB webcam module version 0.73
  *
  * Allows you to use your USB webcam with V4L2 applications
- * This is still in heavy developpement !
+ * This is still in heavy development!
  *
  * Copyright (C) 2004  Antoine Jacquet 
  * http://royale.zerezo.com/zr364xx/
-- 
2.14.1



[PATCH 3/4] [media] zr364xx: Adjust ten checks for null pointers

2017-08-28 Thread SF Markus Elfring
From: Markus Elfring 
Date: Mon, 28 Aug 2017 22:40:47 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The script “checkpatch.pl” pointed information out like the following.

Comparison to NULL could be written !…

Thus fix the affected source code places.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/zr364xx/zr364xx.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/media/usb/zr364xx/zr364xx.c 
b/drivers/media/usb/zr364xx/zr364xx.c
index 37cd6e20e68a..4cc6d2a9d91f 100644
--- a/drivers/media/usb/zr364xx/zr364xx.c
+++ b/drivers/media/usb/zr364xx/zr364xx.c
@@ -385,9 +385,9 @@ static int buffer_prepare(struct videobuf_queue *vq, struct 
videobuf_buffer *vb,
  vb);
int rc;
 
-   DBG("%s, field=%d, fmt name = %s\n", __func__, field, cam->fmt != NULL ?
-   cam->fmt->name : "");
-   if (cam->fmt == NULL)
+   DBG("%s, field=%d, fmt name = %s\n", __func__, field,
+   cam->fmt ? cam->fmt->name : "");
+   if (!cam->fmt)
return -EINVAL;
 
buf->vb.size = cam->width * cam->height * (cam->fmt->depth >> 3);
@@ -787,7 +787,7 @@ static int zr364xx_vidioc_try_fmt_vid_cap(struct file 
*file, void *priv,
struct zr364xx_camera *cam = video_drvdata(file);
char pixelformat_name[5];
 
-   if (cam == NULL)
+   if (!cam)
return -ENODEV;
 
if (f->fmt.pix.pixelformat != V4L2_PIX_FMT_JPEG) {
@@ -817,7 +817,7 @@ static int zr364xx_vidioc_g_fmt_vid_cap(struct file *file, 
void *priv,
 {
struct zr364xx_camera *cam;
 
-   if (file == NULL)
+   if (!file)
return -ENODEV;
cam = video_drvdata(file);
 
@@ -979,13 +979,13 @@ static void read_pipe_completion(struct urb *purb)
 
pipe_info = purb->context;
_DBG("%s %p, status %d\n", __func__, purb, purb->status);
-   if (pipe_info == NULL) {
+   if (!pipe_info) {
printk(KERN_ERR KBUILD_MODNAME ": no context!\n");
return;
}
 
cam = pipe_info->cam;
-   if (cam == NULL) {
+   if (!cam) {
printk(KERN_ERR KBUILD_MODNAME ": no context!\n");
return;
}
@@ -1069,7 +1069,7 @@ static void zr364xx_stop_readpipe(struct zr364xx_camera 
*cam)
 {
struct zr364xx_pipeinfo *pipe_info;
 
-   if (cam == NULL) {
+   if (!cam) {
printk(KERN_ERR KBUILD_MODNAME ": invalid device\n");
return;
}
@@ -1273,7 +1273,7 @@ static int zr364xx_mmap(struct file *file, struct 
vm_area_struct *vma)
struct zr364xx_camera *cam = video_drvdata(file);
int ret;
 
-   if (cam == NULL) {
+   if (!cam) {
DBG("%s: cam == NULL\n", __func__);
return -ENODEV;
}
@@ -1357,7 +1357,7 @@ static int zr364xx_board_init(struct zr364xx_camera *cam)
 
pipe->transfer_buffer = kzalloc(pipe->transfer_size,
GFP_KERNEL);
-   if (pipe->transfer_buffer == NULL) {
+   if (!pipe->transfer_buffer) {
DBG("out of memory!\n");
return -ENOMEM;
}
@@ -1373,7 +1373,7 @@ static int zr364xx_board_init(struct zr364xx_camera *cam)
DBG("valloc %p, idx %lu, pdata %p\n",
>buffer.frame[i], i,
cam->buffer.frame[i].lpvbits);
-   if (cam->buffer.frame[i].lpvbits == NULL) {
+   if (!cam->buffer.frame[i].lpvbits) {
printk(KERN_INFO KBUILD_MODNAME ": out of memory. Using 
less frames\n");
break;
}
-- 
2.14.1



[PATCH 2/4] [media] zr364xx: Improve a size determination in zr364xx_probe()

2017-08-28 Thread SF Markus Elfring
From: Markus Elfring 
Date: Mon, 28 Aug 2017 22:28:02 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/zr364xx/zr364xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/zr364xx/zr364xx.c 
b/drivers/media/usb/zr364xx/zr364xx.c
index 97af697dcc81..37cd6e20e68a 100644
--- a/drivers/media/usb/zr364xx/zr364xx.c
+++ b/drivers/media/usb/zr364xx/zr364xx.c
@@ -1421,7 +1421,7 @@ static int zr364xx_probe(struct usb_interface *intf,
 le16_to_cpu(udev->descriptor.idVendor),
 le16_to_cpu(udev->descriptor.idProduct));
 
-   cam = kzalloc(sizeof(struct zr364xx_camera), GFP_KERNEL);
+   cam = kzalloc(sizeof(*cam), GFP_KERNEL);
if (!cam)
return -ENOMEM;
 
-- 
2.14.1



[PATCH 1/4] [media] zr364xx: Delete an error message for a failed memory allocation in two functions

2017-08-28 Thread SF Markus Elfring
From: Markus Elfring 
Date: Mon, 28 Aug 2017 22:23:56 +0200

Omit an extra message for a memory allocation failure in these functions.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/zr364xx/zr364xx.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/zr364xx/zr364xx.c 
b/drivers/media/usb/zr364xx/zr364xx.c
index efdcd5bd6a4c..97af697dcc81 100644
--- a/drivers/media/usb/zr364xx/zr364xx.c
+++ b/drivers/media/usb/zr364xx/zr364xx.c
@@ -212,7 +212,5 @@ static int send_control_msg(struct usb_device *udev, u8 
request, u16 value,
-   if (!transfer_buffer) {
-   dev_err(>dev, "kmalloc(%d) failed\n", size);
+   if (!transfer_buffer)
return -ENOMEM;
-   }
 
memcpy(transfer_buffer, cp, size);
 
@@ -1427,7 +1425,5 @@ static int zr364xx_probe(struct usb_interface *intf,
-   if (cam == NULL) {
-   dev_err(>dev, "cam: out of memory !\n");
+   if (!cam)
return -ENOMEM;
-   }
 
cam->v4l2_dev.release = zr364xx_release;
err = v4l2_device_register(>dev, >v4l2_dev);
-- 
2.14.1



[PATCH 0/4] [media] zr364xx: Adjustments for some function implementations

2017-08-28 Thread SF Markus Elfring
From: Markus Elfring 
Date: Tue, 29 Aug 2017 07:17:07 +0200

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (4):
  Delete an error message for a failed memory allocation in two functions
  Improve a size determination in zr364xx_probe()
  Adjust ten checks for null pointers
  Fix a typo in a comment line of the file header

 drivers/media/usb/zr364xx/zr364xx.c | 34 +++---
 1 file changed, 15 insertions(+), 19 deletions(-)

-- 
2.14.1



cron job: media_tree daily build: ERRORS

2017-08-28 Thread Hans Verkuil
This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Tue Aug 29 05:00:15 CEST 2017
media-tree git hash:9a45bf28bc39ff6ed45a008f7201289c8e9e60a6
media_build git hash:   96c1c79a9847387da3e8f51c1230b3118eed3ea6
v4l-utils git hash: 5a6e0c38468c629f3f6f4fb988acebb9e66e2917
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.12.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: ERRORS
linux-2.6.37.6-i686: ERRORS
linux-2.6.38.8-i686: ERRORS
linux-2.6.39.4-i686: ERRORS
linux-3.0.60-i686: ERRORS
linux-3.1.10-i686: ERRORS
linux-3.2.37-i686: ERRORS
linux-3.3.8-i686: ERRORS
linux-3.4.27-i686: ERRORS
linux-3.5.7-i686: ERRORS
linux-3.6.11-i686: ERRORS
linux-3.7.4-i686: ERRORS
linux-3.8-i686: ERRORS
linux-3.9.2-i686: ERRORS
linux-3.10.1-i686: ERRORS
linux-3.11.1-i686: ERRORS
linux-3.12.67-i686: ERRORS
linux-3.13.11-i686: ERRORS
linux-3.14.9-i686: ERRORS
linux-3.15.2-i686: ERRORS
linux-3.16.7-i686: ERRORS
linux-3.17.8-i686: ERRORS
linux-3.18.7-i686: ERRORS
linux-3.19-i686: ERRORS
linux-4.0.9-i686: ERRORS
linux-4.1.33-i686: ERRORS
linux-4.2.8-i686: ERRORS
linux-4.3.6-i686: ERRORS
linux-4.4.22-i686: ERRORS
linux-4.5.7-i686: ERRORS
linux-4.6.7-i686: ERRORS
linux-4.7.5-i686: ERRORS
linux-4.8-i686: ERRORS
linux-4.9.26-i686: ERRORS
linux-4.10.14-i686: OK
linux-4.11-i686: OK
linux-4.12.1-i686: OK
linux-2.6.36.4-x86_64: ERRORS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-x86_64: ERRORS
linux-3.0.60-x86_64: ERRORS
linux-3.1.10-x86_64: ERRORS
linux-3.2.37-x86_64: ERRORS
linux-3.3.8-x86_64: ERRORS
linux-3.4.27-x86_64: ERRORS
linux-3.5.7-x86_64: ERRORS
linux-3.6.11-x86_64: ERRORS
linux-3.7.4-x86_64: ERRORS
linux-3.8-x86_64: ERRORS
linux-3.9.2-x86_64: ERRORS
linux-3.10.1-x86_64: ERRORS
linux-3.11.1-x86_64: ERRORS
linux-3.12.67-x86_64: ERRORS
linux-3.13.11-x86_64: ERRORS
linux-3.14.9-x86_64: ERRORS
linux-3.15.2-x86_64: ERRORS
linux-3.16.7-x86_64: ERRORS
linux-3.17.8-x86_64: ERRORS
linux-3.18.7-x86_64: ERRORS
linux-3.19-x86_64: ERRORS
linux-4.0.9-x86_64: ERRORS
linux-4.1.33-x86_64: ERRORS
linux-4.2.8-x86_64: ERRORS
linux-4.3.6-x86_64: ERRORS
linux-4.4.22-x86_64: ERRORS
linux-4.5.7-x86_64: ERRORS
linux-4.6.7-x86_64: ERRORS
linux-4.7.5-x86_64: ERRORS
linux-4.8-x86_64: ERRORS
linux-4.9.26-x86_64: ERRORS
linux-4.10.14-x86_64: WARNINGS
linux-4.11-x86_64: WARNINGS
linux-4.12.1-x86_64: WARNINGS
apps: WARNINGS
spec-git: OK

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Tuesday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html


[PATCH v2 1/2] media:imx274 device tree binding file

2017-08-28 Thread Soren Brinkmann
From: Leon Luo 

The binding file for imx274 CMOS sensor V4l2 driver

Signed-off-by: Leon Luo 
Acked-by: Sören Brinkmann 
---
 .../devicetree/bindings/media/i2c/imx274.txt   | 32 ++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/imx274.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/imx274.txt 
b/Documentation/devicetree/bindings/media/i2c/imx274.txt
new file mode 100644
index ..9154666d1149
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/imx274.txt
@@ -0,0 +1,32 @@
+* Sony 1/2.5-Inch 8.51Mp CMOS Digital Image Sensor
+
+The Sony imx274 is a 1/2.5-inch CMOS active pixel digital image sensor with
+an active array size of 3864H x 2202V. It is programmable through I2C
+interface. The I2C address is fixed to 0x1a as per sensor data sheet.
+Image data is sent through MIPI CSI-2, which is configured as 4 lanes
+at 1440 Mbps.
+
+
+Required Properties:
+- compatible: value should be "sony,imx274" for imx274 sensor
+
+Optional Properties:
+- reset-gpios: Sensor reset GPIO
+
+For further reading on port node refer to
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example:
+   imx274: sensor@1a{
+   compatible = "sony,imx274";
+   reg = <0x1a>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reset-gpios = <_sensor 0 0>;
+   port@0 {
+   reg = <0>;
+   sensor_out: endpoint {
+   remote-endpoint = <_in>;
+   };
+   };
+   };
-- 
2.14.1.3.g5766cf452



[PATCH v2 2/2] media:imx274 V4l2 driver for Sony imx274 CMOS sensor

2017-08-28 Thread Soren Brinkmann
From: Leon Luo 

The imx274 is a Sony CMOS image sensor that has 1/2.5 image size.
It supports up to 3840x2160 (4K) 60fps, 1080p 120fps. The interface
is 4-lane MIPI running at 1.44Gbps each.

This driver has been tested on Xilinx ZCU102 platform with a Leopard
LI-IMX274MIPI-FMC camera board.

Support for the following features:
-Resolutions: 3840x2160, 1920x1080, 1280x720
-Frame rate: 3840x2160 : 5 – 60fps
1920x1080 : 5 – 120fps
1280x720 : 5 – 120fps
-Exposure time: 16 – (frame interval) micro-seconds
-Gain: 1x - 180x
-VFLIP: enable/disable
-Test pattern: 12 test patterns

Signed-off-by: Leon Luo 
Tested-by: Sören Brinkmann 
---
v2:
 - Fix Kconfig to not remove existing options
---
 drivers/media/i2c/Kconfig  |7 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/imx274.c | 1842 
 3 files changed, 1850 insertions(+)
 create mode 100644 drivers/media/i2c/imx274.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 94153895fcd4..ad2e70a02363 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -547,6 +547,13 @@ config VIDEO_APTINA_PLL
 config VIDEO_SMIAPP_PLL
tristate
 
+config VIDEO_IMX274
+   tristate "Sony IMX274 sensor support"
+   depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+   ---help---
+ This is a V4L2 sensor-level driver for the Sony IMX274
+ CMOS image sensor.
+
 config VIDEO_OV2640
tristate "OmniVision OV2640 sensor support"
depends on VIDEO_V4L2 && I2C
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index c843c181dfb9..f8d57e453936 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -92,5 +92,6 @@ obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
 obj-$(CONFIG_VIDEO_ML86V7667)  += ml86v7667.o
 obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
 obj-$(CONFIG_VIDEO_TC358743)   += tc358743.o
+obj-$(CONFIG_VIDEO_IMX274) += imx274.o
 
 obj-$(CONFIG_SDR_MAX2175) += max2175.o
diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
new file mode 100644
index ..fcbb5ad2763c
--- /dev/null
+++ b/drivers/media/i2c/imx274.c
@@ -0,0 +1,1842 @@
+/*
+ * imx274.c - IMX274 CMOS Image Sensor driver
+ *
+ * Copyright (C) 2017, Leopard Imaging, Inc.
+ *
+ * Leon Luo 
+ * Edwin Zou 
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static int debug;
+module_param(debug, int, 0644);
+MODULE_PARM_DESC(debug, "Debug level (0-2)");
+
+/*
+ * See "SHR, SVR Setting" in datasheet
+ */
+#define IMX274_DEFAULT_FRAME_LENGTH(4550)
+#define IMX274_MAX_FRAME_LENGTH(0x000f)
+
+/*
+ * See "Frame Rate Adjustment" in datasheet
+ */
+#define IMX274_PIXCLK_CONST1   (7200)
+#define IMX274_PIXCLK_CONST2   (100)
+
+/*
+ * The input gain is shifted by IMX274_GAIN_SHIFT to get
+ * decimal number. The real gain is
+ * (float)input_gain_value / (1 << IMX274_GAIN_SHIFT)
+ */
+#define IMX274_GAIN_SHIFT  (8)
+#define IMX274_GAIN_SHIFT_MASK ((1 << IMX274_GAIN_SHIFT) - 1)
+
+/*
+ * See "Analog Gain" and "Digital Gain" in datasheet
+ * min gain is 1X
+ * max gain is calculated based on IMX274_GAIN_REG_MAX
+ */
+#define IMX274_GAIN_REG_MAX(1957)
+#define IMX274_MIN_GAIN(0x01 << 
IMX274_GAIN_SHIFT)
+#define IMX274_MAX_ANALOG_GAIN ((2048 << IMX274_GAIN_SHIFT)\
+   / (2048 - IMX274_GAIN_REG_MAX))
+#define IMX274_MAX_DIGITAL_GAIN(8)
+#define IMX274_DEF_GAIN(20 << 
IMX274_GAIN_SHIFT)
+#define IMX274_GAIN_CONST  (2048) /* for gain formula */
+
+/*
+ * 1 line time in us = (HMAX / 72), minimal is 4 lines
+ */
+#define IMX274_MIN_EXPOSURE_TIME   (4 * 260 / 72)
+
+#define IMX274_DEFAULT_MODEIMX274_MODE_3840X2160
+#define 

[PATCH 2/2] media: dvb-core: fix demux.h non-ASCII characters

2017-08-28 Thread Randy Dunlap
From: Randy Dunlap 

Fix non-ASCII charactes in kernel-doc comment to prevent the kernel-doc
build warning below.

WARNING: kernel-doc '../scripts/kernel-doc -rst -enable-lineno 
../drivers/media/dvb-core/demux.h' processing failed with: 'ascii' codec can't 
decode byte 0xe2 in position 6368: ordinal not in range(128)

Signed-off-by: Randy Dunlap 
---
 drivers/media/dvb-core/demux.h |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- lnx-413-rc7.orig/drivers/media/dvb-core/demux.h
+++ lnx-413-rc7/drivers/media/dvb-core/demux.h
@@ -210,7 +210,7 @@ struct dmx_section_feed {
  * the start of the first undelivered TS packet within a circular buffer.
  * The @buffer2 buffer parameter is normally NULL, except when the received
  * TS packets have crossed the last address of the circular buffer and
- * ”wrapped” to the beginning of the buffer. In the latter case the @buffer1
+ * "wrapped" to the beginning of the buffer. In the latter case the @buffer1
  * parameter would contain an address within the circular buffer, while the
  * @buffer2 parameter would contain the first address of the circular buffer.
  * The number of bytes delivered with this function (i.e. @buffer1_length +




[PATCH 1/2] docs: kernel-doc comments are ASCII

2017-08-28 Thread Randy Dunlap
From: Randy Dunlap 

kernel-doc parsing uses as ASCII codec, so let people know that
kernel-doc comments should be in ASCII characters only.

WARNING: kernel-doc '../scripts/kernel-doc -rst -enable-lineno 
../drivers/media/dvb-core/demux.h' processing failed with: 'ascii' codec can't 
decode byte 0xe2 in position 6368: ordinal not in range(128)

Signed-off-by: Randy Dunlap 
---
 Documentation/doc-guide/kernel-doc.rst |3 +++
 1 file changed, 3 insertions(+)

--- lnx-413-rc7.orig/Documentation/doc-guide/kernel-doc.rst
+++ lnx-413-rc7/Documentation/doc-guide/kernel-doc.rst
@@ -108,6 +108,9 @@ The function and type kernel-doc comment
 function or type being described. The overview kernel-doc comments may be 
freely
 placed at the top indentation level.
 
+.. attention:: kernel-doc comments should be written **only** in ASCII
+  characters since they are processed as ASCII input.
+
 Example kernel-doc function comment::
 
   /**




Re: DRM Format Modifiers in v4l2

2017-08-28 Thread Daniel Vetter
On Mon, Aug 28, 2017 at 8:07 PM, Nicolas Dufresne  wrote:
> Le jeudi 24 août 2017 à 13:26 +0100, Brian Starkey a écrit :
>> > What I mean was: an application can use the modifier to give buffers from
>> > one device to another without needing to understand it.
>> >
>> > But a generic video capture application that processes the video itself
>> > cannot be expected to know about the modifiers. It's a custom HW specific
>> > format that you only use between two HW devices or with software written
>> > for that hardware.
>> >
>>
>> Yes, makes sense.
>>
>> > >
>> > > However, in DRM the API lets you get the supported formats for each
>> > > modifier as-well-as the modifier list itself. I'm not sure how exactly
>> > > to provide that in a control.
>> >
>> > We have support for a 'menu' of 64 bit integers: 
>> > V4L2_CTRL_TYPE_INTEGER_MENU.
>> > You use VIDIOC_QUERYMENU to enumerate the available modifiers.
>> >
>> > So enumerating these modifiers would work out-of-the-box.
>>
>> Right. So I guess the supported set of formats could be somehow
>> enumerated in the menu item string. In DRM the pairs are (modifier +
>> bitmask) where bits represent formats in the supported formats list
>> (commit db1689aa61bd in drm-next). Printing a hex representation of
>> the bitmask would be functional but I concede not very pretty.
>
> The problem is that the list of modifiers depends on the format
> selected. Having to call S_FMT to obtain this list is quite
> inefficient.
>
> Also, be aware that DRM_FORMAT_MOD_SAMSUNG_64_32_TILE modifier has been
> implemented in V4L2 with a direct format (V4L2_PIX_FMT_NV12MT). I think
> an other one made it the same way recently, something from Mediatek if
> I remember. Though, unlike the Intel one, the same modifier does not
> have various result depending on the hardware revision.

Note on the intel modifers: On most recent platforms (iirc gen9) the
modifier is well defined and always describes the same byte layout. We
simply didn't want to rewrite our entire software stack for all the
old gunk platforms, hence the language. I guess we could/should
describe the layout in detail, but atm we're the only ones using it.

On your topic of v4l2 encoding the drm fourcc+modifier combo into a
special v4l fourcc: That's exactly the mismatch I was thinking of.
There's other examples of v4l2 fourcc being more specific than their
drm counters (e.g. specific way the different planes are laid out).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch


Re: [PATCH v2] [media] pci: make video_device const

2017-08-28 Thread Ismael Luceno
On 26/Aug/2017 18:38, Bhumika Goyal wrote:
> Make these const as they are either used during a copy operation or
> passed to a const argument of the function cx88_vdev_init.
> 
> Signed-off-by: Bhumika Goyal 
> ---
> * Combine the patch series sent for drivers/media/pci/ into a
> single patch.
> 
>  drivers/media/pci/cx88/cx88-blackbird.c | 2 +-
>  drivers/media/pci/dt3155/dt3155.c   | 2 +-
>  drivers/media/pci/meye/meye.c   | 2 +-
>  drivers/media/pci/saa7134/saa7134-empress.c | 2 +-
>  drivers/media/pci/solo6x10/solo6x10-v4l2.c  | 2 +-
>  drivers/media/pci/sta2x11/sta2x11_vip.c | 2 +-
>  drivers/media/pci/tw68/tw68-video.c | 2 +-
>  7 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/pci/cx88/cx88-blackbird.c 
> b/drivers/media/pci/cx88/cx88-blackbird.c
> index aa49c95..e3101f0 100644
> --- a/drivers/media/pci/cx88/cx88-blackbird.c
> +++ b/drivers/media/pci/cx88/cx88-blackbird.c
> @@ -1075,7 +1075,7 @@ static int vidioc_s_std(struct file *file, void *priv, 
> v4l2_std_id id)
>   .vidioc_unsubscribe_event= v4l2_event_unsubscribe,
>  };
>  
> -static struct video_device cx8802_mpeg_template = {
> +static const struct video_device cx8802_mpeg_template = {
>   .name = "cx8802",
>   .fops = _fops,
>   .ioctl_ops= _ioctl_ops,
> diff --git a/drivers/media/pci/dt3155/dt3155.c 
> b/drivers/media/pci/dt3155/dt3155.c
> index 6a21969..1775c36 100644
> --- a/drivers/media/pci/dt3155/dt3155.c
> +++ b/drivers/media/pci/dt3155/dt3155.c
> @@ -499,7 +499,7 @@ static int dt3155_init_board(struct dt3155_priv *pd)
>   return 0;
>  }
>  
> -static struct video_device dt3155_vdev = {
> +static const struct video_device dt3155_vdev = {
>   .name = DT3155_NAME,
>   .fops = _fops,
>   .ioctl_ops = _ioctl_ops,
> diff --git a/drivers/media/pci/meye/meye.c b/drivers/media/pci/meye/meye.c
> index 0fe76be..49e047e 100644
> --- a/drivers/media/pci/meye/meye.c
> +++ b/drivers/media/pci/meye/meye.c
> @@ -1533,7 +1533,7 @@ static int meye_mmap(struct file *file, struct 
> vm_area_struct *vma)
>   .vidioc_default = vidioc_default,
>  };
>  
> -static struct video_device meye_template = {
> +static const struct video_device meye_template = {
>   .name   = "meye",
>   .fops   = _fops,
>   .ioctl_ops  = _ioctl_ops,
> diff --git a/drivers/media/pci/saa7134/saa7134-empress.c 
> b/drivers/media/pci/saa7134/saa7134-empress.c
> index b1d3648..66acfd3 100644
> --- a/drivers/media/pci/saa7134/saa7134-empress.c
> +++ b/drivers/media/pci/saa7134/saa7134-empress.c
> @@ -205,7 +205,7 @@ static int empress_try_fmt_vid_cap(struct file *file, 
> void *priv,
>  
>  /* --- */
>  
> -static struct video_device saa7134_empress_template = {
> +static const struct video_device saa7134_empress_template = {
>   .name  = "saa7134-empress",
>   .fops  = _fops,
>   .ioctl_ops = _ioctl_ops,
> diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2.c 
> b/drivers/media/pci/solo6x10/solo6x10-v4l2.c
> index 3266fc2..99ffd1e 100644
> --- a/drivers/media/pci/solo6x10/solo6x10-v4l2.c
> +++ b/drivers/media/pci/solo6x10/solo6x10-v4l2.c
> @@ -630,7 +630,7 @@ static int solo_s_ctrl(struct v4l2_ctrl *ctrl)
>   .vidioc_unsubscribe_event   = v4l2_event_unsubscribe,
>  };
>  
> -static struct video_device solo_v4l2_template = {
> +static const struct video_device solo_v4l2_template = {
>   .name   = SOLO6X10_NAME,
>   .fops   = _v4l2_fops,
>   .ioctl_ops  = _v4l2_ioctl_ops,
> diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c 
> b/drivers/media/pci/sta2x11/sta2x11_vip.c
> index 6343d24..eb5a9ea 100644
> --- a/drivers/media/pci/sta2x11/sta2x11_vip.c
> +++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
> @@ -754,7 +754,7 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void 
> *priv,
>   .vidioc_unsubscribe_event = v4l2_event_unsubscribe,
>  };
>  
> -static struct video_device video_dev_template = {
> +static const struct video_device video_dev_template = {
>   .name = KBUILD_MODNAME,
>   .release = video_device_release_empty,
>   .fops = _fops,
> diff --git a/drivers/media/pci/tw68/tw68-video.c 
> b/drivers/media/pci/tw68/tw68-video.c
> index 58c4dd7..8c1f4a0 100644
> --- a/drivers/media/pci/tw68/tw68-video.c
> +++ b/drivers/media/pci/tw68/tw68-video.c
> @@ -916,7 +916,7 @@ static int vidioc_s_register(struct file *file, void 
> *priv,
>  #endif
>  };
>  
> -static struct video_device tw68_video_template = {
> +static const struct video_device tw68_video_template = {
>   .name   = "tw68_video",
>   .fops   = _fops,
>   .ioctl_ops  = _ioctl_ops,
> -- 
> 1.9.1

Signed-off-by: Ismael Luceno 



Re: [PATCH 3/5] [media] solo6x10: make video_device const

2017-08-28 Thread Ismael Luceno
On 26/Aug/2017 16:43, Bhumika Goyal wrote:
> Make this const as it is only used in a copy operation.
> 
> Signed-off-by: Bhumika Goyal 
> ---
>  drivers/media/pci/solo6x10/solo6x10-v4l2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2.c 
> b/drivers/media/pci/solo6x10/solo6x10-v4l2.c
> index 3266fc2..99ffd1e 100644
> --- a/drivers/media/pci/solo6x10/solo6x10-v4l2.c
> +++ b/drivers/media/pci/solo6x10/solo6x10-v4l2.c
> @@ -630,7 +630,7 @@ static int solo_s_ctrl(struct v4l2_ctrl *ctrl)
>   .vidioc_unsubscribe_event   = v4l2_event_unsubscribe,
>  };
>  
> -static struct video_device solo_v4l2_template = {
> +static const struct video_device solo_v4l2_template = {
>   .name   = SOLO6X10_NAME,
>   .fops   = _v4l2_fops,
>   .ioctl_ops  = _v4l2_ioctl_ops,
> -- 
> 1.9.1
> 

Signed-off-by: Ismael Luceno 


Re: [PATCH 2/2] media:imx274 V4l2 driver for Sony imx274 CMOS sensor

2017-08-28 Thread Sören Brinkmann
On Mon, 2017-08-28 at 14:22:03 -0400, Nicolas Dufresne wrote:
> Le lundi 28 août 2017 à 08:15 -0700, Soren Brinkmann a écrit :
[...]
> > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > index 94153895fcd4..4e8b64575b2a 100644
> > --- a/drivers/media/i2c/Kconfig
> > +++ b/drivers/media/i2c/Kconfig
> > @@ -547,16 +547,12 @@ config VIDEO_APTINA_PLL
> >  config VIDEO_SMIAPP_PLL
> > tristate
> >  
> > -config VIDEO_OV2640
> > -   tristate "OmniVision OV2640 sensor support"
> > -   depends on VIDEO_V4L2 && I2C
> > -   depends on MEDIA_CAMERA_SUPPORT
> > -   help
> > - This is a Video4Linux2 sensor-level driver for the OmniVision
> > - OV2640 camera.
> > -
> > - To compile this driver as a module, choose M here: the
> > - module will be called ov2640.
> 
> Is this removal of another sensor intentional ?

Oops, no, some rebase gone wrong, I guess. I'll put that on the list to
fix for v2.

Thanks,
Sören


Re: [PATCH 2/2] media:imx274 V4l2 driver for Sony imx274 CMOS sensor

2017-08-28 Thread Nicolas Dufresne
Le lundi 28 août 2017 à 08:15 -0700, Soren Brinkmann a écrit :
> From: Leon Luo 
> 
> The imx274 is a Sony CMOS image sensor that has 1/2.5 image size.
> It supports up to 3840x2160 (4K) 60fps, 1080p 120fps. The interface
> is 4-lane MIPI running at 1.44Gbps each.
> 
> This driver has been tested on Xilinx ZCU102 platform with a Leopard
> LI-IMX274MIPI-FMC camera board.
> 
> Support for the following features:
> -Resolutions: 3840x2160, 1920x1080, 1280x720
> -Frame rate: 3840x2160 : 5 – 60fps
> 1920x1080 : 5 – 120fps
> 1280x720 : 5 – 120fps
> -Exposure time: 16 – (frame interval) micro-seconds
> -Gain: 1x - 180x
> -VFLIP: enable/disable
> -Test pattern: 12 test patterns
> 
> Signed-off-by: Leon Luo 
> Tested-by: Sören Brinkmann 
> ---
>  drivers/media/i2c/Kconfig  |   16 +-
>  drivers/media/i2c/Makefile |1 +
>  drivers/media/i2c/imx274.c | 1843 
> 
>  3 files changed, 1850 insertions(+), 10 deletions(-)
>  create mode 100644 drivers/media/i2c/imx274.c
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 94153895fcd4..4e8b64575b2a 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -547,16 +547,12 @@ config VIDEO_APTINA_PLL
>  config VIDEO_SMIAPP_PLL
>   tristate
>  
> -config VIDEO_OV2640
> - tristate "OmniVision OV2640 sensor support"
> - depends on VIDEO_V4L2 && I2C
> - depends on MEDIA_CAMERA_SUPPORT
> - help
> -   This is a Video4Linux2 sensor-level driver for the OmniVision
> -   OV2640 camera.
> -
> -   To compile this driver as a module, choose M here: the
> -   module will be called ov2640.

Is this removal of another sensor intentional ?

> +config VIDEO_IMX274
> + tristate "Sony IMX274 sensor support"
> + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> + ---help---
> +   This is a V4L2 sensor-level driver for the Sony IMX274
> +   CMOS image sensor.
>  
>  config VIDEO_OV2659
>   tristate "OmniVision OV2659 sensor support"
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index c843c181dfb9..f8d57e453936 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -92,5 +92,6 @@ obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
>  obj-$(CONFIG_VIDEO_ML86V7667)+= ml86v7667.o
>  obj-$(CONFIG_VIDEO_OV2659)   += ov2659.o
>  obj-$(CONFIG_VIDEO_TC358743) += tc358743.o
> +obj-$(CONFIG_VIDEO_IMX274)   += imx274.o
>  
>  obj-$(CONFIG_SDR_MAX2175) += max2175.o
> diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
> new file mode 100644
> index ..8b0a1316eadf
> --- /dev/null
> +++ b/drivers/media/i2c/imx274.c
> @@ -0,0 +1,1843 @@
> +/*
> + * imx274.c - IMX274 CMOS Image Sensor driver
> + *
> + * Copyright (C) 2017, Leopard Imaging, Inc.
> + *
> + * Leon Luo 
> + * Edwin Zou 
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see .
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static int debug;
> +module_param(debug, int, 0644);
> +MODULE_PARM_DESC(debug, "Debug level (0-2)");
> +
> +/*
> + * See "SHR, SVR Setting" in datasheet
> + */
> +#define IMX274_DEFAULT_FRAME_LENGTH  (4550)
> +#define IMX274_MAX_FRAME_LENGTH  (0x000f)
> +
> +/*
> + * See "Frame Rate Adjustment" in datasheet
> + */
> +#define IMX274_PIXCLK_CONST1 (7200)
> +#define IMX274_PIXCLK_CONST2 (100)
> +
> +/*
> + * The input gain is shifted by IMX274_GAIN_SHIFT to get
> + * decimal number. The real gain is
> + * (float)input_gain_value / (1 << IMX274_GAIN_SHIFT)
> + */
> +#define IMX274_GAIN_SHIFT(8)
> +#define IMX274_GAIN_SHIFT_MASK   ((1 << 
> IMX274_GAIN_SHIFT) - 1)
> +
> +/*
> + * See "Analog Gain" and "Digital Gain" in datasheet
> + * min gain is 1X
> + * max gain is calculated based on IMX274_GAIN_REG_MAX
> + */
> +#define IMX274_GAIN_REG_MAX  

Re: DRM Format Modifiers in v4l2

2017-08-28 Thread Nicolas Dufresne
Le jeudi 24 août 2017 à 13:26 +0100, Brian Starkey a écrit :
> > What I mean was: an application can use the modifier to give buffers from
> > one device to another without needing to understand it.
> > 
> > But a generic video capture application that processes the video itself
> > cannot be expected to know about the modifiers. It's a custom HW specific
> > format that you only use between two HW devices or with software written
> > for that hardware.
> > 
> 
> Yes, makes sense.
> 
> > > 
> > > However, in DRM the API lets you get the supported formats for each
> > > modifier as-well-as the modifier list itself. I'm not sure how exactly
> > > to provide that in a control.
> > 
> > We have support for a 'menu' of 64 bit integers: 
> > V4L2_CTRL_TYPE_INTEGER_MENU.
> > You use VIDIOC_QUERYMENU to enumerate the available modifiers.
> > 
> > So enumerating these modifiers would work out-of-the-box.
> 
> Right. So I guess the supported set of formats could be somehow
> enumerated in the menu item string. In DRM the pairs are (modifier +
> bitmask) where bits represent formats in the supported formats list
> (commit db1689aa61bd in drm-next). Printing a hex representation of
> the bitmask would be functional but I concede not very pretty.

The problem is that the list of modifiers depends on the format
selected. Having to call S_FMT to obtain this list is quite
inefficient.

Also, be aware that DRM_FORMAT_MOD_SAMSUNG_64_32_TILE modifier has been
implemented in V4L2 with a direct format (V4L2_PIX_FMT_NV12MT). I think
an other one made it the same way recently, something from Mediatek if
I remember. Though, unlike the Intel one, the same modifier does not
have various result depending on the hardware revision.

regards,
Nicolas



signature.asc
Description: This is a digitally signed message part


[PATCH] [media_build] update v4.7_dma_attrs.patch

2017-08-28 Thread Daniel Scheller
From: Daniel Scheller 

Fixes apply_patches wrt

  commit 5b6f9abe5a49 ("media: vb2: add bidirectional flag in vb2_queue")

Signed-off-by: Daniel Scheller 
Tested-by: Jasmin Jessich 
---
Tested and verified by Jasmin on 3.13, 3.4 and 2.6.36, and by me on 4.4.

 backports/v4.7_dma_attrs.patch | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/backports/v4.7_dma_attrs.patch b/backports/v4.7_dma_attrs.patch
index 28d8dbc..40a7e5b 100644
--- a/backports/v4.7_dma_attrs.patch
+++ b/backports/v4.7_dma_attrs.patch
@@ -294,18 +294,18 @@ index 9a144f2..c5e3113 100644
   *doesn't fill in the @alloc_devs array.
 - * @dma_attrs:DMA attributes to use for the DMA.
 + * @dma_attrs:DMA attributes to use for the DMA. May be NULL.
-  * @fileio_read_once: report EOF after reading the first buffer
-  * @fileio_write_immediately: queue buffer after each write() call
-  * @allow_zero_bytesused: allow bytesused == 0 to be passed to the driver
+  * @bidirectional: when this flag is set the DMA direction for the buffers of
+  *this queue will be overridden with DMA_BIDIRECTIONAL direction.
+  *This is useful in cases where the hardware (firmware) writes to
 @@ -494,7 +494,7 @@ struct vb2_queue {
unsigned inttype;
unsigned intio_modes;
struct device   *dev;
 -  unsigned long   dma_attrs;
 +  const struct dma_attrs  *dma_attrs;
+   unsignedbidirectional:1;
unsignedfileio_read_once:1;
unsignedfileio_write_immediately:1;
-   unsignedallow_zero_bytesused:1;
 diff --git a/include/media/videobuf2-dma-contig.h 
b/include/media/videobuf2-dma-contig.h
 index 5604818..df2aabe 100644
 --- a/include/media/videobuf2-dma-contig.h
-- 
2.13.5



Re: [3/7,media] dvb: don't use 'time_t' in event ioctl

2017-08-28 Thread Eugene Syromiatnikov
On Tue, Sep 15, 2015 at 05:49:04PM +0200, Arnd Bergmann wrote:
> 'struct video_event' is used for the VIDEO_GET_EVENT ioctl, implemented
> by drivers/media/pci/ivtv/ivtv-ioctl.c and
> drivers/media/pci/ttpci/av7110_av.c. The structure contains a 'time_t',
> which will be redefined in the future to be 64-bit wide, causing an
> incompatible ABI change for this ioctl.
> 
> As it turns out, neither of the drivers currently sets the timestamp
> field, and it is presumably useless anyway because of the limited
> resolutions (no sub-second times). This means we can simply change
> the structure definition to use a 'long' instead of 'time_t' and
> remain compatible with all existing user space binaries when time_t
> gets changed.
> 
> If anybody ever starts using this field, they have to make sure not
> to use 1970 based seconds in there, as those overflow in 2038.
> 
> Signed-off-by: Arnd Bergmann 
> ---
>  include/uapi/linux/dvb/video.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/dvb/video.h b/include/uapi/linux/dvb/video.h
> index d3d14a59d2d5..6c7f9298d7c2 100644
> --- a/include/uapi/linux/dvb/video.h
> +++ b/include/uapi/linux/dvb/video.h
> @@ -135,7 +135,8 @@ struct video_event {
>  #define VIDEO_EVENT_FRAME_RATE_CHANGED   2
>  #define VIDEO_EVENT_DECODER_STOPPED  3
>  #define VIDEO_EVENT_VSYNC4
> - __kernel_time_t timestamp;
> + /* unused, make sure to use atomic time for y2038 if it ever gets used 
> */
> + long timestamp;

This change breaks x32 ABI (and possibly MIPS n32 ABI), as __kernel_time_t
there is 64 bit already:
https://sourceforge.net/p/strace/mailman/message/36015326/

Note the change in structure size from 0x20 to 0x14 for VIDEO_GET_EVENT
command in linux/x32/ioctls_inc0.h.

>   union {
>   video_size_t size;
>   unsigned int frame_rate;/* in frames per 1000sec */


[PATCH 1/2] media:imx274 device tree binding file

2017-08-28 Thread Soren Brinkmann
From: Leon Luo 

The binding file for imx274 CMOS sensor V4l2 driver

Signed-off-by: Leon Luo 
Acked-by: Sören Brinkmann 
---
 .../devicetree/bindings/media/i2c/imx274.txt   | 32 ++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/imx274.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/imx274.txt 
b/Documentation/devicetree/bindings/media/i2c/imx274.txt
new file mode 100644
index ..9154666d1149
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/imx274.txt
@@ -0,0 +1,32 @@
+* Sony 1/2.5-Inch 8.51Mp CMOS Digital Image Sensor
+
+The Sony imx274 is a 1/2.5-inch CMOS active pixel digital image sensor with
+an active array size of 3864H x 2202V. It is programmable through I2C
+interface. The I2C address is fixed to 0x1a as per sensor data sheet.
+Image data is sent through MIPI CSI-2, which is configured as 4 lanes
+at 1440 Mbps.
+
+
+Required Properties:
+- compatible: value should be "sony,imx274" for imx274 sensor
+
+Optional Properties:
+- reset-gpios: Sensor reset GPIO
+
+For further reading on port node refer to
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example:
+   imx274: sensor@1a{
+   compatible = "sony,imx274";
+   reg = <0x1a>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reset-gpios = <_sensor 0 0>;
+   port@0 {
+   reg = <0>;
+   sensor_out: endpoint {
+   remote-endpoint = <_in>;
+   };
+   };
+   };
-- 
2.14.1.3.g5766cf452



[PATCH 2/2] media:imx274 V4l2 driver for Sony imx274 CMOS sensor

2017-08-28 Thread Soren Brinkmann
From: Leon Luo 

The imx274 is a Sony CMOS image sensor that has 1/2.5 image size.
It supports up to 3840x2160 (4K) 60fps, 1080p 120fps. The interface
is 4-lane MIPI running at 1.44Gbps each.

This driver has been tested on Xilinx ZCU102 platform with a Leopard
LI-IMX274MIPI-FMC camera board.

Support for the following features:
-Resolutions: 3840x2160, 1920x1080, 1280x720
-Frame rate: 3840x2160 : 5 – 60fps
1920x1080 : 5 – 120fps
1280x720 : 5 – 120fps
-Exposure time: 16 – (frame interval) micro-seconds
-Gain: 1x - 180x
-VFLIP: enable/disable
-Test pattern: 12 test patterns

Signed-off-by: Leon Luo 
Tested-by: Sören Brinkmann 
---
 drivers/media/i2c/Kconfig  |   16 +-
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/imx274.c | 1843 
 3 files changed, 1850 insertions(+), 10 deletions(-)
 create mode 100644 drivers/media/i2c/imx274.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 94153895fcd4..4e8b64575b2a 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -547,16 +547,12 @@ config VIDEO_APTINA_PLL
 config VIDEO_SMIAPP_PLL
tristate
 
-config VIDEO_OV2640
-   tristate "OmniVision OV2640 sensor support"
-   depends on VIDEO_V4L2 && I2C
-   depends on MEDIA_CAMERA_SUPPORT
-   help
- This is a Video4Linux2 sensor-level driver for the OmniVision
- OV2640 camera.
-
- To compile this driver as a module, choose M here: the
- module will be called ov2640.
+config VIDEO_IMX274
+   tristate "Sony IMX274 sensor support"
+   depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+   ---help---
+ This is a V4L2 sensor-level driver for the Sony IMX274
+ CMOS image sensor.
 
 config VIDEO_OV2659
tristate "OmniVision OV2659 sensor support"
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index c843c181dfb9..f8d57e453936 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -92,5 +92,6 @@ obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
 obj-$(CONFIG_VIDEO_ML86V7667)  += ml86v7667.o
 obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
 obj-$(CONFIG_VIDEO_TC358743)   += tc358743.o
+obj-$(CONFIG_VIDEO_IMX274) += imx274.o
 
 obj-$(CONFIG_SDR_MAX2175) += max2175.o
diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
new file mode 100644
index ..8b0a1316eadf
--- /dev/null
+++ b/drivers/media/i2c/imx274.c
@@ -0,0 +1,1843 @@
+/*
+ * imx274.c - IMX274 CMOS Image Sensor driver
+ *
+ * Copyright (C) 2017, Leopard Imaging, Inc.
+ *
+ * Leon Luo 
+ * Edwin Zou 
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static int debug;
+module_param(debug, int, 0644);
+MODULE_PARM_DESC(debug, "Debug level (0-2)");
+
+/*
+ * See "SHR, SVR Setting" in datasheet
+ */
+#define IMX274_DEFAULT_FRAME_LENGTH(4550)
+#define IMX274_MAX_FRAME_LENGTH(0x000f)
+
+/*
+ * See "Frame Rate Adjustment" in datasheet
+ */
+#define IMX274_PIXCLK_CONST1   (7200)
+#define IMX274_PIXCLK_CONST2   (100)
+
+/*
+ * The input gain is shifted by IMX274_GAIN_SHIFT to get
+ * decimal number. The real gain is
+ * (float)input_gain_value / (1 << IMX274_GAIN_SHIFT)
+ */
+#define IMX274_GAIN_SHIFT  (8)
+#define IMX274_GAIN_SHIFT_MASK ((1 << IMX274_GAIN_SHIFT) - 1)
+
+/*
+ * See "Analog Gain" and "Digital Gain" in datasheet
+ * min gain is 1X
+ * max gain is calculated based on IMX274_GAIN_REG_MAX
+ */
+#define IMX274_GAIN_REG_MAX(1957)
+#define IMX274_MIN_GAIN(0x01 << 
IMX274_GAIN_SHIFT)
+#define IMX274_MAX_ANALOG_GAIN ((2048 << IMX274_GAIN_SHIFT)\
+   / (2048 - IMX274_GAIN_REG_MAX))
+#define IMX274_MAX_DIGITAL_GAIN(8)
+#define IMX274_DEF_GAIN(20 << 
IMX274_GAIN_SHIFT)

[PATCH] Staging: bcm2048 fix bare use of 'unsigned' in radio-bcm2048.c

2017-08-28 Thread Branislav Radocaj
This is a patch to the radio-bcm2048.c file that fixes up
a warning found by the checkpatch.pl tool.

Removed unused 'size' argument from property_read macro.
In property_write macro, 'signal, size' is replaced by 'prop_type'.
This change implys the update of DEFINE_SYSFS_PROPERTY macro
and all places of its usage as well.

Signed-off-by: Branislav Radocaj 
---
 drivers/staging/media/bcm2048/radio-bcm2048.c | 60 +--
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
b/drivers/staging/media/bcm2048/radio-bcm2048.c
index 38f72d069e27..b1e664aeb6ab 100644
--- a/drivers/staging/media/bcm2048/radio-bcm2048.c
+++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
@@ -1965,7 +1965,7 @@ static ssize_t bcm2048_##prop##_write(struct device *dev, 
\
return err < 0 ? err : count;   \
 }
 
-#define property_read(prop, size, mask)
\
+#define property_read(prop, mask)  \
 static ssize_t bcm2048_##prop##_read(struct device *dev,   \
struct device_attribute *attr,  \
char *buf)  \
@@ -2000,9 +2000,9 @@ static ssize_t bcm2048_##prop##_read(struct device *dev,  
\
return sprintf(buf, mask "\n", value);  \
 }
 
-#define DEFINE_SYSFS_PROPERTY(prop, signal, size, mask, check) \
-property_write(prop, signal size, mask, check) \
-property_read(prop, size, mask)
+#define DEFINE_SYSFS_PROPERTY(prop, prop_type, mask, check)\
+property_write(prop, prop_type, mask, check)   \
+property_read(prop, mask)  \
 
 #define property_str_read(prop, size)  \
 static ssize_t bcm2048_##prop##_read(struct device *dev,   \
@@ -2028,39 +2028,39 @@ static ssize_t bcm2048_##prop##_read(struct device 
*dev,\
return count;   \
 }
 
-DEFINE_SYSFS_PROPERTY(power_state, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(mute, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(audio_route, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(dac_output, unsigned, int, "%u", 0)
-
-DEFINE_SYSFS_PROPERTY(fm_hi_lo_injection, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(fm_frequency, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(fm_af_frequency, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(fm_deemphasis, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(fm_rds_mask, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(fm_best_tune_mode, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(fm_search_rssi_threshold, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(fm_search_mode_direction, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(fm_search_tune_mode, unsigned, int, "%u", value > 3)
-
-DEFINE_SYSFS_PROPERTY(rds, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(rds_b_block_mask, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(rds_b_block_match, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(rds_pi_mask, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(rds_pi_match, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(rds_wline, unsigned, int, "%u", 0)
-property_read(rds_pi, unsigned int, "%x")
+DEFINE_SYSFS_PROPERTY(power_state, unsigned int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(mute, unsigned int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(audio_route, unsigned int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(dac_output, unsigned int, "%u", 0)
+
+DEFINE_SYSFS_PROPERTY(fm_hi_lo_injection, unsigned int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(fm_frequency, unsigned int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(fm_af_frequency, unsigned int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(fm_deemphasis, unsigned int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(fm_rds_mask, unsigned int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(fm_best_tune_mode, unsigned int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(fm_search_rssi_threshold, unsigned int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(fm_search_mode_direction, unsigned int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(fm_search_tune_mode, unsigned int, "%u", value > 3)
+
+DEFINE_SYSFS_PROPERTY(rds, unsigned int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(rds_b_block_mask, unsigned int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(rds_b_block_match, unsigned int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(rds_pi_mask, unsigned int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(rds_pi_match, unsigned int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(rds_wline, unsigned int, "%u", 0)
+property_read(rds_pi, "%x")
 property_str_read(rds_rt, (BCM2048_MAX_RDS_RT + 1))
 property_str_read(rds_ps, (BCM2048_MAX_RDS_PS + 1))
 
-property_read(fm_rds_flags, unsigned int, "%u")
+property_read(fm_rds_flags, "%u")
 property_str_read(rds_data, BCM2048_MAX_RDS_RADIO_TEXT * 5)
 
-property_read(region_bottom_frequency, unsigned int, "%u")

Re: [RFC 0/2] BCM283x Camera Receiver driver

2017-08-28 Thread Hans Verkuil
Hi Dave,

What is the status of this work? I ask because I tried to use this driver
plus my tc358743 on my rpi-2b without any luck. Specifically the tc358843
isn't able to read from the i2c bus.

This is probably a bug in my dts, if you have a tree somewhere containing
a working dts for this, then that would be very helpful.

Regards,

Hans

On 14/06/17 17:15, Dave Stevenson wrote:
> Hi All.
> 
> This is adding a V4L2 subdevice driver for the CSI2/CCP2 camera
> receiver peripheral on BCM283x, as used on Raspberry Pi.
> 
> v4l2-compliance results depend on the sensor subdevice this is
> connected to. It passes the basic tests cleanly with TC358743,
> but objects with OV5647
> fail: v4l2-test-controls.cpp(574): g_ext_ctrls does not support count == 0
> Neither OV5647 nor Unicam support any controls.
> 
> I must admit to not having got OV5647 to stream with the current driver
> register settings. It works with a set of register settings for VGA RAW10.
> I also have a couple of patches pending for OV5647, but would like to
> understand the issues better before sending them out.
> 
> Two queries I do have in V4L2-land:
> - When s_dv_timings or s_std is called, is the format meant to
>   be updated automatically? Even if we're already streaming?
>   Some existing drivers seem to, but others don't.
> - With s_fmt, is sizeimage settable by the application in the same
>   way as bytesperline? yavta allows you to specify it on the command
>   line, whilst v4l2-ctl doesn't. Some of the other parts of the Pi
>   firmware have a requirement that the buffer is a multiple of 16 lines
>   high, which can be matched by V4L2 if we can over-allocate the
>   buffers by the app specifying sizeimage. But if I allow that,
>   then I get a v4l2-compliance failure as the size doesn't get
>   reset when switching from RGB3 to UYVY as it takes the request as
>   a request to over-allocate.
> 
> Apologies if I've messed up in sending these patches - so many ways
> to do something.
> 
> Thanks in advance.
>   Dave
> 
> Dave Stevenson (2):
>   [media] dt-bindings: Document BCM283x CSI2/CCP2 receiver
>   [media] bcm2835-unicam: Driver for CCP2/CSI2 camera interface
> 
>  .../devicetree/bindings/media/bcm2835-unicam.txt   |   76 +
>  drivers/media/platform/Kconfig |1 +
>  drivers/media/platform/Makefile|2 +
>  drivers/media/platform/bcm2835/Kconfig |   14 +
>  drivers/media/platform/bcm2835/Makefile|3 +
>  drivers/media/platform/bcm2835/bcm2835-unicam.c| 2100 
> 
>  drivers/media/platform/bcm2835/vc4-regs-unicam.h   |  257 +++
>  7 files changed, 2453 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/bcm2835-unicam.txt
>  create mode 100644 drivers/media/platform/bcm2835/Kconfig
>  create mode 100644 drivers/media/platform/bcm2835/Makefile
>  create mode 100644 drivers/media/platform/bcm2835/bcm2835-unicam.c
>  create mode 100644 drivers/media/platform/bcm2835/vc4-regs-unicam.h
> 



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

2017-08-28 Thread Mauro Carvalho Chehab
Em Mon, 28 Aug 2017 11:41:58 +0200
Hans Verkuil  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 v5 1/7] media: add glossary.rst with a glossary of terms used at V4L2 spec

2017-08-28 Thread 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 
---
 Documentation/media/uapi/v4l/glossary.rst | 147 ++
 Documentation/media/uapi/v4l/v4l2.rst |   1 +
 2 files changed, 148 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 ..0b6ab5adec81
--- /dev/null
+++ b/Documentation/media/uapi/v4l/glossary.rst
@@ -0,0 +1,147 @@
+
+Glossary
+
+
+.. note::
+
+   This goal of section is to standardize the terms used within the V4L2
+   documentation. It is written incrementally as they are standardized in
+   the V4L2 documentation. So, it is a 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 in the file system used to control and do
+   input/output data transfers from/to a Kernel driver.
+
+Digital Signal Processor - DSP
+   A specialized microprocessor, with its architecture optimized for
+   the operational needs of digital signal processing.
+
+Driver
+   The part of the Linux Kernel that implements support
+   for a hardware component.
+
+Field-programmable Gate Array - FPGA
+   A field-programmable gate array (FPGA) is an integrated circuit
+   designed to be configured by a customer or a designer after
+   manufacturing.
+
+   See https://en.wikipedia.org/wiki/Field-programmable_gate_array.
+
+Hardware component
+   A subset of the media hardware. For example an I²C or SPI device,
+   or an IP block inside an SoC or FPGA.
+
+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 for a hardware peripheral supported by V4L2 drivers.
+
+   See :ref:`v4l2_hardware_control`.
+
+Inter-Integrated Circuit - I²C
+   A  multi-master, multi-slave, packet switched, single-ended,
+   serial computer bus used to control V4L2 sub-devices.
+
+   See http://www.nxp.com/docs/en/user-guide/UM10204.pdf.
+
+Integrated circuit - IC
+   A set of electronic circuits on one small flat piece of
+   semiconductor material, normally silicon.
+
+   Also known as chip.
+
+IP block
+   The same as IP core.
+
+Intelectual property core - IP core
+   In electronic design a semiconductor intellectual property core,
+   is a reusable unit of logic, cell, or integrated circuit layout
+   design that is the intellectual property of one party.
+   IP cores may be licensed to another party or can be owned
+   and used by a single party alone.
+
+   See 
https://en.wikipedia.org/wiki/Semiconductor_intellectual_property_core).
+
+Image processor - ISP
+   A specialized digital signal processor used for image processing
+   in digital cameras, mobile phones or other devices.
+
+Peripheral
+   The same as hardware peripheral.
+
+Media Controller
+   An API used to identify the hardware components and (optionally)
+   change the links between hardware components.
+
+   See :ref:`media_controller`.
+
+MC-centric
+   V4L2 hardware that requires a Media controller.
+
+   See :ref:`v4l2_hardware_control`.
+
+Microprocessor
+   An electronic circuitry that carries out the instructions
+   of a computer program by performing the basic arithmetic, logical,
+   control and input/output (I/O) operations specified by the
+   instructions on a single integrated circuit.
+
+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.
+
+System on a Chip - SoC
+   An integrated circuit that integrates all components of a computer
+   or other electronic systems.
+
+Sub-device hardware components
+   Hardware components that aren't controlled by the
+   V4L2 main driver.
+
+V4L2 device node
+   A device node that is associated to a V4L2 main driver,
+   as specified in :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 

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

2017-08-28 Thread 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 
---
 Documentation/media/uapi/v4l/open.rst | 40 +++
 1 file changed, 40 insertions(+)

diff --git a/Documentation/media/uapi/v4l/open.rst 
b/Documentation/media/uapi/v4l/open.rst
index 96ac972c1fa2..21b8f7c5ca55 100644
--- a/Documentation/media/uapi/v4l/open.rst
+++ b/Documentation/media/uapi/v4l/open.rst
@@ -7,6 +7,46 @@ 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`) with are responsible for implementing
+data streaming, if applicable.
+
+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**") control.
+
+For **vdev-centric** hardware peripheral control, the peripheral is
+controlled via the **V4L2 device nodes**. They may optionally support the
+:ref:`media controller API ` as well,
+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 ` before
+the periferal can be used. For such devices, the sub-devices' configuration
+can be controlled via the :ref:`sub-device API `, which creates one
+device node per sub-device.
+
 .. _v4l2_device_naming:
 
 V4L2 Device Node Naming
-- 
2.13.5



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

2017-08-28 Thread 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 
---
 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 275d934d2659..ceffc87d7ca3 100644
--- a/Documentation/media/uapi/v4l/open.rst
+++ b/Documentation/media/uapi/v4l/open.rst
@@ -47,6 +47,13 @@ the periferal can be used. For such devices, the 
sub-devices' configuration
 can be controlled via the :ref:`sub-device API `, which creates one
 device node per sub-device.
 
+.. note::
+
+   A **vdev-centric** may also optionally expose V4L2 sub-devices via
+   :ref:`sub-device API `. In that case, it has to implement
+   the :ref:`media controller API ` as well.
+
+
 .. attention::
 
Devices that require **MC-centric** hardware peripheral control should
-- 
2.13.5



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

2017-08-28 Thread 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.

v5:
- Added more terms to the glossary
- Adjusted some wording as proposed by Hans on a few patches
  and added his ack on others

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| 147 +++
 Documentation/media/uapi/v4l/open.rst| 114 +++---
 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, 256 insertions(+), 14 deletions(-)
 create mode 100644 Documentation/media/uapi/v4l/glossary.rst

-- 
2.13.5




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

2017-08-28 Thread 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 
---
 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 2575b6aea029..275d934d2659 100644
--- a/Documentation/media/uapi/v4l/open.rst
+++ b/Documentation/media/uapi/v4l/open.rst
@@ -47,6 +47,13 @@ the periferal can be used. For such devices, the 
sub-devices' configuration
 can be controlled via the :ref:`sub-device API `, 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`).
+
+
 .. _v4l2_device_naming:
 
 V4L2 Device Node Naming
diff --git a/Documentation/media/uapi/v4l/vidioc-querycap.rst 
b/Documentation/media/uapi/v4l/vidioc-querycap.rst
index 12e0d9a63cd8..3675f484b7ef 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``
   - 0x1000
   - This is a touch device.
+* - ``V4L2_MC_CENTRIC``
+  - 0x2000
+  - Indicates that the device require **MC-centric** hardware
+control, and thus can't be used by **vdev-centric** applications.
+See :ref:`v4l2_hardware_control` for more details.
 * - ``V4L2_CAP_DEVICE_CAPS``
   - 0x8000
   - 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..a35d762c3eef 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -460,6 +460,8 @@ struct v4l2_capability {
 
 #define V4L2_CAP_TOUCH  0x1000  /* Is a touch device */
 
+#define V4L2_CAP_MC_CENTRIC 0x2000  /* Device require 
MC-centric hardware control */
+
 #define V4L2_CAP_DEVICE_CAPS0x8000  /* sets device 
capabilities field */
 
 /*
-- 
2.13.5



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

2017-08-28 Thread 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 
Signed-off-by: Mauro Carvalho Chehab 
---
 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-: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.5



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

2017-08-28 Thread Mauro Carvalho Chehab
As we now have a glossary, some terms used on open.rst
require adjustments.

Acked-by: Hans Verkuil 
Signed-off-by: Mauro Carvalho Chehab 
---
 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 21b8f7c5ca55..2575b6aea029 100644
--- a/Documentation/media/uapi/v4l/open.rst
+++ b/Documentation/media/uapi/v4l/open.rst
@@ -138,7 +138,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
@@ -148,17 +148,17 @@ switching a device node between different functions only 
works when
 using the streaming I/O API, not with the
 :ref:`read() `/\ :ref:`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 `__ git
 repository), there is no library yet that can provide a single API
-- 
2.13.5



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

2017-08-28 Thread 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.

Acked-by: Hans Verkuil 
Signed-off-by: Mauro Carvalho Chehab 
---
 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.5



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

2017-08-28 Thread Mauro Carvalho Chehab
Em Mon, 28 Aug 2017 11:36:13 +0200
Hans Verkuil  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 
> > ---
> >  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 ` 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 ` before
> > 

[PATCH 3/3] [media] Siano: Adjust five checks for null pointers

2017-08-28 Thread SF Markus Elfring
From: Markus Elfring 
Date: Mon, 28 Aug 2017 12:50:28 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The script “checkpatch.pl” pointed information out like the following.

Comparison to NULL could be written !…

Thus fix the affected source code places.

Signed-off-by: Markus Elfring 
---
 drivers/media/common/siano/smscoreapi.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/media/common/siano/smscoreapi.c 
b/drivers/media/common/siano/smscoreapi.c
index ad1c41f727b1..e4ea2a0c7a24 100644
--- a/drivers/media/common/siano/smscoreapi.c
+++ b/drivers/media/common/siano/smscoreapi.c
@@ -749,7 +749,7 @@ static int smscore_sendrequest_and_wait(struct 
smscore_device_t *coredev,
void *buffer, size_t size, struct completion *completion) {
int rc;
 
-   if (completion == NULL)
+   if (!completion)
return -EINVAL;
init_completion(completion);
 
@@ -1151,8 +1151,8 @@ static int smscore_load_firmware_from_file(struct 
smscore_device_t *coredev,
}
pr_debug("Firmware name: %s\n", fw_filename);
 
-   if (loadfirmware_handler == NULL && !(coredev->device_flags
-   & SMS_DEVICE_FAMILY2))
+   if (!loadfirmware_handler &&
+   !(coredev->device_flags & SMS_DEVICE_FAMILY2))
return -EINVAL;
 
rc = request_firmware(, fw_filename, coredev->device);
@@ -1789,7 +1789,7 @@ int smsclient_sendrequest(struct smscore_client_t *client,
struct sms_msg_hdr *phdr = (struct sms_msg_hdr *) buffer;
int rc;
 
-   if (client == NULL) {
+   if (!client) {
pr_err("Got NULL client\n");
return -EINVAL;
}
@@ -1797,7 +1797,7 @@ int smsclient_sendrequest(struct smscore_client_t *client,
coredev = client->coredev;
 
/* check that no other channel with same id exists */
-   if (coredev == NULL) {
+   if (!coredev) {
pr_err("Got NULL coredev\n");
return -EINVAL;
}
@@ -1954,7 +1954,7 @@ int smscore_gpio_configure(struct smscore_device_t 
*coredev, u8 pin_num,
if (pin_num > MAX_GPIO_PIN_NUMBER)
return -EINVAL;
 
-   if (p_gpio_config == NULL)
+   if (!p_gpio_config)
return -EINVAL;
 
total_len = sizeof(struct sms_msg_hdr) + (sizeof(u32) * 6);
-- 
2.14.1



[PATCH 2/3] [media] Siano: Improve a size determination in six functions

2017-08-28 Thread SF Markus Elfring
From: Markus Elfring 
Date: Mon, 28 Aug 2017 12:38:39 +0200

Replace the specification of data structures by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/common/siano/smscoreapi.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/media/common/siano/smscoreapi.c 
b/drivers/media/common/siano/smscoreapi.c
index 889b486fbc72..ad1c41f727b1 100644
--- a/drivers/media/common/siano/smscoreapi.c
+++ b/drivers/media/common/siano/smscoreapi.c
@@ -447,5 +447,5 @@ static struct smscore_registry_entry_t 
*smscore_find_registry(char *devpath)
return entry;
}
}
-   entry = kmalloc(sizeof(struct smscore_registry_entry_t), GFP_KERNEL);
+   entry = kmalloc(sizeof(*entry), GFP_KERNEL);
if (entry) {
@@ -536,7 +536,5 @@ int smscore_register_hotplug(hotplug_t hotplug)
int rc = 0;
 
kmutex_lock(_smscore_deviceslock);
-
-   notifyee = kmalloc(sizeof(struct smscore_device_notifyee_t),
-  GFP_KERNEL);
+   notifyee = kmalloc(sizeof(*notifyee), GFP_KERNEL);
if (notifyee) {
@@ -627,5 +625,5 @@ smscore_buffer_t *smscore_createbuffer(u8 *buffer, void 
*common_buffer,
 {
struct smscore_buffer_t *cb;
 
-   cb = kzalloc(sizeof(struct smscore_buffer_t), GFP_KERNEL);
+   cb = kzalloc(sizeof(*cb), GFP_KERNEL);
if (!cb)
@@ -655,5 +653,5 @@ int smscore_register_device(struct smsdevice_params_t 
*params,
struct smscore_device_t *dev;
u8 *buffer;
 
-   dev = kzalloc(sizeof(struct smscore_device_t), GFP_KERNEL);
+   dev = kzalloc(sizeof(*dev), GFP_KERNEL);
if (!dev)
@@ -1684,5 +1682,5 @@ static int smscore_validate_client(struct 
smscore_device_t *coredev,
pr_err("The msg ID already registered to another client.\n");
return -EEXIST;
}
-   listentry = kzalloc(sizeof(struct smscore_idlist_t), GFP_KERNEL);
+   listentry = kzalloc(sizeof(*listentry), GFP_KERNEL);
if (!listentry)
@@ -1721,5 +1719,5 @@ int smscore_register_client(struct smscore_device_t 
*coredev,
return -EEXIST;
}
 
-   newclient = kzalloc(sizeof(struct smscore_client_t), GFP_KERNEL);
+   newclient = kzalloc(sizeof(*newclient), GFP_KERNEL);
if (!newclient)
-- 
2.14.1



[PATCH 1/3] [media] Siano: Delete an error message for a failed memory allocation in three functions

2017-08-28 Thread SF Markus Elfring
From: Markus Elfring 
Date: Mon, 28 Aug 2017 12:30:11 +0200

Omit an extra message for a memory allocation failure in these functions.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/common/siano/smscoreapi.c | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/media/common/siano/smscoreapi.c 
b/drivers/media/common/siano/smscoreapi.c
index e7a0d7798d5b..889b486fbc72 100644
--- a/drivers/media/common/siano/smscoreapi.c
+++ b/drivers/media/common/siano/smscoreapi.c
@@ -1304,7 +1304,5 @@ static int smscore_init_device(struct smscore_device_t 
*coredev, int mode)
-   if (!buffer) {
-   pr_err("Could not allocate buffer for init device message.\n");
+   if (!buffer)
return -ENOMEM;
-   }
 
msg = (struct sms_msg_data *)SMS_ALIGN_ADDRESS(buffer);
SMS_INIT_MSG(>x_msg_header, MSG_SMS_INIT_DEVICE_REQ,
@@ -1690,7 +1688,6 @@ static int smscore_validate_client(struct 
smscore_device_t *coredev,
-   if (!listentry) {
-   pr_err("Can't allocate memory for client id.\n");
+   if (!listentry)
return -ENOMEM;
-   }
+
listentry->id = id;
listentry->data_type = data_type;
list_add_locked(>entry, >idlist,
@@ -1728,7 +1725,5 @@ int smscore_register_client(struct smscore_device_t 
*coredev,
-   if (!newclient) {
-   pr_err("Failed to allocate memory for client.\n");
+   if (!newclient)
return -ENOMEM;
-   }
 
INIT_LIST_HEAD(>idlist);
newclient->coredev = coredev;
-- 
2.14.1



[PATCH 0/3] [media] Siano: Adjustments for some function implementations

2017-08-28 Thread SF Markus Elfring
From: Markus Elfring 
Date: Mon, 28 Aug 2017 12:55:43 +0200

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (3):
  Delete an error message for a failed memory allocation in three functions
  Improve a size determination in six functions
  Adjust five checks for null pointers

 drivers/media/common/siano/smscoreapi.c | 39 ++---
 1 file changed, 16 insertions(+), 23 deletions(-)

-- 
2.14.1



Re: [PATCH v2 1/3] dt: bindings: Document DT bindings for Analog devices as3645a

2017-08-28 Thread Pavel Machek
Hi!

> Thanks for the review!
> 
> On 08/28/17 13:33, Pavel Machek wrote:
> > Hi!
> > 
> >> +
> >> +Ranges below noted as [a, b] are closed ranges between a and b, i.e. a
> >> +and b are included in the range.
> > 
> > Normally I've seen  for closed ranges, (a, b) for open
> > ranges. Is that different in your country?
> 
> I guess there are different notations. :-) I've seen regular parentheses
> being used for open ranges, too, but not < and >.
> 
> Open range is documented in a related well written Wikipedia article:
> 
> 
> 
> Are there such open ranges in Czechia? For instance, reindeer generally
> roam freely in Finnish Lappland.

:-). Well, we have pigs and roes roaming freely in the woods, but
would not normally call it open range.

> What comes to the patch, I guess "interval" could be a more appropriate
> term to use in this case:
> 
> 
> 
> The patch is in line with the Wikipedia article in notation but not in
> terminology. I'll send a fix.

Ok, that was really nitpicking, thanks!
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature


Re: [PATCH v3 2/3] leds: as3645a: Add LED flash class driver

2017-08-28 Thread Pavel Machek
On Wed 2017-08-23 11:10:59, Sakari Ailus wrote:
> Add a LED flash class driver for the as3654a flash controller. A V4L2 flash
> driver for it already exists (drivers/media/i2c/as3645a.c), and this driver
> is based on that.

We do not want to have two drivers for same hardware... how is that
supposed to work?

Yes, we might want to have both LED and v4l2 interface for a single
LED, because v4l2 is just too hairy to use, but we should not
duplicate drivers for that.

We _might_ want to do some helpers; as these LED drivers are all quite
similar, it should be possible to have "flash led driver" and just
link it to v4l2 and LED interfaces...

Pavel


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature


Re: [PATCH v3 1/3] dt: bindings: Document DT bindings for Analog devices as3645a

2017-08-28 Thread Pavel Machek
Hi!

> +led-max-microamp: Maximum torch (assist) current in microamperes. The
> +   value must be in the range between [2, 16] and
> +   divisible by 2.
> +ams,input-max-microamp: Maximum flash controller input current. The

"in microamperes".

> + value must be in the range [125, 200]
> + and divisible by 5.

Is there any reason for "ams," prefix here?

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature


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

2017-08-28 Thread Hans Verkuil
On 28/08/17 12:30, Mauro Carvalho Chehab wrote:
> Em Mon, 28 Aug 2017 12:05:06 +0200
> Hans Verkuil  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 v2 1/3] dt: bindings: Document DT bindings for Analog devices as3645a

2017-08-28 Thread Sakari Ailus
Hi Pavel,

Thanks for the review!

On 08/28/17 13:33, Pavel Machek wrote:
> Hi!
> 
>> +
>> +Ranges below noted as [a, b] are closed ranges between a and b, i.e. a
>> +and b are included in the range.
> 
> Normally I've seen  for closed ranges, (a, b) for open
> ranges. Is that different in your country?

I guess there are different notations. :-) I've seen regular parentheses
being used for open ranges, too, but not < and >.

Open range is documented in a related well written Wikipedia article:



Are there such open ranges in Czechia? For instance, reindeer generally
roam freely in Finnish Lappland.

What comes to the patch, I guess "interval" could be a more appropriate
term to use in this case:



The patch is in line with the Wikipedia article in notation but not in
terminology. I'll send a fix.

-- 
Regards,

Sakari Ailus
sakari.ai...@linux.intel.com



signature.asc
Description: OpenPGP digital signature


Re: [PATCH v2 1/3] dt: bindings: Document DT bindings for Analog devices as3645a

2017-08-28 Thread Pavel Machek
Hi!

> +
> +Ranges below noted as [a, b] are closed ranges between a and b, i.e. a
> +and b are included in the range.

Normally I've seen  for closed ranges, (a, b) for open
ranges. Is that different in your country?

Otherwise

Acked-by: Pavel Machek 


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature


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

2017-08-28 Thread Mauro Carvalho Chehab
Em Mon, 28 Aug 2017 12:05:06 +0200
Hans Verkuil  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


[PATCH 2/2] [media] Cypress: Improve a size determination in cypress_load_firmware()

2017-08-28 Thread SF Markus Elfring
From: Markus Elfring 
Date: Mon, 28 Aug 2017 11:55:16 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/common/cypress_firmware.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/common/cypress_firmware.c 
b/drivers/media/common/cypress_firmware.c
index bfe47bc5f716..8895158c1962 100644
--- a/drivers/media/common/cypress_firmware.c
+++ b/drivers/media/common/cypress_firmware.c
@@ -74,7 +74,7 @@ int cypress_load_firmware(struct usb_device *udev,
struct hexline *hx;
int ret, pos = 0;
 
-   hx = kmalloc(sizeof(struct hexline), GFP_KERNEL);
+   hx = kmalloc(sizeof(*hx), GFP_KERNEL);
if (!hx)
return -ENOMEM;
 
-- 
2.14.1



[PATCH 1/2] [media] Cypress: Delete an error message for a failed memory allocation in cypress_load_firmware()

2017-08-28 Thread SF Markus Elfring
From: Markus Elfring 
Date: Mon, 28 Aug 2017 11:46:57 +0200

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/common/cypress_firmware.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/common/cypress_firmware.c 
b/drivers/media/common/cypress_firmware.c
index 50e3f76d4847..bfe47bc5f716 100644
--- a/drivers/media/common/cypress_firmware.c
+++ b/drivers/media/common/cypress_firmware.c
@@ -78,7 +78,5 @@ int cypress_load_firmware(struct usb_device *udev,
-   if (!hx) {
-   dev_err(>dev, "%s: kmalloc() failed\n", KBUILD_MODNAME);
+   if (!hx)
return -ENOMEM;
-   }
 
/* stop the CPU */
hx->data[0] = 1;
-- 
2.14.1



[PATCH 0/2] [media] Cypress: Adjustments for cypress_load_firmware()

2017-08-28 Thread SF Markus Elfring
From: Markus Elfring 
Date: Mon, 28 Aug 2017 12:00:21 +0200

Two update suggestions were taken into account
from static source code analysis.

Markus Elfring (2):
  Delete an error message for a failed memory allocation
  Improve a size determination

 drivers/media/common/cypress_firmware.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

-- 
2.14.1



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

2017-08-28 Thread 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 
> ---
>  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 `, which creates one
>  device node per sub-device.
>  
> +.. note::
> +
> +   A **vdev-centric** may also optionally expose V4L2 sub-devices via
> +   :ref:`sub-device API `. In that case, it has to implement
> +   the :ref:`media controller API ` as well.
> +
> +
>  .. attention::
>  
> Devices that require **mc-centric** hardware peripheral control should
> 



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

2017-08-28 Thread 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 
> ---
>  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 `, 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``
>- 0x1000
>- This is a touch device.
> +* - ``V4L2_MC_CENTRIC``
> +  - 0x2000
> +  - 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``
>- 0x8000
>- 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  0x1000  /* Is a touch device */
>  
> +#define V4L2_CAP_MC_CENTRIC 0x2000  /* Device require 
> mc-centric hardware control */

requires
MC-centric

> +
>  #define V4L2_CAP_DEVICE_CAPS0x8000  /* sets device 
> capabilities field */
>  
>  /*
> 

Regards,

Hans


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

2017-08-28 Thread 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 

Acked-by: Hans Verkuil 

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() `/\ :ref:`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 `__ git
>  repository), there is no library yet that can provide a single API
> 



Re: [PATCH] cobalt: do not register subdev nodes

2017-08-28 Thread Sakari Ailus
On Mon, Aug 28, 2017 at 10:45:58AM +0200, Hans Verkuil wrote:
> In the distant past the adv7604 driver used private controls. In order to 
> access
> them the v4l-subdevX nodes were needed. Later the is_private tag was removed 
> in
> the adv7604 driver and the need for v4l-subdevX device nodes disappeared.
> 
> Remove the creation of those device nodes from this driver.
> 
> Note: the cobalt card is only used inside Cisco and we never actually used the
> v4l-subdevX nodes for anything. So this API change can be done safely without
> breaking anything.
> 
> Signed-off-by: Hans Verkuil 

Acked-by: Sakari Ailus 

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi


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

2017-08-28 Thread 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 
> ---
>  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 ` 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 ` before
> +the periferal can be used. For such devices, the sub-devices' configuration
> +can be controlled via the :ref:`sub-device API `, 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 3/7] media: open.rst: remove the minor number range

2017-08-28 Thread 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 

Acked-by: Hans Verkuil 

> ---
>  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:
>  
> 



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

2017-08-28 Thread 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 
> ---
>  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 ..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 
> 

[PATCH] cobalt: do not register subdev nodes

2017-08-28 Thread Hans Verkuil
In the distant past the adv7604 driver used private controls. In order to access
them the v4l-subdevX nodes were needed. Later the is_private tag was removed in
the adv7604 driver and the need for v4l-subdevX device nodes disappeared.

Remove the creation of those device nodes from this driver.

Note: the cobalt card is only used inside Cisco and we never actually used the
v4l-subdevX nodes for anything. So this API change can be done safely without
breaking anything.

Signed-off-by: Hans Verkuil 
---
diff --git a/drivers/media/pci/cobalt/cobalt-driver.c 
b/drivers/media/pci/cobalt/cobalt-driver.c
index 98b6cb9505d1..c9d8006ff697 100644
--- a/drivers/media/pci/cobalt/cobalt-driver.c
+++ b/drivers/media/pci/cobalt/cobalt-driver.c
@@ -738,9 +738,6 @@ static int cobalt_probe(struct pci_dev *pci_dev,
goto err_i2c;
}

-   retval = v4l2_device_register_subdev_nodes(>v4l2_dev);
-   if (retval)
-   goto err_i2c;
retval = cobalt_nodes_register(cobalt);
if (retval) {
cobalt_err("Error %d registering device nodes\n", retval);


[PATCH] [media] pci: make i2c_client const

2017-08-28 Thread Bhumika Goyal
Make these const as they are only used in a copy operation.
Done using Coccinelle.

Signed-off-by: Bhumika Goyal 
---
 drivers/media/pci/cx23885/cx23885-i2c.c | 2 +-
 drivers/media/pci/cx25821/cx25821-i2c.c | 2 +-
 drivers/media/pci/ivtv/ivtv-i2c.c   | 2 +-
 drivers/media/pci/saa7134/saa7134-i2c.c | 2 +-
 drivers/media/pci/saa7164/saa7164-i2c.c | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/pci/cx23885/cx23885-i2c.c 
b/drivers/media/pci/cx23885/cx23885-i2c.c
index 8528032..5bbbdec 100644
--- a/drivers/media/pci/cx23885/cx23885-i2c.c
+++ b/drivers/media/pci/cx23885/cx23885-i2c.c
@@ -270,7 +270,7 @@ static u32 cx23885_functionality(struct i2c_adapter *adap)
.algo  = _i2c_algo_template,
 };
 
-static struct i2c_client cx23885_i2c_client_template = {
+static const struct i2c_client cx23885_i2c_client_template = {
.name   = "cx23885 internal",
 };
 
diff --git a/drivers/media/pci/cx25821/cx25821-i2c.c 
b/drivers/media/pci/cx25821/cx25821-i2c.c
index 263a1cf..012e93a 100644
--- a/drivers/media/pci/cx25821/cx25821-i2c.c
+++ b/drivers/media/pci/cx25821/cx25821-i2c.c
@@ -291,7 +291,7 @@ static u32 cx25821_functionality(struct i2c_adapter *adap)
.algo = _i2c_algo_template,
 };
 
-static struct i2c_client cx25821_i2c_client_template = {
+static const struct i2c_client cx25821_i2c_client_template = {
.name = "cx25821 internal",
 };
 
diff --git a/drivers/media/pci/ivtv/ivtv-i2c.c 
b/drivers/media/pci/ivtv/ivtv-i2c.c
index 69b4fa6..09f95f1 100644
--- a/drivers/media/pci/ivtv/ivtv-i2c.c
+++ b/drivers/media/pci/ivtv/ivtv-i2c.c
@@ -700,7 +700,7 @@ static int ivtv_getsda_old(void *data)
.timeout= IVTV_ALGO_BIT_TIMEOUT * HZ, /* jiffies */
 };
 
-static struct i2c_client ivtv_i2c_client_template = {
+static const struct i2c_client ivtv_i2c_client_template = {
.name = "ivtv internal",
 };
 
diff --git a/drivers/media/pci/saa7134/saa7134-i2c.c 
b/drivers/media/pci/saa7134/saa7134-i2c.c
index 9d0e69e..af94aee 100644
--- a/drivers/media/pci/saa7134/saa7134-i2c.c
+++ b/drivers/media/pci/saa7134/saa7134-i2c.c
@@ -345,7 +345,7 @@ static u32 functionality(struct i2c_adapter *adap)
.algo  = _algo,
 };
 
-static struct i2c_client saa7134_client_template = {
+static const struct i2c_client saa7134_client_template = {
.name   = "saa7134 internal",
 };
 
diff --git a/drivers/media/pci/saa7164/saa7164-i2c.c 
b/drivers/media/pci/saa7164/saa7164-i2c.c
index 430f678..e06b617 100644
--- a/drivers/media/pci/saa7164/saa7164-i2c.c
+++ b/drivers/media/pci/saa7164/saa7164-i2c.c
@@ -84,7 +84,7 @@ static u32 saa7164_functionality(struct i2c_adapter *adap)
.algo  = _i2c_algo_template,
 };
 
-static struct i2c_client saa7164_i2c_client_template = {
+static const struct i2c_client saa7164_i2c_client_template = {
.name   = "saa7164 internal",
 };
 
-- 
1.9.1



[PATCH] [media] usb: make i2c_client const

2017-08-28 Thread Bhumika Goyal
Make these const as they are only used in a copy operation.
Done using Coccinelle.

@match disable optional_qualifier@
identifier s;
@@
static struct i2c_client s = {...};

@ref@
position p;
identifier match.s;
@@
s@p

@good1@
position ref.p;
identifier match.s,f,c;
expression e;
@@
(
e = s@p
|
e = s@p.f
|
c(...,s@p.f,...)
|
c(...,s@p,...)
)

@bad depends on  !good1@
position ref.p;
identifier match.s;
@@
s@p

@depends on forall !bad disable optional_qualifier@
identifier match.s;
@@
static
+ const
struct i2c_client s;

Signed-off-by: Bhumika Goyal 
---
 drivers/media/usb/au0828/au0828-i2c.c   | 2 +-
 drivers/media/usb/em28xx/em28xx-i2c.c   | 2 +-
 drivers/media/usb/stk1160/stk1160-i2c.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/au0828/au0828-i2c.c 
b/drivers/media/usb/au0828/au0828-i2c.c
index 42b352b..348b384 100644
--- a/drivers/media/usb/au0828/au0828-i2c.c
+++ b/drivers/media/usb/au0828/au0828-i2c.c
@@ -342,7 +342,7 @@ static u32 au0828_functionality(struct i2c_adapter *adap)
.algo  = _i2c_algo_template,
 };
 
-static struct i2c_client au0828_i2c_client_template = {
+static const struct i2c_client au0828_i2c_client_template = {
.name   = "au0828 internal",
 };
 
diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c 
b/drivers/media/usb/em28xx/em28xx-i2c.c
index 60b195c..fa3fe4c 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -882,7 +882,7 @@ static u32 functionality(struct i2c_adapter *i2c_adap)
.algo = _algo,
 };
 
-static struct i2c_client em28xx_client_template = {
+static const struct i2c_client em28xx_client_template = {
.name = "em28xx internal",
 };
 
diff --git a/drivers/media/usb/stk1160/stk1160-i2c.c 
b/drivers/media/usb/stk1160/stk1160-i2c.c
index 3f2517b..7151e28 100644
--- a/drivers/media/usb/stk1160/stk1160-i2c.c
+++ b/drivers/media/usb/stk1160/stk1160-i2c.c
@@ -246,7 +246,7 @@ static u32 functionality(struct i2c_adapter *adap)
.algo = ,
 };
 
-static struct i2c_client client_template = {
+static const struct i2c_client client_template = {
.name = "stk1160 internal",
 };
 
-- 
1.9.1



Re: [PATCH v4 04/21] doc: media/v4l-drivers: Add Qualcomm Camera Subsystem driver document

2017-08-28 Thread Todor Tomov
Hi Daniel,

On 25.08.2017 17:10, Daniel Mack wrote:
> Hi Todor,
> 
> Thanks a lot for working on the upstream support for this!
> 
> On 08/08/2017 03:30 PM, Todor Tomov wrote:
>> +The Camera Subsystem hardware found on 8x16 processors and supported by the
>> +driver consists of:
>> +
>> +- 2 CSIPHY modules. They handle the Physical layer of the CSI2 receivers.
>> +  A separate camera sensor can be connected to each of the CSIPHY module;
>> +- 2 CSID (CSI Decoder) modules. They handle the Protocol and Application 
>> layer
>> +  of the CSI2 receivers. A CSID can decode data stream from any of the 
>> CSIPHY.
>> +  Each CSID also contains a TG (Test Generator) block which can generate
>> +  artificial input data for test purposes;
>> +- ISPIF (ISP Interface) module. Handles the routing of the data streams from
>> +  the CSIDs to the inputs of the VFE;
>> +- VFE (Video Front End) module. Contains a pipeline of image processing 
>> hardware
>> +  blocks. The VFE has different input interfaces. The PIX input interface 
>> feeds
>> +  the input data to the image processing pipeline. Three RDI input 
>> interfaces
>> +  bypass the image processing pipeline. The VFE also contains the AXI bus
>> +  interface which writes the output data to memory.
> 
> [I'm based on the 4.9 Linaro downstream version of this code right now,
> but at a glance the driver version there looks very much identical to
> this one.]
> 
> Could you explain how ISPIF, CSID and CSIPHY are related?
> 
> I have a userspace test setup that works fine for USB webcams, but when
> operating on any of the video devices exposed by this driver, the
> lowlevel functions such as .s_power of the ISPIF, CSID, CSIPHY and the
> sensor driver layers aren't called into.

Have you activated the media controller links? The s_power is called
when the subdev is part of a pipeline in which the video device node
is opened. You can see example configurations for the Qualcomm CAMSS
driver on:
https://github.com/96boards/documentation/blob/master/ConsumerEdition/DragonBoard-410c/Guides/CameraModule.md
This will probably answer most of your questions.

> 
> The general setup seems to work fine though. The sensor is probed,
> camss_subdev_notifier_complete() is called, and the v4l2 subdevices
> exist. But the stream start is not propagated to the other layers, and
> I'm trying to understand why.
> 
> My DTS looks something like this right now, and the hardware is an
> APQ8016 board (Variscite DART SD410).
> 
>  {
>   cam0: ov5640@3c {
>   compatible = "ovti,ov5640";
>   reg = <0x3c>;
> 
>   // clocks, regulators, gpios etc are omitted
> 
>   port {
>   cam0_ep: endpoint {
>   clock-lanes = <1>;
>   data-lanes = <0 2>;
>   remote-endpoint = <_ep>;
>   };
>   };
>   };
> };
> 
>  {
>   ports {
>   port@0 {
>   reg = <0>;
>   csiphy0_ep: endpoint {
>   clock-lanes = <1>;
>   data-lanes = <0 1 2 3>;

As far as I know the OV5640 has a two data lane CSI2 interface so
this will probably look like:
data-lanes = <0 2>;

>   qcom,settle-cnt = <0xe>;
>   remote-endpoint = <_ep>;
>   };
>   };
>   };
> };
> 
> Also, which video device should be opened when accessing the cameras on
> each of the hardware ports? And what are the other two devices doing?
> 
> I'm sure I'm missing something trivial, but at least I can't find this
> information in the documentation.
> 
> 
> Thanks,
> Daniel
> 

-- 
Best regards,
Todor Tomov


Re: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.

2017-08-28 Thread Yong
Hi Maxime,

On Fri, 25 Aug 2017 15:41:14 +0200
Maxime Ripard  wrote:

> Hi Yong,
> 
> On Wed, Aug 23, 2017 at 10:32:16AM +0800, Yong wrote:
> > > > > > +static int sun6i_graph_notify_complete(struct v4l2_async_notifier 
> > > > > > *notifier)
> > > > > > +{
> > > > > > +   struct sun6i_csi *csi =
> > > > > > +   container_of(notifier, struct sun6i_csi, 
> > > > > > notifier);
> > > > > > +   struct sun6i_graph_entity *entity;
> > > > > > +   int ret;
> > > > > > +
> > > > > > +   dev_dbg(csi->dev, "notify complete, all subdevs registered\n");
> > > > > > +
> > > > > > +   /* Create links for every entity. */
> > > > > > +   list_for_each_entry(entity, >entities, list) {
> > > > > > +   ret = sun6i_graph_build_one(csi, entity);
> > > > > > +   if (ret < 0)
> > > > > > +   return ret;
> > > > > > +   }
> > > > > > +
> > > > > > +   /* Create links for video node. */
> > > > > > +   ret = sun6i_graph_build_video(csi);
> > > > > > +   if (ret < 0)
> > > > > > +   return ret;
> > > > > 
> > > > > Can you elaborate a bit on the difference between a node parsed with
> > > > > _graph_build_one and _graph_build_video? Can't you just store the
> > > > > remote sensor when you build the notifier, and reuse it here?
> > > > 
> > > > There maybe many usercases:
> > > > 1. CSI->Sensor.
> > > > 2. CSI->MIPI->Sensor.
> > > > 3. CSI->FPGA->Sensor1
> > > > ->Sensor2.
> > > > FPGA maybe some other video processor. FPGA, MIPI, Sensor can be
> > > > registered as v4l2 subdevs. We do not care about the driver code
> > > > of them. But they should be linked together here.
> > > > 
> > > > So, the _graph_build_one is used to link CSI port and subdevs. 
> > > > _graph_build_video is used to link CSI port and video node.
> > > 
> > > So the graph_build_one is for the two first cases, and the
> > > _build_video for the latter case?
> > 
> > No. 
> > The _graph_build_one is used to link the subdevs found in the device 
> > tree. _build_video is used to link the closest subdev to video node.
> > Video node is created in the driver, so the method to get it's pad is
> > diffrent to the subdevs.
> 
> Sorry for being slow here, I'm still not sure I get it.
> 
> In summary, both the sun6i_graph_build_one and sun6i_graph_build_video
> will iterate over each endpoint, will retrieve the remote entity, and
> will create the media link between the CSI pad and the remote pad.
> 
> As far as I can see, there's basically two things that
> sun6i_graph_build_one does that sun6i_graph_build_video doesn't:
>   - It skips all the links that would connect to one of the CSI sinks
>   - It skips all the links that would connect to a remote node that is
> equal to the CSI node.
> 
> I assume the latter is because you want to avoid going in an infinite
> loop when you would follow one of the CSI endpoint (going to the
> sensor), and then follow back the same link in the opposite
> direction. Right?

Not exactly. But any way, some code is true redundant here. I will 
make some improve.

> 
> I'm confused about the first one though. All the pads you create in
> your driver are sink pads, so wouldn't that skip all the pads of the
> CSI nodes?
> 
> Also, why do you iterate on all the CSI endpoints, when there's only
> of them? You want to anticipate the future binding for devices with
> multiple channels?
> 
> > > 
> > > If so, you should take a look at the last iteration of the
> > > subnotifiers rework by Nikas Söderlund (v4l2-async: add subnotifier
> > > registration for subdevices).
> > > 
> > > It allows subdevs to register notifiers, and you don't have to build
> > > the graph from the video device, each device and subdev can only care
> > > about what's next in the pipeline, but not really what's behind it.
> > > 
> > > That would mean in your case that you can only deal with your single
> > > CSI pad, and whatever subdev driver will use it care about its own.
> > 
> > Do you mean the subdevs create pad link in the notifier registered by
> > themself ?
> 
> Yes.
> 
> Thanks!
> Maxime
> 
> -- 
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com


Thanks,
Yong


[PATCH] [media] uvcvideo: zero seq number when disabling stream

2017-08-28 Thread Hans Yang
For bulk-based devices, when disabling the video stream,
in addition to issue CLEAR_FEATURE(HALT), it is better to set
alternate setting 0 as well or the sequnce number in host
side will probably not reset to zero.

Then in next time video stream start, the device will expect
host starts packet from 0 sequence number but host actually
continue the sequence number from last transaction and this
causes transaction errors.

This commit fixes this by adding set alternate setting 0 back
as what isoch-based devices do.

Below error message will also be eliminated for some devices:
uvcvideo: Non-zero status (-71) in video completion handler.

Signed-off-by: Hans Yang 
---
 drivers/media/usb/uvc/uvc_video.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_video.c 
b/drivers/media/usb/uvc/uvc_video.c
index fb86d6af398d..ad80c2a6da6a 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1862,10 +1862,9 @@ int uvc_video_enable(struct uvc_streaming *stream, int 
enable)
 
if (!enable) {
uvc_uninit_video(stream, 1);
-   if (stream->intf->num_altsetting > 1) {
-   usb_set_interface(stream->dev->udev,
+   usb_set_interface(stream->dev->udev,
  stream->intfnum, 0);
-   } else {
+   if (stream->intf->num_altsetting == 1) {
/* UVC doesn't specify how to inform a bulk-based device
 * when the video stream is stopped. Windows sends a
 * CLEAR_FEATURE(HALT) request to the video streaming
-- 
2.1.4