Re: [PATCH v3] media: radio: Critical interrupt bugfix for si470x over i2c

[Hans Verkuil]

On 02/27/2018 12:39 AM, Douglas Fischer wrote:
> Hans,
> 
> See comments below. Thank you for the quick response on this and all
> your patience and help in general with these patches.

My pleasure!

> 
> On Mon, 26 Feb 2018 12:57:26 +0100
> Hans Verkuil  wrote:
> 
>> On 02/26/2018 03:27 AM, Douglas Fischer wrote:
>>> Fixed si470x_start() disabling the interrupt signal, causing tune
>>> operations to never complete. This does not affect USB radios
>>> because they poll the registers instead of using the IRQ line.
>>>
>>> Stylistic and comment changes from v2.
>>>
>>> Signed-off-by: Douglas Fischer 
>>> ---
>>>
>>> diff -uprN
>>> linux.orig/drivers/media/radio/si470x/radio-si470x-common.c
>>> linux/drivers/media/radio/si470x/radio-si470x-common.c ---
>>> linux.orig/drivers/media/radio/si470x/radio-si470x-common.c
>>> 2018-01-15 21:58:10.675620432 -0500 +++
>>> linux/drivers/media/radio/si470x/radio-si470x-common.c
>>> 2018-02-25 19:16:31.785934211 -0500 @@ -377,8 +377,11 @@ int
>>> si470x_start(struct si470x_device *r goto done; /* sysconfig 1 */
>>> -   radio->registers[SYSCONFIG1] =
>>> -   (de << 11) & SYSCONFIG1_DE; /* DE*/
>>> +   radio->registers[SYSCONFIG1] |=
>>> SYSCONFIG1_RDSIEN|SYSCONFIG1_STCIEN|SYSCONFIG1_RDS;
>>> +   radio->registers[SYSCONFIG1] &= ~SYSCONFIG1_GPIO2;
>>> +   radio->registers[SYSCONFIG1] |= (0x01 << 2); /* GPIO2 */  
>>
>> Yes, but what does this do? Enable GPIO2? The header defines two bits
>> for GPIO1/2/3, but it doesn't say what those bits mean. So the
>> question here is what it means to set bit 2 to 1 and bit 3 to 0? The
>> header doesn't give any information about that, nor does this comment.
>>
> SYSCONFIG1_GPIO2 is bits 2 and 3, I need to clear bit 3 and set bit 2 without 
> changing the other bits. This configures GPIO2 as "STC/RDS interrupt. A logic 
> high will be output unless an interrupt occurs". Should I change the comment 
> to read "/* GPIO2 STC/RDS interrupt output */"?

It might be better to have defines in the header that make this explicit.

E.g. right now we have just:

#define SYSCONFIG1_GPIO20x000c  /* bits 03..02: General Purpose I/O 2 */

So this could be changed to:

#define SYSCONFIG1_GPIO2_MSK 0x000c  /* bits 03..02: General Purpose I/O 2 
*/
#define SYSCONFIG1_GPIO2_DIS 0   /* Disable GPIO 2 interrupt */
#define SYSCONFIG1_GPIO2_STC_RDS 0x0004  /* Enable STC/RDS interrupt */
...

(I'm guessing 0 means no irq)

That way the code can become self-documenting.

Regards,

Hans

>> Regards,
>>
>>  Hans
>>
>>> +   if (de)
>>> +   radio->registers[SYSCONFIG1] |= SYSCONFIG1_DE;
>>> retval = si470x_set_register(radio, SYSCONFIG1);
>>> if (retval < 0)
>>> goto done;
>>>   
>>
> Thank you,
>   Doug
> 

Re: [PATCH v2] Staging: bcm2048: Fix function argument alignment in radio-bcm2048.c.

[Hans Verkuil]

On 02/27/2018 02:53 AM, Quytelda Kahja wrote:
> Hans,
> 
> Thank you very much for your input on the patch; however this patch
> has already been applied to the staging tree.  Additionally:

I have no record of this being applied through linux-media. Did someone
else pick this up? Greg perhaps?

>> What coding style problem? You should give a short description of
>> what you are fixing.
> The subject of the patch (which becomes the subject of the email when
> using `git format-patch`) describes the change more fully: "Staging:
> bcm2048: Fix function argument alignment in radio-bcm2048.c".  It's a
> really trivial patch, so there's not too much to say.  That extra
> comment is just redundant, I suppose.

Usually you just show the warnings that gcc or sparse or whatever
produced.

> 
>> Just drop this change: it will replace one warning (non-aligned) with
>> another (> 80 cols).
> Breaking the 80 character line limit is arguably excusable for this
> code because of the 36 character function name and 30 character
> constant name; additionally, it has been said that the 80 character
> line limit will probably be increased in the future since we run
> modern machines that aren't limited to 80 character terminals anymore,
> so this warning may soon be irrelevant anyway.

I know people who would be very annoyed if the 80 char limit is lifted.

Anyway, in the end you look at whether a patch is worth it or not,
and this part isn't.

But if it is already applied by someone then this is all moot.

Regards,

Hans

> 
> Thank you,
> Quytelda Kahja
> 
> On Mon, Feb 26, 2018 at 5:51 AM, Hans Verkuil  wrote:
>> On 02/20/2018 07:53 AM, Quytelda Kahja wrote:
>>> Fix a coding style problem.
>>
>> What coding style problem? You should give a short description of
>> what you are fixing.
>>
>>>
>>> Signed-off-by: Quytelda Kahja 
>>> ---
>>> This is the patch without the unnecessary fixes for line length.
>>>
>>>  drivers/staging/media/bcm2048/radio-bcm2048.c | 22 +++---
>>>  1 file changed, 11 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
>>> b/drivers/staging/media/bcm2048/radio-bcm2048.c
>>> index 06d1920150da..f38a4f2acdde 100644
>>> --- a/drivers/staging/media/bcm2048/radio-bcm2048.c
>>> +++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
>>> @@ -1864,7 +1864,7 @@ static int bcm2048_probe(struct bcm2048_device *bdev)
>>>   goto unlock;
>>>
>>>   err = bcm2048_set_fm_search_rssi_threshold(bdev,
>>> - BCM2048_DEFAULT_RSSI_THRESHOLD);
>>> +
>>> BCM2048_DEFAULT_RSSI_THRESHOLD);
>>>   if (err < 0)
>>>   goto unlock;
>>>
>>
>> Just drop this change: it will replace one warning (non-aligned) with
>> another (> 80 cols).
>>
>> This code is fine as it is.
>>
>> Regards,
>>
>> Hans
>>
>>> @@ -1942,9 +1942,9 @@ static irqreturn_t bcm2048_handler(int irq, void *dev)
>>>   */
>>>  #define property_write(prop, type, mask, check)
>>>   \
>>>  static ssize_t bcm2048_##prop##_write(struct device *dev,\
>>> - struct device_attribute *attr,  \
>>> - const char *buf,\
>>> - size_t count)   \
>>> +   struct device_attribute *attr,\
>>> +   const char *buf,  \
>>> +   size_t count) \
>>>  {\
>>>   struct bcm2048_device *bdev = dev_get_drvdata(dev); \
>>>   type value; \
>>> @@ -1966,8 +1966,8 @@ static ssize_t bcm2048_##prop##_write(struct device 
>>> *dev,   \
>>>
>>>  #define property_read(prop, mask)\
>>>  static ssize_t bcm2048_##prop##_read(struct device *dev, \
>>> - struct device_attribute *attr,  \
>>> - char *buf)  \
>>> +  struct device_attribute *attr, \
>>> +  char *buf) \
>>>  {\
>>>   struct bcm2048_device *bdev = dev_get_drvdata(dev); \
>>>   int value;  \
>>> @@ -1985,8 +1985,8 @@ static ssize_t bcm2048_##prop##_read(struct device 
>>> *dev,\
>>>
>>>  #define property_signed_read(prop, size, mask) 
>>>   \
>>>  static ssize_t bcm2048_##prop##_read(struct device *dev, \
>>> - 

Re: [PATCH v8 2/2] media: V3s: Add support for Allwinner CSI.

[Maxime Ripard]

On Tue, Feb 27, 2018 at 10:12:46AM +0800, Yong Deng wrote:
> Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2
> interface and CSI1 is used for parallel interface. This is not
> documented in datasheet but by test and guess.
> 
> This patch implement a v4l2 framework driver for it.
> 
> Currently, the driver only support the parallel interface. MIPI-CSI2,
> ISP's support are not included in this patch.
> 
> Tested-by: Maxime Ripard 
> Signed-off-by: Yong Deng 

Reviewed-by: Maxime Ripard 

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature

[PATCH v4 1/2] media: dt-bindings: Add bindings for panasonic,amg88xx

[Matt Ranostay]

Define the device tree bindings for the panasonic,amg88xx i2c
video driver.

Cc: Rob Herring 
Cc: devicet...@vger.kernel.org
Signed-off-by: Matt Ranostay 
---
 .../bindings/media/i2c/panasonic,amg88xx.txt  | 19 +++
 1 file changed, 19 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/media/i2c/panasonic,amg88xx.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/panasonic,amg88xx.txt 
b/Documentation/devicetree/bindings/media/i2c/panasonic,amg88xx.txt
new file mode 100644
index ..4a3181a3dd7e
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/panasonic,amg88xx.txt
@@ -0,0 +1,19 @@
+* Panasonic AMG88xx
+
+The Panasonic family of AMG88xx Grid-Eye sensors allow recording
+8x8 10Hz video which consists of thermal datapoints
+
+Required Properties:
+ - compatible : Must be "panasonic,amg88xx"
+ - reg : i2c address of the device
+
+Example:
+
+   i2c0@1c22000 {
+   ...
+   amg88xx@69 {
+   compatible = "panasonic,amg88xx";
+   reg = <0x69>;
+   };
+   ...
+   };
-- 
2.14.1

[PATCH v4 2/2] media: video-i2c: add video-i2c driver

[Matt Ranostay]

There are several thermal sensors that only have a low-speed bus
interface but output valid video data. This patchset enables support
for the AMG88xx "Grid-Eye" sensor family.

Cc: Luca Barbato 
Cc: Laurent Pinchart 
Signed-off-by: Matt Ranostay 
---
 MAINTAINERS   |   6 +
 drivers/media/i2c/Kconfig |   9 +
 drivers/media/i2c/Makefile|   1 +
 drivers/media/i2c/video-i2c.c | 558 ++
 4 files changed, 574 insertions(+)
 create mode 100644 drivers/media/i2c/video-i2c.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 8c2320e1685c..90ae81ae0e09 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14776,6 +14776,12 @@ L: linux-media@vger.kernel.org
 S: Maintained
 F: drivers/media/platform/video-mux.c
 
+VIDEO I2C POLLING DRIVER
+M: Matt Ranostay 
+L: linux-media@vger.kernel.org
+S: Maintained
+F: drivers/media/i2c/video-i2c.c
+
 VIDEOBUF2 FRAMEWORK
 M: Pawel Osciak 
 M: Marek Szyprowski 
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 8fdd673d449f..53aede720e0f 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -917,6 +917,15 @@ config VIDEO_M52790
 
 To compile this driver as a module, choose M here: the
 module will be called m52790.
+
+config VIDEO_I2C
+   tristate "I2C transport video support"
+   depends on VIDEO_V4L2 && I2C
+   select VIDEOBUF2_VMALLOC
+   ---help---
+ Enable the I2C transport video support which supports the
+ following:
+  * Panasonic AMG88xx Grid-Eye Sensors
 endmenu
 
 menu "Sensors used on soc_camera driver"
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 26b19a2e9d04..5d4c06cb3f6f 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -91,6 +91,7 @@ obj-$(CONFIG_VIDEO_LM3646)+= lm3646.o
 obj-$(CONFIG_VIDEO_SMIAPP_PLL) += smiapp-pll.o
 obj-$(CONFIG_VIDEO_AK881X) += ak881x.o
 obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
+obj-$(CONFIG_VIDEO_I2C)+= video-i2c.o
 obj-$(CONFIG_VIDEO_ML86V7667)  += ml86v7667.o
 obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
 obj-$(CONFIG_VIDEO_TC358743)   += tc358743.o
diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c
new file mode 100644
index ..cd64fa80f55d
--- /dev/null
+++ b/drivers/media/i2c/video-i2c.c
@@ -0,0 +1,558 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * video-i2c.c - Support for I2C transport video devices
+ *
+ * Copyright (C) 2018 Matt Ranostay 
+ *
+ * Supported:
+ * - Panasonic AMG88xx Grid-Eye Sensors
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define VIDEO_I2C_DRIVER   "video-i2c"
+#define MAX_BUFFER_SIZE128
+
+struct video_i2c_chip;
+
+struct video_i2c_buffer {
+   struct vb2_v4l2_buffer vb;
+   struct list_head list;
+};
+
+struct video_i2c_data {
+   struct i2c_client *client;
+   const struct video_i2c_chip *chip;
+   struct mutex lock;
+   spinlock_t slock;
+   struct mutex queue_lock;
+
+   struct v4l2_device v4l2_dev;
+   struct video_device vdev;
+   struct vb2_queue vb_vidq;
+
+   struct task_struct *kthread_vid_cap;
+   struct list_head vid_cap_active;
+};
+
+static struct v4l2_fmtdesc amg88xx_format = {
+   .pixelformat = V4L2_PIX_FMT_Y12,
+};
+
+static struct v4l2_frmsize_discrete amg88xx_size = {
+   .width = 8,
+   .height = 8,
+};
+
+struct video_i2c_chip {
+   /* video dimensions */
+   const struct v4l2_fmtdesc *format;
+   const struct v4l2_frmsize_discrete *size;
+
+   /* max frames per second */
+   unsigned int max_fps;
+
+   /* pixel buffer size */
+   unsigned int buffer_size;
+
+   /* pixel size in bits */
+   unsigned int bpp;
+
+   /* xfer function */
+   int (*xfer)(struct video_i2c_data *data, char *buf);
+};
+
+static int amg88xx_xfer(struct video_i2c_data *data, char *buf)
+{
+   struct i2c_client *client = data->client;
+   struct i2c_msg msg[2];
+   u8 reg = 0x80;
+   int ret;
+
+   msg[0].addr = client->addr;
+   msg[0].flags = 0;
+   msg[0].len = 1;
+   msg[0].buf  = (char *) 
+
+   msg[1].addr = client->addr;
+   msg[1].flags = I2C_M_RD;
+   msg[1].len = data->chip->buffer_size;
+   msg[1].buf = (char *) buf;
+
+   ret = i2c_transfer(client->adapter, msg, 2);
+
+   return (ret == 2) ? 0 : -EIO;
+}
+
+#define AMG88XX0
+
+static const struct video_i2c_chip video_i2c_chip[] = {
+   [AMG88XX] = {
+   .size   = _size,
+   .format  

[PATCH v4 0/2] media: video-i2c: add video-i2c driver support

[Matt Ranostay]

Add support for video-i2c polling driver

Changes from v1:
* Switch to SPDX tags versus GPLv2 license text
* Remove unneeded zeroing of data structures
* Add video_i2c_try_fmt_vid_cap call in video_i2c_s_fmt_vid_cap function

Changes from v2:
* Add missing linux/kthread.h include that broke x86_64 build

Changes from v3:
* Add devicetree binding documents
* snprintf check added
* switched to per chip support based on devicetree or i2c client id
* add VB2_DMABUF to io_modes
* added entry to MAINTAINERS file switched to per chip support based on 
devicetree or i2c client id

Matt Ranostay (2):
  media: dt-bindings: Add bindings for panasonic,amg88xx
  media: video-i2c: add video-i2c driver

 .../bindings/media/i2c/panasonic,amg88xx.txt   |  19 +
 MAINTAINERS|   6 +
 drivers/media/i2c/Kconfig  |   9 +
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/video-i2c.c  | 558 +
 5 files changed, 593 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/media/i2c/panasonic,amg88xx.txt
 create mode 100644 drivers/media/i2c/video-i2c.c

-- 
2.14.1

cron job: media_tree daily build: ERRORS

[Hans Verkuil]

This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Tue Feb 27 05:00:15 CET 2018
media-tree git hash:15ea2df9143729a2b722d4ca2b52cfa14a819d8e
media_build git hash:   a9ea3d056e5ce50d37dd6129126f776c3a8ec2d7
v4l-utils git hash: a6185c9b96c101e45aec80fccaa471febbd09b30
gcc version:i686-linux-gcc (GCC) 7.3.0
sparse version: v0.5.0-3994-g45eb2282
smatch version: v0.5.0-3994-g45eb2282
host hardware:  x86_64
host os:4.14.0-3-amd64

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-arm64: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: ERRORS
linux-2.6.36.4-x86_64: ERRORS
linux-2.6.37.6-i686: ERRORS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-i686: ERRORS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-i686: ERRORS
linux-2.6.39.4-x86_64: ERRORS
linux-3.0.60-i686: ERRORS
linux-3.0.60-x86_64: ERRORS
linux-3.1.10-i686: ERRORS
linux-3.1.10-x86_64: ERRORS
linux-3.2.98-i686: ERRORS
linux-3.2.98-x86_64: ERRORS
linux-3.3.8-i686: ERRORS
linux-3.3.8-x86_64: ERRORS
linux-3.4.27-i686: ERRORS
linux-3.4.27-x86_64: ERRORS
linux-3.5.7-i686: ERRORS
linux-3.5.7-x86_64: ERRORS
linux-3.6.11-i686: ERRORS
linux-3.6.11-x86_64: ERRORS
linux-3.7.4-i686: ERRORS
linux-3.7.4-x86_64: ERRORS
linux-3.8-i686: ERRORS
linux-3.8-x86_64: ERRORS
linux-3.9.2-i686: ERRORS
linux-3.9.2-x86_64: ERRORS
linux-3.10.1-i686: ERRORS
linux-3.10.1-x86_64: ERRORS
linux-3.11.1-i686: ERRORS
linux-3.11.1-x86_64: ERRORS
linux-3.12.67-i686: ERRORS
linux-3.12.67-x86_64: ERRORS
linux-3.13.11-i686: ERRORS
linux-3.13.11-x86_64: ERRORS
linux-3.14.9-i686: ERRORS
linux-3.14.9-x86_64: ERRORS
linux-3.15.2-i686: ERRORS
linux-3.15.2-x86_64: ERRORS
linux-3.16.53-i686: ERRORS
linux-3.16.53-x86_64: ERRORS
linux-3.17.8-i686: ERRORS
linux-3.17.8-x86_64: ERRORS
linux-3.18.93-i686: ERRORS
linux-3.18.93-x86_64: ERRORS
linux-3.19-i686: ERRORS
linux-3.19-x86_64: ERRORS
linux-4.0.9-i686: ERRORS
linux-4.0.9-x86_64: ERRORS
linux-4.1.49-i686: ERRORS
linux-4.1.49-x86_64: ERRORS
linux-4.2.8-i686: ERRORS
linux-4.2.8-x86_64: ERRORS
linux-4.3.6-i686: ERRORS
linux-4.3.6-x86_64: ERRORS
linux-4.4.115-i686: ERRORS
linux-4.4.115-x86_64: ERRORS
linux-4.5.7-i686: ERRORS
linux-4.5.7-x86_64: ERRORS
linux-4.6.7-i686: ERRORS
linux-4.6.7-x86_64: ERRORS
linux-4.7.5-i686: ERRORS
linux-4.7.5-x86_64: ERRORS
linux-4.8-i686: ERRORS
linux-4.8-x86_64: ERRORS
linux-4.9.80-i686: ERRORS
linux-4.9.80-x86_64: ERRORS
linux-4.10.14-i686: OK
linux-4.10.14-x86_64: WARNINGS
linux-4.11-i686: OK
linux-4.11-x86_64: WARNINGS
linux-4.12.1-i686: OK
linux-4.12.1-x86_64: WARNINGS
linux-4.13-i686: OK
linux-4.13-x86_64: OK
linux-4.14.17-i686: OK
linux-4.14.17-x86_64: OK
linux-4.15.2-i686: OK
linux-4.15.2-x86_64: OK
linux-4.16-rc1-i686: OK
linux-4.16-rc1-x86_64: OK
apps: WARNINGS
spec-git: OK
sparse: WARNINGS
smatch: OK

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

[PATCH v8 2/2] media: V3s: Add support for Allwinner CSI.

[Yong Deng]

Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2
interface and CSI1 is used for parallel interface. This is not
documented in datasheet but by test and guess.

This patch implement a v4l2 framework driver for it.

Currently, the driver only support the parallel interface. MIPI-CSI2,
ISP's support are not included in this patch.

Tested-by: Maxime Ripard 
Signed-off-by: Yong Deng 
---
 MAINTAINERS|   8 +
 drivers/media/platform/Kconfig |   1 +
 drivers/media/platform/Makefile|   2 +
 drivers/media/platform/sunxi/sun6i-csi/Kconfig |   9 +
 drivers/media/platform/sunxi/sun6i-csi/Makefile|   3 +
 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c | 911 +
 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h | 143 
 .../media/platform/sunxi/sun6i-csi/sun6i_csi_reg.h | 196 +
 .../media/platform/sunxi/sun6i-csi/sun6i_video.c   | 753 +
 .../media/platform/sunxi/sun6i-csi/sun6i_video.h   |  53 ++
 10 files changed, 2079 insertions(+)
 create mode 100644 drivers/media/platform/sunxi/sun6i-csi/Kconfig
 create mode 100644 drivers/media/platform/sunxi/sun6i-csi/Makefile
 create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
 create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h
 create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_reg.h
 create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
 create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_video.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 91ed6adfa4a6..b4a331ad35b5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3793,6 +3793,14 @@ M:   Jaya Kumar 
 S: Maintained
 F: sound/pci/cs5535audio/
 
+CSI DRIVERS FOR ALLWINNER V3s
+M: Yong Deng 
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/platform/sunxi/sun6i-csi/
+F: Documentation/devicetree/bindings/media/sun6i-csi.txt
+
 CW1200 WLAN driver
 M: Solomon Peachy 
 S: Maintained
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index f9cc0582c8a9..7f1ee46c3258 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -159,6 +159,7 @@ source "drivers/media/platform/am437x/Kconfig"
 source "drivers/media/platform/xilinx/Kconfig"
 source "drivers/media/platform/rcar-vin/Kconfig"
 source "drivers/media/platform/atmel/Kconfig"
+source "drivers/media/platform/sunxi/sun6i-csi/Kconfig"
 
 config VIDEO_TI_CAL
tristate "TI CAL (Camera Adaptation Layer) driver"
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 85e112122f32..143d8a473b0a 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -96,3 +96,5 @@ obj-$(CONFIG_VIDEO_QCOM_CAMSS)+= 
qcom/camss-8x16/
 obj-$(CONFIG_VIDEO_QCOM_VENUS) += qcom/venus/
 
 obj-y  += meson/
+
+obj-$(CONFIG_VIDEO_SUN6I_CSI)  += sunxi/sun6i-csi/
diff --git a/drivers/media/platform/sunxi/sun6i-csi/Kconfig 
b/drivers/media/platform/sunxi/sun6i-csi/Kconfig
new file mode 100644
index ..314188aae2c2
--- /dev/null
+++ b/drivers/media/platform/sunxi/sun6i-csi/Kconfig
@@ -0,0 +1,9 @@
+config VIDEO_SUN6I_CSI
+   tristate "Allwinner V3s Camera Sensor Interface driver"
+   depends on VIDEO_V4L2 && COMMON_CLK && VIDEO_V4L2_SUBDEV_API && HAS_DMA
+   depends on ARCH_SUNXI || COMPILE_TEST
+   select VIDEOBUF2_DMA_CONTIG
+   select REGMAP_MMIO
+   select V4L2_FWNODE
+   ---help---
+  Support for the Allwinner Camera Sensor Interface Controller on V3s.
diff --git a/drivers/media/platform/sunxi/sun6i-csi/Makefile 
b/drivers/media/platform/sunxi/sun6i-csi/Makefile
new file mode 100644
index ..213cb6be9e9c
--- /dev/null
+++ b/drivers/media/platform/sunxi/sun6i-csi/Makefile
@@ -0,0 +1,3 @@
+sun6i-csi-y += sun6i_video.o sun6i_csi.o
+
+obj-$(CONFIG_VIDEO_SUN6I_CSI) += sun6i-csi.o
diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c 
b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
new file mode 100644
index ..1e238d57
--- /dev/null
+++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
@@ -0,0 +1,911 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2011-2018 Magewell Electronics Co., Ltd. (Nanjing)
+ * All rights reserved.
+ * Author: Yong Deng 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "sun6i_csi.h"
+#include "sun6i_csi_reg.h"
+
+#define MODULE_NAME"sun6i-csi"
+
+struct sun6i_csi_dev {
+   

[PATCH v8 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI)

[Yong Deng]

Add binding documentation for Allwinner V3s CSI.

Acked-by: Maxime Ripard 
Acked-by: Sakari Ailus 
Reviewed-by: Rob Herring 
Signed-off-by: Yong Deng 
---
 .../devicetree/bindings/media/sun6i-csi.txt| 59 ++
 1 file changed, 59 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt

diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt 
b/Documentation/devicetree/bindings/media/sun6i-csi.txt
new file mode 100644
index ..2ff47a9507a6
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt
@@ -0,0 +1,59 @@
+Allwinner V3s Camera Sensor Interface
+-
+
+Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2
+interface and CSI1 is used for parallel interface.
+
+Required properties:
+  - compatible: value must be "allwinner,sun8i-v3s-csi"
+  - reg: base address and size of the memory-mapped region.
+  - interrupts: interrupt associated to this IP
+  - clocks: phandles to the clocks feeding the CSI
+* bus: the CSI interface clock
+* mod: the CSI module clock
+* ram: the CSI DRAM clock
+  - clock-names: the clock names mentioned above
+  - resets: phandles to the reset line driving the CSI
+
+Each CSI node should contain one 'port' child node with one child 'endpoint'
+node, according to the bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt. As mentioned
+above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel or
+Bt656 for CSI1.
+
+Endpoint node properties for CSI1
+-
+
+- remote-endpoint  : (required) a phandle to the bus receiver's endpoint
+  node
+- bus-width:   : (required) must be 8, 10, 12 or 16
+- pclk-sample  : (optional) (default: sample on falling edge)
+- hsync-active : (only required for parallel)
+- vsync-active : (only required for parallel)
+
+Example:
+
+csi1: csi@1cb4000 {
+   compatible = "allwinner,sun8i-v3s-csi";
+   reg = <0x01cb4000 0x1000>;
+   interrupts = ;
+   clocks = < CLK_BUS_CSI>,
+< CLK_CSI1_SCLK>,
+< CLK_DRAM_CSI>;
+   clock-names = "bus", "mod", "ram";
+   resets = < RST_BUS_CSI>;
+
+   port {
+   /* Parallel bus endpoint */
+   csi1_ep: endpoint {
+   remote-endpoint = <_ep>;
+   bus-width = <16>;
+
+   /* If hsync-active/vsync-active are missing,
+  embedded BT.656 sync is used */
+   hsync-active = <0>; /* Active low */
+   vsync-active = <0>; /* Active low */
+   pclk-sample = <1>;  /* Rising */
+   };
+   };
+};
-- 
1.8.3.1

[PATCH v8 0/2] Initial Allwinner V3s CSI Support

[Yong Deng]

This patchset add initial support for Allwinner V3s CSI.

Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2
interface and CSI1 is used for parallel interface. This is not
documented in datasheet but by test and guess.

This patchset implement a v4l2 framework driver and add a binding 
documentation for it. 

Currently, the driver only support the parallel interface. And has been
tested with a BT1120 signal which generating from FPGA. The following
fetures are not support with this patchset:
  - ISP 
  - MIPI-CSI2
  - Master clock for camera sensor
  - Power regulator for the front end IC

Changes in v8:
  * Revert to v6 and add 'hack' for PHYS_OFFSET.

Changes in v7:
  * Add 'depends on ARM' in Kconfig to avoid built with non-ARM arch.

Changes in v6:
  * Add Rob Herring's review tag.
  * Fix a NULL pointer dereference by picking Maxime Ripard's patch.
  * Add Maxime Ripard's test tag.

Changes in v5:
  * Using the new SPDX tags.
  * Fix MODULE_LICENSE.
  * Add many default cases and warning messages.
  * Detail the parallel bus properties
  * Fix some spelling and syntax mistakes.

Changes in v4:
  * Deal with the CSI 'INNER QUEUE'.
CSI will lookup the next dma buffer for next frame before the
the current frame done IRQ triggered. This is not documented
but reported by Ondřej Jirman.
The BSP code has workaround for this too. It skip to mark the
first buffer as frame done for VB2 and pass the second buffer
to CSI in the first frame done ISR call. Then in second frame
done ISR call, it mark the first buffer as frame done for VB2
and pass the third buffer to CSI. And so on. The bad thing is
that the first buffer will be written twice and the first frame
is dropped even the queued buffer is sufficient.
So, I make some improvement here. Pass the next buffer to CSI
just follow starting the CSI. In this case, the first frame
will be stored in first buffer, second frame in second buffer.
This mothed is used to avoid dropping the first frame, it
would also drop frame when lacking of queued buffer.
  * Fix: using a wrong mbus_code when getting the supported formats
  * Change all fourcc to pixformat
  * Change some function names

Changes in v3:
  * Get rid of struct sun6i_csi_ops
  * Move sun6i-csi to new directory drivers/media/platform/sunxi
  * Merge sun6i_csi.c and sun6i_csi_v3s.c into sun6i_csi.c
  * Use generic fwnode endpoints parser
  * Only support a single subdev to make things simple
  * Many complaintion fix

Changes in v2: 
  * Change sunxi-csi to sun6i-csi
  * Rebase to media_tree master branch 

Following is the 'v4l2-compliance -s -f' output, I have test this
with both interlaced and progressive signal:

# ./v4l2-compliance -s -f
v4l2-compliance SHA   : 6049ea8bd64f9d78ef87ef0c2b3dc9b5de1ca4a1

Driver Info:
Driver name   : sun6i-video
Card type : sun6i-csi
Bus info  : platform:csi
Driver version: 4.15.0
Capabilities  : 0x8421
Video Capture
Streaming
Extended Pix Format
Device Capabilities
Device Caps   : 0x0421
Video Capture
Streaming
Extended Pix Format

Compliance test for device /dev/video0 (not using libv4l2):

Required ioctls:
test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
test second video open: OK
test VIDIOC_QUERYCAP: OK
test VIDIOC_G/S_PRIORITY: OK
test for unlimited opens: OK

Debug ioctls:
test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
test VIDIOC_LOG_STATUS: OK (Not Supported)

Input ioctls:
test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
test VIDIOC_ENUMAUDIO: OK (Not Supported)
test VIDIOC_G/S/ENUMINPUT: OK
test VIDIOC_G/S_AUDIO: OK (Not Supported)
Inputs: 1 Audio Inputs: 0 Tuners: 0

Output ioctls:
test VIDIOC_G/S_MODULATOR: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_ENUMAUDOUT: OK (Not Supported)
test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
test VIDIOC_G/S_AUDOUT: OK (Not Supported)
Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
test VIDIOC_G/S_EDID: OK (Not Supported)

Test input 0:

Control ioctls:
test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK (Not Supported)
test VIDIOC_QUERYCTRL: OK (Not Supported)
test VIDIOC_G/S_CTRL: OK (Not Supported)
test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported)
test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: 

Re: [PATCH v2] Staging: bcm2048: Fix function argument alignment in radio-bcm2048.c.

[Quytelda Kahja]

Hans,

Thank you very much for your input on the patch; however this patch
has already been applied to the staging tree.  Additionally:

> What coding style problem? You should give a short description of
> what you are fixing.
The subject of the patch (which becomes the subject of the email when
using `git format-patch`) describes the change more fully: "Staging:
bcm2048: Fix function argument alignment in radio-bcm2048.c".  It's a
really trivial patch, so there's not too much to say.  That extra
comment is just redundant, I suppose.

> Just drop this change: it will replace one warning (non-aligned) with
> another (> 80 cols).
Breaking the 80 character line limit is arguably excusable for this
code because of the 36 character function name and 30 character
constant name; additionally, it has been said that the 80 character
line limit will probably be increased in the future since we run
modern machines that aren't limited to 80 character terminals anymore,
so this warning may soon be irrelevant anyway.

Thank you,
Quytelda Kahja

On Mon, Feb 26, 2018 at 5:51 AM, Hans Verkuil  wrote:
> On 02/20/2018 07:53 AM, Quytelda Kahja wrote:
>> Fix a coding style problem.
>
> What coding style problem? You should give a short description of
> what you are fixing.
>
>>
>> Signed-off-by: Quytelda Kahja 
>> ---
>> This is the patch without the unnecessary fixes for line length.
>>
>>  drivers/staging/media/bcm2048/radio-bcm2048.c | 22 +++---
>>  1 file changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
>> b/drivers/staging/media/bcm2048/radio-bcm2048.c
>> index 06d1920150da..f38a4f2acdde 100644
>> --- a/drivers/staging/media/bcm2048/radio-bcm2048.c
>> +++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
>> @@ -1864,7 +1864,7 @@ static int bcm2048_probe(struct bcm2048_device *bdev)
>>   goto unlock;
>>
>>   err = bcm2048_set_fm_search_rssi_threshold(bdev,
>> - BCM2048_DEFAULT_RSSI_THRESHOLD);
>> +
>> BCM2048_DEFAULT_RSSI_THRESHOLD);
>>   if (err < 0)
>>   goto unlock;
>>
>
> Just drop this change: it will replace one warning (non-aligned) with
> another (> 80 cols).
>
> This code is fine as it is.
>
> Regards,
>
> Hans
>
>> @@ -1942,9 +1942,9 @@ static irqreturn_t bcm2048_handler(int irq, void *dev)
>>   */
>>  #define property_write(prop, type, mask, check) 
>>  \
>>  static ssize_t bcm2048_##prop##_write(struct device *dev,\
>> - struct device_attribute *attr,  \
>> - const char *buf,\
>> - size_t count)   \
>> +   struct device_attribute *attr,\
>> +   const char *buf,  \
>> +   size_t count) \
>>  {\
>>   struct bcm2048_device *bdev = dev_get_drvdata(dev); \
>>   type value; \
>> @@ -1966,8 +1966,8 @@ static ssize_t bcm2048_##prop##_write(struct device 
>> *dev,   \
>>
>>  #define property_read(prop, mask)\
>>  static ssize_t bcm2048_##prop##_read(struct device *dev, \
>> - struct device_attribute *attr,  \
>> - char *buf)  \
>> +  struct device_attribute *attr, \
>> +  char *buf) \
>>  {\
>>   struct bcm2048_device *bdev = dev_get_drvdata(dev); \
>>   int value;  \
>> @@ -1985,8 +1985,8 @@ static ssize_t bcm2048_##prop##_read(struct device 
>> *dev,\
>>
>>  #define property_signed_read(prop, size, mask)  
>>  \
>>  static ssize_t bcm2048_##prop##_read(struct device *dev, \
>> - struct device_attribute *attr,  \
>> - char *buf)  \
>> +  struct device_attribute *attr, \
>> +  char *buf) \
>>  {\
>>   struct bcm2048_device *bdev = dev_get_drvdata(dev); \
>>   size value; \
>> @@ -2005,8 +2005,8 @@ property_read(prop, mask)  
>>  \
>>
>>  

Re: [PATCH v7 2/2] media: V3s: Add support for Allwinner CSI.

[Yong]

Hi,

On Mon, 26 Feb 2018 12:06:37 +0100
Hans Verkuil  wrote:

> Hi all,
> 
> On 01/30/2018 03:48 AM, Yong wrote:
> > Hi,
> > 
> > On Mon, 29 Jan 2018 13:49:14 -0800
> > Randy Dunlap  wrote:
> > 
> >> On 01/29/2018 01:21 AM, Yong Deng wrote:
> >>> Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2
> >>> interface and CSI1 is used for parallel interface. This is not
> >>> documented in datasheet but by test and guess.
> >>>
> >>> This patch implement a v4l2 framework driver for it.
> >>>
> >>> Currently, the driver only support the parallel interface. MIPI-CSI2,
> >>> ISP's support are not included in this patch.
> >>>
> >>> Tested-by: Maxime Ripard 
> >>> Signed-off-by: Yong Deng 
> >>> ---
> >>
> >>
> >> A previous version (I think v6) had a build error with the use of
> >> PHYS_OFFSET, so Kconfig was modified to depend on ARM and ARCH_SUNXI
> >> (one of which seems to be overkill).  As is here, the COMPILE_TEST piece is
> >> meaningless for all arches except ARM.  If you care enough for COMPILE_TEST
> >> (and I would), then you could make COMPILE_TEST useful on any arch by
> >> removing the "depends on ARM" (the ARCH_SUNXI takes care of that) and by
> >> having an alternate value for PHYS_OFFSET, like so:
> >>
> >> +#if defined(CONFIG_COMPILE_TEST) && !defined(PHYS_OFFSET)
> >> +#define PHYS_OFFSET   0
> >> +#endif
> >>
> >> With those 2 changes, the driver builds for me on x86_64.
> > 
> > I have considered this method.
> > But it's so sick to put these code in dirver (for my own). I mean 
> > this is meaningless for the driver itself and make people confused.
> > 
> > I grepped the driver/ code and I found many drivers writing Kconfig
> > like this. For example:
> > ARM && COMPILE_TEST
> > MIPS && COMPILE_TEST
> > PPC64 && COMPILE_TEST
> > 
> > BTW, for my own, I do not care about COMPILE_TEST.
> 
> There was a discussion about this in the v6 patch, but it petered out.
> 
> I want to merge this driver, but I would very much prefer that this
> compiles with COMPILE_TEST. So unless someone has a better solution, then
> adding 'hack' that defines PHYS_OFFSET to 0 for COMPILE_TEST would be 
> required.

If so, I will take the advice of Randy.

> 
> Otherwise this driver looks good, so it is just this issue blocking it.
> 
> Regards,
> 
>   Hans
> 
> > 
> >>
> >>> diff --git a/drivers/media/platform/sunxi/sun6i-csi/Kconfig 
> >>> b/drivers/media/platform/sunxi/sun6i-csi/Kconfig
> >>> new file mode 100644
> >>> index 000..f80c965
> >>> --- /dev/null
> >>> +++ b/drivers/media/platform/sunxi/sun6i-csi/Kconfig
> >>> @@ -0,0 +1,10 @@
> >>> +config VIDEO_SUN6I_CSI
> >>> + tristate "Allwinner V3s Camera Sensor Interface driver"
> >>> + depends on ARM
> >>> + depends on VIDEO_V4L2 && COMMON_CLK && VIDEO_V4L2_SUBDEV_API && HAS_DMA
> >>> + depends on ARCH_SUNXI || COMPILE_TEST
> >>> + select VIDEOBUF2_DMA_CONTIG
> >>> + select REGMAP_MMIO
> >>> + select V4L2_FWNODE
> >>> + ---help---
> >>> +Support for the Allwinner Camera Sensor Interface Controller on V3s.
> >>
> >> thanks,
> >> -- 
> >> ~Randy
> > 
> > 
> > Thanks,
> > Yong
> > 


Thanks,
Yong

Re: [PATCH v3] media: radio: Critical interrupt bugfix for si470x over i2c

[Douglas Fischer]

Hans,

See comments below. Thank you for the quick response on this and all
your patience and help in general with these patches.

On Mon, 26 Feb 2018 12:57:26 +0100
Hans Verkuil  wrote:

> On 02/26/2018 03:27 AM, Douglas Fischer wrote:
> > Fixed si470x_start() disabling the interrupt signal, causing tune
> > operations to never complete. This does not affect USB radios
> > because they poll the registers instead of using the IRQ line.
> > 
> > Stylistic and comment changes from v2.
> > 
> > Signed-off-by: Douglas Fischer 
> > ---
> > 
> > diff -uprN
> > linux.orig/drivers/media/radio/si470x/radio-si470x-common.c
> > linux/drivers/media/radio/si470x/radio-si470x-common.c ---
> > linux.orig/drivers/media/radio/si470x/radio-si470x-common.c
> > 2018-01-15 21:58:10.675620432 -0500 +++
> > linux/drivers/media/radio/si470x/radio-si470x-common.c
> > 2018-02-25 19:16:31.785934211 -0500 @@ -377,8 +377,11 @@ int
> > si470x_start(struct si470x_device *r goto done; /* sysconfig 1 */
> > -   radio->registers[SYSCONFIG1] =
> > -   (de << 11) & SYSCONFIG1_DE; /* DE*/
> > +   radio->registers[SYSCONFIG1] |=
> > SYSCONFIG1_RDSIEN|SYSCONFIG1_STCIEN|SYSCONFIG1_RDS;
> > +   radio->registers[SYSCONFIG1] &= ~SYSCONFIG1_GPIO2;
> > +   radio->registers[SYSCONFIG1] |= (0x01 << 2); /* GPIO2 */  
> 
> Yes, but what does this do? Enable GPIO2? The header defines two bits
> for GPIO1/2/3, but it doesn't say what those bits mean. So the
> question here is what it means to set bit 2 to 1 and bit 3 to 0? The
> header doesn't give any information about that, nor does this comment.
> 
SYSCONFIG1_GPIO2 is bits 2 and 3, I need to clear bit 3 and set bit 2 without 
changing the other bits. This configures GPIO2 as "STC/RDS interrupt. A logic 
high will be output unless an interrupt occurs". Should I change the comment to 
read "/* GPIO2 STC/RDS interrupt output */"?
> Regards,
> 
>   Hans
> 
> > +   if (de)
> > +   radio->registers[SYSCONFIG1] |= SYSCONFIG1_DE;
> > retval = si470x_set_register(radio, SYSCONFIG1);
> > if (retval < 0)
> > goto done;
> >   
> 
Thank you,
Doug

[PATCH v3] media: radio: Critical v4l2 registration bugfix for si470x over i2c

[Douglas Fischer]

Added the call to v4l2_device_register() required to add a new radio device.
Without this patch, it is impossible for the driver to load. This does not
affect USB devices.

Fixed cleanup order from v2.

Signed-off-by: Douglas Fischer 
---

diff -uprN linux.orig/drivers/media/radio/si470x/radio-si470x-i2c.c 
linux/drivers/media/radio/si470x/radio-si470x-i2c.c
--- linux.orig/drivers/media/radio/si470x/radio-si470x-i2c.c2018-01-15 
21:58:10.675620432 -0500
+++ linux/drivers/media/radio/si470x/radio-si470x-i2c.c 2018-02-25 
19:19:13.796927568 -0500
@@ -43,7 +43,6 @@ static const struct i2c_device_id si470x
 MODULE_DEVICE_TABLE(i2c, si470x_i2c_id);
 
 
-
 /**
  * Module Parameters
  **/
@@ -362,22 +361,43 @@ static int si470x_i2c_probe(struct i2c_c
mutex_init(>lock);
init_completion(>completion);
 
+   retval = v4l2_device_register(>dev, >v4l2_dev);
+   if (retval < 0) {
+   dev_err(>dev, "couldn't register v4l2_device\n");
+   goto err_radio;
+   }
+
+   v4l2_ctrl_handler_init(>hdl, 2);
+   v4l2_ctrl_new_std(>hdl, _ctrl_ops,
+   V4L2_CID_AUDIO_MUTE, 0, 1, 1, 1);
+   v4l2_ctrl_new_std(>hdl, _ctrl_ops,
+   V4L2_CID_AUDIO_VOLUME, 0, 15, 1, 15);
+   if (radio->hdl.error) {
+   retval = radio->hdl.error;
+   dev_err(>dev, "couldn't register control\n");
+   goto err_dev;
+   }
+
/* video device initialization */
radio->videodev = si470x_viddev_template;
+   radio->videodev.ctrl_handler = >hdl;
+   radio->videodev.lock = >lock;
+   radio->videodev.v4l2_dev = >v4l2_dev;
+   radio->videodev.release = video_device_release_empty;
video_set_drvdata(>videodev, radio);
 
/* power up : need 110ms */
radio->registers[POWERCFG] = POWERCFG_ENABLE;
if (si470x_set_register(radio, POWERCFG) < 0) {
retval = -EIO;
-   goto err_radio;
+   goto err_ctrl;
}
msleep(110);
 
/* get device and chip versions */
if (si470x_get_all_registers(radio) < 0) {
retval = -EIO;
-   goto err_radio;
+   goto err_ctrl;
}
dev_info(>dev, "DeviceID=0x%4.4hx ChipID=0x%4.4hx\n",
radio->registers[DEVICEID], 
radio->registers[SI_CHIPID]);
@@ -407,7 +427,7 @@ static int si470x_i2c_probe(struct i2c_c
radio->buffer = kmalloc(radio->buf_size, GFP_KERNEL);
if (!radio->buffer) {
retval = -EIO;
-   goto err_radio;
+   goto err_ctrl;
}
 
/* rds buffer configuration */
@@ -437,6 +457,10 @@ err_all:
free_irq(client->irq, radio);
 err_rds:
kfree(radio->buffer);
+err_ctrl:
+   v4l2_ctrl_handler_free(>hdl);
+err_dev:
+   v4l2_device_unregister(>v4l2_dev);
 err_radio:
kfree(radio);
 err_initial:

[PATCH 03/15] v4l: vsp1: Remove unused field from vsp1_drm_pipeline structure

[Laurent Pinchart]

The vsp1_drm_pipeline enabled field is set but never used. Remove it.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/vsp1_drm.c | 4 
 drivers/media/platform/vsp1/vsp1_drm.h | 2 --
 2 files changed, 6 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index a1f2ba044092..a267f12f0cc8 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -273,10 +273,6 @@ EXPORT_SYMBOL_GPL(vsp1_du_setup_lif);
  */
 void vsp1_du_atomic_begin(struct device *dev, unsigned int pipe_index)
 {
-   struct vsp1_device *vsp1 = dev_get_drvdata(dev);
-   struct vsp1_drm_pipeline *drm_pipe = >drm->pipe[pipe_index];
-
-   drm_pipe->enabled = drm_pipe->pipe.num_inputs != 0;
 }
 EXPORT_SYMBOL_GPL(vsp1_du_atomic_begin);
 
diff --git a/drivers/media/platform/vsp1/vsp1_drm.h 
b/drivers/media/platform/vsp1/vsp1_drm.h
index 1cd9db785bf7..9aa19325cbe9 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.h
+++ b/drivers/media/platform/vsp1/vsp1_drm.h
@@ -20,13 +20,11 @@
 /**
  * vsp1_drm_pipeline - State for the API exposed to the DRM driver
  * @pipe: the VSP1 pipeline used for display
- * @enabled: pipeline state at the beginning of an update
  * @du_complete: frame completion callback for the DU driver (optional)
  * @du_private: data to be passed to the du_complete callback
  */
 struct vsp1_drm_pipeline {
struct vsp1_pipeline pipe;
-   bool enabled;
 
/* Frame synchronisation */
void (*du_complete)(void *, bool);
-- 
Regards,

Laurent Pinchart

[PATCH 13/15] v4l: vsp1: Assign BRU and BRS to pipelines dynamically

[Laurent Pinchart]

The VSPDL variant drives two DU channels through two LIF and two
blenders, BRU and BRS. The DU channels thus share the five available
VSPDL inputs and expose them as five KMS planes.

The current implementation assigns the BRS to the second LIF and thus
artificially limits the number of planes for the second display channel
to two at most.

Lift this artificial limitation by assigning the BRU and BRS to the
display pipelines on demand based on the number of planes used by each
pipeline. When a display pipeline needs more than two inputs and the BRU
is already in use by the other pipeline, this requires reconfiguring the
other pipeline to free the BRU before processing, which can result in
frame drop on both pipelines.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/vsp1_drm.c | 160 +++--
 drivers/media/platform/vsp1/vsp1_drm.h |   9 ++
 2 files changed, 144 insertions(+), 25 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index 6c60b72b6f50..87e31ba0ddf5 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -39,7 +39,13 @@ static void vsp1_du_pipeline_frame_end(struct vsp1_pipeline 
*pipe,
struct vsp1_drm_pipeline *drm_pipe = to_vsp1_drm_pipeline(pipe);
 
if (drm_pipe->du_complete)
-   drm_pipe->du_complete(drm_pipe->du_private, completed);
+   drm_pipe->du_complete(drm_pipe->du_private,
+ completed && !notify);
+
+   if (notify) {
+   drm_pipe->force_bru_release = false;
+   wake_up(_pipe->wait_queue);
+   }
 }
 
 /* 
-
@@ -149,6 +155,10 @@ static int vsp1_du_pipeline_setup_rpf(struct vsp1_device 
*vsp1,
 }
 
 /* Setup the BRU source pad. */
+static int vsp1_du_pipeline_setup_input(struct vsp1_device *vsp1,
+   struct vsp1_pipeline *pipe);
+static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe);
+
 static int vsp1_du_pipeline_setup_bru(struct vsp1_device *vsp1,
  struct vsp1_pipeline *pipe)
 {
@@ -156,8 +166,93 @@ static int vsp1_du_pipeline_setup_bru(struct vsp1_device 
*vsp1,
struct v4l2_subdev_format format = {
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
};
+   struct vsp1_entity *bru;
int ret;
 
+   /*
+* Pick a BRU:
+* - If we need more than two inputs, use the main BRU.
+* - Otherwise, if we are not forced to release our BRU, keep it.
+* - Else, use any free BRU (randomly starting with the main BRU).
+*/
+   if (pipe->num_inputs > 2)
+   bru = >bru->entity;
+   else if (pipe->bru && !drm_pipe->force_bru_release)
+   bru = pipe->bru;
+   else if (!vsp1->bru->entity.pipe)
+   bru = >bru->entity;
+   else
+   bru = >brs->entity;
+
+   /* Switch BRU if needed. */
+   if (bru != pipe->bru) {
+   struct vsp1_entity *released_bru = NULL;
+
+   /* Release our BRU if we have one. */
+   if (pipe->bru) {
+   /*
+* The BRU might be acquired by the other pipeline in
+* the next step. We must thus remove it from the list
+* of entities for this pipeline. The other pipeline's
+* hardware configuration will reconfigure the BRU
+* routing.
+*
+* However, if the other pipeline doesn't acquire our
+* BRU, we need to keep it in the list, otherwise the
+* hardware configuration step won't disconnect it from
+* the pipeline. To solve this, store the released BRU
+* pointer to add it back to the list of entities later
+* if it isn't acquired by the other pipeline.
+*/
+   released_bru = pipe->bru;
+
+   list_del(>bru->list_pipe);
+   pipe->bru->sink = NULL;
+   pipe->bru->pipe = NULL;
+   pipe->bru = NULL;
+   }
+
+   /*
+* If the BRU we need is in use, force the owner pipeline to
+* switch to the other BRU and wait until the switch completes.
+*/
+   if (bru->pipe) {
+   struct vsp1_drm_pipeline *owner_pipe;
+
+   owner_pipe = to_vsp1_drm_pipeline(bru->pipe);
+   owner_pipe->force_bru_release = true;
+
+   vsp1_du_pipeline_setup_input(vsp1, _pipe->pipe);
+   

[PATCH 10/15] v4l: vsp1: Move DRM pipeline output setup code to a function

[Laurent Pinchart]

In order to make the vsp1_du_setup_lif() easier to read, and for
symmetry with the DRM pipeline input setup, move the pipeline output
setup code to a separate function.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/vsp1_drm.c | 107 +++--
 1 file changed, 61 insertions(+), 46 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index 00ce99bd1605..1c8adda47440 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -276,6 +276,66 @@ static int vsp1_du_pipeline_setup_input(struct vsp1_device 
*vsp1,
return 0;
 }
 
+/* Setup the output side of the pipeline (WPF and LIF). */
+static int vsp1_du_pipeline_setup_output(struct vsp1_device *vsp1,
+struct vsp1_pipeline *pipe)
+{
+   struct vsp1_drm_pipeline *drm_pipe = to_vsp1_drm_pipeline(pipe);
+   struct v4l2_subdev_format format = {
+   .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+   };
+   int ret;
+
+   format.pad = RWPF_PAD_SINK;
+   format.format.width = drm_pipe->width;
+   format.format.height = drm_pipe->height;
+   format.format.code = MEDIA_BUS_FMT_ARGB_1X32;
+   format.format.field = V4L2_FIELD_NONE;
+
+   ret = v4l2_subdev_call(>output->entity.subdev, pad, set_fmt, NULL,
+  );
+   if (ret < 0)
+   return ret;
+
+   dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on WPF%u sink\n",
+   __func__, format.format.width, format.format.height,
+   format.format.code, pipe->output->entity.index);
+
+   format.pad = RWPF_PAD_SOURCE;
+   ret = v4l2_subdev_call(>output->entity.subdev, pad, get_fmt, NULL,
+  );
+   if (ret < 0)
+   return ret;
+
+   dev_dbg(vsp1->dev, "%s: got format %ux%u (%x) on WPF%u source\n",
+   __func__, format.format.width, format.format.height,
+   format.format.code, pipe->output->entity.index);
+
+   format.pad = LIF_PAD_SINK;
+   ret = v4l2_subdev_call(>lif->subdev, pad, set_fmt, NULL,
+  );
+   if (ret < 0)
+   return ret;
+
+   dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on LIF%u sink\n",
+   __func__, format.format.width, format.format.height,
+   format.format.code, pipe->lif->index);
+
+   /*
+* Verify that the format at the output of the pipeline matches the
+* requested frame size and media bus code.
+*/
+   if (format.format.width != drm_pipe->width ||
+   format.format.height != drm_pipe->height ||
+   format.format.code != MEDIA_BUS_FMT_ARGB_1X32) {
+   dev_dbg(vsp1->dev, "%s: format mismatch on LIF%u\n", __func__,
+   pipe->lif->index);
+   return -EPIPE;
+   }
+
+   return 0;
+}
+
 /* Configure all entities in the pipeline. */
 static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe)
 {
@@ -356,7 +416,6 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
struct vsp1_drm_pipeline *drm_pipe;
struct vsp1_pipeline *pipe;
struct vsp1_bru *bru;
-   struct v4l2_subdev_format format;
unsigned long flags;
unsigned int i;
int ret;
@@ -417,54 +476,10 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
if (ret < 0)
return ret;
 
-   memset(, 0, sizeof(format));
-   format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
-   format.pad = RWPF_PAD_SINK;
-   format.format.width = cfg->width;
-   format.format.height = cfg->height;
-   format.format.code = MEDIA_BUS_FMT_ARGB_1X32;
-   format.format.field = V4L2_FIELD_NONE;
-
-   ret = v4l2_subdev_call(>output->entity.subdev, pad, set_fmt, NULL,
-  );
+   ret = vsp1_du_pipeline_setup_output(vsp1, pipe);
if (ret < 0)
return ret;
 
-   dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on WPF%u sink\n",
-   __func__, format.format.width, format.format.height,
-   format.format.code, pipe->output->entity.index);
-
-   format.pad = RWPF_PAD_SOURCE;
-   ret = v4l2_subdev_call(>output->entity.subdev, pad, get_fmt, NULL,
-  );
-   if (ret < 0)
-   return ret;
-
-   dev_dbg(vsp1->dev, "%s: got format %ux%u (%x) on WPF%u source\n",
-   __func__, format.format.width, format.format.height,
-   format.format.code, pipe->output->entity.index);
-
-   format.pad = LIF_PAD_SINK;
-   ret = v4l2_subdev_call(>lif->subdev, pad, set_fmt, NULL,
-  );
-   if (ret < 0)
-   return ret;
-
-   dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on LIF%u 

[PATCH 06/15] v4l: vsp1: Share duplicated DRM pipeline configuration code

[Laurent Pinchart]

Move the duplicated DRM pipeline configuration code to a function and
call it from vsp1_du_setup_lif() and vsp1_du_atomic_flush().

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/vsp1_drm.c | 95 +++---
 1 file changed, 43 insertions(+), 52 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index e210917fdc3f..9a043a915c0b 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -42,6 +42,47 @@ static void vsp1_du_pipeline_frame_end(struct vsp1_pipeline 
*pipe,
drm_pipe->du_complete(drm_pipe->du_private, completed);
 }
 
+/* 
-
+ * Pipeline Configuration
+ */
+
+/* Configure all entities in the pipeline. */
+static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe)
+{
+   struct vsp1_entity *entity;
+   struct vsp1_entity *next;
+   struct vsp1_dl_list *dl;
+
+   dl = vsp1_dl_list_get(pipe->output->dlm);
+
+   list_for_each_entry_safe(entity, next, >entities, list_pipe) {
+   /* Disconnect unused RPFs from the pipeline. */
+   if (entity->type == VSP1_ENTITY_RPF &&
+   !pipe->inputs[entity->index]) {
+   vsp1_dl_list_write(dl, entity->route->reg,
+  VI6_DPR_NODE_UNUSED);
+
+   entity->pipe = NULL;
+   list_del(>list_pipe);
+
+   continue;
+   }
+
+   vsp1_entity_route_setup(entity, pipe, dl);
+
+   if (entity->ops->configure) {
+   entity->ops->configure(entity, pipe, dl,
+  VSP1_ENTITY_PARAMS_INIT);
+   entity->ops->configure(entity, pipe, dl,
+  VSP1_ENTITY_PARAMS_RUNTIME);
+   entity->ops->configure(entity, pipe, dl,
+  VSP1_ENTITY_PARAMS_PARTITION);
+   }
+   }
+
+   vsp1_dl_list_commit(dl);
+}
+
 /* 
-
  * DU Driver API
  */
@@ -85,9 +126,6 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
struct vsp1_drm_pipeline *drm_pipe;
struct vsp1_pipeline *pipe;
struct vsp1_bru *bru;
-   struct vsp1_entity *entity;
-   struct vsp1_entity *next;
-   struct vsp1_dl_list *dl;
struct v4l2_subdev_format format;
unsigned long flags;
unsigned int i;
@@ -239,22 +277,7 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
vsp1_write(vsp1, VI6_DISP_IRQ_ENB, 0);
 
/* Configure all entities in the pipeline. */
-   dl = vsp1_dl_list_get(pipe->output->dlm);
-
-   list_for_each_entry_safe(entity, next, >entities, list_pipe) {
-   vsp1_entity_route_setup(entity, pipe, dl);
-
-   if (entity->ops->configure) {
-   entity->ops->configure(entity, pipe, dl,
-  VSP1_ENTITY_PARAMS_INIT);
-   entity->ops->configure(entity, pipe, dl,
-  VSP1_ENTITY_PARAMS_RUNTIME);
-   entity->ops->configure(entity, pipe, dl,
-  VSP1_ENTITY_PARAMS_PARTITION);
-   }
-   }
-
-   vsp1_dl_list_commit(dl);
+   vsp1_du_pipeline_configure(pipe);
 
/* Start the pipeline. */
spin_lock_irqsave(>irqlock, flags);
@@ -490,15 +513,9 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int 
pipe_index)
struct vsp1_pipeline *pipe = _pipe->pipe;
struct vsp1_rwpf *inputs[VSP1_MAX_RPF] = { NULL, };
struct vsp1_bru *bru = to_bru(>bru->subdev);
-   struct vsp1_entity *entity;
-   struct vsp1_entity *next;
-   struct vsp1_dl_list *dl;
unsigned int i;
int ret;
 
-   /* Prepare the display list. */
-   dl = vsp1_dl_list_get(pipe->output->dlm);
-
/* Count the number of enabled inputs and sort them by Z-order. */
pipe->num_inputs = 0;
 
@@ -557,33 +574,7 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int 
pipe_index)
__func__, rpf->entity.index);
}
 
-   /* Configure all entities in the pipeline. */
-   list_for_each_entry_safe(entity, next, >entities, list_pipe) {
-   /* Disconnect unused RPFs from the pipeline. */
-   if (entity->type == VSP1_ENTITY_RPF &&
-   !pipe->inputs[entity->index]) {
-   vsp1_dl_list_write(dl, entity->route->reg,
-  VI6_DPR_NODE_UNUSED);
-
-   

[PATCH 12/15] v4l: vsp1: Generalize detection of entity removal from DRM pipeline

[Laurent Pinchart]

When disabling a DRM plane, the corresponding RPF is only marked as
removed from the pipeline in the atomic update handler, with the actual
removal happening when configuring the pipeline at atomic commit time.
This is required as the RPF has to be disabled in the hardware, which
can't be done from the atomic update handler.

The current implementation is RPF-specific. Make it independent of the
entity type by using the entity's pipe pointer to mark removal from the
pipeline. This will allow using the mechanism to remove BRU instances.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/vsp1_drm.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index d705a6e9fa1d..6c60b72b6f50 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -346,13 +346,12 @@ static void vsp1_du_pipeline_configure(struct 
vsp1_pipeline *pipe)
dl = vsp1_dl_list_get(pipe->output->dlm);
 
list_for_each_entry_safe(entity, next, >entities, list_pipe) {
-   /* Disconnect unused RPFs from the pipeline. */
-   if (entity->type == VSP1_ENTITY_RPF &&
-   !pipe->inputs[entity->index]) {
+   /* Disconnect unused entities from the pipeline. */
+   if (!entity->pipe) {
vsp1_dl_list_write(dl, entity->route->reg,
   VI6_DPR_NODE_UNUSED);
 
-   entity->pipe = NULL;
+   entity->sink = NULL;
list_del(>list_pipe);
 
continue;
@@ -569,10 +568,11 @@ int vsp1_du_atomic_update(struct device *dev, unsigned 
int pipe_index,
rpf_index);
 
/*
-* Remove the RPF from the pipe's inputs. The atomic flush
-* handler will disable the input and remove the entity from the
-* pipe's entities list.
+* Remove the RPF from the pipeline's inputs. Keep it in the
+* pipeline's entity list to let vsp1_du_pipeline_configure()
+* remove it from the hardware pipeline.
 */
+   rpf->entity.pipe = NULL;
drm_pipe->pipe.inputs[rpf_index] = NULL;
return 0;
}
-- 
Regards,

Laurent Pinchart

[PATCH 15/15] v4l: vsp1: Rename BRU to BRx

[Laurent Pinchart]

Some VSP instances have two blending units named BRU (Blend/ROP Unit)
and BRS (Blend/ROP Sub unit). The BRS is a smaller version of the BRU
with only two inputs, but otherwise offers similar features and offers
the same register interface. The BRU and BRS can be used exchangeably in
VSP pipelines (provided no more than two inputs are needed).

Due to historical reasons, the VSP1 driver implements support for both
the BRU and BRS through objects named vsp1_bru. The code uses the name
BRU to refer to either the BRU or the BRS, except in a few places where
noted explicitly. This creates confusion.

In an effort to avoid confusion, rename the vsp1_bru object and the
corresponding API to vsp1_brx, and use BRx to refer to blend unit
instances regardless of their type. The names BRU and BRS are retained
where reference to a particular blend unit type is needed, as well as in
hardware registers to stay close to the datasheet.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/Makefile   |   2 +-
 drivers/media/platform/vsp1/vsp1.h |   6 +-
 .../media/platform/vsp1/{vsp1_bru.c => vsp1_brx.c} | 202 ++---
 .../media/platform/vsp1/{vsp1_bru.h => vsp1_brx.h} |  18 +-
 drivers/media/platform/vsp1/vsp1_drm.c | 172 +-
 drivers/media/platform/vsp1/vsp1_drm.h |   6 +-
 drivers/media/platform/vsp1/vsp1_drv.c |   6 +-
 drivers/media/platform/vsp1/vsp1_pipe.c|  12 +-
 drivers/media/platform/vsp1/vsp1_pipe.h|   4 +-
 drivers/media/platform/vsp1/vsp1_rpf.c |  12 +-
 drivers/media/platform/vsp1/vsp1_rwpf.h|   2 +-
 drivers/media/platform/vsp1/vsp1_video.c   |  16 +-
 drivers/media/platform/vsp1/vsp1_wpf.c |   8 +-
 13 files changed, 233 insertions(+), 233 deletions(-)
 rename drivers/media/platform/vsp1/{vsp1_bru.c => vsp1_brx.c} (63%)
 rename drivers/media/platform/vsp1/{vsp1_bru.h => vsp1_brx.h} (66%)

diff --git a/drivers/media/platform/vsp1/Makefile 
b/drivers/media/platform/vsp1/Makefile
index f5cd6f0491cb..596775f932c0 100644
--- a/drivers/media/platform/vsp1/Makefile
+++ b/drivers/media/platform/vsp1/Makefile
@@ -3,7 +3,7 @@ vsp1-y  := vsp1_drv.o 
vsp1_entity.o vsp1_pipe.o
 vsp1-y += vsp1_dl.o vsp1_drm.o vsp1_video.o
 vsp1-y += vsp1_rpf.o vsp1_rwpf.o vsp1_wpf.o
 vsp1-y += vsp1_clu.o vsp1_hsit.o vsp1_lut.o
-vsp1-y += vsp1_bru.o vsp1_sru.o vsp1_uds.o
+vsp1-y += vsp1_brx.o vsp1_sru.o vsp1_uds.o
 vsp1-y += vsp1_hgo.o vsp1_hgt.o vsp1_histo.o
 vsp1-y += vsp1_lif.o
 
diff --git a/drivers/media/platform/vsp1/vsp1.h 
b/drivers/media/platform/vsp1/vsp1.h
index 78ef838416b3..894cc725c2d4 100644
--- a/drivers/media/platform/vsp1/vsp1.h
+++ b/drivers/media/platform/vsp1/vsp1.h
@@ -30,7 +30,7 @@ struct rcar_fcp_device;
 struct vsp1_drm;
 struct vsp1_entity;
 struct vsp1_platform_data;
-struct vsp1_bru;
+struct vsp1_brx;
 struct vsp1_clu;
 struct vsp1_hgo;
 struct vsp1_hgt;
@@ -78,8 +78,8 @@ struct vsp1_device {
struct rcar_fcp_device *fcp;
struct device *bus_master;
 
-   struct vsp1_bru *brs;
-   struct vsp1_bru *bru;
+   struct vsp1_brx *brs;
+   struct vsp1_brx *bru;
struct vsp1_clu *clu;
struct vsp1_hgo *hgo;
struct vsp1_hgt *hgt;
diff --git a/drivers/media/platform/vsp1/vsp1_bru.c 
b/drivers/media/platform/vsp1/vsp1_brx.c
similarity index 63%
rename from drivers/media/platform/vsp1/vsp1_bru.c
rename to drivers/media/platform/vsp1/vsp1_brx.c
index e8fd2ae3b3eb..b4af1d546022 100644
--- a/drivers/media/platform/vsp1/vsp1_bru.c
+++ b/drivers/media/platform/vsp1/vsp1_brx.c
@@ -1,5 +1,5 @@
 /*
- * vsp1_bru.c  --  R-Car VSP1 Blend ROP Unit
+ * vsp1_brx.c  --  R-Car VSP1 Blend ROP Unit (BRU and BRS)
  *
  * Copyright (C) 2013 Renesas Corporation
  *
@@ -17,45 +17,45 @@
 #include 
 
 #include "vsp1.h"
-#include "vsp1_bru.h"
+#include "vsp1_brx.h"
 #include "vsp1_dl.h"
 #include "vsp1_pipe.h"
 #include "vsp1_rwpf.h"
 #include "vsp1_video.h"
 
-#define BRU_MIN_SIZE   1U
-#define BRU_MAX_SIZE   8190U
+#define BRX_MIN_SIZE   1U
+#define BRX_MAX_SIZE   8190U
 
 /* 
-
  * Device Access
  */
 
-static inline void vsp1_bru_write(struct vsp1_bru *bru, struct vsp1_dl_list 
*dl,
+static inline void vsp1_brx_write(struct vsp1_brx *brx, struct vsp1_dl_list 
*dl,
  u32 reg, u32 data)
 {
-   vsp1_dl_list_write(dl, bru->base + reg, data);
+   vsp1_dl_list_write(dl, brx->base + reg, data);
 }
 
 /* 

[PATCH 04/15] v4l: vsp1: Store pipeline pointer in vsp1_entity

[Laurent Pinchart]

Various types of objects subclassing vsp1_entity currently store a
pointer to the pipeline. Move the pointer to vsp1_entity to simplify the
code and avoid storing the pipeline in more entity subclasses later.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/vsp1_drm.c| 20 +--
 drivers/media/platform/vsp1/vsp1_drv.c|  2 +-
 drivers/media/platform/vsp1/vsp1_entity.h |  2 ++
 drivers/media/platform/vsp1/vsp1_histo.c  |  2 +-
 drivers/media/platform/vsp1/vsp1_histo.h  |  3 ---
 drivers/media/platform/vsp1/vsp1_pipe.c   | 33 +--
 drivers/media/platform/vsp1/vsp1_rwpf.h   |  2 --
 drivers/media/platform/vsp1/vsp1_video.c  | 17 +++-
 8 files changed, 34 insertions(+), 47 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index a267f12f0cc8..a7ad85ab0b08 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -120,6 +120,7 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
 * inputs.
 */
WARN_ON(list_empty(>entity.list_pipe));
+   rpf->entity.pipe = NULL;
list_del_init(>entity.list_pipe);
pipe->inputs[i] = NULL;
 
@@ -536,8 +537,10 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int 
pipe_index)
continue;
}
 
-   if (list_empty(>entity.list_pipe))
+   if (list_empty(>entity.list_pipe)) {
+   rpf->entity.pipe = pipe;
list_add_tail(>entity.list_pipe, >entities);
+   }
 
bru->inputs[i].rpf = rpf;
rpf->bru_input = i;
@@ -562,6 +565,7 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int 
pipe_index)
vsp1_dl_list_write(dl, entity->route->reg,
   VI6_DPR_NODE_UNUSED);
 
+   entity->pipe = NULL;
list_del_init(>list_pipe);
 
continue;
@@ -625,24 +629,28 @@ int vsp1_drm_init(struct vsp1_device *vsp1)
 
vsp1_pipeline_init(pipe);
 
+   pipe->frame_end = vsp1_du_pipeline_frame_end;
+
/*
 * The DRM pipeline is static, add entities manually. The first
 * pipeline uses the BRU and the second pipeline the BRS.
 */
pipe->bru = i == 0 ? >bru->entity : >brs->entity;
-   pipe->lif = >lif[i]->entity;
pipe->output = vsp1->wpf[i];
-   pipe->output->pipe = pipe;
-   pipe->frame_end = vsp1_du_pipeline_frame_end;
+   pipe->lif = >lif[i]->entity;
 
+   pipe->bru->pipe = pipe;
pipe->bru->sink = >output->entity;
pipe->bru->sink_pad = 0;
+   list_add_tail(>bru->list_pipe, >entities);
+
+   pipe->output->entity.pipe = pipe;
pipe->output->entity.sink = pipe->lif;
pipe->output->entity.sink_pad = 0;
+   list_add_tail(>output->entity.list_pipe, >entities);
 
-   list_add_tail(>bru->list_pipe, >entities);
+   pipe->lif->pipe = pipe;
list_add_tail(>lif->list_pipe, >entities);
-   list_add_tail(>output->entity.list_pipe, >entities);
}
 
/* Disable all RPFs initially. */
diff --git a/drivers/media/platform/vsp1/vsp1_drv.c 
b/drivers/media/platform/vsp1/vsp1_drv.c
index eed9516e25e1..58a7993f2306 100644
--- a/drivers/media/platform/vsp1/vsp1_drv.c
+++ b/drivers/media/platform/vsp1/vsp1_drv.c
@@ -63,7 +63,7 @@ static irqreturn_t vsp1_irq_handler(int irq, void *data)
vsp1_write(vsp1, VI6_WPF_IRQ_STA(i), ~status & mask);
 
if (status & VI6_WFP_IRQ_STA_DFE) {
-   vsp1_pipeline_frame_end(wpf->pipe);
+   vsp1_pipeline_frame_end(wpf->entity.pipe);
ret = IRQ_HANDLED;
}
}
diff --git a/drivers/media/platform/vsp1/vsp1_entity.h 
b/drivers/media/platform/vsp1/vsp1_entity.h
index 408602ebeb97..c26523c56c05 100644
--- a/drivers/media/platform/vsp1/vsp1_entity.h
+++ b/drivers/media/platform/vsp1/vsp1_entity.h
@@ -106,6 +106,8 @@ struct vsp1_entity {
unsigned int index;
const struct vsp1_route *route;
 
+   struct vsp1_pipeline *pipe;
+
struct list_head list_dev;
struct list_head list_pipe;
 
diff --git a/drivers/media/platform/vsp1/vsp1_histo.c 
b/drivers/media/platform/vsp1/vsp1_histo.c
index afab77cf4fa5..8638ebc514b4 100644
--- a/drivers/media/platform/vsp1/vsp1_histo.c
+++ b/drivers/media/platform/vsp1/vsp1_histo.c
@@ -61,7 +61,7 @@ void vsp1_histogram_buffer_complete(struct vsp1_histogram 

[PATCH 02/15] v4l: vsp1: Remove outdated comment

[Laurent Pinchart]

The entities in the pipeline are all started when the LIF is setup.
Remove the outdated comment that state otherwise.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/vsp1_drm.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index e31fb371eaf9..a1f2ba044092 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -221,11 +221,7 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
return -EPIPE;
}
 
-   /*
-* Enable the VSP1. We don't start the entities themselves right at this
-* point as there's no plane configured yet, so we can't start
-* processing buffers.
-*/
+   /* Enable the VSP1. */
ret = vsp1_device_get(vsp1);
if (ret < 0)
return ret;
-- 
Regards,

Laurent Pinchart

[PATCH 08/15] v4l: vsp1: Setup BRU at atomic commit time

[Laurent Pinchart]

To implement fully dynamic plane assignment to pipelines, we need to
reassign the BRU and BRS to the DRM pipelines in the atomic commit
handler. In preparation for this setup factor out the BRU source pad
code and call it both at LIF setup and atomic commit time.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/vsp1_drm.c | 56 +-
 drivers/media/platform/vsp1/vsp1_drm.h |  5 +++
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index 7bf697ba7969..6ad8aa6c8138 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -148,12 +148,51 @@ static int vsp1_du_pipeline_setup_rpf(struct vsp1_device 
*vsp1,
return 0;
 }
 
+/* Setup the BRU source pad. */
+static int vsp1_du_pipeline_setup_bru(struct vsp1_device *vsp1,
+ struct vsp1_pipeline *pipe)
+{
+   struct vsp1_drm_pipeline *drm_pipe = to_vsp1_drm_pipeline(pipe);
+   struct v4l2_subdev_format format = {
+   .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+   };
+   int ret;
+
+   /*
+* Configure the format on the BRU source and verify that it matches the
+* requested format. We don't set the media bus code as it is configured
+* on the BRU sink pad 0 and propagated inside the entity, not on the
+* source pad.
+*/
+   format.pad = pipe->bru->source_pad;
+   format.format.width = drm_pipe->width;
+   format.format.height = drm_pipe->height;
+   format.format.field = V4L2_FIELD_NONE;
+
+   ret = v4l2_subdev_call(>bru->subdev, pad, set_fmt, NULL,
+  );
+   if (ret < 0)
+   return ret;
+
+   dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on %s pad %u\n",
+   __func__, format.format.width, format.format.height,
+   format.format.code, BRU_NAME(pipe->bru), pipe->bru->source_pad);
+
+   if (format.format.width != drm_pipe->width ||
+   format.format.height != drm_pipe->height) {
+   dev_dbg(vsp1->dev, "%s: format mismatch\n", __func__);
+   return -EPIPE;
+   }
+
+   return 0;
+}
+
 static unsigned int rpf_zpos(struct vsp1_device *vsp1, struct vsp1_rwpf *rpf)
 {
return vsp1->drm->inputs[rpf->entity.index].zpos;
 }
 
-/* Setup the input side of the pipeline (RPFs and BRU sink pads). */
+/* Setup the input side of the pipeline (RPFs and BRU). */
 static int vsp1_du_pipeline_setup_input(struct vsp1_device *vsp1,
struct vsp1_pipeline *pipe)
 {
@@ -191,6 +230,18 @@ static int vsp1_du_pipeline_setup_input(struct vsp1_device 
*vsp1,
inputs[j] = rpf;
}
 
+   /*
+* Setup the BRU. This must be done before setting up the RPF input
+* pipelines as the BRU sink compose rectangles depend on the BRU source
+* format.
+*/
+   ret = vsp1_du_pipeline_setup_bru(vsp1, pipe);
+   if (ret < 0) {
+   dev_err(vsp1->dev, "%s: failed to setup %s source\n", __func__,
+   BRU_NAME(pipe->bru));
+   return ret;
+   }
+
/* Setup the RPF input pipeline for every enabled input. */
for (i = 0; i < pipe->bru->source_pad; ++i) {
struct vsp1_rwpf *rpf = inputs[i];
@@ -355,6 +406,9 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
return 0;
}
 
+   drm_pipe->width = cfg->width;
+   drm_pipe->height = cfg->height;
+
dev_dbg(vsp1->dev, "%s: configuring LIF%u with format %ux%u\n",
__func__, pipe_index, cfg->width, cfg->height);
 
diff --git a/drivers/media/platform/vsp1/vsp1_drm.h 
b/drivers/media/platform/vsp1/vsp1_drm.h
index 9aa19325cbe9..c8dd75ba01f6 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.h
+++ b/drivers/media/platform/vsp1/vsp1_drm.h
@@ -20,12 +20,17 @@
 /**
  * vsp1_drm_pipeline - State for the API exposed to the DRM driver
  * @pipe: the VSP1 pipeline used for display
+ * @width: output display width
+ * @height: output display height
  * @du_complete: frame completion callback for the DU driver (optional)
  * @du_private: data to be passed to the du_complete callback
  */
 struct vsp1_drm_pipeline {
struct vsp1_pipeline pipe;
 
+   unsigned int width;
+   unsigned int height;
+
/* Frame synchronisation */
void (*du_complete)(void *, bool);
void *du_private;
-- 
Regards,

Laurent Pinchart

[PATCH 09/15] v4l: vsp1: Replace manual DRM pipeline input setup in vsp1_du_setup_lif

[Laurent Pinchart]

The vsp1_du_setup_lif() function setups the DRM pipeline input manually.
This duplicates the code from the vsp1_du_pipeline_setup_input()
function. Replace the manual implementation by a call to the function.

As the pipeline has no enabled input in vsp1_du_setup_lif(), the
vsp1_du_pipeline_setup_input() function will not setup any RPF, and will
thus not setup formats on the BRU sink pads. This isn't a problem as all
inputs are disabled, and the BRU sink pads will be reconfigured from the
atomic commit handler when inputs will be enabled.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/vsp1_drm.c | 40 +-
 1 file changed, 6 insertions(+), 34 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index 6ad8aa6c8138..00ce99bd1605 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -412,47 +412,19 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
dev_dbg(vsp1->dev, "%s: configuring LIF%u with format %ux%u\n",
__func__, pipe_index, cfg->width, cfg->height);
 
-   /*
-* Configure the format at the BRU sinks and propagate it through the
-* pipeline.
-*/
+   /* Setup formats through the pipeline. */
+   ret = vsp1_du_pipeline_setup_input(vsp1, pipe);
+   if (ret < 0)
+   return ret;
+
memset(, 0, sizeof(format));
format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
-
-   for (i = 0; i < pipe->bru->source_pad; ++i) {
-   format.pad = i;
-
-   format.format.width = cfg->width;
-   format.format.height = cfg->height;
-   format.format.code = MEDIA_BUS_FMT_ARGB_1X32;
-   format.format.field = V4L2_FIELD_NONE;
-
-   ret = v4l2_subdev_call(>bru->subdev, pad,
-  set_fmt, NULL, );
-   if (ret < 0)
-   return ret;
-
-   dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on %s pad %u\n",
-   __func__, format.format.width, format.format.height,
-   format.format.code, BRU_NAME(pipe->bru), i);
-   }
-
-   format.pad = pipe->bru->source_pad;
+   format.pad = RWPF_PAD_SINK;
format.format.width = cfg->width;
format.format.height = cfg->height;
format.format.code = MEDIA_BUS_FMT_ARGB_1X32;
format.format.field = V4L2_FIELD_NONE;
 
-   ret = v4l2_subdev_call(>bru->subdev, pad, set_fmt, NULL,
-  );
-   if (ret < 0)
-   return ret;
-
-   dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on %s pad %u\n",
-   __func__, format.format.width, format.format.height,
-   format.format.code, BRU_NAME(pipe->bru), i);
-
-   format.pad = RWPF_PAD_SINK;
ret = v4l2_subdev_call(>output->entity.subdev, pad, set_fmt, NULL,
   );
if (ret < 0)
-- 
Regards,

Laurent Pinchart

[PATCH 00/15] R-Car VSP1: Dynamically assign blend units to display pipelines

[Laurent Pinchart]

Hello,

On R-Car H3 ES2.0+ and M3-N SoCs, two display pipelines are served by the same
VSP instance (named VSPDL). The VSPDL includes two blending units named BRU
and BRS, unlike the other display-related VSPD that serves a single display
channel with a single blending unit.

The VSPDL has five inputs and can freely assign them at runtime to two display
pipelines, using the BRU and BRS to blend multiple inputs. The BRU supports
blending up to five inputs, while the BRS is limited to two inputs.

Each display pipeline requires a blending unit, and the BRU and BRS are
currently assigned statically to the first and second pipeline respectively.
This artificially limits the number of inputs for the second pipeline to two.
To overcome that limitation, the BRU and BRS need to be dynamically assigned
to the display pipelines, which is what this patch series does.

Patches 01/15 to 10/15 perform small cleanups and refactoring to prepare for
the rest of the series. Patches 11/15 and 12/15 implement new internal
features for the same purpose, and patch 13/15 performs the bulk of the work
by implementing dynamic BRU and BRS assignment to pipelines.

Reassigning the BRU and BRS when two pipelines are running results in flicker
as one pipeline has to first release its blending unit. Synchronization
between the two pipelines also require locking that effectively serializes
page flips for the two pipelines, even when not interacting with each other.
This is currently believed to be unavoidable due to the hardware design, but
please feel free to prove me wrong (ideally with a patch - one can always
dream).

Patch 14/15 then adds messages useful for debugging this new feature. I have
kept it separate from 13/15 to make it easier to remove those messages once
dynamic assignment of blending units will be deemed perfectly stable, but I
won't oppose squashing it with 13/15 if that is preferred.

Patch 15/15 finally rename BRU to BRx to avoid confusion between the BRU terms
that refer to the BRU in particular, and the ones that refer to any of the BRU
or BRS. As this might be a bit controversial I've put the patch last in the
series in case it needs to be dropped.

I have decided to CC the dri-devel mailing list even though the code doesn't
touch the R-Car DU driver and will be merged through the Linux media tree as
the display is involved and the series could benefit from the expertise of the
DRM/KMS community from that point of view.

The patches are based on top of the latest Linux media master branch. For
convenience their are available from

git://linuxtv.org/pinchartl/media.git v4l2/vsp1/bru-brs

The series passes the DU test suite with the new BRx dynamic assignment
test available from

git://git.ideasonboard.com/renesas/kms-tests.git bru-brs

The VSP test suite also runs without any noticed regression.

Laurent Pinchart (15):
  v4l: vsp1: Don't start/stop media pipeline for DRM
  v4l: vsp1: Remove outdated comment
  v4l: vsp1: Remove unused field from vsp1_drm_pipeline structure
  v4l: vsp1: Store pipeline pointer in vsp1_entity
  v4l: vsp1: Use vsp1_entity.pipe to check if entity belongs to a
pipeline
  v4l: vsp1: Share duplicated DRM pipeline configuration code
  v4l: vsp1: Move DRM atomic commit pipeline setup to separate function
  v4l: vsp1: Setup BRU at atomic commit time
  v4l: vsp1: Replace manual DRM pipeline input setup in
vsp1_du_setup_lif
  v4l: vsp1: Move DRM pipeline output setup code to a function
  v4l: vsp1: Add per-display list completion notification support
  v4l: vsp1: Generalize detection of entity removal from DRM pipeline
  v4l: vsp1: Assign BRU and BRS to pipelines dynamically
  v4l: vsp1: Add BRx dynamic assignment debugging messages
  v4l: vsp1: Rename BRU to BRx

 drivers/media/platform/vsp1/Makefile   |   2 +-
 drivers/media/platform/vsp1/vsp1.h |   6 +-
 .../media/platform/vsp1/{vsp1_bru.c => vsp1_brx.c} | 202 ++---
 .../media/platform/vsp1/{vsp1_bru.h => vsp1_brx.h} |  18 +-
 drivers/media/platform/vsp1/vsp1_dl.c  |  27 +-
 drivers/media/platform/vsp1/vsp1_dl.h  |   4 +-
 drivers/media/platform/vsp1/vsp1_drm.c | 829 -
 drivers/media/platform/vsp1/vsp1_drm.h |  16 +-
 drivers/media/platform/vsp1/vsp1_drv.c |   8 +-
 drivers/media/platform/vsp1/vsp1_entity.h  |   2 +
 drivers/media/platform/vsp1/vsp1_histo.c   |   2 +-
 drivers/media/platform/vsp1/vsp1_histo.h   |   3 -
 drivers/media/platform/vsp1/vsp1_pipe.c|  50 +-
 drivers/media/platform/vsp1/vsp1_pipe.h|   7 +-
 drivers/media/platform/vsp1/vsp1_rpf.c |  12 +-
 drivers/media/platform/vsp1/vsp1_rwpf.h|   4 +-
 drivers/media/platform/vsp1/vsp1_video.c   |  37 +-
 drivers/media/platform/vsp1/vsp1_wpf.c |   8 +-
 18 files changed, 705 insertions(+), 532 deletions(-)
 rename drivers/media/platform/vsp1/{vsp1_bru.c => 

[PATCH 11/15] v4l: vsp1: Add per-display list completion notification support

[Laurent Pinchart]

Display list completion is already reported to the frame end handler,
but that mechanism is global to all display lists. In order to implement
BRU and BRS reassignment in DRM pipelines we will need to wait for
completion of a particular display list. Extend the display list and
frame end handler APIs to support such a notification.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/vsp1_dl.c| 27 +--
 drivers/media/platform/vsp1/vsp1_dl.h|  4 ++--
 drivers/media/platform/vsp1/vsp1_drm.c   |  4 ++--
 drivers/media/platform/vsp1/vsp1_pipe.c  |  5 +++--
 drivers/media/platform/vsp1/vsp1_pipe.h  |  3 ++-
 drivers/media/platform/vsp1/vsp1_video.c |  4 ++--
 6 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index 0b86ed01e85d..eb2971218e28 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -72,6 +72,7 @@ struct vsp1_dl_body {
  * @fragments: list of extra display list bodies
  * @has_chain: if true, indicates that there's a partition chain
  * @chain: entry in the display list partition chain
+ * @notify: whether the display list completion should be notified
  */
 struct vsp1_dl_list {
struct list_head list;
@@ -85,6 +86,8 @@ struct vsp1_dl_list {
 
bool has_chain;
struct list_head chain;
+
+   bool notify;
 };
 
 enum vsp1_dl_mode {
@@ -550,8 +553,16 @@ static void vsp1_dl_list_commit_continuous(struct 
vsp1_dl_list *dl)
 * case we can't replace the queued list by the new one, as we could
 * race with the hardware. We thus mark the update as pending, it will
 * be queued up to the hardware by the frame end interrupt handler.
+*
+* If a display list is already pending we simply drop it as the new
+* display list is assumed to contain a more recent configuration. It is
+* an error if the already pending list has the notify flag set, as
+* there is then a process waiting for that list to complete. This
+* shouldn't happen as the waiting process should perform proper
+* locking, but warn just in case.
 */
if (vsp1_dl_list_hw_update_pending(dlm)) {
+   WARN_ON(dlm->pending && dlm->pending->notify);
__vsp1_dl_list_put(dlm->pending);
dlm->pending = dl;
return;
@@ -581,7 +592,7 @@ static void vsp1_dl_list_commit_singleshot(struct 
vsp1_dl_list *dl)
dlm->active = dl;
 }
 
-void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
+void vsp1_dl_list_commit(struct vsp1_dl_list *dl, bool notify)
 {
struct vsp1_dl_manager *dlm = dl->dlm;
struct vsp1_dl_list *dl_child;
@@ -598,6 +609,8 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
}
}
 
+   dl->notify = notify;
+
spin_lock_irqsave(>lock, flags);
 
if (dlm->singleshot)
@@ -615,16 +628,23 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
 /**
  * vsp1_dlm_irq_frame_end - Display list handler for the frame end interrupt
  * @dlm: the display list manager
+ * @notify: whether the display list that completed has notification enabled
  *
  * Return true if the previous display list has completed at frame end, or 
false
  * if it has been delayed by one frame because the display list commit raced
  * with the frame end interrupt. The function always returns true in header 
mode
  * as display list processing is then not continuous and races never occur.
+ *
+ * Upon return, the @notify parameter is set to true if the previous display
+ * list has completed and had been queued with the notify flag, or to false
+ * otherwise. Notification is only supported for continuous mode.
  */
-bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
+bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm, bool *notify)
 {
bool completed = false;
 
+   *notify = false;
+
spin_lock(>lock);
 
/*
@@ -652,6 +672,9 @@ bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
 * frame end interrupt. The display list thus becomes active.
 */
if (dlm->queued) {
+   *notify = dlm->queued->notify;
+   dlm->queued->notify = false;
+
__vsp1_dl_list_put(dlm->active);
dlm->active = dlm->queued;
dlm->queued = NULL;
diff --git a/drivers/media/platform/vsp1/vsp1_dl.h 
b/drivers/media/platform/vsp1/vsp1_dl.h
index ee3508172f0a..480c6b0dd2e4 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.h
+++ b/drivers/media/platform/vsp1/vsp1_dl.h
@@ -27,12 +27,12 @@ struct vsp1_dl_manager *vsp1_dlm_create(struct vsp1_device 
*vsp1,
unsigned int prealloc);
 void vsp1_dlm_destroy(struct vsp1_dl_manager *dlm);
 void vsp1_dlm_reset(struct vsp1_dl_manager *dlm);
-bool 

[PATCH 14/15] v4l: vsp1: Add BRx dynamic assignment debugging messages

[Laurent Pinchart]

Dynamic assignment of the BRU and BRS to pipelines is prone to
regressions, add messages to make debugging easier. Keep it as a
separate commit to ease removal of those messages once the code will
deem to be completely stable.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/vsp1_drm.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index 87e31ba0ddf5..521bbc227110 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -190,6 +190,10 @@ static int vsp1_du_pipeline_setup_bru(struct vsp1_device 
*vsp1,
 
/* Release our BRU if we have one. */
if (pipe->bru) {
+   dev_dbg(vsp1->dev, "%s: pipe %u: releasing %s\n",
+   __func__, pipe->lif->index,
+   BRU_NAME(pipe->bru));
+
/*
 * The BRU might be acquired by the other pipeline in
 * the next step. We must thus remove it from the list
@@ -219,6 +223,9 @@ static int vsp1_du_pipeline_setup_bru(struct vsp1_device 
*vsp1,
if (bru->pipe) {
struct vsp1_drm_pipeline *owner_pipe;
 
+   dev_dbg(vsp1->dev, "%s: pipe %u: waiting for %s\n",
+   __func__, pipe->lif->index, BRU_NAME(bru));
+
owner_pipe = to_vsp1_drm_pipeline(bru->pipe);
owner_pipe->force_bru_release = true;
 
@@ -245,6 +252,9 @@ static int vsp1_du_pipeline_setup_bru(struct vsp1_device 
*vsp1,
  >entities);
 
/* Add the BRU to the pipeline. */
+   dev_dbg(vsp1->dev, "%s: pipe %u: acquired %s\n",
+   __func__, pipe->lif->index, BRU_NAME(bru));
+
pipe->bru = bru;
pipe->bru->pipe = pipe;
pipe->bru->sink = >output->entity;
@@ -549,6 +559,10 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
drm_pipe->du_complete = NULL;
pipe->num_inputs = 0;
 
+   dev_dbg(vsp1->dev, "%s: pipe %u: releasing %s\n",
+   __func__, pipe->lif->index,
+   BRU_NAME(pipe->bru));
+
list_del(>bru->list_pipe);
pipe->bru->pipe = NULL;
pipe->bru = NULL;
-- 
Regards,

Laurent Pinchart

[PATCH 07/15] v4l: vsp1: Move DRM atomic commit pipeline setup to separate function

[Laurent Pinchart]

The DRM pipeline setup code used at atomic commit time is similar to the
setup code used when enabling the pipeline. Move it to a separate
function in order to share it.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/vsp1_drm.c | 347 +
 1 file changed, 180 insertions(+), 167 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index 9a043a915c0b..7bf697ba7969 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -46,6 +46,185 @@ static void vsp1_du_pipeline_frame_end(struct vsp1_pipeline 
*pipe,
  * Pipeline Configuration
  */
 
+/* Setup one RPF and the connected BRU sink pad. */
+static int vsp1_du_pipeline_setup_rpf(struct vsp1_device *vsp1,
+ struct vsp1_pipeline *pipe,
+ struct vsp1_rwpf *rpf,
+ unsigned int bru_input)
+{
+   struct v4l2_subdev_selection sel;
+   struct v4l2_subdev_format format;
+   const struct v4l2_rect *crop;
+   int ret;
+
+   /*
+* Configure the format on the RPF sink pad and propagate it up to the
+* BRU sink pad.
+*/
+   crop = >drm->inputs[rpf->entity.index].crop;
+
+   memset(, 0, sizeof(format));
+   format.which = V4L2_SUBDEV_FORMAT_ACTIVE;
+   format.pad = RWPF_PAD_SINK;
+   format.format.width = crop->width + crop->left;
+   format.format.height = crop->height + crop->top;
+   format.format.code = rpf->fmtinfo->mbus;
+   format.format.field = V4L2_FIELD_NONE;
+
+   ret = v4l2_subdev_call(>entity.subdev, pad, set_fmt, NULL,
+  );
+   if (ret < 0)
+   return ret;
+
+   dev_dbg(vsp1->dev,
+   "%s: set format %ux%u (%x) on RPF%u sink\n",
+   __func__, format.format.width, format.format.height,
+   format.format.code, rpf->entity.index);
+
+   memset(, 0, sizeof(sel));
+   sel.which = V4L2_SUBDEV_FORMAT_ACTIVE;
+   sel.pad = RWPF_PAD_SINK;
+   sel.target = V4L2_SEL_TGT_CROP;
+   sel.r = *crop;
+
+   ret = v4l2_subdev_call(>entity.subdev, pad, set_selection, NULL,
+  );
+   if (ret < 0)
+   return ret;
+
+   dev_dbg(vsp1->dev,
+   "%s: set selection (%u,%u)/%ux%u on RPF%u sink\n",
+   __func__, sel.r.left, sel.r.top, sel.r.width, sel.r.height,
+   rpf->entity.index);
+
+   /*
+* RPF source, hardcode the format to ARGB to turn on format
+* conversion if needed.
+*/
+   format.pad = RWPF_PAD_SOURCE;
+
+   ret = v4l2_subdev_call(>entity.subdev, pad, get_fmt, NULL,
+  );
+   if (ret < 0)
+   return ret;
+
+   dev_dbg(vsp1->dev,
+   "%s: got format %ux%u (%x) on RPF%u source\n",
+   __func__, format.format.width, format.format.height,
+   format.format.code, rpf->entity.index);
+
+   format.format.code = MEDIA_BUS_FMT_ARGB_1X32;
+
+   ret = v4l2_subdev_call(>entity.subdev, pad, set_fmt, NULL,
+  );
+   if (ret < 0)
+   return ret;
+
+   /* BRU sink, propagate the format from the RPF source. */
+   format.pad = bru_input;
+
+   ret = v4l2_subdev_call(>bru->subdev, pad, set_fmt, NULL,
+  );
+   if (ret < 0)
+   return ret;
+
+   dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on %s pad %u\n",
+   __func__, format.format.width, format.format.height,
+   format.format.code, BRU_NAME(pipe->bru), format.pad);
+
+   sel.pad = bru_input;
+   sel.target = V4L2_SEL_TGT_COMPOSE;
+   sel.r = vsp1->drm->inputs[rpf->entity.index].compose;
+
+   ret = v4l2_subdev_call(>bru->subdev, pad, set_selection, NULL,
+  );
+   if (ret < 0)
+   return ret;
+
+   dev_dbg(vsp1->dev, "%s: set selection (%u,%u)/%ux%u on %s pad %u\n",
+   __func__, sel.r.left, sel.r.top, sel.r.width, sel.r.height,
+   BRU_NAME(pipe->bru), sel.pad);
+
+   return 0;
+}
+
+static unsigned int rpf_zpos(struct vsp1_device *vsp1, struct vsp1_rwpf *rpf)
+{
+   return vsp1->drm->inputs[rpf->entity.index].zpos;
+}
+
+/* Setup the input side of the pipeline (RPFs and BRU sink pads). */
+static int vsp1_du_pipeline_setup_input(struct vsp1_device *vsp1,
+   struct vsp1_pipeline *pipe)
+{
+   struct vsp1_rwpf *inputs[VSP1_MAX_RPF] = { NULL, };
+   struct vsp1_bru *bru = to_bru(>bru->subdev);
+   unsigned int i;
+   int ret;
+
+   /* Count the number of enabled inputs and sort them by Z-order. */
+   pipe->num_inputs = 0;
+
+   for (i = 0; i < 

[PATCH 01/15] v4l: vsp1: Don't start/stop media pipeline for DRM

[Laurent Pinchart]

The DRM support code manages a pipeline of VSP entities, each backed by
a media entity. When starting or stopping the pipeline, it starts and
stops the media pipeline through the media API in order to store the
pipeline pointer in every entity.

The driver doesn't use the pipe pointer in media entities, neither does
it rely on the other effects of the media_pipeline_start() and
media_pipeline_stop() functions. Furthermore, as the media links for the
DRM pipeline are never set up correctly, and as the pipeline can be
modified dynamically when enabling or disabling planes, the current
implementation is not correct. Remove the incorrect and unneeded code.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/vsp1_drm.c | 18 +++---
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index b8fee1834253..e31fb371eaf9 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -109,8 +109,6 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
if (ret == -ETIMEDOUT)
dev_err(vsp1->dev, "DRM pipeline stop timeout\n");
 
-   media_pipeline_stop(>output->entity.subdev.entity);
-
for (i = 0; i < ARRAY_SIZE(pipe->inputs); ++i) {
struct vsp1_rwpf *rpf = pipe->inputs[i];
 
@@ -224,11 +222,9 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
}
 
/*
-* Mark the pipeline as streaming and enable the VSP1. This will store
-* the pipeline pointer in all entities, which the s_stream handlers
-* will need. We don't start the entities themselves right at this point
-* as there's no plane configured yet, so we can't start processing
-* buffers.
+* Enable the VSP1. We don't start the entities themselves right at this
+* point as there's no plane configured yet, so we can't start
+* processing buffers.
 */
ret = vsp1_device_get(vsp1);
if (ret < 0)
@@ -241,14 +237,6 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
drm_pipe->du_complete = cfg->callback;
drm_pipe->du_private = cfg->callback_data;
 
-   ret = media_pipeline_start(>output->entity.subdev.entity,
- >pipe);
-   if (ret < 0) {
-   dev_dbg(vsp1->dev, "%s: pipeline start failed\n", __func__);
-   vsp1_device_put(vsp1);
-   return ret;
-   }
-
/* Disable the display interrupts. */
vsp1_write(vsp1, VI6_DISP_IRQ_STA, 0);
vsp1_write(vsp1, VI6_DISP_IRQ_ENB, 0);
-- 
Regards,

Laurent Pinchart

[PATCH 05/15] v4l: vsp1: Use vsp1_entity.pipe to check if entity belongs to a pipeline

[Laurent Pinchart]

The DRM pipeline handling code uses the entity's pipe list head to check
whether the entity is already included in a pipeline. This method is a
bit fragile in the sense that it uses list_empty() on a list_head that
is a list member. Replace it by a simpler check for the entity pipe
pointer.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/vsp1_drm.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index a7ad85ab0b08..e210917fdc3f 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -119,9 +119,9 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
 * Remove the RPF from the pipe and the list of BRU
 * inputs.
 */
-   WARN_ON(list_empty(>entity.list_pipe));
+   WARN_ON(!rpf->entity.pipe);
rpf->entity.pipe = NULL;
-   list_del_init(>entity.list_pipe);
+   list_del(>entity.list_pipe);
pipe->inputs[i] = NULL;
 
bru->inputs[rpf->bru_input].rpf = NULL;
@@ -537,7 +537,7 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int 
pipe_index)
continue;
}
 
-   if (list_empty(>entity.list_pipe)) {
+   if (!rpf->entity.pipe) {
rpf->entity.pipe = pipe;
list_add_tail(>entity.list_pipe, >entities);
}
@@ -566,7 +566,7 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int 
pipe_index)
   VI6_DPR_NODE_UNUSED);
 
entity->pipe = NULL;
-   list_del_init(>list_pipe);
+   list_del(>list_pipe);
 
continue;
}
-- 
Regards,

Laurent Pinchart

[PATCH] media: platform: Drop OF dependency of VIDEO_RENESAS_VSP1

[Geert Uytterhoeven]

VIDEO_RENESAS_VSP1 depends on ARCH_RENESAS && OF.
As ARCH_RENESAS implies OF, the latter can be dropped.

Signed-off-by: Geert Uytterhoeven 
---
 drivers/media/platform/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 614fbef08ddcabb0..2b8b1ad0edd9eb31 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -448,7 +448,7 @@ config VIDEO_RENESAS_FCP
 config VIDEO_RENESAS_VSP1
tristate "Renesas VSP1 Video Processing Engine"
depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && HAS_DMA
-   depends on (ARCH_RENESAS && OF) || COMPILE_TEST
+   depends on ARCH_RENESAS || COMPILE_TEST
depends on (!ARM64 && !VIDEO_RENESAS_FCP) || VIDEO_RENESAS_FCP
select VIDEOBUF2_DMA_CONTIG
select VIDEOBUF2_VMALLOC
-- 
2.7.4

[ragnatech:media-tree] BUILD SUCCESS 52e17089d1850774d2ef583cdef2b060b84fca8c

[kbuild test robot]

tree/branch: git://git.ragnatech.se/linux  media-tree
branch HEAD: 52e17089d1850774d2ef583cdef2b060b84fca8c  media: imx: Don't 
initialize vars that won't be used

elapsed time: 203m

configs tested: 133

The following configs have been built successfully.
More configs may be tested in the coming days.

pariscc3000_defconfig
parisc b180_defconfig
parisc  defconfig
alpha   defconfig
pariscallnoconfig
cris etrax-100lx_v2_defconfig
shallnoconfig
sh  rsk7269_defconfig
sh  sh7785lcr_32bit_defconfig
shtitan_defconfig
powerpc  arches_defconfig
powerpc   ep405_defconfig
powerpcsam440ep_defconfig
i386   tinyconfig
x86_64lkp
x86_64   rhel
x86_64   rhel-7.2
powerpc   allnoconfig
powerpc defconfig
powerpc   ppc64_defconfig
s390default_defconfig
i386 randconfig-i0-201808
i386 randconfig-i1-201808
x86_64 acpi-redef
x86_64   allyesdebian
x86_64nfsroot
x86_64  kexec
x86_64  federa-25
i386 allmodconfig
x86_64 randconfig-x017-201808
x86_64 randconfig-x013-201808
x86_64 randconfig-x016-201808
x86_64 randconfig-x018-201808
x86_64 randconfig-x015-201808
x86_64 randconfig-x012-201808
x86_64 randconfig-x014-201808
x86_64 randconfig-x011-201808
x86_64 randconfig-x010-201808
x86_64 randconfig-x019-201808
ia64 alldefconfig
ia64  allnoconfig
ia64defconfig
i386 randconfig-a1-201808
i386 randconfig-a0-201808
x86_64 randconfig-s0-02270100
x86_64 randconfig-s1-02270100
x86_64 randconfig-s2-02270100
frv defconfig
mn10300 asb2364_defconfig
openriscor1ksim_defconfig
tile tilegx_defconfig
um i386_defconfig
um   x86_64_defconfig
c6xevmc6678_defconfig
h8300h8300h-sim_defconfig
m32r   m32104ut_defconfig
m32r mappi3.smp_defconfig
m32r opsput_defconfig
m32r   usrv_defconfig
nios2 10m50_defconfig
score  spct6600_defconfig
xtensa   common_defconfig
xtensa  iss_defconfig
blackfinBF526-EZBRD_defconfig
blackfinBF533-EZKIT_defconfig
blackfinBF561-EZKIT-SMP_defconfig
blackfin  TCM-BF537_defconfig
m68k   sun3_defconfig
m68k  multi_defconfig
m68k   m5475evb_defconfig
i386 randconfig-s0-201808
i386 randconfig-s1-201808
x86_64 randconfig-s3-02270045
x86_64 randconfig-s4-02270045
x86_64 randconfig-s5-02270045
x86_64   randconfig-i0-201808
microblaze  mmu_defconfig
microblazenommu_defconfig
i386   randconfig-x015-201808
i386   randconfig-x019-201808
i386   randconfig-x010-201808
i386   randconfig-x014-201808
i386   randconfig-x016-201808
i386   randconfig-x012-201808
i386   randconfig-x011-201808
i386   randconfig-x017-201808
i386   randconfig-x013-201808
i386   randconfig-x018-201808
mn10300  alldefconfig
powerpc  obs600_defconfig
sparc   defconfig
sparc64   allnoconfig
sparc64 defconfig
arm at91_dt_defconfig
arm   allnoconfig
arm   efm32_defconfig
arm64   defconfig
armmulti_v5_defconfig
arm   sunxi_defconfig
arm64 allnoconfig
arm  exynos_defconfig
armshmobile_defconfig

[PATCH] media: vb2: Makefile: place vb2-trace together with vb2-core

[Mauro Carvalho Chehab]

We don't want a separate module for vb2-trace.

That fixes this warning:

WARNING: modpost: missing MODULE_LICENSE() in 
drivers/media/common/videobuf2/vb2-trace.o

When building as module.

While here, add a SPDX header.

Reported-by: Stephen Rothwell 
Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/common/videobuf2/Makefile | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/media/common/videobuf2/Makefile 
b/drivers/media/common/videobuf2/Makefile
index 067badb1aaa7..77bebe8b202f 100644
--- a/drivers/media/common/videobuf2/Makefile
+++ b/drivers/media/common/videobuf2/Makefile
@@ -1,11 +1,14 @@
+# SPDX-License-Identifier: GPL-2.0
+videobuf2-common-objs := videobuf2-core.o
 
-obj-$(CONFIG_VIDEOBUF2_CORE) += videobuf2-core.o
+ifeq ($(CONFIG_TRACEPOINTS),y)
+  videobuf2-common-objs += vb2-trace.o
+endif
+
+obj-$(CONFIG_VIDEOBUF2_CORE) += videobuf2-common.o
 obj-$(CONFIG_VIDEOBUF2_V4L2) += videobuf2-v4l2.o
 obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o
 obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2-vmalloc.o
 obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o
 obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o
 obj-$(CONFIG_VIDEOBUF2_DVB) += videobuf2-dvb.o
-ifeq ($(CONFIG_TRACEPOINTS),y)
-  obj-$(CONFIG_VIDEOBUF2_CORE) += vb2-trace.o
-endif
-- 
2.14.3

Re: [PATCH V2 1/2] [media] Add Rockchip RK1608 driver

[Heiko Stuebner]

Hi Linus,

thanks for catching these things :-) .

Am Montag, 26. Februar 2018, 11:12:30 CET schrieb Linus Walleij:
> On Mon, Feb 26, 2018 at 9:16 AM, Wen Nuan  wrote:
> > +   pdata->grf_gpio2b_iomux = ioremap((resource_size_t)
> > + (GRF_BASE_ADDR +
> > +  GRF_GPIO2B_IOMUX), 4);
> > +   grf_val = __raw_readl(pdata->grf_gpio2b_iomux);
> > +   __raw_writel(((grf_val) | (1 << 6) | (1 << (6 + 16))),
> > +pdata->grf_gpio2b_iomux);
> > +
> > +   pdata->grf_io_vsel = ioremap((resource_size_t)
> > + (GRF_BASE_ADDR + 
> > GRF_IO_VSEL), 4);
> > +   grf_val = __raw_readl(pdata->grf_io_vsel);
> > +   __raw_writel(((grf_val) | (1 << 1) | (1 << (1 + 16))),
> > +pdata->grf_io_vsel);
> 
> You are doing pin control on the side of the pin control subsystem
> it looks like?
> 
> I think David Wu and Heiko Stubner needs to have a look at what you
> are doing here to suggest other solutions.

Especially as the rk1608 seems to be some a spi-connected peripheral.
So it should definitly not touch _any_ soc-specific registers at all.

I just looked up the patch in patchwork and apart from the one Linus
quoted above, I found quite a bit more open-coded pinctrl settings
as well as direct writes to the clock controller and even io-voltage
selections.

All these things are highly soc-specific so vary with each soc this
ic gets connected to and the kernel does provide abstractions for
all of them. For clock-rates there are the clock-apis and also
the assigned-clock* properties for the devicetree, pinctrl supports
multiple states and io-vsel selections normally should just monitor
the supply-regulator via the io-domain driver we already have
[See vqmmc handling in for sd-cards for example].

So this driver should not touch anything of that sort and therefore
should _not_ contain any iomem-based read or write at all.


Heiko

Re: [PATCHv4 06/15] subdev-formats.rst: fix incorrect types

[Mauro Carvalho Chehab]

Hi Hans,

Em Wed, 21 Feb 2018 16:32:09 +0100
Hans Verkuil  escreveu:

> The ycbcr_enc, quantization and xfer_func fields are __u16 and not enums.
> 
> Signed-off-by: Hans Verkuil 
> Acked-by: Sakari Ailus 

Thanks for your patch. I have one comment about it, though. See below.

> ---
>  Documentation/media/uapi/v4l/subdev-formats.rst | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/media/uapi/v4l/subdev-formats.rst 
> b/Documentation/media/uapi/v4l/subdev-formats.rst
> index b1eea44550e1..4f0c0b282f98 100644
> --- a/Documentation/media/uapi/v4l/subdev-formats.rst
> +++ b/Documentation/media/uapi/v4l/subdev-formats.rst
> @@ -33,17 +33,17 @@ Media Bus Formats
>- Image colorspace, from enum
>   :c:type:`v4l2_colorspace`. See
>   :ref:`colorspaces` for details.
> -* - enum :c:type:`v4l2_ycbcr_encoding`
> +* - __u16
>- ``ycbcr_enc``
>- This information supplements the ``colorspace`` and must be set by
>   the driver for capture streams and by the application for output
>   streams, see :ref:`colorspaces`.

While this patch makes sense, it excludes an important information:
what are the valid values for this field.

I was expecting something like what's written for the code field:

 * - __u32
   - ``code``
   - Format code, from enum
:ref:`v4l2_mbus_pixelcode `.

Something like:

* - enum :c:type:`v4l2_ycbcr_encoding`
 * - __u16
   - This information supplements the ``colorspace`` and must be set by
the driver for capture streams and by the application for output
streams from enum :ref:`v4l2_mbus_pixelcode `.
See :ref:`colorspaces`.

The same applies to the other changes below.

As this patch is independent from the others at your pull request,
I'm skipping it.

Regards,
Mauro

> -* - enum :c:type:`v4l2_quantization`
> +* - __u16
>- ``quantization``
>- This information supplements the ``colorspace`` and must be set by
>   the driver for capture streams and by the application for output
>   streams, see :ref:`colorspaces`.
> -* - enum :c:type:`v4l2_xfer_func`
> +* - __u16
>- ``xfer_func``
>- This information supplements the ``colorspace`` and must be set by
>   the driver for capture streams and by the application for output



Thanks,
Mauro

[GIT PULL FOR v4.17] Various fixes

[Hans Verkuil]

The following changes since commit 52e17089d1850774d2ef583cdef2b060b84fca8c:

  media: imx: Don't initialize vars that won't be used (2018-02-26 08:38:57 
-0500)

are available in the Git repository at:

  git://linuxtv.org/hverkuil/media_tree.git for-v4.17b

for you to fetch changes up to c35d71cf9814a1c0d78356eb0564527318aef440:

  media: ov2685: mark PM functions as __maybe_unused (2018-02-26 15:27:34 +0100)


Arnd Bergmann (3):
  media: i2c: TDA1997x: add CONFIG_SND dependency
  media: ov5695: mark PM functions as __maybe_unused
  media: ov2685: mark PM functions as __maybe_unused

Fabio Estevam (2):
  media: imx-media-internal-sd: Use empty initializer
  media: imx-ic-prpencvf: Use empty initializer to clear all struct members

Hans Verkuil (1):
  imx/Kconfig: add depends on HAS_DMA

Hugues Fruchet (4):
  media: stm32-dcmi: remove redundant capture enable
  media: stm32-dcmi: remove redundant clear of interrupt flags
  media: stm32-dcmi: improve error trace points
  media: stm32-dcmi: add g/s_parm framerate support

Kieran Bingham (1):
  media: i2c: adv748x: Fix cleanup jump on chip identification

Parthiban Nallathambi (1):
  media: imx: capture: reformat line to 80 chars or less

Steve Longerbeam (3):
  media: staging/imx: Implement init_cfg subdev pad op
  media: imx: mipi csi-2: Fix set_fmt try
  media: imx.rst: Fix formatting errors

 Documentation/media/v4l-drivers/imx.rst   | 24 +---
 drivers/media/i2c/Kconfig |  2 ++
 drivers/media/i2c/adv748x/adv748x-core.c  |  2 +-
 drivers/media/i2c/ov2685.c|  4 ++--
 drivers/media/i2c/ov5695.c|  4 ++--
 drivers/media/platform/stm32/stm32-dcmi.c | 40 

 drivers/staging/media/imx/Kconfig |  1 +
 drivers/staging/media/imx/imx-ic-prp.c|  1 +
 drivers/staging/media/imx/imx-ic-prpencvf.c   |  3 ++-
 drivers/staging/media/imx/imx-media-capture.c |  3 ++-
 drivers/staging/media/imx/imx-media-csi.c |  1 +
 drivers/staging/media/imx/imx-media-internal-sd.c |  2 +-
 drivers/staging/media/imx/imx-media-utils.c   | 29 
+
 drivers/staging/media/imx/imx-media-vdic.c|  1 +
 drivers/staging/media/imx/imx-media.h |  2 ++
 drivers/staging/media/imx/imx6-mipi-csi2.c| 25 
-
 16 files changed, 104 insertions(+), 40 deletions(-)

[PATCH 2/2] media: tw9910: solve coding style issues

[Mauro Carvalho Chehab]

As we're adding this as a new driver, make checkpatch happier by
solving several style issues, using --fix-inplace at strict mode.

Some issues required manual work.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/i2c/tw9910.c | 32 
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index 96792df45fb0..cc5d383fc6b8 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -339,6 +339,7 @@ static int tw9910_mask_set(struct i2c_client *client, u8 
command,
   u8 mask, u8 set)
 {
s32 val = i2c_smbus_read_byte_data(client, command);
+
if (val < 0)
return val;
 
@@ -389,7 +390,7 @@ static int tw9910_set_hsync(struct i2c_client *client)
 
/* So far only revisions 0 and 1 have been seen */
/* bit 2 - 0 */
-   if (1 == priv->revision)
+   if (priv->revision == 1)
ret = tw9910_mask_set(client, HSLOWCTL, 0x77,
  (HSYNC_START & 0x0007) << 4 |
  (HSYNC_END   & 0x0007));
@@ -511,10 +512,10 @@ static int tw9910_s_std(struct v4l2_subdev *sd, 
v4l2_std_id norm)
 {
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct tw9910_priv *priv = to_tw9910(client);
-   const unsigned hact = 720;
-   const unsigned hdelay = 15;
-   unsigned vact;
-   unsigned vdelay;
+   const unsigned int hact = 720;
+   const unsigned int hdelay = 15;
+   unsigned int vact;
+   unsigned int vdelay;
int ret;
 
if (!(norm & (V4L2_STD_NTSC | V4L2_STD_PAL)))
@@ -532,16 +533,16 @@ static int tw9910_s_std(struct v4l2_subdev *sd, 
v4l2_std_id norm)
}
if (!ret)
ret = i2c_smbus_write_byte_data(client, CROP_HI,
-   ((vdelay >> 2) & 0xc0) |
+   ((vdelay >> 2) & 0xc0) |
((vact >> 4) & 0x30) |
((hdelay >> 6) & 0x0c) |
((hact >> 8) & 0x03));
if (!ret)
ret = i2c_smbus_write_byte_data(client, VDELAY_LO,
-   vdelay & 0xff);
+   vdelay & 0xff);
if (!ret)
ret = i2c_smbus_write_byte_data(client, VACTIVE_LO,
-   vact & 0xff);
+   vact & 0xff);
 
return ret;
 }
@@ -731,7 +732,7 @@ static int tw9910_set_frame(struct v4l2_subdev *sd, u32 
*width, u32 *height)
 }
 
 static int tw9910_get_selection(struct v4l2_subdev *sd,
-   struct v4l2_subdev_pad_config *cfg,
+   struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_selection *sel)
 {
struct i2c_client *client = v4l2_get_subdevdata(sd);
@@ -756,7 +757,7 @@ static int tw9910_get_selection(struct v4l2_subdev *sd,
 }
 
 static int tw9910_get_fmt(struct v4l2_subdev *sd,
-   struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *format)
 {
struct v4l2_mbus_framefmt *mf = >format;
@@ -807,7 +808,7 @@ static int tw9910_s_fmt(struct v4l2_subdev *sd,
 }
 
 static int tw9910_set_fmt(struct v4l2_subdev *sd,
-   struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *format)
 {
struct v4l2_mbus_framefmt *mf = >format;
@@ -818,9 +819,9 @@ static int tw9910_set_fmt(struct v4l2_subdev *sd,
if (format->pad)
return -EINVAL;
 
-   if (V4L2_FIELD_ANY == mf->field) {
+   if (mf->field == V4L2_FIELD_ANY) {
mf->field = V4L2_FIELD_INTERLACED_BT;
-   } else if (V4L2_FIELD_INTERLACED_BT != mf->field) {
+   } else if (mf->field != V4L2_FIELD_INTERLACED_BT) {
dev_err(>dev, "Field type %d invalid.\n", mf->field);
return -EINVAL;
}
@@ -870,8 +871,7 @@ static int tw9910_video_probe(struct i2c_client *client)
priv->revision = GET_REV(id);
id = GET_ID(id);
 
-   if (0x0B != id ||
-   0x01 < priv->revision) {
+   if (id != 0x0b || priv->revision > 0x01) {
dev_err(>dev,
"Product ID error %x:%x\n",
id, priv->revision);
@@ -899,7 +899,7 @@ static const struct v4l2_subdev_core_ops 
tw9910_subdev_core_ops = {
 };
 
 static int tw9910_enum_mbus_code(struct v4l2_subdev *sd,
-   struct v4l2_subdev_pad_config *cfg,
+struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_mbus_code_enum *code)
 {
if (code->pad || code->index)
-- 
2.14.3

[PATCH 1/2] media: ov772x: fix whitespace issues

[Mauro Carvalho Chehab]

As we're adding this as a new driver, make checkpatch happier by
solving some whitespace issues, using --fix-inplace.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/i2c/ov772x.c | 19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 3edf0cb6fd0e..16665af0c712 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -375,7 +375,7 @@
  */
 #define OV7720  0x7720
 #define OV7725  0x7721
-#define VERSION(pid, ver) ((pid<<8)|(ver&0xFF))
+#define VERSION(pid, ver) ((pid << 8) | (ver & 0xFF))
 
 /*
  * PLL multipliers
@@ -500,7 +500,6 @@ static const struct ov772x_color_format ov772x_cfmts[] = {
},
 };
 
-
 /*
  * window size list
  */
@@ -557,6 +556,7 @@ static int ov772x_mask_set(struct i2c_client *client, u8  
command, u8  mask,
   u8  set)
 {
s32 val = ov772x_read(client, command);
+
if (val < 0)
return val;
 
@@ -919,7 +919,6 @@ static int ov772x_set_params(struct ov772x_priv *priv,
 * Edge Ctrl
 */
if (priv->info->edgectrl.strength & OV772X_MANUAL_EDGE_CTRL) {
-
/*
 * Manual Edge Control Mode
 *
@@ -1064,7 +1063,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
 }
 
 static int ov772x_get_selection(struct v4l2_subdev *sd,
-   struct v4l2_subdev_pad_config *cfg,
+   struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_selection *sel)
 {
struct ov772x_priv *priv = to_ov772x(sd);
@@ -1087,7 +1086,7 @@ static int ov772x_get_selection(struct v4l2_subdev *sd,
 }
 
 static int ov772x_get_fmt(struct v4l2_subdev *sd,
-   struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *format)
 {
struct v4l2_mbus_framefmt *mf = >format;
@@ -1106,7 +1105,7 @@ static int ov772x_get_fmt(struct v4l2_subdev *sd,
 }
 
 static int ov772x_set_fmt(struct v4l2_subdev *sd,
-   struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *format)
 {
struct ov772x_priv *priv = to_ov772x(sd);
@@ -1219,7 +1218,7 @@ static int ov772x_enum_frame_interval(struct v4l2_subdev 
*sd,
 }
 
 static int ov772x_enum_mbus_code(struct v4l2_subdev *sd,
-   struct v4l2_subdev_pad_config *cfg,
+struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_mbus_code_enum *code)
 {
if (code->pad || code->index >= ARRAY_SIZE(ov772x_cfmts))
@@ -1282,11 +1281,11 @@ static int ov772x_probe(struct i2c_client *client,
v4l2_i2c_subdev_init(>subdev, client, _subdev_ops);
v4l2_ctrl_handler_init(>hdl, 3);
v4l2_ctrl_new_std(>hdl, _ctrl_ops,
-   V4L2_CID_VFLIP, 0, 1, 1, 0);
+ V4L2_CID_VFLIP, 0, 1, 1, 0);
v4l2_ctrl_new_std(>hdl, _ctrl_ops,
-   V4L2_CID_HFLIP, 0, 1, 1, 0);
+ V4L2_CID_HFLIP, 0, 1, 1, 0);
v4l2_ctrl_new_std(>hdl, _ctrl_ops,
-   V4L2_CID_BAND_STOP_FILTER, 0, 256, 1, 0);
+ V4L2_CID_BAND_STOP_FILTER, 0, 256, 1, 0);
priv->subdev.ctrl_handler = >hdl;
if (priv->hdl.error)
return priv->hdl.error;
-- 
2.14.3

Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

[Claudiu Beznea]

On 26.02.2018 11:57, Jani Nikula wrote:
> On Thu, 22 Feb 2018, Daniel Thompson  wrote:
>> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
>>> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
>>> were adapted to this change.
>>>
>>> Signed-off-by: Claudiu Beznea 
>>> ---
>>>  arch/arm/mach-s3c24xx/mach-rx1950.c  | 11 +--
>>>  drivers/bus/ts-nbus.c|  2 +-
>>>  drivers/clk/clk-pwm.c|  3 ++-
>>>  drivers/gpu/drm/i915/intel_panel.c   | 17 ++---
>>>  drivers/hwmon/pwm-fan.c  |  2 +-
>>>  drivers/input/misc/max77693-haptic.c |  2 +-
>>>  drivers/input/misc/max8997_haptic.c  |  6 +-
>>>  drivers/leds/leds-pwm.c  |  5 -
>>>  drivers/media/rc/ir-rx51.c   |  5 -
>>>  drivers/media/rc/pwm-ir-tx.c |  5 -
>>>  drivers/video/backlight/lm3630a_bl.c |  4 +++-
>>>  drivers/video/backlight/lp855x_bl.c  |  4 +++-
>>>  drivers/video/backlight/lp8788_bl.c  |  5 -
>>>  drivers/video/backlight/pwm_bl.c | 11 +--
>>>  drivers/video/fbdev/ssd1307fb.c  |  3 ++-
>>>  include/linux/pwm.h  |  6 --
>>>  16 files changed, 70 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/drivers/video/backlight/lm3630a_bl.c 
>>> b/drivers/video/backlight/lm3630a_bl.c
>>> index 2030a6b77a09..696fa25dafd2 100644
>>> --- a/drivers/video/backlight/lm3630a_bl.c
>>> +++ b/drivers/video/backlight/lm3630a_bl.c
>>> @@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip 
>>> *pchip, int br, int br_max)
>>>  {
>>> unsigned int period = pchip->pdata->pwm_period;
>>> unsigned int duty = br * period / br_max;
>>> +   struct pwm_caps caps = { };
>>>  
>>> -   pwm_config(pchip->pwmd, duty, period);
>>> +   pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, );
>>> +   pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));
>>
>> Well... I admit I've only really looked at the patches that impact 
>> backlight but dispersing this really odd looking bit twiddling 
>> throughout the kernel doesn't strike me a great API design.
>>
>> IMHO callers should not be required to find the first set bit in
>> some specially crafted set of capability bits simply to get sane 
>> default behaviour.
> 
> Agreed. IMHO the regular use case becomes rather tedious, ugly, and
> error prone.

Using simply PWM_MODE(NORMAL) instead of BIT(ffs(caps.modes) - 1) would be OK
from your side?

Or, what about using a function like pwm_mode_first() to get the first supported
mode by PWM channel?

Or, would you prefer to solve this inside pwm_config() function, let's say, in
case an invalid mode is passed as argument, to let pwm_config() to choose the
first available PWM mode for PWM channel passed as argument?

Thank you,
Claudiu Beznea

> 
> BR,
> Jani.
> 
> 

[PATCH 2/2] media: ov5695: mark PM functions as __maybe_unused

[Arnd Bergmann]

Without CONFIG_PM, we get a harmless build warning:

drivers/media/i2c/ov2685.c:516:12: error: 'ov2685_runtime_suspend' defined but 
not used [-Werror=unused-function]
 static int ov2685_runtime_suspend(struct device *dev)
^~
drivers/media/i2c/ov2685.c:507:12: error: 'ov2685_runtime_resume' defined but 
not used [-Werror=unused-function]
 static int ov2685_runtime_resume(struct device *dev)

This marks the affected functions as __maybe_unused.

Fixes: e3861d9118c8 ("media: ov2685: add support for OV2685 sensor")
Signed-off-by: Arnd Bergmann 
---
 drivers/media/i2c/ov2685.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ov2685.c b/drivers/media/i2c/ov2685.c
index 904ac305d499..9ac702e3ae95 100644
--- a/drivers/media/i2c/ov2685.c
+++ b/drivers/media/i2c/ov2685.c
@@ -504,7 +504,7 @@ static int ov2685_open(struct v4l2_subdev *sd, struct 
v4l2_subdev_fh *fh)
 }
 #endif
 
-static int ov2685_runtime_resume(struct device *dev)
+static int __maybe_unused ov2685_runtime_resume(struct device *dev)
 {
struct i2c_client *client = to_i2c_client(dev);
struct v4l2_subdev *sd = i2c_get_clientdata(client);
@@ -513,7 +513,7 @@ static int ov2685_runtime_resume(struct device *dev)
return __ov2685_power_on(ov2685);
 }
 
-static int ov2685_runtime_suspend(struct device *dev)
+static int __maybe_unused ov2685_runtime_suspend(struct device *dev)
 {
struct i2c_client *client = to_i2c_client(dev);
struct v4l2_subdev *sd = i2c_get_clientdata(client);
-- 
2.9.0

[PATCH 1/2] media: ov5695: mark PM functions as __maybe_unused

[Arnd Bergmann]

Without CONFIG_PM, we get a harmless build warning:

drivers/media/i2c/ov5695.c:1033:12: error: 'ov5695_runtime_suspend' defined but 
not used [-Werror=unused-function]
 static int ov5695_runtime_suspend(struct device *dev)
^~
drivers/media/i2c/ov5695.c:1024:12: error: 'ov5695_runtime_resume' defined but 
not used [-Werror=unused-function]
 static int ov5695_runtime_resume(struct device *dev)

This marks the affected functions as __maybe_unused.

Fixes: 8a77009be4be ("media: ov5695: add support for OV5695 sensor")
Signed-off-by: Arnd Bergmann 
---
 drivers/media/i2c/ov5695.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ov5695.c b/drivers/media/i2c/ov5695.c
index 2db7d2e535b9..a4985a4715f5 100644
--- a/drivers/media/i2c/ov5695.c
+++ b/drivers/media/i2c/ov5695.c
@@ -1021,7 +1021,7 @@ static void __ov5695_power_off(struct ov5695 *ov5695)
regulator_bulk_disable(OV5695_NUM_SUPPLIES, ov5695->supplies);
 }
 
-static int ov5695_runtime_resume(struct device *dev)
+static int __maybe_unused ov5695_runtime_resume(struct device *dev)
 {
struct i2c_client *client = to_i2c_client(dev);
struct v4l2_subdev *sd = i2c_get_clientdata(client);
@@ -1030,7 +1030,7 @@ static int ov5695_runtime_resume(struct device *dev)
return __ov5695_power_on(ov5695);
 }
 
-static int ov5695_runtime_suspend(struct device *dev)
+static int __maybe_unused ov5695_runtime_suspend(struct device *dev)
 {
struct i2c_client *client = to_i2c_client(dev);
struct v4l2_subdev *sd = i2c_get_clientdata(client);
-- 
2.9.0

Re: [PATCH] media: stm32-dcmi: add JPEG support

[Hans Verkuil]

On 02/22/2018 10:51 AM, Hugues Fruchet wrote:
> Add DCMI JPEG support.

Does this patch depend on the 'fix lock scheme' patch?

It looks good to me except for one small thing (see below), but I think this
depends on the 'fix lock scheme' patch, right?

> 
> Signed-off-by: Hugues Fruchet 
> ---
>  drivers/media/platform/stm32/stm32-dcmi.c | 195 
> +++---
>  1 file changed, 148 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
> b/drivers/media/platform/stm32/stm32-dcmi.c
> index 269e963..7eaaf7c 100644
> --- a/drivers/media/platform/stm32/stm32-dcmi.c
> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
> @@ -93,6 +93,11 @@ enum state {
>  #define MIN_HEIGHT   16U
>  #define MAX_HEIGHT   2048U
>  
> +#define MIN_JPEG_WIDTH   16U
> +#define MAX_JPEG_WIDTH   2592U
> +#define MIN_JPEG_HEIGHT  16U
> +#define MAX_JPEG_HEIGHT  2592U
> +
>  #define TIMEOUT_MS   1000
>  
>  struct dcmi_graph_entity {
> @@ -191,14 +196,67 @@ static inline void reg_clear(void __iomem *base, u32 
> reg, u32 mask)
>  
>  static int dcmi_start_capture(struct stm32_dcmi *dcmi);
>  
> +static void dcmi_buffer_done(struct stm32_dcmi *dcmi,
> +  struct dcmi_buf *buf,
> +  size_t bytesused,
> +  int err)
> +{
> + struct vb2_v4l2_buffer *vbuf;
> +
> + if (!buf)
> + return;
> +
> + vbuf = >vb;
> +
> + vbuf->sequence = dcmi->sequence++;
> + vbuf->field = V4L2_FIELD_NONE;
> + vbuf->vb2_buf.timestamp = ktime_get_ns();
> + vb2_set_plane_payload(>vb2_buf, 0, bytesused);
> + vb2_buffer_done(>vb2_buf,
> + err ? VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
> + dev_dbg(dcmi->dev, "buffer[%d] done seq=%d, bytesused=%zu\n",
> + vbuf->vb2_buf.index, vbuf->sequence, bytesused);
> +
> + dcmi->buffers_count++;
> + dcmi->active = NULL;
> +}
> +
> +static int dcmi_restart_capture(struct stm32_dcmi *dcmi)
> +{
> + spin_lock_irq(>irqlock);
> +
> + if (dcmi->state != RUNNING) {
> + spin_unlock_irq(>irqlock);
> + return -EINVAL;
> + }
> +
> + /* Restart a new DMA transfer with next buffer */
> + if (list_empty(>buffers)) {
> + dev_err(dcmi->dev, "%s: No more buffer queued, cannot capture 
> buffer\n",
> + __func__);
> + dcmi->errors_count++;
> + dcmi->active = NULL;
> +
> + spin_unlock_irq(>irqlock);
> + return -EINVAL;
> + }
> +
> + dcmi->active = list_entry(dcmi->buffers.next,
> +   struct dcmi_buf, list);
> + list_del_init(>active->list);
> +
> + spin_unlock_irq(>irqlock);
> +
> + return dcmi_start_capture(dcmi);
> +}
> +
>  static void dcmi_dma_callback(void *param)
>  {
>   struct stm32_dcmi *dcmi = (struct stm32_dcmi *)param;
>   struct dma_chan *chan = dcmi->dma_chan;
>   struct dma_tx_state state;
>   enum dma_status status;
> -
> - spin_lock_irq(>irqlock);
> + struct dcmi_buf *buf = dcmi->active;
>  
>   /* Check DMA status */
>   status = dmaengine_tx_status(chan, dcmi->dma_cookie, );
> @@ -216,53 +274,18 @@ static void dcmi_dma_callback(void *param)
>   case DMA_COMPLETE:
>   dev_dbg(dcmi->dev, "%s: Received DMA_COMPLETE\n", __func__);
>  
> - if (dcmi->active) {
> - struct dcmi_buf *buf = dcmi->active;
> - struct vb2_v4l2_buffer *vbuf = >active->vb;
> -
> - vbuf->sequence = dcmi->sequence++;
> - vbuf->field = V4L2_FIELD_NONE;
> - vbuf->vb2_buf.timestamp = ktime_get_ns();
> - vb2_set_plane_payload(>vb2_buf, 0, buf->size);
> - vb2_buffer_done(>vb2_buf, VB2_BUF_STATE_DONE);
> - dev_dbg(dcmi->dev, "buffer[%d] done seq=%d\n",
> - vbuf->vb2_buf.index, vbuf->sequence);
> -
> - dcmi->buffers_count++;
> - dcmi->active = NULL;
> - }
> -
> - /* Restart a new DMA transfer with next buffer */
> - if (dcmi->state == RUNNING) {
> - if (list_empty(>buffers)) {
> - dev_err(dcmi->dev, "%s: No more buffer queued, 
> cannot capture buffer\n",
> - __func__);
> - dcmi->errors_count++;
> - dcmi->active = NULL;
> -
> - spin_unlock_irq(>irqlock);
> - return;
> - }
> -
> - dcmi->active = list_entry(dcmi->buffers.next,
> -   struct dcmi_buf, list);
> -
> - list_del_init(>active->list);
> -
> - spin_unlock_irq(>irqlock);
> 

Re: [PATCH 1/2] usbtv: Use same decoder sequence as Windows driver

[Hans Verkuil]

Hi Hugo,

Thanks for this patch, but I am a bit hesitant to apply it. Did you test
that PAL and NTSC still work after this change?

Unless you've tested it then I'm inclined to just apply the second patch that
adds the SECAM sequence.

Regards,

Hans

On 02/24/2018 07:24 PM, Hugo Grostabussiat wrote:
> Re-format the register {address, value} pairs so they follow the same
> order as the decoder configuration sequences in the Windows driver's .INF
> file.
> 
> For instance, for PAL, the "AVPAL" sequence in the .INF file is:
> 0x04,0x68,0xD3,0x72,0xA2,0xB0,0x15,0x01,0x2C,0x10,0x20,0x2e,0x08,0x02,
> 0x02,0x59,0x16,0x35,0x17,0x16,0x36
> 
> Signed-off-by: Hugo Grostabussiat 
> ---
>  drivers/media/usb/usbtv/usbtv-video.c | 26 +-
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/usb/usbtv/usbtv-video.c 
> b/drivers/media/usb/usbtv/usbtv-video.c
> index 3668a04359e8..52d06b30fabb 100644
> --- a/drivers/media/usb/usbtv/usbtv-video.c
> +++ b/drivers/media/usb/usbtv/usbtv-video.c
> @@ -124,15 +124,26 @@ static int usbtv_select_input(struct usbtv *usbtv, int 
> input)
>  static int usbtv_select_norm(struct usbtv *usbtv, v4l2_std_id norm)
>  {
>   int ret;
> + /* These are the series of register values used to configure the
> +  * decoder for a specific standard.
> +  * They are copied from the Settings\DecoderDefaults registry keys
> +  * present in the Windows driver .INF file for each norm.
> +  */
>   static const u16 pal[][2] = {
> + { USBTV_BASE + 0x0003, 0x0004 },
>   { USBTV_BASE + 0x001a, 0x0068 },
> + { USBTV_BASE + 0x0100, 0x00d3 },
>   { USBTV_BASE + 0x010e, 0x0072 },
>   { USBTV_BASE + 0x010f, 0x00a2 },
>   { USBTV_BASE + 0x0112, 0x00b0 },
> + { USBTV_BASE + 0x0115, 0x0015 },
>   { USBTV_BASE + 0x0117, 0x0001 },
>   { USBTV_BASE + 0x0118, 0x002c },
>   { USBTV_BASE + 0x012d, 0x0010 },
>   { USBTV_BASE + 0x012f, 0x0020 },
> + { USBTV_BASE + 0x0220, 0x002e },
> + { USBTV_BASE + 0x0225, 0x0008 },
> + { USBTV_BASE + 0x024e, 0x0002 },
>   { USBTV_BASE + 0x024f, 0x0002 },
>   { USBTV_BASE + 0x0254, 0x0059 },
>   { USBTV_BASE + 0x025a, 0x0016 },
> @@ -143,14 +154,20 @@ static int usbtv_select_norm(struct usbtv *usbtv, 
> v4l2_std_id norm)
>   };
>  
>   static const u16 ntsc[][2] = {
> + { USBTV_BASE + 0x0003, 0x0004 },
>   { USBTV_BASE + 0x001a, 0x0079 },
> + { USBTV_BASE + 0x0100, 0x00d3 },
>   { USBTV_BASE + 0x010e, 0x0068 },
>   { USBTV_BASE + 0x010f, 0x009c },
>   { USBTV_BASE + 0x0112, 0x00f0 },
> + { USBTV_BASE + 0x0115, 0x0015 },
>   { USBTV_BASE + 0x0117, 0x },
>   { USBTV_BASE + 0x0118, 0x00fc },
>   { USBTV_BASE + 0x012d, 0x0004 },
>   { USBTV_BASE + 0x012f, 0x0008 },
> + { USBTV_BASE + 0x0220, 0x002e },
> + { USBTV_BASE + 0x0225, 0x0008 },
> + { USBTV_BASE + 0x024e, 0x0002 },
>   { USBTV_BASE + 0x024f, 0x0001 },
>   { USBTV_BASE + 0x0254, 0x005f },
>   { USBTV_BASE + 0x025a, 0x0012 },
> @@ -236,15 +253,6 @@ static int usbtv_setup_capture(struct usbtv *usbtv)
>   { USBTV_BASE + 0x0158, 0x001f },
>   { USBTV_BASE + 0x0159, 0x0006 },
>   { USBTV_BASE + 0x015d, 0x },
> -
> - { USBTV_BASE + 0x0003, 0x0004 },
> - { USBTV_BASE + 0x0100, 0x00d3 },
> - { USBTV_BASE + 0x0115, 0x0015 },
> - { USBTV_BASE + 0x0220, 0x002e },
> - { USBTV_BASE + 0x0225, 0x0008 },
> - { USBTV_BASE + 0x024e, 0x0002 },
> - { USBTV_BASE + 0x024e, 0x0002 },
> - { USBTV_BASE + 0x024f, 0x0002 },
>   };
>  
>   ret = usbtv_set_regs(usbtv, setup, ARRAY_SIZE(setup));
> 

Re: [PATCH v2] media: stm32-dcmi: fix lock scheme

[Hans Verkuil]

On 02/22/2018 10:49 AM, Hugues Fruchet wrote:
> Fix lock scheme leading to spurious freeze.

Can you elaborate a bit more? It's hard to review since you don't
describe what was wrong and why this fixes the problem.

Regards,

Hans

> 
> Signed-off-by: Hugues Fruchet 
> ---
> version 2:
>   - dcmi_buf_queue() refactor to avoid to have "else" after "return"
> (warning detected by checkpatch.pl --strict -f stm32-dcmi.c)
> 
>  drivers/media/platform/stm32/stm32-dcmi.c | 57 
> +--
>  1 file changed, 24 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
> b/drivers/media/platform/stm32/stm32-dcmi.c
> index 2fd8bed..5de18ad 100644
> --- a/drivers/media/platform/stm32/stm32-dcmi.c
> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
> @@ -197,7 +197,7 @@ static void dcmi_dma_callback(void *param)
>   struct dma_tx_state state;
>   enum dma_status status;
>  
> - spin_lock(>irqlock);
> + spin_lock_irq(>irqlock);
>  
>   /* Check DMA status */
>   status = dmaengine_tx_status(chan, dcmi->dma_cookie, );
> @@ -239,7 +239,7 @@ static void dcmi_dma_callback(void *param)
>   dcmi->errors_count++;
>   dcmi->active = NULL;
>  
> - spin_unlock(>irqlock);
> + spin_unlock_irq(>irqlock);
>   return;
>   }
>  
> @@ -248,13 +248,11 @@ static void dcmi_dma_callback(void *param)
>  
>   list_del_init(>active->list);
>  
> - if (dcmi_start_capture(dcmi)) {
> + spin_unlock_irq(>irqlock);
> + if (dcmi_start_capture(dcmi))
>   dev_err(dcmi->dev, "%s: Cannot restart capture 
> on DMA complete\n",
>   __func__);
> -
> - spin_unlock(>irqlock);
> - return;
> - }
> + return;
>   }
>  
>   break;
> @@ -263,7 +261,7 @@ static void dcmi_dma_callback(void *param)
>   break;
>   }
>  
> - spin_unlock(>irqlock);
> + spin_unlock_irq(>irqlock);
>  }
>  
>  static int dcmi_start_dma(struct stm32_dcmi *dcmi,
> @@ -360,7 +358,7 @@ static irqreturn_t dcmi_irq_thread(int irq, void *arg)
>  {
>   struct stm32_dcmi *dcmi = arg;
>  
> - spin_lock(>irqlock);
> + spin_lock_irq(>irqlock);
>  
>   /* Stop capture is required */
>   if (dcmi->state == STOPPING) {
> @@ -370,7 +368,7 @@ static irqreturn_t dcmi_irq_thread(int irq, void *arg)
>  
>   complete(>complete);
>  
> - spin_unlock(>irqlock);
> + spin_unlock_irq(>irqlock);
>   return IRQ_HANDLED;
>   }
>  
> @@ -383,35 +381,34 @@ static irqreturn_t dcmi_irq_thread(int irq, void *arg)
>__func__);
>  
>   dcmi->errors_count++;
> - dmaengine_terminate_all(dcmi->dma_chan);
> -
>   dev_dbg(dcmi->dev, "Restarting capture after DCMI error\n");
>  
> - if (dcmi_start_capture(dcmi)) {
> + spin_unlock_irq(>irqlock);
> + dmaengine_terminate_all(dcmi->dma_chan);
> +
> + if (dcmi_start_capture(dcmi))
>   dev_err(dcmi->dev, "%s: Cannot restart capture on 
> overflow or error\n",
>   __func__);
> -
> - spin_unlock(>irqlock);
> - return IRQ_HANDLED;
> - }
> + return IRQ_HANDLED;
>   }
>  
> - spin_unlock(>irqlock);
> + spin_unlock_irq(>irqlock);
>   return IRQ_HANDLED;
>  }
>  
>  static irqreturn_t dcmi_irq_callback(int irq, void *arg)
>  {
>   struct stm32_dcmi *dcmi = arg;
> + unsigned long flags;
>  
> - spin_lock(>irqlock);
> + spin_lock_irqsave(>irqlock, flags);
>  
>   dcmi->misr = reg_read(dcmi->regs, DCMI_MIS);
>  
>   /* Clear interrupt */
>   reg_set(dcmi->regs, DCMI_ICR, IT_FRAME | IT_OVR | IT_ERR);
>  
> - spin_unlock(>irqlock);
> + spin_unlock_irqrestore(>irqlock, flags);
>  
>   return IRQ_WAKE_THREAD;
>  }
> @@ -490,9 +487,8 @@ static void dcmi_buf_queue(struct vb2_buffer *vb)
>   struct stm32_dcmi *dcmi =  vb2_get_drv_priv(vb->vb2_queue);
>   struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
>   struct dcmi_buf *buf = container_of(vbuf, struct dcmi_buf, vb);
> - unsigned long flags = 0;
>  
> - spin_lock_irqsave(>irqlock, flags);
> + spin_lock_irq(>irqlock);
>  
>   if ((dcmi->state == RUNNING) && (!dcmi->active)) {
>   dcmi->active = buf;
> @@ -500,19 +496,15 @@ static void dcmi_buf_queue(struct vb2_buffer *vb)
>   dev_dbg(dcmi->dev, "Starting capture on buffer[%d] queued\n",
>   buf->vb.vb2_buf.index);
>  
> - 

Re: [PATCH v2] Staging: bcm2048: Fix function argument alignment in radio-bcm2048.c.

[Hans Verkuil]

On 02/20/2018 07:53 AM, Quytelda Kahja wrote:
> Fix a coding style problem.

What coding style problem? You should give a short description of
what you are fixing.

> 
> Signed-off-by: Quytelda Kahja 
> ---
> This is the patch without the unnecessary fixes for line length.
> 
>  drivers/staging/media/bcm2048/radio-bcm2048.c | 22 +++---
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
> b/drivers/staging/media/bcm2048/radio-bcm2048.c
> index 06d1920150da..f38a4f2acdde 100644
> --- a/drivers/staging/media/bcm2048/radio-bcm2048.c
> +++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
> @@ -1864,7 +1864,7 @@ static int bcm2048_probe(struct bcm2048_device *bdev)
>   goto unlock;
>  
>   err = bcm2048_set_fm_search_rssi_threshold(bdev,
> - BCM2048_DEFAULT_RSSI_THRESHOLD);
> +
> BCM2048_DEFAULT_RSSI_THRESHOLD);
>   if (err < 0)
>   goto unlock;
>  

Just drop this change: it will replace one warning (non-aligned) with
another (> 80 cols).

This code is fine as it is.

Regards,

Hans

> @@ -1942,9 +1942,9 @@ static irqreturn_t bcm2048_handler(int irq, void *dev)
>   */
>  #define property_write(prop, type, mask, check)  
> \
>  static ssize_t bcm2048_##prop##_write(struct device *dev,\
> - struct device_attribute *attr,  \
> - const char *buf,\
> - size_t count)   \
> +   struct device_attribute *attr,\
> +   const char *buf,  \
> +   size_t count) \
>  {\
>   struct bcm2048_device *bdev = dev_get_drvdata(dev); \
>   type value; \
> @@ -1966,8 +1966,8 @@ static ssize_t bcm2048_##prop##_write(struct device 
> *dev,   \
>  
>  #define property_read(prop, mask)\
>  static ssize_t bcm2048_##prop##_read(struct device *dev, \
> - struct device_attribute *attr,  \
> - char *buf)  \
> +  struct device_attribute *attr, \
> +  char *buf) \
>  {\
>   struct bcm2048_device *bdev = dev_get_drvdata(dev); \
>   int value;  \
> @@ -1985,8 +1985,8 @@ static ssize_t bcm2048_##prop##_read(struct device 
> *dev,\
>  
>  #define property_signed_read(prop, size, mask)   
> \
>  static ssize_t bcm2048_##prop##_read(struct device *dev, \
> - struct device_attribute *attr,  \
> - char *buf)  \
> +  struct device_attribute *attr, \
> +  char *buf) \
>  {\
>   struct bcm2048_device *bdev = dev_get_drvdata(dev); \
>   size value; \
> @@ -2005,8 +2005,8 @@ property_read(prop, mask)   
> \
>  
>  #define property_str_read(prop, size)
> \
>  static ssize_t bcm2048_##prop##_read(struct device *dev, \
> - struct device_attribute *attr,  \
> - char *buf)  \
> +  struct device_attribute *attr, \
> +  char *buf) \
>  {\
>   struct bcm2048_device *bdev = dev_get_drvdata(dev); \
>   int count;  \
> @@ -2175,7 +2175,7 @@ static int bcm2048_fops_release(struct file *file)
>  }
>  
>  static __poll_t bcm2048_fops_poll(struct file *file,
> -   struct poll_table_struct *pts)
> +   struct poll_table_struct *pts)
>  {
>   struct bcm2048_device *bdev = video_drvdata(file);
>   __poll_t retval = 0;
> 

[PATCH 2/3] media: tvp541x: fix some kernel-doc parameters

[Mauro Carvalho Chehab]

Solve the following warnings:
  + drivers/media/i2c/tvp514x.c: warning: Excess function parameter 'a' 
description in 'tvp514x_g_frame_interval':  => 759
  + drivers/media/i2c/tvp514x.c: warning: Excess function parameter 'a' 
description in 'tvp514x_s_frame_interval':  => 784
  + drivers/media/i2c/tvp514x.c: warning: Function parameter or member 'ival' 
not described in 'tvp514x_g_frame_interval':  => 759
  + drivers/media/i2c/tvp514x.c: warning: Function parameter or member 'ival' 
not described in 'tvp514x_s_frame_interval':  => 784

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/i2c/tvp514x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c
index 310f9fce520d..6a9890531d01 100644
--- a/drivers/media/i2c/tvp514x.c
+++ b/drivers/media/i2c/tvp514x.c
@@ -749,7 +749,7 @@ static int tvp514x_s_ctrl(struct v4l2_ctrl *ctrl)
 /**
  * tvp514x_g_frame_interval() - V4L2 decoder interface handler
  * @sd: pointer to standard V4L2 sub-device structure
- * @a: pointer to a v4l2_subdev_frame_interval structure
+ * @ival: pointer to a v4l2_subdev_frame_interval structure
  *
  * Returns the decoder's video CAPTURE parameters.
  */
@@ -773,7 +773,7 @@ tvp514x_g_frame_interval(struct v4l2_subdev *sd,
 /**
  * tvp514x_s_frame_interval() - V4L2 decoder interface handler
  * @sd: pointer to standard V4L2 sub-device structure
- * @a: pointer to a v4l2_subdev_frame_interval structure
+ * @ival: pointer to a v4l2_subdev_frame_interval structure
  *
  * Configures the decoder to use the input parameters, if possible. If
  * not possible, returns the appropriate error code.
-- 
2.14.3

[PATCH 3/3] media: imx: Don't initialize vars that won't be used

[Mauro Carvalho Chehab]

As reported by gcc:

  + drivers/staging/media/imx/imx-media-csi.c: warning: variable 'input_fi' set 
but not used [-Wunused-but-set-variable]:  => 671:33
  + drivers/staging/media/imx/imx-media-csi.c: warning: variable 'pinctrl' set 
but not used [-Wunused-but-set-variable]:  => 1742:18

input_fi is not used, so just remove it.

However, pinctrl should be used, as it devm_pinctrl_get_select_default()
is declared with attribute warn_unused_result. What's missing there
is an error handling code, in case it fails. Add it.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/staging/media/imx/imx-media-csi.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index eb7be5093a9d..49b57466e88d 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -668,11 +668,10 @@ static int csi_setup(struct csi_priv *priv)
 
 static int csi_start(struct csi_priv *priv)
 {
-   struct v4l2_fract *output_fi, *input_fi;
+   struct v4l2_fract *output_fi;
int ret;
 
output_fi = >frame_interval[priv->active_output_pad];
-   input_fi = >frame_interval[CSI_SINK_PAD];
 
if (priv->dest == IPU_CSI_DEST_IDMAC) {
ret = csi_idmac_start(priv);
@@ -1797,6 +1796,10 @@ static int imx_csi_probe(struct platform_device *pdev)
 */
priv->dev->of_node = pdata->of_node;
pinctrl = devm_pinctrl_get_select_default(priv->dev);
+   if (IS_ERR(pinctrl)) {
+   ret = PTR_ERR(priv->vdev);
+   goto free;
+   }
 
ret = v4l2_async_register_subdev(>sd);
if (ret)
-- 
2.14.3

[PATCH 1/3] media: ov7740: remove an unused var

[Mauro Carvalho Chehab]

Fix this warning regression:
   drivers/media/i2c/ov7740.c: warning: variable 'ret' set but not used 
[-Wunused-but-set-variable]:  => 276:6

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/i2c/ov7740.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov7740.c b/drivers/media/i2c/ov7740.c
index e1a44a18d9a8..01f578785e79 100644
--- a/drivers/media/i2c/ov7740.c
+++ b/drivers/media/i2c/ov7740.c
@@ -279,7 +279,7 @@ static int ov7740_get_register(struct v4l2_subdev *sd,
reg->val = val;
reg->size = 1;
 
-   return 0;
+   return ret;
 }
 
 static int ov7740_set_register(struct v4l2_subdev *sd,
-- 
2.14.3

Re: [PATCH v3] media: radio: Critical interrupt bugfix for si470x over i2c

[Hans Verkuil]

On 02/26/2018 03:27 AM, Douglas Fischer wrote:
> Fixed si470x_start() disabling the interrupt signal, causing tune
> operations to never complete. This does not affect USB radios
> because they poll the registers instead of using the IRQ line.
> 
> Stylistic and comment changes from v2.
> 
> Signed-off-by: Douglas Fischer 
> ---
> 
> diff -uprN linux.orig/drivers/media/radio/si470x/radio-si470x-common.c 
> linux/drivers/media/radio/si470x/radio-si470x-common.c
> --- linux.orig/drivers/media/radio/si470x/radio-si470x-common.c   
> 2018-01-15 21:58:10.675620432 -0500
> +++ linux/drivers/media/radio/si470x/radio-si470x-common.c2018-02-25 
> 19:16:31.785934211 -0500
> @@ -377,8 +377,11 @@ int si470x_start(struct si470x_device *r
>   goto done;
>  
>   /* sysconfig 1 */
> - radio->registers[SYSCONFIG1] =
> - (de << 11) & SYSCONFIG1_DE; /* DE*/
> + radio->registers[SYSCONFIG1] |= 
> SYSCONFIG1_RDSIEN|SYSCONFIG1_STCIEN|SYSCONFIG1_RDS;
> + radio->registers[SYSCONFIG1] &= ~SYSCONFIG1_GPIO2;
> + radio->registers[SYSCONFIG1] |= (0x01 << 2); /* GPIO2 */

Yes, but what does this do? Enable GPIO2? The header defines two bits for
GPIO1/2/3, but it doesn't say what those bits mean. So the question here is
what it means to set bit 2 to 1 and bit 3 to 0? The header doesn't give any
information about that, nor does this comment.

Regards,

Hans

> + if (de)
> + radio->registers[SYSCONFIG1] |= SYSCONFIG1_DE;
>   retval = si470x_set_register(radio, SYSCONFIG1);
>   if (retval < 0)
>   goto done;
> 

Re: [PATCH v3] media: video-i2c: add video-i2c driver

[Hans Verkuil]

On 02/24/2018 12:05 PM, Matt Ranostay wrote:
> There are several thermal sensors that only have a low-speed bus
> interface but output valid video data. This patchset enables support
> for the AMG88xx "Grid-Eye" sensor family.
> 
> Cc: Luca Barbato 
> Cc: Laurent Pinchart 
> Signed-off-by: Matt Ranostay 
> ---
> Changes from v1:
> * Switch to SPDX tags versus GPLv2 license text
> * Remove unneeded zeroing of data structures
> * Add video_i2c_try_fmt_vid_cap call in video_i2c_s_fmt_vid_cap function
> 
> Changes from v2:
> * Add missing linux/kthread.h include that broke x86_64 build
> 
> drivers/media/i2c/Kconfig |   9 +
>  drivers/media/i2c/Makefile|   1 +
>  drivers/media/i2c/video-i2c.c | 547 
> ++
>  3 files changed, 557 insertions(+)
>  create mode 100644 drivers/media/i2c/video-i2c.c
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 8fdd673d449f..53aede720e0f 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -917,6 +917,15 @@ config VIDEO_M52790
>  
>To compile this driver as a module, choose M here: the
>module will be called m52790.
> +
> +config VIDEO_I2C
> + tristate "I2C transport video support"
> + depends on VIDEO_V4L2 && I2C
> + select VIDEOBUF2_VMALLOC
> + ---help---
> +   Enable the I2C transport video support which supports the
> +   following:
> +* Panasonic AMG88xx Grid-Eye Sensors
>  endmenu
>  
>  menu "Sensors used on soc_camera driver"
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index 26b19a2e9d04..5d4c06cb3f6f 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -91,6 +91,7 @@ obj-$(CONFIG_VIDEO_LM3646)  += lm3646.o
>  obj-$(CONFIG_VIDEO_SMIAPP_PLL)   += smiapp-pll.o
>  obj-$(CONFIG_VIDEO_AK881X)   += ak881x.o
>  obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
> +obj-$(CONFIG_VIDEO_I2C)  += video-i2c.o
>  obj-$(CONFIG_VIDEO_ML86V7667)+= ml86v7667.o
>  obj-$(CONFIG_VIDEO_OV2659)   += ov2659.o
>  obj-$(CONFIG_VIDEO_TC358743) += tc358743.o
> diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c
> new file mode 100644
> index ..ea8ab2fcd580
> --- /dev/null
> +++ b/drivers/media/i2c/video-i2c.c
> @@ -0,0 +1,547 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * video-i2c.c - Support for I2C transport video devices
> + *
> + * Copyright (C) 2018 Matt Ranostay 
> + *
> + * Supported:
> + * - Panasonic AMG88xx Grid-Eye Sensors
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define VIDEO_I2C_DRIVER "video-i2c"
> +#define MAX_BUFFER_SIZE  128
> +
> +struct video_i2c_chip;
> +
> +struct video_i2c_buffer {
> + struct vb2_v4l2_buffer vb;
> + struct list_head list;
> +};
> +
> +struct video_i2c_data {
> + struct i2c_client *client;
> + const struct video_i2c_chip *chip;
> + struct mutex lock;
> + spinlock_t slock;
> + struct mutex queue_lock;
> +
> + struct v4l2_device v4l2_dev;
> + struct video_device vdev;
> + struct vb2_queue vb_vidq;
> +
> + struct task_struct *kthread_vid_cap;
> + struct list_head vid_cap_active;
> +};
> +
> +static struct v4l2_fmtdesc amg88xx_format = {
> + .pixelformat = V4L2_PIX_FMT_Y12,
> +};
> +
> +static struct v4l2_frmsize_discrete amg88xx_size = {
> + .width = 8,
> + .height = 8,
> +};
> +
> +struct video_i2c_chip {
> + /* video dimensions */
> + const struct v4l2_fmtdesc *format;
> + const struct v4l2_frmsize_discrete *size;
> +
> + /* max frames per second */
> + unsigned int max_fps;
> +
> + /* pixel buffer size */
> + unsigned int buffer_size;
> +
> + /* pixel size in bits */
> + unsigned int bpp;
> +
> + /* xfer function */
> + int (*xfer)(struct video_i2c_data *data, char *buf);
> +};
> +
> +static int amg88xx_xfer(struct video_i2c_data *data, char *buf)
> +{
> + struct i2c_client *client = data->client;
> + struct i2c_msg msg[2];
> + u8 reg = 0x80;
> + int ret;
> +
> + msg[0].addr = client->addr;
> + msg[0].flags = 0;
> + msg[0].len = 1;
> + msg[0].buf  = (char *) 
> +
> + msg[1].addr = client->addr;
> + msg[1].flags = I2C_M_RD;
> + msg[1].len = data->chip->buffer_size;
> + msg[1].buf = (char *) buf;
> +
> + ret = i2c_transfer(client->adapter, msg, 2);
> +
> + return (ret == 2) ? 0 : -EIO;
> +}
> +
> +static const struct video_i2c_chip video_i2c_chip = {
> + .size   = _size,
> + .format = _format,
> + .max_fps= 10,
> + .buffer_size= 128,
> + .bpp= 16,
> + 

Re: [PATCH 12/13] media: img-ir: Drop METAG dependency

[Sean Young]

On Wed, Feb 21, 2018 at 11:38:24PM +, James Hogan wrote:
> Now that arch/metag/ has been removed, remove the METAG dependency from
> the IMG IR device driver. The hardware is also present on MIPS SoCs so
> the driver still has value.
> 
> Signed-off-by: James Hogan 
> Cc: Mauro Carvalho Chehab 
> Cc: Mauro Carvalho Chehab 
> Cc: linux-media@vger.kernel.org
> Cc: linux-me...@vger.kernel.org

Acked-by: Sean Young 


> ---
>  drivers/media/rc/img-ir/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/rc/img-ir/Kconfig b/drivers/media/rc/img-ir/Kconfig
> index a896d3c83a1c..d2c6617d468e 100644
> --- a/drivers/media/rc/img-ir/Kconfig
> +++ b/drivers/media/rc/img-ir/Kconfig
> @@ -1,7 +1,7 @@
>  config IR_IMG
>   tristate "ImgTec IR Decoder"
>   depends on RC_CORE
> - depends on METAG || MIPS || COMPILE_TEST
> + depends on MIPS || COMPILE_TEST
>   select IR_IMG_HW if !IR_IMG_RAW
>   help
>  Say Y or M here if you want to use the ImgTec infrared decoder
> -- 
> 2.13.6

Re: [PATCH v3] media: radio: Critical v4l2 registration bugfix for si470x over i2c

[Hans Verkuil]

On 02/26/2018 03:25 AM, Douglas Fischer wrote:
> Added the call to v4l2_device_register() required to add a new radio
> device. Without this patch, it is impossible for the driver to load.
> This does not affect USB devices.
> 
> Fixed cleanup order from v2.
> 
> Signed-off-by: Douglas Fischer 
> ---
> 
> diff -uprN linux.orig/drivers/media/radio/si470x/radio-si470x-i2c.c
> linux/drivers/media/radio/si470x/radio-si470x-i2c.c ---
> linux.orig/drivers/media/radio/si470x/radio-si470x-i2c.c
> 2018-01-15 21:58:10.675620432 -0500 +++
> linux/drivers/media/radio/si470x/radio-si470x-i2c.c   2018-02-25
> 19:19:13.796927568 -0500 @@ -43,7 +43,6 @@ static const struct
> i2c_device_id si470x MODULE_DEVICE_TABLE(i2c, si470x_i2c_id); 

This patch is still corrupt (wrap around). The other two are fine though.

Can you repost this one?

Regards,

Hans

> -
>  /**
>   * Module Parameters
>   **/
> @@ -362,22 +361,43 @@ static int si470x_i2c_probe(struct i2c_c
>   mutex_init(>lock);
>   init_completion(>completion);
>  
> + retval = v4l2_device_register(>dev, >v4l2_dev);
> + if (retval < 0) {
> + dev_err(>dev, "couldn't register
> v4l2_device\n");
> + goto err_radio;
> + }
> +
> + v4l2_ctrl_handler_init(>hdl, 2);
> + v4l2_ctrl_new_std(>hdl, _ctrl_ops,
> + V4L2_CID_AUDIO_MUTE, 0, 1, 1, 1);
> + v4l2_ctrl_new_std(>hdl, _ctrl_ops,
> + V4L2_CID_AUDIO_VOLUME, 0, 15, 1, 15);
> + if (radio->hdl.error) {
> + retval = radio->hdl.error;
> + dev_err(>dev, "couldn't register control\n");
> + goto err_dev;
> + }
> +
>   /* video device initialization */
>   radio->videodev = si470x_viddev_template;
> + radio->videodev.ctrl_handler = >hdl;
> + radio->videodev.lock = >lock;
> + radio->videodev.v4l2_dev = >v4l2_dev;
> + radio->videodev.release = video_device_release_empty;
>   video_set_drvdata(>videodev, radio);
>  
>   /* power up : need 110ms */
>   radio->registers[POWERCFG] = POWERCFG_ENABLE;
>   if (si470x_set_register(radio, POWERCFG) < 0) {
>   retval = -EIO;
> - goto err_radio;
> + goto err_ctrl;
>   }
>   msleep(110);
>  
>   /* get device and chip versions */
>   if (si470x_get_all_registers(radio) < 0) {
>   retval = -EIO;
> - goto err_radio;
> + goto err_ctrl;
>   }
>   dev_info(>dev, "DeviceID=0x%4.4hx ChipID=0x%4.4hx\n",
>   radio->registers[DEVICEID],
> radio->registers[SI_CHIPID]); @@ -407,7 +427,7 @@ static int
> si470x_i2c_probe(struct i2c_c radio->buffer = kmalloc(radio->buf_size,
> GFP_KERNEL); if (!radio->buffer) {
>   retval = -EIO;
> - goto err_radio;
> + goto err_ctrl;
>   }
>  
>   /* rds buffer configuration */
> @@ -437,6 +457,10 @@ err_all:
>   free_irq(client->irq, radio);
>  err_rds:
>   kfree(radio->buffer);
> +err_ctrl:
> + v4l2_ctrl_handler_free(>hdl);
> +err_dev:
> + v4l2_device_unregister(>v4l2_dev);
>  err_radio:
>   kfree(radio);
>  err_initial:
> 

Re: [PATCH v7 2/2] media: V3s: Add support for Allwinner CSI.

[Hans Verkuil]

Hi all,

On 01/30/2018 03:48 AM, Yong wrote:
> Hi,
> 
> On Mon, 29 Jan 2018 13:49:14 -0800
> Randy Dunlap  wrote:
> 
>> On 01/29/2018 01:21 AM, Yong Deng wrote:
>>> Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2
>>> interface and CSI1 is used for parallel interface. This is not
>>> documented in datasheet but by test and guess.
>>>
>>> This patch implement a v4l2 framework driver for it.
>>>
>>> Currently, the driver only support the parallel interface. MIPI-CSI2,
>>> ISP's support are not included in this patch.
>>>
>>> Tested-by: Maxime Ripard 
>>> Signed-off-by: Yong Deng 
>>> ---
>>
>>
>> A previous version (I think v6) had a build error with the use of
>> PHYS_OFFSET, so Kconfig was modified to depend on ARM and ARCH_SUNXI
>> (one of which seems to be overkill).  As is here, the COMPILE_TEST piece is
>> meaningless for all arches except ARM.  If you care enough for COMPILE_TEST
>> (and I would), then you could make COMPILE_TEST useful on any arch by
>> removing the "depends on ARM" (the ARCH_SUNXI takes care of that) and by
>> having an alternate value for PHYS_OFFSET, like so:
>>
>> +#if defined(CONFIG_COMPILE_TEST) && !defined(PHYS_OFFSET)
>> +#define PHYS_OFFSET 0
>> +#endif
>>
>> With those 2 changes, the driver builds for me on x86_64.
> 
> I have considered this method.
> But it's so sick to put these code in dirver (for my own). I mean 
> this is meaningless for the driver itself and make people confused.
> 
> I grepped the driver/ code and I found many drivers writing Kconfig
> like this. For example:
> ARM && COMPILE_TEST
> MIPS && COMPILE_TEST
> PPC64 && COMPILE_TEST
> 
> BTW, for my own, I do not care about COMPILE_TEST.

There was a discussion about this in the v6 patch, but it petered out.

I want to merge this driver, but I would very much prefer that this
compiles with COMPILE_TEST. So unless someone has a better solution, then
adding 'hack' that defines PHYS_OFFSET to 0 for COMPILE_TEST would be required.

Otherwise this driver looks good, so it is just this issue blocking it.

Regards,

Hans

> 
>>
>>> diff --git a/drivers/media/platform/sunxi/sun6i-csi/Kconfig 
>>> b/drivers/media/platform/sunxi/sun6i-csi/Kconfig
>>> new file mode 100644
>>> index 000..f80c965
>>> --- /dev/null
>>> +++ b/drivers/media/platform/sunxi/sun6i-csi/Kconfig
>>> @@ -0,0 +1,10 @@
>>> +config VIDEO_SUN6I_CSI
>>> +   tristate "Allwinner V3s Camera Sensor Interface driver"
>>> +   depends on ARM
>>> +   depends on VIDEO_V4L2 && COMMON_CLK && VIDEO_V4L2_SUBDEV_API && HAS_DMA
>>> +   depends on ARCH_SUNXI || COMPILE_TEST
>>> +   select VIDEOBUF2_DMA_CONTIG
>>> +   select REGMAP_MMIO
>>> +   select V4L2_FWNODE
>>> +   ---help---
>>> +  Support for the Allwinner Camera Sensor Interface Controller on V3s.
>>
>> thanks,
>> -- 
>> ~Randy
> 
> 
> Thanks,
> Yong
> 

Re: [PATCH V2 2/2] dt-bindings: Document the Rockchip RK1608 bindings

[Linus Walleij]

On Mon, Feb 26, 2018 at 9:25 AM, Wen Nuan  wrote:

> From: Leo Wen 
>
> Add DT bindings documentation for Rockchip RK1608.
>
> Changes V2:
> - Delete spi-min-frequency property.
> - Add the external sensor's control pin and clock properties.
> - Delete the '' node.
>
> Signed-off-by: Leo Wen 

(...)
> +- reset-gpio   : GPIO connected to reset pin;
> +- irq-gpio : GPIO connected to irq pin;
> +- sleepst-gpio : GPIO connected to sleepst pin;
> +- wakeup-gpio  : GPIO connected to wakeup pin;
> +- powerdown-gpio   : GPIO connected to powerdown pin;

All these should be named something like:

reset-gpios = <>;
irq-gpios = <>;
etc

See
Documentation/devicetree/bindings/gpio/gpio.txt

So all in pluralis even if it is just one line, that is the standard.

> +- rockchip,powerdown0  : GPIO connected to the sensor0's powerdown pin;
> +- rockchip,reset0  : GPIO connected to the sensor0's reset pin;
> +- rockchip,powerdown1  : GPIO connected to the sensor1's powerdown pin;
> +- rockchip,reset1  : GPIO connected to the sensor1's reset pin;

Also get rid of the custom names here, either no lines should
have a "rockchip", prefix or all of them. Use the name of the
pin on the component, I suspect just

powerdown0-gpios
reset0-gpios
etc

By using the standard "*-gpios" suffix the kernel consumer API
will be much happier as well when you use gpiod_get() & friends.

Yours,
Linus Walleij

Re: [PATCH V2 1/2] [media] Add Rockchip RK1608 driver

[Linus Walleij]

On Mon, Feb 26, 2018 at 9:16 AM, Wen Nuan  wrote:

> +#include 

Please do not use this old API in new drivers.

Just use GPIO descriptors and only
#include 

There is documentation in
Documentation/gpio/consumer.txt

> +static int rk1608_pin_power(struct rk1608_state *pdata, int on)
> +{
> +   unsigned int grf_val = 0;
> +
> +   if (on) {
> +   clk_prepare_enable(pdata->pd_cif);
> +   clk_prepare_enable(pdata->aclk_cif);
> +   clk_prepare_enable(pdata->hclk_cif);
> +   clk_prepare_enable(pdata->cif_clk_in);
> +   clk_prepare_enable(pdata->cif_clk_out);
> +   clk_prepare_enable(pdata->clk_mipi_24m);
> +   clk_prepare_enable(pdata->hclk_mipiphy);
> +   clk_set_rate(pdata->cif_clk_out, RK1608_MCLK_RATE);
> +
> +   clk_prepare_enable(pdata->mipi_clk);
> +   clk_set_rate(pdata->mipi_clk, RK1608_MCLK_RATE);
> +
> +   gpio_direction_output(pdata->rst1, 0);

Instead of this old GPIO API, put GPIO descriptors
struct gpio_desc *rst1; etc in your state container and
use gpiod_direction_output() etc.

> +   pdata->grf_gpio2b_iomux = ioremap((resource_size_t)
> + (GRF_BASE_ADDR +
> +  GRF_GPIO2B_IOMUX), 4);
> +   grf_val = __raw_readl(pdata->grf_gpio2b_iomux);
> +   __raw_writel(((grf_val) | (1 << 6) | (1 << (6 + 16))),
> +pdata->grf_gpio2b_iomux);
> +
> +   pdata->grf_io_vsel = ioremap((resource_size_t)
> + (GRF_BASE_ADDR + GRF_IO_VSEL), 
> 4);
> +   grf_val = __raw_readl(pdata->grf_io_vsel);
> +   __raw_writel(((grf_val) | (1 << 1) | (1 << (1 + 16))),
> +pdata->grf_io_vsel);

You are doing pin control on the side of the pin control subsystem
it looks like?

I think David Wu and Heiko Stubner needs to have a look at what you
are doing here to suggest other solutions.

Apart from that, why use __raw_writel instead of just writel()?

> +static int rk1608_parse_dt_property(struct rk1608_state *pdata)
> +{
(...)
> +   enum of_gpio_flags flags;

Don't use this.

> +   ret = of_get_named_gpio_flags(node, "rockchip,reset1", 0, );

You should not use custom properties for GPIO names,
they need names like this:

gpios-reset1 = < 1 GPIO_ACTIVE_HIGH>

See
Documentation/devicetree/bindings/gpio/gpio.txt

> +   if (ret <= 0)
> +   dev_err(dev, "can not find reset1 error %d\n", ret);
> +   pdata->rst1 = ret;
> +
> +   ret = devm_gpio_request(dev, pdata->rst1, "rockchip-reset1");
> +   if (ret) {
> +   dev_err(dev, "gpio pdata->rst1 %d request error %d\n",
> +   pdata->rst1, ret);
> +   return ret;
> +   }
> +   gpio_set_value(pdata->rst1, 0);
> +   ret = gpio_direction_output(pdata->rst1, 0);
> +   if (ret) {
> +   dev_err(dev, "gpio %d direction output error %d\n",
> +   pdata->rst1, ret);
> +   return ret;
> +   }

As you see this become tedious and repetitive.

Instead use:

pdata->rst1 = gpiod_get(dev, "reset1", GPIOD_OUT_LOW);
if (IS_ERR(pdata->rst1))
return PTR_ERR(pdata->rst1);

After this point you know that you have a valid handle on the
GPIO line and it is initialized low.

> +struct rk1608_state {
(...)
> +   void __iomem*grf_gpio2b_iomux;

I suspect this should be done with pin control.

> +   void __iomem*grf_io_vsel;

And this should maybe use a regulator.

> +   int powerdown1;
> +   int rst1;
> +   int powerdown0;
> +   int rst0;
> +   int power_count;
> +   int reset_gpio;
> +   int reset_active;
> +   int irq_gpio;
> +   int irq;
> +   int sleepst_gpio;
> +   int sleepst_irq;
> +   int wakeup_gpio;
> +   int wakeup_active;
> +   int powerdown_gpio;
> +   int powerdown_active;

Anything that us a GPIO should be a struct gpio_desc *.

You do not need to keep track of if they are active I guess,
but I haven't looked close. They should probably be bool
rather than int if necessary.

But notice that we have
gpiod_get_optional() and if you use that you can
just check if the gpio_desc * is NULL like

if (!pdata->wakeup_gpio)
 gpiod_set_value() ...

Yours,
Linus Walleij

Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

[Jani Nikula]

On Thu, 22 Feb 2018, Daniel Thompson  wrote:
> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
>> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
>> were adapted to this change.
>> 
>> Signed-off-by: Claudiu Beznea 
>> ---
>>  arch/arm/mach-s3c24xx/mach-rx1950.c  | 11 +--
>>  drivers/bus/ts-nbus.c|  2 +-
>>  drivers/clk/clk-pwm.c|  3 ++-
>>  drivers/gpu/drm/i915/intel_panel.c   | 17 ++---
>>  drivers/hwmon/pwm-fan.c  |  2 +-
>>  drivers/input/misc/max77693-haptic.c |  2 +-
>>  drivers/input/misc/max8997_haptic.c  |  6 +-
>>  drivers/leds/leds-pwm.c  |  5 -
>>  drivers/media/rc/ir-rx51.c   |  5 -
>>  drivers/media/rc/pwm-ir-tx.c |  5 -
>>  drivers/video/backlight/lm3630a_bl.c |  4 +++-
>>  drivers/video/backlight/lp855x_bl.c  |  4 +++-
>>  drivers/video/backlight/lp8788_bl.c  |  5 -
>>  drivers/video/backlight/pwm_bl.c | 11 +--
>>  drivers/video/fbdev/ssd1307fb.c  |  3 ++-
>>  include/linux/pwm.h  |  6 --
>>  16 files changed, 70 insertions(+), 21 deletions(-)
>> 
>> diff --git a/drivers/video/backlight/lm3630a_bl.c 
>> b/drivers/video/backlight/lm3630a_bl.c
>> index 2030a6b77a09..696fa25dafd2 100644
>> --- a/drivers/video/backlight/lm3630a_bl.c
>> +++ b/drivers/video/backlight/lm3630a_bl.c
>> @@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip 
>> *pchip, int br, int br_max)
>>  {
>>  unsigned int period = pchip->pdata->pwm_period;
>>  unsigned int duty = br * period / br_max;
>> +struct pwm_caps caps = { };
>>  
>> -pwm_config(pchip->pwmd, duty, period);
>> +pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, );
>> +pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));
>
> Well... I admit I've only really looked at the patches that impact 
> backlight but dispersing this really odd looking bit twiddling 
> throughout the kernel doesn't strike me a great API design.
>
> IMHO callers should not be required to find the first set bit in
> some specially crafted set of capability bits simply to get sane 
> default behaviour.

Agreed. IMHO the regular use case becomes rather tedious, ugly, and
error prone.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH V2 1/2] [media] Add Rockchip RK1608 driver

[Hans Verkuil]

Hi Leo,

Thank you for this patch series!

Here is a quick review of this driver:

On 02/26/2018 09:16 AM, Wen Nuan wrote:
> From: Leo Wen 
> 
> Rk1608 is used as a PreISP to link on Soc, which mainly has two functions.
> One is to download the firmware of RK1608, and the other is to match the
> extra sensor such as camera and enable sensor by calling sensor's s_power.
> 
> use below v4l2-ctl command to capture frames.
> 
> v4l2-ctl --verbose -d /dev/video1 --stream-mmap=2
> --stream-to=/tmp/stream.out --stream-count=60 --stream-poll
> 
> use below command to playback the video on your PC.
> 
> mplayer ./stream.out -loop 0 -demuxer rawvideo -rawvideo
> w=640:h=480:size=$((640*480*3/2)):format=NV12
> 
> Changes V2:
> - Delete long legalese.
> - Add the new SPDX tags.
> - Dual cameras can be captured at the same time.
> - Add some pins and clock control for external sensors.
> - Add a function, the Soc can send control MSG to RK1608. 
> 
> Signed-off-by: Leo Wen 
> ---
>  MAINTAINERS|6 +
>  drivers/media/spi/Makefile |1 +
>  drivers/media/spi/rk1608.c | 1664 
> 
>  drivers/media/spi/rk1608.h |  471 +
>  4 files changed, 2142 insertions(+)
>  create mode 100644 drivers/media/spi/rk1608.c
>  create mode 100644 drivers/media/spi/rk1608.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 93a12af..b2a98e3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -136,6 +136,12 @@ Maintainers List (try to look for most precise areas 
> first)
>  
>   ---
>  
> +ROCKCHIP RK1608 DRIVER
> +M:   Leo Wen 
> +S:   Maintained
> +F:   drivers/media/spi/rk1608.c
> +F:   drivers/media/spi/rk1608.h
> +
>  3C59X NETWORK DRIVER
>  M:   Steffen Klassert 
>  L:   net...@vger.kernel.org
> diff --git a/drivers/media/spi/Makefile b/drivers/media/spi/Makefile
> index ea64013..9d9d9ec 100644
> --- a/drivers/media/spi/Makefile
> +++ b/drivers/media/spi/Makefile
> @@ -1 +1,2 @@
>  obj-$(CONFIG_VIDEO_GS1662) += gs1662.o
> +obj-$(CONFIG_ROCKCHIP_RK1608) += rk1608.o
> diff --git a/drivers/media/spi/rk1608.c b/drivers/media/spi/rk1608.c
> new file mode 100644
> index 000..c229b30
> --- /dev/null
> +++ b/drivers/media/spi/rk1608.c
> @@ -0,0 +1,1664 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/**
> + * Rockchip rk1608 driver
> + *
> + * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "rk1608.h"
> +
> +/**
> + * Rk1608 is used as the Pre-ISP to link on Soc, which mainly has two
> + * functions. One is to download the firmware of RK1608, and the other
> + * is to match the extra sensor such as camera and enable sensor by

"to match the extra sensor": I'm not sure what is meant by that. Can you
elaborate?

> + * calling sensor's s_power.
> + *   |---|
> + *   | Sensor Camera |
> + *   |---|
> + *   |---||--|
> + *   |---||--|
> + *   |---\/--|
> + *   | Pre-ISP RK1608|
> + *   |---|
> + *   |---||--|
> + *   |---||--|
> + *   |---\/--|
> + *   |  Rockchip Soc |
> + *   |---|
> + * Data Transfer As shown above. In RK1608, the data received from the
> + * extra sensor,and it is passed to the Soc through ISP.
> + */
> +
> +static inline struct rk1608_state *to_state(struct v4l2_subdev *sd)
> +{
> + return container_of(sd, struct rk1608_state, sd);
> +}
> +
> +/**
> + * rk1608_operation_query - RK1608 last operation state query
> + *
> + * @spi: device from which data will be read
> + * @state: last operation state [out]
> + * Context: can sleep
> + *
> + * It returns zero on success, else a negative error code.
> + */
> +int rk1608_operation_query(struct spi_device *spi, s32 *state)

Why isn't this static? Ditto for all the other non-static functions
below.

> +{
> + s32 query_cmd = RK1608_CMD_QUERY;
> + struct spi_transfer query_cmd_packet = {
> + .tx_buf = _cmd,
> + .len= sizeof(query_cmd),
> + };
> + struct spi_transfer state_packet = {
> + .rx_buf = state,
> + .len= sizeof(*state),
> + };
> + struct spi_message  m;
> +
> + spi_message_init();
> + spi_message_add_tail(_cmd_packet, );
> + spi_message_add_tail(_packet, );
> + spi_sync(spi, );
> +
> + return ((*state & RK1608_STATE_ID_MASK) == RK1608_STATE_ID) ? 0 : -1;
> +}
> +
> +int rk1608_write(struct spi_device *spi,
> +  s32 addr, const s32 *data, size_t data_len)
> +{
> + s32 write_cmd = RK1608_CMD_WRITE;
> + struct spi_transfer 

[PATCH 1/6] cec: add core error injection support

[Hans Verkuil]

From: Hans Verkuil 

Add two new ops (error_inj_show and error_inj_parse_line) to support
error injection functionality for CEC adapters. If both are present,
then the core will add a new error-inj debugfs file that can be used
to see the current error injection commands and to set error injection
commands.

Signed-off-by: Hans Verkuil 
---
 drivers/media/cec/cec-core.c | 58 
 include/media/cec.h  |  5 
 2 files changed, 63 insertions(+)

diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
index e47ea22b3c23..ea3eccfdba15 100644
--- a/drivers/media/cec/cec-core.c
+++ b/drivers/media/cec/cec-core.c
@@ -195,6 +195,55 @@ void cec_register_cec_notifier(struct cec_adapter *adap,
 EXPORT_SYMBOL_GPL(cec_register_cec_notifier);
 #endif
 
+#ifdef CONFIG_DEBUG_FS
+static ssize_t cec_error_inj_write(struct file *file,
+   const char __user *ubuf, size_t count, loff_t *ppos)
+{
+   struct seq_file *sf = file->private_data;
+   struct cec_adapter *adap = sf->private;
+   char *buf;
+   char *line;
+   char *p;
+
+   buf = memdup_user_nul(ubuf, min_t(size_t, PAGE_SIZE, count));
+   if (IS_ERR(buf))
+   return PTR_ERR(buf);
+   p = buf;
+   while (p && *p && count >= 0) {
+   p = skip_spaces(p);
+   line = strsep(, "\n");
+   if (!*line || *line == '#')
+   continue;
+   if (!adap->ops->error_inj_parse_line(adap, line)) {
+   count = -EINVAL;
+   break;
+   }
+   }
+   kfree(buf);
+   return count;
+}
+
+static int cec_error_inj_show(struct seq_file *sf, void *unused)
+{
+   struct cec_adapter *adap = sf->private;
+
+   return adap->ops->error_inj_show(adap, sf);
+}
+
+static int cec_error_inj_open(struct inode *inode, struct file *file)
+{
+   return single_open(file, cec_error_inj_show, inode->i_private);
+}
+
+static const struct file_operations cec_error_inj_fops = {
+   .open = cec_error_inj_open,
+   .write = cec_error_inj_write,
+   .read = seq_read,
+   .llseek = seq_lseek,
+   .release = single_release,
+};
+#endif
+
 struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
 void *priv, const char *name, u32 caps,
 u8 available_las)
@@ -334,7 +383,16 @@ int cec_register_adapter(struct cec_adapter *adap,
pr_warn("cec-%s: Failed to create status file\n", adap->name);
debugfs_remove_recursive(adap->cec_dir);
adap->cec_dir = NULL;
+   return 0;
}
+   if (!adap->ops->error_inj_show || !adap->ops->error_inj_parse_line)
+   return 0;
+   adap->error_inj_file = debugfs_create_file("error-inj", 0644,
+  adap->cec_dir, adap,
+  _error_inj_fops);
+   if (IS_ERR_OR_NULL(adap->error_inj_file))
+   pr_warn("cec-%s: Failed to create error-inj file\n",
+   adap->name);
 #endif
return 0;
 }
diff --git a/include/media/cec.h b/include/media/cec.h
index 9afba9b558df..41df048efc55 100644
--- a/include/media/cec.h
+++ b/include/media/cec.h
@@ -117,6 +117,10 @@ struct cec_adap_ops {
void (*adap_status)(struct cec_adapter *adap, struct seq_file *file);
void (*adap_free)(struct cec_adapter *adap);
 
+   /* Error injection callbacks */
+   int (*error_inj_show)(struct cec_adapter *adap, struct seq_file *sf);
+   bool (*error_inj_parse_line)(struct cec_adapter *adap, char *line);
+
/* High-level CEC message callback */
int (*received)(struct cec_adapter *adap, struct cec_msg *msg);
 };
@@ -189,6 +193,7 @@ struct cec_adapter {
 
struct dentry *cec_dir;
struct dentry *status_file;
+   struct dentry *error_inj_file;
 
u16 phys_addrs[15];
u32 sequence;
-- 
2.15.1

[PATCH 5/6] cec-pin: add error injection support

[Hans Verkuil]

From: Hans Verkuil 

Implement all the error injection commands.

The state machine gets new states for the various error situations,
helper functions are added to detect whether an error injection is
active and the actual error injections are implemented.

Signed-off-by: Hans Verkuil 
---
 drivers/media/cec/cec-pin-priv.h |  38 ++-
 drivers/media/cec/cec-pin.c  | 502 ++-
 2 files changed, 481 insertions(+), 59 deletions(-)

diff --git a/drivers/media/cec/cec-pin-priv.h b/drivers/media/cec/cec-pin-priv.h
index a33e7ef6cdf5..09ac2da35c20 100644
--- a/drivers/media/cec/cec-pin-priv.h
+++ b/drivers/media/cec/cec-pin-priv.h
@@ -28,14 +28,30 @@ enum cec_pin_state {
CEC_ST_TX_START_BIT_LOW,
/* Drive CEC high for the start bit */
CEC_ST_TX_START_BIT_HIGH,
+   /* Generate a start bit period that is too short */
+   CEC_ST_TX_START_BIT_HIGH_SHORT,
+   /* Generate a start bit period that is too long */
+   CEC_ST_TX_START_BIT_HIGH_LONG,
+   /* Drive CEC low for the start bit using the custom timing */
+   CEC_ST_TX_START_BIT_LOW_CUSTOM,
+   /* Drive CEC high for the start bit using the custom timing */
+   CEC_ST_TX_START_BIT_HIGH_CUSTOM,
/* Drive CEC low for the 0 bit */
CEC_ST_TX_DATA_BIT_0_LOW,
/* Drive CEC high for the 0 bit */
CEC_ST_TX_DATA_BIT_0_HIGH,
+   /* Generate a bit period that is too short */
+   CEC_ST_TX_DATA_BIT_0_HIGH_SHORT,
+   /* Generate a bit period that is too long */
+   CEC_ST_TX_DATA_BIT_0_HIGH_LONG,
/* Drive CEC low for the 1 bit */
CEC_ST_TX_DATA_BIT_1_LOW,
/* Drive CEC high for the 1 bit */
CEC_ST_TX_DATA_BIT_1_HIGH,
+   /* Generate a bit period that is too short */
+   CEC_ST_TX_DATA_BIT_1_HIGH_SHORT,
+   /* Generate a bit period that is too long */
+   CEC_ST_TX_DATA_BIT_1_HIGH_LONG,
/*
 * Wait for start of sample time to check for Ack bit or first
 * four initiator bits to check for Arbitration Lost.
@@ -43,6 +59,20 @@ enum cec_pin_state {
CEC_ST_TX_DATA_BIT_1_HIGH_PRE_SAMPLE,
/* Wait for end of bit period after sampling */
CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE,
+   /* Generate a bit period that is too short */
+   CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE_SHORT,
+   /* Generate a bit period that is too long */
+   CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE_LONG,
+   /* Drive CEC low for a data bit using the custom timing */
+   CEC_ST_TX_DATA_BIT_LOW_CUSTOM,
+   /* Drive CEC high for a data bit using the custom timing */
+   CEC_ST_TX_DATA_BIT_HIGH_CUSTOM,
+   /* Drive CEC low for a standalone pulse using the custom timing */
+   CEC_ST_TX_PULSE_LOW_CUSTOM,
+   /* Drive CEC high for a standalone pulse using the custom timing */
+   CEC_ST_TX_PULSE_HIGH_CUSTOM,
+   /* Start low drive */
+   CEC_ST_TX_LOW_DRIVE,
 
/* Rx states */
 
@@ -54,8 +84,8 @@ enum cec_pin_state {
CEC_ST_RX_DATA_SAMPLE,
/* Wait for earliest end of bit period after sampling */
CEC_ST_RX_DATA_POST_SAMPLE,
-   /* Wait for CEC to go high (i.e. end of bit period */
-   CEC_ST_RX_DATA_HIGH,
+   /* Wait for CEC to go low (i.e. end of bit period) */
+   CEC_ST_RX_DATA_WAIT_FOR_LOW,
/* Drive CEC low to send 0 Ack bit */
CEC_ST_RX_ACK_LOW,
/* End of 0 Ack time, wait for earliest end of bit period */
@@ -64,9 +94,9 @@ enum cec_pin_state {
CEC_ST_RX_ACK_HIGH_POST,
/* Wait for earliest end of bit period and end of message */
CEC_ST_RX_ACK_FINISH,
-
/* Start low drive */
-   CEC_ST_LOW_DRIVE,
+   CEC_ST_RX_LOW_DRIVE,
+
/* Monitor pin using interrupts */
CEC_ST_RX_IRQ,
 
diff --git a/drivers/media/cec/cec-pin.c b/drivers/media/cec/cec-pin.c
index 7920ea1c940b..5a553083d71a 100644
--- a/drivers/media/cec/cec-pin.c
+++ b/drivers/media/cec/cec-pin.c
@@ -39,11 +39,29 @@
 #define CEC_TIM_IDLE_SAMPLE1000
 /* when processing the start bit, sample twice per millisecond */
 #define CEC_TIM_START_BIT_SAMPLE   500
-/* when polling for a state change, sample once every 50 micoseconds */
+/* when polling for a state change, sample once every 50 microseconds */
 #define CEC_TIM_SAMPLE 50
 
 #define CEC_TIM_LOW_DRIVE_ERROR(1.5 * CEC_TIM_DATA_BIT_TOTAL)
 
+/*
+ * Total data bit time that is too short/long for a valid bit,
+ * used for error injection.
+ */
+#define CEC_TIM_DATA_BIT_TOTAL_SHORT   1800
+#define CEC_TIM_DATA_BIT_TOTAL_LONG2900
+
+/*
+ * Total start bit time that is too short/long for a valid bit,
+ * used for error injection.
+ */
+#define CEC_TIM_START_BIT_TOTAL_SHORT  4100
+#define CEC_TIM_START_BIT_TOTAL_LONG   5000
+
+/* Data bits are 0-7, EOM is bit 8 and ACK is bit 9 */
+#define EOM_BIT

[PATCH 2/6] cec-core.rst: document the error injection ops.

[Hans Verkuil]

From: Hans Verkuil 

Document the new core error injection callbacks.

Signed-off-by: Hans Verkuil 
---
 Documentation/media/kapi/cec-core.rst | 72 ++-
 1 file changed, 71 insertions(+), 1 deletion(-)

diff --git a/Documentation/media/kapi/cec-core.rst 
b/Documentation/media/kapi/cec-core.rst
index 62b9a1448177..a9f53f069a2d 100644
--- a/Documentation/media/kapi/cec-core.rst
+++ b/Documentation/media/kapi/cec-core.rst
@@ -110,11 +110,14 @@ your driver:
void (*adap_status)(struct cec_adapter *adap, struct seq_file 
*file);
void (*adap_free)(struct cec_adapter *adap);
 
+   /* Error injection callbacks */
+   ...
+
/* High-level callbacks */
...
};
 
-The five low-level ops deal with various aspects of controlling the CEC adapter
+The seven low-level ops deal with various aspects of controlling the CEC 
adapter
 hardware:
 
 
@@ -286,6 +289,70 @@ handling the receive interrupt. The framework expects to 
see the cec_transmit_do
 call before the cec_received_msg call, otherwise it can get confused if the
 received message was in reply to the transmitted message.
 
+Optional: Implementing Error Injection Support
+--
+
+If the CEC adapter supports Error Injection functionality, then that can
+be exposed through the Error Injection callbacks:
+
+.. code-block:: none
+
+   struct cec_adap_ops {
+   /* Low-level callbacks */
+   ...
+
+   /* Error injection callbacks */
+   int (*error_inj_show)(struct cec_adapter *adap, struct seq_file 
*sf);
+   bool (*error_inj_parse_line)(struct cec_adapter *adap, char 
*line);
+
+   /* High-level CEC message callback */
+   ...
+   };
+
+If both callbacks are set, then an ``error-inj`` file will appear in debugfs.
+The basic syntax is as follows:
+
+Leading spaces/tabs are ignored. If the next character is a ``#`` or the end 
of the
+line was reached, then the whole line is ignored. Otherwise a command is 
expected.
+
+This basic parsing is done in the CEC Framework. It is up to the driver to 
decide
+what commands to implement. The only requirement is that the command ``clear`` 
without
+any arguments must be implemented and that it will remove all current error 
injection
+commands.
+
+This ensures that you can always do ``echo clear >error-inj`` to clear any 
error
+injections without having to know the details of the driver-specific commands.
+
+Note that the output of ``error-inj`` shall be valid as input to ``error-inj``.
+So this must work:
+
+.. code-block:: none
+
+   $ cat error-inj >einj.txt
+   $ cat einj.txt >error-inj
+
+The first callback is called when this file is read and it should show the
+the current error injection state:
+
+.. c:function::
+   int (*error_inj_show)(struct cec_adapter *adap, struct seq_file *sf);
+
+It is recommended that it starts with a comment block with basic usage
+information. It returns 0 for success and an error otherwise.
+
+The second callback will parse commands written to the ``error-inj`` file:
+
+.. c:function::
+   bool (*error_inj_parse_line)(struct cec_adapter *adap, char *line);
+
+The ``line`` argument points to the start of the command. Any leading
+spaces or tabs have already been skipped. It is a single line only (so there
+are no embedded newlines) and it is 0-terminated. The callback is free to
+modify the contents of the buffer. It is only called for lines containing a
+command, so this callback is never called for empty lines or comment lines.
+
+Return true if the command was valid or false if there were syntax errors.
+
 Implementing the High-Level CEC Adapter
 ---
 
@@ -298,6 +365,9 @@ CEC protocol driven. The following high-level callbacks are 
available:
/* Low-level callbacks */
...
 
+   /* Error injection callbacks */
+   ...
+
/* High-level CEC message callback */
int (*received)(struct cec_adapter *adap, struct cec_msg *msg);
};
-- 
2.15.1

[PATCH 3/6] cec-pin: create cec_pin_start_timer() function

[Hans Verkuil]

From: Hans Verkuil 

This function will be needed for injecting a custom pulse.

Signed-off-by: Hans Verkuil 
---
 drivers/media/cec/cec-pin-priv.h |  2 ++
 drivers/media/cec/cec-pin.c  | 21 +
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/media/cec/cec-pin-priv.h b/drivers/media/cec/cec-pin-priv.h
index cf41c4236efd..4571a0001a9d 100644
--- a/drivers/media/cec/cec-pin-priv.h
+++ b/drivers/media/cec/cec-pin-priv.h
@@ -118,4 +118,6 @@ struct cec_pin {
u32 timer_sum_overrun;
 };
 
+void cec_pin_start_timer(struct cec_pin *pin);
+
 #endif
diff --git a/drivers/media/cec/cec-pin.c b/drivers/media/cec/cec-pin.c
index 8e834b9f72c6..67d6ea9f56b6 100644
--- a/drivers/media/cec/cec-pin.c
+++ b/drivers/media/cec/cec-pin.c
@@ -680,6 +680,18 @@ static int cec_pin_adap_log_addr(struct cec_adapter *adap, 
u8 log_addr)
return 0;
 }
 
+void cec_pin_start_timer(struct cec_pin *pin)
+{
+   if (pin->state != CEC_ST_RX_IRQ)
+   return;
+
+   atomic_set(>work_irq_change, CEC_PIN_IRQ_UNCHANGED);
+   pin->ops->disable_irq(pin->adap);
+   cec_pin_high(pin);
+   cec_pin_to_idle(pin);
+   hrtimer_start(>timer, ns_to_ktime(0), HRTIMER_MODE_REL);
+}
+
 static int cec_pin_adap_transmit(struct cec_adapter *adap, u8 attempts,
  u32 signal_free_time, struct cec_msg *msg)
 {
@@ -689,14 +701,7 @@ static int cec_pin_adap_transmit(struct cec_adapter *adap, 
u8 attempts,
pin->tx_msg = *msg;
pin->work_tx_status = 0;
pin->tx_bit = 0;
-   if (pin->state == CEC_ST_RX_IRQ) {
-   atomic_set(>work_irq_change, CEC_PIN_IRQ_UNCHANGED);
-   pin->ops->disable_irq(adap);
-   cec_pin_high(pin);
-   cec_pin_to_idle(pin);
-   hrtimer_start(>timer, ns_to_ktime(0),
- HRTIMER_MODE_REL);
-   }
+   cec_pin_start_timer(pin);
return 0;
 }
 
-- 
2.15.1

[PATCH 6/6] cec-pin-error-inj.rst: document CEC Pin Error Injection

[Hans Verkuil]

From: Hans Verkuil 

The CEC Pin framework adds support for Error Injection.

Document all the error injections commands and how to use it.

Signed-off-by: Hans Verkuil 
---
 .../media/cec-drivers/cec-pin-error-inj.rst| 321 +
 Documentation/media/cec-drivers/index.rst  |   1 +
 MAINTAINERS|   1 +
 3 files changed, 323 insertions(+)
 create mode 100644 Documentation/media/cec-drivers/cec-pin-error-inj.rst

diff --git a/Documentation/media/cec-drivers/cec-pin-error-inj.rst 
b/Documentation/media/cec-drivers/cec-pin-error-inj.rst
new file mode 100644
index ..128755f43832
--- /dev/null
+++ b/Documentation/media/cec-drivers/cec-pin-error-inj.rst
@@ -0,0 +1,321 @@
+CEC Pin Framework Error Injection
+=
+
+The CEC Pin Framework is a core CEC framework for CEC hardware that only
+has low-level support for the CEC bus. Most hardware today will have
+high-level CEC support where the hardware deals with driving the CEC bus,
+but some older devices aren't that fancy. However, this framework also
+allows you to connect the CEC pin to a GPIO on e.g. a Raspberry Pi and
+you can become an instant CEC adapter.
+
+What makes doing this so interesting is that since we have full control
+over the bus it is easy to support error injection. This is ideal to
+test how well CEC adapters can handle error conditions.
+
+Currently only the cec-gpio driver (when the CEC line is directly
+connected to a pull-up GPIO line) and the AllWinner A10/A20 drm driver
+support this framework.
+
+If ``CONFIG_CEC_PIN_ERROR_INJ`` is enabled, then error injection is available
+through debugfs. Specifically, in ``/sys/kernel/debug/cec/cecX/`` there is
+now an ``error-inj`` file.
+
+With ``cat error-inj`` you can see both the possible commands and the current
+error injection status:
+
+.. code-block:: none
+
+   $ cat /sys/kernel/debug/cec/cec0/error-inj
+   # Clear error injections:
+   #   clear: clear all rx and tx error injections
+   #   rx-clear:  clear all rx error injections
+   #   tx-clear:  clear all tx error injections
+   #clear:clear all rx and tx error injections for 
+   #rx-clear: clear all rx error injections for 
+   #tx-clear: clear all tx error injections for 
+   #
+   # RX error injection:
+   #rx-nack:NACK the message instead of sending an ACK
+   #rx-low-drive : force a low-drive condition at this bit 
position
+   #rx-add-byte:add a spurious byte to the received CEC message
+   #rx-remove-byte: remove the last byte from the received CEC 
message
+   #rx-arb-lost :generate a POLL message to trigger an 
arbitration lost
+   #
+   # TX error injection settings:
+   #   tx-ignore-nack-until-eom: ignore early NACKs until EOM
+   #   tx-custom-low-usecs :  define the 'low' time for the custom 
pulse
+   #   tx-custom-high-usecs : define the 'high' time for the custom 
pulse
+   #   tx-custom-pulse:  transmit the custom pulse once the 
bus is idle
+   #
+   # TX error injection:
+   #tx-no-eom:don't set the EOM bit
+   #tx-early-eom: set the EOM bit one byte too soon
+   #tx-add-bytes :   append  (1-255) spurious bytes to 
the message
+   #tx-remove-byte:   drop the last byte from the message
+   #tx-short-bit :   make this bit shorter than allowed
+   #tx-long-bit :make this bit longer than allowed
+   #tx-custom-bit :  send the custom pulse instead of this bit
+   #tx-short-start:   send a start pulse that's too short
+   #tx-long-start:send a start pulse that's too long
+   #tx-custom-start:  send the custom pulse instead of the 
start pulse
+   #tx-last-bit :stop sending after this bit
+   #tx-low-drive :   force a low-drive condition at this bit 
position
+   #
+   # :CEC message opcode (0-255), 'next' or 'all'
+   # :   CEC message bit (0-159)
+   #  10 bits per 'byte': bits 0-7: data, bit 8: EOM, bit 9: ACK
+   # :  CEC poll message used to test arbitration lost (0x00-0xff, 
default 0x0f)
+   # : microseconds (0-1000, default 1000)
+
+   clear
+
+You can write error injection commands to ``error-inj`` using ``echo 'cmd' 
>error-inj``
+or ``cat cmd.txt >error-inj``. The ``cat error-inj`` output contains the 
current
+error commands. You can save the output to a file and use it as an input to
+``error-inj`` later.
+
+Basic Syntax
+
+
+Leading spaces/tabs are ignored. If the next character is a ``#`` or the end 
of the
+line was reached, then the whole line is ignored. Otherwise a command is 
expected.
+
+The error injection commands fall in two main groups: those relating to 
receiving
+CEC 

[PATCH 4/6] cec-pin-error-inj: parse/show error injection

[Hans Verkuil]

From: Hans Verkuil 

Add support to the CEC Pin framework to parse error injection commands
and to show them.

The next patch will do the actual implementation of this.

Signed-off-by: Hans Verkuil 
---
 drivers/media/cec/Kconfig |   6 +
 drivers/media/cec/Makefile|   4 +
 drivers/media/cec/cec-pin-error-inj.c | 372 ++
 drivers/media/cec/cec-pin-priv.h  |  96 +
 drivers/media/cec/cec-pin.c   |   6 +
 5 files changed, 484 insertions(+)
 create mode 100644 drivers/media/cec/cec-pin-error-inj.c

diff --git a/drivers/media/cec/Kconfig b/drivers/media/cec/Kconfig
index 43428cec3a01..9c2b108c613a 100644
--- a/drivers/media/cec/Kconfig
+++ b/drivers/media/cec/Kconfig
@@ -4,3 +4,9 @@ config MEDIA_CEC_RC
depends on CEC_CORE=m || RC_CORE=y
---help---
  Pass on CEC remote control messages to the RC framework.
+
+config CEC_PIN_ERROR_INJ
+   bool "Enable CEC error injection support"
+   depends on CEC_PIN && DEBUG_FS
+   ---help---
+ This option enables CEC error injection using debugfs.
diff --git a/drivers/media/cec/Makefile b/drivers/media/cec/Makefile
index 41ee3325e1ea..29a2ab9e77c5 100644
--- a/drivers/media/cec/Makefile
+++ b/drivers/media/cec/Makefile
@@ -9,4 +9,8 @@ ifeq ($(CONFIG_CEC_PIN),y)
   cec-objs += cec-pin.o
 endif
 
+ifeq ($(CONFIG_CEC_PIN_ERROR_INJ),y)
+  cec-objs += cec-pin-error-inj.o
+endif
+
 obj-$(CONFIG_CEC_CORE) += cec.o
diff --git a/drivers/media/cec/cec-pin-error-inj.c 
b/drivers/media/cec/cec-pin-error-inj.c
new file mode 100644
index ..d03d7f0c2652
--- /dev/null
+++ b/drivers/media/cec/cec-pin-error-inj.c
@@ -0,0 +1,372 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2017 Cisco Systems, Inc. and/or its affiliates. All rights 
reserved.
+ */
+
+#include 
+#include 
+#include 
+
+#include 
+#include "cec-pin-priv.h"
+
+u32 cec_pin_rx_error_inj(struct cec_pin *pin)
+{
+   u16 cmd = CEC_ERROR_INJ_OP_NEXT;
+
+   /* Only when 18 bits have been received do we have a valid cmd */
+   if (!pin->rx_error_inj[cmd] && pin->rx_bit >= 18)
+   cmd = pin->rx_msg.msg[1];
+   return pin->rx_error_inj[cmd] ? :
+   (pin->rx_error_inj[CEC_ERROR_INJ_OP_NEXT] ? :
+pin->rx_error_inj[CEC_ERROR_INJ_OP_ALL]);
+}
+
+u64 cec_pin_tx_error_inj(struct cec_pin *pin)
+{
+   u16 cmd = CEC_ERROR_INJ_OP_NEXT;
+
+   if (!pin->tx_error_inj[cmd] && pin->tx_msg.len > 1)
+   cmd = pin->tx_msg.msg[1];
+   return pin->tx_error_inj[cmd] ? :
+   (pin->tx_error_inj[CEC_ERROR_INJ_OP_NEXT] ? :
+pin->tx_error_inj[CEC_ERROR_INJ_OP_ALL]);
+}
+
+bool cec_pin_error_inj_parse_line(struct cec_adapter *adap, char *line)
+{
+   static const char *delims = " \t\r";
+   struct cec_pin *pin = adap->pin;
+   bool has_pos = false;
+   char *p = line;
+   char *token;
+   u32 *rx_error;
+   u64 *tx_error;
+   bool need_op;
+   u32 op;
+   u8 pos;
+   u8 v;
+
+   p = skip_spaces(p);
+   token = strsep(, delims);
+   if (!strcmp(token, "clear")) {
+   memset(pin->tx_error_inj, 0, sizeof(pin->tx_error_inj));
+   memset(pin->rx_error_inj, 0, sizeof(pin->rx_error_inj));
+   pin->tx_ignore_nack_until_eom = false;
+   pin->tx_custom_pulse = false;
+   pin->tx_custom_low_usecs = CEC_TIM_CUSTOM_DEFAULT;
+   pin->tx_custom_high_usecs = CEC_TIM_CUSTOM_DEFAULT;
+   return true;
+   }
+   if (!strcmp(token, "rx-clear")) {
+   memset(pin->rx_error_inj, 0, sizeof(pin->rx_error_inj));
+   return true;
+   }
+   if (!strcmp(token, "tx-clear")) {
+   memset(pin->tx_error_inj, 0, sizeof(pin->tx_error_inj));
+   pin->tx_ignore_nack_until_eom = false;
+   pin->tx_custom_pulse = false;
+   pin->tx_custom_low_usecs = CEC_TIM_CUSTOM_DEFAULT;
+   pin->tx_custom_high_usecs = CEC_TIM_CUSTOM_DEFAULT;
+   return true;
+   }
+   if (!strcmp(token, "tx-ignore-nack-until-eom")) {
+   pin->tx_ignore_nack_until_eom = true;
+   return true;
+   }
+   if (!strcmp(token, "tx-custom-pulse")) {
+   pin->tx_custom_pulse = true;
+   cec_pin_start_timer(pin);
+   return true;
+   }
+   if (!p)
+   return false;
+
+   p = skip_spaces(p);
+   if (!strcmp(token, "tx-custom-low-usecs")) {
+   u32 usecs;
+
+   if (kstrtou32(p, 0, ) || usecs > 1000)
+   return false;
+   pin->tx_custom_low_usecs = usecs;
+   return true;
+   }
+   if (!strcmp(token, "tx-custom-high-usecs")) {
+   u32 usecs;
+
+   if (kstrtou32(p, 0, ) || usecs > 1000)
+ 

[PATCH 0/6] cec: add error injection support

[Hans Verkuil]

From: Hans Verkuil 

This patch series adds support for CEC error injection for drivers
using the CEC Pin Framework (cec-pin.c). There are two CEC drivers
currently using this framework: the sun4i Allwinner A10/A20 driver
and the cec-gpio driver. This patch series was developed with the
cec-gpio driver and a Raspberry Pi.

The CEC Pin Framework is meant for hardware that has no high-level
support but only direct low-level control of the bus (i.e. pull the
CEC line down or read the CEC line). Low-level bus access like that
is ideal to implement error injection since you have full control of
the bus and you can do anything you want.

This new error injection framework can create most if not all error
conditions that I could think of. We (Cisco) used it to verify our
own CEC implementation and in fact this error injection framework
was developed together with the low-level CEC analysis code in the
cec-ctl userspace utility to analyze what is happening on the bus.

I have been working on creating scripts that can test a remote CEC
adapter for low-level compliance with the CEC standard:

https://git.linuxtv.org/hverkuil/v4l-utils.git/log/?h=cec-pin-tests

It is not complete yet since it isn't smart enough to tell the
difference between different (but valid) interpretations of the CEC
specification and actual violations of the specification. I plan
to continue working on that since I would like to have a test-suite
that can check a CEC implementation automatically.

Special thanks go to Wolfram Sang since his i2c error injection
presentation at the Embedded Linux Conference Europe 2017 inspired
me to switch to debugfs for this instead of using ioctls.

Regards,

Hans


Hans Verkuil (6):
  cec: add core error injection support
  cec-core.rst: document the error injection ops.
  cec-pin: create cec_pin_start_timer() function
  cec-pin-error-inj: parse/show error injection
  cec-pin: add error injection support
  cec-pin-error-inj.rst: document CEC Pin Error Injection

 .../media/cec-drivers/cec-pin-error-inj.rst| 321 +
 Documentation/media/cec-drivers/index.rst  |   1 +
 Documentation/media/kapi/cec-core.rst  |  72 ++-
 MAINTAINERS|   1 +
 drivers/media/cec/Kconfig  |   6 +
 drivers/media/cec/Makefile |   4 +
 drivers/media/cec/cec-core.c   |  58 +++
 drivers/media/cec/cec-pin-error-inj.c  | 372 +++
 drivers/media/cec/cec-pin-priv.h   | 136 +-
 drivers/media/cec/cec-pin.c| 529 ++---
 include/media/cec.h|   5 +
 11 files changed, 1437 insertions(+), 68 deletions(-)
 create mode 100644 Documentation/media/cec-drivers/cec-pin-error-inj.rst
 create mode 100644 drivers/media/cec/cec-pin-error-inj.c

-- 
2.15.1

[PATCH V2 2/2] dt-bindings: Document the Rockchip RK1608 bindings

[Wen Nuan]

From: Leo Wen 

Add DT bindings documentation for Rockchip RK1608.

Changes V2:
- Delete spi-min-frequency property.
- Add the external sensor's control pin and clock properties.
- Delete the '' node.

Signed-off-by: Leo Wen 
---
 Documentation/devicetree/bindings/media/rk1608.txt | 97 ++
 MAINTAINERS|  1 +
 2 files changed, 98 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/rk1608.txt

diff --git a/Documentation/devicetree/bindings/media/rk1608.txt 
b/Documentation/devicetree/bindings/media/rk1608.txt
new file mode 100644
index 000..a9721a8
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/rk1608.txt
@@ -0,0 +1,97 @@
+Rockchip RK1608 as a PreISP to link on Soc
+--
+
+Required properties:
+
+- compatible   : "rockchip,rk1608";
+- reg  : SPI slave address of the rk1608;
+- clocks   : Must contain an entry for each entry in clock-names;
+- clock-names  : Must contain "mclk" for the device's master clock;
+- reset-gpio   : GPIO connected to reset pin;
+- irq-gpio : GPIO connected to irq pin;
+- sleepst-gpio : GPIO connected to sleepst pin;
+- wakeup-gpio  : GPIO connected to wakeup pin;
+- powerdown-gpio   : GPIO connected to powerdown pin;
+- rockchip,powerdown0  : GPIO connected to the sensor0's powerdown pin;
+- rockchip,reset0  : GPIO connected to the sensor0's reset pin;
+- rockchip,powerdown1  : GPIO connected to the sensor1's powerdown pin;
+- rockchip,reset1  : GPIO connected to the sensor1's reset pin;
+- pinctrl-names: Should contain only one value - "default";
+- pinctrl-0: Pin control group to be used for this controller;
+
+Optional properties:
+
+- spi-max-frequency: Maximum SPI clocking speed of the device;
+
+The device node should contain one 'port' child node with one child 'endpoint'
+node, according to the bindings defined in Documentation/devicetree/bindings/
+media/video-interfaces.txt. The following are properties specific to those
+nodes.
+
+endpoint node
+-
+
+- data-lanes : (optional) specifies MIPI CSI-2 data lanes as covered in
+  video-interfaces.txt. If present it should be <1> - the device
+  supports only one data lane without re-mapping.
+
+Note1: Since no data is generated in RK1608，so this is meaningful that you need
+a extra sensor (such as a camera) mounted on RK1608. You need to use endpoint@x
+to match these sensors.
+
+Note2:You must set the current value of the spi pins to be 8mA, if they are 
not.
+
+Example:
+ {
+   status = "okay";
+   spi_rk1608@00 {
+   compatible =  "rockchip,rk1608";
+   status = "okay";
+   reg = <0>;
+   spi-max-frequency = <2400>;
+   link-freqs = /bits/ 64 <4>;
+   clocks = < SCLK_SPI0>, < SCLK_VIP_OUT>,
+   < DCLK_VOP0>, < ACLK_VIP>, < HCLK_VIP>,
+   < PCLK_ISP_IN>, < PCLK_ISP_IN>,
+   < PCLK_ISP_IN>, < SCLK_MIPIDSI_24M>,
+   < PCLK_MIPI_CSI>;
+   clock-names = "mclk", "mipi_clk",  "pd_cif", "aclk_cif",
+   "hclk_cif", "cif0_in", "g_pclkin_cif",
+   "cif0_out", "clk_mipi_24m", "hclk_mipiphy";
+   reset-gpio = < 0 GPIO_ACTIVE_HIGH>;
+   irq-gpio = < 2 GPIO_ACTIVE_HIGH>;
+   sleepst-gpio = < 1 GPIO_ACTIVE_HIGH>;
+   wakeup-gpio = < 4 GPIO_ACTIVE_HIGH>;
+   powerdown-gpio = < 0 GPIO_ACTIVE_HIGH>;
+
+   rockchip,powerdown1 = < 9 GPIO_ACTIVE_HIGH>;
+   rockchip,reset1 = < 8 GPIO_ACTIVE_HIGH>;
+
+   rockchip,powerdown0 = < 8 GPIO_ACTIVE_HIGH>;
+   rockchip,reset0 = < 7 GPIO_ACTIVE_HIGH>;
+
+   pinctrl-names = "default";
+   pinctrl-0 = <_irq_gpios _wake_gpios
+_sleep_gpios>;
+
+   port@0 {
+   mipi_dphy_out: endpoint {
+   remote-endpoint = <_dphy_in>;
+   clock-lanes = <0>;
+   data-lanes = <1 2 3 4>;
+   clock-noncontinuous;
+   link-frequencies =
+   /bits/ 64 <4>;
+   };
+   };
+   /* Example: we have two cameras */
+   port@1 {
+   sensor_in0: endpoint@0 {
+   remote-endpoint = <_out0>;
+   };
+   sensor_in1: endpoint@1 {
+   remote-endpoint = <_out1>;
+   };
+   };
+   };
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index b2a98e3..04d227b 

[PATCH V2 1/2] [media] Add Rockchip RK1608 driver

[Wen Nuan]

From: Leo Wen 

Rk1608 is used as a PreISP to link on Soc, which mainly has two functions.
One is to download the firmware of RK1608, and the other is to match the
extra sensor such as camera and enable sensor by calling sensor's s_power.

use below v4l2-ctl command to capture frames.

v4l2-ctl --verbose -d /dev/video1 --stream-mmap=2
--stream-to=/tmp/stream.out --stream-count=60 --stream-poll

use below command to playback the video on your PC.

mplayer ./stream.out -loop 0 -demuxer rawvideo -rawvideo
w=640:h=480:size=$((640*480*3/2)):format=NV12

Changes V2:
- Delete long legalese.
- Add the new SPDX tags.
- Dual cameras can be captured at the same time.
- Add some pins and clock control for external sensors.
- Add a function, the Soc can send control MSG to RK1608. 

Signed-off-by: Leo Wen 
---
 MAINTAINERS|6 +
 drivers/media/spi/Makefile |1 +
 drivers/media/spi/rk1608.c | 1664 
 drivers/media/spi/rk1608.h |  471 +
 4 files changed, 2142 insertions(+)
 create mode 100644 drivers/media/spi/rk1608.c
 create mode 100644 drivers/media/spi/rk1608.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 93a12af..b2a98e3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -136,6 +136,12 @@ Maintainers List (try to look for most precise areas first)
 
---
 
+ROCKCHIP RK1608 DRIVER
+M: Leo Wen 
+S: Maintained
+F: drivers/media/spi/rk1608.c
+F: drivers/media/spi/rk1608.h
+
 3C59X NETWORK DRIVER
 M: Steffen Klassert 
 L: net...@vger.kernel.org
diff --git a/drivers/media/spi/Makefile b/drivers/media/spi/Makefile
index ea64013..9d9d9ec 100644
--- a/drivers/media/spi/Makefile
+++ b/drivers/media/spi/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_VIDEO_GS1662) += gs1662.o
+obj-$(CONFIG_ROCKCHIP_RK1608) += rk1608.o
diff --git a/drivers/media/spi/rk1608.c b/drivers/media/spi/rk1608.c
new file mode 100644
index 000..c229b30
--- /dev/null
+++ b/drivers/media/spi/rk1608.c
@@ -0,0 +1,1664 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * Rockchip rk1608 driver
+ *
+ * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "rk1608.h"
+
+/**
+ * Rk1608 is used as the Pre-ISP to link on Soc, which mainly has two
+ * functions. One is to download the firmware of RK1608, and the other
+ * is to match the extra sensor such as camera and enable sensor by
+ * calling sensor's s_power.
+ * |---|
+ * | Sensor Camera |
+ * |---|
+ * |---||--|
+ * |---||--|
+ * |---\/--|
+ * | Pre-ISP RK1608|
+ * |---|
+ * |---||--|
+ * |---||--|
+ * |---\/--|
+ * |  Rockchip Soc |
+ * |---|
+ * Data Transfer As shown above. In RK1608, the data received from the
+ * extra sensor,and it is passed to the Soc through ISP.
+ */
+
+static inline struct rk1608_state *to_state(struct v4l2_subdev *sd)
+{
+   return container_of(sd, struct rk1608_state, sd);
+}
+
+/**
+ * rk1608_operation_query - RK1608 last operation state query
+ *
+ * @spi: device from which data will be read
+ * @state: last operation state [out]
+ * Context: can sleep
+ *
+ * It returns zero on success, else a negative error code.
+ */
+int rk1608_operation_query(struct spi_device *spi, s32 *state)
+{
+   s32 query_cmd = RK1608_CMD_QUERY;
+   struct spi_transfer query_cmd_packet = {
+   .tx_buf = _cmd,
+   .len= sizeof(query_cmd),
+   };
+   struct spi_transfer state_packet = {
+   .rx_buf = state,
+   .len= sizeof(*state),
+   };
+   struct spi_message  m;
+
+   spi_message_init();
+   spi_message_add_tail(_cmd_packet, );
+   spi_message_add_tail(_packet, );
+   spi_sync(spi, );
+
+   return ((*state & RK1608_STATE_ID_MASK) == RK1608_STATE_ID) ? 0 : -1;
+}
+
+int rk1608_write(struct spi_device *spi,
+s32 addr, const s32 *data, size_t data_len)
+{
+   s32 write_cmd = RK1608_CMD_WRITE;
+   struct spi_transfer write_cmd_packet = {
+   .tx_buf = _cmd,
+   .len= sizeof(write_cmd),
+   };
+   struct spi_transfer addr_packet = {
+   .tx_buf = ,
+   .len= sizeof(addr),
+   };
+   struct spi_transfer data_packet = {
+   .tx_buf = data,
+   .len= data_len,
+   };
+   struct spi_message  m;
+
+   spi_message_init();
+   spi_message_add_tail(_cmd_packet, );
+   spi_message_add_tail(_packet, );
+   

[PATCH V2 0/2] Rockchip: Add RK1608 driver and DT-bindings

[Wen Nuan]

From: Leo Wen 

You can use the v4l2-ctl command to capture frames for RK1608.
Add DT-bindings documentation for Rockchip RK1608.
Add the information of the MAINTAINERS.

Leo Wen (2):
  [media] Add Rockchip RK1608 driver
  dt-bindings: Document the Rockchip RK1608 bindings

 Documentation/devicetree/bindings/media/rk1608.txt |   97 ++
 MAINTAINERS|7 +
 drivers/media/spi/Makefile |1 +
 drivers/media/spi/rk1608.c | 1664 
 drivers/media/spi/rk1608.h |  471 ++
 5 files changed, 2240 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/rk1608.txt
 create mode 100644 drivers/media/spi/rk1608.c
 create mode 100644 drivers/media/spi/rk1608.h

-- 
2.7.4

Re: [PATCH v3 01/10] pwm: extend PWM framework with PWM modes

[Claudiu Beznea]

I'll rebase it on latest for-next in next version.

Thank you,
Claudiu Beznea

On 24.02.2018 22:49, kbuild test robot wrote:
> Hi Claudiu,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on pwm/for-next]
> [also build test WARNING on v4.16-rc2 next-20180223]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Claudiu-Beznea/extend-PWM-framework-to-support-PWM-modes/20180225-024011
> base:   
> https://git.kernel.org/pub/scm/linux/kernel/git/thierry.reding/linux-pwm.git 
> for-next
> config: xtensa-allmodconfig (attached as .config)
> compiler: xtensa-linux-gcc (GCC) 7.2.0
> reproduce:
> wget 
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=xtensa 
> 
> All warnings (new ones prefixed by >>):
> 
>>> drivers//pwm/pwm-sun4i.c:36:0: warning: "PWM_MODE" redefined
> #define PWM_MODE  BIT(7)
> 
>In file included from drivers//pwm/pwm-sun4i.c:19:0:
>include/linux/pwm.h:40:0: note: this is the location of the previous 
> definition
> #define PWM_MODE(name)  BIT(PWM_MODE_##name##_BIT)
> 
> 
> vim +/PWM_MODE +36 drivers//pwm/pwm-sun4i.c
> 
> 09853ce7 Alexandre Belloni 2014-12-17  29  
> 09853ce7 Alexandre Belloni 2014-12-17  30  #define PWMCH_OFFSET   
> 15
> 09853ce7 Alexandre Belloni 2014-12-17  31  #define PWM_PRESCAL_MASK   
> GENMASK(3, 0)
> 09853ce7 Alexandre Belloni 2014-12-17  32  #define PWM_PRESCAL_OFF> 0
> 09853ce7 Alexandre Belloni 2014-12-17  33  #define PWM_EN 
> BIT(4)
> 09853ce7 Alexandre Belloni 2014-12-17  34  #define PWM_ACT_STATE  
> BIT(5)
> 09853ce7 Alexandre Belloni 2014-12-17  35  #define PWM_CLK_GATING 
> BIT(6)
> 09853ce7 Alexandre Belloni 2014-12-17 @36  #define PWM_MODE   BIT(7)
> 09853ce7 Alexandre Belloni 2014-12-17  37  #define PWM_PULSE  BIT(8)
> 09853ce7 Alexandre Belloni 2014-12-17  38  #define PWM_BYPASS BIT(9)
> 09853ce7 Alexandre Belloni 2014-12-17  39  
> 
> :: The code at line 36 was first introduced by commit
> :: 09853ce7bc1003a490c7ee74a5705d7a7cf16b7d pwm: Add Allwinner SoC support
> 
> :: TO: Alexandre Belloni 
> :: CC: Thierry Reding 
> 
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation
>