[PATCH v2] v4l: async: Protect against double notifier registrations

[Kieran Bingham]

From: Kieran Bingham 

It can be easy to attempt to register the same notifier twice
in mis-handled error cases such as working with -EPROBE_DEFER.

This results in odd kernel crashes where the notifier_list becomes
corrupted due to adding the same entry twice.

Protect against this so that a developer has some sense of the pending
failure, and use a WARN_ON to identify the fault.

Signed-off-by: Kieran Bingham 

---
v2:
 - Reduce verbosity
 - use WARN_ON()
 - Move notifier list initialisation after registration check

 drivers/media/v4l2-core/v4l2-async.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-async.c 
b/drivers/media/v4l2-core/v4l2-async.c
index 2b08d03b251d..17a779440ae3 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -374,17 +374,26 @@ static int __v4l2_async_notifier_register(struct 
v4l2_async_notifier *notifier)
struct device *dev =
notifier->v4l2_dev ? notifier->v4l2_dev->dev : NULL;
struct v4l2_async_subdev *asd;
+   struct v4l2_async_notifier *n;
int ret;
int i;
 
if (notifier->num_subdevs > V4L2_MAX_SUBDEVS)
return -EINVAL;
 
+   mutex_lock(_lock);
+
+   /* Avoid re-registering a notifier. */
+   list_for_each_entry(n, _list, list) {
+   if (WARN_ON(n == notifier)) {
+   ret = -EEXIST;
+   goto err_unlock;
+   }
+   }
+
INIT_LIST_HEAD(>waiting);
INIT_LIST_HEAD(>done);
 
-   mutex_lock(_lock);
-
for (i = 0; i < notifier->num_subdevs; i++) {
asd = notifier->subdevs[i];
 
-- 
2.7.4

Re: [PATCH v2 21/22] mmc: tmio: move {tmio_}mmc_of_parse() to tmio_mmc_host_alloc()

[Wolfram Sang]

On Sat, Nov 25, 2017 at 01:24:56AM +0900, Masahiro Yamada wrote:
> mmc_of_parse() parses various DT properties and sets capability flags
> accordingly.  However, drivers have no chance to run platform init
> code depending on such flags because mmc_of_parse() is called from
> tmio_mmc_host_probe().
> 
> Move mmc_of_parse() to tmio_mmc_host_alloc() so that drivers can
> handle capabilities before mmc_add_host().  Move tmio_mmc_of_parse()
> likewise.
> 
> Signed-off-by: Masahiro Yamada 

Reviewed-by: Wolfram Sang 



signature.asc
Description: PGP signature

Re: [PATCH v2 22/22] mmc: tmio: remove dma_ops from tmio_mmc_host_probe() argument

[Wolfram Sang]

On Sat, Nov 25, 2017 at 01:24:57AM +0900, Masahiro Yamada wrote:
> Drivers need to set up various struct members for tmio_mmc_host before
> calling tmio_mmc_host_probe().  Do likewise for host->dma_ops instead
> of passing it as a function argument.
> 
> Signed-off-by: Masahiro Yamada 

Reviewed-by: Wolfram Sang 

I think I have done all the review by now. I'll do some more regression
testing tomorrow and then give Tested-by as well if everything works
fine (but it looks like it so far).



signature.asc
Description: PGP signature

Re: [PATCH v2 20/22] mmc: tmio: move clk_enable/disable out of tmio_mmc_host_probe()

[Wolfram Sang]

On Sat, Nov 25, 2017 at 01:24:55AM +0900, Masahiro Yamada wrote:
> The clock is enabled in the tmio_mmc_host_probe().  It also prevents
> drivers from performing platform-specific settings before mmc_add_host()
> because the register access generally requires a clock.
> 
> Enable/disable the clock in drivers' probe/remove.  Also, I passed
> tmio_mmc_data to tmio_mmc_host_alloc() because renesas_sdhi_clk_enable()
> needs it to get the private data from tmio_mmc_host.
> 
> Signed-off-by: Masahiro Yamada 

Reviewed-by: Wolfram Sang 



signature.asc
Description: PGP signature

Re: [PATCH v2 2/2] drm: rcar-du: lvds: Fix LVDS startup on R-Car gen2

[Laurent Pinchart]

Hi Sergei,

On Tuesday, 16 January 2018 22:17:31 EET Sergei Shtylyov wrote:
> On 01/16/2018 06:46 PM, Laurent Pinchart wrote:
> >>> From: Sergei Shtylyov 
> >>> 
> >>> According to the latest revision 2.00 of the R-Car gen2 manual, the LVDS
> >>> must be enabled and the bias crcuit enabled after the LVDS I/O pins are
> 
> Oops, this needs fixing (note the typo!). Could you please change this
> passage to:
> 
> and the bias circuit must be enabled after the LVDS I/O pins are
> 
> ?

Sure I'll fix that.

> >>> enabled, not before. Fix the gen2 LVDS startup sequence accordingly.
> >>> 
> >>> While at it, also fix the comment preceding the first LVDCR0 write that
> >>> still talks about hardcoding the LVDS mode 0.
> >> 
> >> Please do this in a separate commit then...
> > 
> > The reason I added it here is that I think we don't need patch 1/2 from
> > this series, and I found a bit overkill to split a comment fix to a
> > separate patch when we have a patch that touches the code around the
> > comment.
> 
> OK, you're the maintainer of this driver, you know better. :-)
> 
> >>> Fixes: 90374b5c25c9 ("drm/rcar-du: Add internal LVDS encoder support")
> >> 
> >> You forgot to specify the other commit this one fixes -- I mean the
> >> comment fix.
> > 
> > Do we need to for a comment update ? It doesn't affect fix the behaviour
> > of the driver or device, and I'd thus prefer to avoid giving the wrong
> > impression that this patch fixes an bug introduced in a previous commit,
> > otherwise it might end up being backported unnecessarily.
> 
> OK, no dire need to backport indeed...
> 
> >>> Signed-off-by: Sergei Shtylyov 
> >>> Reviewed-by: Laurent Pinchart
> >>> 
> >>> [Set the mode and input at the same time as the BEN and LVEN bits]
> >>> Tested-by: Laurent Pinchart 
> >>> Signed-off-by: Laurent Pinchart
> >>> 
> >>> ---
> >>> 
> >>>drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 14 +++---
> >>>1 file changed, 7 insertions(+), 7 deletions(-)
> >>> 
> >>> Hi Sergei,
> >>> 
> >>> For your convenience (and if you agree with bundling mode setup with the
> >>> first write as explained in my review of patch 1/2), here's the updated
> >>> version of patch 2/2 that I have taken in my development branch. If
> >>> you're fine with it I'll keep it, otherwise we can continue the review
> >>> discussion.
> >> 
> >> As I said, I don't know how to interpret the note 3 in either manual.
> 
> Moreover, it seems to me that the notes don't match the start-up procedure
> anymore...

How so ?

> > As explained in my latest reply to patch 1/2, my understanding is that the
> > parameters can be programmed at any time before step 6. The fact that the
> > current code works seems to confirm that interpretation. We could ask
> > Renesas for a confirmation if you want.
> 
> Would be good to ask, indeed.

I'll ask.

-- 
Regards,

Laurent Pinchart

[PATCH v6 0/9] Renesas Capture Engine Unit (CEU) V4L2 driver

[Jacopo Mondi]

Hello,
   new version of CEU after Hans' review.

Added his Acked-by to most patches and closed review comments.
Running v4l2-compliance, I noticed a new failure introduced by the way I now
calculate the plane sizes in set/try_fmt.

This is the function used to update per-plane bytesperline and sizeimage:

static void ceu_update_plane_sizes(struct v4l2_plane_pix_format *plane,
   unsigned int bpl, unsigned int szimage)
{
if (plane->bytesperline < bpl)
plane->bytesperline = bpl;
if (plane->sizeimage < szimage)
plane->sizeimage = szimage;
}

I'm seeing a failure as v4l2-compliance requires buffers with both bytesperline
and sizeimage set to MAX_INT . Hans, is this expected from v4l2-compliance?
How should I handle this, if that has to be handled by the single drivers?

Apart from that, here it is the output of v4l2-compliance, with the last tests
failing due to the above stated reason, and two errors in try/set format due to
the fact the driver is not setting ycbcr encoding after it receives an invalid
format. I would set those, but I'm not sure what it the correct value and not
all mainline drivers do that.

---
v4l2-compliance SHA   : 1d3c611dee82090d9456730e24af368b51dcb4a9

Driver Info:
Driver name   : renesas-ceu
Card type : Renesas CEU e821.ceu
Bus info  : platform:renesas-ceu-e821.c
Driver version: 4.14.0
Capabilities  : 0x84201000
Video Capture Multiplanar
Streaming
Extended Pix Format
Device Capabilities
Device Caps   : 0x04201000
Video Capture Multiplanar
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
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
test VIDIOC_QUERYCTRL: OK
test VIDIOC_G/S_CTRL: OK
test VIDIOC_G/S/TRY_EXT_CTRLS: OK
fail: v4l2-test-controls.cpp(782): subscribe event for control 
'User Controls' failed
test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL
test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
Standard Controls: 12 Private Controls: 0

Format ioctls:
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
fail: v4l2-test-formats.cpp(1162): ret && node->has_frmintervals
test VIDIOC_G/S_PARM: FAIL
test VIDIOC_G_FBUF: OK (Not Supported)
test VIDIOC_G_FMT: OK
fail: v4l2-test-formats.cpp(335): ycbcr_enc >= 0xff
fail: v4l2-test-formats.cpp(451): 
testColorspace(pix_mp.pixelformat, pix_mp.colorspace, pix_mp.ycbcr_enc, 
pix_mp.quantization)
fail: v4l2-test-formats.cpp(736): Video Capture Multiplanar is 
valid, but TRY_FMT failed to return a format
test VIDIOC_TRY_FMT: FAIL
fail: v4l2-test-formats.cpp(335): ycbcr_enc >= 0xff
fail: v4l2-test-formats.cpp(451): 
testColorspace(pix_mp.pixelformat, pix_mp.colorspace, pix_mp.ycbcr_enc, 
pix_mp.quantization)
fail: v4l2-test-formats.cpp(996): Video Capture Multiplanar is 
valid, but no S_FMT was implemented
test VIDIOC_S_FMT: FAIL
test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
test Cropping: OK (Not Supported)
test Composing: OK (Not Supported)
test Scaling: OK (Not Supported)

  

[PATCH v6 1/9] dt-bindings: media: Add Renesas CEU bindings

[Jacopo Mondi]

Add bindings documentation for Renesas Capture Engine Unit (CEU).

Signed-off-by: Jacopo Mondi 
Reviewed-by: Rob Herring 
Reviewed-by: Laurent Pinchart 
Acked-by: Hans Verkuil 
---
 .../devicetree/bindings/media/renesas,ceu.txt  | 81 ++
 1 file changed, 81 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/renesas,ceu.txt

diff --git a/Documentation/devicetree/bindings/media/renesas,ceu.txt 
b/Documentation/devicetree/bindings/media/renesas,ceu.txt
new file mode 100644
index 000..590ee27
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/renesas,ceu.txt
@@ -0,0 +1,81 @@
+Renesas Capture Engine Unit (CEU)
+--
+
+The Capture Engine Unit is the image capture interface found in the Renesas
+SH Mobile and RZ SoCs.
+
+The interface supports a single parallel input with data bus width of 8 or 16
+bits.
+
+Required properties:
+- compatible: Shall be "renesas,r7s72100-ceu" for CEU units found in RZ/A1-H
+  and RZ/A1-M SoCs.
+- reg: Registers address base and size.
+- interrupts: The interrupt specifier.
+
+The CEU supports a single parallel input and should contain a single 'port'
+subnode with a single 'endpoint'. Connection to input devices are modeled
+according to the video interfaces OF bindings specified in:
+Documentation/devicetree/bindings/media/video-interfaces.txt
+
+Optional endpoint properties applicable to parallel input bus described in
+the above mentioned "video-interfaces.txt" file are supported.
+
+- hsync-active: Active state of the HSYNC signal, 0/1 for LOW/HIGH 
respectively.
+  If property is not present, default is active high.
+- vsync-active: Active state of the VSYNC signal, 0/1 for LOW/HIGH 
respectively.
+  If property is not present, default is active high.
+
+Example:
+
+The example describes the connection between the Capture Engine Unit and an
+OV7670 image sensor connected to i2c1 interface.
+
+ceu: ceu@e821 {
+   reg = <0xe821 0x209c>;
+   compatible = "renesas,r7s72100-ceu";
+   interrupts = ;
+
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins>;
+
+   status = "okay";
+
+   port {
+   ceu_in: endpoint {
+   remote-endpoint = <_out>;
+
+   hsync-active = <1>;
+   vsync-active = <0>;
+   };
+   };
+};
+
+i2c1: i2c@fcfee400 {
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins>;
+
+   status = "okay";
+
+   clock-frequency = <10>;
+
+   ov7670: camera@21 {
+   compatible = "ovti,ov7670";
+   reg = <0x21>;
+
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins>;
+
+   reset-gpios = < 11 GPIO_ACTIVE_LOW>;
+   powerdown-gpios = < 12 GPIO_ACTIVE_HIGH>;
+
+   port {
+   ov7670_out: endpoint {
+   remote-endpoint = <_in>;
+
+   hsync-active = <1>;
+   vsync-active = <0>;
+   };
+   };
+   };
+};
-- 
2.7.4

[PATCH v6 2/9] include: media: Add Renesas CEU driver interface

[Jacopo Mondi]

Add renesas-ceu header file.

Do not remove the existing sh_mobile_ceu.h one as long as the original
driver does not go away.

Signed-off-by: Jacopo Mondi 
Reviewed-by: Laurent Pinchart 
Acked-by: Hans Verkuil 
---
 include/media/drv-intf/renesas-ceu.h | 26 ++
 1 file changed, 26 insertions(+)
 create mode 100644 include/media/drv-intf/renesas-ceu.h

diff --git a/include/media/drv-intf/renesas-ceu.h 
b/include/media/drv-intf/renesas-ceu.h
new file mode 100644
index 000..52841d1
--- /dev/null
+++ b/include/media/drv-intf/renesas-ceu.h
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * renesas-ceu.h - Renesas CEU driver interface
+ *
+ * Copyright 2017-2018 Jacopo Mondi 
+ */
+
+#ifndef __MEDIA_DRV_INTF_RENESAS_CEU_H__
+#define __MEDIA_DRV_INTF_RENESAS_CEU_H__
+
+#define CEU_MAX_SUBDEVS2
+
+struct ceu_async_subdev {
+   unsigned long flags;
+   unsigned char bus_width;
+   unsigned char bus_shift;
+   unsigned int i2c_adapter_id;
+   unsigned int i2c_address;
+};
+
+struct ceu_platform_data {
+   unsigned int num_subdevs;
+   struct ceu_async_subdev subdevs[CEU_MAX_SUBDEVS];
+};
+
+#endif /* ___MEDIA_DRV_INTF_RENESAS_CEU_H__ */
-- 
2.7.4

[PATCH v6 3/9] v4l: platform: Add Renesas CEU driver

[Jacopo Mondi]

Add driver for Renesas Capture Engine Unit (CEU).

The CEU interface supports capturing 'data' (YUV422) and 'images'
(NV[12|21|16|61]).

This driver aims to replace the soc_camera-based sh_mobile_ceu one.

Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ
platform GR-Peach.

Tested with ov7725 camera sensor on SH4 platform Migo-R.

Signed-off-by: Jacopo Mondi 
Reviewed-by: Laurent Pinchart 
---
 drivers/media/platform/Kconfig   |9 +
 drivers/media/platform/Makefile  |1 +
 drivers/media/platform/renesas-ceu.c | 1659 ++
 3 files changed, 1669 insertions(+)
 create mode 100644 drivers/media/platform/renesas-ceu.c

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index fd0c998..fe7bd26 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -144,6 +144,15 @@ config VIDEO_STM32_DCMI
  To compile this driver as a module, choose M here: the module
  will be called stm32-dcmi.
 
+config VIDEO_RENESAS_CEU
+   tristate "Renesas Capture Engine Unit (CEU) driver"
+   depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA
+   depends on ARCH_SHMOBILE || ARCH_R7S72100 || COMPILE_TEST
+   select VIDEOBUF2_DMA_CONTIG
+   select V4L2_FWNODE
+   ---help---
+ This is a v4l2 driver for the Renesas CEU Interface
+
 source "drivers/media/platform/soc_camera/Kconfig"
 source "drivers/media/platform/exynos4-is/Kconfig"
 source "drivers/media/platform/am437x/Kconfig"
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 003b0bb..6580a6b 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -62,6 +62,7 @@ obj-$(CONFIG_VIDEO_SH_VOU)+= sh_vou.o
 obj-$(CONFIG_SOC_CAMERA)   += soc_camera/
 
 obj-$(CONFIG_VIDEO_RCAR_DRIF)  += rcar_drif.o
+obj-$(CONFIG_VIDEO_RENESAS_CEU)+= renesas-ceu.o
 obj-$(CONFIG_VIDEO_RENESAS_FCP)+= rcar-fcp.o
 obj-$(CONFIG_VIDEO_RENESAS_FDP1)   += rcar_fdp1.o
 obj-$(CONFIG_VIDEO_RENESAS_JPU)+= rcar_jpu.o
diff --git a/drivers/media/platform/renesas-ceu.c 
b/drivers/media/platform/renesas-ceu.c
new file mode 100644
index 000..1b8f0ef
--- /dev/null
+++ b/drivers/media/platform/renesas-ceu.c
@@ -0,0 +1,1659 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * V4L2 Driver for Renesas Capture Engine Unit (CEU) interface
+ * Copyright (C) 2017-2018 Jacopo Mondi 
+ *
+ * Based on soc-camera driver "soc_camera/sh_mobile_ceu_camera.c"
+ * Copyright (C) 2008 Magnus Damm
+ *
+ * Based on V4L2 Driver for PXA camera host - "pxa_camera.c",
+ * Copyright (C) 2006, Sascha Hauer, Pengutronix
+ * Copyright (C) 2008, Guennadi Liakhovetski 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#define DRIVER_NAME"renesas-ceu"
+
+/* CEU registers offsets and masks. */
+#define CEU_CAPSR  0x00 /* Capture start register  */
+#define CEU_CAPCR  0x04 /* Capture control register*/
+#define CEU_CAMCR  0x08 /* Capture interface control register  */
+#define CEU_CAMOR  0x10 /* Capture interface offset register   */
+#define CEU_CAPWR  0x14 /* Capture interface width register*/
+#define CEU_CAIFR  0x18 /* Capture interface input format register */
+#define CEU_CRCNTR 0x28 /* CEU register control register   */
+#define CEU_CRCMPR 0x2c /* CEU register forcible control register  */
+#define CEU_CFLCR  0x30 /* Capture filter control register */
+#define CEU_CFSZR  0x34 /* Capture filter size clip register   */
+#define CEU_CDWDR  0x38 /* Capture destination width register  */
+#define CEU_CDAYR  0x3c /* Capture data address Y register */
+#define CEU_CDACR  0x40 /* Capture data address C register */
+#define CEU_CFWCR  0x5c /* Firewall operation control register */
+#define CEU_CDOCR  0x64 /* Capture data output control register*/
+#define CEU_CEIER  0x70 /* Capture event interrupt enable register */
+#define CEU_CETCR  0x74 /* Capture event flag clear register   */
+#define CEU_CSTSR  0x7c /* Capture status register */
+#define CEU_CSRTR  0x80 /* Capture software reset register */
+
+/* Data synchronous fetch mode. */
+#define CEU_CAMCR_JPEG BIT(4)
+
+/* Input components ordering: CEU_CAMCR.DTARY field. */
+#define CEU_CAMCR_DTARY_8_UYVY (0x00 << 8)
+#define CEU_CAMCR_DTARY_8_VYUY (0x01 << 8)
+#define CEU_CAMCR_DTARY_8_YUYV (0x02 << 8)
+#define 

[PATCH v6 5/9] v4l: i2c: Copy ov772x soc_camera sensor driver

[Jacopo Mondi]

Copy the soc_camera based driver in v4l2 sensor driver directory.
This commit just copies the original file without modifying it.
No modification to KConfig and Makefile as soc_camera framework
dependencies need to be removed first in next commit.

Signed-off-by: Jacopo Mondi 
Acked-by: Laurent Pinchart 
Acked-by: Hans Verkuil 
---
 drivers/media/i2c/ov772x.c | 1124 
 1 file changed, 1124 insertions(+)
 create mode 100644 drivers/media/i2c/ov772x.c

diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
new file mode 100644
index 000..8063835
--- /dev/null
+++ b/drivers/media/i2c/ov772x.c
@@ -0,0 +1,1124 @@
+/*
+ * ov772x Camera Driver
+ *
+ * Copyright (C) 2008 Renesas Solutions Corp.
+ * Kuninori Morimoto 
+ *
+ * Based on ov7670 and soc_camera_platform driver,
+ *
+ * Copyright 2006-7 Jonathan Corbet 
+ * Copyright (C) 2008 Magnus Damm
+ * Copyright (C) 2008, Guennadi Liakhovetski 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/*
+ * register offset
+ */
+#define GAIN0x00 /* AGC - Gain control gain setting */
+#define BLUE0x01 /* AWB - Blue channel gain setting */
+#define RED 0x02 /* AWB - Red   channel gain setting */
+#define GREEN   0x03 /* AWB - Green channel gain setting */
+#define COM10x04 /* Common control 1 */
+#define BAVG0x05 /* U/B Average Level */
+#define GAVG0x06 /* Y/Gb Average Level */
+#define RAVG0x07 /* V/R Average Level */
+#define AECH0x08 /* Exposure Value - AEC MSBs */
+#define COM20x09 /* Common control 2 */
+#define PID 0x0A /* Product ID Number MSB */
+#define VER 0x0B /* Product ID Number LSB */
+#define COM30x0C /* Common control 3 */
+#define COM40x0D /* Common control 4 */
+#define COM50x0E /* Common control 5 */
+#define COM60x0F /* Common control 6 */
+#define AEC 0x10 /* Exposure Value */
+#define CLKRC   0x11 /* Internal clock */
+#define COM70x12 /* Common control 7 */
+#define COM80x13 /* Common control 8 */
+#define COM90x14 /* Common control 9 */
+#define COM10   0x15 /* Common control 10 */
+#define REG16   0x16 /* Register 16 */
+#define HSTART  0x17 /* Horizontal sensor size */
+#define HSIZE   0x18 /* Horizontal frame (HREF column) end high 8-bit */
+#define VSTART  0x19 /* Vertical frame (row) start high 8-bit */
+#define VSIZE   0x1A /* Vertical sensor size */
+#define PSHFT   0x1B /* Data format - pixel delay select */
+#define MIDH0x1C /* Manufacturer ID byte - high */
+#define MIDL0x1D /* Manufacturer ID byte - low  */
+#define LAEC0x1F /* Fine AEC value */
+#define COM11   0x20 /* Common control 11 */
+#define BDBASE  0x22 /* Banding filter Minimum AEC value */
+#define DBSTEP  0x23 /* Banding filter Maximum Setp */
+#define AEW 0x24 /* AGC/AEC - Stable operating region (upper limit) */
+#define AEB 0x25 /* AGC/AEC - Stable operating region (lower limit) */
+#define VPT 0x26 /* AGC/AEC Fast mode operating region */
+#define REG28   0x28 /* Register 28 */
+#define HOUTSIZE0x29 /* Horizontal data output size MSBs */
+#define EXHCH   0x2A /* Dummy pixel insert MSB */
+#define EXHCL   0x2B /* Dummy pixel insert LSB */
+#define VOUTSIZE0x2C /* Vertical data output size MSBs */
+#define ADVFL   0x2D /* LSB of insert dummy lines in Vertical direction */
+#define ADVFH   0x2E /* MSG of insert dummy lines in Vertical direction */
+#define YAVE0x2F /* Y/G Channel Average value */
+#define LUMHTH  0x30 /* Histogram AEC/AGC Luminance high level threshold */
+#define LUMLTH  0x31 /* Histogram AEC/AGC Luminance low  level threshold */
+#define HREF0x32 /* Image start and size control */
+#define DM_LNL  0x33 /* Dummy line low  8 bits */
+#define DM_LNH  0x34 /* Dummy line high 8 bits */
+#define ADOFF_B 0x35 /* AD offset compensation value for B  channel */
+#define ADOFF_R 0x36 /* AD offset compensation value for R  channel */
+#define ADOFF_GB0x37 /* AD offset compensation value for Gb channel */
+#define ADOFF_GR0x38 /* AD offset compensation value for Gr channel */
+#define OFF_B   0x39 /* Analog process B  channel offset value */
+#define OFF_R   0x3A /* Analog process R  channel offset value */
+#define OFF_GB  0x3B /* Analog process Gb channel offset value */
+#define OFF_GR  0x3C /* Analog process Gr 

[PATCH v6 6/9] media: i2c: ov772x: Remove soc_camera dependencies

[Jacopo Mondi]

Remove soc_camera framework dependencies from ov772x sensor driver.
- Handle clock and gpios
- Register async subdevice
- Remove soc_camera specific g/s_mbus_config operations
- Change image format colorspace from JPEG to SRGB as the two use the
  same colorspace information but JPEG makes assumptions on color
  components quantization that do not apply to the sensor
- Remove sizes crop from get_selection as driver can't scale
- Add kernel doc to driver interface header file
- Adjust build system

This commit does not remove the original soc_camera based driver as long
as other platforms depends on soc_camera-based CEU driver.

Signed-off-by: Jacopo Mondi 
Reviewed-by: Laurent Pinchart 
---
 drivers/media/i2c/Kconfig  |  11 +++
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/ov772x.c | 169 ++---
 include/media/i2c/ov772x.h |   6 +-
 4 files changed, 130 insertions(+), 57 deletions(-)

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index cb5d7ff..a61d7f4 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -645,6 +645,17 @@ config VIDEO_OV5670
  To compile this driver as a module, choose M here: the
  module will be called ov5670.
 
+config VIDEO_OV772X
+   tristate "OmniVision OV772x sensor support"
+   depends on I2C && VIDEO_V4L2
+   depends on MEDIA_CAMERA_SUPPORT
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV772x camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov772x.
+
 config VIDEO_OV7640
tristate "OmniVision OV7640 sensor support"
depends on I2C && VIDEO_V4L2
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 548a9ef..fb99293 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -66,6 +66,7 @@ obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
 obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
 obj-$(CONFIG_VIDEO_OV5670) += ov5670.o
 obj-$(CONFIG_VIDEO_OV6650) += ov6650.o
+obj-$(CONFIG_VIDEO_OV772X) += ov772x.o
 obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
 obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
 obj-$(CONFIG_VIDEO_OV9650) += ov9650.o
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 8063835..912b1b9 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -1,6 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * ov772x Camera Driver
  *
+ * Copyright (C) 2017 Jacopo Mondi 
+ *
  * Copyright (C) 2008 Renesas Solutions Corp.
  * Kuninori Morimoto 
  *
@@ -9,27 +12,25 @@
  * Copyright 2006-7 Jonathan Corbet 
  * Copyright (C) 2008 Magnus Damm
  * Copyright (C) 2008, Guennadi Liakhovetski 
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
  */
 
+#include 
+#include 
+#include 
+#include 
 #include 
 #include 
 #include 
-#include 
 #include 
-#include 
 #include 
 #include 
 
 #include 
-#include 
-#include 
+
 #include 
-#include 
+#include 
 #include 
+#include 
 
 /*
  * register offset
@@ -393,8 +394,10 @@ struct ov772x_win_size {
 struct ov772x_priv {
struct v4l2_subdevsubdev;
struct v4l2_ctrl_handler  hdl;
-   struct v4l2_clk  *clk;
+   struct clk   *clk;
struct ov772x_camera_info*info;
+   struct gpio_desc *pwdn_gpio;
+   struct gpio_desc *rstb_gpio;
const struct ov772x_color_format *cfmt;
const struct ov772x_win_size *win;
unsigned shortflag_vflip:1;
@@ -409,7 +412,7 @@ struct ov772x_priv {
 static const struct ov772x_color_format ov772x_cfmts[] = {
{
.code   = MEDIA_BUS_FMT_YUYV8_2X8,
-   .colorspace = V4L2_COLORSPACE_JPEG,
+   .colorspace = V4L2_COLORSPACE_SRGB,
.dsp3   = 0x0,
.dsp4   = DSP_OFMT_YUV,
.com3   = SWAP_YUV,
@@ -417,7 +420,7 @@ static const struct ov772x_color_format ov772x_cfmts[] = {
},
{
.code   = MEDIA_BUS_FMT_YVYU8_2X8,
-   .colorspace = V4L2_COLORSPACE_JPEG,
+   .colorspace = V4L2_COLORSPACE_SRGB,
.dsp3   = UV_ON,
.dsp4   = DSP_OFMT_YUV,
.com3   = SWAP_YUV,
@@ -425,7 +428,7 @@ static const struct ov772x_color_format ov772x_cfmts[] = {
},
{
.code   = MEDIA_BUS_FMT_UYVY8_2X8,
-   .colorspace = V4L2_COLORSPACE_JPEG,
+   .colorspace = 

[PATCH v6 4/9] ARM: dts: r7s72100: Add Capture Engine Unit (CEU)

[Jacopo Mondi]

Add Capture Engine Unit (CEU) node to device tree.

Signed-off-by: Jacopo Mondi 
Reviewed-by: Geert Uytterhoeven 
Reviewed-by: Laurent Pinchart 
Acked-by: Hans Verkuil 
---
 arch/arm/boot/dts/r7s72100.dtsi | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
index ab9645a..5fe62f9 100644
--- a/arch/arm/boot/dts/r7s72100.dtsi
+++ b/arch/arm/boot/dts/r7s72100.dtsi
@@ -135,9 +135,9 @@
#clock-cells = <1>;
compatible = "renesas,r7s72100-mstp-clocks", 
"renesas,cpg-mstp-clocks";
reg = <0xfcfe042c 4>;
-   clocks = <_clk>;
-   clock-indices = ;
-   clock-output-names = "rtc";
+   clocks = <_clk>, <_clk>;
+   clock-indices = ;
+   clock-output-names = "ceu", "rtc";
};
 
mstp7_clks: mstp7_clks@fcfe0430 {
@@ -667,4 +667,13 @@
power-domains = <_clocks>;
status = "disabled";
};
+
+   ceu: ceu@e821 {
+   reg = <0xe821 0x3000>;
+   compatible = "renesas,r7s72100-ceu";
+   interrupts = ;
+   clocks = <_clks R7S72100_CLK_CEU>;
+   power-domains = <_clocks>;
+   status = "disabled";
+   };
 };
-- 
2.7.4

[PATCH v6 7/9] v4l: i2c: Copy tw9910 soc_camera sensor driver

[Jacopo Mondi]

Copy the soc_camera based driver in v4l2 sensor driver directory.
This commit just copies the original file without modifying it.
No modification to KConfig and Makefile as soc_camera framework
dependencies need to be removed first in next commit.

Signed-off-by: Jacopo Mondi 
Acked-by: Laurent Pinchart 
Acked-by: Hans Verkuil 
---
 drivers/media/i2c/tw9910.c | 999 +
 1 file changed, 999 insertions(+)
 create mode 100644 drivers/media/i2c/tw9910.c

diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
new file mode 100644
index 000..bdb5e0a
--- /dev/null
+++ b/drivers/media/i2c/tw9910.c
@@ -0,0 +1,999 @@
+/*
+ * tw9910 Video Driver
+ *
+ * Copyright (C) 2008 Renesas Solutions Corp.
+ * Kuninori Morimoto 
+ *
+ * Based on ov772x driver,
+ *
+ * Copyright (C) 2008 Kuninori Morimoto 
+ * Copyright 2006-7 Jonathan Corbet 
+ * Copyright (C) 2008 Magnus Damm
+ * Copyright (C) 2008, Guennadi Liakhovetski 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+#define GET_ID(val)  ((val & 0xF8) >> 3)
+#define GET_REV(val) (val & 0x07)
+
+/*
+ * register offset
+ */
+#define ID 0x00 /* Product ID Code Register */
+#define STATUS10x01 /* Chip Status Register I */
+#define INFORM 0x02 /* Input Format */
+#define OPFORM 0x03 /* Output Format Control Register */
+#define DLYCTR 0x04 /* Hysteresis and HSYNC Delay Control */
+#define OUTCTR10x05 /* Output Control I */
+#define ACNTL1 0x06 /* Analog Control Register 1 */
+#define CROP_HI0x07 /* Cropping Register, High */
+#define VDELAY_LO  0x08 /* Vertical Delay Register, Low */
+#define VACTIVE_LO 0x09 /* Vertical Active Register, Low */
+#define HDELAY_LO  0x0A /* Horizontal Delay Register, Low */
+#define HACTIVE_LO 0x0B /* Horizontal Active Register, Low */
+#define CNTRL1 0x0C /* Control Register I */
+#define VSCALE_LO  0x0D /* Vertical Scaling Register, Low */
+#define SCALE_HI   0x0E /* Scaling Register, High */
+#define HSCALE_LO  0x0F /* Horizontal Scaling Register, Low */
+#define BRIGHT 0x10 /* BRIGHTNESS Control Register */
+#define CONTRAST   0x11 /* CONTRAST Control Register */
+#define SHARPNESS  0x12 /* SHARPNESS Control Register I */
+#define SAT_U  0x13 /* Chroma (U) Gain Register */
+#define SAT_V  0x14 /* Chroma (V) Gain Register */
+#define HUE0x15 /* Hue Control Register */
+#define CORING10x17
+#define CORING20x18 /* Coring and IF compensation */
+#define VBICNTL0x19 /* VBI Control Register */
+#define ACNTL2 0x1A /* Analog Control 2 */
+#define OUTCTR20x1B /* Output Control 2 */
+#define SDT0x1C /* Standard Selection */
+#define SDTR   0x1D /* Standard Recognition */
+#define TEST   0x1F /* Test Control Register */
+#define CLMPG  0x20 /* Clamping Gain */
+#define IAGC   0x21 /* Individual AGC Gain */
+#define AGCGAIN0x22 /* AGC Gain */
+#define PEAKWT 0x23 /* White Peak Threshold */
+#define CLMPL  0x24 /* Clamp level */
+#define SYNCT  0x25 /* Sync Amplitude */
+#define MISSCNT0x26 /* Sync Miss Count Register */
+#define PCLAMP 0x27 /* Clamp Position Register */
+#define VCNTL1 0x28 /* Vertical Control I */
+#define VCNTL2 0x29 /* Vertical Control II */
+#define CKILL  0x2A /* Color Killer Level Control */
+#define COMB   0x2B /* Comb Filter Control */
+#define LDLY   0x2C /* Luma Delay and H Filter Control */
+#define MISC1  0x2D /* Miscellaneous Control I */
+#define LOOP   0x2E /* LOOP Control Register */
+#define MISC2  0x2F /* Miscellaneous Control II */
+#define MVSN   0x30 /* Macrovision Detection */
+#define STATUS20x31 /* Chip STATUS II */
+#define HFREF  0x32 /* H monitor */
+#define CLMD   0x33 /* CLAMP MODE */
+#define IDCNTL 0x34 /* ID Detection Control */
+#define CLCNTL10x35 /* Clamp Control I */
+#define ANAPLLCTL  0x4C
+#define VBIMIN 0x4D
+#define HSLOWCTL   0x4E
+#define WSS3   0x4F
+#define FILLDATA   0x50
+#define SDID   0x51
+#define DID0x52
+#define WSS1   0x53
+#define WSS2   0x54
+#define VVBI   0x55
+#define LCTL6  0x56
+#define LCTL7  

[PATCH v6 8/9] media: i2c: tw9910: Remove soc_camera dependencies

[Jacopo Mondi]

Remove soc_camera framework dependencies from tw9910 sensor driver.
- Handle clock and gpios
- Register async subdevice
- Remove soc_camera specific g/s_mbus_config operations
- Add kernel doc to driver interface header file
- Adjust build system

This commit does not remove the original soc_camera based driver as long
as other platforms depends on soc_camera-based CEU driver.

Signed-off-by: Jacopo Mondi 
Reviewed-by: Laurent Pinchart 
Acked-by: Hans Verkuil 
---
 drivers/media/i2c/Kconfig  |   9 +++
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/tw9910.c | 162 -
 include/media/i2c/tw9910.h |   9 +++
 4 files changed, 120 insertions(+), 61 deletions(-)

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index a61d7f4..804a1bf 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -423,6 +423,15 @@ config VIDEO_TW9906
  To compile this driver as a module, choose M here: the
  module will be called tw9906.
 
+config VIDEO_TW9910
+   tristate "Techwell TW9910 video decoder"
+   depends on VIDEO_V4L2 && I2C
+   ---help---
+ Support for Techwell TW9910 NTSC/PAL/SECAM video decoder.
+
+ To compile this driver as a module, choose M here: the
+ module will be called tw9910.
+
 config VIDEO_VPX3220
tristate "vpx3220a, vpx3216b & vpx3214c video decoders"
depends on VIDEO_V4L2 && I2C
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index fb99293..e26544f 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_VIDEO_TVP7002) += tvp7002.o
 obj-$(CONFIG_VIDEO_TW2804) += tw2804.o
 obj-$(CONFIG_VIDEO_TW9903) += tw9903.o
 obj-$(CONFIG_VIDEO_TW9906) += tw9906.o
+obj-$(CONFIG_VIDEO_TW9910) += tw9910.o
 obj-$(CONFIG_VIDEO_CS3308) += cs3308.o
 obj-$(CONFIG_VIDEO_CS5345) += cs5345.o
 obj-$(CONFIG_VIDEO_CS53L32A) += cs53l32a.o
diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index bdb5e0a..96792df 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -1,6 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * tw9910 Video Driver
  *
+ * Copyright (C) 2017 Jacopo Mondi 
+ *
  * Copyright (C) 2008 Renesas Solutions Corp.
  * Kuninori Morimoto 
  *
@@ -10,12 +13,10 @@
  * Copyright 2006-7 Jonathan Corbet 
  * Copyright (C) 2008 Magnus Damm
  * Copyright (C) 2008, Guennadi Liakhovetski 
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
  */
 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -25,9 +26,7 @@
 #include 
 #include 
 
-#include 
 #include 
-#include 
 #include 
 
 #define GET_ID(val)  ((val & 0xF8) >> 3)
@@ -228,8 +227,10 @@ struct tw9910_scale_ctrl {
 
 struct tw9910_priv {
struct v4l2_subdev  subdev;
-   struct v4l2_clk *clk;
+   struct clk  *clk;
struct tw9910_video_info*info;
+   struct gpio_desc*pdn_gpio;
+   struct gpio_desc*rstb_gpio;
const struct tw9910_scale_ctrl  *scale;
v4l2_std_id norm;
u32 revision;
@@ -582,13 +583,66 @@ static int tw9910_s_register(struct v4l2_subdev *sd,
 }
 #endif
 
+static int tw9910_power_on(struct tw9910_priv *priv)
+{
+   struct i2c_client *client = v4l2_get_subdevdata(>subdev);
+   int ret;
+
+   if (priv->clk) {
+   ret = clk_prepare_enable(priv->clk);
+   if (ret)
+   return ret;
+   }
+
+   if (priv->pdn_gpio) {
+   gpiod_set_value(priv->pdn_gpio, 0);
+   usleep_range(500, 1000);
+   }
+
+   /*
+* FIXME: The reset signal is connected to a shared GPIO on some
+* platforms (namely the SuperH Migo-R). Until a framework becomes
+* available to handle this cleanly, request the GPIO temporarily
+* to avoid conflicts.
+*/
+   priv->rstb_gpio = gpiod_get_optional(>dev, "rstb",
+GPIOD_OUT_LOW);
+   if (IS_ERR(priv->rstb_gpio)) {
+   dev_info(>dev, "Unable to get GPIO \"rstb\"");
+   return PTR_ERR(priv->rstb_gpio);
+   }
+
+   if (priv->rstb_gpio) {
+   gpiod_set_value(priv->rstb_gpio, 1);
+   usleep_range(500, 1000);
+   gpiod_set_value(priv->rstb_gpio, 0);
+   usleep_range(500, 1000);
+
+   gpiod_put(priv->rstb_gpio);
+   }
+
+   return 0;
+}
+
+static int tw9910_power_off(struct tw9910_priv *priv)
+{
+   

Re: [PATCH v2 2/2] drm: rcar-du: lvds: Fix LVDS startup on R-Car gen2

[Sergei Shtylyov]

On 01/16/2018 06:46 PM, Laurent Pinchart wrote:


From: Sergei Shtylyov 

According to the latest revision 2.00 of the R-Car gen2 manual, the LVDS
must be enabled and the bias crcuit enabled after the LVDS I/O pins are


   Oops, this needs fixing (note the typo!). Could you please change this 
passage to:


and the bias circuit must be enabled after the LVDS I/O pins are

?


enabled, not before. Fix the gen2 LVDS startup sequence accordingly.

While at it, also fix the comment preceding the first LVDCR0 write that
still talks about hardcoding the LVDS mode 0.


Please do this in a separate commit then...


The reason I added it here is that I think we don't need patch 1/2 from this
series, and I found a bit overkill to split a comment fix to a separate patch
when we have a patch that touches the code around the comment.


   OK, you're the maintainer of this driver, you know better. :-)


Fixes: 90374b5c25c9 ("drm/rcar-du: Add internal LVDS encoder support")


You forgot to specify the other commit this one fixes -- I mean the comment
fix.


Do we need to for a comment update ? It doesn't affect fix the behaviour of
the driver or device, and I'd thus prefer to avoid giving the wrong impression
that this patch fixes an bug introduced in a previous commit, otherwise it
might end up being backported unnecessarily.


   OK, no dire need to backport indeed...


Signed-off-by: Sergei Shtylyov 
Reviewed-by: Laurent Pinchart 
[Set the mode and input at the same time as the BEN and LVEN bits]
Tested-by: Laurent Pinchart 
Signed-off-by: Laurent Pinchart

---

   drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 14 +++---
   1 file changed, 7 insertions(+), 7 deletions(-)

Hi Sergei,

For your convenience (and if you agree with bundling mode setup with the
first write as explained in my review of patch 1/2), here's the updated
version of patch 2/2 that I have taken in my development branch. If
you're fine with it I'll keep it, otherwise we can continue the review
discussion.


As I said, I don't know how to interpret the note 3 in either manual.


   Moreover, it seems to me that the notes don't match the start-up procedure 
anymore...



As explained in my latest reply to patch 1/2, my understanding is that the
parameters can be programmed at any time before step 6. The fact that the
current code works seems to confirm that interpretation. We could ask Renesas
for a confirmation if you want.


   Would be good to ask, indeed.

MBR, Sergei

Re: [PATCH v5 3/9] v4l: platform: Add Renesas CEU driver

[jacopo mondi]

Hi Hans,

On Tue, Jan 16, 2018 at 10:46:42AM +0100, Hans Verkuil wrote:
> Hi Jacopo,
>
> Sorry for the late review, but here is finally is.
>
> BTW, can you provide the v4l2-compliance output (ideally with the -f option)
> in the cover letter for v6?

Sure, it was attacched to v3 I guess, since then it has not changed.
I will include that in v6 cover letter.

> On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> > Add driver for Renesas Capture Engine Unit (CEU).
> >
> > The CEU interface supports capturing 'data' (YUV422) and 'images'
> > (NV[12|21|16|61]).
> >
> > This driver aims to replace the soc_camera-based sh_mobile_ceu one.
> >
> > Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ
> > platform GR-Peach.
> >
> > Tested with ov7725 camera sensor on SH4 platform Migo-R.
> >
> > Signed-off-by: Jacopo Mondi 
> > Reviewed-by: Laurent Pinchart 
> > ---
> >  drivers/media/platform/Kconfig   |9 +
> >  drivers/media/platform/Makefile  |1 +
> >  drivers/media/platform/renesas-ceu.c | 1648 
> > ++
> >  3 files changed, 1658 insertions(+)
> >  create mode 100644 drivers/media/platform/renesas-ceu.c
> >
> > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> > index fd0c998..fe7bd26 100644
> > --- a/drivers/media/platform/Kconfig
> > +++ b/drivers/media/platform/Kconfig
> > @@ -144,6 +144,15 @@ config VIDEO_STM32_DCMI
> >   To compile this driver as a module, choose M here: the module
> >   will be called stm32-dcmi.
> >
> > +config VIDEO_RENESAS_CEU
> > +   tristate "Renesas Capture Engine Unit (CEU) driver"
> > +   depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA
> > +   depends on ARCH_SHMOBILE || ARCH_R7S72100 || COMPILE_TEST
> > +   select VIDEOBUF2_DMA_CONTIG
> > +   select V4L2_FWNODE
> > +   ---help---
> > + This is a v4l2 driver for the Renesas CEU Interface
> > +
> >  source "drivers/media/platform/soc_camera/Kconfig"
> >  source "drivers/media/platform/exynos4-is/Kconfig"
> >  source "drivers/media/platform/am437x/Kconfig"
> > diff --git a/drivers/media/platform/Makefile 
> > b/drivers/media/platform/Makefile
> > index 003b0bb..6580a6b 100644
> > --- a/drivers/media/platform/Makefile
> > +++ b/drivers/media/platform/Makefile
> > @@ -62,6 +62,7 @@ obj-$(CONFIG_VIDEO_SH_VOU)+= sh_vou.o
> >  obj-$(CONFIG_SOC_CAMERA)   += soc_camera/
> >
> >  obj-$(CONFIG_VIDEO_RCAR_DRIF)  += rcar_drif.o
> > +obj-$(CONFIG_VIDEO_RENESAS_CEU)+= renesas-ceu.o
> >  obj-$(CONFIG_VIDEO_RENESAS_FCP)+= rcar-fcp.o
> >  obj-$(CONFIG_VIDEO_RENESAS_FDP1)   += rcar_fdp1.o
> >  obj-$(CONFIG_VIDEO_RENESAS_JPU)+= rcar_jpu.o
> > diff --git a/drivers/media/platform/renesas-ceu.c 
> > b/drivers/media/platform/renesas-ceu.c
> > new file mode 100644
> > index 000..ccca838
> > --- /dev/null
> > +++ b/drivers/media/platform/renesas-ceu.c
>
> 
>
> > +/*
> > + * ceu_vb2_setup() - is called to check whether the driver can accept the
> > + *  requested number of buffers and to fill in plane sizes
> > + *  for the current frame format, if required.
> > + */
> > +static int ceu_vb2_setup(struct vb2_queue *vq, unsigned int *count,
> > +unsigned int *num_planes, unsigned int sizes[],
> > +struct device *alloc_devs[])
> > +{
> > +   struct ceu_device *ceudev = vb2_get_drv_priv(vq);
> > +   struct v4l2_pix_format_mplane *pix = >v4l2_pix;
> > +   unsigned int i;
> > +
> > +   if (!*count)
> > +   *count = 2;
>
> Don't do this. Instead set the min_buffers_needed field to 2 in the vb2_queue
> struct.
>

I guess setting min_buffers_needed makes this check not required. Will
drop.

> > +
> > +   /* num_planes is set: just check plane sizes. */
> > +   if (*num_planes) {
> > +   for (i = 0; i < pix->num_planes; i++)
> > +   if (sizes[i] < pix->plane_fmt[i].sizeimage)
> > +   return -EINVAL;
> > +
> > +   return 0;
> > +   }
> > +
> > +   /* num_planes not set: called from REQBUFS, just set plane sizes. */
> > +   *num_planes = pix->num_planes;
> > +   for (i = 0; i < pix->num_planes; i++)
> > +   sizes[i] = pix->plane_fmt[i].sizeimage;
> > +
> > +   return 0;
> > +}
> > +
> > +static void ceu_vb2_queue(struct vb2_buffer *vb)
> > +{
> > +   struct ceu_device *ceudev = vb2_get_drv_priv(vb->vb2_queue);
> > +   struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> > +   struct v4l2_pix_format_mplane *pix = >v4l2_pix;
> > +   struct ceu_buffer *buf = vb2_to_ceu(vbuf);
> > +   unsigned long irqflags;
> > +   unsigned int i;
> > +
> > +   for (i = 0; i < pix->num_planes; i++) {
> > +   if (vb2_plane_size(vb, i) < pix->plane_fmt[i].sizeimage) {
> > +   vb2_buffer_done(vb, VB2_BUF_STATE_ERROR);
> > +   return;
> > +   }
> > +
> > +   

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Rob Herring]

On Tue, Jan 16, 2018 at 10:32 AM, Laurent Pinchart
 wrote:
> Hi Rob,
>
> On Tuesday, 16 January 2018 16:35:26 EET Rob Herring wrote:
>> On Mon, Jan 15, 2018 at 5:46 PM, Frank Rowand wrote:
>> > On 01/15/18 12:29, Laurent Pinchart wrote:
>> >> On Monday, 15 January 2018 22:12:33 EET Frank Rowand wrote:
>> >>> On 01/15/18 11:22, Laurent Pinchart wrote:
>>  On Monday, 15 January 2018 21:12:44 EET Frank Rowand wrote:
>> > On 01/15/18 09:09, Rob Herring wrote:
>> >> On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart wrote:
>> >>> The internal LVDS encoders now have their own DT bindings. Before
>> >>> switching the driver infrastructure to those new bindings, implement
>> >>> backward-compatibility through live DT patching.

[...]

>> >> How would you like to proceed ? I can try the manual approach again but
>> >> need information about how I could cleanly implement phandle allocation.
>> >> I will likely then run into other issues for which I might need help.
>> >
>> > Here are my first thoughts, based on 4.15-rc7:
>> >
>> > Question to Rob and Frank: should of_attach_node() be called directly, or
>> > should it be called indirectly by creating a change set that adds the
>> > node?
>> >
>> > Frank's off the cuff answer (but would like to think more about it): since
>> > the driver is modifying its own devicetree data, and thus no other entity
>> > needs to be notified about the changes, there is no need to add the
>> > complexity of using a change set.
>>
>> There's exactly 2 users outside of the DT core. That's generally a
>> sign of an API I'd like to make private.
>>
>> > The following is how of_attach_node() could be modified to dynamically
>> > create a phandle on request.
>>
>> How would this work for all the phandle references that need to be fixed up?
>
> As I know which properties contain a phandle that needs to be fixed up, my
> plan is to update those properties manually with the value of the newly
> allocated phandle.

That sounds like more low level, internal detail mucking with than
this current patch.

> What I need here is the ability to insert the following structure in the
> device tree:
>
> lvds@feb9 {
>compatible = "renesas,r8a7796-lvds"; (*)
>reg = <0 0xfeb9 0 0x14>; (*)
>clocks = < CPG_MOD 727>; (*)

I'm still of the opinion that all of this should be in a per SoC overlay.

>
>ports {
> #address-cells = <1>;
> #size-cells = <0>;
>
> port@0 {
> reg = <0>;
> lvds0_in: endpoint {
> remote-endpoint = <_out_lvds0>; (*)
> };
> };
> port@1 {
> reg = <1>;
> lvds0_out: endpoint {
> remote-endpoint = <_in>; (*)

Then do the fixup of just the remote-endpoint properties. While it
would be nice to say overlays are completely static, I'm guessing
that's not going to be the case. After all, we pretty much have
overlays because DT being static has limitations.

> };
> };
> };
> };
>
> with the node unit address and all properties marked with a (*) computed at
> runtime based on information extract from the device tree. Additionally I need
> phandles for the lvds0_in and lvds0_out nodes, and set the value of two
> properties in the tree with those phandles.
>
> I've used overlays as that was the only way I found to allocate phandles, but
> any other API will work for me as well.

I don't think drivers mucking with phandles directly to avoid another
overlay user is an improvement.

Rob

Re: [PATCH v2] sata: sata_rcar: Reset SATA PHY when Salvator-X board resumes

[Sergei Shtylyov]

On 01/16/2018 07:25 PM, Sergei Shtylyov wrote:


From: Khiem Nguyen 

Because power of Salvator-X board is cut off in suspend,
it needs to reset SATA PHY state in resume.
Otherwise, SATA partition could not be accessed anymore.

Signed-off-by: Khiem Nguyen 
Signed-off-by: Hien Dang 
[reinit phy in sata_rcar_resume() function on R-Car Gen3 only]
[fixed the prefix for the subject]
Signed-off-by: Yoshihiro Kaneko 
---

This patch is based on the for-next branch of libata tree.

v2 [Yoshihiro Kaneko]
* reinit phy on R-Car Gen3 only as suggested by Geert Uytterhoeven
* use "sata_rcar" prefix for the subject as suggested by Sergei Shtylyov
  drivers/ata/sata_rcar.c | 38 --
  1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
index 80ee2f2..4adc0d6 100644
--- a/drivers/ata/sata_rcar.c
+++ b/drivers/ata/sata_rcar.c

[...]

@@ -977,11 +979,43 @@ static int sata_rcar_resume(struct device *dev)
  struct sata_rcar_priv *priv = host->private_data;
  void __iomem *base = priv->base;
  int ret;
+u32 val;
  ret = clk_prepare_enable(priv->clk);
  if (ret)
  return ret;
+/* reinit phy on R-Car Gen3 only */
+switch (priv->type) {
+case RCAR_GEN1_SATA:
+case RCAR_GEN2_SATA:
+case RCAR_R8A7790_ES1_SATA:
+break;
+case RCAR_GEN3_SATA:
+sata_rcar_gen2_phy_init(priv);
+break;
+default:
+dev_warn(host->dev, "SATA phy is not initialized\n");
+break;
+}
+
+/* SATA-IP reset state */
+val = ioread32(base + ATAPI_CONTROL1_REG);
+val |= ATAPI_CONTROL1_RESET;
+iowrite32(val, base + ATAPI_CONTROL1_REG);
+
+/* ISM mode, PRD mode, DTEND flag at bit 0 */
+val = ioread32(base + ATAPI_CONTROL1_REG);
+val |= ATAPI_CONTROL1_ISM;
+val |= ATAPI_CONTROL1_DESE;
+val |= ATAPI_CONTROL1_DTA32M;
+iowrite32(val, base + ATAPI_CONTROL1_REG);
+
+/* Release the SATA-IP from the reset state */
+val = ioread32(base + ATAPI_CONTROL1_REG);
+val &= ~ATAPI_CONTROL1_RESET;
+iowrite32(val, base + ATAPI_CONTROL1_REG);
+


As it stands, we could surely do the same with a much shorter code:

 if (priv->type == RCAR_GEN3_SATA) {
   sata_rcar_init_controller(host);
 } else {
  /* ack and mask */
iowrite32(0, base + SATAINTSTAT_REG);
   iowrite32(0x7ff, base + SATAINTMASK_REG);
 }

But do we really need this reset/init sequence on gen1/2 chips?


Oops, the above suggestion is what I think is a correct approach, it 
can't just replace your code...


MBR, Sergei

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Laurent Pinchart]

Hi Rob,

On Tuesday, 16 January 2018 16:35:26 EET Rob Herring wrote:
> On Mon, Jan 15, 2018 at 5:46 PM, Frank Rowand wrote:
> > On 01/15/18 12:29, Laurent Pinchart wrote:
> >> On Monday, 15 January 2018 22:12:33 EET Frank Rowand wrote:
> >>> On 01/15/18 11:22, Laurent Pinchart wrote:
>  On Monday, 15 January 2018 21:12:44 EET Frank Rowand wrote:
> > On 01/15/18 09:09, Rob Herring wrote:
> >> On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart wrote:
> >>> The internal LVDS encoders now have their own DT bindings. Before
> >>> switching the driver infrastructure to those new bindings, implement
> >>> backward-compatibility through live DT patching.
> >> 
> >> Uhh, we just got rid of TI's patching and now adding this one. I
> >> guess
> >>> 
> >>> Let me first answer the question that you ask later.  You ask "Can we
> >>> work on this together to find a solution that would suit us both ?"
> >>> 
> >>> My answer to that is emphatically YES.  I will definitely work with you
> >>> to try to find a good solution.
> >> 
> >> \o/
> >> 
> > Please no.  What we just got rid of was making it difficult for me to
> > make changes to the overlay infrastructure.  There are issues with
> > overlays that need to be fixed before overlays become really usable.
> > I am about to propose the next change, which involves removing a
> > chunk of code that I will not be able to remove if this patch is
> > accepted (the proposal is awaiting me collecting some data about
> > the impact of the change, which I expect to complete this week).
> >>> 
> >>> I should have thought just a little bit more before I hit send.  The
> >>> situation is even worse than I wrote.  One of the next steps (in
> >>> addition to what I wrote about above) is to change the overlay apply
> >>> function to accept a flattened device tree (FDT), not an expanded
> >>> device tree.  In this changed model, the unflattened overlay is
> >>> not available to be modified before it is applied.
> >> 
> >> That makes sense if we consider overlays to be immutable objects that we
> >> apply without offering a way to modify them. I won't challenge that API
> >> decision, as my use of an overlay here is a bit of a hack indeed.
> >> 
> >>> It is important for the devicetree infrastructure to have ownership
> >>> of the FDT that is used to create the unflattened tree.  (Notice
> >>> that in the proposed patch, rcar_du_of_get_overlay() follows the
> >>> TI example of doing a kmemdup of the blob (FDT), then uses the
> >>> copy for the unflatten.  The kmemdup() in this case is to create
> >>> a persistent copy of the FDT.)  The driver having ownership of
> >>> this copy, and having the ability to free it is one of the many
> >>> problems with the current overlay implementation.
> >> 
> >> Yes, that's something I've identified as well. Lots of work has been done
> >> to clean up the OF core and we're clearly not done yet. I'm happy to see
> >> all the improvements you're working on.
> >> 
> > Can you please handle both the old and new bindings through driver
> > code instead?
>  
>  I could, but it would be pointless. The point here is to allow cleanups
>  in the driver. The LVDS encoder handling code is very intrusive in its
>  current form and I need to get rid of it. There would be zero point in
>  moving to the new infrastructure, as the main point is to get rid of
>  the old code which prevents moving forward. As a consequence that would
>  block new boards from receiving proper upstream support. An easy option
>  is to break backward compatibility. I'm personally fine with that, but
>  I assume other people would complain :-)
>  
>  I can, on the other hand, work with you to see how live DT patching
>  could be implemented in this driver without blocking your code. When
>  developing this patch series I start by patching the device tree
>  manually without relying on overlays at all, but got blocked by the
>  fact that I need to allocate phandles for new nodes I create. If there
>  was an API to allocate an unused phandle I could avoid using the
>  overlay infrastructure at all. Or there could be other
> >>> 
> >>> It seems reasonable to provide a way to autogenerate a phandle (if
> >>> requested) by the devicetree code that creates a new node.  Were you
> >>> using a function from drivers/of/dynamic.c to create the node?
> >> 
> >> Not to allocate the node, no. I allocated the device_node structure
> >> manually with kzalloc(), and inserted it in the device tree with
> >> of_attach_node(). Is that the right approach ? I haven't been able to
> >> test the code as I stopped when I realized I couldn't allocate phandles.
> >> 
>  options I'm not thinking of as I don't know what the changes you're
>  working on are. Can we work on this together to find a solution that
>  would suit us both ?
> >>> 
> >>> Again, yes, I would be 

Re: [PATCH v2] sata: sata_rcar: Reset SATA PHY when Salvator-X board resumes

[Sergei Shtylyov]

On 01/16/2018 05:39 PM, Yoshihiro Kaneko wrote:


From: Khiem Nguyen 

Because power of Salvator-X board is cut off in suspend,
it needs to reset SATA PHY state in resume.
Otherwise, SATA partition could not be accessed anymore.

Signed-off-by: Khiem Nguyen 
Signed-off-by: Hien Dang 
[reinit phy in sata_rcar_resume() function on R-Car Gen3 only]
[fixed the prefix for the subject]
Signed-off-by: Yoshihiro Kaneko 
---

This patch is based on the for-next branch of libata tree.

v2 [Yoshihiro Kaneko]
* reinit phy on R-Car Gen3 only as suggested by Geert Uytterhoeven
* use "sata_rcar" prefix for the subject as suggested by Sergei Shtylyov


  And I don't think you need the redundant "sata: " prefix either...

MBR, Sergei

Re: [PATCH v2] sata: sata_rcar: Reset SATA PHY when Salvator-X board resumes

[Sergei Shtylyov]

Hello!

On 01/16/2018 05:39 PM, Yoshihiro Kaneko wrote:


From: Khiem Nguyen 

Because power of Salvator-X board is cut off in suspend,
it needs to reset SATA PHY state in resume.
Otherwise, SATA partition could not be accessed anymore.

Signed-off-by: Khiem Nguyen 
Signed-off-by: Hien Dang 
[reinit phy in sata_rcar_resume() function on R-Car Gen3 only]
[fixed the prefix for the subject]
Signed-off-by: Yoshihiro Kaneko 
---

This patch is based on the for-next branch of libata tree.

v2 [Yoshihiro Kaneko]
* reinit phy on R-Car Gen3 only as suggested by Geert Uytterhoeven
* use "sata_rcar" prefix for the subject as suggested by Sergei Shtylyov
  drivers/ata/sata_rcar.c | 38 --
  1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
index 80ee2f2..4adc0d6 100644
--- a/drivers/ata/sata_rcar.c
+++ b/drivers/ata/sata_rcar.c

[...]

@@ -977,11 +979,43 @@ static int sata_rcar_resume(struct device *dev)
struct sata_rcar_priv *priv = host->private_data;
void __iomem *base = priv->base;
int ret;
+   u32 val;
  
  	ret = clk_prepare_enable(priv->clk);

if (ret)
return ret;
  
+	/* reinit phy on R-Car Gen3 only */

+   switch (priv->type) {
+   case RCAR_GEN1_SATA:
+   case RCAR_GEN2_SATA:
+   case RCAR_R8A7790_ES1_SATA:
+   break;
+   case RCAR_GEN3_SATA:
+   sata_rcar_gen2_phy_init(priv);
+   break;
+   default:
+   dev_warn(host->dev, "SATA phy is not initialized\n");
+   break;
+   }
+
+   /* SATA-IP reset state */
+   val = ioread32(base + ATAPI_CONTROL1_REG);
+   val |= ATAPI_CONTROL1_RESET;
+   iowrite32(val, base + ATAPI_CONTROL1_REG);
+
+   /* ISM mode, PRD mode, DTEND flag at bit 0 */
+   val = ioread32(base + ATAPI_CONTROL1_REG);
+   val |= ATAPI_CONTROL1_ISM;
+   val |= ATAPI_CONTROL1_DESE;
+   val |= ATAPI_CONTROL1_DTA32M;
+   iowrite32(val, base + ATAPI_CONTROL1_REG);
+
+   /* Release the SATA-IP from the reset state */
+   val = ioread32(base + ATAPI_CONTROL1_REG);
+   val &= ~ATAPI_CONTROL1_RESET;
+   iowrite32(val, base + ATAPI_CONTROL1_REG);
+


   As it stands, we could surely do the same with a much shorter code:

if (priv->type == RCAR_GEN3_SATA) {
sata_rcar_init_controller(host);

} else {
/* ack and mask */
iowrite32(0, base + SATAINTSTAT_REG);
iowrite32(0x7ff, base + SATAINTMASK_REG);
}

   But do we really need this reset/init sequence on gen1/2 chips?

MBR, Sergei

Re: iwg20d display driver

[Laurent Pinchart]

Hi Chris,

On Wednesday, 3 January 2018 17:19:51 EET Chris Paterson wrote:
> On Sent: 03 January 2018 13:34 Laurent Pinchart wrote:
> > On Wednesday, 3 January 2018 12:09:48 EET Chris Paterson wrote:
> >> Hello Laurent,
> >> 
> >> Happy new year!
> > 
> > Onnellista uutta vuotta to you too :)
> > 
> >> We are looking at upstreaming support for the display provided in the
> >> iwg20d development kit [1]. The setup is slightly unusual in the sense
> >> that the display is an RGB panel (EDT etm070001adh6), connected to the
> >> RZ/G1M via an LVDS/RGB transmitter (DS90CF384AMTDX/NOPB).
> >> 
> >> We think that the correct driver to use is
> >> drivers/gpu/drm/panel/panel-lvds.c as as far as the SoC/Kernel is
> >> aware there is an LVDS display connected. However the bindings
> >> documentation states: "compatible: Shall contain "panel-lvds" in addition
> >> to a mandatory panel-specific compatible string defined in individual
> >> panel bindings. The "panel-lvds" value shall never be used on its own."
> >> 
> >> As the development kit uses an RGB panel there are no suitable LVDS
> >> panel-specific bindings. Would it be okay in this case to used
> >> panel-lvds on its own?
> > 
> > The rationale is that there's no such thing as a fully generic LVDS panel.
> > The panel-lvds compatible string allows supporting panels that are
> > compatible with the interface defined by the panel-lvds driver, but using
> > the compatible string on its own would mean that the operating system
> > would have no way to tweak operation for a particular panel. It might be
> > that using the panel- lvds driver works today, but that you will realize
> > tomorrow that you need some panel-specific quirks. Being able to identify
> > the exact panel model is thus crucial to be future-proof.
> 
> Agreed.
> 
> > The exact compatible string for your panel should be "edt,etm070001adh6".
> > Using
> > 
> > compatible = "edt,etm070001adh6", "panel-lvds";
> > 
> > will work today and be somehow future-proof, but is a bit of a hack as the
> > panel isn't an LVDS panel.
> 
> This was our thought. Would using this compatible string be acceptable (for
> now), even if it isn't the preferred option?

I'm afraid not.

However, I also want to bring good news, I've reworked the DU LVDS code (see 
[2]) which should make it much easier to implement proper support for the 
board. The only missing piece is now a DRM bridge driver for the LVDS to RGB 
decoder. With such a driver in place, and with the proper DT description, I 
believe your board will be supported without having to touch the DU driver 
except for required change to the DU or internal LVDS encoder themselves.

Writing such a driver is also on my to-do list. I might schedule the work for 
the second half of this quarter but I can't guarantee that, so feel free to 
beat me to it. In the meantime I'll rework [2] to try and get it upstream, as 
the current approach was frowned upon. You can however base development on 
that patch series as only the DT backward compatibility code will need to be 
reworked, and that shouldn't influence your work in any way.

> > The right solution would be to model the full display pipeline in DT,
> > including the DS90CF384AMTDX. However, the DU driver doesn't support that
> > yet. Some Gen2 boards suffer from a similar issue in the sense that the
> > device tree pretends that a RGB to HDMI encoder is connected directly to
> > the LVDS output of the SoC, while an LVDS to RGB decoder is present
> > in-between.
> > 
> > Fixing this is somewhere in my todo list but with a low priority I'm
> > afraid. Would you like to give it a go ?
> 
> Depends on your answer to the above ;)
> 
> >> [1]
> >> http://www.iwavesystems.com/product/development-platform/qseven-> >> 
> >> evaluation-board/rz-g1m-qseven-development-kit-49/rz-g1m-qseven-
> >> development-kit.html

[2] https://www.spinics.net/lists/linux-renesas-soc/msg22369.html

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 2/2] drm: rcar-du: lvds: Fix LVDS startup on R-Car gen2

[Laurent Pinchart]

Hi Sergei,

On Saturday, 13 January 2018 11:33:55 EET Sergei Shtylyov wrote:
> On 1/13/2018 2:10 AM, Laurent Pinchart wrote:
> > From: Sergei Shtylyov 
> > 
> > According to the latest revision 2.00 of the R-Car gen2 manual, the LVDS
> > must be enabled and the bias crcuit enabled after the LVDS I/O pins are
> > enabled, not before. Fix the gen2 LVDS startup sequence accordingly.
> > 
> > While at it, also fix the comment preceding the first LVDCR0 write that
> > still talks about hardcoding the LVDS mode 0.
> 
> Please do this in a separate commit then...

The reason I added it here is that I think we don't need patch 1/2 from this 
series, and I found a bit overkill to split a comment fix to a separate patch 
when we have a patch that touches the code around the comment.

> > Fixes: 90374b5c25c9 ("drm/rcar-du: Add internal LVDS encoder support")
> 
> You forgot to specify the other commit this one fixes -- I mean the comment
> fix.

Do we need to for a comment update ? It doesn't affect fix the behaviour of 
the driver or device, and I'd thus prefer to avoid giving the wrong impression 
that this patch fixes an bug introduced in a previous commit, otherwise it 
might end up being backported unnecessarily.

> > Signed-off-by: Sergei Shtylyov 
> > Reviewed-by: Laurent Pinchart 
> > [Set the mode and input at the same time as the BEN and LVEN bits]
> > Tested-by: Laurent Pinchart 
> > Signed-off-by: Laurent Pinchart
> > 
> > ---
> > 
> >   drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 14 +++---
> >   1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > Hi Sergei,
> > 
> > For your convenience (and if you agree with bundling mode setup with the
> > first write as explained in my review of patch 1/2), here's the updated
> > version of patch 2/2 that I have taken in my development branch. If
> > you're fine with it I'll keep it, otherwise we can continue the review
> > discussion.
> 
> As I said, I don't know how to interpret the note 3 in either manual.

As explained in my latest reply to patch 1/2, my understanding is that the 
parameters can be programmed at any time before step 6. The fact that the 
current code works seems to confirm that interpretation. We could ask Renesas 
for a confirmation if you want.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] drm: rcar-du: lvds: fix LVDS startup on R-Car gen3

[Laurent Pinchart]

Hi Sergei,

On Saturday, 13 January 2018 11:25:31 EET Sergei Shtylyov wrote:
> On 1/13/2018 1:15 AM, Laurent Pinchart wrote:
> >>> According to the latest revisions of the R-Car gen3 manual, the LVDS
> >>> mode must be set before the LVDS I/O pins are enabled, not after --  fix 
> >>> the gen3 LVDS startup sequence accordingly...
> >>> 
> >>> While  at it,  also fix the comment  preceding the first LVDCR0 write in
> >>> the R-Car gen2 startup code that still talks about hardcoding the LVDS
> >>> mode 0...
> >> 
> >> How about fixing that in patch 2/2 that touches the Gen2 initialization
> >> sequence ? I think I'd even go as far as squashing both patches, I don't
> >> think there's a need to split them.
> >> 
> >>> Fixes: e947eccbeba4 ("drm: rcar-du: Add support for LVDS mode
> >>> selection")
> >>> Signed-off-by: Sergei Shtylyov 
> >> 
> >> Is this really needed ? Does it fix a problem you've experienced, or is
> >> it theoretical only ? The mode shouldn't matter before the LVDS internal
> >> logic is turned on. Unless there's a real issue I'm not sure we should
> >> make the code more complex.
> > 
> > Furthermore the datasheet states
> > 
> > "3. This refers to settings other than those that are concerned with
> > LVDS-IF startup. These items may be set while waiting for the conditions
> > of step 6 to be met."
> 
> Ah, I hadn't paid much attention to this note. Howeve, it seems quite
> vague to me... and there's no condition in step 6. ;-)

Lots of bits and pieces are lost in translation yes :-)

> > Doesn't this mean that the mode and input selector don't have to be set as
> > the very first step, but can be programmed at any point before starting
> > the LVDS encoder through the PWD bit (on Gen3) or the PLLON bit (on Gen2)
> > ?
> 
> Frankly speaking, I don't know how to interpret that note...

My understanding is that the parameters can be programmed at any time before 
step 6. The fact that the current code works seems to confirm that 
interpretation. We could ask Renesas for a confirmation if you want.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Rob Herring]

On Tue, Jan 16, 2018 at 2:56 AM, Geert Uytterhoeven
 wrote:
> Hi Laurent,
>
> On Mon, Jan 15, 2018 at 7:01 PM, Laurent Pinchart
>  wrote:
>> On Monday, 15 January 2018 19:09:53 EET Rob Herring wrote:
>>> On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart wrote:
>>> > The internal LVDS encoders now have their own DT bindings. Before
>>> > switching the driver infrastructure to those new bindings, implement
>>> > backward-compatibility through live DT patching.
>>>
>>> Uhh, we just got rid of TI's patching and now adding this one. I guess
>>> it's necessary, but I'd like to know how long we need to keep this?
>>
>> That's a good question. How long are we supposed to keep DT backward
>> compatibility for ? I don't think there's a one-size-fits-them-all answer to
>> this question. Geert, any opinion ? How long do you plan to keep the CPG
>> (clocks) DT backward compatibility for instance ?
>
> Good question indeed...
>
> It's not just CPG/MSSR. Over the years we also added or changed APMU (SMP),
> SYSC (power domains), RST (mode pins), CMT (timers), ..., all of which have
> some sort of compatibility support in the kernel.
>
> Hence to avoid having to remember the kernel versions that dropped backwards
> compatibility for each of the above components, I was thinking about an
> R-Car Gen2 DT Flag Day.

There's probably also DT changes that enable new features folks would
want/need? Or maybe carrying for some number of LTS releases is
enough.

> However, that was before I learned about your plans for LVDS (it seems every
> kernel release we learn about something new, postponing the flag day ;-).
> And now I'm quite sure we'll have another change in the future (DU per
> channel device nodes)...
>
> About using DT fixups to implement backwards compatibility: I did my share
> of thinking and experimenting with DT fixups (see e.g. "[PATCH/RFC 0/1]
> soc: renesas: Add DT fixup code for backwards compatibility"
> (https://www.spinics.net/lists/linux-renesas-soc/msg04305.html).
> DT fixups are hard to implement right, and not everything can be done
> that way.  Hence in the end, none of this was ever used upstream, and
> everything was handled in C...

In an ideal world, we would have some tool:

dt-diff-to-overlay old.dts new.dts > my-fixup-overlay.dts

And then in the kernel have infrastructure such you just declare match
tables with overlays to apply:

struct of_device_id dt_match[] = {
  {
.compatible = "vendor,board",
  },
  {},
};
DT_FIXUP(dt_match, my-fixup-overlay.dtbo);

But maybe I'm dreaming...

> So I'm wondering if it would be easier if you would implement backwards
> compatibility in C, using different compatible values for the new bindings?
> I.e. switch from "renesas,du-r8a77*" to "renesas,r8a77*-du" +
> "renesas,r8a77*-lvds"?
> That way it also becomes very clear that there are old and new bindings,
> and that there is backwards compatibility code for the old binding.
>
> I know you're aware (the rest of the audience may not be) that the LVDS
> part is not the only separate hardware block current unified in the single
> DU node: each DU channel has its own hardware block.  Perhaps you can also
> bite the bullet and have a single device node per DT channel, allowing
> Runtime PM for DU channels?
> Of course you have to tie channels together using a "companion" (cfr. USB)
> or "renesas,bonding" (cfr. DRIF) property (unfortunately I learned about
> the former only after the latter was already established).
>
> Finally, implementing backwards compatibility support by DT fixup using
> overlays may complicate backporting to LTSI kernels, due to the dependency
> on DT overlays, which were reworked lately.
>
>>> > --- a/drivers/gpu/drm/rcar-du/Kconfig
>>> > +++ b/drivers/gpu/drm/rcar-du/Kconfig
>>> > @@ -22,6 +22,8 @@ config DRM_RCAR_LVDS
>>> > bool "R-Car DU LVDS Encoder Support"
>>> > depends on DRM_RCAR_DU
>>> > select DRM_PANEL
>>> > +   select OF_FLATTREE
>>> > +   select OF_OVERLAY
>>>
>>> OF_OVERLAY should probably select OF_FLATTREE. I guess in theory, we
>>> could have an overlay from a non-FDT source...
>
> Currently the select is needed for of_fdt_unflatten_tree() only, which is
> not used by the core OF_OVERLAY code. So you could build an overlay in
> memory yourself, and pass that, without using of_fdt_unflatten_tree().
> But that is going to change if I read Frank's reponse well?

Yes, it's currently a 4 or so step process that we plan to make a single call.

Rob

[PATCH v2] sata: sata_rcar: Reset SATA PHY when Salvator-X board resumes

[Yoshihiro Kaneko]

From: Khiem Nguyen 

Because power of Salvator-X board is cut off in suspend,
it needs to reset SATA PHY state in resume.
Otherwise, SATA partition could not be accessed anymore.

Signed-off-by: Khiem Nguyen 
Signed-off-by: Hien Dang 
[reinit phy in sata_rcar_resume() function on R-Car Gen3 only]
[fixed the prefix for the subject]
Signed-off-by: Yoshihiro Kaneko 
---

This patch is based on the for-next branch of libata tree.

v2 [Yoshihiro Kaneko]
* reinit phy on R-Car Gen3 only as suggested by Geert Uytterhoeven
* use "sata_rcar" prefix for the subject as suggested by Sergei Shtylyov

 drivers/ata/sata_rcar.c | 38 --
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
index 80ee2f2..4adc0d6 100644
--- a/drivers/ata/sata_rcar.c
+++ b/drivers/ata/sata_rcar.c
@@ -146,6 +146,7 @@
 enum sata_rcar_type {
RCAR_GEN1_SATA,
RCAR_GEN2_SATA,
+   RCAR_GEN3_SATA,
RCAR_R8A7790_ES1_SATA,
 };
 
@@ -796,6 +797,7 @@ static void sata_rcar_init_controller(struct ata_host *host)
sata_rcar_gen1_phy_init(priv);
break;
case RCAR_GEN2_SATA:
+   case RCAR_GEN3_SATA:
case RCAR_R8A7790_ES1_SATA:
sata_rcar_gen2_phy_init(priv);
break;
@@ -856,7 +858,7 @@ static void sata_rcar_init_controller(struct ata_host *host)
},
{
.compatible = "renesas,sata-r8a7795",
-   .data = (void *)RCAR_GEN2_SATA
+   .data = (void *)RCAR_GEN3_SATA
},
{
.compatible = "renesas,rcar-gen2-sata",
@@ -864,7 +866,7 @@ static void sata_rcar_init_controller(struct ata_host *host)
},
{
.compatible = "renesas,rcar-gen3-sata",
-   .data = (void *)RCAR_GEN2_SATA
+   .data = (void *)RCAR_GEN3_SATA
},
{ },
 };
@@ -977,11 +979,43 @@ static int sata_rcar_resume(struct device *dev)
struct sata_rcar_priv *priv = host->private_data;
void __iomem *base = priv->base;
int ret;
+   u32 val;
 
ret = clk_prepare_enable(priv->clk);
if (ret)
return ret;
 
+   /* reinit phy on R-Car Gen3 only */
+   switch (priv->type) {
+   case RCAR_GEN1_SATA:
+   case RCAR_GEN2_SATA:
+   case RCAR_R8A7790_ES1_SATA:
+   break;
+   case RCAR_GEN3_SATA:
+   sata_rcar_gen2_phy_init(priv);
+   break;
+   default:
+   dev_warn(host->dev, "SATA phy is not initialized\n");
+   break;
+   }
+
+   /* SATA-IP reset state */
+   val = ioread32(base + ATAPI_CONTROL1_REG);
+   val |= ATAPI_CONTROL1_RESET;
+   iowrite32(val, base + ATAPI_CONTROL1_REG);
+
+   /* ISM mode, PRD mode, DTEND flag at bit 0 */
+   val = ioread32(base + ATAPI_CONTROL1_REG);
+   val |= ATAPI_CONTROL1_ISM;
+   val |= ATAPI_CONTROL1_DESE;
+   val |= ATAPI_CONTROL1_DTA32M;
+   iowrite32(val, base + ATAPI_CONTROL1_REG);
+
+   /* Release the SATA-IP from the reset state */
+   val = ioread32(base + ATAPI_CONTROL1_REG);
+   val &= ~ATAPI_CONTROL1_RESET;
+   iowrite32(val, base + ATAPI_CONTROL1_REG);
+
/* ack and mask */
iowrite32(0, base + SATAINTSTAT_REG);
iowrite32(0x7ff, base + SATAINTMASK_REG);
-- 
1.9.1

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Rob Herring]

On Mon, Jan 15, 2018 at 5:46 PM, Frank Rowand  wrote:
> On 01/15/18 12:29, Laurent Pinchart wrote:
>> Hi Frank,
>>
>> On Monday, 15 January 2018 22:12:33 EET Frank Rowand wrote:
>>> On 01/15/18 11:22, Laurent Pinchart wrote:
 On Monday, 15 January 2018 21:12:44 EET Frank Rowand wrote:
> On 01/15/18 09:09, Rob Herring wrote:
>> +Frank
>>
>> On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart wrote:
>>> The internal LVDS encoders now have their own DT bindings. Before
>>> switching the driver infrastructure to those new bindings, implement
>>> backward-compatibility through live DT patching.
>>
>> Uhh, we just got rid of TI's patching and now adding this one. I guess
>>>
>>> Let me first answer the question that you ask later.  You ask "Can we work
>>> on this together to find a solution that would suit us both ?"
>>>
>>> My answer to that is emphatically YES.  I will definitely work with you to
>>> try to find a good solution.
>>
>> \o/
>>
> Please no.  What we just got rid of was making it difficult for me to
> make changes to the overlay infrastructure.  There are issues with
> overlays that need to be fixed before overlays become really usable.
> I am about to propose the next change, which involves removing a
> chunk of code that I will not be able to remove if this patch is
> accepted (the proposal is awaiting me collecting some data about
> the impact of the change, which I expect to complete this week).
>>>
>>> I should have thought just a little bit more before I hit send.  The
>>> situation is even worse than I wrote.  One of the next steps (in
>>> addition to what I wrote about above) is to change the overlay apply
>>> function to accept a flattened device tree (FDT), not an expanded
>>> device tree.  In this changed model, the unflattened overlay is
>>> not available to be modified before it is applied.
>>
>> That makes sense if we consider overlays to be immutable objects that we 
>> apply
>> without offering a way to modify them. I won't challenge that API decision, 
>> as
>> my use of an overlay here is a bit of a hack indeed.
>>
>>> It is important for the devicetree infrastructure to have ownership
>>> of the FDT that is used to create the unflattened tree.  (Notice
>>> that in the proposed patch, rcar_du_of_get_overlay() follows the
>>> TI example of doing a kmemdup of the blob (FDT), then uses the
>>> copy for the unflatten.  The kmemdup() in this case is to create
>>> a persistent copy of the FDT.)  The driver having ownership of
>>> this copy, and having the ability to free it is one of the many
>>> problems with the current overlay implementation.
>>
>> Yes, that's something I've identified as well. Lots of work has been done to
>> clean up the OF core and we're clearly not done yet. I'm happy to see all the
>> improvements you're working on.
>>
> Can you please handle both the old and new bindings through driver
> code instead?

 I could, but it would be pointless. The point here is to allow cleanups in
 the driver. The LVDS encoder handling code is very intrusive in its
 current form and I need to get rid of it. There would be zero point in
 moving to the new infrastructure, as the main point is to get rid of the
 old code which prevents moving forward. As a consequence that would block
 new boards from receiving proper upstream support. An easy option is to
 break backward compatibility. I'm personally fine with that, but I assume
 other people would complain :-)

 I can, on the other hand, work with you to see how live DT patching could
 be implemented in this driver without blocking your code. When developing
 this patch series I start by patching the device tree manually without
 relying on overlays at all, but got blocked by the fact that I need to
 allocate phandles for new nodes I create. If there was an API to allocate
 an unused phandle I could avoid using the overlay infrastructure at all.
 Or there could be other
>>>
>>> It seems reasonable to provide a way to autogenerate a phandle (if
>>> requested) by the devicetree code that creates a new node.  Were you using
>>> a function from drivers/of/dynamic.c to create the node?
>>
>> Not to allocate the node, no. I allocated the device_node structure manually
>> with kzalloc(), and inserted it in the device tree with of_attach_node(). Is
>> that the right approach ? I haven't been able to test the code as I stopped
>> when I realized I couldn't allocate phandles.
>>
 options I'm not thinking of as I don't know what the changes you're
 working on are. Can we work on this together to find a solution that
 would suit us both ?
>>>
>>> Again, yes, I would be glad to work with you on this.
>>
>> How would you like to proceed ? I can try the manual approach again but need
>> information about how I could cleanly implement phandle allocation. I will
>> likely 

Re: [PATCH 3/4] ARM: r8a7790: sort subnodes of soc node

[Simon Horman]

On Tue, Jan 16, 2018 at 11:13:29AM +0100, Simon Horman wrote:
> Sort the subnodes of the soc node to improve maintainability.
> The sort key is the addresss on the bus with instances of the same
> IP block grouped together.
> 
> This patch should not introduce any functional change.
> 
> Signed-off-by: Simon Horman 

Looking over this it seems I messed up the ordering of
qspi, msiof*, can*, and vin* nodes.

Re: [PATCH v5 6/9] media: i2c: ov772x: Remove soc_camera dependencies

[jacopo mondi]

Hi Hans,

On Tue, Jan 16, 2018 at 11:08:17AM +0100, Hans Verkuil wrote:
> On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> > Remove soc_camera framework dependencies from ov772x sensor driver.
> > - Handle clock and gpios
> > - Register async subdevice
> > - Remove soc_camera specific g/s_mbus_config operations
> > - Change image format colorspace from JPEG to SRGB as the two use the
> >   same colorspace information but JPEG makes assumptions on color
> >   components quantization that do not apply to the sensor
> > - Remove sizes crop from get_selection as driver can't scale
> > - Add kernel doc to driver interface header file
> > - Adjust build system
> >
> > This commit does not remove the original soc_camera based driver as long
> > as other platforms depends on soc_camera-based CEU driver.
> >
> > Signed-off-by: Jacopo Mondi 
> > Reviewed-by: Laurent Pinchart 

[snip]

> >  static const struct ov772x_win_size *ov772x_select_win(u32 width, u32 
> > height)
> > @@ -855,24 +910,21 @@ static int ov772x_get_selection(struct v4l2_subdev 
> > *sd,
> > struct v4l2_subdev_pad_config *cfg,
> > struct v4l2_subdev_selection *sel)
> >  {
> > +   struct ov772x_priv *priv = to_ov772x(sd);
> > +
> > if (sel->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> > return -EINVAL;
> >
> > -   sel->r.left = 0;
> > -   sel->r.top = 0;
>
> Why are these two lines removed?
>
> > switch (sel->target) {
> > case V4L2_SEL_TGT_CROP_BOUNDS:
> > case V4L2_SEL_TGT_CROP_DEFAULT:
> > -   sel->r.width = OV772X_MAX_WIDTH;
> > -   sel->r.height = OV772X_MAX_HEIGHT;
> > -   return 0;
> > case V4L2_SEL_TGT_CROP:
> > -   sel->r.width = VGA_WIDTH;
> > -   sel->r.height = VGA_HEIGHT;
> > -   return 0;
> > -   default:
> > -   return -EINVAL;
>
> Why is this default case removed?
>

Ooops. I have badly addressed your comment on v1.

Will fix in v6.

> > +   sel->r.width = priv->win->rect.width;
> > +   sel->r.height = priv->win->rect.height;
> > +   break;
> > }
> > +
> > +   return 0;
> >  }
> >
> >  static int ov772x_get_fmt(struct v4l2_subdev *sd,
> > @@ -997,24 +1049,8 @@ static int ov772x_enum_mbus_code(struct v4l2_subdev 
> > *sd,
> > return 0;
> >  }
> >
> > -static int ov772x_g_mbus_config(struct v4l2_subdev *sd,
> > -   struct v4l2_mbus_config *cfg)
> > -{
> > -   struct i2c_client *client = v4l2_get_subdevdata(sd);
> > -   struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
> > -
> > -   cfg->flags = V4L2_MBUS_PCLK_SAMPLE_RISING | V4L2_MBUS_MASTER |
> > -   V4L2_MBUS_VSYNC_ACTIVE_HIGH | V4L2_MBUS_HSYNC_ACTIVE_HIGH |
> > -   V4L2_MBUS_DATA_ACTIVE_HIGH;
> > -   cfg->type = V4L2_MBUS_PARALLEL;
> > -   cfg->flags = soc_camera_apply_board_flags(ssdd, cfg);
> > -
> > -   return 0;
> > -}
> > -
> >  static const struct v4l2_subdev_video_ops ov772x_subdev_video_ops = {
> > .s_stream   = ov772x_s_stream,
> > -   .g_mbus_config  = ov772x_g_mbus_config,
> >  };
> >
> >  static const struct v4l2_subdev_pad_ops ov772x_subdev_pad_ops = {
> > @@ -1038,12 +1074,11 @@ static int ov772x_probe(struct i2c_client *client,
> > const struct i2c_device_id *did)
> >  {
> > struct ov772x_priv  *priv;
> > -   struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
> > -   struct i2c_adapter  *adapter = to_i2c_adapter(client->dev.parent);
> > +   struct i2c_adapter  *adapter = client->adapter;
> > int ret;
> >
> > -   if (!ssdd || !ssdd->drv_priv) {
> > -   dev_err(>dev, "OV772X: missing platform data!\n");
> > +   if (!client->dev.platform_data) {
> > +   dev_err(>dev, "Missing OV7725 platform data\n");
>
> Nitpick: I'd prefer lowercase in this string: ov7725. It also should be
> ov772x.
>
> > return -EINVAL;
> > }
> >
> > @@ -1059,7 +1094,7 @@ static int ov772x_probe(struct i2c_client *client,
> > if (!priv)
> > return -ENOMEM;
> >
> > -   priv->info = ssdd->drv_priv;
> > +   priv->info = client->dev.platform_data;
> >
> > v4l2_i2c_subdev_init(>subdev, client, _subdev_ops);
> > v4l2_ctrl_handler_init(>hdl, 3);
> > @@ -1073,22 +1108,42 @@ static int ov772x_probe(struct i2c_client *client,
> > if (priv->hdl.error)
> > return priv->hdl.error;
> >
> > -   priv->clk = v4l2_clk_get(>dev, "mclk");
> > +   priv->clk = clk_get(>dev, "xclk");
> > if (IS_ERR(priv->clk)) {
> > +   dev_err(>dev, "Unable to get xclk clock\n");
> > ret = PTR_ERR(priv->clk);
> > -   goto eclkget;
> > +   goto error_ctrl_free;
> > }
> >
> > -   ret = ov772x_video_probe(priv);
> > -   if (ret < 0) {
> > -   v4l2_clk_put(priv->clk);
> > -eclkget:
> > -   v4l2_ctrl_handler_free(>hdl);
> > -   } else {
> > -   priv->cfmt = _cfmts[0];

Re: [PATCH v5 3/9] v4l: platform: Add Renesas CEU driver

[Laurent Pinchart]

Hi Hans,

On Tuesday, 16 January 2018 11:46:42 EET Hans Verkuil wrote:
> Hi Jacopo,
> 
> Sorry for the late review, but here is finally is.
> 
> BTW, can you provide the v4l2-compliance output (ideally with the -f option)
> in the cover letter for v6?
> 
> On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> > Add driver for Renesas Capture Engine Unit (CEU).
> > 
> > The CEU interface supports capturing 'data' (YUV422) and 'images'
> > (NV[12|21|16|61]).
> > 
> > This driver aims to replace the soc_camera-based sh_mobile_ceu one.
> > 
> > Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ
> > platform GR-Peach.
> > 
> > Tested with ov7725 camera sensor on SH4 platform Migo-R.
> > 
> > Signed-off-by: Jacopo Mondi 
> > Reviewed-by: Laurent Pinchart 
> > ---
> > 
> >  drivers/media/platform/Kconfig   |9 +
> >  drivers/media/platform/Makefile  |1 +
> >  drivers/media/platform/renesas-ceu.c | 1648 +
> >  3 files changed, 1658 insertions(+)
> >  create mode 100644 drivers/media/platform/renesas-ceu.c

[snip]

> > diff --git a/drivers/media/platform/renesas-ceu.c
> > b/drivers/media/platform/renesas-ceu.c new file mode 100644
> > index 000..ccca838
> > --- /dev/null
> > +++ b/drivers/media/platform/renesas-ceu.c

[snip]

> > +static int ceu_s_input(struct file *file, void *priv, unsigned int i)
> > +{
> > +   struct ceu_device *ceudev = video_drvdata(file);
> > +   struct ceu_subdev *ceu_sd_old;
> > +   int ret;
> > +
> 
> Add a check:
> 
>   if (i == ceudev->sd_index)
>   return 0;
> 
> I.e. if the new input == the old input, then that's fine regardless of the
> streaming state.

On a side note this is the kind of checks that the core should handle, but 
that's out of scope for this patch.

> > +   if (vb2_is_streaming(>vb2_vq))
> > +   return -EBUSY;
> > +
> > +   if (i >= ceudev->num_sd)
> > +   return -EINVAL;
> 
> Move this up as the first test.
> 
> > +
> > +   ceu_sd_old = ceudev->sd;
> > +   ceudev->sd = >subdevs[i];
> > +
> > +   /* Make sure we can generate output image formats. */
> > +   ret = ceu_init_formats(ceudev);
> > +   if (ret) {
> > +   ceudev->sd = ceu_sd_old;
> > +   return -EINVAL;
> > +   }
> > +
> > +   /* now that we're sure we can use the sensor, power off the old one. */
> > +   v4l2_subdev_call(ceu_sd_old->v4l2_sd, core, s_power, 0);
> > +   v4l2_subdev_call(ceudev->sd->v4l2_sd, core, s_power, 1);
> > +
> > +   ceudev->sd_index = i;
> > +
> > +   return 0;
> > +}

[snip]

> > +
> > +static int ceu_notify_complete(struct v4l2_async_notifier *notifier)
> > +{
> > +   struct v4l2_device *v4l2_dev = notifier->v4l2_dev;
> > +   struct ceu_device *ceudev = v4l2_to_ceu(v4l2_dev);
> > +   struct video_device *vdev = >vdev;
> > +   struct vb2_queue *q = >vb2_vq;
> > +   struct v4l2_subdev *v4l2_sd;
> > +   int ret;
> > +
> > +   /* Initialize vb2 queue. */
> > +   q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> > +   q->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
> 
> Don't include VB2_USERPTR. It shouldn't be used with dma_contig.
> 
> You also added a read() fop (vb2_fop_read), so either add VB2_READ here
> or remove the read fop.

Agreed. I'd drop both VB2_USERPTR and vb2_fop_read().

> > +   q->drv_priv = ceudev;
> > +   q->ops  = _vb2_ops;
> > +   q->mem_ops  = _dma_contig_memops;
> > +   q->buf_struct_size  = sizeof(struct ceu_buffer);
> > +   q->timestamp_flags  = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> > +   q->lock = >mlock;
> > +   q->dev  = ceudev->v4l2_dev.dev;
> > +
> > +   ret = vb2_queue_init(q);
> > +   if (ret)
> > +   return ret;
> > +
> > +   /*
> > +* Make sure at least one sensor is primary and use it to initialize
> > +* ceu formats.
> > +*/
> > +   if (!ceudev->sd) {
> > +   ceudev->sd = >subdevs[0];
> > +   ceudev->sd_index = 0;
> > +   }
> > +
> > +   v4l2_sd = ceudev->sd->v4l2_sd;
> > +
> > +   ret = ceu_init_formats(ceudev);
> > +   if (ret)
> > +   return ret;
> > +
> > +   ret = ceu_set_default_fmt(ceudev);
> > +   if (ret)
> > +   return ret;
> > +
> > +   /* Register the video device. */
> > +   strncpy(vdev->name, DRIVER_NAME, strlen(DRIVER_NAME));
> > +   vdev->v4l2_dev  = v4l2_dev;
> > +   vdev->lock  = >mlock;
> > +   vdev->queue = >vb2_vq;
> > +   vdev->ctrl_handler  = v4l2_sd->ctrl_handler;
> > +   vdev->fops  = _fops;
> > +   vdev->ioctl_ops = _ioctl_ops;
> > +   vdev->release   = ceu_vdev_release;
> > +   vdev->device_caps   = V4L2_CAP_VIDEO_CAPTURE_MPLANE |
> > + V4L2_CAP_STREAMING;
> > +   video_set_drvdata(vdev, ceudev);
> > +
> > +   ret = video_register_device(vdev, VFL_TYPE_GRABBER, -1);
> > +   if (ret < 0) {
> > +   

Re: [PATCH v5 6/9] media: i2c: ov772x: Remove soc_camera dependencies

[Laurent Pinchart]

Hi Hans,

On Tuesday, 16 January 2018 12:08:17 EET Hans Verkuil wrote:
> On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> > Remove soc_camera framework dependencies from ov772x sensor driver.
> > - Handle clock and gpios
> > - Register async subdevice
> > - Remove soc_camera specific g/s_mbus_config operations
> > - Change image format colorspace from JPEG to SRGB as the two use the
> > 
> >   same colorspace information but JPEG makes assumptions on color
> >   components quantization that do not apply to the sensor
> > 
> > - Remove sizes crop from get_selection as driver can't scale
> > - Add kernel doc to driver interface header file
> > - Adjust build system
> > 
> > This commit does not remove the original soc_camera based driver as long
> > as other platforms depends on soc_camera-based CEU driver.
> > 
> > Signed-off-by: Jacopo Mondi 
> > Reviewed-by: Laurent Pinchart 
> > ---
> > 
> >  drivers/media/i2c/Kconfig  |  11 +++
> >  drivers/media/i2c/Makefile |   1 +
> >  drivers/media/i2c/ov772x.c | 177 
> >  include/media/i2c/ov772x.h |   6 +-
> >  4 files changed, 133 insertions(+), 62 deletions(-)

[snip]

> > diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
> > index 8063835..df2516c 100644
> > --- a/drivers/media/i2c/ov772x.c
> > +++ b/drivers/media/i2c/ov772x.c

[snip]

> > @@ -1038,12 +1074,11 @@ static int ov772x_probe(struct i2c_client *client,
> > const struct i2c_device_id *did)
> >  {
> > struct ov772x_priv  *priv;
> > -   struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
> > -   struct i2c_adapter  *adapter = to_i2c_adapter(client->dev.parent);
> > +   struct i2c_adapter  *adapter = client->adapter;
> > int ret;
> > 
> > -   if (!ssdd || !ssdd->drv_priv) {
> > -   dev_err(>dev, "OV772X: missing platform data!\n");
> > +   if (!client->dev.platform_data) {
> > +   dev_err(>dev, "Missing OV7725 platform data\n");
> 
> Nitpick: I'd prefer lowercase in this string: ov7725. It also should be
> ov772x.

Agreed.

> > return -EINVAL;
> > 
> > }

[snip]

> > @@ -1119,6 +1176,6 @@ static struct i2c_driver ov772x_i2c_driver = {
> > 
> >  module_i2c_driver(ov772x_i2c_driver);
> > 
> > -MODULE_DESCRIPTION("SoC Camera driver for ov772x");
> > +MODULE_DESCRIPTION("V4L2 driver for OV772x image sensor");
> 
> Ditto: lower case ov772x.

I'd keep that uppercase. The usual practice (unless I'm mistaken) is to use 
uppercase for chip names and lowercase for driver names. The description 
clearly refers to the chip, so uppercase seems better to me.

> >  MODULE_AUTHOR("Kuninori Morimoto");
> >  MODULE_LICENSE("GPL v2");
> 
> Hmm, shouldn't there be a struct of_device_id as well? So this can be
> used in the device tree?
> 
> I see this sensor was only tested with a non-dt platform. Is it possible
> to test this sensor with the GR-Peach platform (which I gather uses the
> device tree)?
> 
> Making this driver DT compliant can be done as a follow-up patch.

I think it's a good idea, but I'd prefer having that in a separate patch. We 
will also need DT bindings, it's a bit out of scope for this series.

Jacopo, you can keep my ack after addressing Hans' comments.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Laurent Pinchart]

Hi Geert,

On Tuesday, 16 January 2018 10:56:10 EET Geert Uytterhoeven wrote:
> On Mon, Jan 15, 2018 at 7:01 PM, Laurent Pinchart wrote:
> > On Monday, 15 January 2018 19:09:53 EET Rob Herring wrote:
> >> On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart wrote:
> >>> The internal LVDS encoders now have their own DT bindings. Before
> >>> switching the driver infrastructure to those new bindings, implement
> >>> backward-compatibility through live DT patching.
> >> 
> >> Uhh, we just got rid of TI's patching and now adding this one. I guess
> >> it's necessary, but I'd like to know how long we need to keep this?
> > 
> > That's a good question. How long are we supposed to keep DT backward
> > compatibility for ? I don't think there's a one-size-fits-them-all answer
> > to this question. Geert, any opinion ? How long do you plan to keep the
> > CPG (clocks) DT backward compatibility for instance ?
> 
> Good question indeed...
> 
> It's not just CPG/MSSR. Over the years we also added or changed APMU (SMP),
> SYSC (power domains), RST (mode pins), CMT (timers), ..., all of which have
> some sort of compatibility support in the kernel.
> 
> Hence to avoid having to remember the kernel versions that dropped backwards
> compatibility for each of the above components, I was thinking about an
> R-Car Gen2 DT Flag Day.
> 
> However, that was before I learned about your plans for LVDS (it seems every
> kernel release we learn about something new, postponing the flag day ;-).
> And now I'm quite sure we'll have another change in the future (DU per
> channel device nodes)...

I don't think the DU and LVDS rework should postpone your flag day for all the 
core components.

> About using DT fixups to implement backwards compatibility: I did my share
> of thinking and experimenting with DT fixups (see e.g. "[PATCH/RFC 0/1]
> soc: renesas: Add DT fixup code for backwards compatibility"
> (https://www.spinics.net/lists/linux-renesas-soc/msg04305.html).
> DT fixups are hard to implement right, and not everything can be done
> that way.  Hence in the end, none of this was ever used upstream, and
> everything was handled in C...
> 
> So I'm wondering if it would be easier if you would implement backwards
> compatibility in C, using different compatible values for the new bindings?
> I.e. switch from "renesas,du-r8a77*" to "renesas,r8a77*-du" +
> "renesas,r8a77*-lvds"?
> That way it also becomes very clear that there are old and new bindings,
> and that there is backwards compatibility code for the old binding.

Quoting my reply to Frank,

I could, but it would be pointless. The point here is to allow cleanups in the 
driver. The LVDS encoder handling code is very intrusive in its current form 
and I need to get rid of it. There would be zero point in moving to the new 
infrastructure, as the main point is to get rid of the old code which prevents 
moving forward. As a consequence that would block new boards from receiving 
proper upstream support. An easy option is to break backward compatibility. 
I'm personally fine with that, but I assume other people would complain :-)

> I know you're aware (the rest of the audience may not be) that the LVDS
> part is not the only separate hardware block current unified in the single
> DU node: each DU channel has its own hardware block.  Perhaps you can also
> bite the bullet and have a single device node per DT channel, allowing
> Runtime PM for DU channels?

That's more difficult as the channels have cross-dependencies. I might give it 
a try at some point, or I might not. In any case it's a separate piece of 
work, and backward compatibility for that one might be handled in the driver 
instead of through DT patching.

> Of course you have to tie channels together using a "companion" (cfr. USB)
> or "renesas,bonding" (cfr. DRIF) property (unfortunately I learned about
> the former only after the latter was already established).
> 
> Finally, implementing backwards compatibility support by DT fixup using
> overlays may complicate backporting to LTSI kernels, due to the dependency
> on DT overlays, which were reworked lately.

I can drop backward compatibility completely if you prefer, that would be much 
easier to backport ;-)

As discussed with Frank I will likely try to patch the DT live without using 
overlays, but that will likely also be annoying to backport as the ongoing 
modifications to the OF core are not limited to overlays anyway.

> >>> --- a/drivers/gpu/drm/rcar-du/Kconfig
> >>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
> >>> @@ -22,6 +22,8 @@ config DRM_RCAR_LVDS
> >>> bool "R-Car DU LVDS Encoder Support"
> >>> depends on DRM_RCAR_DU
> >>> select DRM_PANEL
> >>> +   select OF_FLATTREE
> >>> +   select OF_OVERLAY
> >> 
> >> OF_OVERLAY should probably select OF_FLATTREE. I guess in theory, we
> >> could have an overlay from a non-FDT source...
> 
> Currently the select is needed for of_fdt_unflatten_tree() only, which is
> not used by the core 

[PATCH 1/4] ARM: dts: r8a7790: consistently use single space after =

[Simon Horman]

Consistently use a single space after a =.
This patch removes instances where a tab is used instead.

This patch should not introduce any functional change.

Signed-off-by: Simon Horman 
---
 arch/arm/boot/dts/r8a7790.dtsi | 72 +-
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index 13926fc7abfa..26da3b282913 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -291,9 +291,9 @@
};
 
thermal: thermal@e61f {
-   compatible ="renesas,thermal-r8a7790",
-   "renesas,rcar-gen2-thermal",
-   "renesas,rcar-thermal";
+   compatible = "renesas,thermal-r8a7790",
+"renesas,rcar-gen2-thermal",
+"renesas,rcar-thermal";
reg = <0 0xe61f 0 0x10>, <0 0xe61f0100 0 0x38>;
interrupts = ;
clocks = < CPG_MOD 522>;
@@ -423,20 +423,20 @@
audma0: dma-controller@ec70 {
compatible = "renesas,dmac-r8a7790", "renesas,rcar-dmac";
reg = <0 0xec70 0 0x1>;
-   interrupts =;
+   interrupts = ;
interrupt-names = "error",
"ch0", "ch1", "ch2", "ch3",
"ch4", "ch5", "ch6", "ch7",
@@ -453,20 +453,20 @@
audma1: dma-controller@ec72 {
compatible = "renesas,dmac-r8a7790", "renesas,rcar-dmac";
reg = <0 0xec72 0 0x1>;
-   interrupts =;
+   interrupts = ;
interrupt-names = "error",
"ch0", "ch1", "ch2", "ch3",
"ch4", "ch5", "ch6", "ch7",
@@ -1438,11 +1438,11 @@
 * Multi  DAI : #sound-dai-cells = <1>; <_sound N>;
 */
compatible =  "renesas,rcar_sound-r8a7790", 
"renesas,rcar_sound-gen2";
-   reg =   <0 0xec50 0 0x1000>, /* SCU */
-   <0 0xec5a 0 0x100>,  /* ADG */
-   <0 0xec54 0 0x1000>, /* SSIU */
-   <0 0xec541000 0 0x280>,  /* SSI */
-   <0 0xec74 0 0x200>;  /* Audio DMAC peri peri*/
+   reg = <0 0xec50 0 0x1000>, /* SCU */
+ <0 0xec5a 0 0x100>,  /* ADG */
+ <0 0xec54 0 0x1000>, /* SSIU */
+ <0 0xec541000 0 0x280>,  /* SSI */
+ <0 0xec74 0 0x200>;  /* Audio DMAC peri peri*/
reg-names = "scu", "adg", "ssiu", "ssi", "audmapp";
 
clocks = < CPG_MOD 1005>,
-- 
2.11.0

[PATCH 4/4] ARM: dts: r8a7790: sort subnodes of root node

[Simon Horman]

Sort subnodes of root node to aid maintenance.

This patch should not introduce any functional change.

Signed-off-by: Simon Horman 
---
 arch/arm/boot/dts/r8a7790.dtsi | 104 -
 1 file changed, 52 insertions(+), 52 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index 297840a05399..4dbea88de879 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -40,6 +40,35 @@
vin3 = 
};
 
+   /*
+* The external audio clocks are configured as 0 Hz fixed frequency
+* clocks by default.
+* Boards that provide audio clocks should override them.
+*/
+   audio_clk_a: audio_clk_a {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <0>;
+   };
+   audio_clk_b: audio_clk_b {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <0>;
+   };
+   audio_clk_c: audio_clk_c {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <0>;
+   };
+
+   /* External CAN clock */
+   can_clk: can {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   /* This value must be overridden by the board. */
+   clock-frequency = <0>;
+   };
+
cpus {
#address-cells = <1>;
#size-cells = <0>;
@@ -158,6 +187,29 @@
};
};
 
+   /* External root clock */
+   extal_clk: extal {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   /* This value must be overridden by the board. */
+   clock-frequency = <0>;
+   };
+
+   /* External PCIe clock - can be overridden by the board */
+   pcie_bus_clk: pcie_bus {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <0>;
+   };
+
+   /* External SCIF clock */
+   scif_clk: scif {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   /* This value must be overridden by the board. */
+   clock-frequency = <0>;
+   };
+
soc {
compatible = "simple-bus";
interrupt-parent = <>;
@@ -1680,62 +1732,10 @@
  < GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) 
| IRQ_TYPE_LEVEL_LOW)>;
};
 
-   /* External root clock */
-   extal_clk: extal {
-   compatible = "fixed-clock";
-   #clock-cells = <0>;
-   /* This value must be overridden by the board. */
-   clock-frequency = <0>;
-   };
-
-   /* External PCIe clock - can be overridden by the board */
-   pcie_bus_clk: pcie_bus {
-   compatible = "fixed-clock";
-   #clock-cells = <0>;
-   clock-frequency = <0>;
-   };
-
-   /*
-* The external audio clocks are configured as 0 Hz fixed frequency
-* clocks by default.
-* Boards that provide audio clocks should override them.
-*/
-   audio_clk_a: audio_clk_a {
-   compatible = "fixed-clock";
-   #clock-cells = <0>;
-   clock-frequency = <0>;
-   };
-   audio_clk_b: audio_clk_b {
-   compatible = "fixed-clock";
-   #clock-cells = <0>;
-   clock-frequency = <0>;
-   };
-   audio_clk_c: audio_clk_c {
-   compatible = "fixed-clock";
-   #clock-cells = <0>;
-   clock-frequency = <0>;
-   };
-
-   /* External SCIF clock */
-   scif_clk: scif {
-   compatible = "fixed-clock";
-   #clock-cells = <0>;
-   /* This value must be overridden by the board. */
-   clock-frequency = <0>;
-   };
-
/* External USB clock - can be overridden by the board */
usb_extal_clk: usb_extal {
compatible = "fixed-clock";
#clock-cells = <0>;
clock-frequency = <4800>;
};
-
-   /* External CAN clock */
-   can_clk: can {
-   compatible = "fixed-clock";
-   #clock-cells = <0>;
-   /* This value must be overridden by the board. */
-   clock-frequency = <0>;
-   };
 };
-- 
2.11.0

[PATCH 2/4] ARM: r8a7790: Add soc node

[Simon Horman]

Add soc node to represent the bus and move all nodes with a base address
into this node. This is consistent with handling of R-Car Gen3, RZ/G1, and
R-Car V2H (R8A77920) SoCs upstream. It is intended to migrate other R-Car
Gen2 SoCs to this scheme.

The ordering is derived from simply moving each node with an address up to
before any nodes without a base address that occur before the soc node.  To
improve maintainability follow-up patches will sort subnodes of both the
new soc node and the root node.

This patch should not introduce any functional change.

Signed-off-by: Simon Horman 
---
 arch/arm/boot/dts/r8a7790.dtsi | 2795 +---
 1 file changed, 1434 insertions(+), 1361 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index 26da3b282913..81f70b9fb6cb 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -17,7 +17,6 @@
 
 / {
compatible = "renesas,r8a7790";
-   interrupt-parent = <>;
#address-cells = <2>;
#size-cells = <2>;
 
@@ -159,981 +158,1526 @@
};
};
 
-   thermal-zones {
-   cpu_thermal: cpu-thermal {
-   polling-delay-passive   = <0>;
-   polling-delay   = <0>;
+   soc {
+   compatible = "simple-bus";
+   interrupt-parent = <>;
 
-   thermal-sensors = <>;
+   #address-cells = <2>;
+   #size-cells = <2>;
+   ranges;
 
-   trips {
-   cpu-crit {
-   temperature = <95000>;
-   hysteresis  = <0>;
-   type= "critical";
-   };
-   };
-   cooling-maps {
-   };
+   apmu@e6151000 {
+   compatible = "renesas,r8a7790-apmu", "renesas,apmu";
+   reg = <0 0xe6151000 0 0x188>;
+   cpus = <   >;
};
-   };
 
-   apmu@e6151000 {
-   compatible = "renesas,r8a7790-apmu", "renesas,apmu";
-   reg = <0 0xe6151000 0 0x188>;
-   cpus = <   >;
-   };
+   apmu@e6152000 {
+   compatible = "renesas,r8a7790-apmu", "renesas,apmu";
+   reg = <0 0xe6152000 0 0x188>;
+   cpus = <   >;
+   };
 
-   apmu@e6152000 {
-   compatible = "renesas,r8a7790-apmu", "renesas,apmu";
-   reg = <0 0xe6152000 0 0x188>;
-   cpus = <   >;
-   };
+   gic: interrupt-controller@f1001000 {
+   compatible = "arm,gic-400";
+   #interrupt-cells = <3>;
+   #address-cells = <0>;
+   interrupt-controller;
+   reg = <0 0xf1001000 0 0x1000>,
+   <0 0xf1002000 0 0x2000>,
+   <0 0xf1004000 0 0x2000>,
+   <0 0xf1006000 0 0x2000>;
+   interrupts = ;
+   clocks = < CPG_MOD 408>;
+   clock-names = "clk";
+   power-domains = < R8A7790_PD_ALWAYS_ON>;
+   resets = < 408>;
+   };
 
-   gic: interrupt-controller@f1001000 {
-   compatible = "arm,gic-400";
-   #interrupt-cells = <3>;
-   #address-cells = <0>;
-   interrupt-controller;
-   reg = <0 0xf1001000 0 0x1000>,
-   <0 0xf1002000 0 0x2000>,
-   <0 0xf1004000 0 0x2000>,
-   <0 0xf1006000 0 0x2000>;
-   interrupts = ;
-   clocks = < CPG_MOD 408>;
-   clock-names = "clk";
-   power-domains = < R8A7790_PD_ALWAYS_ON>;
-   resets = < 408>;
-   };
+   gpio0: gpio@e605 {
+   compatible = "renesas,gpio-r8a7790",
+"renesas,rcar-gen2-gpio";
+   reg = <0 0xe605 0 0x50>;
+   interrupts = ;
+   #gpio-cells = <2>;
+   gpio-controller;
+   gpio-ranges = < 0 0 32>;
+   #interrupt-cells = <2>;
+   interrupt-controller;
+   clocks = < CPG_MOD 912>;
+   power-domains = < R8A7790_PD_ALWAYS_ON>;
+   resets = < 912>;
+   };
 
-   gpio0: gpio@e605 {
-   compatible = "renesas,gpio-r8a7790", "renesas,rcar-gen2-gpio";
-   reg = <0 0xe605 0 0x50>;
-   interrupts = ;
-  

[PATCH 3/4] ARM: r8a7790: sort subnodes of soc node

[Simon Horman]

Sort the subnodes of the soc node to improve maintainability.
The sort key is the addresss on the bus with instances of the same
IP block grouped together.

This patch should not introduce any functional change.

Signed-off-by: Simon Horman 
---
 arch/arm/boot/dts/r8a7790.dtsi | 1772 
 1 file changed, 886 insertions(+), 886 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index 81f70b9fb6cb..297840a05399 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -166,34 +166,6 @@
#size-cells = <2>;
ranges;
 
-   apmu@e6151000 {
-   compatible = "renesas,r8a7790-apmu", "renesas,apmu";
-   reg = <0 0xe6151000 0 0x188>;
-   cpus = <   >;
-   };
-
-   apmu@e6152000 {
-   compatible = "renesas,r8a7790-apmu", "renesas,apmu";
-   reg = <0 0xe6152000 0 0x188>;
-   cpus = <   >;
-   };
-
-   gic: interrupt-controller@f1001000 {
-   compatible = "arm,gic-400";
-   #interrupt-cells = <3>;
-   #address-cells = <0>;
-   interrupt-controller;
-   reg = <0 0xf1001000 0 0x1000>,
-   <0 0xf1002000 0 0x2000>,
-   <0 0xf1004000 0 0x2000>,
-   <0 0xf1006000 0 0x2000>;
-   interrupts = ;
-   clocks = < CPG_MOD 408>;
-   clock-names = "clk";
-   power-domains = < R8A7790_PD_ALWAYS_ON>;
-   resets = < 408>;
-   };
-
gpio0: gpio@e605 {
compatible = "renesas,gpio-r8a7790",
 "renesas,rcar-gen2-gpio";
@@ -284,50 +256,42 @@
resets = < 907>;
};
 
-   thermal: thermal@e61f {
-   compatible = "renesas,thermal-r8a7790",
-"renesas,rcar-gen2-thermal",
-"renesas,rcar-thermal";
-   reg = <0 0xe61f 0 0x10>, <0 0xe61f0100 0 0x38>;
-   interrupts = ;
-   clocks = < CPG_MOD 522>;
-   power-domains = < R8A7790_PD_ALWAYS_ON>;
-   resets = < 522>;
-   #thermal-sensor-cells = <0>;
+   pfc: pin-controller@e606 {
+   compatible = "renesas,pfc-r8a7790";
+   reg = <0 0xe606 0 0x250>;
};
 
-   cmt0: timer@ffca {
-   compatible = "renesas,r8a7790-cmt0",
-"renesas,rcar-gen2-cmt0";
-   reg = <0 0xffca 0 0x1004>;
-   interrupts = ,
-;
-   clocks = < CPG_MOD 124>;
-   clock-names = "fck";
-   power-domains = < R8A7790_PD_ALWAYS_ON>;
-   resets = < 124>;
+   cpg: clock-controller@e615 {
+   compatible = "renesas,r8a7790-cpg-mssr";
+   reg = <0 0xe615 0 0x1000>;
+   clocks = <_clk>, <_extal_clk>;
+   clock-names = "extal", "usb_extal";
+   #clock-cells = <2>;
+   #power-domain-cells = <0>;
+   #reset-cells = <1>;
+   };
 
-   status = "disabled";
+   apmu@e6151000 {
+   compatible = "renesas,r8a7790-apmu", "renesas,apmu";
+   reg = <0 0xe6151000 0 0x188>;
+   cpus = <   >;
};
 
-   cmt1: timer@e613 {
-   compatible = "renesas,r8a7790-cmt1",
-"renesas,rcar-gen2-cmt1";
-   reg = <0 0xe613 0 0x1004>;
-   interrupts = ,
-,
-,
-,
-,
-,
-,
-;
-   clocks = < CPG_MOD 329>;
-   clock-names = "fck";
-   power-domains = < R8A7790_PD_ALWAYS_ON>;
-   resets = < 329>;
+   apmu@e6152000 {
+   compatible = "renesas,r8a7790-apmu", "renesas,apmu";
+   reg = <0 0xe6152000 0 0x188>;
+   cpus = <   >;
+   

[PATCH 0/4] ARM: r8a7790: Add soc node

[Simon Horman]

Hi,

this patchset adds an soc node to the r8a7790 (H3) DT and moves all
nodes for IP blocks onto the bus to be sub nodes of the soc node.
This is consistent with handling of R-Car Gen3, RZ/G1, and
R-Car V2H (R8A77920) SoCs upstream. It is intended to migrate other R-Car
Gen2 SoCs to this scheme.

The patchset also fixes some minor whitespace problems and sorts the DT
nodes to improve maintainability.

Based on renesas-devel-20180116-v4.15-rc8.

As this patchset contains patches with large numbers of lines changed I
have also pushed it to the topic/r8a7790-soc-node branch of my renesas tree
on kernel.org.

Simon Horman (4):
  ARM: dts: r8a7790: consistently use single space after =
  ARM: r8a7790: Add soc node
  ARM: r8a7790: sort subnodes of soc node
  ARM: dts: r8a7790: sort subnodes of root node

 arch/arm/boot/dts/r8a7790.dtsi | 2839 +---
 1 file changed, 1456 insertions(+), 1383 deletions(-)

-- 
2.11.0

Re: [PATCH v5 9/9] arch: sh: migor: Use new renesas-ceu camera driver

[Hans Verkuil]

On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> Migo-R platform uses sh_mobile_ceu camera driver, which is now being
> replaced by a proper V4L2 camera driver named 'renesas-ceu'.
> 
> Move Migo-R platform to use the v4l2 renesas-ceu camera driver
> interface and get rid of soc_camera defined components used to register
> sensor drivers and of platform specific enable/disable routines.
> 
> Register clock source and GPIOs for sensor drivers, so they can use
> clock and gpio APIs.
> 
> Also, memory for CEU video buffers is now reserved with membocks APIs,
> and need to be declared as dma_coherent during machine initialization to
> remove that architecture specific part from CEU driver.
> 
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Laurent Pinchart 

Acked-by: Hans Verkuil 

Regards,

Hans

Re: [PATCH v5 8/9] media: i2c: tw9910: Remove soc_camera dependencies

[Hans Verkuil]

On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> Remove soc_camera framework dependencies from tw9910 sensor driver.
> - Handle clock and gpios
> - Register async subdevice
> - Remove soc_camera specific g/s_mbus_config operations
> - Add kernel doc to driver interface header file
> - Adjust build system
> 
> This commit does not remove the original soc_camera based driver as long
> as other platforms depends on soc_camera-based CEU driver.
> 
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Laurent Pinchart 

Acked-by: Hans Verkuil 

Regards,

Hans

Re: [PATCH v5 6/9] media: i2c: ov772x: Remove soc_camera dependencies

[Hans Verkuil]

On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> Remove soc_camera framework dependencies from ov772x sensor driver.
> - Handle clock and gpios
> - Register async subdevice
> - Remove soc_camera specific g/s_mbus_config operations
> - Change image format colorspace from JPEG to SRGB as the two use the
>   same colorspace information but JPEG makes assumptions on color
>   components quantization that do not apply to the sensor
> - Remove sizes crop from get_selection as driver can't scale
> - Add kernel doc to driver interface header file
> - Adjust build system
> 
> This commit does not remove the original soc_camera based driver as long
> as other platforms depends on soc_camera-based CEU driver.
> 
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Laurent Pinchart 
> ---
>  drivers/media/i2c/Kconfig  |  11 +++
>  drivers/media/i2c/Makefile |   1 +
>  drivers/media/i2c/ov772x.c | 177 
> ++---
>  include/media/i2c/ov772x.h |   6 +-
>  4 files changed, 133 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index cb5d7ff..a61d7f4 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -645,6 +645,17 @@ config VIDEO_OV5670
> To compile this driver as a module, choose M here: the
> module will be called ov5670.
>  
> +config VIDEO_OV772X
> + tristate "OmniVision OV772x sensor support"
> + depends on I2C && VIDEO_V4L2
> + depends on MEDIA_CAMERA_SUPPORT
> + ---help---
> +   This is a Video4Linux2 sensor-level driver for the OmniVision
> +   OV772x camera.
> +
> +   To compile this driver as a module, choose M here: the
> +   module will be called ov772x.
> +
>  config VIDEO_OV7640
>   tristate "OmniVision OV7640 sensor support"
>   depends on I2C && VIDEO_V4L2
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index 548a9ef..fb99293 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -66,6 +66,7 @@ obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
>  obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
>  obj-$(CONFIG_VIDEO_OV5670) += ov5670.o
>  obj-$(CONFIG_VIDEO_OV6650) += ov6650.o
> +obj-$(CONFIG_VIDEO_OV772X) += ov772x.o
>  obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
>  obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
>  obj-$(CONFIG_VIDEO_OV9650) += ov9650.o
> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
> index 8063835..df2516c 100644
> --- a/drivers/media/i2c/ov772x.c
> +++ b/drivers/media/i2c/ov772x.c
> @@ -1,6 +1,9 @@
> +// SPDX-License-Identifier: GPL-2.0
>  /*
>   * ov772x Camera Driver
>   *
> + * Copyright (C) 2017 Jacopo Mondi 
> + *
>   * Copyright (C) 2008 Renesas Solutions Corp.
>   * Kuninori Morimoto 
>   *
> @@ -9,27 +12,25 @@
>   * Copyright 2006-7 Jonathan Corbet 
>   * Copyright (C) 2008 Magnus Damm
>   * Copyright (C) 2008, Guennadi Liakhovetski 
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 as
> - * published by the Free Software Foundation.
>   */
>  
> +#include 
> +#include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
> -#include 
>  #include 
>  #include 
>  
>  #include 
> -#include 
> -#include 
> +
>  #include 
> -#include 
> +#include 
>  #include 
> +#include 
>  
>  /*
>   * register offset
> @@ -393,8 +394,10 @@ struct ov772x_win_size {
>  struct ov772x_priv {
>   struct v4l2_subdevsubdev;
>   struct v4l2_ctrl_handler  hdl;
> - struct v4l2_clk  *clk;
> + struct clk   *clk;
>   struct ov772x_camera_info*info;
> + struct gpio_desc *pwdn_gpio;
> + struct gpio_desc *rstb_gpio;
>   const struct ov772x_color_format *cfmt;
>   const struct ov772x_win_size *win;
>   unsigned shortflag_vflip:1;
> @@ -409,7 +412,7 @@ struct ov772x_priv {
>  static const struct ov772x_color_format ov772x_cfmts[] = {
>   {
>   .code   = MEDIA_BUS_FMT_YUYV8_2X8,
> - .colorspace = V4L2_COLORSPACE_JPEG,
> + .colorspace = V4L2_COLORSPACE_SRGB,
>   .dsp3   = 0x0,
>   .dsp4   = DSP_OFMT_YUV,
>   .com3   = SWAP_YUV,
> @@ -417,7 +420,7 @@ static const struct ov772x_color_format ov772x_cfmts[] = {
>   },
>   {
>   .code   = MEDIA_BUS_FMT_YVYU8_2X8,
> - .colorspace = V4L2_COLORSPACE_JPEG,
> + .colorspace = V4L2_COLORSPACE_SRGB,
>   .dsp3   = UV_ON,
>   .dsp4   = DSP_OFMT_YUV,
>   .com3   = SWAP_YUV,
> @@ -425,7 

Re: [PATCH v5 4/9] ARM: dts: r7s72100: Add Capture Engine Unit (CEU)

[Hans Verkuil]

On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> Add Capture Engine Unit (CEU) node to device tree.
> 
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Geert Uytterhoeven 
> Reviewed-by: Laurent Pinchart 

Acked-by: Hans Verkuil 

Regards,

Hans

> ---
>  arch/arm/boot/dts/r7s72100.dtsi | 15 ---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
> index ab9645a..5fe62f9 100644
> --- a/arch/arm/boot/dts/r7s72100.dtsi
> +++ b/arch/arm/boot/dts/r7s72100.dtsi
> @@ -135,9 +135,9 @@
>   #clock-cells = <1>;
>   compatible = "renesas,r7s72100-mstp-clocks", 
> "renesas,cpg-mstp-clocks";
>   reg = <0xfcfe042c 4>;
> - clocks = <_clk>;
> - clock-indices = ;
> - clock-output-names = "rtc";
> + clocks = <_clk>, <_clk>;
> + clock-indices = ;
> + clock-output-names = "ceu", "rtc";
>   };
>  
>   mstp7_clks: mstp7_clks@fcfe0430 {
> @@ -667,4 +667,13 @@
>   power-domains = <_clocks>;
>   status = "disabled";
>   };
> +
> + ceu: ceu@e821 {
> + reg = <0xe821 0x3000>;
> + compatible = "renesas,r7s72100-ceu";
> + interrupts = ;
> + clocks = <_clks R7S72100_CLK_CEU>;
> + power-domains = <_clocks>;
> + status = "disabled";
> + };
>  };
> 

Re: [PATCH v5 1/9] dt-bindings: media: Add Renesas CEU bindings

[Hans Verkuil]

On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> Add bindings documentation for Renesas Capture Engine Unit (CEU).
> 
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Rob Herring 
> Reviewed-by: Laurent Pinchart 

Acked-by: Hans Verkuil 

Regards,

Hans

> ---
>  .../devicetree/bindings/media/renesas,ceu.txt  | 81 
> ++
>  1 file changed, 81 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/renesas,ceu.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/renesas,ceu.txt 
> b/Documentation/devicetree/bindings/media/renesas,ceu.txt
> new file mode 100644
> index 000..590ee27
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/renesas,ceu.txt
> @@ -0,0 +1,81 @@
> +Renesas Capture Engine Unit (CEU)
> +--
> +
> +The Capture Engine Unit is the image capture interface found in the Renesas
> +SH Mobile and RZ SoCs.
> +
> +The interface supports a single parallel input with data bus width of 8 or 16
> +bits.
> +
> +Required properties:
> +- compatible: Shall be "renesas,r7s72100-ceu" for CEU units found in RZ/A1-H
> +  and RZ/A1-M SoCs.
> +- reg: Registers address base and size.
> +- interrupts: The interrupt specifier.
> +
> +The CEU supports a single parallel input and should contain a single 'port'
> +subnode with a single 'endpoint'. Connection to input devices are modeled
> +according to the video interfaces OF bindings specified in:
> +Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +Optional endpoint properties applicable to parallel input bus described in
> +the above mentioned "video-interfaces.txt" file are supported.
> +
> +- hsync-active: Active state of the HSYNC signal, 0/1 for LOW/HIGH 
> respectively.
> +  If property is not present, default is active high.
> +- vsync-active: Active state of the VSYNC signal, 0/1 for LOW/HIGH 
> respectively.
> +  If property is not present, default is active high.
> +
> +Example:
> +
> +The example describes the connection between the Capture Engine Unit and an
> +OV7670 image sensor connected to i2c1 interface.
> +
> +ceu: ceu@e821 {
> + reg = <0xe821 0x209c>;
> + compatible = "renesas,r7s72100-ceu";
> + interrupts = ;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_pins>;
> +
> + status = "okay";
> +
> + port {
> + ceu_in: endpoint {
> + remote-endpoint = <_out>;
> +
> + hsync-active = <1>;
> + vsync-active = <0>;
> + };
> + };
> +};
> +
> +i2c1: i2c@fcfee400 {
> + pinctrl-names = "default";
> + pinctrl-0 = <_pins>;
> +
> + status = "okay";
> +
> + clock-frequency = <10>;
> +
> + ov7670: camera@21 {
> + compatible = "ovti,ov7670";
> + reg = <0x21>;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_pins>;
> +
> + reset-gpios = < 11 GPIO_ACTIVE_LOW>;
> + powerdown-gpios = < 12 GPIO_ACTIVE_HIGH>;
> +
> + port {
> + ov7670_out: endpoint {
> + remote-endpoint = <_in>;
> +
> + hsync-active = <1>;
> + vsync-active = <0>;
> + };
> + };
> + };
> +};
> 

Re: [PATCH v5 7/9] v4l: i2c: Copy tw9910 soc_camera sensor driver

[Hans Verkuil]

On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> Copy the soc_camera based driver in v4l2 sensor driver directory.
> This commit just copies the original file without modifying it.
> No modification to KConfig and Makefile as soc_camera framework
> dependencies need to be removed first in next commit.
> 
> Signed-off-by: Jacopo Mondi 
> Acked-by: Laurent Pinchart 

Acked-by: Hans Verkuil 

Regards,

Hans

Re: [PATCH v5 2/9] include: media: Add Renesas CEU driver interface

[Hans Verkuil]

On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> Add renesas-ceu header file.
> 
> Do not remove the existing sh_mobile_ceu.h one as long as the original
> driver does not go away.
> 
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Laurent Pinchart 

Acked-by: Hans Verkuil 

Regards,

Hans

> ---
>  include/media/drv-intf/renesas-ceu.h | 26 ++
>  1 file changed, 26 insertions(+)
>  create mode 100644 include/media/drv-intf/renesas-ceu.h
> 
> diff --git a/include/media/drv-intf/renesas-ceu.h 
> b/include/media/drv-intf/renesas-ceu.h
> new file mode 100644
> index 000..52841d1
> --- /dev/null
> +++ b/include/media/drv-intf/renesas-ceu.h
> @@ -0,0 +1,26 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * renesas-ceu.h - Renesas CEU driver interface
> + *
> + * Copyright 2017-2018 Jacopo Mondi 
> + */
> +
> +#ifndef __MEDIA_DRV_INTF_RENESAS_CEU_H__
> +#define __MEDIA_DRV_INTF_RENESAS_CEU_H__
> +
> +#define CEU_MAX_SUBDEVS  2
> +
> +struct ceu_async_subdev {
> + unsigned long flags;
> + unsigned char bus_width;
> + unsigned char bus_shift;
> + unsigned int i2c_adapter_id;
> + unsigned int i2c_address;
> +};
> +
> +struct ceu_platform_data {
> + unsigned int num_subdevs;
> + struct ceu_async_subdev subdevs[CEU_MAX_SUBDEVS];
> +};
> +
> +#endif /* ___MEDIA_DRV_INTF_RENESAS_CEU_H__ */
> 

Re: [PATCH v5 5/9] v4l: i2c: Copy ov772x soc_camera sensor driver

[Hans Verkuil]

On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> Copy the soc_camera based driver in v4l2 sensor driver directory.
> This commit just copies the original file without modifying it.
> No modification to KConfig and Makefile as soc_camera framework
> dependencies need to be removed first in next commit.
> 
> Signed-off-by: Jacopo Mondi 
> Acked-by: Laurent Pinchart 

Acked-by: Hans Verkuil 

Regards,

Hans

Re: [PATCH v5 3/9] v4l: platform: Add Renesas CEU driver

[Hans Verkuil]

Hi Jacopo,

Sorry for the late review, but here is finally is.

BTW, can you provide the v4l2-compliance output (ideally with the -f option)
in the cover letter for v6?

On 01/12/2018 03:04 PM, Jacopo Mondi wrote:
> Add driver for Renesas Capture Engine Unit (CEU).
> 
> The CEU interface supports capturing 'data' (YUV422) and 'images'
> (NV[12|21|16|61]).
> 
> This driver aims to replace the soc_camera-based sh_mobile_ceu one.
> 
> Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ
> platform GR-Peach.
> 
> Tested with ov7725 camera sensor on SH4 platform Migo-R.
> 
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Laurent Pinchart 
> ---
>  drivers/media/platform/Kconfig   |9 +
>  drivers/media/platform/Makefile  |1 +
>  drivers/media/platform/renesas-ceu.c | 1648 
> ++
>  3 files changed, 1658 insertions(+)
>  create mode 100644 drivers/media/platform/renesas-ceu.c
> 
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index fd0c998..fe7bd26 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -144,6 +144,15 @@ config VIDEO_STM32_DCMI
> To compile this driver as a module, choose M here: the module
> will be called stm32-dcmi.
>  
> +config VIDEO_RENESAS_CEU
> + tristate "Renesas Capture Engine Unit (CEU) driver"
> + depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA
> + depends on ARCH_SHMOBILE || ARCH_R7S72100 || COMPILE_TEST
> + select VIDEOBUF2_DMA_CONTIG
> + select V4L2_FWNODE
> + ---help---
> +   This is a v4l2 driver for the Renesas CEU Interface
> +
>  source "drivers/media/platform/soc_camera/Kconfig"
>  source "drivers/media/platform/exynos4-is/Kconfig"
>  source "drivers/media/platform/am437x/Kconfig"
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index 003b0bb..6580a6b 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -62,6 +62,7 @@ obj-$(CONFIG_VIDEO_SH_VOU)  += sh_vou.o
>  obj-$(CONFIG_SOC_CAMERA) += soc_camera/
>  
>  obj-$(CONFIG_VIDEO_RCAR_DRIF)+= rcar_drif.o
> +obj-$(CONFIG_VIDEO_RENESAS_CEU)  += renesas-ceu.o
>  obj-$(CONFIG_VIDEO_RENESAS_FCP)  += rcar-fcp.o
>  obj-$(CONFIG_VIDEO_RENESAS_FDP1) += rcar_fdp1.o
>  obj-$(CONFIG_VIDEO_RENESAS_JPU)  += rcar_jpu.o
> diff --git a/drivers/media/platform/renesas-ceu.c 
> b/drivers/media/platform/renesas-ceu.c
> new file mode 100644
> index 000..ccca838
> --- /dev/null
> +++ b/drivers/media/platform/renesas-ceu.c



> +/*
> + * ceu_vb2_setup() - is called to check whether the driver can accept the
> + *requested number of buffers and to fill in plane sizes
> + *for the current frame format, if required.
> + */
> +static int ceu_vb2_setup(struct vb2_queue *vq, unsigned int *count,
> +  unsigned int *num_planes, unsigned int sizes[],
> +  struct device *alloc_devs[])
> +{
> + struct ceu_device *ceudev = vb2_get_drv_priv(vq);
> + struct v4l2_pix_format_mplane *pix = >v4l2_pix;
> + unsigned int i;
> +
> + if (!*count)
> + *count = 2;

Don't do this. Instead set the min_buffers_needed field to 2 in the vb2_queue
struct.

> +
> + /* num_planes is set: just check plane sizes. */
> + if (*num_planes) {
> + for (i = 0; i < pix->num_planes; i++)
> + if (sizes[i] < pix->plane_fmt[i].sizeimage)
> + return -EINVAL;
> +
> + return 0;
> + }
> +
> + /* num_planes not set: called from REQBUFS, just set plane sizes. */
> + *num_planes = pix->num_planes;
> + for (i = 0; i < pix->num_planes; i++)
> + sizes[i] = pix->plane_fmt[i].sizeimage;
> +
> + return 0;
> +}
> +
> +static void ceu_vb2_queue(struct vb2_buffer *vb)
> +{
> + struct ceu_device *ceudev = vb2_get_drv_priv(vb->vb2_queue);
> + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> + struct v4l2_pix_format_mplane *pix = >v4l2_pix;
> + struct ceu_buffer *buf = vb2_to_ceu(vbuf);
> + unsigned long irqflags;
> + unsigned int i;
> +
> + for (i = 0; i < pix->num_planes; i++) {
> + if (vb2_plane_size(vb, i) < pix->plane_fmt[i].sizeimage) {
> + vb2_buffer_done(vb, VB2_BUF_STATE_ERROR);
> + return;
> + }
> +
> + vb2_set_plane_payload(vb, i, pix->plane_fmt[i].sizeimage);

This is not the right vb2 op for this test, this belongs in the buf_prepare
op. There you can just return an error and you don't need to call buffer_done.

> + }
> +
> + spin_lock_irqsave(>lock, irqflags);
> + list_add_tail(>queue, >capture);
> + spin_unlock_irqrestore(>lock, irqflags);
> +}
> +
> +static int ceu_start_streaming(struct vb2_queue *vq, 

Re: [PATCH v2 19/22] mmc: tmio: ioremap memory resource in tmio_mmc_host_alloc()

[Wolfram Sang]

On Sat, Nov 25, 2017 at 01:24:54AM +0900, Masahiro Yamada wrote:
> The register region is ioremap'ed in the tmio_mmc_host_probe(), i.e.
> drivers cannot get access to the hardware before mmc_add_host().
> 
> Actually, renesas_sdhi_core.c reads out the CTL_VERSION register to
> complete the platform-specific settings.  However, at this point,
> the MMC host is already running.
> 
> Move the register ioremap to tmio_mmc_host_alloc() so that drivers
> can perform platform-specific settings between tmio_mmc_host_alloc()
> and tmio_mmc_host_probe().
> 
> I changed tmio_mmc_host_alloc() to return an error pointer to
> propagate the return code from devm_ioremap_resource().
> 
> Signed-off-by: Masahiro Yamada 
> ---

Reviewed-by: Wolfram Sang 



signature.asc
Description: PGP signature

Re: [PATCH v2 18/22] mmc: tmio: remove useless TMIO_MASK_CMD handling in tmio_mmc_host_probe()

[Wolfram Sang]

On Sat, Nov 25, 2017 at 01:24:53AM +0900, Masahiro Yamada wrote:
> TMIO_MASK_CMD is properly enabled in tmio_mmc_start_command().
> 
> We have no reason to set it up in tmio_mmc_host_probe().  (If we
> really wanted to set it in the probe, we would have to do likewise
> when resuming.)
> 
> Even worse, the following code is extremely confusing:
> 
>   _host->sdcard_irq_mask &= ~irq_mask;
> 
> The logic is opposite between "->sdcard_irq_mask" and "irq_mask".
> The intention is not clear at a glance.
> 
> Signed-off-by: Masahiro Yamada 

In general, I like it a lot. It just depends on the previous patches
which are still subject to discussion.



signature.asc
Description: PGP signature

Re: [PATCH] [LOCAL] arm64: renesas_defconfig: Use a default 128MB of CMA

[Simon Horman]

On Mon, Jan 15, 2018 at 12:59:16PM +, Kieran Bingham wrote:
> Our media tests frequently need large areas of CMA.
> Define the default allocation to be 128 MBytes to support
> larger use-cases.
> 
> Signed-off-by: Kieran Bingham 
> Acked-by: Laurent Pinchart 
> ---
> 
> v1.1
>  - Collected Laurent's Acked-by

Thanks, I have applied this to the topic/renesas-defconfig branch which is
included in the devel branch and tags of the Renesas tree as a convenience
to developers.  The branch is not, however, included in the next branch or
tags nor is it targeted at upstream.

Re: [PATCH 05/10] ARM: dts: porter: Fix HDMI output routing

[Simon Horman]

On Mon, Jan 15, 2018 at 10:25:34AM +0200, Laurent Pinchart wrote:
> Hi Simon,
> 
> On Monday, 15 January 2018 09:56:03 EET Simon Horman wrote:
> > On Fri, Jan 12, 2018 at 02:58:53AM +0200, Laurent Pinchart wrote:
> > > The HDMI encoder is connected to the RGB output of the DU, which is
> > > port@0, not port@1. Fix the incorrect DT description.
> > > 
> > > Signed-off-by: Laurent Pinchart
> > > 
> > 
> > Should this have the following tag?
> > 
> > Fixes: c5af8a4248d3 ("ARM: dts: porter: add DU DT support")
> 
> That would make sense, yes.
> 
> > If so, is it a fix to apply for v4.15 or a more regular patch
> > to apply for v4.16?
> 
> I believe we can wait for v4.16. HDMI output works on Porter at the
> moment due to a separate bug in the DU driver that clones outputs by
> default when it shouldn't. There's however no harm in backporting the
> patch to stable series if desired, but it's not required.

Thanks, I've applied this for v4.16 with the fixes tag above.
Possibly the fixes tag will result in some automated backporting occurring.
If so we can let that run its course, if not we can leave things be with
the option to backport later if we decide its necessary.

I have not applied the other dts patches in this series at this point.

Re: [PATCH v2 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Geert Uytterhoeven]

Hi Laurent,

On Mon, Jan 15, 2018 at 7:01 PM, Laurent Pinchart
 wrote:
> On Monday, 15 January 2018 19:09:53 EET Rob Herring wrote:
>> On Fri, Jan 12, 2018 at 5:14 PM, Laurent Pinchart wrote:
>> > The internal LVDS encoders now have their own DT bindings. Before
>> > switching the driver infrastructure to those new bindings, implement
>> > backward-compatibility through live DT patching.
>>
>> Uhh, we just got rid of TI's patching and now adding this one. I guess
>> it's necessary, but I'd like to know how long we need to keep this?
>
> That's a good question. How long are we supposed to keep DT backward
> compatibility for ? I don't think there's a one-size-fits-them-all answer to
> this question. Geert, any opinion ? How long do you plan to keep the CPG
> (clocks) DT backward compatibility for instance ?

Good question indeed...

It's not just CPG/MSSR. Over the years we also added or changed APMU (SMP),
SYSC (power domains), RST (mode pins), CMT (timers), ..., all of which have
some sort of compatibility support in the kernel.

Hence to avoid having to remember the kernel versions that dropped backwards
compatibility for each of the above components, I was thinking about an
R-Car Gen2 DT Flag Day.

However, that was before I learned about your plans for LVDS (it seems every
kernel release we learn about something new, postponing the flag day ;-).
And now I'm quite sure we'll have another change in the future (DU per
channel device nodes)...

About using DT fixups to implement backwards compatibility: I did my share
of thinking and experimenting with DT fixups (see e.g. "[PATCH/RFC 0/1]
soc: renesas: Add DT fixup code for backwards compatibility"
(https://www.spinics.net/lists/linux-renesas-soc/msg04305.html).
DT fixups are hard to implement right, and not everything can be done
that way.  Hence in the end, none of this was ever used upstream, and
everything was handled in C...

So I'm wondering if it would be easier if you would implement backwards
compatibility in C, using different compatible values for the new bindings?
I.e. switch from "renesas,du-r8a77*" to "renesas,r8a77*-du" +
"renesas,r8a77*-lvds"?
That way it also becomes very clear that there are old and new bindings,
and that there is backwards compatibility code for the old binding.

I know you're aware (the rest of the audience may not be) that the LVDS
part is not the only separate hardware block current unified in the single
DU node: each DU channel has its own hardware block.  Perhaps you can also
bite the bullet and have a single device node per DT channel, allowing
Runtime PM for DU channels?
Of course you have to tie channels together using a "companion" (cfr. USB)
or "renesas,bonding" (cfr. DRIF) property (unfortunately I learned about
the former only after the latter was already established).

Finally, implementing backwards compatibility support by DT fixup using
overlays may complicate backporting to LTSI kernels, due to the dependency
on DT overlays, which were reworked lately.

>> > --- a/drivers/gpu/drm/rcar-du/Kconfig
>> > +++ b/drivers/gpu/drm/rcar-du/Kconfig
>> > @@ -22,6 +22,8 @@ config DRM_RCAR_LVDS
>> > bool "R-Car DU LVDS Encoder Support"
>> > depends on DRM_RCAR_DU
>> > select DRM_PANEL
>> > +   select OF_FLATTREE
>> > +   select OF_OVERLAY
>>
>> OF_OVERLAY should probably select OF_FLATTREE. I guess in theory, we
>> could have an overlay from a non-FDT source...

Currently the select is needed for of_fdt_unflatten_tree() only, which is
not used by the core OF_OVERLAY code. So you could build an overlay in
memory yourself, and pass that, without using of_fdt_unflatten_tree().
But that is going to change if I read Frank's reponse well?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v2 17/22] mmc: tmio: move TMIO_MASK_{READOP,WRITEOP} handling to correct place

[Wolfram Sang]

On Sat, Nov 25, 2017 at 01:24:52AM +0900, Masahiro Yamada wrote:
> This driver was largely extended by Renesas, but actually used by
> several SoC vendors.  The current code does not work for UniPhier
> SoCs at least.

Do you insist on this paragraph? All people working on the code so far
tried hard to make it work on all devices they had access to. So far, I
can't recall one of the several SoC vendors contacting upstream to get
their support merged (until now, of course) or even to file bugs.
Because I can't guess unknown stuff, I'd prefer to skip this paragraph.

> The DMA mode for UniPhier SoCs failed with the following error message:
>   PIO IRQ in DMA mode!
> 
> For UniPhier SoCs, the TMIO_MASK_{READOP,WRITEOP} are asserted in the
> DMA mode as well.

Sure, that we need to fix! Note that both Renesas DMA drivers also
enable DATAEND and disable RXRDY | TXRQ on their own, too. So, probably
the same issue? And IIUC we can decide now to prepare the irqs like this
in tmio_core because all known DMA engines need it. Or we keep it that
DMA engines set up irqs themselves. More flexible but maybe
over-engineered?

> In fact, the code is very strange.

Yes.

> The TMIO_MASK_{READOP,WRITEOP} IRQs are set as follows:
> 
> /* Unmask the IRQs we want to know about */
> if (!_host->chan_rx)
> irq_mask |= TMIO_MASK_READOP;
> if (!_host->chan_tx)
> irq_mask |= TMIO_MASK_WRITEOP;
> 
> At this point, _host->{chan_rx,chan_tx} are _always_ NULL because
> tmio_mmc_request_dma() is called after this code.  Consequently,
> TMIO_MASK_{READOP,WRITEOP} are set whether DMA is used or not.

Yes :( Bummer.

> tmio_mmc_cmd_irq() enables TMIO_MASK_{READOP,WRITEOP}, but never
> disables them.  This does not take care of a case where ->force_pio
> is set, but unset later.

force_pio gets disabled within the same request? I may be overlooking
something but I only see code paths where force_pio gets cleared and a
little later the request gets finished. Can you give me a pointer?

> After all, the correct place to handle those flags is just before
> starting the data transfer.

While I totally agree the code below can be improved for sure, I'd
prefer to keep the design pattern that irqs get disabled once they are
not actively used. Generally spoken, I think it makes sense to keep
regressions on old platforms low. Any chance this can be achieved?
Other than that, fixing/removing this irq_mask handling from probe() is
really good.

> 
> Signed-off-by: Masahiro Yamada 
> ---
> 
> Changes in v2:
>   - Newly added
> 
>  drivers/mmc/host/tmio_mmc_core.c | 18 --
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mmc/host/tmio_mmc_core.c 
> b/drivers/mmc/host/tmio_mmc_core.c
> index 7d169ed..345e379 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -621,15 +621,19 @@ static void tmio_mmc_cmd_irq(struct tmio_mmc_host 
> *host, unsigned int stat)
>*/
>   if (host->data && (!cmd->error || cmd->error == -EILSEQ)) {
>   if (host->data->flags & MMC_DATA_READ) {
> - if (host->force_pio || !host->chan_rx)
> + if (host->force_pio || !host->chan_rx) {
>   tmio_mmc_enable_mmc_irqs(host, 
> TMIO_MASK_READOP);
> - else
> + } else {
> + tmio_mmc_disable_mmc_irqs(host, 
> TMIO_MASK_READOP);
>   tasklet_schedule(>dma_issue);
> + }
>   } else {
> - if (host->force_pio || !host->chan_tx)
> + if (host->force_pio || !host->chan_tx) {
>   tmio_mmc_enable_mmc_irqs(host, 
> TMIO_MASK_WRITEOP);
> - else
> + } else {
> + tmio_mmc_disable_mmc_irqs(host, 
> TMIO_MASK_WRITEOP);
>   tasklet_schedule(>dma_issue);
> + }
>   }
>   } else {
>   schedule_work(>done);
> @@ -1285,12 +1289,6 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
>   _host->sdcard_irq_mask = sd_ctrl_read16_and_16_as_32(_host, 
> CTL_IRQ_MASK);
>   tmio_mmc_disable_mmc_irqs(_host, TMIO_MASK_ALL);
>  
> - /* Unmask the IRQs we want to know about */
> - if (!_host->chan_rx)
> - irq_mask |= TMIO_MASK_READOP;
> - if (!_host->chan_tx)
> - irq_mask |= TMIO_MASK_WRITEOP;
> -
>   _host->sdcard_irq_mask &= ~irq_mask;
>  
>   if (_host->native_hotplug)
> -- 
> 2.7.4
> 


signature.asc
Description: PGP signature