RE: [PATCH] media: uvcvideo: Add support for changing UVC_URBS/UVC_MAX_PACKETS from sysfs

[Anurag Kumar Vulisha]

Ping !

>-Original Message-
>From: Anurag Kumar Vulisha [mailto:anurag.kumar.vuli...@xilinx.com]
>Sent: Friday, February 03, 2017 10:10 PM
>To: Laurent Pinchart ; Mauro Carvalho
>Chehab 
>Cc: Punnaiah Choudary Kalluri ; Anirudha Sarangi
>; linux-media@vger.kernel.org; linux-
>ker...@vger.kernel.org; Anurag Kumar Vulisha 
>Subject: [PATCH] media: uvcvideo: Add support for changing
>UVC_URBS/UVC_MAX_PACKETS from sysfs
>
>The uvc_video.c driver currently has fixed the maximum UVC_URBS queued to 5
>and max UVC_MAX_PACKETS per URB to 32K. This configuration works fine with
>USB 2.0 and some USB 3.0 cameras on embedded platforms(like Zynq Ultrascale).
>Since embedded platforms has slow processing speed as compared to
>servers/x86 machines, we may need to increase the number of URBs(UVC_URBS)
>queued. Also some next generation USB 3.0 cameras (like ZED stereo camera)
>splits each frame into multiple chunks of 48K bytes (which is greater than the 
>size
>of UVC_MAX_PACKETS per URB), so we may need to increase
>UVC_MAX_PACKETS also at runtime instead of #define it.
>
>This patch adds the solution to change UVC_URBS and UVC_MAX_PACKETS at
>runtime using sysfs layer before starting the video application.
>
>Signed-off-by: Anurag Kumar Vulisha 
>---
> drivers/media/usb/uvc/uvc_driver.c | 89
>++
> drivers/media/usb/uvc/uvc_video.c  | 39 -
> drivers/media/usb/uvc/uvcvideo.h   | 12 +++--
> 3 files changed, 126 insertions(+), 14 deletions(-)
>
>diff --git a/drivers/media/usb/uvc/uvc_driver.c
>b/drivers/media/usb/uvc/uvc_driver.c
>index 04bf350..51c8058 100644
>--- a/drivers/media/usb/uvc/uvc_driver.c
>+++ b/drivers/media/usb/uvc/uvc_driver.c
>@@ -190,6 +190,89 @@ static struct uvc_format_desc uvc_fmts[] = {
>   },
> };
>
>+/* Sysfs attributes for show and store max_urbs/max_packets per URB */
>+
>+static ssize_t get_max_urbs_show(struct device *dev,
>+  struct device_attribute *attr, char *buf) {
>+
>+  struct uvc_streaming *stream = NULL;
>+  struct usb_interface *intf = to_usb_interface(dev);
>+  struct uvc_device *udev = usb_get_intfdata(intf);
>+  u32 ret_len = 0;
>+  u32 stream_num = 0;
>+
>+  list_for_each_entry(stream, >streams, list) {
>+  ret_len += scnprintf((char *)(buf + ret_len), PAGE_SIZE,
>+  "stream[%d] = %d\n", stream_num++,
>+  stream->max_urbs);
>+  }
>+
>+  return ret_len;
>+}
>+static DEVICE_ATTR_RO(get_max_urbs);
>+
>+static ssize_t set_max_urbs_store(struct device *dev,
>+  struct device_attribute *attr, const char *buf, size_t count) {
>+
>+  struct uvc_streaming *stream;
>+  struct usb_interface *intf = to_usb_interface(dev);
>+  struct uvc_device *udev = usb_get_intfdata(intf);
>+
>+  list_for_each_entry(stream, >streams, list) {
>+  sscanf(buf, "%d", >max_urbs);
>+  }
>+
>+  return count;
>+}
>+static DEVICE_ATTR_WO(set_max_urbs);
>+
>+static ssize_t get_max_packets_show(struct device *dev,
>+  struct device_attribute *attr, char *buf) {
>+
>+  struct uvc_streaming *stream = NULL;
>+  struct usb_interface *intf = to_usb_interface(dev);
>+  struct uvc_device *udev = usb_get_intfdata(intf);
>+  u32 ret_len = 0;
>+  u32 stream_num = 0;
>+
>+  list_for_each_entry(stream, >streams, list) {
>+  ret_len += scnprintf((char *)(buf + ret_len), PAGE_SIZE,
>+  "stream[%d] = %d\n", stream_num++,
>+  stream->max_packets);
>+  }
>+
>+  return ret_len;
>+}
>+static DEVICE_ATTR_RO(get_max_packets);
>+
>+static ssize_t set_max_packets_store(struct device *dev,
>+  struct device_attribute *attr, const char *buf, size_t count) {
>+
>+  struct uvc_streaming *stream;
>+  struct usb_interface *intf = to_usb_interface(dev);
>+  struct uvc_device *udev = usb_get_intfdata(intf);
>+
>+  list_for_each_entry(stream, >streams, list) {
>+  sscanf(buf, "%d", >max_packets);
>+  }
>+
>+  return count;
>+}
>+static DEVICE_ATTR_WO(set_max_packets);
>+
>+static struct attribute *uvc_attributes[] = {
>+  _attr_get_max_urbs.attr,
>+  _attr_set_max_urbs.attr,
>+  _attr_get_max_packets.attr,
>+  _attr_set_max_packets.attr,
>+  NULL
>+};
>+
>+static const struct attribute_group uvc_attr_group = {
>+  .attrs = uvc_attributes,
>+};
>+
>+
> /* 
>  * Utility functions
>  */
>@@ -2097,6 +2180,12 @@ static int uvc_probe(struct usb_interface *intf,
>   "supported.\n", ret);
>   }
>
>+  ret = sysfs_create_group(>intf->dev.kobj, _attr_group);
>+  if (ret < 0) {
>+  uvc_printk(KERN_ERR, "Failed to 

cron job: media_tree daily build: WARNINGS

[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:   Sat Feb 18 05:00:17 CET 2017
media-tree git hash:9eeb0ed0f30938f31a3d9135a88b9502192c18dd
media_build git hash:   785cdf7f0798964681b33aad44fc2ff4d734733d
v4l-utils git hash: 1edd6920bed585d0ea70a2d400182ba17ee2e7fc
gcc version:i686-linux-gcc (GCC) 6.2.0
sparse version: v0.5.0-3553-g78b2ea6
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.9.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: 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: WARNINGS
linux-2.6.37.6-i686: WARNINGS
linux-2.6.38.8-i686: WARNINGS
linux-2.6.39.4-i686: WARNINGS
linux-3.0.60-i686: WARNINGS
linux-3.1.10-i686: WARNINGS
linux-3.2.37-i686: WARNINGS
linux-3.3.8-i686: WARNINGS
linux-3.4.27-i686: WARNINGS
linux-3.5.7-i686: WARNINGS
linux-3.6.11-i686: WARNINGS
linux-3.7.4-i686: WARNINGS
linux-3.8-i686: WARNINGS
linux-3.9.2-i686: WARNINGS
linux-3.10.1-i686: WARNINGS
linux-3.11.1-i686: WARNINGS
linux-3.12.67-i686: WARNINGS
linux-3.13.11-i686: WARNINGS
linux-3.14.9-i686: WARNINGS
linux-3.15.2-i686: WARNINGS
linux-3.16.7-i686: WARNINGS
linux-3.17.8-i686: WARNINGS
linux-3.18.7-i686: WARNINGS
linux-3.19-i686: WARNINGS
linux-4.0.9-i686: WARNINGS
linux-4.1.33-i686: WARNINGS
linux-4.2.8-i686: WARNINGS
linux-4.3.6-i686: WARNINGS
linux-4.4.22-i686: WARNINGS
linux-4.5.7-i686: WARNINGS
linux-4.6.7-i686: WARNINGS
linux-4.7.5-i686: WARNINGS
linux-4.8-i686: OK
linux-4.9-i686: OK
linux-4.10-rc3-i686: OK
linux-2.6.36.4-x86_64: WARNINGS
linux-2.6.37.6-x86_64: WARNINGS
linux-2.6.38.8-x86_64: WARNINGS
linux-2.6.39.4-x86_64: WARNINGS
linux-3.0.60-x86_64: WARNINGS
linux-3.1.10-x86_64: WARNINGS
linux-3.2.37-x86_64: WARNINGS
linux-3.3.8-x86_64: WARNINGS
linux-3.4.27-x86_64: WARNINGS
linux-3.5.7-x86_64: WARNINGS
linux-3.6.11-x86_64: WARNINGS
linux-3.7.4-x86_64: WARNINGS
linux-3.8-x86_64: WARNINGS
linux-3.9.2-x86_64: WARNINGS
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: WARNINGS
linux-3.12.67-x86_64: WARNINGS
linux-3.13.11-x86_64: WARNINGS
linux-3.14.9-x86_64: WARNINGS
linux-3.15.2-x86_64: WARNINGS
linux-3.16.7-x86_64: WARNINGS
linux-3.17.8-x86_64: WARNINGS
linux-3.18.7-x86_64: WARNINGS
linux-3.19-x86_64: WARNINGS
linux-4.0.9-x86_64: WARNINGS
linux-4.1.33-x86_64: WARNINGS
linux-4.2.8-x86_64: WARNINGS
linux-4.3.6-x86_64: WARNINGS
linux-4.4.22-x86_64: WARNINGS
linux-4.5.7-x86_64: WARNINGS
linux-4.6.7-x86_64: WARNINGS
linux-4.7.5-x86_64: WARNINGS
linux-4.8-x86_64: OK
linux-4.9-x86_64: OK
linux-4.10-rc3-x86_64: OK
apps: WARNINGS
spec-git: OK
sparse: WARNINGS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

[PATCH] bcm2048: Fix checkpatch checks

[Man Choy]

Fix following checks:

CHECK: Avoid crashing the kernel - try using WARN_ON & recovery code rather 
than BUG() or BUG_ON()
+   BUG_ON((index+2) >= BCM2048_MAX_RDS_RT);

CHECK: spaces preferred around that '+' (ctx:VxV)
+   BUG_ON((index+2) >= BCM2048_MAX_RDS_RT);
 ^

CHECK: Avoid crashing the kernel - try using WARN_ON & recovery code rather 
than BUG() or BUG_ON()
+   BUG_ON((index+4) >= BCM2048_MAX_RDS_RT);

CHECK: spaces preferred around that '+' (ctx:VxV)
+   BUG_ON((index+4) >= BCM2048_MAX_RDS_RT);
 ^
---
 drivers/staging/media/bcm2048/radio-bcm2048.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
b/drivers/staging/media/bcm2048/radio-bcm2048.c
index 37bd439..d5ee279 100644
--- a/drivers/staging/media/bcm2048/radio-bcm2048.c
+++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
@@ -1534,7 +1534,7 @@ static int bcm2048_parse_rt_match_c(struct bcm2048_device 
*bdev, int i,
if (crc == BCM2048_RDS_CRC_UNRECOVARABLE)
return 0;
 
-   BUG_ON((index+2) >= BCM2048_MAX_RDS_RT);
+   WARN_ON((index + 2) >= BCM2048_MAX_RDS_RT);
 
if ((bdev->rds_info.radio_text[i] & BCM2048_RDS_BLOCK_MASK) ==
BCM2048_RDS_BLOCK_C) {
@@ -1557,7 +1557,7 @@ static void bcm2048_parse_rt_match_d(struct 
bcm2048_device *bdev, int i,
if (crc == BCM2048_RDS_CRC_UNRECOVARABLE)
return;
 
-   BUG_ON((index+4) >= BCM2048_MAX_RDS_RT);
+   WARN_ON((index + 4) >= BCM2048_MAX_RDS_RT);
 
if ((bdev->rds_info.radio_text[i] & BCM2048_RDS_BLOCK_MASK) ==
BCM2048_RDS_BLOCK_D)
-- 
2.7.4

cx231xx: disagrees about version of symbol videobuf_streamoff

[Bill Atwood]

I have downloaded the V4L git tree and complied it (apparently) 
successfully.  I did "sudo make install", and (at the end) I see the 
message that several items of firmware are installed in /lib/firmware.  
v4l-cx231xx-avcore-01.fw is one of those:


-rw-r--r-- 1 root root  16382 Feb 17 17:46 v4l-cx231xx-avcore-01.fw

Kernel version: 4.4.0-62-generic

OS version: Ubuntu 16.04.2 LTS
(64-bit)

My device (Hauppauge WinTV-HVR-955Q) is recognized by the kernel:

Feb 17 17:54:58 willow kernel: [1.858780] usb 2-2: New USB device 
found, idVendor=2040, idProduct=b123
Feb 17 17:54:58 willow kernel: [1.858783] usb 2-2: New USB device 
strings: Mfr=1, Product=2, SerialNumber=3
Feb 17 17:54:58 willow kernel: [1.858785] usb 2-2: Product: 
Hauppauge Device
Feb 17 17:54:58 willow kernel: [1.858787] usb 2-2: Manufacturer: 
Hauppauge
Feb 17 17:54:58 willow kernel: [1.858789] usb 2-2: SerialNumber: 
4035560228


However, once the kernel starts processing the software that must be 
present for the 955Q, it issues a series of messages, of which this is 
the first pair:


Feb 17 17:54:58 willow kernel: [   11.491848] cx231xx: disagrees about 
version of symbol videobuf_streamoff


Feb 17 17:54:58 willow kernel: [   11.491852] cx231xx: Unknown symbol 
videobuf_streamoff (err -22)


This pair then repeats going through a large number of unresolved symbols.

I built exactly the same software for a 32-bit machine a few days ago, 
and it installed the device successfully.  I was able to do a channel 
scan with the device, so it was clearly working.  (I did not try to 
watch LiveTV, because the display capabilities of the 32-bit processor 
were not sufficient.)


The sha-256 sum for the firmware in my 32-bit machine is

09a39c139d8e47ebfa2e7f64472e7165dff66359277ca02bcfdcd79f515764ef

and the date is Apr 25 2016.

The sha-256 sum for the firmware in my 64-bit machine is

c2a75fc710f51c778abe7c7e8b54ed5686b17811dd203d1de3070d3df70d70f6

and (as noted above) the date is Feb 17 17:46 (i.e., the time when the 
"sudo make install" command was run).


The sha-256 sum for the file on 
https://www.linuxtv.org/downloads/firmware/ is identical to the sha-256 
sum for the firmware in my 64-bit machine.



The OSes for both the 32-bit machine and the 64-bit machine were built 
as clean installs from downloaded Ubuntu 16.04 LTS .iso files, and 
allowed to update to 16.04.2 (i.e., all updates applied) before I 
attempted to install MythTV and the linux-media drivers.



I did find an old reference to this "unresolved symbols" issue on the 
web, but it dates from 2008, and does not seem pertinent to the problem 
that I am having.



Does anyone have any suggestions for how to fix this?


  Bill

Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates

[Steve Longerbeam]

On 02/15/2017 06:19 PM, Steve Longerbeam wrote:

From: Russell King 

Setting and getting frame rates is part of the negotiation mechanism
between subdevs.  The lack of support means that a frame rate at the
sensor can't be negotiated through the subdev path.

Add support at MIPI CSI2 level for handling this part of the
negotiation.

Signed-off-by: Russell King 
Signed-off-by: Steve Longerbeam 



Hi Russell,

I signed-off on this but after more review I'm not sure this is right.

The CSI-2 receiver really has no control over frame rate. It's output
frame rate is the same as the rate that is delivered to it.

So this subdev should either not implement these ops, or it should
refer them to the attached source subdev.

Steve


---
 drivers/staging/media/imx/imx6-mipi-csi2.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c 
b/drivers/staging/media/imx/imx6-mipi-csi2.c
index 23dca80..c62f14e 100644
--- a/drivers/staging/media/imx/imx6-mipi-csi2.c
+++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
@@ -34,6 +34,7 @@ struct csi2_dev {
struct v4l2_subdev  sd;
struct media_pad   pad[CSI2_NUM_PADS];
struct v4l2_mbus_framefmt format_mbus;
+   struct v4l2_fract  frame_interval;
struct clk *dphy_clk;
struct clk *cfg_clk;
struct clk *pix_clk; /* what is this? */
@@ -397,6 +398,30 @@ static int csi2_set_fmt(struct v4l2_subdev *sd,
return 0;
 }

+static int csi2_g_frame_interval(struct v4l2_subdev *sd,
+struct v4l2_subdev_frame_interval *fi)
+{
+   struct csi2_dev *csi2 = sd_to_dev(sd);
+
+   fi->interval = csi2->frame_interval;
+
+   return 0;
+}
+
+static int csi2_s_frame_interval(struct v4l2_subdev *sd,
+struct v4l2_subdev_frame_interval *fi)
+{
+   struct csi2_dev *csi2 = sd_to_dev(sd);
+
+   /* Output pads mirror active input pad, no limits on input pads */
+   if (fi->pad != CSI2_SINK_PAD)
+   fi->interval = csi2->frame_interval;
+
+   csi2->frame_interval = fi->interval;
+
+   return 0;
+}
+
 /*
  * retrieve our pads parsed from the OF graph by the media device
  */
@@ -430,6 +455,8 @@ static struct v4l2_subdev_core_ops csi2_core_ops = {

 static struct v4l2_subdev_video_ops csi2_video_ops = {
.s_stream = csi2_s_stream,
+   .g_frame_interval = csi2_g_frame_interval,
+   .s_frame_interval = csi2_s_frame_interval,
 };

 static struct v4l2_subdev_pad_ops csi2_pad_ops = {



--
Steve Longerbeam | Senior Embedded Engineer, ESD Services
Mentor Embedded(tm) | 46871 Bayside Parkway, Fremont, CA 94538
P 510.354.5838 | M 408.410.2735

Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates

[Steve Longerbeam]

On 02/15/2017 06:19 PM, Steve Longerbeam wrote:

From: Russell King 

Setting and getting frame rates is part of the negotiation mechanism
between subdevs.  The lack of support means that a frame rate at the
sensor can't be negotiated through the subdev path.

Add support at MIPI CSI2 level for handling this part of the
negotiation.

Signed-off-by: Russell King 
Signed-off-by: Steve Longerbeam 



Hi Russell,

I signed-off on this but after more review I'm not sure this is right.

The CSI-2 receiver really has no control over frame rate. It's output
frame rate is the same as the rate that is delivered to it.

So this subdev should either not implement these ops, or it should
refer them to the attached source subdev.

Steve


---
 drivers/staging/media/imx/imx6-mipi-csi2.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c 
b/drivers/staging/media/imx/imx6-mipi-csi2.c
index 23dca80..c62f14e 100644
--- a/drivers/staging/media/imx/imx6-mipi-csi2.c
+++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
@@ -34,6 +34,7 @@ struct csi2_dev {
struct v4l2_subdev  sd;
struct media_pad   pad[CSI2_NUM_PADS];
struct v4l2_mbus_framefmt format_mbus;
+   struct v4l2_fract  frame_interval;
struct clk *dphy_clk;
struct clk *cfg_clk;
struct clk *pix_clk; /* what is this? */
@@ -397,6 +398,30 @@ static int csi2_set_fmt(struct v4l2_subdev *sd,
return 0;
 }

+static int csi2_g_frame_interval(struct v4l2_subdev *sd,
+struct v4l2_subdev_frame_interval *fi)
+{
+   struct csi2_dev *csi2 = sd_to_dev(sd);
+
+   fi->interval = csi2->frame_interval;
+
+   return 0;
+}
+
+static int csi2_s_frame_interval(struct v4l2_subdev *sd,
+struct v4l2_subdev_frame_interval *fi)
+{
+   struct csi2_dev *csi2 = sd_to_dev(sd);
+
+   /* Output pads mirror active input pad, no limits on input pads */
+   if (fi->pad != CSI2_SINK_PAD)
+   fi->interval = csi2->frame_interval;
+
+   csi2->frame_interval = fi->interval;
+
+   return 0;
+}
+
 /*
  * retrieve our pads parsed from the OF graph by the media device
  */
@@ -430,6 +455,8 @@ static struct v4l2_subdev_core_ops csi2_core_ops = {

 static struct v4l2_subdev_video_ops csi2_video_ops = {
.s_stream = csi2_s_stream,
+   .g_frame_interval = csi2_g_frame_interval,
+   .s_frame_interval = csi2_s_frame_interval,
 };

 static struct v4l2_subdev_pad_ops csi2_pad_ops = {



--
Steve Longerbeam | Senior Embedded Engineer, ESD Services
Mentor Embedded(tm) | 46871 Bayside Parkway, Fremont, CA 94538
P 510.354.5838 | M 408.410.2735

[PATCH] [media] dvb-usb-dibusb-mc-common: Add MODULE_LICENSE

[Ben Hutchings]

dvb-usb-dibusb-mc-common is licensed under GPLv2, and if we don't say
so then it won't even load since it needs a GPL-only symbol.

Reported-by: Dominique Dumont 
References: https://bugs.debian.org/853110
Cc: sta...@vger.kernel.org # 4.9+
Fixes: e91455a1495a ("[media] dvb-usb: split out common parts of dibusb")
Signed-off-by: Ben Hutchings 
---
 drivers/media/usb/dvb-usb/dibusb-mc-common.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/usb/dvb-usb/dibusb-mc-common.c 
b/drivers/media/usb/dvb-usb/dibusb-mc-common.c
index c989cac9343d..0c2bc97436d5 100644
--- a/drivers/media/usb/dvb-usb/dibusb-mc-common.c
+++ b/drivers/media/usb/dvb-usb/dibusb-mc-common.c
@@ -11,6 +11,8 @@
 
 #include "dibusb.h"
 
+MODULE_LICENSE("GPL");
+
 /* 3000MC/P stuff */
 // Config Adjacent channels  Perf -cal22
 static struct dibx000_agc_config dib3000p_mt2060_agc_config = {


signature.asc
Description: Digital signature

[patch v2] staging: bcm2835-camera: fix error handling in init

[Dan Carpenter]

The unwinding here isn't right.  We don't free gdev[0] and instead
free 1 step past what was allocated.  Also we can't allocate "dev" then
we should unwind instead of returning directly.

Fixes: 7b3ad5abf027 ("staging: Import the BCM2835 MMAL-based V4L2 camera 
driver.")
Signed-off-by: Dan Carpenter 
---
v2: Change the style to make Walter Harms happy.  Fix some additional
bugs I missed in the first patch.

diff --git a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c 
b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
index ca15a698e018..c4dad30dd133 100644
--- a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
+++ b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
@@ -1901,6 +1901,7 @@ static int __init bm2835_mmal_init(void)
unsigned int num_cameras;
struct vchiq_mmal_instance *instance;
unsigned int resolutions[MAX_BCM2835_CAMERAS][2];
+   int i;
 
ret = vchiq_mmal_init();
if (ret < 0)
@@ -1914,8 +1915,10 @@ static int __init bm2835_mmal_init(void)
 
for (camera = 0; camera < num_cameras; camera++) {
dev = kzalloc(sizeof(struct bm2835_mmal_dev), GFP_KERNEL);
-   if (!dev)
-   return -ENOMEM;
+   if (!dev) {
+   ret = -ENOMEM;
+   goto cleanup_gdev;
+   }
 
dev->camera_num = camera;
dev->max_width = resolutions[camera][0];
@@ -1998,9 +2001,10 @@ static int __init bm2835_mmal_init(void)
 free_dev:
kfree(dev);
 
-   for ( ; camera > 0; camera--) {
-   bcm2835_cleanup_instance(gdev[camera]);
-   gdev[camera] = NULL;
+cleanup_gdev:
+   for (i = 0; i < camera; i++) {
+   bcm2835_cleanup_instance(gdev[i]);
+   gdev[i] = NULL;
}
pr_info("%s: error %d while loading driver\n",
BM2835_MMAL_MODULE_NAME, ret);

[patch] staging: bcm2835/mmal-vchiq: unlock on error in buffer_from_host()

[Dan Carpenter]

We should unlock before returning on this error path.

Fixes: 7b3ad5abf027 ("staging: Import the BCM2835 MMAL-based V4L2 camera 
driver.")
Signed-off-by: Dan Carpenter 

diff --git a/drivers/staging/media/platform/bcm2835/mmal-vchiq.c 
b/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
index f0639ee6c8b9..fdfb6a620a43 100644
--- a/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
+++ b/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
@@ -397,8 +397,10 @@ buffer_from_host(struct vchiq_mmal_instance *instance,
 
/* get context */
msg_context = get_msg_context(instance);
-   if (msg_context == NULL)
-   return -ENOMEM;
+   if (!msg_context) {
+   ret = -ENOMEM;
+   goto unlock;
+   }
 
/* store bulk message context for when data arrives */
msg_context->u.bulk.instance = instance;
@@ -454,6 +456,7 @@ buffer_from_host(struct vchiq_mmal_instance *instance,
 
vchi_service_release(instance->handle);
 
+unlock:
mutex_unlock(>bulk_mutex);
 
return ret;

Dead code or otherwise invalid memory access in drivers/media/v4l2-core/videobuf-core.c

[Shaobo]

Hey guys,

I found that the definition and usage of macro `CALLPTR` may be 
problematic. Its definition is,


54 #define CALLPTR(q, f, arg...)
   \

55 ((q->int_ops->f) ? q->int_ops->f(arg) : NULL)


, which means it can evaluate to NULL. It has two occurrences: one in 
line 839 and the other is line 856. It appears to me that it's very 
likely that there will be invalid memory accesses if `CALLPTR` evaluates 
to NULL since there is no NULL test in either location. In other words, 
programmers' assumption suggest the else branch of the conditional 
expression dead. Please let me know if makes sense or not.


Thanks for your time and I am looking forward to your reply.

Best,
Shaobo

Re: [PATCH] [media] Staging: media/lirc: don't call put_ir_rx on rx twice

[Dan Carpenter]

This one is a false positive.  The original code is correct.

I was looking through my mail boxes to see the history of this and why
it hadn't been fixed earlier.  Someone tried to fix it in 2011:
https://www.spinics.net/lists/linux-driver-devel/msg17403.html
Then I complained about it again in 2014 when I was looking at a
different bug in that same function.  Now you're the third person to
think this code is suspicious.

I think part of the problem is that get_ir_rx(ir) is hidden as a
function parameter instead of on its own line.  But really even that
wouldn't totally fix the issue.

regards,
dan carpenter

Re: [PATCH 00/15] Exynos MFC v6+ - remove the need for the reserved memory

[Javier Martinez Canillas]

Hello Marek,

On 02/14/2017 04:51 AM, Marek Szyprowski wrote:
> Dear All,
> 
> This patchset is a result of my work on enabling full support for MFC device
> (multimedia codec) on Exynos 5433 on ARM64 architecture. Initially I thought
> that to let it working on ARM64 architecture with IOMMU, I would need to
> solve the issue related to the fact that s5p-mfc driver was depending on the
> first-fit allocation method in the DMA-mapping / IOMMU glue code (ARM64 use
> different algorithm). It turned out, that there is a much simpler way.
> 

A nice side effect of these patches is that I don't see anymore a page fault
error with CMA when sharing dma-buf between s5p-fmc and exynos-gsc drivers.

Following GST pipeline used to lead to a system crash, but it's working now:

$ gst-launch-1.0 filesrc location=test.mp4 ! qtdemux ! h264parse ! \
v4l2video2dec capture-io-mode=dmabuf ! v4l2video0convert \
output-io-mode=dmabuf-import capture-io-mode=dmabuf ! kmssink

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: Bug#854100: libdvbv5-0: fails to tune / scan

[Marcel Heinz]

Hi,

Am 13. Februar 2017 schrieb Mauro Carvalho Chehab:

> Em Fri, 10 Feb 2017 22:02:01 +0100
> Gregor Jasny  escreveu:
> 
>> Bug report against libdvbv5 is here:
>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=854100
> 
> There was a bug at the logic that was checking if the frequency was
> at the range of the local oscillators. This patch should be addressing
> it:
>   
> https://git.linuxtv.org/v4l-utils.git/commit/?id=5380ad44de416a41b4972e8a9c147ce42b0e3ba0
> 
> With that, the logic now seems to be working fine:
> 
> $ ./utils/dvb/dvbv5-scan ~/Intelsat-34 --lnbf universal -vv
> Using LNBf UNIVERSAL
>   Universal, Europe
>   10800 to 11800 MHz, LO: 9750 MHz
>   11600 to 12700 MHz, LO: 10600 MHz
> ...
> Seeking for LO for 12.17 MHz frequency
> LO setting 0: 10.80 MHz to 11.80 MHz
> LO setting 1: 11.60 MHz to 12.70 MHz
> Multi-LO LNBf. using LO setting 1 at 10600.00 MHz
> frequency: 12170.00 MHz, high_band: 1
> L-Band frequency: 1570.00 MHz (offset = 10600.00 MHz)
> 
> I can't really test it here, as my satellite dish uses a different
> type of LNBf, but, from the above logs, the bug should be fixed.
> 
> Marcel,
> 
> Could you please test? The patch is already upstream.
> I added a debug patch after it, in order to help LNBf issues
> (enabled by using "-vv" command line parameters).

I can confirm that 1.12.3 solves the issue for me. Thanks for the fix.

Regards,
Marcel

Re: [PATCH 15/15] ARM: dts: exynos: Remove MFC reserved buffers

[Javier Martinez Canillas]

Hello Marek,

On 02/14/2017 04:52 AM, Marek Szyprowski wrote:
> During my research I found that some of the requirements for the memory
> buffers for MFC v6+ devices were blindly copied from the previous (v5)
> version and simply turned out to be excessive. The relaxed requirements
> are applied by the recent patches to the MFC driver and the driver is
> now fully functional even without the reserved memory blocks for all
> v6+ variants. This patch removes those reserved memory nodes from all
> boards having MFC v6+ hardware block.
> 
> Signed-off-by: Marek Szyprowski 
> ---

Reviewed-by: Javier Martinez Canillas 
Tested-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 14/15] media: s5p-mfc: Use preallocated block allocator always for MFC v6+

[Javier Martinez Canillas]

Hello Marek,

On 02/14/2017 04:52 AM, Marek Szyprowski wrote:
> It turned out that all versions of MFC v6+ hardware doesn't have a strict
> requirement for ALL buffers to be allocated on higher addresses than the
> firmware base like it was documented for MFC v5. This requirement is true
> only for the device and per-context buffers. All video data buffers can be
> allocated anywhere for all MFC v6+ versions. Basing on this fact, the
> special DMA configuration based on two reserved memory regions is not
> really needed for MFC v6+ devices, because the memory requirements for the
> firmware, device and per-context buffers can be fulfilled by the simple
> probe-time pre-allocated block allocator instroduced in previous patch.

s/instroduced/introduced

> 
> This patch enables support for such pre-allocated block based allocator
> always for MFC v6+ devices. Due to the limitations of the memory management
> subsystem the largest supported size of the pre-allocated buffer when no
> CMA (Contiguous Memory Allocator) is enabled is 4MiB.
> 
> This patch also removes the requirement to provide two reserved memory
> regions for MFC v6+ devices in device tree. Now the driver is fully
> functional without them.
>

Great!

> Signed-off-by: Marek Szyprowski 
> ---

The patch looks good to me though and it works on my tests,
I've just one comment below.

Reviewed-by: Javier Martinez Canillas 
Tested-by: Javier Martinez Canillas 

>  Documentation/devicetree/bindings/media/s5p-mfc.txt | 2 +-
>  drivers/media/platform/s5p-mfc/s5p_mfc.c| 9 ++---
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt 
> b/Documentation/devicetree/bindings/media/s5p-mfc.txt
> index 2c901286d818..d3404b5d4d17 100644
> --- a/Documentation/devicetree/bindings/media/s5p-mfc.txt
> +++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt
> @@ -28,7 +28,7 @@ Optional properties:
>- memory-region : from reserved memory binding: phandles to two reserved
>   memory regions, first is for "left" mfc memory bus interfaces,
>   second if for the "right" mfc memory bus, used when no SYSMMU
> - support is available
> + support is available; used only by MFC v5 present in Exynos4 SoCs
>  
>  Obsolete properties:
>- samsung,mfc-r, samsung,mfc-l : support removed, please use memory-region
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c 
> b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> index 8fc6fe4ba087..36f0aec2a1b3 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> @@ -1178,9 +1178,12 @@ static void s5p_mfc_unconfigure_2port_memory(struct 
> s5p_mfc_dev *mfc_dev)
>  static int s5p_mfc_configure_common_memory(struct s5p_mfc_dev *mfc_dev)
>  {
>   struct device *dev = _dev->plat_dev->dev;
> - unsigned long mem_size = SZ_8M;
> + unsigned long mem_size = SZ_4M;
>   unsigned int bitmap_size;
>  
> + if (IS_ENABLED(CONFIG_DMA_CMA) || exynos_is_iommu_available(dev))
> + mem_size = SZ_8M;
> +
>   if (mfc_mem_size)
>   mem_size = memparse(mfc_mem_size, NULL);
> 

The driver allows the user to set mem_size > SZ_4M using the s5p_mfc.mem
parameter even when CMA ins't enabled and IOMMU isn't available. Maybe it
should fail early instead and notify the user what's missing?

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 13/15] media: s5p-mfc: Remove special configuration of IOMMU domain

[Javier Martinez Canillas]

Hello Marek,

On 02/14/2017 04:52 AM, Marek Szyprowski wrote:
> The main reason for using special configuration of IOMMU domain was the
> problem with MFC firmware, which failed to operate properly when placed
> at 0 DMA address. Instead of adding custom code for configuring each
> variant of IOMMU domain and architecture specific glue code, simply use
> what arch code provides and if the DMA base address equals zero, skip
> first 128 KiB to keep required alignment. This patch also make the driver
> operational on ARM64 architecture, because it no longer depends on ARM
> specific DMA-mapping and IOMMU glue code functions.
> 
> Signed-off-by: Marek Szyprowski 
> ---

Reviewed-by: Javier Martinez Canillas 
Tested-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 12/15] media: s5p-mfc: Add support for probe-time preallocated block based allocator

[Javier Martinez Canillas]

Hello Marek,

On 02/14/2017 04:52 AM, Marek Szyprowski wrote:
> Current MFC driver depends on the fact that when IOMMU is available, the
> DMA-mapping framework and its IOMMU glue will use first-fit allocator.
> This was true for ARM architecture, but its not for ARM64 arch. However, in
> case of MFC v6+ hardware and latest firmware, it turned out that there is
> no strict requirement for ALL buffers to be allocated on higher addresses
> than the firmware base. This requirement is true only for the device and
> per-context buffers. All video data buffers can be allocated anywhere for
> all MFC v6+ versions.
> 
> Such relaxed requirements for the memory buffers can be easily fulfilled
> by allocating firmware, device and per-context buffers from the probe-time
> preallocated larger buffer. This patch adds support for it. This way the
> driver finally works fine on ARM64 architecture. The size of the
> preallocated buffer is 8 MiB, what is enough for three instances H264
> decoders or encoders (other codecs have smaller memory requirements).
> If one needs more for particular use case, one can use "mem" module
> parameter to force larger (or smaller) buffer (for example by adding
> "s5p_mfc.mem=16M" to kernel command line).
> 
> Signed-off-by: Marek Szyprowski 
> ---

Reviewed-by: Javier Martinez Canillas 
Tested-by: Javier Martinez Canillas 

Best regards,

-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 11/15] media: s5p-mfc: Split variant DMA memory configuration into separate functions

[Javier Martinez Canillas]

Hello Marek,

On 02/14/2017 04:52 AM, Marek Szyprowski wrote:
> Move code for DMA memory configuration with IOMMU into separate function
> to make it easier to compare what is being done in each case.
> 
> Signed-off-by: Marek Szyprowski 
> ---
>  drivers/media/platform/s5p-mfc/s5p_mfc.c | 102 
> ++-
>  1 file changed, 61 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c 
> b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> index 92a88c20b26d..a18740c81c55 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> @@ -1107,41 +1107,13 @@ static struct device *s5p_mfc_alloc_memdev(struct 
> device *dev,
>   return NULL;
>  }
>  
> -static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev)
> +static int s5p_mfc_configure_2port_memory(struct s5p_mfc_dev *mfc_dev)
>  {
>   struct device *dev = _dev->plat_dev->dev;
>   void *bank2_virt;
>   dma_addr_t bank2_dma_addr;
>   unsigned long align_size = 1 << MFC_BASE_ALIGN_ORDER;
> - struct s5p_mfc_priv_buf *fw_buf = _dev->fw_buf;
> -
> - /*
> -  * When IOMMU is available, we cannot use the default configuration,
> -  * because of MFC firmware requirements: address space limited to
> -  * 256M and non-zero default start address.
> -  * This is still simplified, not optimal configuration, but for now
> -  * IOMMU core doesn't allow to configure device's IOMMUs channel
> -  * separately.
> -  */
> - if (exynos_is_iommu_available(dev)) {
> - int ret = exynos_configure_iommu(dev, S5P_MFC_IOMMU_DMA_BASE,
> -  S5P_MFC_IOMMU_DMA_SIZE);
> - if (ret)
> - return ret;
> -
> - mfc_dev->mem_dev[BANK1_CTX] = mfc_dev->mem_dev[BANK2_CTX] = dev;
> - ret = s5p_mfc_alloc_firmware(mfc_dev);
> - if (ret) {
> - exynos_unconfigure_iommu(dev);
> - return ret;
> - }
> -
> - mfc_dev->dma_base[BANK1_CTX] = mfc_dev->fw_buf.dma;
> - mfc_dev->dma_base[BANK2_CTX] = mfc_dev->fw_buf.dma;
> - vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32));
> -
> - return 0;
> - }
> + int ret;

This should be declared in patch 8/15.

>  
>   /*
>* Create and initialize virtual devices for accessing
> @@ -1188,26 +1160,74 @@ static int s5p_mfc_configure_dma_memory(struct 
> s5p_mfc_dev *mfc_dev)
>   DMA_BIT_MASK(32));
>   vb2_dma_contig_set_max_seg_size(mfc_dev->mem_dev[BANK2_CTX],
>   DMA_BIT_MASK(32));
> -

This seems to be an unrelated change.

The rest looks good to me.

Reviewed-by: Javier Martinez Canillas 
Tested-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 10/15] media: s5p-mfc: Reduce firmware buffer size for MFC v6+ variants

[Javier Martinez Canillas]

Hello Marek,

On 02/14/2017 04:52 AM, Marek Szyprowski wrote:
> Firmware for MFC v6+ variants is not larger than 400 KiB, so there is no
> need to allocate a full 1 MiB buffer for it. Reduce it to 512 KiB to keep
> proper alignment of allocated buffer.
> 
> Signed-off-by: Marek Szyprowski 
> ---

Reviewed-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 09/15] media: s5p-mfc: Allocate firmware with internal private buffer alloc function

[Javier Martinez Canillas]

Hell Marek,

On 02/14/2017 04:52 AM, Marek Szyprowski wrote:
> Once firmware buffer has been converted to use s5p_mfc_priv_buf structure,
> it is possible to allocate it with existing s5p_mfc_alloc_priv_buf()
> function. This change will help to reduce code variants in the next
> patches.
> 
> Signed-off-by: Marek Szyprowski 
> ---

Reviewed-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 08/15] media: s5p-mfc: Move firmware allocation to DMA configure function

[Javier Martinez Canillas]

Hello Marek,

On 02/14/2017 04:52 AM, Marek Szyprowski wrote:
> To complete DMA memory configuration for MFC device, allocation of the
> firmware buffer is needed, because some parameters are dependant on its base
> address. Till now, this has been handled in the s5p_mfc_alloc_firmware()
> function. This patch moves that logic to s5p_mfc_configure_dma_memory() to
> keep DMA memory related operations in a single place. This way
> s5p_mfc_alloc_firmware() is simplified and does what it name says. The
> other consequence of this change is moving s5p_mfc_alloc_firmware() call
> from the s5p_mfc_probe() function to the s5p_mfc_configure_dma_memory().
> 
> Signed-off-by: Marek Szyprowski 
> ---
>  drivers/media/platform/s5p-mfc/s5p_mfc.c  | 58 
> +--
>  drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c | 31 --
>  2 files changed, 45 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c 
> b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> index bc1aeb25ebeb..92a88c20b26d 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> @@ -1110,6 +1110,10 @@ static struct device *s5p_mfc_alloc_memdev(struct 
> device *dev,
>  static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev)
>  {
>   struct device *dev = _dev->plat_dev->dev;
> + void *bank2_virt;
> + dma_addr_t bank2_dma_addr;
> + unsigned long align_size = 1 << MFC_BASE_ALIGN_ORDER;
> + struct s5p_mfc_priv_buf *fw_buf = _dev->fw_buf;
>  
>   /*
>* When IOMMU is available, we cannot use the default configuration,
> @@ -1122,14 +1126,21 @@ static int s5p_mfc_configure_dma_memory(struct 
> s5p_mfc_dev *mfc_dev)
>   if (exynos_is_iommu_available(dev)) {
>   int ret = exynos_configure_iommu(dev, S5P_MFC_IOMMU_DMA_BASE,
>S5P_MFC_IOMMU_DMA_SIZE);
> - if (ret == 0) {
> - mfc_dev->mem_dev[BANK1_CTX] =
> - mfc_dev->mem_dev[BANK2_CTX] = dev;
> - vb2_dma_contig_set_max_seg_size(dev,
> - DMA_BIT_MASK(32));
> + if (ret)
> + return ret;
> +
> + mfc_dev->mem_dev[BANK1_CTX] = mfc_dev->mem_dev[BANK2_CTX] = dev;
> + ret = s5p_mfc_alloc_firmware(mfc_dev);
> + if (ret) {
> + exynos_unconfigure_iommu(dev);
> + return ret;
>   }
>  
> - return ret;
> + mfc_dev->dma_base[BANK1_CTX] = mfc_dev->fw_buf.dma;
> + mfc_dev->dma_base[BANK2_CTX] = mfc_dev->fw_buf.dma;

I guess you meant to use fw_buf->dma here? Since otherwise the fw_buf
variable won't be used.

> + vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32));
> +
> + return 0;
>   }
>  
>   /*
> @@ -1147,6 +1158,32 @@ static int s5p_mfc_configure_dma_memory(struct 
> s5p_mfc_dev *mfc_dev)
>   return -ENODEV;
>   }
>  
> + /* Allocate memory for firmware and initialize both banks addresses */
> + ret = s5p_mfc_alloc_firmware(mfc_dev);
> + if (ret)

Shouldn't you have to unregister both banks devices here in the error path?

Also, ret is not declared so this patch will cause a compile error, breaking
git bisect-ability.

> + return ret;
> +
> + mfc_dev->dma_base[BANK1_CTX] = mfc_dev->fw_buf.dma;

Same comment than before, probably you wanted to use fw_buf->dma here?

The rest of the patch looks good to me. 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

RE: Dead code in v4l2-mem2mem.c?

[Shaobo]

Hi Laurent,

Thanks a lot for your reply.

I would like to also point out the inconsistency of using `v4l2_m2m_get_vq`
inside drivers/media/v4l2-core/v4l2-mem2mem.c and inside other files. It
appears to me almost all call sites of `v4l2_m2m_get_vq` in
drivers/media/v4l2-core/v4l2-mem2mem.c does not have NULL check afterwards
while in other files (e.g., drivers/media/platform/mx2_emmaprp.c) they do. I
was wondering if there is special assumption on this function in mem2mem.c.

Best,
Shaobo
-Original Message-
From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] 
Sent: 2017年2月17日 3:26
To: Shaobo 
Cc: linux-media@vger.kernel.org; mche...@kernel.org; hverk...@xs4all.nl;
sakari.ai...@linux.intel.com; ricardo.riba...@gmail.com
Subject: Re: Dead code in v4l2-mem2mem.c?

Hi Shaobo,

First of all, could you please make sure you send future mails to the linux-
media mailing list in plain text only (no HTML) ? The mailing list server
rejects HTML e-mails.

On Thursday 16 Feb 2017 16:08:25 Shaobo wrote:
> Hi there,
> 
> My name is Shaobo He and I am a graduate student at University of 
> Utah. I am applying a static analysis tool to the Linux device 
> drivers, looking for NULL pointer dereference and accidentally found a 
> plausible dead code location in v4l2-mem2mem.c due to undefined behavior.
> 
> The following is the problematic code segment,
> 
> static struct v4l2_m2m_queue_ctx *get_queue_ctx(struct v4l2_m2m_ctx 
> *m2m_ctx,
> enum v4l2_buf_type type)
> {
>   if (V4L2_TYPE_IS_OUTPUT(type))
>   return _ctx->out_q_ctx;
>   else
>   return _ctx->cap_q_ctx;
> }
> 
> struct vb2_queue *v4l2_m2m_get_vq(struct v4l2_m2m_ctx *m2m_ctx,
>   enum v4l2_buf_type type)
> {
>   struct v4l2_m2m_queue_ctx *q_ctx;
> 
>   q_ctx = get_queue_ctx(m2m_ctx, type);
>   if (!q_ctx)
>   return NULL;
> 
>   return _ctx->q;
> }
> 
> `get_queue_ctx` returns a pointer value that is an addition of the 
> base pointer address (`m2m_ctx`) to a non-zero offset. The following 
> is the definition of struct v4l2_m2m_ctx,
> 
> struct v4l2_m2m_ctx {
>   /* optional cap/out vb2 queues lock */
>   struct mutex*q_lock;
> 
>   /* internal use only */
>   struct v4l2_m2m_dev *m2m_dev;
> 
>   struct v4l2_m2m_queue_ctx   cap_q_ctx;
> 
>   struct v4l2_m2m_queue_ctx   out_q_ctx;
> 
>   /* For device job queue */
>   struct list_headqueue;
>   unsigned long   job_flags;
>   wait_queue_head_t   finished;
> 
>   void*priv;
> };
> 
> There is a NULL test in a caller of `get_queue_ctx` (line 85), which 
> appears problematic to me. I'm not sure if it is defined or feasible 
> under the context of Linux kernel. This blog
> (https://wdtz.org/undefined-behavior-in-binutils-causes-segfault.html)
> suggests that the NULL check can be optimized away because the only 
> case that the return value can be NULL triggers pointer overflow, 
> which is undefined.
> 
> Please let me know if it makes sense or not. Thanks for your time and 
> I am looking forward to your reply.

The NULL check is indeed wrong. I believe that the m2m_ctx argument passed
to the v4l2_m2m_get_vq() function should never be NULL. We will however need
to audit drivers to make sure that's the case. The NULL check could then be
removed. Alternatively we could check m2m_ctx above the get_queue_ctx()
call, which wouldn't require auditing drivers. It's a safe option, but would
likely result in an unneeded NULL check.

--
Regards,

Laurent Pinchart

Re: [PATCH 07/15] media: s5p-mfc: Put firmware to private buffer structure

[Javier Martinez Canillas]

Hello Marek,

On 02/14/2017 04:52 AM, Marek Szyprowski wrote:
> Use s5p_mfc_priv_buf structure for keeping the firmware image. This will
> help handling of firmware buffer allocation in the next patches.
> 
> Signed-off-by: Marek Szyprowski 
> ---

Reviewed-by: Javier Martinez Canillas 
Tested-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v4 23/36] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Steve Longerbeam]

On 02/17/2017 06:16 AM, Philipp Zabel wrote:

On Fri, 2017-02-17 at 11:47 +0100, Philipp Zabel wrote:

On Wed, 2017-02-15 at 18:19 -0800, Steve Longerbeam wrote:

+static void csi2_dphy_init(struct csi2_dev *csi2)
+{
+   /*
+* FIXME: 0x14 is derived from a fixed D-PHY reference
+* clock from the HSI_TX PLL, and a fixed target lane max
+* bandwidth of 300 Mbps. This value should be derived


If the table in https://community.nxp.com/docs/DOC-94312 is correct,
this should be 850 Mbps. Where does this 300 Mbps value come from?


I got it, the dptdin_map value for 300 Mbps is 0x14 in the Rockchip DSI
driver. But that value is written to the register as HSFREQRANGE_SEL(x):

#define HSFREQRANGE_SEL(val)(((val) & 0x3f) << 1)


Ah you are right, 0x14 would be a "testdin" value of 0x0a, which from
the Rockchip table would be 950 MHz per lane.

But thanks for pointing the table at
https://community.nxp.com/docs/DOC-94312. That table is what
should be referenced in the above comment (850 MHz per lane
for a 27MHz reference clock). I will update the comment based
on that table.

Steve





which is 0x28. Further, the Rockchip D-PHY probably is another version,
as its max_mbps goes up to 1500.

Re: [PATCH 06/15] media: s5p-mfc: Move setting DMA max segmetn size to DMA configure function

[Javier Martinez Canillas]

Hello Marek,

On 02/14/2017 04:51 AM, Marek Szyprowski wrote:
> Setting DMA max segment size to 32 bit mask is a part of DMA memory
> configuration, so move those calls to s5p_mfc_configure_dma_memory()
> function.
> 
> Signed-off-by: Marek Szyprowski 
> ---

Reviewed-by: Javier Martinez Canillas 
Tested-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 05/15] media: s5p-mfc: Simplify alloc/release private buffer functions

[Javier Martinez Canillas]

Hello Marek,

On 02/14/2017 04:51 AM, Marek Szyprowski wrote:
> Change parameters for s5p_mfc_alloc_priv_buf() and s5p_mfc_release_priv_buf()
> functions. Instead of DMA device pointer and a base, provide common MFC
> device structure and memory bank context identifier.
> 
> Signed-off-by: Marek Szyprowski 
> ---

Reviewed-by: Javier Martinez Canillas 
Tested-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 01/15] media: s5p-mfc: Remove unused structures and dead code

[Javier Martinez Canillas]

Hello Marek,

On 02/14/2017 04:51 AM, Marek Szyprowski wrote:
> Remove unused structures, definitions and functions that are no longer
> called from the driver code.
> 
> Signed-off-by: Marek Szyprowski 
> ---

Reviewed-by: Javier Martinez Canillas 

Also on an Exynos5422 Odroid XU4 and Exyos5800 Peach Pi boards:

Tested-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 02/15] media: s5p-mfc: Use generic of_device_get_match_data helper

[Javier Martinez Canillas]

Hello Marek,

On 02/14/2017 04:51 AM, Marek Szyprowski wrote:
> Replace custom code with generic helper to retrieve driver data.
> 
> Signed-off-by: Marek Szyprowski 
> ---

Reviewed-by: Javier Martinez Canillas 
Tested-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 04/15] media: s5p-mfc: Replace bank1/bank2 entries with an array

[Javier Martinez Canillas]

Hello Marek,

On 02/14/2017 04:51 AM, Marek Szyprowski wrote:
> Internal MFC driver device structure contains two entries for keeping
> addresses of the DMA memory banks. Replace them with the dma_base[] array
> and use defines for accessing particular banks. This will help to simplify
> code in the next patches.
> 
> Signed-off-by: Marek Szyprowski 
> ---

Reviewed-by: Javier Martinez Canillas 
Tested-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH 03/15] media: s5p-mfc: Replace mem_dev_* entries with an array

[Javier Martinez Canillas]

Hello Marek,

On 02/14/2017 04:51 AM, Marek Szyprowski wrote:
> Internal MFC driver device structure contains two pointers to devices used
> for DMA memory allocation: mem_dev_l and mem_dev_r. Replace them with the
> mem_dev[] array and use defines for accessing particular banks. This will
> help to simplify code in the next patches.
> 
> Signed-off-by: Marek Szyprowski 
> ---

Reviewed-by: Javier Martinez Canillas 
Tested-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

[PATCH] [media] Staging: media/lirc: don't call put_ir_rx on rx twice

[Colin King]

From: Colin Ian King 

There is an exit path where rx is kfree'd on put_ir_rx and then
a jump to label out_put_xx will again kfree it with another
call to put_ir_rx.  Fix this by adding a new label that avoids
this 2nd call to put_ir_rx for this specific case.

Detected with CoverityScan, CID#145119 ("Use after free")

Signed-off-by: Colin Ian King 
---
 drivers/staging/media/lirc/lirc_zilog.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
b/drivers/staging/media/lirc/lirc_zilog.c
index 34aac3e..5dd1e62 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -1597,7 +1597,7 @@ static int ir_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
i2c_set_clientdata(client, NULL);
put_ir_rx(rx, true);
ir->l.features &= ~LIRC_CAN_REC_LIRCCODE;
-   goto out_put_xx;
+   goto out_put_tx;
}
 
/* Proceed only if the Tx client is also ready */
@@ -1637,6 +1637,7 @@ static int ir_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
 out_put_xx:
if (rx != NULL)
put_ir_rx(rx, true);
+out_put_tx:
if (tx != NULL)
put_ir_tx(tx, true);
 out_put_ir:
-- 
2.10.2

Re: [PATCH v4 00/36] i.MX Media Driver

[Philipp Zabel]

On Fri, 2017-02-17 at 14:22 +0200, Sakari Ailus wrote:
> Hi Philipp, Steve and Russell,
> 
> On Fri, Feb 17, 2017 at 12:43:38PM +0100, Philipp Zabel wrote:
> > On Thu, 2017-02-16 at 14:27 -0800, Steve Longerbeam wrote:
> > > 
> > > On 02/16/2017 02:20 PM, Russell King - ARM Linux wrote:
> > > > On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote:
> > > >> In version 4:
> > > >
> > > > With this version, I get:
> > > >
> > > > [28762.892053] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x
> > > > [28762.899409] ipu1_csi0: pipeline_set_stream failed with -110
> > > >
> > > 
> > > Right, in the imx219, on exit from s_power(), the clock and data lanes
> > > must be placed in the LP-11 state. This has been done in the ov5640 and
> > > tc358743 subdevs.
> > > 
> > > If we want to bring in the patch that adds a .prepare_stream() op,
> > > the csi-2 bus would need to be placed in LP-11 in that op instead.
> > > 
> > > Philipp, should I go ahead and add your .prepare_stream() patch?
> > 
> > I think with Russell's explanation of how the imx219 sensor operates,
> > we'll have to do something before calling the sensor s_stream, but right
> > now I'm still unsure what exactly.
> 
> Indeed there appears to be no other way to achieve the LP-11 state than
> going through the streaming state for this particular sensor, apart from
> starting streaming.
> 
> Is there a particular reason why you're waiting for the transmitter to
> transfer to LP-11 state? That appears to be the last step which is done in
> the csi2_s_stream() callback.
> 
> What the sensor does next is to start streaming, and the first thing it does
> in that process is to switch to LP-11 state.
> 
> Have you tried what happens if you simply drop the LP-11 check? To me that
> would seem the right thing to do.

Removing the wait for LP-11 alone might not be an issue in my case, as
the TC358743 is known to be in stop state all along. So I just have to
make sure that the time between s_stream(csi2) starting the receiver and
s_stream(tc358743) causing LP-11 to be changed to the next state is long
enough for the receiver to detect LP-11 (which I really can't, I just
have to pray I2C transmissions are slow enough).

The problems start if we have to enable the D-PHY and deassert resets
either before the sensor enters LP-11 state or after it already started
streaming, because we don't know when the sensor drives that state on
the bus.

The latter case I is simulated easily by again changing the order so
that the "sensor" (tc358743) is enabled before the CSI-2 receiver D-PHY
initialization. The result is that captures time out, presumably because
the receiver never entered HS mode because it didn't see LP-11. The
PHY_STATE register contains 0x200, meaning RXCLKACTIVEHS (which we
should wait for in step 7.) is never set.

I tried to test the former by instead modifying the tc358743 driver a
bit:

--8<--
diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 39d4cdd328c0f..43df80903215b 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1378,8 +1378,6 @@ static int tc358743_s_dv_timings(struct v4l2_subdev *sd,
state->timings = *timings;
 
enable_stream(sd, false);
-   tc358743_set_pll(sd);
-   tc358743_set_csi(sd);
 
return 0;
 }
@@ -1469,6 +1467,11 @@ static int tc358743_g_mbus_config(struct v4l2_subdev *sd,
 
 static int tc358743_s_stream(struct v4l2_subdev *sd, int enable)
 {
+   if (enable) {
+   tc358743_set_pll(sd);
+   tc358743_set_csi(sd);
+   tc358743_set_csi_color_space(sd);
+   }
enable_stream(sd, enable);
if (!enable) {
/* Put all lanes in PL-11 state (STOPSTATE) */
@@ -1657,9 +1660,6 @@ static int tc358743_set_fmt(struct v4l2_subdev *sd,
state->vout_color_sel = vout_color_sel;
 
enable_stream(sd, false);
-   tc358743_set_pll(sd);
-   tc358743_set_csi(sd);
-   tc358743_set_csi_color_space(sd);
 
return 0;
 }
-->8--

That should enable the CSI-2 Tx and put it in LP-11 only after the CSI-2
receiver is enabled, right before starting streaming.

That did seem to work the few times I tested, but I have no idea how
this will behave with other chips that do something else to the bus
while not streaming, and whether it is ok to enable the CSI right after
the sensor without waiting for the CSI-2 bus to settle.

regards
Philipp

Re: [PATCH v4 23/36] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Philipp Zabel]

On Fri, 2017-02-17 at 11:47 +0100, Philipp Zabel wrote:
> On Wed, 2017-02-15 at 18:19 -0800, Steve Longerbeam wrote:
> > Adds MIPI CSI-2 Receiver subdev driver. This subdev is required
> > for sensors with a MIPI CSI2 interface.
> > 
> > Signed-off-by: Steve Longerbeam 
> > ---
> >  drivers/staging/media/imx/Makefile |   1 +
> >  drivers/staging/media/imx/imx6-mipi-csi2.c | 573 
> > +
> >  2 files changed, 574 insertions(+)
> >  create mode 100644 drivers/staging/media/imx/imx6-mipi-csi2.c
> > 
> > diff --git a/drivers/staging/media/imx/Makefile 
> > b/drivers/staging/media/imx/Makefile
> > index 878a126..3569625 100644
> > --- a/drivers/staging/media/imx/Makefile
> > +++ b/drivers/staging/media/imx/Makefile
> > @@ -9,3 +9,4 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-vdic.o
> >  obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-ic.o
> >  
> >  obj-$(CONFIG_VIDEO_IMX_CSI) += imx-media-csi.o
> > +obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o
> > diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c 
> > b/drivers/staging/media/imx/imx6-mipi-csi2.c
> > new file mode 100644
> > index 000..23dca80
> > --- /dev/null
> > +++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
> > @@ -0,0 +1,573 @@
> > +/*
> > + * MIPI CSI-2 Receiver Subdev for Freescale i.MX6 SOC.
> > + *
> > + * Copyright (c) 2012-2017 Mentor Graphics Inc.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + */
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include "imx-media.h"
> > +
> > +/*
> > + * there must be 5 pads: 1 input pad from sensor, and
> > + * the 4 virtual channel output pads
> > + */
> > +#define CSI2_SINK_PAD   0
> > +#define CSI2_NUM_SINK_PADS  1
> > +#define CSI2_NUM_SRC_PADS   4
> > +#define CSI2_NUM_PADS   5
> > +
> > +struct csi2_dev {
> > +   struct device  *dev;
> > +   struct v4l2_subdev  sd;
> > +   struct media_pad   pad[CSI2_NUM_PADS];
> > +   struct v4l2_mbus_framefmt format_mbus;
> > +   struct clk *dphy_clk;
> > +   struct clk *cfg_clk;
> > +   struct clk *pix_clk; /* what is this? */
> > +   void __iomem   *base;
> > +   struct v4l2_of_bus_mipi_csi2 bus;
> > +   boolon;
> > +   boolstream_on;
> > +   boolsrc_linked;
> > +   boolsink_linked[CSI2_NUM_SRC_PADS];
> > +};
> > +
> > +#define DEVICE_NAME "imx6-mipi-csi2"
> > +
> > +/* Register offsets */
> > +#define CSI2_VERSION0x000
> > +#define CSI2_N_LANES0x004
> > +#define CSI2_PHY_SHUTDOWNZ  0x008
> > +#define CSI2_DPHY_RSTZ  0x00c
> > +#define CSI2_RESETN 0x010
> > +#define CSI2_PHY_STATE  0x014
> > +#define PHY_STOPSTATEDATA_BIT   4
> > +#define PHY_STOPSTATEDATA(n)BIT(PHY_STOPSTATEDATA_BIT + (n))
> > +#define PHY_RXCLKACTIVEHS   BIT(8)
> > +#define PHY_RXULPSCLKNOTBIT(9)
> > +#define PHY_STOPSTATECLKBIT(10)
> > +#define CSI2_DATA_IDS_1 0x018
> > +#define CSI2_DATA_IDS_2 0x01c
> > +#define CSI2_ERR1   0x020
> > +#define CSI2_ERR2   0x024
> > +#define CSI2_MSK1   0x028
> > +#define CSI2_MSK2   0x02c
> > +#define CSI2_PHY_TST_CTRL0  0x030
> > +#define PHY_TESTCLRBIT(0)
> > +#define PHY_TESTCLKBIT(1)
> > +#define CSI2_PHY_TST_CTRL1  0x034
> > +#define PHY_TESTEN BIT(16)
> > +#define CSI2_SFT_RESET  0xf00
> > +
> > +static inline struct csi2_dev *sd_to_dev(struct v4l2_subdev *sdev)
> > +{
> > +   return container_of(sdev, struct csi2_dev, sd);
> > +}
> > +
> > +static void csi2_enable(struct csi2_dev *csi2, bool enable)
> > +{
> > +   if (enable) {
> > +   writel(0x1, csi2->base + CSI2_PHY_SHUTDOWNZ);
> > +   writel(0x1, csi2->base + CSI2_DPHY_RSTZ);
> > +   writel(0x1, csi2->base + CSI2_RESETN);
> > +   } else {
> > +   writel(0x0, csi2->base + CSI2_PHY_SHUTDOWNZ);
> > +   writel(0x0, csi2->base + CSI2_DPHY_RSTZ);
> > +   writel(0x0, csi2->base + CSI2_RESETN);
> > +   }
> > +}
> > +
> > +static void csi2_set_lanes(struct csi2_dev *csi2)
> > +{
> > +   int lanes = csi2->bus.num_data_lanes;
> > +
> > +   writel(lanes - 1, csi2->base + CSI2_N_LANES);
> > +}
> > +
> > +static void dw_mipi_csi2_phy_write(struct csi2_dev *csi2,
> > +  u32 test_code, u32 test_data)
> > +{
> > +   /* Clear PHY test interface */
> > +   writel(PHY_TESTCLR, csi2->base + CSI2_PHY_TST_CTRL0);
> > +   writel(0x0, csi2->base + CSI2_PHY_TST_CTRL1);
> > +   writel(0x0, csi2->base + 

v4l2: Adding support for multiple MIPI CSI-2 virtual channels

[Thomas Axelsson]

Hi,

I have a v4l2_subdev that provides multiple MIPI CSI-2 Virtual Channels. I want 
to configure each virtual channel individually (e.g. set_fmt), but the v4l2 
interface does not seem to have a clear way to access configuration on a 
virtual channel level, but only the v4l2_subdev as a whole. Using one 
v4l2_subdev for multiple virtual channels by extending the "reg" tag to be an 
array looks like the correct way to do it, based on the mipi-dsi-bus.txt 
document and current device tree endpoint structure.

However, I cannot figure out how to extend e.g. set_fmt/get_fmt subdev ioctls 
to specify which virtual channel the call applies to. Does anyone have any 
advice on how to handle this case?

Previous thread: "Device Tree formatting for multiple virtual channels in 
ti-vpe/cal driver?"


Best Regards,
Thomas Axelsson

PS. First e-mail seems to have gotten caught in the spam filter. I apologize if 
this is a duplicate.

[PATCH v9 0/2] Add support for Omnivision OV5647

[Ramiro Oliveira]

Hello,

This patchset adds support for the Omnivision OV5647 sensor.

At the moment it only supports 640x480 in RAW 8.

This is the ninth version of the OV5647 camera driver patchset.

v9:
 - Remove unused struct
 - Remove comments
 - Refactor error handling in i2c r/w functions
 - Change declarations to single line.
 - Remove value assignment in variable declarion
 - Refactor configurion write loop 
 - Change the variable type that received ov5647_read() read value
 - Remove print from probe function
 - Remove unused device struct
 - Remove OF dependency from Kconfig
Suggested-by: Vladimir Zapolskiy 

v8:
 - Remove a part of the initialization procedure which wasn't doing 
 anything
 - Check for i2c read/writes return values
 - Add stream_on/off functions
Suggested-by: Sakari Ailus 

v7:
 - Remove "0x" and leading 0 from DT documentation examples

v6:
 - Add example to DT documentation
 - Remove data-lanes and clock-lane property from DT
 - Add external clock property to DT
 - Order includes
 - Remove unused variables and functions
 - Add external clock handling
 - Add power on counter
 - Change from g/s_parm to g/s_frame_interval

v5:
 - Refactor code 
 - Change comments
 - Add missing error handling in some functions

v4: 
 - Add correct license
 - Revert debugging info to generic infrastructure
 - Turn defines into enums
 - Correct code style issues
 - Remove unused defines
 - Make sure all errors where being handled
 - Rename some functions to make code more readable
 - Add some debugging info

v3: 
 - No changes. Re-submitted due to lack of responses

v2: 
 - Corrections in DT documentation


Ramiro Oliveira (2):
  Add OV5647 device tree documentation
  Add support for OV5647 sensor.

 .../devicetree/bindings/media/i2c/ov5647.txt   |  35 ++
 MAINTAINERS|   7 +
 drivers/media/i2c/Kconfig  |  11 +
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/ov5647.c | 638 +
 5 files changed, 692 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5647.txt
 create mode 100644 drivers/media/i2c/ov5647.c

-- 
2.11.0

[PATCH v9 2/2] Add support for OV5647 sensor.

[Ramiro Oliveira]

The OV5647 sensor from Omnivision supports up to 2592x1944 @ 15 fps, RAW 8
and RAW 10 output formats, and MIPI CSI-2 interface.

The driver adds support for 640x480 RAW 8.

Signed-off-by: Ramiro Oliveira 
---
 MAINTAINERS|   7 +
 drivers/media/i2c/Kconfig  |  11 +
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/ov5647.c | 638 +
 4 files changed, 657 insertions(+)
 create mode 100644 drivers/media/i2c/ov5647.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 8e7e8d7855ee..7bbca271acc8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9109,6 +9109,13 @@ M:   Harald Welte 
 S: Maintained
 F: drivers/char/pcmcia/cm4040_cs.*
 
+OMNIVISION OV5647 SENSOR DRIVER
+M: Ramiro Oliveira 
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/i2c/ov5647.c
+
 OMNIVISION OV7670 SENSOR DRIVER
 M: Jonathan Corbet 
 L: linux-media@vger.kernel.org
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index cee1dae6e014..8435b99f38bc 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -531,6 +531,17 @@ config VIDEO_OV2659
  To compile this driver as a module, choose M here: the
  module will be called ov2659.
 
+config VIDEO_OV5647
+   tristate "OmniVision OV5647 sensor support"
+   depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+   depends on MEDIA_CAMERA_SUPPORT
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV5647 camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov5647.
+
 config VIDEO_OV7640
tristate "OmniVision OV7640 sensor support"
depends on I2C && VIDEO_V4L2
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 5bc7bbeb5499..ef2ccf65f94c 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -83,3 +83,4 @@ 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_OV5647) += ov5647.o
diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
new file mode 100644
index ..34e620fabbaf
--- /dev/null
+++ b/drivers/media/i2c/ov5647.c
@@ -0,0 +1,638 @@
+/*
+ * A V4L2 driver for OmniVision OV5647 cameras.
+ *
+ * Based on Samsung S5K6AAFX SXGA 1/6" 1.3M CMOS Image Sensor driver
+ * Copyright (C) 2011 Sylwester Nawrocki 
+ *
+ * Based on Omnivision OV7670 Camera Driver
+ * Copyright (C) 2006-7 Jonathan Corbet 
+ *
+ * Copyright (C) 2016, Synopsys, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed .as is. WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define SENSOR_NAME "ov5647"
+
+#define OV5647_SW_RESET0x1003
+#define OV5647_REG_CHIPID_H0x300A
+#define OV5647_REG_CHIPID_L0x300B
+
+#define REG_TERM 0xfffe
+#define VAL_TERM 0xfe
+#define REG_DLY  0x
+
+#define OV5647_ROW_START   0x01
+#define OV5647_ROW_START_MIN   0
+#define OV5647_ROW_START_MAX   2004
+#define OV5647_ROW_START_DEF   54
+
+#define OV5647_COLUMN_START0x02
+#define OV5647_COLUMN_START_MIN0
+#define OV5647_COLUMN_START_MAX2750
+#define OV5647_COLUMN_START_DEF16
+
+#define OV5647_WINDOW_HEIGHT   0x03
+#define OV5647_WINDOW_HEIGHT_MIN   2
+#define OV5647_WINDOW_HEIGHT_MAX   2006
+#define OV5647_WINDOW_HEIGHT_DEF   1944
+
+#define OV5647_WINDOW_WIDTH0x04
+#define OV5647_WINDOW_WIDTH_MIN2
+#define OV5647_WINDOW_WIDTH_MAX2752
+#define OV5647_WINDOW_WIDTH_DEF2592
+
+struct regval_list {
+   u16 addr;
+   u8 data;
+};
+
+struct ov5647 {
+   struct v4l2_subdev  sd;
+   struct media_padpad;
+   struct mutexlock;
+   struct v4l2_mbus_framefmt   format;
+   unsigned intwidth;
+   unsigned intheight;
+   int power_count;
+   struct clk  *xclk;
+   /* External clock frequency currently supported is 30MHz */
+   u32 

[PATCH v9 1/2] Add OV5647 device tree documentation

[Ramiro Oliveira]

Create device tree bindings documentation.

Signed-off-by: Ramiro Oliveira 
Acked-by: Rob Herring 
---
 .../devicetree/bindings/media/i2c/ov5647.txt   | 35 ++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5647.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov5647.txt 
b/Documentation/devicetree/bindings/media/i2c/ov5647.txt
new file mode 100644
index ..31956426d3b9
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov5647.txt
@@ -0,0 +1,35 @@
+Omnivision OV5647 raw image sensor
+-
+
+OV5647 is a raw image sensor with MIPI CSI-2 and CCP2 image data interfaces
+and CCI (I2C compatible) control bus.
+
+Required properties:
+
+- compatible   : "ovti,ov5647".
+- reg  : I2C slave address of the sensor.
+- clocks   : Reference to the xclk clock.
+- clock-names  : Should be "xclk".
+- clock-frequency  : Frequency of the xclk clock.
+
+The common video interfaces bindings (see video-interfaces.txt) should be
+used to specify link to the image data receiver. The OV5647 device
+node should contain one 'port' child node with an 'endpoint' subnode.
+
+Example:
+
+   i2c@2000 {
+   ...
+   ov: camera@36 {
+   compatible = "ovti,ov5647";
+   reg = <0x36>;
+   clocks = <_clk>;
+   clock-names = "xclk";
+   clock-frequency = <2500>;
+   port {
+   camera_1: endpoint {
+   remote-endpoint = <_ep1>;
+   };
+   };
+   };
+   };
-- 
2.11.0

Re: [PATCH v4 00/36] i.MX Media Driver

[Russell King - ARM Linux]

On Fri, Feb 17, 2017 at 02:22:14PM +0200, Sakari Ailus wrote:
> Hi Philipp, Steve and Russell,
> 
> On Fri, Feb 17, 2017 at 12:43:38PM +0100, Philipp Zabel wrote:
> > I think with Russell's explanation of how the imx219 sensor operates,
> > we'll have to do something before calling the sensor s_stream, but right
> > now I'm still unsure what exactly.
> 
> Indeed there appears to be no other way to achieve the LP-11 state than
> going through the streaming state for this particular sensor, apart from
> starting streaming.
> 
> Is there a particular reason why you're waiting for the transmitter to
> transfer to LP-11 state? That appears to be the last step which is done in
> the csi2_s_stream() callback.
> 
> What the sensor does next is to start streaming, and the first thing it does
> in that process is to switch to LP-11 state.
> 
> Have you tried what happens if you simply drop the LP-11 check? To me that
> would seem the right thing to do.

The Freescale documentation for iMX6's CSI2 receiver (chapter 40.3.1)
specifies a very specific sequence to be followed to safely bring up the
CSI2 receiver.  Bold text gets used, which implies emphasis on certain
points, which suggests that it's important to follow it.

Presumably, the reason for this is to ensure that a state machine within
the CSI2 receiver is properly synchronised to the incoming data stream,
and while avoiding the sequence may work, it may not be guaranteed to
work every time.

I guess we need someone from NXP to comment.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 00/35] treewide trivial patches converting pr_warning to pr_warn

[Rafael J. Wysocki]

On Fri, Feb 17, 2017 at 8:11 AM, Joe Perches  wrote:
> There are ~4300 uses of pr_warn and ~250 uses of the older
> pr_warning in the kernel source tree.
>
> Make the use of pr_warn consistent across all kernel files.
>
> This excludes all files in tools/ as there is a separate
> define pr_warning for that directory tree and pr_warn is
> not used in tools/.
>
> Done with 'sed s/\bpr_warning\b/pr_warn/' and some emacsing.

Sorry about asking if that has been asked already.

Wouldn't it be slightly less intrusive to simply redefined
pr_warning() as a synonym for pr_warn()?

Thanks,
Rafael

Re: [PATCH v4 00/36] i.MX Media Driver

[Sakari Ailus]

Hi Philipp, Steve and Russell,

On Fri, Feb 17, 2017 at 12:43:38PM +0100, Philipp Zabel wrote:
> On Thu, 2017-02-16 at 14:27 -0800, Steve Longerbeam wrote:
> > 
> > On 02/16/2017 02:20 PM, Russell King - ARM Linux wrote:
> > > On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote:
> > >> In version 4:
> > >
> > > With this version, I get:
> > >
> > > [28762.892053] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x
> > > [28762.899409] ipu1_csi0: pipeline_set_stream failed with -110
> > >
> > 
> > Right, in the imx219, on exit from s_power(), the clock and data lanes
> > must be placed in the LP-11 state. This has been done in the ov5640 and
> > tc358743 subdevs.
> > 
> > If we want to bring in the patch that adds a .prepare_stream() op,
> > the csi-2 bus would need to be placed in LP-11 in that op instead.
> > 
> > Philipp, should I go ahead and add your .prepare_stream() patch?
> 
> I think with Russell's explanation of how the imx219 sensor operates,
> we'll have to do something before calling the sensor s_stream, but right
> now I'm still unsure what exactly.

Indeed there appears to be no other way to achieve the LP-11 state than
going through the streaming state for this particular sensor, apart from
starting streaming.

Is there a particular reason why you're waiting for the transmitter to
transfer to LP-11 state? That appears to be the last step which is done in
the csi2_s_stream() callback.

What the sensor does next is to start streaming, and the first thing it does
in that process is to switch to LP-11 state.

Have you tried what happens if you simply drop the LP-11 check? To me that
would seem the right thing to do.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCH v4 00/36] i.MX Media Driver

[Philipp Zabel]

On Thu, 2017-02-16 at 14:27 -0800, Steve Longerbeam wrote:
> 
> On 02/16/2017 02:20 PM, Russell King - ARM Linux wrote:
> > On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote:
> >> In version 4:
> >
> > With this version, I get:
> >
> > [28762.892053] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x
> > [28762.899409] ipu1_csi0: pipeline_set_stream failed with -110
> >
> 
> Right, in the imx219, on exit from s_power(), the clock and data lanes
> must be placed in the LP-11 state. This has been done in the ov5640 and
> tc358743 subdevs.
> 
> If we want to bring in the patch that adds a .prepare_stream() op,
> the csi-2 bus would need to be placed in LP-11 in that op instead.
> 
> Philipp, should I go ahead and add your .prepare_stream() patch?

I think with Russell's explanation of how the imx219 sensor operates,
we'll have to do something before calling the sensor s_stream, but right
now I'm still unsure what exactly.

regards
Philipp

Re: [PATCH v4 23/36] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Philipp Zabel]

On Fri, 2017-02-17 at 11:06 +, Russell King - ARM Linux wrote:
> On Fri, Feb 17, 2017 at 11:47:59AM +0100, Philipp Zabel wrote:
> > On Wed, 2017-02-15 at 18:19 -0800, Steve Longerbeam wrote:
> > > +static void csi2_dphy_init(struct csi2_dev *csi2)
> > > +{
> > > + /*
> > > +  * FIXME: 0x14 is derived from a fixed D-PHY reference
> > > +  * clock from the HSI_TX PLL, and a fixed target lane max
> > > +  * bandwidth of 300 Mbps. This value should be derived
> > 
> > If the table in https://community.nxp.com/docs/DOC-94312 is correct,
> > this should be 850 Mbps. Where does this 300 Mbps value come from?
> 
> I thought you had some code to compute the correct value, although
> I guess we've lost the ability to know how fast the sensor is going
> to drive the link.

I had code to calculate the number of needed lanes from the bit rate and
link frequency. I did not actually change the D-PHY register value.
And as you pointed out, calculating the number of lanes is not useful
without input from the sensor driver, as some lane configurations might
not be supported.

> Note that the IMX219 currently drives the data lanes at 912Mbps almost
> exclusively, as I've yet to finish working out how to derive the PLL
> parameters.  (I have something that works, but it currently takes on
> the order of 100k iterations to derive the parameters.  gcd() doesn't
> help you in this instance.)

The tc358743 also currently only implements a fixed rate (of 594 Mbps).

regards
Philipp

Re: [PATCH v4 00/36] i.MX Media Driver

[Philipp Zabel]

On Fri, 2017-02-17 at 10:56 +, Russell King - ARM Linux wrote:
> On Fri, Feb 17, 2017 at 11:39:11AM +0100, Philipp Zabel wrote:
> > On Thu, 2017-02-16 at 22:57 +, Russell King - ARM Linux wrote:
> > > On Thu, Feb 16, 2017 at 02:27:41PM -0800, Steve Longerbeam wrote:
> > > > 
> > > > 
> > > > On 02/16/2017 02:20 PM, Russell King - ARM Linux wrote:
> > > > >On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote:
> > > > >>In version 4:
> > > > >
> > > > >With this version, I get:
> > > > >
> > > > >[28762.892053] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x
> > > > >[28762.899409] ipu1_csi0: pipeline_set_stream failed with -110
> > > > >
> > > > 
> > > > Right, in the imx219, on exit from s_power(), the clock and data lanes
> > > > must be placed in the LP-11 state. This has been done in the ov5640 and
> > > > tc358743 subdevs.
> > > 
> > > The only way to do that is to enable streaming from the sensor, wait
> > > an initialisation time, and then disable streaming, and wait for the
> > > current line to finish.  There is _no_ other way to get the sensor to
> > > place its clock and data lines into LP-11 state.
> > 
> > I thought going through LP-11 is part of the D-PHY transmitter
> > initialization, during the LP->HS wakeup sequence. But then I have no
> > access to MIPI specs.
> 
> The D-PHY transmitter initialisation *only* happens as part of the
> wake-up from standby to streaming mode.  That is because Sony expect
> that you program the sensor, and then when you switch it to streaming
> mode, it computes the D-PHY parameters from the PLL, input clock rate
> (you have to tell it the clock rate in 1/256 MHz units), number of
> lanes, and other parameters.
> 
> It is possible to program the D-PHY parameters manually, but that
> doesn't change the above sequence in any way (it just avoids the
> chip computing the values, it doesn't result in any change of
> behaviour on the bus.)
>
> The IMX219 specifications are clear: the clock and data lines are
> held low (LP-00 state) after releasing the hardware enable signal.
> There's a period of chip initialisation, and then you can access the
> I2C bus and configure it.  There's a further period of initialisation
> where charge pumps are getting to their operating state.  Then, you
> set the streaming bit, and a load more initialisation happens before
> the CSI bus enters LP-11 state and the first frame pops out.  When
> entering standby, the last frame is completed, and then the CSI bus
> enters LP-11 state.

How about firing off a thread in imx6-mipi-csi2 prepare_stream that
spins on the LP-11 check and then continues with the receiver D-PHY
initialization once the condition is met? I think we should have at
least 100 us to do this, but maybe the IMX219 can be programmed to stay
in LP-11 for a longer time.

> SMIA are slightly different - mostly following what I've said above,
> but the clock and data lines are tristated after releasing the
> xshutdown signal, and they remain tristated until the clock line
> starts toggling before the first frame appears.  There appears to
> be no point that the clock line enters LP-11 state before it starts
> toggling.  When entering standby, the last frame is completed, and
> the CSI bus enters tristate mode (so floating.)  There is no way to
> get these sensors into LP-11 state.

I have no idea what to do about those.

regards
Philipp

[PATCH] CEC documentation fixes

[Hans Verkuil]

Fixed a few spelling mistakes, but mostly incorrect rst syntax that caused wrong
references or font style.

No actual documentation changes, just fixes.

Signed-off-by: Hans Verkuil 
---
diff --git a/Documentation/media/uapi/cec/cec-func-ioctl.rst 
b/Documentation/media/uapi/cec/cec-func-ioctl.rst
index 9e8dbb118d6a..071d18cec7b7 100644
--- a/Documentation/media/uapi/cec/cec-func-ioctl.rst
+++ b/Documentation/media/uapi/cec/cec-func-ioctl.rst
@@ -30,7 +30,7 @@ Arguments

 ``request``
 CEC ioctl request code as defined in the cec.h header file, for
-example :c:func:`CEC_ADAP_G_CAPS`.
+example :ref:`CEC_ADAP_G_CAPS `.

 ``argp``
 Pointer to a request-specific structure.
diff --git a/Documentation/media/uapi/cec/cec-func-open.rst 
b/Documentation/media/uapi/cec/cec-func-open.rst
index af3f5b5c24c6..5aab5cd345b1 100644
--- a/Documentation/media/uapi/cec/cec-func-open.rst
+++ b/Documentation/media/uapi/cec/cec-func-open.rst
@@ -33,7 +33,7 @@ Arguments
 Open flags. Access mode must be ``O_RDWR``.

 When the ``O_NONBLOCK`` flag is given, the
-:ref:`CEC_RECEIVE ` and :c:func:`CEC_DQEVENT` ioctls
+:ref:`CEC_RECEIVE ` and :ref:`CEC_DQEVENT ` 
ioctls
 will return the ``EAGAIN`` error code when no message or event is 
available, and
 ioctls :ref:`CEC_TRANSMIT `,
 :ref:`CEC_ADAP_S_PHYS_ADDR ` and
diff --git a/Documentation/media/uapi/cec/cec-func-poll.rst 
b/Documentation/media/uapi/cec/cec-func-poll.rst
index cfb73e6027a5..d48dee0f00d6 100644
--- a/Documentation/media/uapi/cec/cec-func-poll.rst
+++ b/Documentation/media/uapi/cec/cec-func-poll.rst
@@ -30,7 +30,7 @@ Arguments
List of FD events to be watched

 ``nfds``
-   Number of FD efents at the \*ufds array
+   Number of FD events at the \*ufds array

 ``timeout``
Timeout to wait for events
@@ -54,7 +54,7 @@ is non-zero). CEC devices set the ``POLLIN`` and 
``POLLRDNORM`` flags in
 the ``revents`` field if there are messages in the receive queue. If the
 transmit queue has room for new messages, the ``POLLOUT`` and
 ``POLLWRNORM`` flags are set. If there are events in the event queue,
-then the ``POLLPRI`` flag is set. When the function timed out it returns
+then the ``POLLPRI`` flag is set. When the function times out it returns
 a value of zero, on failure it returns -1 and the ``errno`` variable is
 set appropriately.

diff --git a/Documentation/media/uapi/cec/cec-ioc-dqevent.rst 
b/Documentation/media/uapi/cec/cec-ioc-dqevent.rst
index e256c6605de7..012e589d90ce 100644
--- a/Documentation/media/uapi/cec/cec-ioc-dqevent.rst
+++ b/Documentation/media/uapi/cec/cec-ioc-dqevent.rst
@@ -61,7 +61,7 @@ it is guaranteed that the state did change in between the two 
events.
 * - __u16
   - ``phys_addr``
   - The current physical address. This is ``CEC_PHYS_ADDR_INVALID`` if no
-  valid physical address is set.
+valid physical address is set.
 * - __u16
   - ``log_addr_mask``
   - The current set of claimed logical addresses. This is 0 if no logical
diff --git a/Documentation/media/uapi/cec/cec-ioc-receive.rst 
b/Documentation/media/uapi/cec/cec-ioc-receive.rst
index bdf015b1d1dc..3677fe6baf56 100644
--- a/Documentation/media/uapi/cec/cec-ioc-receive.rst
+++ b/Documentation/media/uapi/cec/cec-ioc-receive.rst
@@ -56,13 +56,13 @@ A received message can be:
be non-zero).

 To send a CEC message the application has to fill in the struct
-:c:type:` cec_msg` and pass it to :ref:`ioctl CEC_TRANSMIT `.
+:c:type:`cec_msg` and pass it to :ref:`ioctl CEC_TRANSMIT `.
 The :ref:`ioctl CEC_TRANSMIT ` is only available if
 ``CEC_CAP_TRANSMIT`` is set. If there is no more room in the transmit
 queue, then it will return -1 and set errno to the ``EBUSY`` error code.
 The transmit queue has enough room for 18 messages (about 1 second worth
 of 2-byte messages). Note that the CEC kernel framework will also reply
-to core messages (see :ref:cec-core-processing), so it is not a good
+to core messages (see :ref:`cec-core-processing`), so it is not a good
 idea to fully fill up the transmit queue.

 If the file descriptor is in non-blocking mode then the transmit will

Re: [PATCH v4 23/36] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Russell King - ARM Linux]

On Fri, Feb 17, 2017 at 11:47:59AM +0100, Philipp Zabel wrote:
> On Wed, 2017-02-15 at 18:19 -0800, Steve Longerbeam wrote:
> > +static void csi2_dphy_init(struct csi2_dev *csi2)
> > +{
> > +   /*
> > +* FIXME: 0x14 is derived from a fixed D-PHY reference
> > +* clock from the HSI_TX PLL, and a fixed target lane max
> > +* bandwidth of 300 Mbps. This value should be derived
> 
> If the table in https://community.nxp.com/docs/DOC-94312 is correct,
> this should be 850 Mbps. Where does this 300 Mbps value come from?

I thought you had some code to compute the correct value, although
I guess we've lost the ability to know how fast the sensor is going
to drive the link.

Note that the IMX219 currently drives the data lanes at 912Mbps almost
exclusively, as I've yet to finish working out how to derive the PLL
parameters.  (I have something that works, but it currently takes on
the order of 100k iterations to derive the parameters.  gcd() doesn't
help you in this instance.)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH v4 00/36] i.MX Media Driver

[Russell King - ARM Linux]

On Fri, Feb 17, 2017 at 11:39:11AM +0100, Philipp Zabel wrote:
> On Thu, 2017-02-16 at 22:57 +, Russell King - ARM Linux wrote:
> > On Thu, Feb 16, 2017 at 02:27:41PM -0800, Steve Longerbeam wrote:
> > > 
> > > 
> > > On 02/16/2017 02:20 PM, Russell King - ARM Linux wrote:
> > > >On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote:
> > > >>In version 4:
> > > >
> > > >With this version, I get:
> > > >
> > > >[28762.892053] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x
> > > >[28762.899409] ipu1_csi0: pipeline_set_stream failed with -110
> > > >
> > > 
> > > Right, in the imx219, on exit from s_power(), the clock and data lanes
> > > must be placed in the LP-11 state. This has been done in the ov5640 and
> > > tc358743 subdevs.
> > 
> > The only way to do that is to enable streaming from the sensor, wait
> > an initialisation time, and then disable streaming, and wait for the
> > current line to finish.  There is _no_ other way to get the sensor to
> > place its clock and data lines into LP-11 state.
> 
> I thought going through LP-11 is part of the D-PHY transmitter
> initialization, during the LP->HS wakeup sequence. But then I have no
> access to MIPI specs.

The D-PHY transmitter initialisation *only* happens as part of the
wake-up from standby to streaming mode.  That is because Sony expect
that you program the sensor, and then when you switch it to streaming
mode, it computes the D-PHY parameters from the PLL, input clock rate
(you have to tell it the clock rate in 1/256 MHz units), number of
lanes, and other parameters.

It is possible to program the D-PHY parameters manually, but that
doesn't change the above sequence in any way (it just avoids the
chip computing the values, it doesn't result in any change of
behaviour on the bus.)

The IMX219 specifications are clear: the clock and data lines are
held low (LP-00 state) after releasing the hardware enable signal.
There's a period of chip initialisation, and then you can access the
I2C bus and configure it.  There's a further period of initialisation
where charge pumps are getting to their operating state.  Then, you
set the streaming bit, and a load more initialisation happens before
the CSI bus enters LP-11 state and the first frame pops out.  When
entering standby, the last frame is completed, and then the CSI bus
enters LP-11 state.

SMIA are slightly different - mostly following what I've said above,
but the clock and data lines are tristated after releasing the
xshutdown signal, and they remain tristated until the clock line
starts toggling before the first frame appears.  There appears to
be no point that the clock line enters LP-11 state before it starts
toggling.  When entering standby, the last frame is completed, and
the CSI bus enters tristate mode (so floating.)  There is no way to
get these sensors into LP-11 state.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH v4 23/36] media: imx: Add MIPI CSI-2 Receiver subdev driver

[Philipp Zabel]

On Wed, 2017-02-15 at 18:19 -0800, Steve Longerbeam wrote:
> Adds MIPI CSI-2 Receiver subdev driver. This subdev is required
> for sensors with a MIPI CSI2 interface.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  drivers/staging/media/imx/Makefile |   1 +
>  drivers/staging/media/imx/imx6-mipi-csi2.c | 573 
> +
>  2 files changed, 574 insertions(+)
>  create mode 100644 drivers/staging/media/imx/imx6-mipi-csi2.c
> 
> diff --git a/drivers/staging/media/imx/Makefile 
> b/drivers/staging/media/imx/Makefile
> index 878a126..3569625 100644
> --- a/drivers/staging/media/imx/Makefile
> +++ b/drivers/staging/media/imx/Makefile
> @@ -9,3 +9,4 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-vdic.o
>  obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-ic.o
>  
>  obj-$(CONFIG_VIDEO_IMX_CSI) += imx-media-csi.o
> +obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o
> diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c 
> b/drivers/staging/media/imx/imx6-mipi-csi2.c
> new file mode 100644
> index 000..23dca80
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
> @@ -0,0 +1,573 @@
> +/*
> + * MIPI CSI-2 Receiver Subdev for Freescale i.MX6 SOC.
> + *
> + * Copyright (c) 2012-2017 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "imx-media.h"
> +
> +/*
> + * there must be 5 pads: 1 input pad from sensor, and
> + * the 4 virtual channel output pads
> + */
> +#define CSI2_SINK_PAD   0
> +#define CSI2_NUM_SINK_PADS  1
> +#define CSI2_NUM_SRC_PADS   4
> +#define CSI2_NUM_PADS   5
> +
> +struct csi2_dev {
> + struct device  *dev;
> + struct v4l2_subdev  sd;
> + struct media_pad   pad[CSI2_NUM_PADS];
> + struct v4l2_mbus_framefmt format_mbus;
> + struct clk *dphy_clk;
> + struct clk *cfg_clk;
> + struct clk *pix_clk; /* what is this? */
> + void __iomem   *base;
> + struct v4l2_of_bus_mipi_csi2 bus;
> + boolon;
> + boolstream_on;
> + boolsrc_linked;
> + boolsink_linked[CSI2_NUM_SRC_PADS];
> +};
> +
> +#define DEVICE_NAME "imx6-mipi-csi2"
> +
> +/* Register offsets */
> +#define CSI2_VERSION0x000
> +#define CSI2_N_LANES0x004
> +#define CSI2_PHY_SHUTDOWNZ  0x008
> +#define CSI2_DPHY_RSTZ  0x00c
> +#define CSI2_RESETN 0x010
> +#define CSI2_PHY_STATE  0x014
> +#define PHY_STOPSTATEDATA_BIT   4
> +#define PHY_STOPSTATEDATA(n)BIT(PHY_STOPSTATEDATA_BIT + (n))
> +#define PHY_RXCLKACTIVEHS   BIT(8)
> +#define PHY_RXULPSCLKNOTBIT(9)
> +#define PHY_STOPSTATECLKBIT(10)
> +#define CSI2_DATA_IDS_1 0x018
> +#define CSI2_DATA_IDS_2 0x01c
> +#define CSI2_ERR1   0x020
> +#define CSI2_ERR2   0x024
> +#define CSI2_MSK1   0x028
> +#define CSI2_MSK2   0x02c
> +#define CSI2_PHY_TST_CTRL0  0x030
> +#define PHY_TESTCLR  BIT(0)
> +#define PHY_TESTCLK  BIT(1)
> +#define CSI2_PHY_TST_CTRL1  0x034
> +#define PHY_TESTEN   BIT(16)
> +#define CSI2_SFT_RESET  0xf00
> +
> +static inline struct csi2_dev *sd_to_dev(struct v4l2_subdev *sdev)
> +{
> + return container_of(sdev, struct csi2_dev, sd);
> +}
> +
> +static void csi2_enable(struct csi2_dev *csi2, bool enable)
> +{
> + if (enable) {
> + writel(0x1, csi2->base + CSI2_PHY_SHUTDOWNZ);
> + writel(0x1, csi2->base + CSI2_DPHY_RSTZ);
> + writel(0x1, csi2->base + CSI2_RESETN);
> + } else {
> + writel(0x0, csi2->base + CSI2_PHY_SHUTDOWNZ);
> + writel(0x0, csi2->base + CSI2_DPHY_RSTZ);
> + writel(0x0, csi2->base + CSI2_RESETN);
> + }
> +}
> +
> +static void csi2_set_lanes(struct csi2_dev *csi2)
> +{
> + int lanes = csi2->bus.num_data_lanes;
> +
> + writel(lanes - 1, csi2->base + CSI2_N_LANES);
> +}
> +
> +static void dw_mipi_csi2_phy_write(struct csi2_dev *csi2,
> +u32 test_code, u32 test_data)
> +{
> + /* Clear PHY test interface */
> + writel(PHY_TESTCLR, csi2->base + CSI2_PHY_TST_CTRL0);
> + writel(0x0, csi2->base + CSI2_PHY_TST_CTRL1);
> + writel(0x0, csi2->base + CSI2_PHY_TST_CTRL0);
> +
> + /* Raise test interface strobe signal */
> + writel(PHY_TESTCLK, csi2->base + CSI2_PHY_TST_CTRL0);
> +
> + /* Configure address write on falling edge and lower strobe signal */
> + writel(PHY_TESTEN | test_code, csi2->base + 

[PATCH v3 3/3] arm: sti: update sti-cec for HPD notifier support

[Benjamin Gaignard]

To use HPD notifier sti CEC driver needs to get phandle
of the hdmi device.

Signed-off-by: Benjamin Gaignard 
Signed-off-by: Hans Verkuil 
CC: devicet...@vger.kernel.org

version 3:
- change hdmi phandle from "st,hdmi-handle" to "hdmi-handle"
---
 arch/arm/boot/dts/stih407-family.dtsi | 12 
 arch/arm/boot/dts/stih410.dtsi| 13 +
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/arch/arm/boot/dts/stih407-family.dtsi 
b/arch/arm/boot/dts/stih407-family.dtsi
index c8b2944..592d235 100644
--- a/arch/arm/boot/dts/stih407-family.dtsi
+++ b/arch/arm/boot/dts/stih407-family.dtsi
@@ -756,18 +756,6 @@
 <_s_c0_flexgen CLK_ETH_PHY>;
};
 
-   cec: sti-cec@094a087c {
-   compatible = "st,stih-cec";
-   reg = <0x94a087c 0x64>;
-   clocks = <_sysin>;
-   clock-names = "cec-clk";
-   interrupts = ;
-   interrupt-names = "cec-irq";
-   pinctrl-names = "default";
-   pinctrl-0 = <_cec0_default>;
-   resets = < STIH407_LPM_SOFTRESET>;
-   };
-
rng10: rng@08a89000 {
compatible  = "st,rng";
reg = <0x08a89000 0x1000>;
diff --git a/arch/arm/boot/dts/stih410.dtsi b/arch/arm/boot/dts/stih410.dtsi
index 281a124..71deb02 100644
--- a/arch/arm/boot/dts/stih410.dtsi
+++ b/arch/arm/boot/dts/stih410.dtsi
@@ -259,5 +259,18 @@
clocks = <_sysin>;
interrupts = ;
};
+
+   sti-cec@094a087c {
+   compatible = "st,stih-cec";
+   reg = <0x94a087c 0x64>;
+   clocks = <_sysin>;
+   clock-names = "cec-clk";
+   interrupts = ;
+   interrupt-names = "cec-irq";
+   pinctrl-names = "default";
+   pinctrl-0 = <_cec0_default>;
+   resets = < STIH407_LPM_SOFTRESET>;
+   hdmi-handle = <_hdmi>;
+   };
};
 };
-- 
1.9.1

[PATCH v3 1/3] sti: hdmi: add HPD notifier support

[Benjamin Gaignard]

Implement the HPD notifier support to allow CEC drivers to
be informed when there is a new EDID and when a connect or
disconnect happens.

Signed-off-by: Benjamin Gaignard 
Signed-off-by: Hans Verkuil 
---
 drivers/gpu/drm/sti/Kconfig|  1 +
 drivers/gpu/drm/sti/sti_hdmi.c | 14 ++
 drivers/gpu/drm/sti/sti_hdmi.h |  3 +++
 3 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/sti/Kconfig b/drivers/gpu/drm/sti/Kconfig
index acd7286..f5c9572 100644
--- a/drivers/gpu/drm/sti/Kconfig
+++ b/drivers/gpu/drm/sti/Kconfig
@@ -8,5 +8,6 @@ config DRM_STI
select DRM_PANEL
select FW_LOADER
select SND_SOC_HDMI_CODEC if SND_SOC
+   select HPD_NOTIFIER
help
  Choose this option to enable DRM on STM stiH4xx chipset
diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index 376b076..d32a383 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -786,6 +786,8 @@ static void sti_hdmi_disable(struct drm_bridge *bridge)
clk_disable_unprepare(hdmi->clk_pix);
 
hdmi->enabled = false;
+
+   hpd_event_disconnect(hdmi->notifier);
 }
 
 static void sti_hdmi_pre_enable(struct drm_bridge *bridge)
@@ -892,6 +894,9 @@ static int sti_hdmi_connector_get_modes(struct 
drm_connector *connector)
if (!edid)
goto fail;
 
+   hpd_event_new_edid(hdmi->notifier, edid,
+  EDID_LENGTH * (edid->extensions + 1));
+
count = drm_add_edid_modes(connector, edid);
drm_mode_connector_update_edid_property(connector, edid);
drm_edid_to_eld(connector, edid);
@@ -949,10 +954,12 @@ struct drm_connector_helper_funcs 
sti_hdmi_connector_helper_funcs = {
 
if (hdmi->hpd) {
DRM_DEBUG_DRIVER("hdmi cable connected\n");
+   hpd_event_connect(hdmi->notifier);
return connector_status_connected;
}
 
DRM_DEBUG_DRIVER("hdmi cable disconnected\n");
+   hpd_event_disconnect(hdmi->notifier);
return connector_status_disconnected;
 }
 
@@ -1464,6 +1471,10 @@ static int sti_hdmi_probe(struct platform_device *pdev)
goto release_adapter;
}
 
+   hdmi->notifier = hpd_notifier_get(>dev);
+   if (!hdmi->notifier)
+   goto release_adapter;
+
hdmi->reset = devm_reset_control_get(dev, "hdmi");
/* Take hdmi out of reset */
if (!IS_ERR(hdmi->reset))
@@ -1483,11 +1494,14 @@ static int sti_hdmi_remove(struct platform_device *pdev)
 {
struct sti_hdmi *hdmi = dev_get_drvdata(>dev);
 
+   hpd_event_disconnect(hdmi->notifier);
+
i2c_put_adapter(hdmi->ddc_adapt);
if (hdmi->audio_pdev)
platform_device_unregister(hdmi->audio_pdev);
component_del(>dev, _hdmi_ops);
 
+   hpd_notifier_put(hdmi->notifier);
return 0;
 }
 
diff --git a/drivers/gpu/drm/sti/sti_hdmi.h b/drivers/gpu/drm/sti/sti_hdmi.h
index 119bc35..2109c97 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.h
+++ b/drivers/gpu/drm/sti/sti_hdmi.h
@@ -8,6 +8,7 @@
 #define _STI_HDMI_H_
 
 #include 
+#include 
 #include 
 
 #include 
@@ -77,6 +78,7 @@ enum sti_hdmi_modes {
  * @audio_pdev: ASoC hdmi-codec platform device
  * @audio: hdmi audio parameters.
  * @drm_connector: hdmi connector
+ * @notifier: hotplug detect notifier
  */
 struct sti_hdmi {
struct device dev;
@@ -102,6 +104,7 @@ struct sti_hdmi {
struct platform_device *audio_pdev;
struct hdmi_audio_params audio;
struct drm_connector *drm_connector;
+   struct hpd_notifier *notifier;
 };
 
 u32 hdmi_read(struct sti_hdmi *hdmi, int offset);
-- 
1.9.1

[PATCH v3 0/3] video/sti/cec: add HPD notifier support

[Benjamin Gaignard]

This patch series following what Hans is doing on exynos to support
hotplug detect notifier code.

It add support of HPD in sti_hdmi drm driver and stih-cec driver which
move out of staging.

Those patches should be applied on top of Hans branch exynos4-cec.

I have tested hdmi notifier by pluging/unpluging HDMI cable and check
the value of the physical address with "cec-ctl --tuner".
"cec-compliance -A" is also functional.

version 3:
- change hdmi phandle from "st,hdmi-handle" to "hdmi-handle"
- fix typo in bindings

version 2:
- use HPD notifier instead of HDMI notifier
- move stih-cec out of staging
- rebase code on top of git://linuxtv.org/hverkuil/media_tree.git exynos4-cec
  branch
- split DT modifications in a separate patch

Benjamin Gaignard (3):
  sti: hdmi: add HPD notifier support
  stih-cec: add HPD notifier support
  arm: sti: update sti-cec for HPD notifier support

 .../devicetree/bindings/media/stih-cec.txt |   2 +
 arch/arm/boot/dts/stih407-family.dtsi  |  12 -
 arch/arm/boot/dts/stih410.dtsi |  13 +
 drivers/gpu/drm/sti/Kconfig|   1 +
 drivers/gpu/drm/sti/sti_hdmi.c |  14 +
 drivers/gpu/drm/sti/sti_hdmi.h |   3 +
 drivers/media/platform/Kconfig |  10 +
 drivers/media/platform/Makefile|   1 +
 drivers/media/platform/sti/cec/Makefile|   1 +
 drivers/media/platform/sti/cec/stih-cec.c  | 404 +
 drivers/staging/media/Kconfig  |   2 -
 drivers/staging/media/Makefile |   1 -
 drivers/staging/media/st-cec/Kconfig   |   8 -
 drivers/staging/media/st-cec/Makefile  |   1 -
 drivers/staging/media/st-cec/TODO  |   7 -
 drivers/staging/media/st-cec/stih-cec.c| 379 ---
 16 files changed, 449 insertions(+), 410 deletions(-)
 create mode 100644 drivers/media/platform/sti/cec/Makefile
 create mode 100644 drivers/media/platform/sti/cec/stih-cec.c
 delete mode 100644 drivers/staging/media/st-cec/Kconfig
 delete mode 100644 drivers/staging/media/st-cec/Makefile
 delete mode 100644 drivers/staging/media/st-cec/TODO
 delete mode 100644 drivers/staging/media/st-cec/stih-cec.c

-- 
1.9.1

[PATCH v3 2/3] stih-cec: add HPD notifier support

[Benjamin Gaignard]

By using the HPD notifier framework there is no longer any reason
to manually set the physical address. This was the one blocking
issue that prevented this driver from going out of staging, so do
this move as well.

Update the bindings documentation the new hdmi phandle.

Signed-off-by: Benjamin Gaignard 
Signed-off-by: Hans Verkuil 
CC: devicet...@vger.kernel.org

version 3:
- change hdmi phandle from "st,hdmi-handle" to "hdmi-handle"
---
 .../devicetree/bindings/media/stih-cec.txt |   2 +
 drivers/media/platform/Kconfig |  10 +
 drivers/media/platform/Makefile|   1 +
 drivers/media/platform/sti/cec/Makefile|   1 +
 drivers/media/platform/sti/cec/stih-cec.c  | 404 +
 drivers/staging/media/Kconfig  |   2 -
 drivers/staging/media/Makefile |   1 -
 drivers/staging/media/st-cec/Kconfig   |   8 -
 drivers/staging/media/st-cec/Makefile  |   1 -
 drivers/staging/media/st-cec/TODO  |   7 -
 drivers/staging/media/st-cec/stih-cec.c| 379 ---
 11 files changed, 418 insertions(+), 398 deletions(-)
 create mode 100644 drivers/media/platform/sti/cec/Makefile
 create mode 100644 drivers/media/platform/sti/cec/stih-cec.c
 delete mode 100644 drivers/staging/media/st-cec/Kconfig
 delete mode 100644 drivers/staging/media/st-cec/Makefile
 delete mode 100644 drivers/staging/media/st-cec/TODO
 delete mode 100644 drivers/staging/media/st-cec/stih-cec.c

diff --git a/Documentation/devicetree/bindings/media/stih-cec.txt 
b/Documentation/devicetree/bindings/media/stih-cec.txt
index 71c4b2f..1f7da58 100644
--- a/Documentation/devicetree/bindings/media/stih-cec.txt
+++ b/Documentation/devicetree/bindings/media/stih-cec.txt
@@ -9,6 +9,7 @@ Required properties:
  - pinctrl-names: Contains only one value - "default"
  - pinctrl-0: Specifies the pin control groups used for CEC hardware.
  - resets: Reference to a reset controller
+ - hdmi-handle: Phandle to the HDMI controller
 
 Example for STIH407:
 
@@ -22,4 +23,5 @@ sti-cec@094a087c {
pinctrl-names = "default";
pinctrl-0 = <_cec0_default>;
resets = < STIH407_LPM_SOFTRESET>;
+   hdmi-handle = <>;
 };
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 9920726..46db8a3 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -422,6 +422,16 @@ config VIDEO_SAMSUNG_S5P_CEC
  CEC bus is present in the HDMI connector and enables communication
  between compatible devices.
 
+config VIDEO_STI_HDMI_CEC
+   tristate "STMicroelectronics STiH4xx HDMI CEC driver"
+   depends on VIDEO_DEV && MEDIA_CEC_SUPPORT && (ARCH_STI || COMPILE_TEST)
+   select HPD_NOTIFIER
+   ---help---
+ This is a driver for STIH4xx HDMI CEC interface. It uses the
+ generic CEC framework interface.
+ CEC bus is present in the HDMI connector and enables communication
+ between compatible devices.
+
 endif #V4L_CEC_DRIVERS
 
 menuconfig V4L_TEST_DRIVERS
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index ad3bf22..01b689c 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS_GSC)+= exynos-gsc/
 obj-$(CONFIG_VIDEO_STI_BDISP)  += sti/bdisp/
 obj-$(CONFIG_VIDEO_STI_HVA)+= sti/hva/
 obj-$(CONFIG_DVB_C8SECTPFE)+= sti/c8sectpfe/
+obj-$(CONFIG_VIDEO_STI_HDMI_CEC)   += sti/cec/
 
 obj-$(CONFIG_BLACKFIN)  += blackfin/
 
diff --git a/drivers/media/platform/sti/cec/Makefile 
b/drivers/media/platform/sti/cec/Makefile
new file mode 100644
index 000..f07905e
--- /dev/null
+++ b/drivers/media/platform/sti/cec/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_VIDEO_STI_HDMI_CEC) += stih-cec.o
diff --git a/drivers/media/platform/sti/cec/stih-cec.c 
b/drivers/media/platform/sti/cec/stih-cec.c
new file mode 100644
index 000..4242dad
--- /dev/null
+++ b/drivers/media/platform/sti/cec/stih-cec.c
@@ -0,0 +1,404 @@
+/*
+ * STIH4xx CEC driver
+ * Copyright (C) STMicroelectronic SA 2016
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#define CEC_NAME   "stih-cec"
+
+/* CEC registers  */
+#define CEC_CLK_DIV   0x0
+#define CEC_CTRL  0x4
+#define CEC_IRQ_CTRL  0x8
+#define CEC_STATUS0xC
+#define CEC_EXT_STATUS0x10
+#define CEC_TX_CTRL   0x14
+#define CEC_FREE_TIME_THRESH  0x18
+#define CEC_BIT_TOUT_THRESH   

Re: [PATCH v4 00/36] i.MX Media Driver

[Philipp Zabel]

On Thu, 2017-02-16 at 22:57 +, Russell King - ARM Linux wrote:
> On Thu, Feb 16, 2017 at 02:27:41PM -0800, Steve Longerbeam wrote:
> > 
> > 
> > On 02/16/2017 02:20 PM, Russell King - ARM Linux wrote:
> > >On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote:
> > >>In version 4:
> > >
> > >With this version, I get:
> > >
> > >[28762.892053] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x
> > >[28762.899409] ipu1_csi0: pipeline_set_stream failed with -110
> > >
> > 
> > Right, in the imx219, on exit from s_power(), the clock and data lanes
> > must be placed in the LP-11 state. This has been done in the ov5640 and
> > tc358743 subdevs.
> 
> The only way to do that is to enable streaming from the sensor, wait
> an initialisation time, and then disable streaming, and wait for the
> current line to finish.  There is _no_ other way to get the sensor to
> place its clock and data lines into LP-11 state.

I thought going through LP-11 is part of the D-PHY transmitter
initialization, during the LP->HS wakeup sequence. But then I have no
access to MIPI specs.
It is unfortunate that the i.MX6 MIPI CSI-2 core needs software
assistance here, but could it be possible to trigger that sequence in
the sensor and then without waiting switching to polling for LP-11 state
in the i.MX6 MIPI CSI-2 receiver?

> For that to happen, we need to program the sensor a bit more than we
> currently do at power on (to a minimal resolution, and setting up the
> PLLs), and introduce another 4ms on top of the 8ms or so that the
> runtime resume function already takes.
> 
> Looking at the SMIA driver, things are worse, and I suspect that it also
> will not work with the current setup - the SMIA spec shows that the CSI
> clock and data lines are tristated while the sensor is not streaming,
> which means they aren't held at a guaranteed LP-11 state, even if that
> driver momentarily enabled streaming.  Hence, Freescale's (or is it
> Synopsis') requirement may actually be difficult to satisfy.
> 
> However, I regard runtime PM broken with the current imx-capture setup.
> At the moment, power is controlled at the sensor by whether the media
> links are enabled.  So, if you have an enabled link coming off the
> sensor, the sensor will be powered up, whether you're using it or not.
> 
> Given that the number of applications out there that know about the
> media subdevs is really quite small, this combination makes having
> runtime PM in sensor devices completely pointless - they can't sleep
> as long as they have an enabled link, which could be persistent over
> many days or weeks.

regards
Philipp

Re: Dead code in v4l2-mem2mem.c?

[Laurent Pinchart]

Hi Shaobo,

First of all, could you please make sure you send future mails to the linux-
media mailing list in plain text only (no HTML) ? The mailing list server 
rejects HTML e-mails.

On Thursday 16 Feb 2017 16:08:25 Shaobo wrote:
> Hi there,
> 
> My name is Shaobo He and I am a graduate student at University of Utah. I am
> applying a static analysis tool to the Linux device drivers, looking for
> NULL pointer dereference and accidentally found a plausible dead code
> location in v4l2-mem2mem.c due to undefined behavior.
> 
> The following is the problematic code segment,
> 
> static struct v4l2_m2m_queue_ctx *get_queue_ctx(struct v4l2_m2m_ctx
> *m2m_ctx,
> enum v4l2_buf_type type)
> {
>   if (V4L2_TYPE_IS_OUTPUT(type))
>   return _ctx->out_q_ctx;
>   else
>   return _ctx->cap_q_ctx;
> }
> 
> struct vb2_queue *v4l2_m2m_get_vq(struct v4l2_m2m_ctx *m2m_ctx,
>   enum v4l2_buf_type type)
> {
>   struct v4l2_m2m_queue_ctx *q_ctx;
> 
>   q_ctx = get_queue_ctx(m2m_ctx, type);
>   if (!q_ctx)
>   return NULL;
> 
>   return _ctx->q;
> }
> 
> `get_queue_ctx` returns a pointer value that is an addition of the base
> pointer address (`m2m_ctx`) to a non-zero offset. The following is the
> definition of struct v4l2_m2m_ctx,
> 
> struct v4l2_m2m_ctx {
>   /* optional cap/out vb2 queues lock */
>   struct mutex*q_lock;
> 
>   /* internal use only */
>   struct v4l2_m2m_dev *m2m_dev;
> 
>   struct v4l2_m2m_queue_ctx   cap_q_ctx;
> 
>   struct v4l2_m2m_queue_ctx   out_q_ctx;
> 
>   /* For device job queue */
>   struct list_headqueue;
>   unsigned long   job_flags;
>   wait_queue_head_t   finished;
> 
>   void*priv;
> };
> 
> There is a NULL test in a caller of `get_queue_ctx` (line 85), which appears
> problematic to me. I'm not sure if it is defined or feasible under the
> context of Linux kernel. This blog
> (https://wdtz.org/undefined-behavior-in-binutils-causes-segfault.html)
> suggests that the NULL check can be optimized away because the only case
> that the return value can be NULL triggers pointer overflow, which is
> undefined.
> 
> Please let me know if it makes sense or not. Thanks for your time and I am
> looking forward to your reply.

The NULL check is indeed wrong. I believe that the m2m_ctx argument passed to 
the v4l2_m2m_get_vq() function should never be NULL. We will however need to 
audit drivers to make sure that's the case. The NULL check could then be 
removed. Alternatively we could check m2m_ctx above the get_queue_ctx() call, 
which wouldn't require auditing drivers. It's a safe option, but would likely 
result in an unneeded NULL check.

-- 
Regards,

Laurent Pinchart

Re: [Patch 0/2] media: ti-vpe: allow user specified stride

[Tomi Valkeinen]

On 13/02/17 15:06, Benoit Parrot wrote:
> This patch series enables user specified buffer stride to be used
> instead of always forcing the stride from the driver side.
> 
> Benoit Parrot (2):
>   media: ti-vpe: vpdma: add support for user specified stride
>   media: ti-vpe: vpe: allow use of user specified stride
> 
>  drivers/media/platform/ti-vpe/vpdma.c | 14 --
>  drivers/media/platform/ti-vpe/vpdma.h |  6 +++---
>  drivers/media/platform/ti-vpe/vpe.c   | 34 --
>  3 files changed, 31 insertions(+), 23 deletions(-)

Reviewed-by: Tomi Valkeinen 

 Tomi



signature.asc
Description: OpenPGP digital signature

[PATCH] v4l-utils: fix invalid protocol in streamzap keymap

[Matthias Reichl]

ir-keytable can't load the streamzap keymap because the
protocol type RC5_SZ is invalid:

./ir-keytable -w rc_keymaps/streamzap
Protocol RC5_SZ invalid
...

Fix this by changing the protocol type to RC-5-SZ which
matches the kernel protocol rc-5-sz

Signed-off-by: Matthias Reichl 
---
 utils/keytable/rc_keymaps/streamzap | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/utils/keytable/rc_keymaps/streamzap 
b/utils/keytable/rc_keymaps/streamzap
index 3512cd8..03d2cb8 100644
--- a/utils/keytable/rc_keymaps/streamzap
+++ b/utils/keytable/rc_keymaps/streamzap
@@ -1,4 +1,4 @@
-# table streamzap, type: RC5_SZ
+# table streamzap, type: RC-5-SZ
 0x28c0 KEY_NUMERIC_0
 0x28c1 KEY_NUMERIC_1
 0x28c2 KEY_NUMERIC_2
-- 
2.1.4

Re: [PATCH v4 18/36] media: Add i.MX media core driver

[Philipp Zabel]

On Thu, 2017-02-16 at 17:33 -0800, Steve Longerbeam wrote:
> 
> On 02/16/2017 05:02 AM, Philipp Zabel wrote:
> > On Wed, 2017-02-15 at 18:19 -0800, Steve Longerbeam wrote:
> 
> >> +
> >> +- Clean up and move the ov5642 subdev driver to drivers/media/i2c, and
> >> +  create the binding docs for it.
> >
> > This is done already, right?
> 
> 
> I cleaned up ov5640 and moved it to drivers/media/i2c with binding docs,
> but not the ov5642 yet.

Ok, thanks.

> >> +- The Frame Interval Monitor could be exported to v4l2-core for
> >> +  general use.
> >> +
> >> +- The subdev that is the original source of video data (referred to as
> >> +  the "sensor" in the code), is called from various subdevs in the
> >> +  pipeline in order to set/query the video standard ({g|s|enum}_std)
> >> +  and to get/set the original frame interval from the capture interface
> >> +  ([gs]_parm). Instead, the entities that need this info should call its
> >> +  direct neighbor, and the neighbor should propagate the call to its
> >> +  neighbor in turn if necessary.
> >
> > Especially the [gs]_parm fix is necessary to present userspace with the
> > correct frame interval in case of frame skipping in the CSI.
> 
> 
> Right, understood. I've added this to list of fixes for version 5.
> 
> What a pain though! It means propagating every call to g_frame_interval
> upstream until a subdev "that cares" returns ret == 0 or
> ret != -ENOIOCTLCMD. And that goes for any other chained subdev call
> as well.

Not at all. Since the frame interval is a property of the pad, that had
to be propagated downstream by media-ctl along with media bus format,
frame size, and colorimetry earlier.

regards
Philipp

[no subject]

[MACDONALD, MISHUNA]

Good morning sir/madam, we are presently offering business and personal loans
at low rates at Al Futtaim GE Finance for 2 percent per year and
you are qualified to received the loan. if interested, send request now to: 
aeonthan...@gmail.com